fix: boolean false overrides dropped in multi-document helmfiles (#2527) by yxxhero · Pull Request #2532 · helmfile/helmfile

yxxhero · 2026-04-10T00:08:22Z

Summary

Fix boolean false (and other) overrides being silently dropped in multi-document helmfiles when environment values are re-assigned mid-helmfile, a regression introduced in fix: array merge regression - layer arrays now replace defaults #2367 (v1.3.0+).
The root cause was NewEnvironmentTemplateData setting .Environment.Values to the full merged values (Defaults + Values + CLIOverrides), which caused Defaults to pollute the Values layer when users re-assigned environment values via {{ toYaml .Environment.Values }}.
One-line fix: set .Environment.Values to Values + CLIOverrides only, excluding Defaults.

Changes

pkg/state/types.go: Changed NewEnvironmentTemplateData to set envCopy.Values to MergeMaps(env.Values, env.CLIOverrides) instead of the full GetMergedValues() result, preventing Defaults from leaking into .Environment.Values.
pkg/environment/environment_test.go: Added regression test for Defaults/Values merge ordering.
pkg/state/types_test.go: Added regression test verifying .Environment.Values excludes Defaults while still including CLI overrides.

Test plan

All existing tests pass (go test -race -p=1 ./pkg/...)
New regression tests pass
make check passes
Manual reproduction from Boolean false overrides silently dropped in multi-document helmfiles since v1.3.0 #2527 (requires multi-part helmfile with defaults + env overrides)

…be dropped (#2527) PR #2367 introduced envCopy.Values = values in NewEnvironmentTemplateData, where values = GetMergedValues() = Defaults + Values + CLIOverrides. This caused .Environment.Values to include Defaults, so when multi-part helmfiles re-assigned environment values via {{ toYaml .Environment.Values }}, Defaults values (e.g. helmDefaults.atomic: true) were written into the environment Values field. Later, GetMergedValues() applied Values over Defaults, causing the stale atomic: true to win over the correct atomic: false override. Fix: set .Environment.Values to Values + CLIOverrides only (excluding Defaults), so re-assignment patterns don't pollute the Values layer with Defaults. Signed-off-by: yxx <[email protected]> Signed-off-by: yxxhero <[email protected]>

Copilot

Pull request overview

Fixes a regression where boolean false (and other) overrides could be silently dropped in multi-document helmfiles when users re-assign environment values mid-file, by preventing Defaults from leaking into .Environment.Values.

Changes:

Adjusted NewEnvironmentTemplateData so .Environment.Values is derived from Values + CLIOverrides only (excluding Defaults).
Added a regression test ensuring .Environment.Values excludes Defaults while still including CLI overrides.
Added a regression test verifying GetMergedValues() layering keeps Values overriding Defaults.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
pkg/state/types.go	Stops Defaults from polluting `.Environment.Values` by merging only `env.Values` and `env.CLIOverrides`.
pkg/state/types_test.go	Regression test ensuring `.Environment.Values` excludes Defaults but includes CLI overrides.
pkg/environment/environment_test.go	Regression test covering Defaults/Values precedence in `GetMergedValues()`.

Copilot · 2026-04-10T00:22:40Z

 	assert.Equal(t, "cmdline", elem1["anotherThing"])
 }
+
+func TestEnvironment_GetMergedValues_Issue2527_DefaultsOverrideValues(t *testing.T) {


The test name reads as if Defaults should override Values, but the assertions verify the opposite (Values overriding Defaults). Renaming the test to reflect the actual expected precedence would avoid confusion when reading failures or grepping for related regressions.

Suggested change

func TestEnvironment_GetMergedValues_Issue2527_DefaultsOverrideValues(t *testing.T) {

func TestEnvironment_GetMergedValues_Issue2527_ValuesOverrideDefaults(t *testing.T) {

yxxhero · 2026-04-10T00:23:42Z

@copilot fix comments.

…ence Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1b251877-7050-404b-8cc7-abd6aa3ec36b Co-authored-by: yxxhero <[email protected]>

Copilot · 2026-04-10T00:51:03Z

@copilot fix comments.

Renamed the test from TestEnvironment_GetMergedValues_Issue2527_DefaultsOverrideValues to TestEnvironment_GetMergedValues_Issue2527_ValuesOverrideDefaults to correctly reflect that Values override Defaults (commit 709f7e1).

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Copilot · 2026-04-10T03:47:38Z

+func TestEnvironment_GetMergedValues_Issue2527_ValuesOverrideDefaults(t *testing.T) {
+	env := &Environment{
+		Name: "test",
+		Defaults: map[string]any{
+			"helmDefaults": map[string]any{
+				"atomic":  false,
+				"wait":    true,
+				"timeout": 300,
+			},
+		},
+		Values: map[string]any{
+			"appName": "my-app",
+			"helmDefaults": map[string]any{
+				"atomic":  true,
+				"wait":    true,
+				"timeout": 300,
+			},
+		},
+		CLIOverrides: map[string]any{},
+	}
+
+	mergedValues, err := env.GetMergedValues()
+	require.NoError(t, err)
+
+	hd := mergedValues["helmDefaults"].(map[string]any)
+	assert.Equal(t, true, hd["atomic"], "Values should override Defaults atomic value")
+	assert.Equal(t, true, hd["wait"])
+	assert.Equal(t, 300, hd["timeout"])


The new regression test is labeled for #2527 (false overrides being dropped), but it currently asserts that a true value in Values overrides a false value in Defaults. That doesn’t exercise the reported failure mode (where a false override gets lost). Consider flipping the fixture to Defaults.atomic: true and Values.atomic: false (and asserting false), or otherwise adding an assertion that specifically covers the true → false override case so this test actually guards against the regression described in the PR/issue.

yxxhero · 2026-04-10T03:55:21Z

@copilot please fix comments.

…2527) Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c428fd46-b698-4e88-bff2-4c9ac72d2deb Co-authored-by: yxxhero <[email protected]>

Copilot · 2026-04-10T03:56:44Z

@copilot please fix comments.

Fixed in 644f8c3. Flipped the fixture to Defaults.atomic: true / Values.atomic: false and assert the result is false, directly exercising the regression where a false boolean override was silently lost.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

yxxhero requested review from aditmeno, itscaro, mumoshu, xiaomudk and zhaque44 as code owners April 10, 2026 00:08

yxxhero requested a review from Copilot April 10, 2026 00:08

Copilot started reviewing on behalf of yxxhero April 10, 2026 00:09 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Copilot started work on behalf of yxxhero April 10, 2026 00:23 View session

Copilot finished work on behalf of yxxhero April 10, 2026 00:51

yxxhero requested a review from Copilot April 10, 2026 03:43

Copilot started reviewing on behalf of yxxhero April 10, 2026 03:44 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

Copilot started work on behalf of yxxhero April 10, 2026 03:55 View session

Copilot finished work on behalf of yxxhero April 10, 2026 03:57

yxxhero requested a review from Copilot April 12, 2026 01:19

Copilot started reviewing on behalf of yxxhero April 12, 2026 01:19 View session

Copilot AI reviewed Apr 12, 2026

View reviewed changes

xiaomudk approved these changes Apr 12, 2026

View reviewed changes

yxxhero merged commit b9378b1 into main Apr 12, 2026
20 checks passed

yxxhero deleted the fix/issue-2527-environment-values-pollution branch April 12, 2026 01:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: boolean false overrides dropped in multi-document helmfiles (#2527)#2532

fix: boolean false overrides dropped in multi-document helmfiles (#2527)#2532
yxxhero merged 3 commits into
mainfrom
fix/issue-2527-environment-values-pollution

yxxhero commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

yxxhero commented Apr 10, 2026

Uh oh!

Copilot AI commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

yxxhero commented Apr 10, 2026

Uh oh!

Copilot AI commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	func TestEnvironment_GetMergedValues_Issue2527_DefaultsOverrideValues(t *testing.T) {
	func TestEnvironment_GetMergedValues_Issue2527_ValuesOverrideDefaults(t *testing.T) {

Uh oh!

Uh oh!

Conversation

yxxhero commented Apr 10, 2026

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

yxxhero commented Apr 10, 2026

Uh oh!

Copilot AI commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

yxxhero commented Apr 10, 2026

Uh oh!

Copilot AI commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants