fix(values): preserve nil values when chart default is empty map by banjoh · Pull Request #31644 · helm/helm

banjoh · 2025-12-12T16:33:49Z

What this PR does / why we need it:

Only delete nil user values when overriding a non-nil chart default. When chart has empty map or no default for a key, preserve user's nil.

Scenario	Result
User sets `baz: ~`, chart has `baz: "value"`	Key deleted
User sets `baz: ~`, chart has empty map `{}`	Nil preserved
User sets `baz: ~`, chart has `baz: ~`	Nil preserved

Fixes #31643

Special notes for your reviewer:

If applicable:

this PR contains user facing changes (the docs needed label should be applied if so)
this PR contains unit tests
this PR has been tested for backwards compatibility

Only delete nil user values when overriding a non-nil chart default. When chart has empty map or no default for a key, preserve user's nil. | Scenario | Result | |----------|--------| | User sets `baz: ~`, chart has `baz: "value"` | Key deleted | | User sets `baz: ~`, chart has empty map `{}` | Nil preserved | | User sets `baz: ~`, chart has `baz: ~` | Nil preserved | Fixes helm#31643 Signed-off-by: Evans Mungai <mbuevans@gmail.com>

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

robertsirc

LGTM

pkg/chart/common/util/coalesce_test.go

gjenkins8 · 2025-12-22T18:09:56Z

pkg/chart/common/util/coalesce_test.go

+		// otherwise the values are coalesced.
+		CoalesceTables(dst, src)
+
+		is.Equal("Ishmael", dst["name"], "Unexpected name: %s", dst["name"])


Would it be okay to split this refactoring of conditional logic to assert into a new PR. Why assert (IMHO) is much better for test readability, it is difficult here to review any logic changes (due to any changes in the code logic)

gjenkins8 · 2025-12-22T18:12:02Z

pkg/chart/common/util/coalesce_test.go

-	if !ok {
-		t.Fatal("Address went away.")
-	}
+	t.Run("case 1", func(t *testing.T) {


Perhaps this and the below t.Run(... separate test cases?

(but see my comment below about creating a new PR for these refactoring changes)

Co-authored-by: George Jenkins <gvjenkins@gmail.com> Signed-off-by: Evans Mungai <mbuevans@gmail.com>

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

banjoh · 2026-01-07T13:19:01Z

@gjenkins8 I removed all the test refactoring changes and left the unit test validating the reported bug

KrzysztofDziankowski · 2026-01-30T15:37:05Z

I have verified the fix for my use case and it appears to work.
Could this pull request be included in the next Helm 3.20.x release?

TerryHowe

/lgtm

chojnack · 2026-02-05T08:52:18Z

Could you please merge this into 3.20 branch too? We could build that ourselves then for our testing.

Three test cases that cover the regression scenarios introduced by the helm#31644 nil preservation fix: - subchart default nils should be cleaned up when parent doesn't set those keys (helm#31919) - user-supplied null should erase subchart defaults (helm#31919) - subchart default nil should not shadow global values via pluck (helm#31971) Tests are expected to fail until the regression is fixed.

Three test cases that cover the regression scenarios introduced by the helm#31644 nil preservation fix: - subchart default nils should be cleaned up when parent doesn't set those keys (helm#31919) - user-supplied null should erase subchart defaults (helm#31919) - subchart default nil should not shadow global values via pluck (helm#31971) Tests are expected to fail until the regression is fixed. Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 12, 2025

banjoh added the bug Categorizes issue or PR as related to a bug. label Dec 12, 2025

banjoh marked this pull request as draft December 12, 2025 16:34

Preserve nil values in chart already

679f051

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

banjoh marked this pull request as ready for review December 12, 2025 17:01

banjoh mentioned this pull request Dec 12, 2025

Regression in map values merging in helm v4 #31643

Closed

Fix lint warning

3416dd5

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

robertsirc previously approved these changes Dec 17, 2025

View reviewed changes

robertsirc added the Has One Approval This PR has one approval. It still needs a second approval to be merged. label Dec 17, 2025

gjenkins8 reviewed Dec 22, 2025

View reviewed changes

pkg/chart/common/util/coalesce_test.go Outdated Show resolved Hide resolved

gjenkins8 reviewed Dec 22, 2025

View reviewed changes

Update pkg/chart/common/util/coalesce_test.go

a333bba

Co-authored-by: George Jenkins <gvjenkins@gmail.com> Signed-off-by: Evans Mungai <mbuevans@gmail.com>

banjoh dismissed robertsirc’s stale review via a333bba January 7, 2026 12:51

banjoh added 2 commits January 7, 2026 12:58

Fix import

b8937ad

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

Remove refactorring changes from coalesce_test.go

0298b2f

Signed-off-by: Evans Mungai <mbuevans@gmail.com>

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 7, 2026

TerryHowe approved these changes Jan 31, 2026

View reviewed changes

banjoh merged commit 5b78ee8 into helm:main Jan 31, 2026
5 checks passed

banjoh deleted the em/fix-nil-values branch January 31, 2026 13:03

joejulian added this to the 4.1.2 milestone Feb 11, 2026

joejulian added needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. and removed Has One Approval This PR has one approval. It still needs a second approval to be merged. labels Feb 11, 2026

KrzysztofDziankowski mentioned this pull request Feb 11, 2026

[v3 backport] fix(values): preserve nil values when chart default is empty map #31829

Merged

3 tasks

scottrigby removed this from the 4.1.2 milestone Mar 11, 2026

scottrigby added this to the 4.1.3 milestone Mar 11, 2026

scottrigby added v3 port complete For completed v2->v3 ports picked Indicates that a PR has been cherry-picked into the next release candidate. and removed needs-pick Indicates that a PR needs to be cherry-picked into the next release candidate. labels Mar 11, 2026

scottrigby mentioned this pull request Mar 12, 2026

Regression in null merging between parent.values and --values #31919

Open

cmarker-gl mentioned this pull request Mar 26, 2026

Subchart default nil values shadow global values via pluck (regression from #31644) #31971

Open

Y0-L0 mentioned this pull request Mar 28, 2026

Jlohmer/coalesce nil #31979

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(values): preserve nil values when chart default is empty map#31644

fix(values): preserve nil values when chart default is empty map#31644
banjoh merged 6 commits intohelm:mainfrom
banjoh:em/fix-nil-values

banjoh commented Dec 12, 2025 •

edited

Loading

Uh oh!

robertsirc left a comment

Uh oh!

Uh oh!

gjenkins8 Dec 22, 2025 •

edited

Loading

Uh oh!

gjenkins8 Dec 22, 2025

Uh oh!

banjoh commented Jan 7, 2026

Uh oh!

KrzysztofDziankowski commented Jan 30, 2026

Uh oh!

TerryHowe left a comment

Uh oh!

Uh oh!

chojnack commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

banjoh commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertsirc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gjenkins8 Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gjenkins8 Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

banjoh commented Jan 7, 2026

Uh oh!

KrzysztofDziankowski commented Jan 30, 2026

Uh oh!

TerryHowe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chojnack commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

banjoh commented Dec 12, 2025 •

edited

Loading

gjenkins8 Dec 22, 2025 •

edited

Loading