diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 4b191182..ff5be865 100644 --- a/internal/catalogue/declaration.go +++ b/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). diff --git a/internal/catalogue/environment_into.go b/internal/catalogue/environment_into.go index e5cb3d9b..ed2bbe28 100644 --- a/internal/catalogue/environment_into.go +++ b/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 - // namespace, or a key its namespace cannot take (novox/hq issue 231). - problems = append(problems, unconsumedPlaceholders(m.Module, r, filledByAPass, nil)...) + // And every placeholder no pass fills where it stands: a misspelt namespace, a key its + // namespace cannot take, or a field its pass does not read (novox/hq issue 231). + 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** - // (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 { + if problems := append(placeholderProblems(m, resource), seatPlaceholderProblems(m, resource)...); len(problems) > 0 { return fmt.Errorf("%s", problems[0]) } content, ok := resource["content"].(string) diff --git a/internal/catalogue/unconsumed_placeholder.go b/internal/catalogue/unconsumed_placeholder.go index 556096c4..32dc95e7 100644 --- a/internal/catalogue/unconsumed_placeholder.go +++ b/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 -// pattern matches — `${machnie:address}`, `${shel:zsh:first}`, a setting key no definition could -// declare such as `${setting:Undeclared}` — was left in the file as it was written, and reached a -// machine as a value. That is the predecessor's failure the namespaced placeholders were meant to end -// (novox/hq ADR 0164). So after every pass, what is left is swept: a `${:}` whose word is -// a lower-case token and whose key begins with a letter or a digit is the mesh's shape, and nothing -// the mesh could still fill. +// Each namespace is filled by its own pass with its own pattern, in the fields that pass reads. A +// word in that shape that no pattern matched — `${machnie:address}`, `${shel:zsh:first}`, a setting +// key no definition could declare such as `${setting:Undeclared}` — was left in the file as it was +// written and reached a machine as a value: the predecessor's failure the namespaced placeholders +// were meant to end (novox/hq ADR 0164). So the definition is swept, field by field, against the +// same table of what each pass fills where. +// +// **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 `${:`, 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 -// digit; nor does an expansion that holds a space, a brace or a `$`. What the shape does catch is a -// zsh modifier (`${path:t}`) or a substring (`${where:0:12}`) in a resource's own text: shell code of -// that kind belongs in the module's contributed shell code, which no pass reads (novox/hq ADR 0204) -// and which this sweep never sees, because it runs before that code is placed. +// colon — `${count:-}`, `${trial:+…}`, `${state:=…}` — has no key that begins with a letter or a digit. +// What the shape does catch is a zsh modifier (`${path:t}`) or a substring (`${where:0:12}`) in a +// resource's own text: shell code of that kind belongs in the module's contributed shell code, which +// is not a resource and is never swept (novox/hq ADR 0204). + +// 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 `${:` 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 -// fills with. Judged at the catalogue check, before any of them has run. -var filledByAPass = []*regexp.Regexp{ - settingRef, dirRef, accessRef, placeholder, bound, ofPort, ofSeat, ofSeatReach, ofMachine, - ofEnvironment, ofShell, ofContribution, +func namespacesOf(fs []filler) []string { + out := make([]string, len(fs)) + for i, f := range fs { + out[i] = f.namespace + } + return out } -// leftForLater is what composition leaves in a file's content when the sweep runs, on purpose: a -// secret the node-engine fills from what it alone decrypts (ADR 0086), and the environment, the -// shell's code and the seats' contributions, which are placed after the sweep so that contributed -// text is never swept (ADR 0203, ADR 0204, ADR 0212). Each is judged by its own rules -// (placeholderProblems, seatPlaceholderProblems); anything else still standing was consumed by no -// 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 { +// unconsumedPlaceholders is every placeholder in one resource of a definition that no pass fills +// where it stands, each named with the module, the resource, the field and the token. The same +// function at the catalogue check and at composition, over the resource as the manifest wrote it. +func unconsumedPlaceholders(module string, r map[string]any) []string { + kind := fmt.Sprint(r["type"]) var problems []string - var sweep func(field string, v any) - sweep = func(field string, v any) { + var sweep func(top, field string, v any) + sweep = func(top, field string, v any) { switch v := v.(type) { case string: rest := v - for _, p := range anywhere { - rest = p.ReplaceAllString(rest, "") - } - if field == "content" { - for _, p := range inContent { + for _, f := range fillers { + for _, p := range f.patterns { + if !f.reads(kind, top) { + for _, token := range p.FindAllString(rest, -1) { + 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 "+ - "text (novox/hq issue 231). The mesh fills %s; the shell's own syntax belongs in "+ - "the module's shell code (ADR 0204)", - module, r["id"], token, field, strings.Join(placeholderNamespaces, ", "))) + "placeholder would reach the machine as that text (novox/hq issue 231). The mesh "+ + "fills ${%s:…}; the shell's own syntax belongs in the module's shell code (ADR 0204)", + module, r["id"], token, field, strings.Join(namespacesOf(fillers), ":…}, ${"))) } 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. -var placeholderNamespaces = []string{ - "${access:…}", "${bound:…}", "${contribution:…}", "${dir:…}", "${environment:…}", "${machine:…}", - "${port:…}", "${seat:…}", "${secret:…}", "${setting:…}", "${shell:…}", +// whereFilled says where a namespace's pass does fill, for a refusal of one written elsewhere. +func whereFilled(f filler) string { + if f.why != "" { + 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, "; ") } diff --git a/internal/catalogue/unconsumed_placeholder_test.go b/internal/catalogue/unconsumed_placeholder_test.go index 4eb6ca2c..fdc34d4d 100644 --- a/internal/catalogue/unconsumed_placeholder_test.go +++ b/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" { - t.Fatalf("the shell's own syntax did not pass through as written: %q", res["content"]) + if res["id"] == id { + 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 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", res["content"]) - } + if got := contentOf(t, out, "zsh.zshrc"); !strings.Contains(got, "echo ${path:t} ${shel:zsh:first}") { + t.Fatalf("the contributed code did not reach the holder's file as written: %q", got) } }