From c375b48550dfa8214615bf091463b9767af57794 Mon Sep 17 00:00:00 2001 From: Aditya Menon Date: Mon, 9 Mar 2026 16:01:21 +0530 Subject: [PATCH] fix: nested helmfile values should replace arrays, not merge element-by-element (#2458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/app/app.go | 1 + pkg/app/desired_state_file_loader.go | 13 ++++++++++--- pkg/app/load_opts.go | 2 ++ pkg/app/load_opts_test.go | 14 ++++++++++++++ pkg/state/state.go | 3 ++- test/integration/run.sh | 1 + .../issue-2451-nested-helmfile-array-replace.sh | 13 +++++++++++++ .../input/child.yaml.gotmpl | 17 +++++++++++++++++ .../input/defaults.yaml | 3 +++ .../input/helmfile.yaml | 5 +++++ .../output/output.yaml | 7 +++++++ 11 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh create mode 100644 test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/child.yaml.gotmpl create mode 100644 test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/defaults.yaml create mode 100644 test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/helmfile.yaml create mode 100644 test/integration/test-cases/issue-2451-nested-helmfile-array-replace/output/output.yaml diff --git a/pkg/app/app.go b/pkg/app/app.go index 021eab95..cdc31745 100644 --- a/pkg/app/app.go +++ b/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) diff --git a/pkg/app/desired_state_file_loader.go b/pkg/app/desired_state_file_loader.go index deb833f0..03064c47 100644 --- a/pkg/app/desired_state_file_loader.go +++ b/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, + } } } diff --git a/pkg/app/load_opts.go b/pkg/app/load_opts.go index f321237b..fc7d8bfb 100644 --- a/pkg/app/load_opts.go +++ b/pkg/app/load_opts.go @@ -30,5 +30,7 @@ func (o LoadOpts) DeepCopy() LoadOpts { panic(err) } + new.Environment.OverrideValuesAreCLI = o.Environment.OverrideValuesAreCLI + return new } diff --git a/pkg/app/load_opts_test.go b/pkg/app/load_opts_test.go index 1d1f996a..7db6f1fb 100644 --- a/pkg/app/load_opts_test.go +++ b/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") +} diff --git a/pkg/state/state.go b/pkg/state/state.go index e8d16a32..046031e0 100644 --- a/pkg/state/state.go +++ b/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 diff --git a/test/integration/run.sh b/test/integration/run.sh index 8b4767e9..80099707 100755 --- a/test/integration/run.sh +++ b/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 diff --git a/test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh b/test/integration/test-cases/issue-2451-nested-helmfile-array-replace.sh new file mode 100644 index 00000000..78d3c09f --- /dev/null +++ b/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)" diff --git a/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/child.yaml.gotmpl b/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/child.yaml.gotmpl new file mode 100644 index 00000000..04c3636e --- /dev/null +++ b/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 }} diff --git a/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/defaults.yaml b/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/defaults.yaml new file mode 100644 index 00000000..77387d3c --- /dev/null +++ b/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/defaults.yaml @@ -0,0 +1,3 @@ +list: + - name: default1 + - name: default2 diff --git a/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/helmfile.yaml b/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/input/helmfile.yaml new file mode 100644 index 00000000..6d6983ea --- /dev/null +++ b/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 diff --git a/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/output/output.yaml b/test/integration/test-cases/issue-2451-nested-helmfile-array-replace/output/output.yaml new file mode 100644 index 00000000..a23ff2d3 --- /dev/null +++ b/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