Fill ${setting:} in a service's unit name, and refuse a value that makes no unit name

A module that holds the distribution's scrub timer for the pool the operator names
(zfs-scrub-weekly@<pool>.timer) cannot write the pool into its definition (hq ADR 0112). Settings were
substituted only into file content, so the unit reached the machine as
"zfs-scrub-weekly@${setting:scrub-pool}.timer", a unit no machine has, and the apply failed far from its
cause (second review of mesh-catalog #147).

A service's unit now takes ${setting:} from the same layers a file does, and is refused by key when
nothing sets it. A value is also refused unless it is only letters, digits, ':', '_', '.' and '-': the
name ends up in unit files and systemctl arguments, and a space, a slash or a newline must never reach
them. A key a unit asks for is not called stray.

internal/catalogue/setting_in_unit_test.go

@@ -0,0 +1,109 @@
+package catalogue
+
+import (
+	"strings"
+	"testing"
+)
+
+// A setting may name the unit a service resource holds: a module that stops the distribution's own timer
+// for the pool the operator names cannot write the pool into its definition (novox/hq ADR 0112), and a
+// unit name with ${setting:…} left in it is a unit no machine has, so the apply fails far from its cause.
+
+func scrubber() Manifest {
+	return Manifest{Module: "zfs",
+		Settings: map[string]SettingDeclaration{
+			"scrub-cadence": {Kind: KindPreference, Default: "weekly", Why: "what the distribution's timer did"},
+		},
+		Resources: []map[string]any{
+			{"id": "distribution-weekly", "type": "service", "unit": "zfs-scrub-weekly@${setting:scrub-pool}.timer",
+				"state": "stopped", "boot": "disabled"},
+			{"id": "distribution-monthly", "type": "service", "unit": "zfs-scrub-monthly@${setting:scrub-pool}.timer",
+				"state": "stopped", "boot": "disabled"},
+			{"id": "scrub-config", "type": "file", "path": "/etc/zfs-tools/scrub.conf",
+				"content": "pool=${setting:scrub-pool}\ncadence=${setting:scrub-cadence}\n"},
+		},
+	}
+}
+
+func TestASettingNamesAServicesUnit(t *testing.T) {
+	m := scrubber()
+	layers := WithDefaults(m, []Layer{{From: "ace", Values: map[string]any{"scrub-pool": "storage", "scrub-cadence": "monthly"}}})
+	for i, want := range []string{"zfs-scrub-weekly@storage.timer", "zfs-scrub-monthly@storage.timer"} {
+		r := map[string]any{}
+		for k, v := range m.Resources[i] {
+			r[k] = v
+		}
+		if err := settingInto(r, layers, m.Module); err != nil {
+			t.Fatal(err)
+		}
+		if r["unit"] != want {
+			t.Errorf("composed as %q, not %q", r["unit"], want)
+		}
+	}
+}
+
+// Composed for a machine, the way a push writes it: the operator's pool and cadence reach the units' names
+// and the file.
+func TestAComposedMachineGetsTheUnitTheSettingNames(t *testing.T) {
+	r := anAdoptedAnchor()
+	r.Modules = append(r.Modules, scrubber())
+	with := anchorRendering(false)
+	with.Settings["zfs"] = []Layer{{From: "anchor", Values: map[string]any{"scrub-pool": "storage", "scrub-cadence": "monthly"}}}
+	composed, err := r.Compose(with)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if why, left := composed.LeftOut["zfs"]; left {
+		t.Fatalf("left out: %s", why)
+	}
+	got := byID(composed.Resources)
+	for id, want := range map[string]string{"zfs.distribution-weekly": "zfs-scrub-weekly@storage.timer",
+		"zfs.distribution-monthly": "zfs-scrub-monthly@storage.timer"} {
+		if got[id]["unit"] != want {
+			t.Errorf("%s composed as %v, not %s", id, got[id]["unit"], want)
+		}
+		t.Logf("%s: %v", id, got[id]["unit"])
+	}
+	if c := got["zfs.scrub-config"]["content"]; c != "pool=storage\ncadence=monthly\n" {
+		t.Errorf("the file: %q", c)
+	}
+}
+
+// A value that would not make a unit's name is refused by name, never written: a space, a slash, a
+// newline that would begin a directive, or a second suffix.
+func TestASettingThatMakesNoUnitNameIsRefused(t *testing.T) {
+	m := scrubber()
+	for _, bad := range []string{"", "stor age", "a/b", "storage\nExecStart=/bin/sh", "a@b", "$(x)", "*", `a\\x2d`} {
+		r := map[string]any{}
+		for k, v := range m.Resources[0] {
+			r[k] = v
+		}
+		err := settingInto(r, []Layer{{From: "ace", Values: map[string]any{"scrub-pool": bad}}}, m.Module)
+		if err == nil || !strings.Contains(err.Error(), "scrub-pool") || !strings.Contains(err.Error(), "unit") {
+			t.Errorf("%q: %v", bad, err)
+		}
+		if r["unit"] != m.Resources[0]["unit"] {
+			t.Errorf("%q: the unit was changed on refusal: %v", bad, r["unit"])
+		}
+	}
+	r := map[string]any{}
+	for k, v := range m.Resources[0] {
+		r[k] = v
+	}
+	if err := settingInto(r, nil, m.Module); err == nil || !strings.Contains(err.Error(), "${setting:scrub-pool}") {
+		t.Errorf("nothing set: %v", err)
+	}
+}
+
+// A key a unit's name asks for is a destination, so setting it is not called stray, and judging the
+// settings sees it.
+func TestASettingAUnitAsksForIsNotStrayAndIsJudged(t *testing.T) {
+	m := scrubber()
+	stray := strings.Join(UnusedSettings(m, []Layer{{From: "ace", Values: map[string]any{"scrub-pool": "storage"}}}), "; ")
+	if strings.Contains(stray, "scrub-pool") {
+		t.Errorf("called stray: %s", stray)
+	}
+	if err := JudgeSettings(m, nil, false); err == nil || !strings.Contains(err.Error(), "scrub-pool") {
+		t.Errorf("a unit's setting nothing sets is not refused when judged: %v", err)
+	}
+}

internal/catalogue/setting_into.go

@@ -15,7 +15,7 @@ import (
 // (novox/hq issues 122, 134). ADR 0112 names the operator as one of the four providers; this is the
 // operator answering.
 //
-// `${setting:<key>}` in a file's content is filled from the module's settings layers — the mesh's,
+// `${setting:<key>}` in a file's content, and in a service's unit name, is filled from the module's settings layers — the mesh's,
 // then this node's — the same layers a mergeable JSON file and a contribution already take, so
 // `settings set <module>` is the one place a person's values go. **Refused when no layer sets it**,
 // naming the key and the remedy: a definition that carried a default for a mail domain would be
@@ -45,6 +45,9 @@ func settingsUsed(content string) []string {
 // 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"]) == "service" {
+		return settingIntoUnit(resource, layers, module)
+	}
 	if fmt.Sprint(resource["type"]) != "file" {
 		return nil
 	}
@@ -68,6 +71,40 @@ func settingInto(resource map[string]any, layers []Layer, module string) error {
 	return nil
 }
 
+// unitPart is what a setting may put into a unit's name: the characters systemd allows in a unit name,
+// less the instance's `@` and the escape's `\`, and at least one of them. Anything else — a space, a slash,
+// a newline that would begin a directive in the unit file the name ends up in — is refused, never written.
+var unitPart = regexp.MustCompile(`^[A-Za-z0-9:_.-]+$`)
+
+// settingIntoUnit fills ${setting:…} in a service resource's unit name: a module that holds the
+// distribution's timer for the pool the operator names cannot write the pool into its definition
+// (novox/hq ADR 0112). Refused, with the key and why, when nothing sets it or the value would not make a
+// unit's name; the resource is left as it was.
+func settingIntoUnit(resource map[string]any, layers []Layer, module string) error {
+	unit, ok := resource["unit"].(string)
+	if !ok {
+		return nil
+	}
+	for _, key := range settingsUsed(unit) {
+		value, set := settingValue(layers, key)
+		if !set {
+			return fmt.Errorf(
+				"%s has a service whose unit says ${setting:%s}, and nothing sets %q for it — an operator's "+
+					"value is the assignment's, never the definition's (novox/hq ADR 0112): "+
+					"`settings set %s <file>` with {%q: …}%s",
+				module, key, key, module, key, orNoSettings(layers))
+		}
+		v := plainly(value)
+		if !unitPart.MatchString(v) {
+			return fmt.Errorf("%s: the setting %q is %q, which cannot be part of the unit %s: a unit's name takes "+
+				"letters, digits, ':', '_', '.' and '-' only", module, key, v, unit)
+		}
+		unit = strings.ReplaceAll(unit, "${setting:"+key+"}", v)
+	}
+	resource["unit"] = unit
+	return nil
+}
+
 func settingValue(layers []Layer, key string) (any, bool) {
 	var value any
 	set := false
@@ -106,11 +143,15 @@ func settingKeysUsedBy(m Manifest) map[string]bool {
 		}
 	}
 	for _, r := range m.Resources {
-		if fmt.Sprint(r["type"]) != "file" {
-			continue
-		}
-		if content, ok := r["content"].(string); ok {
-			note(content)
+		switch fmt.Sprint(r["type"]) {
+		case "file":
+			if content, ok := r["content"].(string); ok {
+				note(content)
+			}
+		case "service":
+			if unit, ok := r["unit"].(string); ok {
+				note(unit)
+			}
 		}
 	}
 	inValues := func(values map[string]any) {