diff --git a/loader/loader.go b/loader/loader.go index 24911eb75..167fb6903 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -497,7 +497,11 @@ func loadYamlFile(ctx context.Context, } - dict, err = override.Merge(dict, cfg) + var mergePaths []tree.Path + if rp, ok := processor.(*ResetProcessor); ok { + mergePaths = rp.MergePaths() + } + dict, err = override.MergeWithPositionalPaths(dict, cfg, mergePaths) if err != nil { return err } diff --git a/loader/merge_positional_test.go b/loader/merge_positional_test.go new file mode 100644 index 000000000..6a28c5757 --- /dev/null +++ b/loader/merge_positional_test.go @@ -0,0 +1,113 @@ +/* + Copyright 2020 The Compose Specification Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package loader + +import ( + "context" + "testing" + + "github.com/compose-spec/compose-go/v2/types" + "gotest.tools/v3/assert" +) + +func loadMerge(t *testing.T, base, override string) (*types.Project, error) { + t.Helper() + return LoadWithContext(context.Background(), types.ConfigDetails{ + ConfigFiles: []types.ConfigFile{ + {Filename: "compose.yaml", Content: []byte(base)}, + {Filename: "compose.override.yaml", Content: []byte(override)}, + }, + }, func(o *Options) { o.SkipNormalization = true }) +} + +// The `!merge` tag turns the default sequence handling into a positional merge: +// an override sequence is aligned with the base by index, and a `- {}` element +// is a no-op that leaves the corresponding base element untouched. Here a single +// argument of `command` is changed — note that `command` normally *replaces* the +// whole sequence, so this also shows `!merge` taking precedence over that rule. +func TestMerge_CommandSingleArg(t *testing.T) { + base := ` +name: merge-example +services: + app: + image: alpine + command: + - server + - --port + - "8080" + - --verbose +` + override := ` +services: + app: + command: !merge + - {} + - {} + - "9090" +` + p, err := loadMerge(t, base, override) + assert.NilError(t, err) + assert.DeepEqual(t, []string(p.Services["app"].Command), []string{"server", "--port", "9090", "--verbose"}) +} + +// `dns` normally *appends*. With `!merge` the override replaces the first entry +// positionally and keeps the second (`- {}`), instead of appending. +func TestMerge_DnsPositional(t *testing.T) { + base := ` +name: merge-example +services: + app: + image: alpine + dns: + - 1.1.1.1 + - 8.8.8.8 +` + override := ` +services: + app: + dns: !merge + - 9.9.9.9 + - {} +` + p, err := loadMerge(t, base, override) + assert.NilError(t, err) + assert.DeepEqual(t, []string(p.Services["app"].DNS), []string{"9.9.9.9", "8.8.8.8"}) +} + +// The tag is ignored on anything that is not a sequence, so it is a harmless +// no-op there and the mapping merges as usual. +func TestMerge_IgnoredOnMapping(t *testing.T) { + base := ` +name: merge-example +services: + app: + image: alpine + environment: + FOO: bar +` + override := ` +services: + app: + environment: !merge + BAZ: qux +` + p, err := loadMerge(t, base, override) + assert.NilError(t, err) + env := p.Services["app"].Environment + assert.Equal(t, *env["FOO"], "bar") + assert.Equal(t, *env["BAZ"], "qux") +} diff --git a/loader/reset.go b/loader/reset.go index 78836b5ca..3ff4473a6 100644 --- a/loader/reset.go +++ b/loader/reset.go @@ -40,6 +40,7 @@ type nodeCache struct { type ResetProcessor struct { target any paths []tree.Path + mergePaths []tree.Path visitedNodes map[*yaml.Node][]tree.Path resolvedNodes map[*yaml.Node]nodeCache visitCount int @@ -47,6 +48,13 @@ type ResetProcessor struct { maxNodeVisits int } +// MergePaths returns the paths of the sequences tagged with `!merge` in the +// processed document. The loader passes them to override.MergeWithPositionalPaths +// so those sequences are merged element by element instead of appended. +func (p *ResetProcessor) MergePaths() []tree.Path { + return p.mergePaths +} + // UnmarshalYAML implement yaml.Unmarshaler func (p *ResetProcessor) UnmarshalYAML(value *yaml.Node) error { p.visitedNodes = make(map[*yaml.Node][]tree.Path) @@ -88,6 +96,17 @@ func (p *ResetProcessor) resolveReset(node *yaml.Node, path tree.Path) (*yaml.No p.paths = append(p.paths, path) return node, nil } + // `!merge` requests a positional (element by element) merge of a sequence. + // Record the path so the merge step handles it, strip the tag so the node + // decodes as a plain sequence, then keep processing its children. The tag is + // ignored on anything that is not a sequence. + if node.Tag == "!merge" { + node.Tag = "" + if node.Kind == yaml.SequenceNode { + p.mergePaths = append(p.mergePaths, path) + return p.resolveContainer(node, path) + } + } // If the node is an alias, process the alias target via the cache so each anchor is // processed at most once. diff --git a/override/merge.go b/override/merge.go index 525299cd2..3da2a3184 100644 --- a/override/merge.go +++ b/override/merge.go @@ -20,13 +20,22 @@ import ( "cmp" "fmt" "slices" + "strconv" "github.com/compose-spec/compose-go/v2/tree" ) // Merge applies overrides to a config model func Merge(right, left map[string]any) (map[string]any, error) { - merged, err := MergeYaml(right, left, tree.NewPath()) + return MergeWithPositionalPaths(right, left, nil) +} + +// MergeWithPositionalPaths is like Merge but performs a positional (element by +// element) merge for the sequences located at the given paths, instead of the +// default append. These paths are collected by the loader from sequences tagged +// with `!merge` in an override file. See mergePositional for the semantics. +func MergeWithPositionalPaths(right, left map[string]any, positional []tree.Path) (map[string]any, error) { + merged, err := mergeYaml(right, left, tree.NewPath(), positional) if err != nil { return nil, err } @@ -72,6 +81,19 @@ func init() { // MergeYaml merges map[string]any yaml trees handling special rules func MergeYaml(e any, o any, p tree.Path) (any, error) { + return mergeYaml(e, o, p, nil) +} + +// mergeYaml is MergeYaml with the set of positional-merge paths threaded through +// the recursion. A sequence whose path matches one of them is merged element by +// element (see mergePositional) — this takes precedence over both the default +// append and the mergeSpecials rules, since it is an explicit user request. +func mergeYaml(e any, o any, p tree.Path, positional []tree.Path) (any, error) { + for _, mp := range positional { + if p.Matches(mp) { + return mergePositional(e, o, p, positional) + } + } for pattern, merger := range mergeSpecials { if p.Matches(pattern) { merged, err := merger(e, o, p) @@ -90,7 +112,7 @@ func MergeYaml(e any, o any, p tree.Path) (any, error) { if !ok { return nil, fmt.Errorf("cannot override %s", p) } - return mergeMappings(value, other, p) + return mergeMappings(value, other, p, positional) case []any: other, ok := o.([]any) if !ok { @@ -102,7 +124,60 @@ func MergeYaml(e any, o any, p tree.Path) (any, error) { } } -func mergeMappings(mapping map[string]any, other map[string]any, p tree.Path) (map[string]any, error) { +// mergePositional merges two sequences element by element, by index, instead of +// appending. It backs the `!merge` tag: an override can align its sequence with +// the base and touch a single element while leaving the others untouched with a +// no-op entry (`- {}` or `- null`). Semantics per index i: +// +// - override shorter than base at i: keep base[i] +// - base shorter than override at i: take override[i] (the sequence is extended) +// - override[i] is a no-op (nil / empty map / empty seq): keep base[i] +// - otherwise: recursively merge base[i] with override[i] (deep-merge for +// mappings, replace for scalars) +func mergePositional(e any, o any, p tree.Path, positional []tree.Path) (any, error) { + base, ok1 := e.([]any) + over, ok2 := o.([]any) + if !ok1 || !ok2 { + // The loader only records the tag for sequences, so this is defensive: + // fall back to a plain override rather than failing. + return o, nil + } + n := max(len(base), len(over)) + out := make([]any, 0, n) + for i := range n { + switch { + case i >= len(over): + out = append(out, base[i]) + case i >= len(base): + out = append(out, over[i]) + case isNoOp(over[i]): + out = append(out, base[i]) + default: + merged, err := mergeYaml(base[i], over[i], p.Next(strconv.Itoa(i)), positional) + if err != nil { + return nil, err + } + out = append(out, merged) + } + } + return out, nil +} + +// isNoOp reports whether a positional override element must leave the base +// element untouched: an explicit null or an empty container (`- {}`). +func isNoOp(x any) bool { + switch v := x.(type) { + case nil: + return true + case map[string]any: + return len(v) == 0 + case []any: + return len(v) == 0 + } + return false +} + +func mergeMappings(mapping map[string]any, other map[string]any, p tree.Path, positional []tree.Path) (map[string]any, error) { for k, v := range other { e, ok := mapping[k] if !ok { @@ -110,7 +185,7 @@ func mergeMappings(mapping map[string]any, other map[string]any, p tree.Path) (m continue } next := p.Next(k) - merged, err := MergeYaml(e, v, next) + merged, err := mergeYaml(e, v, next, positional) if err != nil { return nil, err } @@ -127,7 +202,7 @@ func mergeLogging(c any, o any, p tree.Path) (any, error) { d, ok1 := other["driver"] o, ok2 := config["driver"] if d == o || !ok1 || !ok2 { - return mergeMappings(config, other, p) + return mergeMappings(config, other, p, nil) } return other, nil } @@ -144,7 +219,7 @@ func mergeBuild(c any, o any, path tree.Path) (any, error) { } return nil } - return mergeMappings(toBuild(c), toBuild(o), path) + return mergeMappings(toBuild(c), toBuild(o), path, nil) } func mergeDependsOn(c any, o any, path tree.Path) (any, error) { @@ -156,19 +231,19 @@ func mergeDependsOn(c any, o any, path tree.Path) (any, error) { "condition": "service_started", "required": true, }) - return mergeMappings(right, left, path) + return mergeMappings(right, left, path, nil) } func mergeModels(c any, o any, path tree.Path) (any, error) { right := convertIntoMapping(c, nil) left := convertIntoMapping(o, nil) - return mergeMappings(right, left, path) + return mergeMappings(right, left, path, nil) } func mergeNetworks(c any, o any, path tree.Path) (any, error) { right := convertIntoMapping(c, nil) left := convertIntoMapping(o, nil) - return mergeMappings(right, left, path) + return mergeMappings(right, left, path, nil) } func mergeExtraHosts(c any, o any, _ tree.Path) (any, error) { @@ -227,7 +302,7 @@ func convertIntoSequence(value any) []any { func mergeUlimit(_ any, o any, p tree.Path) (any, error) { over, ismapping := o.(map[string]any) if base, ok := o.(map[string]any); ok && ismapping { - return mergeMappings(base, over, p) + return mergeMappings(base, over, p, nil) } return o, nil } @@ -255,7 +330,7 @@ func mergeIPAMConfig(c any, o any, path tree.Path) (any, error) { continue } } - merged, err := mergeMappings(right, left, path) + merged, err := mergeMappings(right, left, path, nil) if err != nil { return nil, err } diff --git a/override/merge_positional_test.go b/override/merge_positional_test.go new file mode 100644 index 000000000..4d6343870 --- /dev/null +++ b/override/merge_positional_test.go @@ -0,0 +1,140 @@ +/* + Copyright 2020 The Compose Specification Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package override + +import ( + "testing" + + "github.com/compose-spec/compose-go/v2/tree" + "gotest.tools/v3/assert" +) + +// mergePositional performs an element-by-element (positional) merge for the +// sequences at the given paths instead of the default append. In a compose file +// this is requested with the `!merge` tag on the override sequence; the tests +// below drive the merge directly with explicit paths to illustrate the +// semantics. +func assertPositionalMerge(t *testing.T, right, left, want string, paths ...string) { + t.Helper() + pp := make([]tree.Path, len(paths)) + for i, p := range paths { + pp[i] = tree.NewPath(p) + } + got, err := MergeWithPositionalPaths(unmarshal(t, right), unmarshal(t, left), pp) + assert.NilError(t, err) + assert.DeepEqual(t, got, unmarshal(t, want)) +} + +// A no-op element (`- {}`) keeps the base value at that index; a non-null element +// replaces it. Here only the third argument (the port value) is changed. +func TestMergePositional_TargetsSingleScalar(t *testing.T) { + base := ` +command: + - server + - --port + - "8080" + - --verbose +` + override := ` +command: + - {} + - {} + - "9090" +` + want := ` +command: + - server + - --port + - "9090" + - --verbose +` + assertPositionalMerge(t, base, override, want, "command") +} + +// A non-null map element is deep-merged with the base element at the same index, +// so a single field can be changed while the rest of that element is preserved. +func TestMergePositional_DeepMergesMapElement(t *testing.T) { + base := ` +ports: + - target: 80 + published: "8080" + protocol: tcp + - target: 443 + published: "8443" +` + override := ` +ports: + - published: "9090" + - {} +` + want := ` +ports: + - target: 80 + published: "9090" + protocol: tcp + - target: 443 + published: "8443" +` + assertPositionalMerge(t, base, override, want, "ports") +} + +// An explicit null element is a no-op too. +func TestMergePositional_NullElementIsNoOp(t *testing.T) { + base := ` +dns: + - 1.1.1.1 + - 8.8.8.8 +` + override := ` +dns: + - 9.9.9.9 + - ~ +` + want := ` +dns: + - 9.9.9.9 + - 8.8.8.8 +` + assertPositionalMerge(t, base, override, want, "dns") +} + +// When the override is longer than the base the sequence is extended; when it is +// shorter the base tail is preserved. +func TestMergePositional_LengthMismatch(t *testing.T) { + t.Run("override extends base", func(t *testing.T) { + base := "seq: [a, b]" + override := "seq: [{}, B, c]" + want := "seq: [a, B, c]" + assertPositionalMerge(t, base, override, want, "seq") + }) + t.Run("base longer than override", func(t *testing.T) { + base := "seq: [a, b, c]" + override := "seq: [A]" + want := "seq: [A, b, c]" + assertPositionalMerge(t, base, override, want, "seq") + }) +} + +// Without a positional path the default append behavior is unchanged — this is +// the contrast that makes `!merge` opt-in and non-breaking. +func TestMergePositional_NotEnabledStillAppends(t *testing.T) { + base := "seq: [a, b]" + override := "seq: [c, d]" + got, err := MergeWithPositionalPaths(unmarshal(t, base), unmarshal(t, override), nil) + assert.NilError(t, err) + assert.DeepEqual(t, got, unmarshal(t, "seq: [a, b, c, d]")) +}