Place a left-out module's backup lines best effort, and refuse more identity keys (hq ADR 0262 review)

An unplaceable line of a left-out module, such as an access nobody placed, failed the whole machine's
declaration. Say it among what could not be placed instead, never copy the definition's path past a
placement that does not read, and accept a removal only when the decoder is past it.

internal/catalogue/manifest.go

@@ -471,6 +471,11 @@ type Manifest struct {
 	// machine's declaration by name until the controller is updated (LeftOut).
 	unknown string
 
+	// bestEffort marks the view of a left-out module that only its backup lines are made from
+	// (backupView, novox/hq ADR 0262): a line of it that cannot be placed is said in the plan's list of
+	// what could not be placed, never an error that would cost the whole machine its declaration.
+	bestEffort bool
+
 	// 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

internal/catalogue/seat_contributions.go

@@ -212,9 +212,20 @@ func seatContributions(modules []Manifest, holder Manifest, placeholder string,
 		return shapedContributions(modules, holder, facts["name"], s, r, where, caps)
 	}
 	var failed error
+	var unplacedLines []string
 	var b strings.Builder
 	for _, m := range inModuleOrder(modules) {
 		named := false
+		// A left-out module's lines are best effort (novox/hq ADR 0262): what cannot be placed is said,
+		// and the machine is declared without it. A placement setting that does not read is not
+		// replaced by the definition's own path, which may not be where the data is.
+		if m.bestEffort && r.Dirs {
+			if _, err := Places(m, with.Settings[m.Module]); err != nil {
+				unplacedLines = append(unplacedLines, fmt.Sprintf("%s's %s for %s, kept while it is left out, "+
+					"is not placed: its placement setting does not read (%v)", m.Module, kind, s.Name, err))
+				continue
+			}
+		}
 		for _, c := range m.allContributions() {
 			if c.Kind != kind || !capable(c, caps) {
 				continue
@@ -222,15 +233,12 @@ func seatContributions(modules []Manifest, holder Manifest, placeholder string,
 			if cs, known := SeatNamed(c.Seat); !known || cs.Name != s.Name {
 				continue
 			}
-			if !named {
-				fmt.Fprintf(&b, "%s %s\n", r.Comment, m.Module)
-				named = true
-			}
 			content := c.Content
+			var lineFailed error
 			if r.Dirs {
 				filled, err := dirFill(content, dirsFor(m, with), m.Module)
-				if err != nil && failed == nil {
-					failed = err
+				if err != nil && lineFailed == nil {
+					lineFailed = err
 				}
 				// An operator's path the module was given, as an item of data on it (novox/hq ADR 0233).
 				if accessRef.MatchString(filled) {
@@ -238,15 +246,15 @@ func seatContributions(modules []Manifest, holder Manifest, placeholder string,
 					if err == nil {
 						filled, err = accessFill(filled, byID, m.Module)
 					}
-					if err != nil && failed == nil {
-						failed = err
+					if err != nil && lineFailed == nil {
+						lineFailed = err
 					}
 				}
 				for _, key := range machineUsed(filled) {
 					value, has := facts[key]
 					if !has {
-						if failed == nil {
-							failed = fmt.Errorf("%s's %s for %s says ${machine:%s}, and this machine says %s",
+						if lineFailed == nil {
+							lineFailed = fmt.Errorf("%s's %s for %s says ${machine:%s}, and this machine says %s",
 								m.Module, kind, s.Name, key, orNothing(namesOfFacts(facts)))
 						}
 						continue
@@ -255,11 +263,25 @@ func seatContributions(modules []Manifest, holder Manifest, placeholder string,
 				}
 				content = filled
 			}
+			if lineFailed != nil {
+				if m.bestEffort {
+					unplacedLines = append(unplacedLines, fmt.Sprintf("%s's %s for %s, kept while it is left "+
+						"out, is not placed: %v", m.Module, kind, s.Name, lineFailed))
+					continue
+				}
+				if failed == nil {
+					failed = lineFailed
+				}
+			}
+			if !named {
+				fmt.Fprintf(&b, "%s %s\n", r.Comment, m.Module)
+				named = true
+			}
 			b.WriteString(content)
 			if !strings.HasSuffix(content, "\n") {
 				b.WriteString("\n")
 			}
 		}
 	}
-	return b.String(), nil, failed
+	return b.String(), unplacedLines, failed
 }

internal/catalogue/setting_defaults.go

@@ -59,6 +59,8 @@ var operatorsOwn = map[string]bool{
 	"password": true, "pass": true, "passwd": true, "passphrase": true, "secret": true, "token": 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,
+	"trusted": true, "whitelist": true, "peer": true, "bind": true, "listen": true, "upstream": true,
+	"proxy": true, "admin": true, "mac": true,
 }
 
 // operatorsCompounds are names of two words that are the operator's though neither word alone says so
@@ -67,7 +69,7 @@ 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,
+	"allow-from": true, "deny-from": true, "allow-list": true,
 }
 
 // aboutAnAmount are first words that make a key about how many or whether, never about whom:
@@ -87,11 +89,19 @@ func operatorsWord(key string) string {
 		return ""
 	}
 	for i, w := range words {
-		if !operatorsOwn[w] && strings.HasSuffix(w, "s") && operatorsOwn[strings.TrimSuffix(w, "s")] {
+		switch {
+		case operatorsOwn[w]:
+		case strings.HasSuffix(w, "ies") && operatorsOwn[strings.TrimSuffix(w, "ies")+"y"]:
+			words[i] = strings.TrimSuffix(w, "ies") + "y"
+		case strings.HasSuffix(w, "s") && operatorsOwn[strings.TrimSuffix(w, "s")]:
 			words[i] = strings.TrimSuffix(w, "s")
 		}
 	}
 	if n := len(words); n > 1 {
+		// Where a secret or an identity is kept is the operator's too: `password-file`, `token-path`.
+		if (words[n-1] == "file" || words[n-1] == "path") && operatorsOwn[words[n-2]] {
+			return words[n-2] + "-" + words[n-1]
+		}
 		for _, last := range []string{words[n-1], strings.TrimSuffix(words[n-1], "s")} {
 			if pair := words[n-2] + "-" + last; operatorsCompounds[pair] {
 				return pair

internal/catalogue/setting_defaults_test.go

@@ -224,7 +224,9 @@ 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", "allow-resize", "use-gpu", "disable-sender-check",
-		"max-recipients", "gateway-timeout", "sender-delay"} {
+		"max-recipients", "gateway-timeout", "sender-delay", "upstream-resolvers", "mirror-countries",
+		"pool-region", "proxy-timeout", "listen-backlog", "peer-keepalive", "admin-theme", "log-file",
+		"cache-path"} {
 		if w := operatorsWord(key); w != "" {
 			t.Errorf("%s read as the operator's (%s)", key, w)
 		}
@@ -241,7 +243,12 @@ func TestAKeyIsTheOperatorsByWhatItIsAbout(t *testing.T) {
 		"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"} {
+		"allow-from": "allow-from", "deny-from": "deny-from", "allow-hosts": "host", "use-host": "host",
+		"trusted": "trusted", "allow-list": "allow-list", "ip-whitelist": "whitelist", "peer": "peer",
+		"wireguard-peers": "peer", "bind": "bind", "listen": "listen", "upstream": "upstream", "http-proxy": "proxy",
+		"trusted-proxies": "proxy", "admin": "admin", "notify-admin": "admin", "wake-mac": "mac",
+		"password-file": "password-file", "key-file": "key-file", "token-path": "token-path",
+		"secret-file": "secret-file", "cert-path": "cert-path"} {
 		if w := operatorsWord(key); w != word {
 			t.Errorf("%s: read %q, want %q", key, w, word)
 		}
@@ -402,3 +409,42 @@ func TestALeftOutModulesDataIsStillBackedUp(t *testing.T) {
 		t.Fatalf("the left-out module's data is no longer backed up:\n%s", list)
 	}
 }
+
+// A left-out module's backup lines are best effort: a line that cannot be placed — an access nobody
+// placed, a placement setting that does not read — is said among what could not be placed, and the
+// machine is declared (novox/hq ADR 0262).
+func TestALeftOutModulesUnplaceableBackupLineCostsOnlyThatLine(t *testing.T) {
+	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}"}}}
+	media := Manifest{Module: "media", Version: "1",
+		Accesses: []Access{{ID: "library", Mode: "read"}},
+		Data:     &Data{Own: []DataItem{{ID: "library", Path: "${access:library}", Class: "valuable"}}}}
+	placedBadly := Manifest{Module: "notes", Version: "1",
+		Resources: []map[string]any{{"id": "d", "type": "directory", "path": "/srv/notes", "mode": "0700"}},
+		Data:      &Data{Own: []DataItem{{ID: "d", Path: "${dir:d}", Class: "valuable"}}}}
+	r := anAdoptedAnchor()
+	r.Modules = append(r.Modules, holder, media, placedBadly)
+	with := anchorRendering(false)
+	with.Settings["notes"] = []Layer{{From: "anchor", Values: map[string]any{PlacesSetting: "not a map"}}}
+	composed, err := r.Compose(with)
+	if err != nil {
+		t.Fatalf("an unplaceable line of a left-out module failed the machine: %v", err)
+	}
+	for _, m := range []string{"media", "notes"} {
+		if _, left := composed.LeftOut[m]; !left {
+			t.Errorf("%s is not left out: %v", m, composed.LeftOut)
+		}
+	}
+	unplaced := strings.Join(composed.Unplaced, "\n")
+	for _, want := range []string{"media's backup for node-backup", "notes's backup for node-backup", "placement setting does not read"} {
+		if !strings.Contains(unplaced, want) {
+			t.Errorf("not said among what could not be placed: %q in\n%s", want, unplaced)
+		}
+	}
+	list, _ := byID(composed.Resources)["backups.list"]["content"].(string)
+	if strings.Contains(list, "/srv/notes") || strings.Contains(list, "${") {
+		t.Fatalf("a line was placed from the definition's default or unfilled:\n%s", list)
+	}
+}

internal/catalogue/unknown_field.go

@@ -58,15 +58,16 @@ func UnknownFieldReason(m Manifest) string {
 	}
 	return m.Module + " uses a field this controller does not know (" + m.unknown + "); it is left out " +
 		"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)"
+		"other modules and its open ports stop. Its data is still backed up as this controller reads it, " +
+		"which may not be what its newer manifest asks, 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}
+	view := Manifest{Module: m.Module, Data: m.Data, Accesses: m.Accesses, bestEffort: true}
 	for _, r := range m.Resources {
 		if fmt.Sprint(r["type"]) == "directory" {
 			view.Resources = append(view.Resources, r)
@@ -156,7 +157,11 @@ func removalThatHelps(k string, v any, key, said string) (string, bool) {
 		case map[string]any:
 			if value, has := n[key]; has {
 				delete(n, key)
-				if err := decodesAlone(k, v); err == nil || err.Error() != said {
+				// Accepted only when the decoder is past it: nothing left, or an unknown key said
+				// elsewhere. A different kind of error means the removal broke the entry; the same
+				// words mean this was not the occurrence it refused.
+				err := decodesAlone(k, v)
+				if err == nil || (asUnknownField(err) != nil && err.Error() != said) {
 					found, where = true, at+"."+key
 					return true
 				}