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

@@ -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)
 		}

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" +

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

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.

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:<key>}`.
@@ -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))

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())
+	}
+}

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 {

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:<key>} 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).

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