diff --git a/internal/catalogue/config.go b/internal/catalogue/config.go index d00da709..90e9ea71 100644 --- a/internal/catalogue/config.go +++ b/internal/catalogue/config.go @@ -279,8 +279,9 @@ func ConfigProblems(m Manifest) []string { } } } - // A template's text is read where its tree or its build is (WithTemplatesFrom, Resolve), and this is - // judged again there. + // A template's text is not here yet. Resolve fills it from the build, WithTemplatesFrom from the tree; neither + // judges again itself, but WithTemplatesFrom parses its result again, and a resolved build is parsed again + // where it is registered, and this is judged then. if !placed[key] && !d.Runtime && !m.Templated() { say("placed nowhere: no file says ${config:%s} or takes it under %s, and it is not marked "+ "\"runtime\": true for a value the module's code reads itself (novox/hq ADR 0348 §5)", key, TakesConfigField) @@ -399,10 +400,19 @@ func configTakesProblems(m Manifest) []string { } naming = true for _, k := range stringsOf(r[TakesSettingsField]) { - if accountFacts[k] { + _, declared := m.Settings[k] + switch { + case accountFacts[k]: problems = append(problems, fmt.Sprintf("%s's %v names %q in %s, and %q is an account fact the "+ "mesh places, never a config value (novox/hq ADR 0348 §11)", m.Module, r["id"], k, TakesConfigField, k)) + case meshWords[k]: + // Refused for both forms by TakenSettingsProblems, in its words. + case !declared: + problems = append(problems, fmt.Sprintf("%s's %v names %q in %s and declares no such value under "+ + "%s: a value a file takes is declared like every other, with its type, description and a "+ + "default or required, or the config verb could never set it (novox/hq ADR 0348 §4, §11)", + m.Module, r["id"], k, TakesConfigField, ConfigField)) } } } diff --git a/internal/catalogue/config_test.go b/internal/catalogue/config_test.go index 1c89bb4d..1a2d8499 100644 --- a/internal/catalogue/config_test.go +++ b/internal/catalogue/config_test.go @@ -3,6 +3,7 @@ package catalogue import ( "bytes" "encoding/json" + "errors" "strings" "testing" ) @@ -401,3 +402,52 @@ func TestAModuleRewrittenFromSettingsToConfigComposesByteIdentically(t *testing. } } } + +// A key a mergeable file takes is a config value like every other: one not declared under config is refused by +// name (ADR 0348 §4, review of mesh-controller#320). +func TestConfigRefusesATakesConfigKeyNotDeclared(t *testing.T) { + m := inConfigForm() + resourceOf(m, "rules")["takes-config"] = []any{"position", "theme", "urgency"} + refused(t, m, `"urgency"`, "takes-config", "declares no such value under config") +} + +// A required value nothing sets is refused at composition in the config form's words: ${config:} and the +// config verb, never `settings set` (review of mesh-controller#320). The settings form's words are unchanged. +func TestAnUnsetRequiredValueIsRefusedInTheConfigFormsWords(t *testing.T) { + m, err := parseRaw(t, inConfigForm()) + if err != nil { + t.Fatal(err) + } + r := anAdoptedAnchor() + r.Modules = append(r.Modules, m) + composed, err := r.Compose(anchorRendering(false)) + if err != nil { + t.Fatal(err) + } + why, left := composed.LeftOut["notifier"] + if !left { + t.Fatal("a module whose required value nothing sets was not left out") + } + for _, said := range []string{"${config:sender}", `required config value "sender"`, "config verb"} { + if !strings.Contains(why, said) { + t.Fatalf("left out without %q: %s", said, why) + } + } + for _, unsaid := range []string{"${setting:", "settings set"} { + if strings.Contains(why, unsaid) { + t.Fatalf("left out in the settings form's words (%q): %s", unsaid, why) + } + } + var unset *UnsetSettingError + if err := unsetInConfigWords(settingInto(map[string]any{"type": "file", "content": "${setting:sender}"}, nil, + "notifier"), m); !errors.As(err, &unset) || unset.Setting != "sender" { + t.Fatalf("not typed as a value nobody gave: %v", err) + } + + settingsForm := notifier() + settingsForm.Resources[0]["content"] = "from = ${setting:domain}\n" + err = unsetInConfigWords(settingInto(settingsForm.Resources[0], nil, "notifier"), settingsForm) + if err == nil || !strings.Contains(err.Error(), "`settings set notifier `") { + t.Fatalf("the settings form's refusal changed: %v", err) + } +} diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 761a5515..25e39e27 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -1003,7 +1003,7 @@ func (r Resolution) compose(with Rendering, owner map[string]string, // **An operator's value, from the assignment** (novox/hq ADR 0112, ADR 0155): what a // definition may not carry because it is true of one installation only. Filled from // the same layers a mergeable file takes, and refused when no layer set it. - if err := settingInto(copied, WithDefaults(m, with.Settings[m.Module]), m.Module); err != nil { + if err := unsetInConfigWords(settingInto(copied, WithDefaults(m, with.Settings[m.Module]), m.Module), m); err != nil { return nil, err } // **Placed before anything reads a path.** A pathless directory receives the path diff --git a/internal/catalogue/setting_into.go b/internal/catalogue/setting_into.go index ee04ee18..1ea94437 100644 --- a/internal/catalogue/setting_into.go +++ b/internal/catalogue/setting_into.go @@ -1,6 +1,7 @@ package catalogue import ( + "errors" "fmt" "regexp" "sort" @@ -47,10 +48,30 @@ func settingsUsed(content string) []string { type UnsetSettingError struct { Module, Setting string said string + // where says what asks for it — "a file", "a service whose unit", "a directory whose owner" — and + // setToday which of the module's keys a layer sets, for saying it again in the config form's words. + where, setToday string } func (e *UnsetSettingError) Error() string { return e.said } +// unsetInConfigWords is the refusal of a required config value nothing sets, in the words of a module written in the +// config form (novox/hq ADR 0348 §1, §4): `${config:}` and the config verb, never `settings set`. Any other +// error, and any error of a module in the settings form, is returned as it was. +func unsetInConfigWords(err error, m Manifest) error { + var unset *UnsetSettingError + if err == nil || !m.DeclaresConfig() || !errors.As(err, &unset) { + return err + } + return &UnsetSettingError{Module: unset.Module, Setting: unset.Setting, where: unset.where, + setToday: unset.setToday, said: fmt.Sprintf( + "%s has %s that says ${config:%s}, and nothing sets the required config value %q for it — a "+ + "required value is the operator's own and has no default (novox/hq ADR 0348 §4): set it with "+ + "the config verb, for %s on this node, key %q%s", + unset.Module, unset.where, unset.Setting, unset.Setting, unset.Module, unset.Setting, + strings.ReplaceAll(unset.setToday, "no setting is set", "no config value is set"))} +} + // settingInto fills a file's ${setting:…} placeholders from the layers over a module. // // The last layer setting a key wins, which is the node's over the mesh's over the module's own @@ -74,12 +95,13 @@ func settingInto(resource map[string]any, layers []Layer, module string) error { for _, key := range settingsUsed(content) { value, set := settingValue(layers, key) if !set { - return &UnsetSettingError{Module: module, Setting: key, said: fmt.Sprintf( - "%s has a file that says ${setting:%s}, and nothing sets %q for it — an operator's "+ - "value is the assignment's, never the definition's (novox/hq ADR 0112), and only a "+ - "preference has a default in the definition (ADR 0262): "+ - "`settings set %s ` with {%q: …}%s", - module, key, key, module, key, orNoSettings(layers))} + return &UnsetSettingError{Module: module, Setting: key, where: "a file", + setToday: orNoSettings(layers), said: fmt.Sprintf( + "%s has a file that says ${setting:%s}, and nothing sets %q for it — an operator's "+ + "value is the assignment's, never the definition's (novox/hq ADR 0112), and only a "+ + "preference has a default in the definition (ADR 0262): "+ + "`settings set %s ` with {%q: …}%s", + module, key, key, module, key, orNoSettings(layers))} } content = strings.ReplaceAll(content, "${setting:"+key+"}", plainly(value)) } @@ -130,11 +152,12 @@ func settingIntoUnit(resource map[string]any, layers []Layer, module string) err for _, key := range settingsUsed(unit) { value, set := settingValue(layers, key) if !set { - return &UnsetSettingError{Module: module, Setting: key, said: fmt.Sprintf( - "%s has a service whose unit says ${setting:%s}, and nothing sets %q for it — an operator's "+ - "value is the assignment's, never the definition's (novox/hq ADR 0112): "+ - "`settings set %s ` with {%q: …}%s", - module, key, key, module, key, orNoSettings(layers))} + return &UnsetSettingError{Module: module, Setting: key, where: "a service whose unit", + setToday: orNoSettings(layers), said: fmt.Sprintf( + "%s has a service whose unit says ${setting:%s}, and nothing sets %q for it — an operator's "+ + "value is the assignment's, never the definition's (novox/hq ADR 0112): "+ + "`settings set %s ` with {%q: …}%s", + module, key, key, module, key, orNoSettings(layers))} } v := plainly(value) if !unitPart.MatchString(v) { @@ -229,11 +252,13 @@ func settingIntoOwner(resource map[string]any, layers []Layer, module string) er for _, key := range settingsUsed(owner) { value, set := settingValue(layers, key) if !set { - return &UnsetSettingError{Module: module, Setting: key, said: fmt.Sprintf( - "%s has a directory %v whose owner says ${setting:%s}, and nothing sets %q for it — an operator's "+ - "value is the assignment's, never the definition's (novox/hq ADR 0112): "+ - "`settings set %s ` with {%q: …}%s", - module, resource["id"], key, key, module, key, orNoSettings(layers))} + return &UnsetSettingError{Module: module, Setting: key, + where: fmt.Sprintf("a directory %v whose owner", resource["id"]), setToday: orNoSettings(layers), + said: fmt.Sprintf( + "%s has a directory %v whose owner says ${setting:%s}, and nothing sets %q for it — an operator's "+ + "value is the assignment's, never the definition's (novox/hq ADR 0112): "+ + "`settings set %s ` with {%q: …}%s", + module, resource["id"], key, key, module, key, orNoSettings(layers))} } v := plainly(value) id, whole := wholeID(v) diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index 45c992e8..5494c11b 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -590,7 +590,7 @@ func JudgeSettings(m Manifest, layers []Layer, adopted bool) error { for k, v := range settled { copied[k] = v } - if err := settingInto(copied, WithDefaults(m, layers), m.Module); err != nil { + if err := unsetInConfigWords(settingInto(copied, WithDefaults(m, layers), m.Module), m); err != nil { return err } }