From 7868c9f365325eeb1f07063f10f8dde4a00c5ea7 Mon Sep 17 00:00:00 2001 From: jochens Date: Fri, 13 Nov 2026 01:32:27 +0100 Subject: [PATCH] Fix 1979: ${config-json} writes only the keys the module declares under config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A key a layer holds that the manifest does not declare (scrub-pool during a rename) is not a config value of the module. Builds: design 42 zfs-owns-zfs §9, A-15, A-24, A-25 --- internal/compose/resolution/config_lines.go | 18 +++++++++++++++--- internal/compose/resolution/declaration.go | 2 +- internal/compose/resolution/setting_into.go | 7 ++++--- .../compose/resolution/setting_into_test.go | 4 +++- internal/compose/resolution/settings.go | 2 +- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/internal/compose/resolution/config_lines.go b/internal/compose/resolution/config_lines.go index 4758f3fe..456f6e8d 100644 --- a/internal/compose/resolution/config_lines.go +++ b/internal/compose/resolution/config_lines.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "regexp" + "slices" "strconv" "strings" @@ -66,16 +67,18 @@ func configLinesInto(content string, layers []mcat.Layer, module string) (string } // configJSONInto writes `${config-json}` in a file's content as the module's set config values in one JSON object -// (design 42 zfs-owns-zfs §9, answer A-15): what no layer sets is absent, and no unset value refuses the module; a +// (design 42 zfs-owns-zfs §9, answer A-15): only keys the module declares under `config` are written, and what no layer sets is absent, and no unset value refuses the module; a // list and an object are written as JSON, a number, a boolean and a text as JSON values of their own type. -func configJSONInto(content string, layers []mcat.Layer) (string, error) { +func configJSONInto(content string, layers []mcat.Layer, declared []string) (string, error) { if !configJSONRef.MatchString(content) { return content, nil } set := map[string]any{} for _, layer := range layers { for k, v := range layer.Values { - set[k] = v + if slices.Contains(declared, k) { + set[k] = v + } } } var out strings.Builder @@ -88,3 +91,12 @@ func configJSONInto(content string, layers []mcat.Layer) (string, error) { object := strings.TrimSuffix(out.String(), "\n") return configJSONRef.ReplaceAllLiteralString(content, object), nil } + +// declaredConfig is the keys a module's manifest declares under `config`. +func declaredConfig(m mcat.Manifest) []string { + keys := make([]string, 0, len(m.Settings)) + for k := range m.Settings { + keys = append(keys, k) + } + return keys +} diff --git a/internal/compose/resolution/declaration.go b/internal/compose/resolution/declaration.go index d72d33fe..faa2202d 100644 --- a/internal/compose/resolution/declaration.go +++ b/internal/compose/resolution/declaration.go @@ -909,7 +909,7 @@ func resolutionCompose(r mcat.Resolution, with mcat.Rendering, owner map[string] // **An operator's value, from the assignment** (novox/hq ADR 0112, ADR 0155): what a // definition may not carry because it is true of one installation only. Filled from // the same layers a mergeable file takes, and refused when no layer set it. - if err := unsetInConfigWords(settingInto(copied, WithDefaults(m, with.Settings[m.Module]), m.Module), m); err != nil { + if err := unsetInConfigWords(settingInto(copied, WithDefaults(m, with.Settings[m.Module]), m.Module, declaredConfig(m)...), m); err != nil { return nil, err } // **Placed before anything reads a path.** A pathless directory receives the path diff --git a/internal/compose/resolution/setting_into.go b/internal/compose/resolution/setting_into.go index 61b69a48..984621bc 100644 --- a/internal/compose/resolution/setting_into.go +++ b/internal/compose/resolution/setting_into.go @@ -65,8 +65,9 @@ func unsetInConfigWords(err error, m mcat.Manifest) error { // The last layer setting a key wins, which is the node's over the mesh's over the module's own // default (novox/hq ADR 0262) — the same order settle applies to a mergeable file. The caller lays // 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 []mcat.Layer, module string) error { +// it (a number without a trailing .000000, a boolean as true/false). declared are the keys the module's manifest +// declares under `config`, the only ones `${config-json}` writes (design 42 §9, answer A-15). +func settingInto(resource map[string]any, layers []mcat.Layer, module string, declared ...string) error { if fmt.Sprint(resource["type"]) == "service" { return settingIntoUnit(resource, layers, module) } @@ -87,7 +88,7 @@ func settingInto(resource map[string]any, layers []mcat.Layer, module string) er if err != nil { return err } - if content, err = configJSONInto(content, layers); err != nil { + if content, err = configJSONInto(content, layers, declared); err != nil { return err } for _, key := range settingsUsed(content) { diff --git a/internal/compose/resolution/setting_into_test.go b/internal/compose/resolution/setting_into_test.go index a2c0a98f..f497f411 100644 --- a/internal/compose/resolution/setting_into_test.go +++ b/internal/compose/resolution/setting_into_test.go @@ -17,7 +17,7 @@ func TestAConfigValueReachesAResourceAndDecidesWhetherItIsDeclared(t *testing.T) filled := func(content string) func([]mcat.Layer) (string, error) { return func(layers []mcat.Layer) (string, error) { file := map[string]any{"id": "env", "type": "file", "path": "/x/mail.env", "content": content} - err := settingInto(file, layers, "mail") + err := settingInto(file, layers, "mail", "n", "b", "s", "l", "o") return file["content"].(string), err } } @@ -66,6 +66,8 @@ func TestAConfigValueReachesAResourceAndDecidesWhetherItIsDeclared(t *testing.T) {"config-json: the set values only, each as its own type", values(map[string]any{"n": float64(64), "b": true, "s": "x", "l": []any{"a"}, "o": map[string]any{"f": float64(1)}}), filled("${config-json}\n"), "{\n \"b\": true,\n \"l\": [\n \"a\"\n ],\n \"n\": 64,\n \"o\": {\n \"f\": 1\n },\n \"s\": \"x\"\n}\n", nil}, + {"config-json: a key the module does not declare is not written", values(map[string]any{"s": "x", "scrub-pool": "tank"}), + filled("${config-json}\n"), "{\n \"s\": \"x\"\n}\n", nil}, {"config-json: nothing set is an empty object, not a refusal", nil, filled("${config-json}\n"), "{}\n", nil}, {"if-config: a key never set leaves it out", nil, declared("k"), "plain", nil}, {"if-config: a key set to text declares it", values(map[string]any{"k": "x"}), declared("k"), "plain,cond", nil}, diff --git a/internal/compose/resolution/settings.go b/internal/compose/resolution/settings.go index 71c1864f..ccc3037a 100644 --- a/internal/compose/resolution/settings.go +++ b/internal/compose/resolution/settings.go @@ -458,7 +458,7 @@ func JudgeSettings(m mcat.Manifest, layers []mcat.Layer, adopted bool) error { for k, v := range settled { copied[k] = v } - if err := unsetInConfigWords(settingInto(copied, WithDefaults(m, layers), m.Module), m); err != nil { + if err := unsetInConfigWords(settingInto(copied, WithDefaults(m, layers), m.Module, declaredConfig(m)...), m); err != nil { return err } }