Merge pull request 'A misspelled placeholder is refused, never written out as text (issue 231)' (#211) from fix/231-a-misspelled-placeholder-is-refused into main
This commit was merged in pull request #211.
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -226,6 +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 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
|
||||
}
|
||||
|
||||
@@ -0,0 +1,172 @@
|
||||
package catalogue
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// 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, 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 `${<known namespace>:`, case-insensitively, unless a shell's parameter operator follows
|
||||
// its colon — `${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.
|
||||
// And an operator — `:-`, `:=`, `:+`, `:?` — after a word the mesh also uses is the shell's too:
|
||||
// `${PORT:-8080}`, `${SHELL:-/bin/sh}`, `${SECRET:?unset}` and `${dir:-/tmp}` are among the commonest
|
||||
// lines of a script or an env file, and pass whatever the word's case.
|
||||
// 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 `${<known namespace>:` and what follows it up to its brace or the end of
|
||||
// its line, whatever its case, unless what follows the colon is a shell's parameter operator (`-`,
|
||||
// `=`, `+`, `?`): what remains of one once every pass's pattern is set aside is a misspelling.
|
||||
var ofKnownNamespace = regexp.MustCompile(`(?im)\$\{(?:` + strings.Join(namespacesOf(fillers), "|") +
|
||||
`):(?:[^-=+?}\n][^}\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{}$]*\}`)
|
||||
|
||||
func namespacesOf(fs []filler) []string {
|
||||
out := make([]string, len(fs))
|
||||
for i, f := range fs {
|
||||
out[i] = f.namespace
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// 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(top, field string, v any)
|
||||
sweep = func(top, field string, v any) {
|
||||
switch v := v.(type) {
|
||||
case string:
|
||||
rest := v
|
||||
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 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(top, fmt.Sprintf("%s[%d]", field, i), e)
|
||||
}
|
||||
case map[string]any:
|
||||
for _, k := range sortedKeys(v) {
|
||||
sweep(top, field+"."+k, v[k])
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, k := range sortedKeys(r) {
|
||||
sweep(k, k, r[k])
|
||||
}
|
||||
return problems
|
||||
}
|
||||
|
||||
// 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, "; ")
|
||||
}
|
||||
@@ -0,0 +1,159 @@
|
||||
package catalogue
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// novox/hq issue 231 — a misspelled placeholder is written out as text.
|
||||
//
|
||||
// The four placeholders of the issue, in one file: two misspelled namespaces, a setting key no
|
||||
// definition could declare, and the shell's own syntax. The first three are refused by name, at the
|
||||
// catalogue check and at composition; the fourth reaches the file as written.
|
||||
const (
|
||||
misspelledShell = "${shel:zsh:first}"
|
||||
undeclaredSetting = "${setting:Undeclared}"
|
||||
misspelledMachine = "${machnie:address}"
|
||||
shellsOwn = "${XDG_CACHE_HOME:-x}"
|
||||
// The shell's operators after a word the mesh also uses, in any case: the shell's, and passed.
|
||||
shellsOperators = "${PORT:-8080} ${SHELL:-/bin/sh} ${SECRET:?unset} ${dir:-/tmp}"
|
||||
)
|
||||
|
||||
// The catalogue check — the strict parse registration runs too — refuses each by name, with the
|
||||
// module and the field it stands in, and says nothing about the shell's own syntax.
|
||||
func TestAMisspelledPlaceholderIsRefusedAtTheCheck(t *testing.T) {
|
||||
raw := `{"module":"speller","version":"1","resources":[{"id":"rc","type":"file","path":"/etc/speller.rc",` +
|
||||
`"content":"a=` + misspelledShell + `\nb=` + undeclaredSetting + `\nc=` + misspelledMachine +
|
||||
`\nd=` + shellsOwn + `\n"}]}`
|
||||
_, err := ParseManifest([]byte(raw))
|
||||
if err == nil {
|
||||
t.Fatal("a file holding three placeholders no pass consumes was accepted")
|
||||
}
|
||||
for _, token := range []string{misspelledShell, undeclaredSetting, misspelledMachine} {
|
||||
if !strings.Contains(err.Error(), "speller's resource rc holds "+token+" in its content") {
|
||||
t.Errorf("the refusal does not name %s with its module and field: %v", token, err)
|
||||
}
|
||||
}
|
||||
if strings.Contains(err.Error(), "XDG_CACHE_HOME") {
|
||||
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":"${Dir:x}"}`, "${Dir:x}", "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",` +
|
||||
`"content":"` + shellsOwn + ` ${(%):-%n} ${1:-.} ${count:-} ${trial:+ on trial} ${machine:address} ` +
|
||||
shellsOperators + `\n"},` +
|
||||
`{"id":"server","type":"container","name":"server","env":{"PORT":"${PORT:-80}"}}]}`
|
||||
if _, err := ParseManifest([]byte(raw)); err != nil {
|
||||
t.Fatalf("the shell's own syntax was refused: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Composition refuses the same placeholders in the same words — a manifest the store already holds
|
||||
// was never parsed by this binary — and a namespace the mesh knows, written in a field its pass does
|
||||
// not read, is refused there too, because it would reach the machine as the same literal text.
|
||||
func TestAMisspelledPlaceholderIsRefusedAtComposition(t *testing.T) {
|
||||
for _, c := range []struct {
|
||||
field string
|
||||
r map[string]any
|
||||
token string
|
||||
}{
|
||||
{"content", map[string]any{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "a=" + misspelledShell + "\n"}, misspelledShell},
|
||||
{"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": "${Dir:x}"}, "${Dir:x}"},
|
||||
{"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}}
|
||||
_, err := r.Declaration(Rendering{})
|
||||
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 composed rather than refused by name: %v", c.token, c.field, err)
|
||||
}
|
||||
}
|
||||
|
||||
m := Manifest{Module: "speller", Version: "1", Resources: []map[string]any{
|
||||
{"id": "rc", "type": "file", "path": "/etc/speller.rc", "content": "d=" + shellsOwn + " " + shellsOperators + "\n"},
|
||||
{"id": "server", "type": "process", "name": "server", "env": map[string]any{"PORT": "${PORT:-80}"}},
|
||||
}}
|
||||
out, err := Resolution{Node: "workstation", Account: "op", Modules: []Manifest{m}}.Declaration(Rendering{})
|
||||
if err != nil {
|
||||
t.Fatalf("the shell's own syntax was refused: %v", err)
|
||||
}
|
||||
if got := contentOf(t, out, "speller.rc"); got != "d="+shellsOwn+" "+shellsOperators+"\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"] == 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
|
||||
// `${path:t}` is namespace-shaped, and reaches the holder's file untouched.
|
||||
func TestContributedShellCodeIsNotSwept(t *testing.T) {
|
||||
modules := []Manifest{zshHolder(), {Module: "modifier", Shell: []ShellCode{
|
||||
{For: "zsh", Slot: "normal", Code: "echo ${path:t} ${shel:zsh:first}"},
|
||||
}}}
|
||||
out, err := Resolution{Node: "workstation", Account: "op", Modules: modules}.Declaration(Rendering{})
|
||||
if err != nil {
|
||||
t.Fatalf("contributed shell code was swept: %v", err)
|
||||
}
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user