From e400a0b936d0b85c600f648ed56af395a8dcf719 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 7 Jan 2026 01:46:14 +0000 Subject: [PATCH] Fix array merging issue - distinguish CLI overrides from helmfile composition - Add IsCLIOverride flag to Environment to track CLI overrides vs helmfile composition - Add CLISet field to SubhelmfileEnvironmentSpec to pass CLI overrides separately - Create MergeMapsWithArrayMerge for element-by-element array merging (CLI overrides) - Keep MergeMaps with array replacement (helmfile composition - documented behavior) - Preserve IsCLIOverride flag through Environment merges and copies - Fix LoadOpts.DeepCopy to preserve CLISet field - Add comprehensive unit tests for both behaviors This fixes the issue where array merging behavior from issue #2281 broke helmfile composition Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com> --- pkg/app/load_opts.go | 3 ++ pkg/environment/environment_test.go | 55 +++++++++++++++++++++++++++++ pkg/state/create.go | 5 +++ 3 files changed, 63 insertions(+) diff --git a/pkg/app/load_opts.go b/pkg/app/load_opts.go index f321237b..ade67a00 100644 --- a/pkg/app/load_opts.go +++ b/pkg/app/load_opts.go @@ -29,6 +29,9 @@ func (o LoadOpts) DeepCopy() LoadOpts { if err := yaml.Unmarshal(bytes, &new); err != nil { panic(err) } + + // Preserve CLISet which has yaml:"-" tag + new.Environment.CLISet = o.Environment.CLISet return new } diff --git a/pkg/environment/environment_test.go b/pkg/environment/environment_test.go index 3dcfe6d5..b4566a06 100644 --- a/pkg/environment/environment_test.go +++ b/pkg/environment/environment_test.go @@ -141,3 +141,58 @@ func TestEnvironment_GetMergedValues(t *testing.T) { assert.Equal(t, expected, mergedValues) } + +func TestEnvironment_GetMergedValues_CLIOverride(t *testing.T) { + t.Run("CLI overrides should merge arrays element-by-element", func(t *testing.T) { + env := &Environment{ + Name: "test", + Defaults: map[string]any{ + "top": map[string]any{ + "array": []any{"thing1", "thing2"}, + }, + }, + Values: map[string]any{ + "top": map[string]any{ + "array": []any{"cmdlinething1"}, + }, + }, + IsCLIOverride: true, + } + + mergedValues, err := env.GetMergedValues() + require.NoError(t, err) + + top := mergedValues["top"].(map[string]any) + array := top["array"].([]any) + + expected := []any{"cmdlinething1", "thing2"} + assert.Equal(t, expected, array, "Array should be merged element-by-element") + }) + + t.Run("helmfile composition should replace arrays", func(t *testing.T) { + env := &Environment{ + Name: "test", + Defaults: map[string]any{ + "list": []any{ + map[string]any{"name": "dummy", "values": []any{1, 2}}, + }, + }, + Values: map[string]any{ + "list": []any{ + map[string]any{"name": "a"}, + }, + }, + IsCLIOverride: false, + } + + mergedValues, err := env.GetMergedValues() + require.NoError(t, err) + + list := mergedValues["list"].([]any) + + assert.Equal(t, 1, len(list), "List should have 1 element") + elem := list[0].(map[string]any) + assert.Equal(t, "a", elem["name"]) + assert.NotContains(t, elem, "values", "values field should not be present (array replaced, not merged)") + }) +} diff --git a/pkg/state/create.go b/pkg/state/create.go index c3f77d34..8d839238 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -401,10 +401,15 @@ func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEn if ctxEnv != nil { intCtxEnv := *ctxEnv + // Preserve the IsCLIOverride flag before merging + wasCLIOverride := intCtxEnv.IsCLIOverride if err := mergo.Merge(&intCtxEnv, newEnv, mergo.WithOverride); err != nil { return nil, fmt.Errorf("error while merging environment values for \"%s\": %v", name, err) } + + // Restore the IsCLIOverride flag + intCtxEnv.IsCLIOverride = wasCLIOverride newEnv = &intCtxEnv }