Keep a left-out module's provisions and backups, and refuse more identity keys (hq ADR 0262)

A module left out for an unknown key inside an entry lost its whole manifest, so every consumer of what
it provides was refused and its data stopped being copied. Read past only the unknown key, keep its
backup lines, and say in the condition what stops.

cmd/mesh-controller/unknown_fields.go

@@ -38,7 +38,7 @@ func unknownFieldObservations(known map[string]catalogue.Manifest) []conditions.
 			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.",
+			Explanation: name + " uses a field this controller does not know. Until the controller is updated, nothing of it changes on its machines, and what it adds to other modules and the ports opened for it stop. Its data is still backed up.",
 			Needs:       "update the controller, or register " + name + " again at a version this controller knows.",
 			Resolved:    "the controller reads " + name + " again",
 		})

internal/catalogue/declaration.go

@@ -369,11 +369,16 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 	// Before placing, because a placement is a setting too.
 	left := r.LeftOut(with.Settings, with.Adopted)
 	kept := make([]Manifest, 0, len(r.Modules))
+	var stillBackedUp []Manifest
 	for _, m := range r.Modules {
 		if why, isLeft := left[m.Module]; isLeft {
 			if leftOut != nil {
 				leftOut[m.Module] = why
 			}
+			// **Its data is still copied** (novox/hq ADR 0262): a module left out runs nothing new, and
+			// the data it already holds on the machine is the reason to keep copying it. Only its data,
+			// as the backup holder's lines are derived from it, and the directories they name.
+			stillBackedUp = append(stillBackedUp, backupView(m))
 			continue
 		}
 		kept = append(kept, m)
@@ -979,7 +984,8 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 			// seat that places them (novox/hq ADR 0203, ADR 0204). Gathered from every module on
 			// the node, as the jails are, and **last of every placeholder pass**: shell code is a
 			// shell's own syntax, full of `${…}` no pass above should ever be shown.
-			if err := contributionsInto(copied, m, r.Modules, thisMachine, with, r.Capabilities, unplaced); err != nil {
+			contributing := append(append([]Manifest(nil), r.Modules...), stillBackedUp...)
+			if err := contributionsInto(copied, m, contributing, thisMachine, with, r.Capabilities, unplaced); err != nil {
 				return nil, err
 			}
 			copied["id"] = m.Module + "." + fmt.Sprint(resource["id"])

internal/catalogue/manifest.go

@@ -1283,16 +1283,21 @@ func (m *Manifest) UnmarshalJSON(raw []byte) error {
 		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.
+		// Read without it, at whatever depth it is (prunedFields): the module is left out of every
+		// declaration by name (LeftOut), and still provides what it provides, and still has its data
+		// copied, so nothing that requires it is refused and nothing it holds goes uncopied.
 		unknown = err.Error()
-		fields = manifestFields{}
-		if json.Unmarshal(rest, &fields) != nil {
+		var pruned []string
+		var perr error
+		if fields, pruned, perr = prunedFields(keys); perr != nil {
+			// Not read past: the manifest keeps its name and version alone. It is left out and raised
+			// all the same, and one manifest never fails the whole catalogue.
 			fields = manifestFields{}
 			_ = json.Unmarshal(keys["module"], &fields.Module)
 			_ = json.Unmarshal(keys["version"], &fields.Version)
+			unknown += " (read no further: " + perr.Error() + ")"
+		} else {
+			unknown += " (read without " + strings.Join(pruned, ", ") + ")"
 		}
 	}
 	*m = Manifest(fields)

internal/catalogue/setting_defaults.go

@@ -57,20 +57,25 @@ var operatorsOwn = map[string]bool{
 	"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, "apikey": true, "bearer": true, "cert": true, "credential": true,
+	"key": true, "apikey": true, "bearer": true, "cert": true, "certificate": true, "credential": true,
+	"nameserver": true, "gateway": true, "subnet": true, "sender": true, "recipient": true, "contact": true,
 }
 
 // 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,
+	"host-name": true, "user-name": true, "domain-name": true, "dns-server": true,
+	// Whom a rule lets in or keeps out: a list of addresses or networks.
+	"allow-from": true, "deny-from": 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.
+// `max-tokens` is a number, `show-hostname` a switch. Not `allow` or `use`: `allow-from` and
+// `use-host` name whom.
 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,
+	"disable": true,
 }
 
 // operatorsWord is what in a key's name says its value is the operator's, or "". A key is about its
@@ -87,8 +92,10 @@ func operatorsWord(key string) string {
 		}
 	}
 	if n := len(words); n > 1 {
-		if pair := words[n-2] + "-" + words[n-1]; operatorsCompounds[pair] {
-			return pair
+		for _, last := range []string{words[n-1], strings.TrimSuffix(words[n-1], "s")} {
+			if pair := words[n-2] + "-" + last; operatorsCompounds[pair] {
+				return pair
+			}
 		}
 	}
 	if last := words[len(words)-1]; operatorsOwn[last] {

internal/catalogue/setting_defaults_test.go

@@ -223,7 +223,8 @@ func TestANodeCalledDefaultIsANodesLayer(t *testing.T) {
 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", "user-agent", "url-timeout",
-		"site-title", "cert-renewal-days", "name", "font-name"} {
+		"site-title", "cert-renewal-days", "name", "font-name", "allow-resize", "use-gpu", "disable-sender-check",
+		"max-recipients", "gateway-timeout", "sender-delay"} {
 		if w := operatorsWord(key); w != "" {
 			t.Errorf("%s read as the operator's (%s)", key, w)
 		}
@@ -235,7 +236,12 @@ func TestAKeyIsTheOperatorsByWhatItIsAbout(t *testing.T) {
 		"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"} {
+		"server-name": "server-name", "public-name": "public-name", "smtp-relay": "smtp-relay",
+		"host-name": "host-name", "user-name": "user-name", "domain-name": "domain-name",
+		"nameserver": "nameserver", "upstream-nameservers": "nameserver", "dns-server": "dns-server",
+		"dns-servers": "dns-server", "default-gateway": "gateway", "lan-subnet": "subnet", "sender": "sender",
+		"notify-recipients": "recipient", "contact": "contact", "tls-certificate": "certificate",
+		"allow-from": "allow-from", "deny-from": "deny-from", "allow-hosts": "host", "use-host": "host"} {
 		if w := operatorsWord(key); w != word {
 			t.Errorf("%s: read %q, want %q", key, w, word)
 		}
@@ -332,3 +338,67 @@ func TestAStoredManifestWithAnUnknownKeyIsLeftOutAndRegistrationRefusesIt(t *tes
 		t.Fatal("a malformed stored manifest was read")
 	}
 }
+
+// A module left out for a key this controller does not know, inside an entry, is read past that key
+// alone: it still provides what it provides, its other entries are whole, and a key of the same name
+// that another entry knows is kept (novox/hq ADR 0262).
+func TestALeftOutModuleStillProvidesWhatItProvides(t *testing.T) {
+	var m Manifest
+	raw := `{"module": "later", "version": "2",
+		"provides": [{"name": "db", "scope": "mesh"}, {"name": "cache", "a-field-from-later": 1}],
+		"state": [{"name": "s", "history": 3}],
+		"data": {"own": [{"id": "d", "path": "${dir:d}", "class": "valuable", "backup": {"dump": "x", "into": "d", "class": "later"}}]},
+		"resources": [{"id": "d", "type": "directory", "mode": "0700"}]}`
+	if err := json.Unmarshal([]byte(raw), &m); err != nil {
+		t.Fatal(err)
+	}
+	if len(m.Provides) != 2 || m.Provides[0].Name != "db" || m.Provides[1].Name != "cache" {
+		t.Fatalf("provides: %+v", m.Provides)
+	}
+	if len(m.State) != 1 || m.State[0].History != 3 {
+		t.Fatalf("state: %+v", m.State)
+	}
+	if m.Data == nil || len(m.Data.Own) != 1 || m.Data.Own[0].Class != "valuable" {
+		t.Fatalf("data: the item's own class was taken for the backup's unknown one: %+v", m.Data)
+	}
+	for _, want := range []string{"provides[1].a-field-from-later", "data.own[0].backup.class"} {
+		if !strings.Contains(m.UnknownField(), want) {
+			t.Errorf("unknown %q does not say it read without %s", m.UnknownField(), want)
+		}
+	}
+	if !strings.Contains(UnknownFieldReason(m), "left out") {
+		t.Fatal(UnknownFieldReason(m))
+	}
+}
+
+// A left-out module's data is still copied: the backup holder on its machine keeps its lines while every
+// other thing of it is left out.
+func TestALeftOutModulesDataIsStillBackedUp(t *testing.T) {
+	var later Manifest
+	if err := json.Unmarshal([]byte(`{"module": "later", "version": "2", "a-field-from-later": 1,
+		"resources": [{"id": "d", "type": "directory", "mode": "0700"}, {"id": "rc", "type": "file", "path": "/etc/later.conf", "content": "x\n"}],
+		"data": {"own": [{"id": "d", "path": "${dir:d}", "class": "valuable"}]}}`), &later); err != nil {
+		t.Fatal(err)
+	}
+	holder := Manifest{Module: "backups", Version: "1",
+		Claims: []Claim{{Name: BackupSeat, Scope: ScopeNode}},
+		Resources: []map[string]any{{"id": "list", "type": "file", "path": "/etc/backups.list", "mode": "0644",
+			"content": "${contribution:" + BackupSeat + ":backup}"}}}
+	r := anAdoptedAnchor()
+	r.Modules = append(r.Modules, holder, later)
+	composed, err := r.Compose(anchorRendering(false))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, left := composed.LeftOut["later"]; !left {
+		t.Fatalf("not left out: %v", composed.LeftOut)
+	}
+	got := byID(composed.Resources)
+	if _, declared := got["later.rc"]; declared {
+		t.Fatal("the left-out module's file is still declared")
+	}
+	list, _ := got["backups.list"]["content"].(string)
+	if !strings.Contains(list, "# later") || !strings.Contains(list, "path /var/lib/later/d") {
+		t.Fatalf("the left-out module's data is no longer backed up:\n%s", list)
+	}
+}

internal/catalogue/unknown_field.go

@@ -1,7 +1,10 @@
 package catalogue
 
 import (
+	"bytes"
+	"encoding/json"
 	"errors"
+	"fmt"
 	"regexp"
 )
 
@@ -54,5 +57,145 @@ func UnknownFieldReason(m Manifest) string {
 		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)"
+		"until the controller is updated: nothing of it is changed on its machines and its contributions to " +
+		"other modules and its open ports stop, while its data is still backed up and it still provides " +
+		"what it provides (novox/hq ADR 0262)"
+}
+
+// backupView is what of a left-out module still reaches its machine: its data, so the backup holder
+// keeps copying it, and the directories and accesses its data items name. No contribution, shell code
+// or environment of its own: those are what leaving it out stops.
+func backupView(m Manifest) Manifest {
+	view := Manifest{Module: m.Module, Data: m.Data, Accesses: m.Accesses}
+	for _, r := range m.Resources {
+		if fmt.Sprint(r["type"]) == "directory" {
+			view.Resources = append(view.Resources, r)
+		}
+	}
+	return view
+}
+
+// prunedFields is a stored manifest's fields with every key this controller does not know taken out,
+// and the keys taken out (novox/hq ADR 0262). A second pass, after the strict one refused: each
+// top-level field is decoded alone, and where an entry inside it has an unknown key, the one
+// occurrence whose removal moves the decoder past it is removed — never a key of the same name that
+// the entry around it knows. So a left-out module still provides what it provides, and its data and
+// directories are still read, which its backup lines are made from.
+func prunedFields(keys map[string]json.RawMessage) (manifestFields, []string, error) {
+	var removed []string
+	tree := map[string]any{}
+	for k, raw := range keys {
+		var v any
+		dec := json.NewDecoder(bytes.NewReader(raw))
+		dec.UseNumber()
+		if err := dec.Decode(&v); err != nil {
+			return manifestFields{}, nil, err
+		}
+		tree[k] = v
+	}
+	for _, k := range sortedAnyKeys(tree) {
+		for tries := 0; ; tries++ {
+			err := decodesAlone(k, tree[k])
+			if err == nil {
+				break
+			}
+			unknown := asUnknownField(err)
+			if unknown == nil || tries > 64 {
+				return manifestFields{}, nil, err
+			}
+			if unknown.Field == k {
+				delete(tree, k)
+				removed = append(removed, k)
+				break
+			}
+			path, ok := removalThatHelps(k, tree[k], unknown.Field, err.Error())
+			if !ok {
+				// The same key unknown in two entries alike: no one removal changes the words.
+				// Every occurrence goes, and the field is judged again.
+				if removeEvery(tree[k], unknown.Field) == 0 {
+					return manifestFields{}, nil, err
+				}
+				path = "…." + unknown.Field
+			}
+			removed = append(removed, k+path)
+		}
+	}
+	raw, err := json.Marshal(tree)
+	if err != nil {
+		return manifestFields{}, nil, err
+	}
+	var fields manifestFields
+	dec := json.NewDecoder(bytes.NewReader(raw))
+	dec.DisallowUnknownFields()
+	if err := dec.Decode(&fields); err != nil {
+		return manifestFields{}, nil, err
+	}
+	return fields, removed, nil
+}
+
+// decodesAlone is whether one top-level field decodes strictly on its own.
+func decodesAlone(k string, v any) error {
+	raw, err := json.Marshal(map[string]any{k: v})
+	if err != nil {
+		return err
+	}
+	var fields manifestFields
+	dec := json.NewDecoder(bytes.NewReader(raw))
+	dec.DisallowUnknownFields()
+	return dec.Decode(&fields)
+}
+
+// removalThatHelps removes, from v, the one occurrence of key whose removal changes what the strict
+// decoder says of field k, and says where it was. Every other occurrence is left as it was.
+func removalThatHelps(k string, v any, key, said string) (string, bool) {
+	var found bool
+	var where string
+	var walk func(node any, at string) bool
+	walk = func(node any, at string) bool {
+		switch n := node.(type) {
+		case map[string]any:
+			if value, has := n[key]; has {
+				delete(n, key)
+				if err := decodesAlone(k, v); err == nil || err.Error() != said {
+					found, where = true, at+"."+key
+					return true
+				}
+				n[key] = value
+			}
+			for _, sub := range sortedAnyKeys(n) {
+				if walk(n[sub], at+"."+sub) {
+					return true
+				}
+			}
+		case []any:
+			for i, item := range n {
+				if walk(item, fmt.Sprintf("%s[%d]", at, i)) {
+					return true
+				}
+			}
+		}
+		return false
+	}
+	walk(v, "")
+	return where, found
+}
+
+// removeEvery removes key from every object in v, and says how many it removed.
+func removeEvery(v any, key string) int {
+	n := 0
+	switch node := v.(type) {
+	case map[string]any:
+		if _, has := node[key]; has {
+			delete(node, key)
+			n++
+		}
+		for _, sub := range node {
+			n += removeEvery(sub, key)
+		}
+	case []any:
+		for _, item := range node {
+			n += removeEvery(item, key)
+		}
+	}
+	return n
 }