From e41b78cd77907eb9c557e035fd43dc9b826fafed Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 16:23:03 +0200 Subject: [PATCH 1/4] Fill a preference's ${setting:} from its manifest default (hq ADR 0262) Without a default, a running module could never gain a setting: the file asking for it failed to compose until set, and the key was refused as stray until a file asked for it. Defaults sit under the mesh's and the node's settings, never merge into a JSON file, are refused for the operator's own values, and settings shows each value's source. --- cmd/mesh-controller/modules.go | 48 ++++- cmd/mesh-controller/plan.go | 2 +- .../settings_effective_test.go | 21 ++ internal/catalogue/consumer_into_serves.go | 2 +- internal/catalogue/declaration.go | 6 +- internal/catalogue/manifest.go | 9 + internal/catalogue/setting_defaults.go | 172 +++++++++++++++ internal/catalogue/setting_defaults_test.go | 195 ++++++++++++++++++ internal/catalogue/setting_into.go | 11 +- internal/catalogue/settings.go | 13 +- internal/catalogue/verbs.go | 4 +- 11 files changed, 470 insertions(+), 13 deletions(-) create mode 100644 cmd/mesh-controller/settings_effective_test.go create mode 100644 internal/catalogue/setting_defaults.go create mode 100644 internal/catalogue/setting_defaults_test.go diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 5a302da1..ba777a02 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -502,13 +502,38 @@ func settingsCommand(ctx context.Context, args []string) error { } if !has { fmt.Printf("%s on %s: no layer — the module's definition says\n", positionals[0], where) - return nil + } else { + shown, err := json.MarshalIndent(values, "", " ") + if err != nil { + return err + } + fmt.Println(string(shown)) } - shown, err := json.MarshalIndent(values, "", " ") + // Every value the module gives a default or a layer sets, and where it came from (novox/hq + // ADR 0262): the default, the mesh's layer, or this node's. Said after the layer, which stays + // the first thing printed because a caller reads it before replacing it (ADR 0217). + known, err := inv.Catalogue(ctx) if err != nil { return err } - fmt.Println(string(shown)) + m, ok := known[positionals[0]] + if !ok || len(m.Settings) == 0 { + return nil + } + var layers []catalogue.Layer + if *node != "" { + if mesh, has, err := inv.Layer(ctx, "", positionals[0]); err != nil { + return err + } else if has { + layers = append(layers, catalogue.Layer{From: catalogue.MeshWideLayer, Values: mesh}) + } + if has { + layers = append(layers, catalogue.Layer{From: *node, Values: values}) + } + } else if has { + layers = append(layers, catalogue.Layer{From: catalogue.MeshWideLayer, Values: values}) + } + fmt.Print(describeEffective(positionals[0], where, catalogue.Effective(m, layers))) return nil case "clear": @@ -526,6 +551,23 @@ func settingsCommand(ctx context.Context, args []string) error { } } +// describeEffective says each setting's value on a machine or the whole mesh, where it came from, and +// the module's default when a layer overrides it. +func describeEffective(module, where string, values []catalogue.SettingSource) string { + var b strings.Builder + fmt.Fprintf(&b, "%s on %s, every value and where it comes from:\n", module, where) + for _, v := range values { + value, _ := json.Marshal(v.Value) + fmt.Fprintf(&b, " %s = %s (%s", v.Key, value, v.From) + if v.HasDefault && v.From != catalogue.DefaultLayer { + d, _ := json.Marshal(v.Default) + fmt.Fprintf(&b, "; the default is %s", d) + } + b.WriteString(")\n") + } + return b.String() +} + // nodeFlag is ` --node ` for a machine's layer, nothing for the whole mesh's. func nodeFlag(node string) string { if node == "" { diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 5b19c60c..26dddca4 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -1425,7 +1425,7 @@ func servedOnNode(ctx context.Context, inv *inventory.Inventory, node string, } serves := catalogue.ServedOn(m, provision, ports) if len(serves) > 0 { - serves, err = catalogue.Settle(serves, layers) + serves, err = catalogue.Settle(serves, catalogue.WithDefaults(m, layers)) if err != nil { return nil, err } diff --git a/cmd/mesh-controller/settings_effective_test.go b/cmd/mesh-controller/settings_effective_test.go new file mode 100644 index 00000000..02b979af --- /dev/null +++ b/cmd/mesh-controller/settings_effective_test.go @@ -0,0 +1,21 @@ +package main + +import ( + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" +) + +// `settings` says each value and where it came from, and the default a layer overrides (novox/hq ADR 0262). +func TestSettingsSayWhereEachValueComesFrom(t *testing.T) { + got := describeEffective("dunst", "laptop", []catalogue.SettingSource{ + {Key: "font-size", Value: float64(13), From: "laptop", Default: float64(10), HasDefault: true}, + {Key: "width", Value: float64(250), From: catalogue.DefaultLayer, Default: float64(250), HasDefault: true}, + }) + want := "dunst on laptop, every value and where it comes from:\n" + + " font-size = 13 (laptop; the default is 10)\n" + + " width = 250 (default)\n" + if got != want { + t.Fatalf("said\n%s\nwant\n%s", got, want) + } +} diff --git a/internal/catalogue/consumer_into_serves.go b/internal/catalogue/consumer_into_serves.go index 2333dd12..6757e614 100644 --- a/internal/catalogue/consumer_into_serves.go +++ b/internal/catalogue/consumer_into_serves.go @@ -238,7 +238,7 @@ func (r Resolution) derivedFor(provision, as, consumer, local string, settings S "different things and nothing would compare them (novox/hq ADR 0202)", consumer, local, provision, m.Module, orNothing(sortedAnyKeys(names))) } - settled, err := Settle(names, settings[m.Module]) + settled, err := Settle(names, WithDefaults(m, settings[m.Module])) if err != nil { return nil, fmt.Errorf("%s serving %s: %w", m.Module, provision, err) } diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 27f42b4f..2cc13ba3 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -907,7 +907,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, with.Settings[m.Module], m.Module); err != nil { + if err := settingInto(copied, WithDefaults(m, with.Settings[m.Module]), m.Module); err != nil { return nil, err } // **Placed before anything reads a path.** A pathless directory receives the path @@ -1495,7 +1495,7 @@ func (r Resolution) composed(m Manifest, to string, raw map[string]any, layers [ // Overridden, not merged: a setting changes a key the contribution declares and adds none. // The provider reads the contribution as a contract, and a setting made for one of this // module's files is no part of it (novox/hq 04-ISSUES/173). - values, err := overridden(raw, layers, what) + values, err := overridden(raw, WithDefaults(m, layers), what) if err != nil { return nil, fmt.Errorf("%s: %w", what, err) } @@ -1926,7 +1926,7 @@ func (r Resolution) servedOnThisMachine(provision string, with Rendering) (map[s // one, so keep looking rather than concluding from the first. continue } - settled, err := Settle(serves, with.Settings[m.Module]) + settled, err := Settle(serves, WithDefaults(m, with.Settings[m.Module])) if err != nil { return nil, false, fmt.Errorf("%s serving %s: %w", m.Module, provision, err) } diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 2c64d59c..978c131e 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -458,6 +458,13 @@ type Manifest struct { // (novox/hq ADR 0201). Not history — that is an event — and never a secret, sealed or not. State []StateDeclaration `json:"state,omitempty"` + // Settings are the defaults this module gives its settings (novox/hq ADR 0262): each key a file, + // a contribution or a served fact asks for as `${setting:}`, its default, and why that + // default. Only a preference has one — a font size, a width, a number of workers — and a value + // that is inherently the operator's (a domain, an identity, a secret) is declared nowhere here and + // stays refused by name until a layer sets it. The mesh's layer, then the node's, override it. + Settings map[string]SettingDeclaration `json:"settings,omitempty"` + // Data is every kind of data this module keeps — its own, by directory, and what it keeps for // its consumers, by provision — each with a class the mesh protects and watches it by (novox/hq // ADR 0233). One list: the backup holder's lines, the bindings that do not move, what an @@ -1541,6 +1548,8 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, EventProblems(m)...) // And what it may call its state, and whose it may read (state.go, novox/hq ADR 0201). problems = append(problems, StateProblems(m)...) + // And the defaults it gives its settings (setting_defaults.go, novox/hq ADR 0262). + problems = append(problems, SettingProblems(m)...) wellFormed := true for _, c := range m.Claims { if !name.MatchString(c.Name) { diff --git a/internal/catalogue/setting_defaults.go b/internal/catalogue/setting_defaults.go new file mode 100644 index 00000000..d16a2a9a --- /dev/null +++ b/internal/catalogue/setting_defaults.go @@ -0,0 +1,172 @@ +package catalogue + +import ( + "fmt" + "sort" + "strings" +) + +// A preference has a default; the operator's own value has none (novox/hq ADR 0262, extending ADR +// 0112 and taking the default half of ADR 0164). +// +// `${setting:}` was refused whenever no layer set the key, and `settings set` refused a key no +// file asked for yet. Together they meant a running module could never gain a setting: the file that +// asks for it fails to compose until somebody sets it, and nobody can set it until the file asks. +// A definition may now give a key a default, with why, when the value is a **preference** — a font +// size, a width, a number of workers: something true of the software that a machine may tune. A +// value that is inherently the operator's — a domain, a public name, an identity, a secret — gets no +// default and stays refused by name until a layer sets it, which is what ADR 0112 and ADR 0155 were +// written for. +// +// The default is the lowest layer. The mesh's layer, then the node's, override it. It fills only +// `${setting:}`: a mergeable file's content already is its defaults, and laying a default over +// it would reach every mergeable file of the module (novox/hq issue 168). + +// SettingDeclaration is one key's default, as the definition gives it. +type SettingDeclaration struct { + // Kind says what sort of value this is. `preference` is the only kind with a default; it is + // stated rather than assumed so a reviewer sees the claim being made. + Kind string `json:"kind"` + // Default is the value when no layer sets the key: a string, a number or a boolean. + Default any `json:"default"` + // Why this default: one sentence for whoever wonders whether to change it. + Why string `json:"why"` +} + +// KindPreference is the kind of a setting that may have a default. +const KindPreference = "preference" + +// DefaultLayer is what the layer of a module's own defaults is called where a value's source is said. +const DefaultLayer = "default" + +// meshWords are the settings keys the mesh reads itself; a module declares none of them. +var meshWords = map[string]bool{ + PortsSetting: true, ExposeSetting: true, ReachSetting: true, EndpointsSetting: true, + PlacesSetting: true, AccessesSetting: true, NetworksSetting: true, +} + +// operatorsOwn are words that, in a key's name, say its value is the operator's and never a +// preference: a default for one would be the literal ADR 0112 removed from definitions. +var operatorsOwn = []string{ + "domain", "host", "issuer", "url", "email", "mail", "address", "identity", "login", "user", + "account", "password", "secret", "token", "credential", +} + +// SettingProblems is every way a definition's setting defaults are wrong, in its own words. +func SettingProblems(m Manifest) []string { + if len(m.Settings) == 0 { + return nil + } + used := settingKeysUsedBy(m) + var problems []string + for _, key := range sortedSettingKeys(m.Settings) { + d := m.Settings[key] + say := func(format string, args ...any) { + problems = append(problems, fmt.Sprintf("%s's setting %q: ", m.Module, key)+fmt.Sprintf(format, args...)) + } + if !settingRef.MatchString("${setting:" + key + "}") { + say("not a usable key: lower-case letters, digits, dots, dashes and underscores") + continue + } + if meshWords[key] { + say("the mesh reads %q itself, and a module gives it no default", key) + continue + } + if d.Kind != KindPreference { + say("kind is %q, and only a %q has a default (novox/hq ADR 0262)", d.Kind, KindPreference) + } + for _, word := range operatorsOwn { + if strings.Contains(key, word) { + say("a key naming %q is the operator's value, and has no default — it is the "+ + "assignment's, never the definition's (novox/hq ADR 0112, ADR 0262)", word) + break + } + } + switch v := d.Default.(type) { + case string: + if strings.TrimSpace(v) == "" { + say("an empty default is no default; give the value, or declare nothing") + } + case float64, bool: + case nil: + say("no default: a key without one is the operator's, and is not declared here") + default: + say("a default is a string, a number or a boolean, and this is %T", d.Default) + } + if strings.TrimSpace(d.Why) == "" { + say("no why: say in one sentence why this default") + } + if !used[key] { + say("nothing asks for ${setting:%s}, so the default reaches nothing", key) + } + } + return problems +} + +// Defaults is the layer a module's own defaults make, or nothing when it gives none. +func Defaults(m Manifest) (Layer, bool) { + if len(m.Settings) == 0 { + return Layer{}, false + } + values := map[string]any{} + for key, d := range m.Settings { + if d.Default != nil { + values[key] = d.Default + } + } + return Layer{From: DefaultLayer, Values: values}, len(values) > 0 +} + +// WithDefaults is a module's layers with its defaults under them, for filling `${setting:}`. +func WithDefaults(m Manifest, layers []Layer) []Layer { + d, has := Defaults(m) + if !has { + return layers + } + return append([]Layer{d}, layers...) +} + +// SettingSource is one key's effective value and the layer it came from. +type SettingSource struct { + Key string + Value any + // From is DefaultLayer, MeshWideLayer or the node's name. + From string + // Default is the module's default, when it gives one. + Default any + HasDefault bool +} + +// Effective is every key a module gives a default or a layer sets, with its value and where it came +// from: the default, then the mesh's layer, then the node's — later wins. +func Effective(m Manifest, layers []Layer) []SettingSource { + byKey := map[string]*SettingSource{} + for key, d := range m.Settings { + byKey[key] = &SettingSource{Key: key, Value: d.Default, From: DefaultLayer, Default: d.Default, HasDefault: true} + } + for _, layer := range layers { + for key, v := range layer.Values { + s, ok := byKey[key] + if !ok { + s = &SettingSource{Key: key} + byKey[key] = s + } + s.Value, s.From = v, layer.From + } + } + out := make([]SettingSource, 0, len(byKey)) + for _, s := range byKey { + out = append(out, *s) + } + sort.Slice(out, func(i, j int) bool { return out[i].Key < out[j].Key }) + return out +} + +func sortedSettingKeys(in map[string]SettingDeclaration) []string { + keys := make([]string, 0, len(in)) + for k := range in { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} diff --git a/internal/catalogue/setting_defaults_test.go b/internal/catalogue/setting_defaults_test.go new file mode 100644 index 00000000..4464c62c --- /dev/null +++ b/internal/catalogue/setting_defaults_test.go @@ -0,0 +1,195 @@ +package catalogue + +import ( + "encoding/json" + "strings" + "testing" +) + +// A preference has a default in the definition; the mesh's layer, then the node's, override it; a key +// with a default is not stray; and a value that is the operator's still has none (novox/hq ADR 0262). + +func notifier() Manifest { + return Manifest{Module: "notifier", + Settings: map[string]SettingDeclaration{ + "font-size": {Kind: KindPreference, Default: float64(10), Why: "readable at a scale of one"}, + "width": {Kind: KindPreference, Default: float64(250), Why: "fits a title of forty characters"}, + }, + Resources: []map[string]any{{"id": "configuration", "type": "file", "path": "/x/notifierrc", + "content": "font = Inter ${setting:font-size}\nwidth = ${setting:width}\n"}}, + } +} + +func parsed(t *testing.T, m Manifest) error { + t.Helper() + raw, err := json.Marshal(m) + if err != nil { + t.Fatal(err) + } + _, err = ParseManifest(raw) + return err +} + +func TestAnUnsetPreferenceTakesItsDefault(t *testing.T) { + m := notifier() + for _, layers := range [][]Layer{nil, {{From: MeshWideLayer, Values: map[string]any{}}}} { + file := map[string]any{} + for k, v := range m.Resources[0] { + file[k] = v + } + if err := settingInto(file, WithDefaults(m, layers), m.Module); err != nil { + t.Fatal(err) + } + if file["content"] != "font = Inter 10\nwidth = 250\n" { + t.Fatalf("filled as %q", file["content"]) + } + } + if err := JudgeSettings(m, nil, false); err != nil { + t.Fatalf("a module whose every key has a default does not compose with no layer: %v", err) + } +} + +func TestTheNodeOverTheMeshOverTheDefault(t *testing.T) { + m := notifier() + layers := []Layer{ + {From: MeshWideLayer, Values: map[string]any{"width": float64(300)}}, + {From: "laptop", Values: map[string]any{"font-size": float64(13), "width": float64(340)}}, + } + file := map[string]any{"type": "file", "content": m.Resources[0]["content"]} + if err := settingInto(file, WithDefaults(m, layers), m.Module); err != nil { + t.Fatal(err) + } + if file["content"] != "font = Inter 13\nwidth = 340\n" { + t.Fatalf("filled as %q", file["content"]) + } + file = map[string]any{"type": "file", "content": m.Resources[0]["content"]} + if err := settingInto(file, WithDefaults(m, layers[:1]), m.Module); err != nil { + t.Fatal(err) + } + if file["content"] != "font = Inter 10\nwidth = 300\n" { + t.Fatalf("the mesh's layer over the default filled as %q", file["content"]) + } +} + +// Composed for a machine, the way a push writes it: the default reaches the file, and the node's +// layer overrides it. +func TestAComposedMachineGetsTheDefaultAndTheNodesValue(t *testing.T) { + r := anAdoptedAnchor() + r.Modules = append(r.Modules, notifier()) + with := anchorRendering(false) + composed, err := r.Compose(with) + if err != nil { + t.Fatal(err) + } + if why, left := composed.LeftOut["notifier"]; left { + t.Fatalf("the notifier was left out: %s", why) + } + if c := byID(composed.Resources)["notifier.configuration"]["content"]; c != "font = Inter 10\nwidth = 250\n" { + t.Fatalf("composed with no layer as %q", c) + } + with.Settings["notifier"] = []Layer{{From: "anchor", Values: map[string]any{"font-size": float64(13)}}} + if composed, err = r.Compose(with); err != nil { + t.Fatal(err) + } + if c := byID(composed.Resources)["notifier.configuration"]["content"]; c != "font = Inter 13\nwidth = 250\n" { + t.Fatalf("composed with the node's font size as %q", c) + } +} + +// Setting a key the module gives a default is not refused as reaching nothing: it overrides the +// default, which is how a running module gains a setting with no gap between. +func TestAKeyWithADefaultIsNotStray(t *testing.T) { + m := notifier() + m.Resources[0]["content"] = "font = Inter ${setting:font-size}\nwidth = ${setting:width}\n" + stray := UnusedSettings(m, []Layer{{From: "laptop", Values: map[string]any{"font-size": float64(13), "colour": "red"}}}) + joined := strings.Join(stray, "; ") + if strings.Contains(joined, `"font-size"`) || !strings.Contains(joined, `"colour"`) { + t.Fatalf("stray: %s", joined) + } +} + +// A default fills ${setting:…} only: it never becomes a key of a mergeable file (issue 168). +func TestADefaultIsNotMergedIntoAJSONFile(t *testing.T) { + m := notifier() + m.Resources = append(m.Resources, map[string]any{"id": "other", "type": "file", "path": "/x/other.json", + "merge": MergeJSON, "content": `{"keep": 1}`}) + out, err := ApplySettings(m.Resources[1], WithDefaults(m, nil)) + if err != nil { + t.Fatal(err) + } + if strings.Contains(out["content"].(string), "font-size") { + t.Fatalf("a default reached a mergeable file: %s", out["content"]) + } +} + +// A value that is the operator's has no default: no layer setting it is refused by name, as before. +func TestAnOperatorsValueWithoutADefaultIsStillRefused(t *testing.T) { + m := notifier() + m.Resources[0]["content"] = "font = Inter ${setting:font-size}\nwidth = ${setting:width}\nfrom = ${setting:domain}\n" + err := JudgeSettings(m, nil, false) + if err == nil || !strings.Contains(err.Error(), "${setting:domain}") { + t.Fatalf("judged %v", err) + } + if strings.Contains(err.Error(), "font-size") { + t.Fatalf("a default was named as set today: %v", err) + } +} + +func TestTheParserTakesAPreferenceAndRefusesTheRest(t *testing.T) { + if err := parsed(t, notifier()); err != nil { + t.Fatalf("a preference with a default was refused: %v", err) + } + for _, c := range []struct { + name string + key string + d SettingDeclaration + refuse string + }{ + {"no kind", "font-size", SettingDeclaration{Default: float64(10), Why: "x"}, `only a "preference" has a default`}, + {"another kind", "font-size", SettingDeclaration{Kind: "operator", Default: float64(10), Why: "x"}, `only a "preference"`}, + {"no default", "font-size", SettingDeclaration{Kind: KindPreference, Why: "x"}, "no default"}, + {"an empty default", "font-size", SettingDeclaration{Kind: KindPreference, Default: " ", Why: "x"}, "an empty default"}, + {"an object", "font-size", SettingDeclaration{Kind: KindPreference, Default: map[string]any{"a": 1.0}, Why: "x"}, "a string, a number or a boolean"}, + {"no why", "font-size", SettingDeclaration{Kind: KindPreference, Default: float64(10)}, "no why"}, + {"the operator's", "mail-domain", SettingDeclaration{Kind: KindPreference, Default: "example.tld", Why: "x"}, "is the operator's value"}, + {"a secret", "api-token", SettingDeclaration{Kind: KindPreference, Default: "x", Why: "x"}, "is the operator's value"}, + {"the mesh's word", PortsSetting, SettingDeclaration{Kind: KindPreference, Default: float64(1), Why: "x"}, "the mesh reads"}, + {"read by nothing", "height", SettingDeclaration{Kind: KindPreference, Default: float64(300), Why: "x"}, "reaches nothing"}, + } { + m := notifier() + m.Resources[0]["content"] = m.Resources[0]["content"].(string) + "x = ${setting:" + c.key + "}\n" + if c.name == "read by nothing" { + m.Resources[0]["content"] = "font = Inter ${setting:font-size}\nwidth = ${setting:width}\n" + } + m.Settings[c.key] = c.d + err := parsed(t, m) + if err == nil || !strings.Contains(err.Error(), c.refuse) { + t.Errorf("%s: parsed %v, want %q", c.name, err, c.refuse) + } + } +} + +func TestEveryValueSaysWhereItCameFrom(t *testing.T) { + m := notifier() + m.Resources[0]["content"] = m.Resources[0]["content"].(string) + "x = ${setting:position}\n" + got := Effective(m, []Layer{ + {From: MeshWideLayer, Values: map[string]any{"width": float64(300), "position": "top-right"}}, + {From: "laptop", Values: map[string]any{"font-size": float64(13)}}, + }) + want := map[string]string{"font-size": "laptop", "position": MeshWideLayer, "width": MeshWideLayer} + if len(got) != 3 { + t.Fatalf("effective: %+v", got) + } + for _, s := range got { + if s.From != want[s.Key] { + t.Errorf("%s from %q, want %q", s.Key, s.From, want[s.Key]) + } + } + if got[0].Key != "font-size" || got[0].Value != float64(13) || got[0].Default != float64(10) { + t.Fatalf("font-size: %+v", got[0]) + } + only := Effective(m, nil) + if only[0].From != DefaultLayer || only[0].Value != float64(10) { + t.Fatalf("with no layer: %+v", only) + } +} diff --git a/internal/catalogue/setting_into.go b/internal/catalogue/setting_into.go index 751f3b17..8f631c63 100644 --- a/internal/catalogue/setting_into.go +++ b/internal/catalogue/setting_into.go @@ -40,8 +40,9 @@ func settingsUsed(content string) []string { // 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 — the same order settle -// applies to a mergeable file. A value that is not a string is written the way a program would read +// The last layer setting a key wins, which is the node's over the mesh's over the module's own +// default (novox/hq ADR 0262) — the same order settle applies to a mergeable file. The caller lays +// the defaults under the layers with WithDefaults. A value that is not a string is written the way a program would read // it (a number without a trailing .000000, a boolean as true/false). func settingInto(resource map[string]any, layers []Layer, module string) error { if fmt.Sprint(resource["type"]) != "file" { @@ -56,7 +57,8 @@ func settingInto(resource map[string]any, layers []Layer, module string) error { if !set { return fmt.Errorf( "%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): "+ + "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)) } @@ -80,6 +82,9 @@ func settingValue(layers []Layer, key string) (any, bool) { func orNoSettings(layers []Layer) string { var keys []string for _, l := range layers { + if l.From == DefaultLayer { + continue + } for k := range l.Values { keys = append(keys, k) } diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index 1c5c03e9..ea330369 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -151,6 +151,12 @@ func settle(base map[string]any, layers []Layer, protected map[string]bool, what map[string]any, error) { merged := deepCopy(base) for _, layer := range layers { + if layer.From == DefaultLayer { + // A module's own defaults fill ${setting:} only (novox/hq ADR 0262). A mergeable + // file's content already is its defaults, and a contribution or served fact declares its + // own; laid on here, a default would reach every mergeable file of its module (issue 168). + continue + } for key, value := range layer.Values { if key == PortsSetting { // Where the machine puts a port is the mesh's to apply, not a value for a file or @@ -224,6 +230,11 @@ func UnusedSettings(m Manifest, layers []Layer) []string { } lands := settingKeysUsedBy(m) + // A key the module gives a default reaches what asks for it (novox/hq ADR 0262): setting it + // overrides the default, which is how a running module gains a setting without a gap between. + for key := range m.Settings { + lands[key] = true + } for _, values := range m.Contributes { for key := range values { lands[key] = true @@ -420,7 +431,7 @@ func JudgeSettings(m Manifest, layers []Layer, adopted bool) error { for k, v := range settled { copied[k] = v } - if err := settingInto(copied, layers, m.Module); err != nil { + if err := settingInto(copied, WithDefaults(m, layers), m.Module); err != nil { return err } } diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index edb2c92b..f2ddd8e6 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -249,7 +249,9 @@ var ControllerVerbs = []Verb{ }, nil, "adopted"), Replaces: []string{"mesh-controller token issue", "wg set"}}, {Name: "settings", Description: "Read or set what an assignment is configured with: a module's settings for the whole " + - "mesh, or for one machine. Without values or clear, answers the layer as it stands — read it before setting it; " + + "mesh, or for one machine. Without values or clear, answers the layer as it stands — read it before setting it — " + + "then every value the module gives a default or a layer sets, with where it comes from: the module's default, " + + "the mesh, or the machine (novox/hq ADR 0262); " + "with history, the layers it replaced. Setting replaces that layer whole and answers each key it adds (+), " + "changes (~) and removes (-); a set that would remove a key is refused unless replace says it is meant " + "(novox/hq ADR 0217). Takes effect at the next push. With clear, removes the layer and the module is back to " + From 76babaea525430b2a53fc4e6be9e238ce65f525d Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 16:53:20 +0200 Subject: [PATCH 2/4] Read stored manifests leniently and mark the defaults layer (hq ADR 0262 review) A strict read of the stored catalogue fails every plan and send once a manifest uses a field an older controller lacks; registration stays strict. A node named default lost its layer to the name check. Judge the operator's keys by whole words, and scan a default under any key. --- cmd/mesh-controller/modules.go | 2 +- .../settings_effective_test.go | 2 +- internal/catalogue/installation.go | 5 +- internal/catalogue/manifest.go | 35 +++++- internal/catalogue/setting_defaults.go | 51 ++++++--- internal/catalogue/setting_defaults_test.go | 105 ++++++++++++++++++ internal/catalogue/setting_into.go | 2 +- internal/catalogue/settings.go | 7 +- internal/inventory/catalogue.go | 22 ++++ 9 files changed, 208 insertions(+), 23 deletions(-) diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index ba777a02..c35b1f46 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -559,7 +559,7 @@ func describeEffective(module, where string, values []catalogue.SettingSource) s for _, v := range values { value, _ := json.Marshal(v.Value) fmt.Fprintf(&b, " %s = %s (%s", v.Key, value, v.From) - if v.HasDefault && v.From != catalogue.DefaultLayer { + if v.HasDefault && !v.FromDefault { d, _ := json.Marshal(v.Default) fmt.Fprintf(&b, "; the default is %s", d) } diff --git a/cmd/mesh-controller/settings_effective_test.go b/cmd/mesh-controller/settings_effective_test.go index 02b979af..472bd485 100644 --- a/cmd/mesh-controller/settings_effective_test.go +++ b/cmd/mesh-controller/settings_effective_test.go @@ -10,7 +10,7 @@ import ( func TestSettingsSayWhereEachValueComesFrom(t *testing.T) { got := describeEffective("dunst", "laptop", []catalogue.SettingSource{ {Key: "font-size", Value: float64(13), From: "laptop", Default: float64(10), HasDefault: true}, - {Key: "width", Value: float64(250), From: catalogue.DefaultLayer, Default: float64(250), HasDefault: true}, + {Key: "width", Value: float64(250), From: catalogue.DefaultLayer, FromDefault: true, Default: float64(250), HasDefault: true}, }) want := "dunst on laptop, every value and where it comes from:\n" + " font-size = 13 (laptop; the default is 10)\n" + diff --git a/internal/catalogue/installation.go b/internal/catalogue/installation.go index 3068b9f0..d1325e72 100644 --- a/internal/catalogue/installation.go +++ b/internal/catalogue/installation.go @@ -153,7 +153,10 @@ func walk(node any, at string, meant map[string]bool, visit func(at, value strin } sort.Strings(keys) for _, k := range keys { - if prose[k] || k == NamesOnPurpose || (at == "" && k == "module") { + // Prose is a string a person reads. A key that is called `why` or `description` and holds + // anything else — a setting of that name, whose default the mesh writes — is walked like any + // other (novox/hq ADR 0262). + if _, isString := v[k].(string); (prose[k] && isString) || k == NamesOnPurpose || (at == "" && k == "module") { continue } child := at + "." + k diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 978c131e..e8cdea05 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -465,6 +465,11 @@ type Manifest struct { // stays refused by name until a layer sets it. The mesh's layer, then the node's, override it. Settings map[string]SettingDeclaration `json:"settings,omitempty"` + // unknown is the first key this manifest has that this controller does not know, when it was read + // leniently (novox/hq ADR 0262): a stored manifest written for a newer controller. ParseManifest + // refuses it; reading the stored catalogue keeps the rest of the manifest. + unknown string + // Data is every kind of data this module keeps — its own, by directory, and what it keeps for // its consumers, by provision — each with a class the mesh protects and watches it by (novox/hq // ADR 0233). One list: the backup holder's lines, the bindings that do not move, what an @@ -1185,7 +1190,13 @@ func ReceivedID(requirement string) string { return "received-" + requirement } type manifestFields Manifest // UnmarshalJSON reads `secrets` in both of its shapes — a path, or an object of local names to -// paths (ADR 0094) — and everything else exactly as the fields declare, unknown keys refused. +// paths (ADR 0094) — and everything else exactly as the fields declare. +// +// **An unknown key is kept aside, not refused here** (novox/hq ADR 0262). Registration and the module +// check refuse it, through ParseManifest. Reading the catalogue the store already holds does not: a +// manifest registered under a newer controller carries a field an older one does not know, and a +// strict read there failed the whole catalogue, and with it every plan and every send, the moment a +// controller was rolled back. UnknownField says what was set aside. func (m *Manifest) UnmarshalJSON(raw []byte) error { var keys map[string]json.RawMessage if err := json.Unmarshal(raw, &keys); err != nil { @@ -1266,10 +1277,19 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { decoder := json.NewDecoder(bytes.NewReader(rest)) decoder.DisallowUnknownFields() var fields manifestFields + unknown := "" if err := decoder.Decode(&fields); err != nil { - return err + if !strings.HasPrefix(err.Error(), "json: unknown field ") { + return err + } + unknown = err.Error() + fields = manifestFields{} + if err := json.Unmarshal(rest, &fields); err != nil { + return err + } } *m = Manifest(fields) + m.unknown = unknown if len(plain) > 0 { m.Secrets = plain } @@ -1373,6 +1393,10 @@ func SecretLocal(to, local string) string { return local } +// UnknownField is the first key a leniently read manifest had that this controller does not know, as +// the JSON decoder words it, or "" when it had none (novox/hq ADR 0262). +func (m Manifest) UnknownField() string { return m.unknown } + func ParseManifest(raw []byte) (Manifest, error) { var m Manifest // Strictly. **An unknown key is refused**, which is the discipline the host's declaration @@ -1384,7 +1408,12 @@ func ParseManifest(raw []byte) (Manifest, error) { // checking whether something is restricted will find that it is, and be wrong. decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.DisallowUnknownFields() - if err := decoder.Decode(&m); err != nil { + err := decoder.Decode(&m) + if err == nil && m.unknown != "" { + // Kept aside by UnmarshalJSON for the stored catalogue's sake; registration refuses it. + err = errors.New(m.unknown) + } + if err != nil { // A key that used to mean something says what it became. Refusing a renamed field with // "unknown field" is correct and unhelpful: whoever wrote it knew what they meant, and // the mesh knows what it is called now. diff --git a/internal/catalogue/setting_defaults.go b/internal/catalogue/setting_defaults.go index d16a2a9a..d28912d4 100644 --- a/internal/catalogue/setting_defaults.go +++ b/internal/catalogue/setting_defaults.go @@ -37,6 +37,7 @@ type SettingDeclaration struct { const KindPreference = "preference" // DefaultLayer is what the layer of a module's own defaults is called where a value's source is said. +// Only said: the layer is recognised by Layer.Default, never by this name, which a node may also have. const DefaultLayer = "default" // meshWords are the settings keys the mesh reads itself; a module declares none of them. @@ -45,11 +46,33 @@ var meshWords = map[string]bool{ PlacesSetting: true, AccessesSetting: true, NetworksSetting: true, } -// operatorsOwn are words that, in a key's name, say its value is the operator's and never a -// preference: a default for one would be the literal ADR 0112 removed from definitions. -var operatorsOwn = []string{ - "domain", "host", "issuer", "url", "email", "mail", "address", "identity", "login", "user", - "account", "password", "secret", "token", "credential", +// operatorsOwn are words that, as a whole word of a key's name, say its value is the operator's and +// never a preference: a default for one would be the literal ADR 0112 removed from definitions. Whole +// words, split at dashes, underscores and dots, so `max-tokens`, `show-hostname` and `mailbox-size` are +// preferences and `api-token`, `host` and `mail-domain` are not. +var operatorsOwn = map[string]bool{ + "domain": true, "host": true, "fqdn": true, "zone": true, "realm": true, "tenant": true, + "issuer": true, "url": true, "uri": true, "webhook": true, "origin": true, "dsn": true, "ip": true, + "email": true, "mail": true, "phone": true, "address": true, + "identity": true, "login": true, "user": true, "username": true, "account": true, "owner": true, + "uid": true, "gid": true, "puid": true, "pgid": true, + "password": true, "pass": true, "passwd": true, "passphrase": true, "secret": true, "token": true, + "key": true, "credential": true, +} + +// operatorsWord is the word of a key's name that says its value is the operator's, or "". +func operatorsWord(key string) string { + words := strings.FieldsFunc(key, func(r rune) bool { return r == '-' || r == '_' || r == '.' }) + for i, w := range words { + if operatorsOwn[w] { + return w + } + // An identifier a client is known by: `client-id`, `oauth-client-id`. + if w == "client" && i+1 < len(words) && words[i+1] == "id" { + return "client-id" + } + } + return "" } // SettingProblems is every way a definition's setting defaults are wrong, in its own words. @@ -75,12 +98,9 @@ func SettingProblems(m Manifest) []string { if d.Kind != KindPreference { say("kind is %q, and only a %q has a default (novox/hq ADR 0262)", d.Kind, KindPreference) } - for _, word := range operatorsOwn { - if strings.Contains(key, word) { - say("a key naming %q is the operator's value, and has no default — it is the "+ - "assignment's, never the definition's (novox/hq ADR 0112, ADR 0262)", word) - break - } + if word := operatorsWord(key); word != "" { + say("a key naming %q is the operator's value, and has no default — it is the "+ + "assignment's, never the definition's (novox/hq ADR 0112, ADR 0262)", word) } switch v := d.Default.(type) { case string: @@ -114,7 +134,7 @@ func Defaults(m Manifest) (Layer, bool) { values[key] = d.Default } } - return Layer{From: DefaultLayer, Values: values}, len(values) > 0 + return Layer{From: DefaultLayer, Values: values, Default: true}, len(values) > 0 } // WithDefaults is a module's layers with its defaults under them, for filling `${setting:}`. @@ -132,6 +152,8 @@ type SettingSource struct { Value any // From is DefaultLayer, MeshWideLayer or the node's name. From string + // FromDefault is whether the value is the module's default, whatever From reads. + FromDefault bool // Default is the module's default, when it gives one. Default any HasDefault bool @@ -142,7 +164,8 @@ type SettingSource struct { func Effective(m Manifest, layers []Layer) []SettingSource { byKey := map[string]*SettingSource{} for key, d := range m.Settings { - byKey[key] = &SettingSource{Key: key, Value: d.Default, From: DefaultLayer, Default: d.Default, HasDefault: true} + byKey[key] = &SettingSource{Key: key, Value: d.Default, From: DefaultLayer, FromDefault: true, + Default: d.Default, HasDefault: true} } for _, layer := range layers { for key, v := range layer.Values { @@ -151,7 +174,7 @@ func Effective(m Manifest, layers []Layer) []SettingSource { s = &SettingSource{Key: key} byKey[key] = s } - s.Value, s.From = v, layer.From + s.Value, s.From, s.FromDefault = v, layer.From, layer.Default } } out := make([]SettingSource, 0, len(byKey)) diff --git a/internal/catalogue/setting_defaults_test.go b/internal/catalogue/setting_defaults_test.go index 4464c62c..fe68c915 100644 --- a/internal/catalogue/setting_defaults_test.go +++ b/internal/catalogue/setting_defaults_test.go @@ -193,3 +193,108 @@ func TestEveryValueSaysWhereItCameFrom(t *testing.T) { t.Fatalf("with no layer: %+v", only) } } + +// A node may be called `default`. Its layer is a node's like any other: it overrides the module's +// default, it merges into a mergeable file, and it is named among what is set. +func TestANodeCalledDefaultIsANodesLayer(t *testing.T) { + m := notifier() + node := []Layer{{From: DefaultLayer, Values: map[string]any{"font-size": float64(13)}}} + file := map[string]any{"type": "file", "content": m.Resources[0]["content"]} + if err := settingInto(file, WithDefaults(m, node), m.Module); err != nil { + t.Fatal(err) + } + if file["content"] != "font = Inter 13\nwidth = 250\n" { + t.Fatalf("the node called default was dropped: %q", file["content"]) + } + out, err := ApplySettings(map[string]any{"id": "j", "type": "file", "merge": MergeJSON, "content": `{}`}, node) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out["content"].(string), `"font-size": 13`) { + t.Fatalf("the node called default did not merge: %s", out["content"]) + } + if got := Effective(m, node); got[0].FromDefault || got[0].Value != float64(13) { + t.Fatalf("the node's value read as the default: %+v", got[0]) + } +} + +// The operator's own value is told by a whole word of the key's name, not by a part of one. +func TestAKeyIsTheOperatorsByAWholeWord(t *testing.T) { + for _, key := range []string{"max-tokens", "show-hostname", "ghost-opacity", "users-per-page", "mailbox-size", + "font-size", "width", "keyboard-delay", "ipv6-preferred", "client-width"} { + if w := operatorsWord(key); w != "" { + t.Errorf("%s read as the operator's (%s)", key, w) + } + } + for key, word := range map[string]string{"mail-domain": "mail", "site-domain": "domain", "api-key": "key", "admin-password": "password", + "oauth-client-id": "client-id", "db-dsn": "dsn", "public-ip": "ip", "puid": "puid", "dns-zone": "zone", + "webhook": "webhook", "notify_phone": "phone", "cors.origin": "origin", "smtp-pass": "pass", + "backup-passphrase": "passphrase", "data-owner": "owner", "fqdn": "fqdn", "tenant": "tenant", "host": "host"} { + if w := operatorsWord(key); w != word { + t.Errorf("%s: read %q, want %q", key, w, word) + } + } +} + +// A default is a value the mesh writes, so the installation check reads it, even under a setting +// called `description` or `why`; a setting's own why is prose and is not read. +func TestTheInstallationCheckReadsADefault(t *testing.T) { + m := notifier() + m.Settings["relay"] = SettingDeclaration{Kind: KindPreference, Default: "relay.acme.be", Why: "as at relay.acme.be"} + m.Settings["description"] = SettingDeclaration{Kind: KindPreference, Default: "notes.acme.be", Why: "x"} + problems := strings.Join(InstallationProblems(m), "; ") + for _, want := range []string{"relay.acme.be at settings.relay.default", "notes.acme.be at settings.description.default"} { + if !strings.Contains(problems, want) { + t.Errorf("not reported: %q in %s", want, problems) + } + } + if strings.Contains(problems, "settings.relay.why") { + t.Errorf("a why was read as a value: %s", problems) + } +} + +// A default fills ${setting:…} in what a module contributes and serves, and never replaces a value a +// contribution states itself. +func TestADefaultFillsAContributionAndAServedFactButNotALiteral(t *testing.T) { + m := notifier() + m.Settings["site-title"] = SettingDeclaration{Kind: KindPreference, Default: "Notes", Why: "x"} + contribution := map[string]any{"title": "${setting:site-title}", "width": "fixed", "port": float64(8080)} + got, err := overridden(contribution, WithDefaults(m, nil), "a contribution") + if err != nil { + t.Fatal(err) + } + if got["title"] != "Notes" || got["width"] != "fixed" { + t.Fatalf("contribution: %v", got) + } + got, err = overridden(contribution, WithDefaults(m, []Layer{{From: "laptop", Values: map[string]any{"width": "wide"}}}), "a contribution") + if err != nil || got["width"] != "wide" { + t.Fatalf("a node's value did not override a contribution's own key: %v %v", got, err) + } + served, err := Settle(map[string]any{"name": "${setting:site-title}"}, WithDefaults(m, nil)) + if err != nil || served["name"] != "Notes" { + t.Fatalf("served: %v %v", served, err) + } + if _, err := Settle(map[string]any{"name": "${setting:site-title}"}, nil); err == nil { + t.Fatal("a served fact without the defaults was filled") + } +} + +// A stored manifest with a key this controller does not know is read without it, and said; the module +// check still refuses it. +func TestAStoredManifestWithAnUnknownKeyIsReadAndRegistrationRefusesIt(t *testing.T) { + raw := []byte(`{"module": "later", "version": "1", "tools": ["later_x"], "a-field-from-later": {"x": 1}}`) + var m Manifest + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatalf("a stored manifest with an unknown key was not read: %v", err) + } + if m.Module != "later" || len(m.Tools) != 1 || !strings.Contains(m.UnknownField(), `"a-field-from-later"`) { + t.Fatalf("read as %+v, unknown %q", m, m.UnknownField()) + } + if _, err := ParseManifest(raw); err == nil || !strings.Contains(err.Error(), `unknown field "a-field-from-later"`) { + t.Fatalf("registration took it: %v", err) + } + var known Manifest + if err := json.Unmarshal([]byte(`{"module": "now"}`), &known); err != nil || known.UnknownField() != "" { + t.Fatalf("a known manifest: %v %q", err, known.UnknownField()) + } +} diff --git a/internal/catalogue/setting_into.go b/internal/catalogue/setting_into.go index 8f631c63..caa2a18d 100644 --- a/internal/catalogue/setting_into.go +++ b/internal/catalogue/setting_into.go @@ -82,7 +82,7 @@ func settingValue(layers []Layer, key string) (any, bool) { func orNoSettings(layers []Layer) string { var keys []string for _, l := range layers { - if l.From == DefaultLayer { + if l.Default { continue } for k := range l.Values { diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index ea330369..d7ff5f8b 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -30,6 +30,9 @@ type Layer struct { // Where these came from, for saying which layer set a value. From string Values map[string]any + // Default marks the layer a module's own defaults make (novox/hq ADR 0262). Marked rather than + // recognised by From, which is a node's name for a node's layer, and a node may be called anything. + Default bool } // ApplySettings produces a resource's final content from the module's own and the layers over it. @@ -114,7 +117,7 @@ func overridden(base map[string]any, layers []Layer, what string) (map[string]an values[key] = value } } - kept = append(kept, Layer{From: layer.From, Values: values}) + kept = append(kept, Layer{From: layer.From, Values: values, Default: layer.Default}) } merged, err := settle(base, kept, nil, what) if err != nil { @@ -151,7 +154,7 @@ func settle(base map[string]any, layers []Layer, protected map[string]bool, what map[string]any, error) { merged := deepCopy(base) for _, layer := range layers { - if layer.From == DefaultLayer { + if layer.Default { // A module's own defaults fill ${setting:} only (novox/hq ADR 0262). A mergeable // file's content already is its defaults, and a contribution or served fact declares its // own; laid on here, a default would reach every mergeable file of its module (issue 168). diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index 1fd0cbda..a82d4368 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -5,15 +5,35 @@ import ( "encoding/json" "errors" "fmt" + "log" "sort" "strconv" "strings" + "sync" "time" "github.com/jackc/pgx/v5" "github.com/novox/mesh-controller/internal/catalogue" ) +// noted is each stored manifest's unknown key already said, so a catalogue read on every plan says it once. +var noted sync.Map + +// noteUnknown says, once per module and key, that a stored manifest has a key this controller does not +// know and that it was read without it (novox/hq ADR 0262). A manifest registered under a newer +// controller is read by an older one after a rollback; refusing it here failed the whole catalogue. +func noteUnknown(m catalogue.Manifest) { + u := m.UnknownField() + if u == "" { + return + } + if _, said := noted.LoadOrStore(m.Module+"\x00"+u, true); said { + return + } + log.Printf("the stored manifest of %s has a key this controller does not know (%s); read without it — "+ + "a newer controller registered it (novox/hq ADR 0262)", m.Module, u) +} + // ErrNoSuchModule is what the mesh says about a module it has never been told about. var ErrNoSuchModule = errors.New("no module of that name") @@ -259,6 +279,7 @@ func (i *Inventory) Catalogue(ctx context.Context) (map[string]catalogue.Manifes if err := json.Unmarshal(raw, &m); err != nil { return nil, err } + noteUnknown(m) out[m.Module] = m } return out, rows.Err() @@ -1217,6 +1238,7 @@ func (i *Inventory) Catalogued(ctx context.Context) ([]Entry, error) { if err := json.Unmarshal(raw, &m); err != nil { return nil, err } + noteUnknown(m) entry := Entry{Manifest: m, Source: source, On: on} if source.Repository == providedBy { // It came with the control plane. Not a repository, and showing it as one would have From f5680ba8da267a5a66a9b8956c6fd9eeaac50c56 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 17:10:16 +0200 Subject: [PATCH 3/4] List every module's preferences in the settings verb (hq ADR 0262) One verb is the interface to every preference, so no module builds a settings tool of its own: each key, its default and why, and every assigned machine's value with its source. --- cmd/mesh-controller/modules.go | 98 ++++++++++++++++++- cmd/mesh-controller/seatverbs.go | 16 +++ .../settings_effective_test.go | 50 ++++++++++ internal/catalogue/verbs.go | 9 +- 4 files changed, 168 insertions(+), 5 deletions(-) diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index c35b1f46..0c9070ca 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "os" + "sort" "strings" "github.com/novox/mesh-controller/internal/broker" @@ -396,7 +397,8 @@ func assignCommand(ctx context.Context, verb string, args []string) error { func settingsCommand(ctx context.Context, args []string) error { if len(args) == 0 { return errors.New("settings show [--node ] [--history], settings set " + - "[--node ] [--replace], or settings clear [--node ]") + "[--node ] [--replace], settings clear [--node ], or settings preferences " + + "[] [--node ]") } open, err := openStores(ctx) if err != nil { @@ -536,6 +538,46 @@ func settingsCommand(ctx context.Context, args []string) error { fmt.Print(describeEffective(positionals[0], where, catalogue.Effective(m, layers))) return nil + case "preferences": + // Every module's preferences, and each machine's value with its source (novox/hq ADR 0262). + if len(positionals) > 1 { + return errors.New("settings preferences [] [--node ]") + } + only := "" + if len(positionals) == 1 { + only = positionals[0] + } + entries, err := inv.Catalogued(ctx) + if err != nil { + return err + } + var listed []preferencesOf + for _, e := range entries { + m := e.Manifest + if len(m.Settings) == 0 || (only != "" && m.Module != only) { + continue + } + p := preferencesOf{Manifest: m, On: map[string][]catalogue.SettingSource{}} + for _, n := range e.On { + if *node != "" && n != *node { + continue + } + p.Nodes = append(p.Nodes, n) + layers, err := inv.SettingsFor(ctx, n, m.Module) + if err != nil { + return err + } + p.On[n] = catalogue.Effective(m, layers) + } + listed = append(listed, p) + } + if only != "" && len(listed) == 0 { + fmt.Printf("%s declares no preferences\n", only) + return nil + } + fmt.Print(describePreferences(listed)) + return nil + case "clear": if len(positionals) != 1 { return errors.New("settings clear [--node ]") @@ -547,7 +589,7 @@ func settingsCommand(ctx context.Context, args []string) error { return nil default: - return fmt.Errorf("settings has no %q; it has show, set and clear", args[0]) + return fmt.Errorf("settings has no %q; it has show, set, clear and preferences", args[0]) } } @@ -568,6 +610,58 @@ func describeEffective(module, where string, values []catalogue.SettingSource) s return b.String() } +// preferencesOf is one module's preferences and its value on each machine it is assigned to. +type preferencesOf struct { + Manifest catalogue.Manifest + Nodes []string + On map[string][]catalogue.SettingSource +} + +// describePreferences lists each module's preferences — key, default and why — and, per machine it is +// assigned to, the value and where it comes from (novox/hq ADR 0262). +func describePreferences(modules []preferencesOf) string { + if len(modules) == 0 { + return "no module declares a preference\n" + } + var b strings.Builder + for i, p := range modules { + if i > 0 { + b.WriteString("\n") + } + on := "assigned nowhere" + if len(p.Nodes) > 0 { + on = "on " + strings.Join(p.Nodes, ", ") + } + fmt.Fprintf(&b, "%s (%s)\n", p.Manifest.Module, on) + keys := make([]string, 0, len(p.Manifest.Settings)) + for k := range p.Manifest.Settings { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + d := p.Manifest.Settings[k] + def, _ := json.Marshal(d.Default) + fmt.Fprintf(&b, " %s, default %s: %s\n", k, def, d.Why) + for _, n := range p.Nodes { + for _, s := range p.On[n] { + if s.Key != k { + continue + } + v, _ := json.Marshal(s.Value) + from := s.From + if s.FromDefault { + from = catalogue.DefaultLayer + } else if s.From != catalogue.MeshWideLayer { + from = "the node" + } + fmt.Fprintf(&b, " %s: %s (%s)\n", n, v, from) + } + } + } + } + return b.String() +} + // nodeFlag is ` --node ` for a machine's layer, nothing for the whole mesh's. func nodeFlag(node string) string { if node == "" { diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 9b4a9beb..0a233da3 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -761,6 +761,22 @@ func (a *verbArguments) commandLine() ([]string, error) { } return argv, nil case "settings": + // Every module's preferences, their defaults and each machine's value (novox/hq ADR 0262): + // the one interface for them, so no module builds a settings tool of its own. Asked for by + // name, or by naming no module, since a layer is always some module's. + if list := str("list"); list != "" || str("module") == "" { + if list != "" && list != "preferences" { + return nil, fmt.Errorf("settings lists %q only; %q is not a listing", "preferences", list) + } + argv := []string{"settings", "preferences"} + if m := str("module"); m != "" { + argv = append(argv, m) + } + if n := str("node"); n != "" { + argv = append(argv, "--node", n) + } + return argv, nil + } // `settings set|clear` at a shell (novox/hq issue 198). The values travel as an argument // because a tool has no file to hand the command; the command reads either. if err := need("module"); err != nil { diff --git a/cmd/mesh-controller/settings_effective_test.go b/cmd/mesh-controller/settings_effective_test.go index 472bd485..2f2ddf1a 100644 --- a/cmd/mesh-controller/settings_effective_test.go +++ b/cmd/mesh-controller/settings_effective_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" "github.com/novox/mesh-controller/internal/catalogue" @@ -19,3 +20,52 @@ func TestSettingsSayWhereEachValueComesFrom(t *testing.T) { t.Fatalf("said\n%s\nwant\n%s", got, want) } } + +// `settings` with list "preferences" is the one listing of every module's preferences; module and node +// narrow it, and no other listing is taken. +func TestSettingsListPreferences(t *testing.T) { + for _, c := range []struct { + args map[string]any + want string + }{ + {map[string]any{"list": "preferences"}, "settings preferences"}, + {map[string]any{"list": "preferences", "module": "dunst"}, "settings preferences dunst"}, + {map[string]any{"list": "preferences", "node": "laptop"}, "settings preferences --node laptop"}, + } { + argv, err := argvFor("settings", c.args) + if err != nil || strings.Join(argv, " ") != c.want { + t.Errorf("%v: %v %v, want %s", c.args, argv, err, c.want) + } + } + if _, err := argvFor("settings", map[string]any{"list": "everything"}); err == nil { + t.Error("a listing other than preferences was taken") + } + if argv, err := argvFor("settings", map[string]any{}); err != nil || strings.Join(argv, " ") != "settings preferences" { + t.Errorf("settings naming no module is the listing: %v %v", argv, err) + } +} + +func TestPreferencesSayEachMachinesValueAndItsSource(t *testing.T) { + m := catalogue.Manifest{Module: "dunst", Settings: map[string]catalogue.SettingDeclaration{ + "font-size": {Kind: catalogue.KindPreference, Default: float64(10), Why: "readable at 100 DPI"}, + "width": {Kind: catalogue.KindPreference, Default: float64(250), Why: "forty characters"}, + }} + on := map[string][]catalogue.SettingSource{ + "laptop": catalogue.Effective(m, []catalogue.Layer{{From: "laptop", Values: map[string]any{"font-size": float64(16)}}}), + "desk": catalogue.Effective(m, []catalogue.Layer{{From: catalogue.MeshWideLayer, Values: map[string]any{"width": float64(300)}}}), + } + got := describePreferences([]preferencesOf{{Manifest: m, Nodes: []string{"desk", "laptop"}, On: on}}) + want := "dunst (on desk, laptop)\n" + + " font-size, default 10: readable at 100 DPI\n" + + " desk: 10 (default)\n" + + " laptop: 16 (the node)\n" + + " width, default 250: forty characters\n" + + " desk: 300 (the mesh)\n" + + " laptop: 250 (default)\n" + if got != want { + t.Fatalf("said\n%s\nwant\n%s", got, want) + } + if describePreferences(nil) != "no module declares a preference\n" { + t.Fatal("an empty listing") + } +} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index f2ddd8e6..e326b231 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -251,19 +251,22 @@ var ControllerVerbs = []Verb{ {Name: "settings", Description: "Read or set what an assignment is configured with: a module's settings for the whole " + "mesh, or for one machine. Without values or clear, answers the layer as it stands — read it before setting it — " + "then every value the module gives a default or a layer sets, with where it comes from: the module's default, " + - "the mesh, or the machine (novox/hq ADR 0262); " + + "the mesh, or the machine (novox/hq ADR 0262); with list \"preferences\", or with no module, every " + + "module's preferences and each machine's value; " + "with history, the layers it replaced. Setting replaces that layer whole and answers each key it adds (+), " + "changes (~) and removes (-); a set that would remove a key is refused unless replace says it is meant " + "(novox/hq ADR 0217). Takes effect at the next push. With clear, removes the layer and the module is back to " + "what its definition says; a cleared or replaced layer is kept in the history.", Input: schema(map[string]string{ - "module": "the module's name", + "module": "the module's name; with list, only that module's", "values": "the settings as a JSON object, for set", "node": "one machine; the whole mesh when absent", "clear": "\"true\" to remove the layer instead of setting it; not with values", "replace": "\"true\": with values, the set is meant to remove the keys the layer had and it does not name", "history": "\"true\": without values or clear, the layers this one replaced, the latest first", - }, []string{"module"}, "clear", "replace", "history")}, + "list": "\"preferences\": every module's preferences — key, default and why — and the value on each " + + "machine it is assigned to with where it comes from; module and node narrow it (novox/hq ADR 0262)", + }, nil, "clear", "replace", "history")}, {Name: "command", Description: "Run one command line of the controller's own, as you would type it at its " + "shell — `node account g14 jochen`, `node show ace`, `module list` — and answer what it printed. The " + "generic verb beside the named ones (novox/hq ADR 0154): everything the binary can do, without a verb " + From af63b233dbd5609bf46f93dd62fddc8b078fd6f5 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 17:34:53 +0200 Subject: [PATCH 4/4] Leave out a module whose stored manifest has an unknown field, and raise it (hq ADR 0262 review) A key dropped silently ran a module without what its manifest says, and a key inside a block still failed the whole catalogue. Judge a key by what it is about, and narrow the listing to one machine. --- cmd/mesh-controller/modules.go | 24 ++++++- cmd/mesh-controller/pending.go | 3 + cmd/mesh-controller/plan.go | 5 +- cmd/mesh-controller/seatverbs.go | 6 ++ .../settings_effective_test.go | 38 ++++++++++ cmd/mesh-controller/unknown_fields.go | 64 +++++++++++++++++ cmd/mesh-controller/unknown_fields_test.go | 50 +++++++++++++ internal/catalogue/data.go | 2 +- internal/catalogue/declaration.go | 4 ++ internal/catalogue/manifest.go | 25 ++++--- internal/catalogue/setting_defaults.go | 48 +++++++++---- internal/catalogue/setting_defaults_test.go | 72 ++++++++++++++----- internal/catalogue/state.go | 2 +- internal/catalogue/unknown_field.go | 58 +++++++++++++++ internal/catalogue/verbs.go | 2 +- internal/inventory/catalogue.go | 10 +-- internal/inventory/unknown_field_test.go | 34 +++++++++ 17 files changed, 395 insertions(+), 52 deletions(-) create mode 100644 cmd/mesh-controller/unknown_fields.go create mode 100644 cmd/mesh-controller/unknown_fields_test.go create mode 100644 internal/catalogue/unknown_field.go create mode 100644 internal/inventory/unknown_field_test.go diff --git a/cmd/mesh-controller/modules.go b/cmd/mesh-controller/modules.go index 0c9070ca..9a8e3c8b 100644 --- a/cmd/mesh-controller/modules.go +++ b/cmd/mesh-controller/modules.go @@ -547,6 +547,12 @@ func settingsCommand(ctx context.Context, args []string) error { if len(positionals) == 1 { only = positionals[0] } + if *node != "" { + // A machine the mesh does not know is refused, never answered with an empty listing. + if _, err := inv.NodeByName(ctx, *node); err != nil { + return err + } + } entries, err := inv.Catalogued(ctx) if err != nil { return err @@ -557,6 +563,10 @@ func settingsCommand(ctx context.Context, args []string) error { if len(m.Settings) == 0 || (only != "" && m.Module != only) { continue } + if *node != "" && !containsString(e.On, *node) { + // Asked for one machine: a module not on it has no value there to say. + continue + } p := preferencesOf{Manifest: m, On: map[string][]catalogue.SettingSource{}} for _, n := range e.On { if *node != "" && n != *node { @@ -572,7 +582,11 @@ func settingsCommand(ctx context.Context, args []string) error { listed = append(listed, p) } if only != "" && len(listed) == 0 { - fmt.Printf("%s declares no preferences\n", only) + fmt.Printf("%s declares no preferences%s\n", only, onNode(*node)) + return nil + } + if len(listed) == 0 && *node != "" { + fmt.Printf("no module on %s declares a preference\n", *node) return nil } fmt.Print(describePreferences(listed)) @@ -610,6 +624,14 @@ func describeEffective(module, where string, values []catalogue.SettingSource) s return b.String() } +// onNode is ` on ` for one machine, nothing for the whole mesh. +func onNode(node string) string { + if node == "" { + return "" + } + return " on " + node +} + // preferencesOf is one module's preferences and its value on each machine it is assigned to. type preferencesOf struct { Manifest catalogue.Manifest diff --git a/cmd/mesh-controller/pending.go b/cmd/mesh-controller/pending.go index 3627eb68..7743f165 100644 --- a/cmd/mesh-controller/pending.go +++ b/cmd/mesh-controller/pending.go @@ -478,6 +478,9 @@ func settlingPending(ctx context.Context, open *stores) { for _, line := range settlePending(ctx, open, time.Now()) { fmt.Println(line) } + for _, line := range raiseUnknownFields(ctx, open.inventory) { + fmt.Println(line) + } select { case <-ctx.Done(): return diff --git a/cmd/mesh-controller/plan.go b/cmd/mesh-controller/plan.go index 26dddca4..55586d33 100644 --- a/cmd/mesh-controller/plan.go +++ b/cmd/mesh-controller/plan.go @@ -515,9 +515,8 @@ func sortedKeysOf(m map[string]string) []string { // 0163, rule 6), one line each: the machine is told everything else, and is told it was left out. func reportLeftOut(node string, declared sendable) { for _, m := range declared.LeftOut { - fmt.Printf("%s: %s left out — a setting stored for it cannot compose with its definition; "+ - "what the machine holds for it is kept and its containers are untouched. %s\n", - node, m, declared.leftOutWhy[m]) + fmt.Printf("%s: %s left out — what the machine holds for it is kept and its containers are "+ + "untouched. %s\n", node, m, declared.leftOutWhy[m]) } // And whom it serves nothing, because their identity overflows what the provision keeps (ADR // 0225): the machine is sent everything else, and the consumer is named. diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 0a233da3..e134cf3e 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -764,6 +764,12 @@ func (a *verbArguments) commandLine() ([]string, error) { // Every module's preferences, their defaults and each machine's value (novox/hq ADR 0262): // the one interface for them, so no module builds a settings tool of its own. Asked for by // name, or by naming no module, since a layer is always some module's. + if str("module") == "" && str("values") != "" { + return nil, errors.New("settings: a module is needed to set values; name it with module") + } + if str("module") == "" && on("clear") { + return nil, errors.New("settings: a module is needed to clear a layer; name it with module") + } if list := str("list"); list != "" || str("module") == "" { if list != "" && list != "preferences" { return nil, fmt.Errorf("settings lists %q only; %q is not a listing", "preferences", list) diff --git a/cmd/mesh-controller/settings_effective_test.go b/cmd/mesh-controller/settings_effective_test.go index 2f2ddf1a..f248e2b4 100644 --- a/cmd/mesh-controller/settings_effective_test.go +++ b/cmd/mesh-controller/settings_effective_test.go @@ -37,6 +37,14 @@ func TestSettingsListPreferences(t *testing.T) { t.Errorf("%v: %v %v, want %s", c.args, argv, err, c.want) } } + for args, want := range map[string]map[string]any{ + "a module is needed to set values": {"values": `{"width": 300}`}, + "a module is needed to clear a layer": {"clear": "true"}, + } { + if _, err := argvFor("settings", want); err == nil || !strings.Contains(err.Error(), args) { + t.Errorf("%v: %v, want %q", want, err, args) + } + } if _, err := argvFor("settings", map[string]any{"list": "everything"}); err == nil { t.Error("a listing other than preferences was taken") } @@ -69,3 +77,33 @@ func TestPreferencesSayEachMachinesValueAndItsSource(t *testing.T) { t.Fatal("an empty listing") } } + +// The listing over the real stores: each machine's value with its source; a machine names only the +// modules on it; a machine the mesh does not know is refused (novox/hq ADR 0262). +func TestPreferencesListedFromTheStores(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + register(t, open, catalogue.Manifest{Module: "notes", Version: "1", + Settings: map[string]catalogue.SettingDeclaration{ + "font-size": {Kind: catalogue.KindPreference, Default: float64(10), Why: "readable at 100 DPI"}, + }, + Resources: []map[string]any{{"id": "rc", "type": "file", "path": "/etc/notes.conf", "mode": "0644", + "content": "font = ${setting:font-size}\n"}}}) + if _, err := assign(ctx, open, "laptop", "notes"); err != nil { + t.Fatal(err) + } + if err := open.inventory.SetSettings(ctx, "laptop", "notes", map[string]any{"font-size": float64(16)}); err != nil { + t.Fatal(err) + } + all := stdoutOf(t, func() error { return settingsCommand(ctx, []string{"preferences"}) }) + if !strings.Contains(all, "notes (on laptop)") || !strings.Contains(all, "laptop: 16 (the node)") || + !strings.Contains(all, "font-size, default 10: readable at 100 DPI") { + t.Fatalf("the listing:\n%s", all) + } + if got := stdoutOf(t, func() error { return settingsCommand(ctx, []string{"preferences", "--node", "anchor"}) }); got != "no module on anchor declares a preference\n" { + t.Fatalf("a machine without the module:\n%s", got) + } + if err := settingsCommand(ctx, []string{"preferences", "--node", "nowhere"}); err == nil { + t.Fatal("a machine the mesh does not know was answered") + } +} diff --git a/cmd/mesh-controller/unknown_fields.go b/cmd/mesh-controller/unknown_fields.go new file mode 100644 index 00000000..020da9b1 --- /dev/null +++ b/cmd/mesh-controller/unknown_fields.go @@ -0,0 +1,64 @@ +package main + +import ( + "context" + "fmt" + "sort" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/conditions" + "github.com/novox/mesh-controller/internal/inventory" +) + +// A stored manifest with a key this controller does not know (novox/hq ADR 0262). The module is left out +// of every machine's declaration by name; this is the loud half: a condition per module until the +// controller is updated, or the module is registered again in a shape this controller reads. + +const ( + sourceUnknownFields = "the catalogue" + kindUnknownField = "unknown-field" +) + +// unknownFieldObservations is one condition for each module of the catalogue whose stored manifest has +// a key this controller does not know. +func unknownFieldObservations(known map[string]catalogue.Manifest) []conditions.Observation { + names := make([]string, 0, len(known)) + for name, m := range known { + if m.UnknownField() != "" { + names = append(names, name) + } + } + sort.Strings(names) + var out []conditions.Observation + for _, name := range names { + m := known[name] + out = append(out, conditions.Observation{ + Scope: conditions.ScopeMesh, ID: name, Kind: kindUnknownField, Severity: conditions.Warning, + Resolver: conditions.ResolverOperator, Source: sourceUnknownFields, + Summary: catalogue.UnknownFieldReason(m), + Said: m.UnknownField(), + Headline: name + " is left out until the controller is updated", + Explanation: name + " uses a field this controller does not know, so it is left out of every machine it is on, and nothing of it changes there until the controller is updated.", + Needs: "update the controller, or register " + name + " again at a version this controller knows.", + Resolved: "the controller reads " + name + " again", + }) + } + return out +} + +// raiseUnknownFields raises those conditions and clears the ones no longer true, on the controller's +// tick. A catalogue that could not be read raises and clears nothing: "none" is not said for "could not +// tell" (ADR 0227 rule 4). +func raiseUnknownFields(ctx context.Context, inv *inventory.Inventory) []string { + if conditionsFrom == nil { + return nil + } + known, err := inv.Catalogue(ctx) + if err != nil { + return []string{fmt.Sprintf("the catalogue could not be read to say which modules it cannot read: %v", err)} + } + if err := conditionsFrom.Reconcile(ctx, sourceUnknownFields, unknownFieldObservations(known)); err != nil { + return []string{fmt.Sprintf("the modules with a field this controller does not know could not be kept as conditions: %v", err)} + } + return nil +} diff --git a/cmd/mesh-controller/unknown_fields_test.go b/cmd/mesh-controller/unknown_fields_test.go new file mode 100644 index 00000000..9776331c --- /dev/null +++ b/cmd/mesh-controller/unknown_fields_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "encoding/json" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/conditions" +) + +// A module whose stored manifest has a key this controller does not know is a condition, in plain +// words, until it is read again; every other module raises nothing (novox/hq ADR 0262). +func TestAModuleWithAnUnknownFieldIsACondition(t *testing.T) { + var later, now catalogue.Manifest + if err := json.Unmarshal([]byte(`{"module": "dunst", "version": "2", "settings": {}, "a-field-from-later": 1}`), &later); err != nil { + t.Fatal(err) + } + if err := json.Unmarshal([]byte(`{"module": "xorg", "version": "1"}`), &now); err != nil { + t.Fatal(err) + } + observed := unknownFieldObservations(map[string]catalogue.Manifest{"dunst": later, "xorg": now}) + if len(observed) != 1 || observed[0].ID != "dunst" || observed[0].Kind != kindUnknownField { + t.Fatalf("observed: %+v", observed) + } + o := observed[0] + if why, ok := conditions.PlainWords(conditions.Words{Headline: o.Headline, Explanation: o.Explanation, + Resolved: o.Resolved, Needs: o.Needs}); !ok { + t.Fatalf("not plain: %s", why) + } + + k, _ := withConditionsInMemory(t) + if err := k.Reconcile(t.Context(), sourceUnknownFields, observed); err != nil { + t.Fatal(err) + } + if _, open, _ := k.Get(t.Context(), o.Key()); !open { + t.Fatal("not raised") + } + if err := k.Reconcile(t.Context(), sourceUnknownFields, unknownFieldObservations(map[string]catalogue.Manifest{"xorg": now})); err != nil { + t.Fatal(err) + } + still, err := k.Open(t.Context()) + if err != nil { + t.Fatal(err) + } + for _, c := range still { + if c.Key == o.Key() { + t.Fatalf("not cleared once read again: %+v", c) + } + } +} diff --git a/internal/catalogue/data.go b/internal/catalogue/data.go index e44ac75a..5a43203c 100644 --- a/internal/catalogue/data.go +++ b/internal/catalogue/data.go @@ -159,7 +159,7 @@ func (b *DataBackup) UnmarshalJSON(raw []byte) error { dec := json.NewDecoder(bytes.NewReader(raw)) dec.DisallowUnknownFields() if err := dec.Decode(&full); err != nil { - return fmt.Errorf("a data item's backup is \"copy\", \"none\" or {dump, into}: %w", err) + return fmt.Errorf("a data item's backup is \"copy\", \"none\" or {dump, into}: %w", typedUnknown(err)) } *b = DataBackup{Dump: full.Dump, Into: full.Into} return nil diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index 2cc13ba3..dac7a6f7 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -318,6 +318,10 @@ func (e *NotMadeError) Error() string { func (r Resolution) LeftOut(settings SettingsBy, adopted bool) map[string]string { out := map[string]string{} for _, m := range r.Modules { + if why := UnknownFieldReason(m); why != "" { + out[m.Module] = why + continue + } if err := JudgeSettings(m, settings[m.Module], adopted); err != nil { out[m.Module] = err.Error() } diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index e8cdea05..c0c0fe8e 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -197,7 +197,7 @@ func (i *OfferIdentity) UnmarshalJSON(raw []byte) error { dec := json.NewDecoder(bytes.NewReader(raw)) dec.DisallowUnknownFields() if err := dec.Decode(&full); err != nil { - return fmt.Errorf("an offer's identity is false or {max, in}: %w", err) + return fmt.Errorf("an offer's identity is false or {max, in}: %w", typedUnknown(err)) } *i = OfferIdentity{Max: full.Max, In: full.In} return nil @@ -316,7 +316,7 @@ func (o *Offer) UnmarshalJSON(raw []byte) error { dec.DisallowUnknownFields() if err := dec.Decode(&full); err != nil { return fmt.Errorf("a provided name is either a string or {name, scope, credential, reach, identity, "+ - "keeps-consumer-data}: %w", err) + "keeps-consumer-data}: %w", typedUnknown(err)) } o.Name, o.Scope, o.Credential, o.Reach, o.Identity = full.Name, full.Scope, full.Credential, full.Reach, full.Identity o.KeepsConsumerData = full.Keeps @@ -465,9 +465,10 @@ type Manifest struct { // stays refused by name until a layer sets it. The mesh's layer, then the node's, override it. Settings map[string]SettingDeclaration `json:"settings,omitempty"` - // unknown is the first key this manifest has that this controller does not know, when it was read - // leniently (novox/hq ADR 0262): a stored manifest written for a newer controller. ParseManifest - // refuses it; reading the stored catalogue keeps the rest of the manifest. + // unknown is the first key this manifest has that this controller does not know, at any depth, when + // it was read from the store (novox/hq ADR 0262): a manifest a newer controller registered. + // ParseManifest refuses it; the store's catalogue still loads, and the module is left out of every + // machine's declaration by name until the controller is updated (LeftOut). unknown string // Data is every kind of data this module keeps — its own, by directory, and what it keeps for @@ -1279,13 +1280,19 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error { var fields manifestFields unknown := "" if err := decoder.Decode(&fields); err != nil { - if !strings.HasPrefix(err.Error(), "json: unknown field ") { + if asUnknownField(err) == nil { return err } + // Read without it where the key is at the top; where it is inside a block, the block's own + // decoder refuses it again, and the manifest keeps its name and version alone. Either way the + // module is left out of every declaration by name (LeftOut), so nothing runs on a part-read + // manifest. unknown = err.Error() fields = manifestFields{} - if err := json.Unmarshal(rest, &fields); err != nil { - return err + if json.Unmarshal(rest, &fields) != nil { + fields = manifestFields{} + _ = json.Unmarshal(keys["module"], &fields.Module) + _ = json.Unmarshal(keys["version"], &fields.Version) } } *m = Manifest(fields) @@ -2479,7 +2486,7 @@ func (o *OwnSecrets) UnmarshalJSON(raw []byte) error { dec := json.NewDecoder(bytes.NewReader(body)) dec.DisallowUnknownFields() if err := dec.Decode(&long); err != nil { - return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\"}: %w", name, err) + return fmt.Errorf("own-secrets.%s: a path, or {\"path\", \"taken\", \"issued-by\"}: %w", name, typedUnknown(err)) } out[name] = OwnSecret{Path: long.Path, Taken: long.Taken, IssuedBy: long.IssuedBy} } diff --git a/internal/catalogue/setting_defaults.go b/internal/catalogue/setting_defaults.go index d28912d4..2297c17b 100644 --- a/internal/catalogue/setting_defaults.go +++ b/internal/catalogue/setting_defaults.go @@ -46,32 +46,54 @@ var meshWords = map[string]bool{ PlacesSetting: true, AccessesSetting: true, NetworksSetting: true, } -// operatorsOwn are words that, as a whole word of a key's name, say its value is the operator's and -// never a preference: a default for one would be the literal ADR 0112 removed from definitions. Whole -// words, split at dashes, underscores and dots, so `max-tokens`, `show-hostname` and `mailbox-size` are -// preferences and `api-token`, `host` and `mail-domain` are not. +// operatorsOwn are the words that, as what a key's name is about, say its value is the operator's and +// never a preference: a default for one would be the literal ADR 0112 removed from definitions. var operatorsOwn = map[string]bool{ - "domain": true, "host": true, "fqdn": true, "zone": true, "realm": true, "tenant": true, - "issuer": true, "url": true, "uri": true, "webhook": true, "origin": true, "dsn": true, "ip": true, + "domain": true, "host": true, "hostname": true, "servername": true, "fqdn": true, "zone": true, + "realm": true, "tenant": true, "site": true, "timezone": true, + "issuer": true, "url": true, "uri": true, "webhook": true, "origin": true, "dsn": true, + "ip": true, "ipv4": true, "ipv6": true, "email": true, "mail": true, "phone": true, "address": true, "identity": true, "login": true, "user": true, "username": true, "account": true, "owner": true, "uid": true, "gid": true, "puid": true, "pgid": true, "password": true, "pass": true, "passwd": true, "passphrase": true, "secret": true, "token": true, - "key": true, "credential": true, + "key": true, "apikey": true, "bearer": true, "cert": true, "credential": true, } -// operatorsWord is the word of a key's name that says its value is the operator's, or "". +// operatorsCompounds are names of two words that are the operator's though neither word alone says so +// at the end of a key: a client's identifier, and a name the world knows a site or server by. +var operatorsCompounds = map[string]bool{ + "client-id": true, "site-name": true, "server-name": true, "public-name": true, "smtp-relay": true, +} + +// aboutAnAmount are first words that make a key about how many or whether, never about whom: +// `max-tokens` is a number, `show-hostname` a switch. +var aboutAnAmount = map[string]bool{ + "max": true, "min": true, "num": true, "count": true, "show": true, "hide": true, "enable": true, + "disable": true, "use": true, "allow": true, +} + +// operatorsWord is what in a key's name says its value is the operator's, or "". A key is about its +// last word — `url-timeout` is a timeout, `user-agent` an agent, `mail-domain` a domain — or its last two +// as one of operatorsCompounds. A plural is read as its singular. func operatorsWord(key string) string { words := strings.FieldsFunc(key, func(r rune) bool { return r == '-' || r == '_' || r == '.' }) + if len(words) == 0 || (len(words) > 1 && aboutAnAmount[words[0]]) { + return "" + } for i, w := range words { - if operatorsOwn[w] { - return w + if !operatorsOwn[w] && strings.HasSuffix(w, "s") && operatorsOwn[strings.TrimSuffix(w, "s")] { + words[i] = strings.TrimSuffix(w, "s") } - // An identifier a client is known by: `client-id`, `oauth-client-id`. - if w == "client" && i+1 < len(words) && words[i+1] == "id" { - return "client-id" + } + if n := len(words); n > 1 { + if pair := words[n-2] + "-" + words[n-1]; operatorsCompounds[pair] { + return pair } } + if last := words[len(words)-1]; operatorsOwn[last] { + return last + } return "" } diff --git a/internal/catalogue/setting_defaults_test.go b/internal/catalogue/setting_defaults_test.go index fe68c915..cb3be6d8 100644 --- a/internal/catalogue/setting_defaults_test.go +++ b/internal/catalogue/setting_defaults_test.go @@ -218,18 +218,24 @@ func TestANodeCalledDefaultIsANodesLayer(t *testing.T) { } } -// The operator's own value is told by a whole word of the key's name, not by a part of one. -func TestAKeyIsTheOperatorsByAWholeWord(t *testing.T) { +// The operator's own value is told by what a key's name is about: its last word, or its last two as +// a known compound; a plural as its singular; and never a number or a switch. +func TestAKeyIsTheOperatorsByWhatItIsAbout(t *testing.T) { for _, key := range []string{"max-tokens", "show-hostname", "ghost-opacity", "users-per-page", "mailbox-size", - "font-size", "width", "keyboard-delay", "ipv6-preferred", "client-width"} { + "font-size", "width", "keyboard-delay", "ipv6-preferred", "client-width", "user-agent", "url-timeout", + "site-title", "cert-renewal-days", "name", "font-name"} { if w := operatorsWord(key); w != "" { t.Errorf("%s read as the operator's (%s)", key, w) } } - for key, word := range map[string]string{"mail-domain": "mail", "site-domain": "domain", "api-key": "key", "admin-password": "password", + for key, word := range map[string]string{"mail-domain": "domain", "api-key": "key", "admin-password": "password", "oauth-client-id": "client-id", "db-dsn": "dsn", "public-ip": "ip", "puid": "puid", "dns-zone": "zone", "webhook": "webhook", "notify_phone": "phone", "cors.origin": "origin", "smtp-pass": "pass", - "backup-passphrase": "passphrase", "data-owner": "owner", "fqdn": "fqdn", "tenant": "tenant", "host": "host"} { + "backup-passphrase": "passphrase", "data-owner": "owner", "fqdn": "fqdn", "tenant": "tenant", "host": "host", + "allowed-hosts": "host", "admin-emails": "email", "tokens": "token", "hostname": "hostname", + "apikey": "apikey", "servername": "servername", "tls-cert": "cert", "site": "site", "timezone": "timezone", + "bearer": "bearer", "bind-ipv4": "ipv4", "listen-ipv6": "ipv6", "site-name": "site-name", + "server-name": "server-name", "public-name": "public-name", "smtp-relay": "smtp-relay"} { if w := operatorsWord(key); w != word { t.Errorf("%s: read %q, want %q", key, w, word) } @@ -279,22 +285,50 @@ func TestADefaultFillsAContributionAndAServedFactButNotALiteral(t *testing.T) { } } -// A stored manifest with a key this controller does not know is read without it, and said; the module -// check still refuses it. -func TestAStoredManifestWithAnUnknownKeyIsReadAndRegistrationRefusesIt(t *testing.T) { - raw := []byte(`{"module": "later", "version": "1", "tools": ["later_x"], "a-field-from-later": {"x": 1}}`) - var m Manifest - if err := json.Unmarshal(raw, &m); err != nil { - t.Fatalf("a stored manifest with an unknown key was not read: %v", err) - } - if m.Module != "later" || len(m.Tools) != 1 || !strings.Contains(m.UnknownField(), `"a-field-from-later"`) { - t.Fatalf("read as %+v, unknown %q", m, m.UnknownField()) - } - if _, err := ParseManifest(raw); err == nil || !strings.Contains(err.Error(), `unknown field "a-field-from-later"`) { - t.Fatalf("registration took it: %v", err) +// A stored manifest with a key this controller does not know, at the top or inside any block, is read +// and its module is left out of every declaration by name; the rest of the catalogue is read; the module +// check refuses it. One case per block whose decoder wraps the decoder's words in its own. +func TestAStoredManifestWithAnUnknownKeyIsLeftOutAndRegistrationRefusesIt(t *testing.T) { + for where, raw := range map[string]string{ + "the top": `{"module": "later", "version": "2", "a-field-from-later": {"x": 1}}`, + "a state": `{"module": "later", "version": "2", "state": [{"name": "s", "a-field-from-later": 1}]}`, + "a provided name": `{"module": "later", "version": "2", "provides": [{"name": "p", "a-field-from-later": 1}]}`, + "an offer's identity": `{"module": "later", "version": "2", "provides": [{"name": "p", "identity": {"in": "x", "a-field-from-later": 1}}]}`, + "a backup": `{"module": "later", "version": "2", "data": {"own": [{"id": "d", "path": "${dir:d}", "class": "valuable", "backup": {"dump": "x", "into": "y", "a-field-from-later": 1}}]}}`, + "an own secret": `{"module": "later", "version": "2", "own-secrets": {"s": {"path": "/x", "a-field-from-later": 1}}}`, + "a seat's verb": `{"module": "later", "version": "2", "seats": [{"name": "later-seat", "serves": [{"name": "v", "a-field-from-later": 1}]}]}`, + } { + var m Manifest + if err := json.Unmarshal([]byte(raw), &m); err != nil { + t.Errorf("%s: the stored manifest was not read: %v", where, err) + continue + } + if m.Module != "later" || m.Version != "2" || !strings.Contains(m.UnknownField(), `"a-field-from-later"`) { + t.Errorf("%s: read as %q %q, unknown %q", where, m.Module, m.Version, m.UnknownField()) + continue + } + if where != "the top" && !strings.Contains(m.UnknownField(), ": json: unknown field") { + t.Errorf("%s: the block's decoder did not say it: %q", where, m.UnknownField()) + } + left := Resolution{Node: "laptop", Modules: []Manifest{m, notifier()}}.LeftOut(nil, false) + if why := left["later"]; !strings.Contains(why, "uses a field this controller does not know") || + !strings.Contains(why, "a-field-from-later") { + t.Errorf("%s: not left out by name: %v", where, left) + } + if _, notifierLeft := left["notifier"]; notifierLeft { + t.Errorf("%s: another module was left out with it: %v", where, left) + } + if _, err := ParseManifest([]byte(raw)); err == nil || !strings.Contains(err.Error(), "a-field-from-later") { + t.Errorf("%s: the module check took it: %v", where, err) + } } var known Manifest - if err := json.Unmarshal([]byte(`{"module": "now"}`), &known); err != nil || known.UnknownField() != "" { + if err := json.Unmarshal([]byte(`{"module": "now", "state": [{"name": "s"}]}`), &known); err != nil || known.UnknownField() != "" { t.Fatalf("a known manifest: %v %q", err, known.UnknownField()) } + // A malformed manifest is still refused: only an unknown key is read past. + var bad Manifest + if err := json.Unmarshal([]byte(`{"module": "bad", "state": [{"name": 3}]}`), &bad); err == nil { + t.Fatal("a malformed stored manifest was read") + } } diff --git a/internal/catalogue/state.go b/internal/catalogue/state.go index d21297cd..1944b778 100644 --- a/internal/catalogue/state.go +++ b/internal/catalogue/state.go @@ -49,7 +49,7 @@ func (s *StateDeclaration) UnmarshalJSON(raw []byte) error { dec := json.NewDecoder(bytes.NewReader(trimmed)) dec.DisallowUnknownFields() if err := dec.Decode(&full); err != nil { - return fmt.Errorf("a state is either a name or {name, history, ttl-seconds, per-machine}: %w", err) + return fmt.Errorf("a state is either a name or {name, history, ttl-seconds, per-machine}: %w", typedUnknown(err)) } *s = StateDeclaration(full) return nil diff --git a/internal/catalogue/unknown_field.go b/internal/catalogue/unknown_field.go new file mode 100644 index 00000000..ed2d3319 --- /dev/null +++ b/internal/catalogue/unknown_field.go @@ -0,0 +1,58 @@ +package catalogue + +import ( + "errors" + "regexp" +) + +// A key this controller does not know, in a manifest (novox/hq ADR 0262). +// +// Registration refuses it, as it always has. A manifest the store already holds was registered by a newer +// controller, and is read by this one after a rollback: refusing it there failed the whole catalogue, and +// with it every plan and every send. Dropping the key silently would be worse — a module running without +// something its manifest says. So the manifest is read, the module is left out of every machine's +// declaration by name, and the controller raises a condition until it is updated. + +// UnknownFieldError is a key a manifest has that this controller does not know, wherever it is: at the +// top of the manifest or inside a block (a state, an offer, a data item's backup, an own secret, a verb). +// Typed, because the blocks' decoders wrap the decoder's words in their own. +type UnknownFieldError struct { + Field string + err error +} + +func (e *UnknownFieldError) Error() string { return e.err.Error() } +func (e *UnknownFieldError) Unwrap() error { return e.err } + +// unknownFieldText is how the JSON decoder words a key its target does not have. +var unknownFieldText = regexp.MustCompile(`^json: unknown field "([^"]*)"$`) + +// typedUnknown is err as an UnknownFieldError when it is the decoder refusing an unknown key, else err. +func typedUnknown(err error) error { + if err == nil { + return nil + } + if m := unknownFieldText.FindStringSubmatch(err.Error()); m != nil { + return &UnknownFieldError{Field: m[1], err: err} + } + return err +} + +// asUnknownField is the unknown key err is about, at any depth, or nil. +func asUnknownField(err error) *UnknownFieldError { + var u *UnknownFieldError + if errors.As(typedUnknown(err), &u) { + return u + } + return nil +} + +// UnknownFieldReason is why a module whose stored manifest has a key this controller does not know is +// left out of a machine's declaration, or "" when it has none. +func UnknownFieldReason(m Manifest) string { + if m.unknown == "" { + return "" + } + return m.Module + " uses a field this controller does not know (" + m.unknown + "); it is left out " + + "until the controller is updated (novox/hq ADR 0262)" +} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index e326b231..de1faf8a 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -56,7 +56,7 @@ func (v *Verb) UnmarshalJSON(raw []byte) error { decoder := json.NewDecoder(bytes.NewReader(trimmed)) decoder.DisallowUnknownFields() if err := decoder.Decode(&p); err != nil { - return fmt.Errorf("a served verb is a name or {name, description, input, output}: %w", err) + return fmt.Errorf("a served verb is a name or {name, description, input, output}: %w", typedUnknown(err)) } if p.Name == "" { return fmt.Errorf("a served verb has no name: %s", trimmed) diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index a82d4368..3112962c 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -20,8 +20,9 @@ import ( var noted sync.Map // noteUnknown says, once per module and key, that a stored manifest has a key this controller does not -// know and that it was read without it (novox/hq ADR 0262). A manifest registered under a newer -// controller is read by an older one after a rollback; refusing it here failed the whole catalogue. +// know (novox/hq ADR 0262). A manifest registered under a newer controller is read by an older one after +// a rollback; refusing it here failed the whole catalogue. The module is left out of every machine by name +// (catalogue.LeftOut), and the controller's tick raises a condition for it. func noteUnknown(m catalogue.Manifest) { u := m.UnknownField() if u == "" { @@ -30,8 +31,9 @@ func noteUnknown(m catalogue.Manifest) { if _, said := noted.LoadOrStore(m.Module+"\x00"+u, true); said { return } - log.Printf("the stored manifest of %s has a key this controller does not know (%s); read without it — "+ - "a newer controller registered it (novox/hq ADR 0262)", m.Module, u) + log.Printf("the stored manifest of %s has a key this controller does not know (%s): a newer controller "+ + "registered it, and %s is left out of every machine until this controller is updated (novox/hq ADR 0262)", + m.Module, u, m.Module) } // ErrNoSuchModule is what the mesh says about a module it has never been told about. diff --git a/internal/inventory/unknown_field_test.go b/internal/inventory/unknown_field_test.go new file mode 100644 index 00000000..fd163f86 --- /dev/null +++ b/internal/inventory/unknown_field_test.go @@ -0,0 +1,34 @@ +package inventory + +import ( + "strings" + "testing" +) + +// A manifest a newer controller registered, with a key this one does not know, does not stop the +// catalogue from loading: it is read, marked, and every other module is read as before (novox/hq ADR 0262). +func TestTheCatalogueLoadsAroundAManifestWithAnUnknownKey(t *testing.T) { + inv := fresh(t) + if err := inv.RegisterModule(t.Context(), manifest("thing", nil, nil), Source{}); err != nil { + t.Fatal(err) + } + if _, err := inv.store.Pool().Exec(t.Context(), + `insert into module (name, manifest) values ($1, $2)`, "later", + `{"module": "later", "version": "2", "state": [{"name": "s", "a-field-from-later": 1}]}`); err != nil { + t.Fatal(err) + } + known, err := inv.Catalogue(t.Context()) + if err != nil { + t.Fatalf("one manifest with an unknown key failed the whole catalogue: %v", err) + } + if known["thing"].Module != "thing" || known["thing"].UnknownField() != "" { + t.Fatalf("the other module: %+v", known["thing"]) + } + if !strings.Contains(known["later"].UnknownField(), "a-field-from-later") { + t.Fatalf("the newer manifest is not marked: %q", known["later"].UnknownField()) + } + entries, err := inv.Catalogued(t.Context()) + if err != nil || len(entries) < 2 { + t.Fatalf("the listing: %v %d", err, len(entries)) + } +}