Jlohmer/coalesce nil by Y0-L0 · Pull Request #31979 · helm/helm

Y0-L0 · 2026-03-28T19:21:00Z

Fix nil value preservation regression introduced by #31644, which was too aggressive — it preserved nil values from subchart defaults, not just user overrides. This caused three downstream problems:

%!s(<nil>) in rendered output — subchart default null values survive coalescing and produce %!s(<nil>) when stringified (breaks Bitnami common.secrets.key / subPath mounts)
User null can't erase subchart defaults — --values with null no longer deletes child values, breaking documented behavior
Subchart default nil shadows global values — pluck-based fallback patterns return nil instead of falling through to globals (Subchart default nil values shadow global values via pluck (regression from #31644) #31971)

Changes

coalesce_test.go — 3 unit tests covering each regression scenario (Regression in null merging between parent.values and --values #31919, Subchart default nil values shadow global values via pluck (regression from #31644) #31971)
engine_test.go — 1 integration test for the full render pipeline producing %!s(<nil>)

closes #31919
closes #31971

Special notes for your reviewer:

If applicable:

this PR contains unit tests
this PR has been tested for backwards compatibility.

Copilot

Pull request overview

Adds regression tests around Helm values coalescing when subchart defaults contain nil, aiming to prevent nils from being preserved in final merged values (which can break pluck-style fallbacks and lead to nil stringification artifacts during rendering).

Changes:

Add 3 unit tests covering subchart-default-nil cleanup, user null erasure behavior, and global fallback behavior during coalescing.
Add 1 engine-level render test intended to cover the “%!s(<nil>) in rendered output” regression.
Minor doc comment tweaks, .gitignore update, and Go module dependency bumps.

Reviewed changes

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

Show a summary per file

File	Description
pkg/registry/client.go	Comment/docstring refinements for registry client options and login.
pkg/engine/engine_test.go	Adds an integration-style render test for subchart default nil behavior.
pkg/chart/common/util/coalesce_test.go	Adds regression-focused unit tests for coalescing behavior around nils.
go.mod	Bumps `github.com/ProtonMail/go-crypto` and `github.com/lib/pq` versions.
go.sum	Updates checksums corresponding to the module version bumps.
.gitignore	Adds `.claude` to ignored files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/engine/engine_test.go

pkg/chart/common/util/coalesce_test.go

Copilot

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/chart/common/util/coalesce.go

go.mod

pkg/engine/engine_test.go

pkg/chart/common/util/coalesce.go

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/chart/common/util/coalesce.go

pkg/engine/engine_test.go

pkg/chart/common/util/coalesce.go

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

banjoh

Thank you for making this contribution to fix the regression.

Note to self: The fix ensures that no null values are left behind after merging. These null values cause conditionals such as subPath: {{ include "common.secrets.key" (dict "existingSecret" .Values.existingSecret "key" "password") | quote }} template to evaluate as subPath: null since existingSecret would be null instead of being removed

As a side comment, this code has gotten really complex overtime (not related to you PR, just an observation)

pkg/chart/common/util/coalesce.go

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Y0-L0 · 2026-04-02T07:02:55Z

As a side comment, this code has gotten really complex overtime (not related to you PR, just an observation)

100% agree. While implementing the fix, I had to stop myself multiple times from refactoring things. 😉
I'm quite tempted to refactor it. And with some more property-based regression tests around it, I believe I could do it quite safely.

On the other hand with the values/v1 plugin, I suppose bigger changes are coming to this part of the code. Maybe we wait until that's fleshed out further to avoid a double refactoring. 🤔
Also a refactoring in v4 would make future backports harder...

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>

Regression test for the Bitnami common.secrets.key issue. Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>

Only user-supplied nils should survive coalescing. Chart-default nils defaults, not just user overrides. This caused: - %!s(<nil>) in templates using Bitnami common.secrets.key (helm#31919) - pluck fallbacks returning nil instead of falling through to globals (helm#31971) Fixes helm#31919 Fixes helm#31971 Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>

…rt maps Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>

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

Copilot AI review requested due to automatic review settings March 28, 2026 19:21

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 28, 2026

Copilot started reviewing on behalf of Y0-L0 March 28, 2026 19:21 View session

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from bfbdf7f to 06ed8a9 Compare March 28, 2026 19:23

Copilot AI reviewed Mar 28, 2026

View reviewed changes

pkg/engine/engine_test.go Outdated Show resolved Hide resolved

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

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from 06ed8a9 to cfbde43 Compare March 28, 2026 19:33

Copilot AI review requested due to automatic review settings March 28, 2026 20:00

Copilot started reviewing on behalf of Y0-L0 March 28, 2026 20:01 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

pkg/chart/common/util/coalesce.go Show resolved Hide resolved

go.mod Show resolved Hide resolved

pkg/engine/engine_test.go Show resolved Hide resolved

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

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from 723037d to 06d510b Compare March 28, 2026 20:18

Copilot AI review requested due to automatic review settings March 29, 2026 18:11

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from 06d510b to a307ce0 Compare March 29, 2026 18:11

Copilot started reviewing on behalf of Y0-L0 March 29, 2026 18:11 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

pkg/chart/common/util/coalesce.go Show resolved Hide resolved

pkg/engine/engine_test.go Show resolved Hide resolved

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

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from a307ce0 to ef4aec0 Compare March 30, 2026 08:17

Y0-L0 mentioned this pull request Mar 30, 2026

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

Open

Copilot AI review requested due to automatic review settings March 31, 2026 09:19

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from ef4aec0 to a9c1b0c Compare March 31, 2026 09:19

Copilot started reviewing on behalf of Y0-L0 March 31, 2026 09:20 View session

Copilot AI reviewed Mar 31, 2026

View reviewed changes

banjoh previously approved these changes Apr 1, 2026

View reviewed changes

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

banjoh dismissed their stale review via 00d224f April 1, 2026 18:25

Copilot AI review requested due to automatic review settings April 1, 2026 18:25

Copilot started reviewing on behalf of banjoh April 1, 2026 18:26 View session

banjoh approved these changes Apr 1, 2026

View reviewed changes

Copilot AI reviewed Apr 1, 2026

View reviewed changes

Y0-L0 and others added 3 commits April 2, 2026 15:48

test(values): add test for subchart nil producing %!s(<nil>)

8629e59

Regression test for the Bitnami common.secrets.key issue. Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>

Y0-L0 and others added 2 commits April 2, 2026 15:48

test(values): Add test for nil cleanup in partially overridden subcha…

09df93f

…rt maps Signed-off-by: Johannes Lohmer <jojo.dev@lohmer.com>

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

cd697b9

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

Y0-L0 force-pushed the jlohmer/coalesce-nil branch from 00d224f to cd697b9 Compare April 2, 2026 13:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jlohmer/coalesce nil#31979

Jlohmer/coalesce nil#31979
Y0-L0 wants to merge 5 commits intohelm:mainfrom
Y0-L0:jlohmer/coalesce-nil

Y0-L0 commented Mar 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

banjoh left a comment •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Y0-L0 commented Apr 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Y0-L0 commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

banjoh left a comment • 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!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Y0-L0 commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Y0-L0 commented Mar 28, 2026 •

edited

Loading

banjoh left a comment •

edited

Loading

Y0-L0 commented Apr 2, 2026 •

edited

Loading