Sweep the definition, not the filled values, and judge each field alike at check and composition (issue 231)

The first sweep ran at composition over the resource after settings, bindings and
machine facts were filled, so an operator's value such as ${labels:instance} was
refused as a misspelling its author never wrote, and the whole machine got nothing.
Both points now sweep the resource as the manifest wrote it, against one table of
which fields each pass fills, so the check and composition refuse the same things.
Any ${<known namespace>: its pattern does not take is refused whatever its case or key.

Issue 231's undeclared-setting half is not met here: refusing a key the module does
not declare (ADR 0164) is not built and is handed to issue 398.

internal/catalogue/declaration.go

@@ -911,6 +911,15 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 				return nil, err
 			}
 		}
+		// **And every placeholder no pass fills where it stands is refused** (novox/hq issue 231):
+		// judged here, over the definition as written and before any pass, by the function the
+		// catalogue check runs — so a manifest registered by an older binary is judged too, and
+		// what a setting or a binding later puts into a file is its software's text, never swept.
+		for _, own := range m.Resources {
+			if problems := unconsumedPlaceholders(m.Module, own); len(problems) > 0 {
+				return nil, fmt.Errorf("%s", problems[0])
+			}
+		}
 
 		// Which of this module's files carry a secret, for the rule that a container may not read
 		// one of them as its environment without saying so (ADR 0086, issue 041).

internal/catalogue/environment_into.go

@@ -226,9 +226,9 @@ func (m Manifest) contributionPlaceholderProblems() []string {
 	for _, r := range m.Resources {
 		problems = append(problems, placeholderProblems(m, r)...)
 		problems = append(problems, seatPlaceholderProblems(m, r)...)
-		// And whatever is left once every pass's own placeholders are set aside: a misspelt
+		// And every placeholder no pass fills where it stands: a misspelt namespace, a key its
-		// namespace, or a key its namespace cannot take (novox/hq issue 231).
+		// namespace cannot take, or a field its pass does not read (novox/hq issue 231).
-		problems = append(problems, unconsumedPlaceholders(m.Module, r, filledByAPass, nil)...)
+		problems = append(problems, unconsumedPlaceholders(m.Module, r)...)
 	}
 	return problems
 }
@@ -551,13 +551,7 @@ func shellCode(modules []Manifest, shell, slot string) string {
 // is filled before the shell's code is, and each is replaced in a single pass over what the holder
 // wrote, so a contributed piece is never scanned again.
 func contributionsInto(resource map[string]any, m Manifest, modules []Manifest, facts map[string]string, with Rendering, caps map[string]bool, unplaced *[]string) error {
-	// **The sweep stands here, after every other pass and before any contributed text is placed**
+	if problems := append(placeholderProblems(m, resource), seatPlaceholderProblems(m, resource)...); len(problems) > 0 {
-	// (novox/hq issue 231): what is left that the mesh did not fill would reach the machine as text,
-	// and contributed shell code, placed below, is the shell's and is never swept (ADR 0204). The
-	// same function the catalogue check runs, over what the passes left.
-	problems := append(placeholderProblems(m, resource), seatPlaceholderProblems(m, resource)...)
-	problems = append(problems, unconsumedPlaceholders(m.Module, resource, nil, leftForLater)...)
-	if len(problems) > 0 {
 		return fmt.Errorf("%s", problems[0])
 	}
 	content, ok := resource["content"].(string)

internal/catalogue/unconsumed_placeholder.go

@@ -8,85 +8,161 @@ import (
 
 // A placeholder no pass consumes is refused, never written out as text (novox/hq issue 231).
 //
-// Each namespace is filled by its own pass with its own pattern, and a word in that shape that no
+// Each namespace is filled by its own pass with its own pattern, in the fields that pass reads. A
-// pattern matches — `${machnie:address}`, `${shel:zsh:first}`, a setting key no definition could
+// word in that shape that no pattern matched — `${machnie:address}`, `${shel:zsh:first}`, a setting
-// declare such as `${setting:Undeclared}` — was left in the file as it was written, and reached a
+// key no definition could declare such as `${setting:Undeclared}` — was left in the file as it was
-// machine as a value. That is the predecessor's failure the namespaced placeholders were meant to end
+// written and reached a machine as a value: the predecessor's failure the namespaced placeholders
-// (novox/hq ADR 0164). So after every pass, what is left is swept: a `${<word>:<key>}` whose word is
+// were meant to end (novox/hq ADR 0164). So the definition is swept, field by field, against the
-// a lower-case token and whose key begins with a letter or a digit is the mesh's shape, and nothing
+// same table of what each pass fills where.
-// the mesh could still fill.
+//
+// **The definition, never the values.** Swept as the manifest wrote it, at the catalogue check and
+// again at composition before any pass has run: what an operator's setting, a binding or a merged
+// JSON setting puts into a file is its own software's text — `${env:HOME}` for Log4j, `${timeout:30}`
+// for Spring — and refusing it there would refuse something its author never wrote, on a machine the
+// check had passed (novox/hq issue 231, review of mesh-controller #211).
+//
+// What is refused:
+//   - a placeholder of a namespace the mesh knows, in a field that namespace's pass does not read: it
+//     would reach the machine as the same text;
+//   - any other `${<known namespace>:`, case-insensitively and whatever follows — `${Machine:address}`,
+//     `${machine:.address}`, `${machine: address}`, an unclosed `${machine:address` — because no
+//     pass's pattern takes it;
+//   - a namespace-shaped placeholder of a word the mesh does not know: a lower-case word, a colon, and
+//     a key that begins with a letter or a digit and holds no space, brace or `$`.
 //
 // **The shell's own syntax is not that shape, and passes.** `${NAME:-…}` has an upper-case word,
 // `${(%):-…}` and `${1:-.}` begin with no letter, and a lower-case variable with an operator after its
-// colon — `${count:-}`, `${trial:+…}`, `${state:=…}` — has no key that begins with a letter or a
+// colon — `${count:-}`, `${trial:+…}`, `${state:=…}` — has no key that begins with a letter or a digit.
-// digit; nor does an expansion that holds a space, a brace or a `$`. What the shape does catch is a
+// What the shape does catch is a zsh modifier (`${path:t}`) or a substring (`${where:0:12}`) in a
-// zsh modifier (`${path:t}`) or a substring (`${where:0:12}`) in a resource's own text: shell code of
+// resource's own text: shell code of that kind belongs in the module's contributed shell code, which
-// that kind belongs in the module's contributed shell code, which no pass reads (novox/hq ADR 0204)
+// is not a resource and is never swept (novox/hq ADR 0204).
-// and which this sweep never sees, because it runs before that code is placed.
+
+// filler is one namespace's pass: the patterns it fills with, and the fields it reads them in, by
+// resource type ("*" for any type).
+type filler struct {
+	namespace string
+	patterns  []*regexp.Regexp
+	fields    map[string][]string
+	// why, when set, is the rule that keeps the namespace out of every other field, said with a
+	// refusal of one written there.
+	why string
+}
+
+// fillers is the table of what each pass fills where — read from the passes themselves: settingInto,
+// dirInto, accessInto, intoFile (whose ${secret:…} the node-engine fills), boundInto, portInto,
+// seatInto, machineInto and contributionsInto. A pass that comes to read another field adds it here,
+// or the sweep refuses the placeholder it would have filled.
+var fillers = []filler{
+	{namespace: "setting", patterns: []*regexp.Regexp{settingRef}, fields: map[string][]string{"file": {"content"}, "service": {"unit"}}},
+	{namespace: "dir", patterns: []*regexp.Regexp{dirRef}, fields: map[string][]string{"*": {"path", "content", "volumes", "env", "env-file"}}},
+	{namespace: "access", patterns: []*regexp.Regexp{accessRef}, fields: map[string][]string{"*": {"path", "content", "volumes", "env", "env-file"}}},
+	{namespace: "secret", patterns: []*regexp.Regexp{placeholder}, fields: map[string][]string{"file": {"content"}},
+		why: "a secret is never filled into an environment variable or any other field: put it in a file the " +
+			"module declares and mount that (novox/hq ADR 0086)"},
+	{namespace: "bound", patterns: []*regexp.Regexp{bound}, fields: map[string][]string{"file": {"content"}}},
+	{namespace: "port", patterns: []*regexp.Regexp{ofPort}, fields: map[string][]string{"file": {"content"}, "container": {"env"}, "process": {"env"}}},
+	{namespace: "seat", patterns: []*regexp.Regexp{ofSeat, ofSeatReach}, fields: map[string][]string{"file": {"content"}, "container": {"env"}, "process": {"env"}}},
+	{namespace: "machine", patterns: []*regexp.Regexp{ofMachine}, fields: map[string][]string{"*": {"path", "owner", "content", "name", "user", "root", "home"}}},
+	// Placed in a file's content by their holders, and judged there by their own rules
+	// (placeholderProblems, seatPlaceholderProblems).
+	{namespace: "environment", patterns: []*regexp.Regexp{ofEnvironment}, fields: map[string][]string{"*": {"content"}}},
+	{namespace: "shell", patterns: []*regexp.Regexp{ofShell}, fields: map[string][]string{"*": {"content"}}},
+	{namespace: "contribution", patterns: []*regexp.Regexp{ofContribution}, fields: map[string][]string{"*": {"content"}}},
+	// Filled in what a provider serves (consumer_into_serves.go), never in a resource.
+	{namespace: "consumer", patterns: []*regexp.Regexp{consumerFact}},
+}
+
+// reads is whether this pass fills a field of a resource of this type.
+func (f filler) reads(kind, field string) bool {
+	return oneOf(f.fields[kind], field) || oneOf(f.fields["*"], field)
+}
+
+// ofKnownNamespace is any `${<known namespace>:` and what follows it up to its brace or the end of
+// its line, whatever its case: what remains of one once every pass's pattern is set aside is a
+// misspelling.
+var ofKnownNamespace = regexp.MustCompile(`(?i)\$\{(?:` + strings.Join(namespacesOf(fillers), "|") + `):[^}\n]*\}?`)
 
 // namespaceShaped is a placeholder in the mesh's shape: a lower-case word, a colon, and a key that
 // begins with a letter or a digit and holds no space, brace or `$`.
 var namespaceShaped = regexp.MustCompile(`\$\{[a-z][a-z0-9_-]*:[A-Za-z0-9][^\s{}$]*\}`)
 
-// filledByAPass is every placeholder a pass over a resource consumes, each by the pattern that pass
+func namespacesOf(fs []filler) []string {
-// fills with. Judged at the catalogue check, before any of them has run.
+	out := make([]string, len(fs))
-var filledByAPass = []*regexp.Regexp{
+	for i, f := range fs {
-	settingRef, dirRef, accessRef, placeholder, bound, ofPort, ofSeat, ofSeatReach, ofMachine,
+		out[i] = f.namespace
-	ofEnvironment, ofShell, ofContribution,
+	}
+	return out
 }
 
-// leftForLater is what composition leaves in a file's content when the sweep runs, on purpose: a
+// unconsumedPlaceholders is every placeholder in one resource of a definition that no pass fills
-// secret the node-engine fills from what it alone decrypts (ADR 0086), and the environment, the
+// where it stands, each named with the module, the resource, the field and the token. The same
-// shell's code and the seats' contributions, which are placed after the sweep so that contributed
+// function at the catalogue check and at composition, over the resource as the manifest wrote it.
-// text is never swept (ADR 0203, ADR 0204, ADR 0212). Each is judged by its own rules
+func unconsumedPlaceholders(module string, r map[string]any) []string {
-// (placeholderProblems, seatPlaceholderProblems); anything else still standing was consumed by no
+	kind := fmt.Sprint(r["type"])
-// pass, in whatever field.
-var leftForLater = []*regexp.Regexp{placeholder, ofEnvironment, ofShell, ofContribution}
-
-// unconsumedPlaceholders is every namespace-shaped placeholder in one resource that none of the
-// given patterns accounts for, named with the module, the resource and the field it stands in.
-// `anywhere` are accounted for in any field; `inContent` only in a file's content.
-func unconsumedPlaceholders(module string, r map[string]any, anywhere, inContent []*regexp.Regexp) []string {
 	var problems []string
-	var sweep func(field string, v any)
+	var sweep func(top, field string, v any)
-	sweep = func(field string, v any) {
+	sweep = func(top, field string, v any) {
 		switch v := v.(type) {
 		case string:
 			rest := v
-			for _, p := range anywhere {
+			for _, f := range fillers {
-				rest = p.ReplaceAllString(rest, "")
+				for _, p := range f.patterns {
-			}
+					if !f.reads(kind, top) {
-			if field == "content" {
+						for _, token := range p.FindAllString(rest, -1) {
-				for _, p := range inContent {
+							problems = append(problems, fmt.Sprintf(
+								"%s's resource %v holds %s in its %s, and ${%s:…} is not filled in this field: "+
+									"it would reach the machine as that text (novox/hq issue 231)%s",
+								module, r["id"], token, field, f.namespace, whereFilled(f)))
+						}
+					}
 					rest = p.ReplaceAllString(rest, "")
 				}
 			}
+			for _, token := range ofKnownNamespace.FindAllString(rest, -1) {
+				problems = append(problems, fmt.Sprintf(
+					"%s's resource %v holds %s in its %s, which is no placeholder the mesh fills: a "+
+						"misspelt key, or a namespace in the wrong case, would reach the machine as that "+
+						"text (novox/hq issue 231)", module, r["id"], token, field))
+			}
+			rest = ofKnownNamespace.ReplaceAllString(rest, "")
 			for _, token := range namespaceShaped.FindAllString(rest, -1) {
 				problems = append(problems, fmt.Sprintf(
 					"%s's resource %v holds %s in its %s, and no pass of the mesh fills it: a misspelt "+
-						"placeholder, or a key its namespace cannot take, would reach the machine as that "+
+						"placeholder would reach the machine as that text (novox/hq issue 231). The mesh "+
-						"text (novox/hq issue 231). The mesh fills %s; the shell's own syntax belongs in "+
+						"fills ${%s:…}; the shell's own syntax belongs in the module's shell code (ADR 0204)",
-						"the module's shell code (ADR 0204)",
+					module, r["id"], token, field, strings.Join(namespacesOf(fillers), ":…}, ${")))
-					module, r["id"], token, field, strings.Join(placeholderNamespaces, ", ")))
 			}
 		case []any:
 			for i, e := range v {
-				sweep(fmt.Sprintf("%s[%d]", field, i), e)
+				sweep(top, fmt.Sprintf("%s[%d]", field, i), e)
 			}
 		case map[string]any:
 			for _, k := range sortedKeys(v) {
-				sweep(field+"."+k, v[k])
+				sweep(top, field+"."+k, v[k])
 			}
 		}
 	}
 	for _, k := range sortedKeys(r) {
-		sweep(k, r[k])
+		sweep(k, k, r[k])
 	}
 	return problems
 }
 
-// placeholderNamespaces is what a refusal lists as the namespaces the mesh fills in a resource.
+// whereFilled says where a namespace's pass does fill, for a refusal of one written elsewhere.
-var placeholderNamespaces = []string{
+func whereFilled(f filler) string {
-	"${access:…}", "${bound:…}", "${contribution:…}", "${dir:…}", "${environment:…}", "${machine:…}",
+	if f.why != "" {
-	"${port:…}", "${seat:…}", "${secret:…}", "${setting:…}", "${shell:…}",
+		return ". " + strings.ToUpper(f.why[:1]) + f.why[1:]
+	}
+	if len(f.fields) == 0 {
+		return "; it is filled only in what a provider serves"
+	}
+	var where []string
+	for _, kind := range sortedKeys(f.fields) {
+		of := "a " + kind + "'s"
+		if kind == "*" {
+			of = "any resource's"
+		}
+		where = append(where, of+" "+strings.Join(f.fields[kind], ", "))
+	}
+	return "; it is filled in " + strings.Join(where, "; ")
 }

internal/catalogue/unconsumed_placeholder_test.go

@@ -36,6 +36,22 @@ func TestAMisspelledPlaceholderIsRefusedAtTheCheck(t *testing.T) {
 		t.Errorf("the shell's own syntax was refused: %v", err)
 	}
 
+	// A namespace the mesh knows, misspelt in any way its own pattern does not take, and the same
+	// namespace in a field its pass does not read: refused at the check as at composition.
+	for _, c := range []struct{ resource, token, field string }{
+		{`{"id":"rc","type":"file","path":"/etc/rc","content":"${Machine:address}"}`, "${Machine:address}", "content"},
+		{`{"id":"rc","type":"file","path":"/etc/rc","content":"${machine:.address}"}`, "${machine:.address}", "content"},
+		{`{"id":"rc","type":"file","path":"/etc/rc","content":"${machine: address}"}`, "${machine: address}", "content"},
+		{`{"id":"rc","type":"file","path":"/etc/rc","content":"a=${machine:address\nb=1"}`, "${machine:address", "content"},
+		{`{"id":"rc","type":"process","name":"speller","env":{"NAME":"${machine:name}"}}`, "${machine:name}", "env.NAME"},
+	} {
+		raw := `{"module":"speller","version":"1","resources":[` + c.resource + `]}`
+		_, err := ParseManifest([]byte(raw))
+		if err == nil || !strings.Contains(err.Error(), "speller's resource rc holds "+c.token+" in its "+c.field) {
+			t.Errorf("%s in its %s was accepted at the check, or not named: %v", c.token, c.field, err)
+		}
+	}
+
 	// And the shell's syntax alone, beside placeholders every pass knows, is accepted — as is the
 	// shape a lower-case shell variable takes with an operator after its colon.
 	raw = `{"module":"speller","version":"1","resources":[{"id":"rc","type":"file","path":"${machine:account-home}/.rc",` +
@@ -58,6 +74,10 @@ func TestAMisspelledPlaceholderIsRefusedAtComposition(t *testing.T) {
 		{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "b=" + undeclaredSetting + "\n"}, undeclaredSetting},
 		{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "c=" + misspelledMachine + "\n"}, misspelledMachine},
 		{"env.NAME", map[string]any{"id": "rc", "type": "process", "name": "speller", "env": map[string]any{"NAME": "${machine:name}"}}, "${machine:name}"},
+		{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${Machine:address}"}, "${Machine:address}"},
+		{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${machine:.address}"}, "${machine:.address}"},
+		{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "${machine: address}"}, "${machine: address}"},
+		{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "a=${machine:address\nb=1"}, "${machine:address"},
 	} {
 		m := Manifest{Module: "speller", Version: "1", Resources: []map[string]any{c.r}}
 		r := Resolution{Node: "workstation", Account: "op", Modules: []Manifest{m}}
@@ -74,11 +94,46 @@ func TestAMisspelledPlaceholderIsRefusedAtComposition(t *testing.T) {
 	if err != nil {
 		t.Fatalf("the shell's own syntax was refused: %v", err)
 	}
+	if got := contentOf(t, out, "speller.rc"); got != "d="+shellsOwn+"\n" {
+		t.Fatalf("the shell's own syntax did not pass through as written: %q", got)
+	}
+}
+
+// contentOf is the content of the composed resource with this id, failing when it was not composed.
+func contentOf(t *testing.T, out []map[string]any, id string) string {
+	t.Helper()
 	for _, res := range out {
-		if res["id"] == "speller.rc" && res["content"] != "d="+shellsOwn+"\n" {
+		if res["id"] == id {
-			t.Fatalf("the shell's own syntax did not pass through as written: %q", res["content"])
+			return plainly(res["content"])
 		}
 	}
+	t.Fatalf("%s was not composed: %v", id, out)
+	return ""
+}
+
+// What a value puts into a file is its software's text, not the definition's, and is never swept
+// (review of mesh-controller #211): an operator's setting holding `${labels:instance}` filled through
+// ${setting:…}, and a JSON setting `${level:upper}` merged into a mergeable file, reach the machine as
+// set — software that templates its own configuration (Log4j's `${env:…}`, Spring's `${timeout:30}`)
+// is configured exactly this way.
+func TestAValueHoldingAPlaceholderShapeComposesUnchanged(t *testing.T) {
+	m := Manifest{Module: "templater", Version: "1", Resources: []map[string]any{
+		{"id": "conf", "type": "file", "path": "/etc/templater.conf", "content": "template=${setting:template}\n"},
+		{"id": "json", "type": "file", "path": "/etc/templater.json", "merge": MergeJSON, "content": `{"fmt":"plain"}`},
+	}}
+	settings := SettingsBy{"templater": {{From: "the operator", Values: map[string]any{
+		"template": "${labels:instance}", "fmt": "${level:upper}",
+	}}}}
+	out, err := Resolution{Node: "workstation", Account: "op", Modules: []Manifest{m}}.Declaration(Rendering{Settings: settings})
+	if err != nil {
+		t.Fatalf("a value holding a placeholder's shape was refused as the definition's: %v", err)
+	}
+	if got := contentOf(t, out, "templater.conf"); got != "template=${labels:instance}\n" {
+		t.Errorf("the setting did not reach the file as set: %q", got)
+	}
+	if got := contentOf(t, out, "templater.json"); !strings.Contains(got, `${level:upper}`) {
+		t.Errorf("the merged setting did not reach the file as set: %q", got)
+	}
 }
 
 // Contributed shell code is the shell's, and no pass reads it (novox/hq ADR 0204): zsh's own
@@ -91,9 +146,7 @@ func TestContributedShellCodeIsNotSwept(t *testing.T) {
 	if err != nil {
 		t.Fatalf("contributed shell code was swept: %v", err)
 	}
-	for _, res := range out {
+	if got := contentOf(t, out, "zsh.zshrc"); !strings.Contains(got, "echo ${path:t} ${shel:zsh:first}") {
-		if res["id"] == "zsh.zshrc" && !strings.Contains(res["content"].(string), "echo ${path:t} ${shel:zsh:first}") {
+		t.Fatalf("the contributed code did not reach the holder's file as written: %q", got)
-			t.Fatalf("the contributed code did not reach the holder's file as written: %q", res["content"])
-		}
 	}
 }