diff --git a/internal/catalogue/setting_in_unit_test.go b/internal/catalogue/setting_in_unit_test.go new file mode 100644 index 00000000..bac0d0ad --- /dev/null +++ b/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) + } +} diff --git a/internal/catalogue/setting_into.go b/internal/catalogue/setting_into.go index caa2a18d..05ea8872 100644 --- a/internal/catalogue/setting_into.go +++ b/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:}` in a file's content is filled from the module's settings layers — the mesh's, +// `${setting:}` 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 ` 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 ` 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) {