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>
This commit is contained in:
copilot-swe-agent[bot] 2026-01-07 01:46:14 +00:00
parent 1802fd4888
commit e400a0b936
3 changed files with 63 additions and 0 deletions

View File

@ -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
}

View File

@ -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)")
})
}

View File

@ -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
}