fix: nested helmfile values should replace arrays, not merge element-by-element (#2458)

PR #2367 introduced CLIOverrides to give --state-values-set element-by-element
array merge semantics. However, nested helmfile values (helmfiles[].values:)
were also routed into CLIOverrides, causing their arrays to merge instead of
replace. This broke the pre-v1.3.0 behavior where passing an array via
helmfiles[].values: would fully replace the child's default array.

Add OverrideValuesAreCLI flag to SubhelmfileEnvironmentSpec so the loader can
distinguish CLI flags from nested helmfile values. CLI values continue using
CLIOverrides (element-by-element merge); nested helmfile values now use Values
(Sparse merge strategy → full array replacement).

Fixes #2451

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

pkg/app/app.go

@@ -1349,6 +1349,7 @@ func (a *App) visitStatesWithSelectorsAndRemoteSupportWithContext(fileOrDir stri
 
 	if len(envvals) > 0 {
 		opts.Environment.OverrideValues = envvals
+		opts.Environment.OverrideValuesAreCLI = true
 	}
 
 	a.remote = remote.NewRemote(a.Logger, "", a.fs)

pkg/app/desired_state_file_loader.go

@@ -64,9 +64,16 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e
 			return nil, err
 		}
 
-		overrodeEnv = &environment.Environment{
-			Name:         ld.env,
-			CLIOverrides: vals,
+		if opts.Environment.OverrideValuesAreCLI {
+			overrodeEnv = &environment.Environment{
+				Name:         ld.env,
+				CLIOverrides: vals,
+			}
+		} else {
+			overrodeEnv = &environment.Environment{
+				Name:   ld.env,
+				Values: vals,
+			}
 		}
 	}
 

pkg/app/load_opts.go

@@ -30,5 +30,7 @@ func (o LoadOpts) DeepCopy() LoadOpts {
 		panic(err)
 	}
 
+	new.Environment.OverrideValuesAreCLI = o.Environment.OverrideValuesAreCLI
+
 	return new
 }

pkg/app/load_opts_test.go

@@ -20,3 +20,17 @@ func TestLoadOptsDeepCopy(t *testing.T) {
 	// Check that the new struct is not the same as the old one.
 	require.Equal(t, lOld, lNew, "DeepCopy should return a copy of the LoadOpts struct")
 }
+
+// TestLoadOptsDeepCopyPreservesOverrideValuesAreCLI verifies that DeepCopy
+// preserves the OverrideValuesAreCLI flag which is tagged yaml:"-".
+func TestLoadOptsDeepCopyPreservesOverrideValuesAreCLI(t *testing.T) {
+	lOld := LoadOpts{
+		Selectors:  []string{"test"},
+		CalleePath: "test",
+	}
+	lOld.Environment.OverrideValuesAreCLI = true
+
+	lNew := lOld.DeepCopy()
+
+	require.True(t, lNew.Environment.OverrideValuesAreCLI, "DeepCopy should preserve OverrideValuesAreCLI flag")
+}

pkg/state/state.go

@@ -155,7 +155,8 @@ type SubHelmfileSpec struct {
 
 // SubhelmfileEnvironmentSpec is the environment spec for a subhelmfile
 type SubhelmfileEnvironmentSpec struct {
-	OverrideValues []any `yaml:"values,omitempty"`
+	OverrideValues       []any `yaml:"values,omitempty"`
+	OverrideValuesAreCLI bool  `yaml:"-"`
 }
 
 // HelmSpec to defines helmDefault values

test/integration/run.sh

@@ -121,6 +121,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
 . ${dir}/test-cases/state-values-set-cli-args-in-environments.sh
 . ${dir}/test-cases/issue-2281-array-merge.sh
 . ${dir}/test-cases/issue-2353-layer-array-replace.sh
+. ${dir}/test-cases/issue-2451-nested-helmfile-array-replace.sh
 . ${dir}/test-cases/issue-2247.sh
 . ${dir}/test-cases/issue-2097.sh
 . ${dir}/test-cases/issue-2291.sh

test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh

@@ -0,0 +1,13 @@
+issue_2451_nested_helmfile_array_replace_input_dir="${cases_dir}/issue-2451-nested-helmfile-array-replace/input"
+issue_2451_nested_helmfile_array_replace_output_dir="${cases_dir}/issue-2451-nested-helmfile-array-replace/output"
+
+issue_2451_nested_helmfile_array_replace_tmp=$(mktemp -d)
+issue_2451_nested_helmfile_array_replace_result="${issue_2451_nested_helmfile_array_replace_tmp}/issue.2451.nested.helmfile.array.replace.yaml"
+
+test_start "issue 2451 - nested helmfile array replace (not merge)"
+info "Comparing issue 2451 nested helmfile array replace output ${issue_2451_nested_helmfile_array_replace_result} with ${issue_2451_nested_helmfile_array_replace_output_dir}/output.yaml"
+
+${helmfile} -f "${issue_2451_nested_helmfile_array_replace_input_dir}/helmfile.yaml" template --skip-deps > "${issue_2451_nested_helmfile_array_replace_result}" || fail "\"helmfile template\" shouldn't fail"
+./dyff between -bs "${issue_2451_nested_helmfile_array_replace_output_dir}/output.yaml" "${issue_2451_nested_helmfile_array_replace_result}" || fail "nested helmfile array values should replace default arrays entirely, not merge element-by-element"
+
+test_pass "issue 2451 - nested helmfile array replace (not merge)"

test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/child.yaml.gotmpl

@@ -0,0 +1,17 @@
+values:
+  - defaults.yaml
+
+---
+releases:
+  - name: test
+    chart: ../../../charts/raw
+    values:
+      - templates:
+          - |
+            {{- range $item := .Values.list }}
+            ---
+            apiVersion: v1
+            kind: ConfigMap
+            metadata:
+              name: {{ $item.name }}
+            {{- end }}

test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/defaults.yaml

@@ -0,0 +1,3 @@
+list:
+  - name: default1
+  - name: default2

test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/helmfile.yaml

@@ -0,0 +1,5 @@
+helmfiles:
+  - path: child.yaml.gotmpl
+    values:
+      - list:
+          - name: override1

test/integration/test-cases/issue-2451-nested-helmfile-array-replace/output/output.yaml

@@ -0,0 +1,7 @@
+---
+# Source: raw/templates/resources.yaml
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: override1