From 98df1b3621e71b7c0fddcc45a95fe18b6e19baec Mon Sep 17 00:00:00 2001 From: Johannes Lohmer Date: Sat, 28 Mar 2026 10:23:17 +0000 Subject: [PATCH 1/5] test(values): add tests for subchart nil value regressions Three test cases that cover the regression scenarios introduced by the #31644 nil preservation fix: - subchart default nils should be cleaned up when parent doesn't set those keys (#31919) - user-supplied null should erase subchart defaults (#31919) - subchart default nil should not shadow global values via pluck (#31971) Tests are expected to fail until the regression is fixed. Signed-off-by: Johannes Lohmer --- .gitignore | 1 + pkg/chart/common/util/coalesce_test.go | 118 +++++++++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/.gitignore b/.gitignore index 0fd2c6bda..2209e9809 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ .DS_Store .coverage/ .idea +.claude .vimrc .vscode/ .devcontainer/ diff --git a/pkg/chart/common/util/coalesce_test.go b/pkg/chart/common/util/coalesce_test.go index 1d0baa84d..f7bf541a2 100644 --- a/pkg/chart/common/util/coalesce_test.go +++ b/pkg/chart/common/util/coalesce_test.go @@ -765,3 +765,121 @@ func TestCoalesceValuesEmptyMapWithNils(t *testing.T) { is.True(ok, "Expected data.baz key to be present but it was removed") is.Nil(data["baz"], "Expected data.baz key to be nil but it is not") } + +// TestCoalesceValuesSubchartDefaultNilsCleaned tests that nil values in subchart defaults +// are cleaned up during coalescing when the parent doesn't set those keys. +// Regression test for issue #31919. +func TestCoalesceValuesSubchartDefaultNilsCleaned(t *testing.T) { + is := assert.New(t) + + // Subchart has a default with nil values (e.g. keyMapping: {password: null}) + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "child"}, + Values: map[string]any{ + "keyMapping": map[string]any{ + "password": nil, + }, + }, + } + + parent := withDeps(&chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{}, + }, subchart) + + // Parent user values don't mention keyMapping at all + vals := map[string]any{} + + v, err := CoalesceValues(parent, vals) + is.NoError(err) + + childVals, ok := v["child"].(map[string]any) + is.True(ok, "child values should be a map") + + keyMapping, ok := childVals["keyMapping"].(map[string]any) + is.True(ok, "keyMapping should be a map") + + // The nil "password" key from chart defaults should be cleaned up + _, ok = keyMapping["password"] + is.False(ok, "Expected keyMapping.password (nil from chart defaults) to be removed, but it is still present") +} + +// TestCoalesceValuesUserNullErasesSubchartDefault tests that a user-supplied null +// value erases a subchart's default value during coalescing. +// Regression test for issue #31919. +func TestCoalesceValuesUserNullErasesSubchartDefault(t *testing.T) { + is := assert.New(t) + + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "child"}, + Values: map[string]any{ + "someKey": "default", + }, + } + + parent := withDeps(&chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{}, + }, subchart) + + // User explicitly nullifies the subchart key via parent values + vals := map[string]any{ + "child": map[string]any{ + "someKey": nil, + }, + } + + v, err := CoalesceValues(parent, vals) + is.NoError(err) + + childVals, ok := v["child"].(map[string]any) + is.True(ok, "child values should be a map") + + // someKey should be erased — user null overrides subchart default + _, ok = childVals["someKey"] + is.False(ok, "Expected someKey to be removed by user null override, but it is still present") +} + +// TestCoalesceValuesSubchartNilDoesNotShadowGlobal tests that a nil value in +// subchart defaults doesn't shadow a global value accessible via pluck-like access. +// Regression test for issue #31971. +func TestCoalesceValuesSubchartNilDoesNotShadowGlobal(t *testing.T) { + is := assert.New(t) + + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "child"}, + Values: map[string]any{ + "ingress": map[string]any{ + "feature": nil, // nil in subchart defaults + }, + }, + } + + parent := withDeps(&chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{}, + }, subchart) + + // Parent sets the global value + vals := map[string]any{ + "global": map[string]any{ + "ingress": map[string]any{ + "feature": true, + }, + }, + } + + v, err := CoalesceValues(parent, vals) + is.NoError(err) + + childVals, ok := v["child"].(map[string]any) + is.True(ok, "child values should be a map") + + ingress, ok := childVals["ingress"].(map[string]any) + is.True(ok, "ingress should be a map") + + // The nil "feature" from subchart defaults should be cleaned up, + // so that pluck can fall through to the global value + _, ok = ingress["feature"] + is.False(ok, "Expected ingress.feature (nil from chart defaults) to be removed so global can be used via pluck, but it is still present") +} From 2ee439ac68c3bd336eb5057ae4285d77fc34bae5 Mon Sep 17 00:00:00 2001 From: Johannes Lohmer Date: Sat, 28 Mar 2026 10:23:21 +0000 Subject: [PATCH 2/5] test(values): add test for subchart nil producing %!s() Regression test for the Bitnami common.secrets.key issue. Signed-off-by: Johannes Lohmer --- pkg/engine/engine_test.go | 62 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index c674a11ec..869b5d202 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -1505,3 +1505,65 @@ func TestTraceableError_NoTemplateForm(t *testing.T) { } } } + +// TestRenderSubchartDefaultNilNoStringify tests the full pipeline: subchart default +// nil values should not produce "%!s()" in rendered template output. +// Regression test for the Bitnami common.secrets.key issue. +func TestRenderSubchartDefaultNilNoStringify(t *testing.T) { + modTime := time.Now() + + // Subchart has a default with nil values + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "child"}, + Templates: []*common.File{ + { + Name: "templates/test.yaml", + ModTime: modTime, + Data: []byte(`{{- if hasKey .Values.keyMapping "password" -}}{{- printf "subPath: %s" (index .Values.keyMapping "password") -}}{{- else -}}subPath: fallback{{- end -}}`), + }, + }, + Values: map[string]any{ + "keyMapping": map[string]any{ + "password": nil, // nil in chart defaults + }, + }, + } + + parent := &chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{}, + } + parent.AddDependency(subchart) + + // Parent user values don't set keyMapping + injValues := map[string]any{} + + tmp, err := util.CoalesceValues(parent, injValues) + if err != nil { + t.Fatalf("Failed to coalesce values: %s", err) + } + + inject := common.Values{ + "Values": tmp, + "Chart": parent.Metadata, + "Release": common.Values{ + "Name": "test-release", + }, + } + + out, err := Render(parent, inject) + if err != nil { + t.Fatalf("Failed to render templates: %s", err) + } + + rendered := out["parent/charts/child/templates/test.yaml"] + + if strings.Contains(rendered, "%!s()") { + t.Errorf("Rendered output contains %%!s(), got: %q", rendered) + } + + expected := "subPath: fallback" + if rendered != expected { + t.Errorf("Expected %q, got %q", expected, rendered) + } +} From 04c6d5cd75646ba5d36ed70e36994efa2405c4ec Mon Sep 17 00:00:00 2001 From: Johannes Lohmer Date: Sat, 28 Mar 2026 20:59:31 +0100 Subject: [PATCH 3/5] fix(values): do not copy chart-default nils into coalesced values Only user-supplied nils should survive coalescing. Chart-default nils defaults, not just user overrides. This caused: - %!s() in templates using Bitnami common.secrets.key (#31919) - pluck fallbacks returning nil instead of falling through to globals (#31971) Fixes #31919 Fixes #31971 Signed-off-by: Johannes Lohmer --- pkg/chart/common/util/coalesce.go | 29 +++++++++++++++++++++++++- pkg/cmd/testdata/output/issue-9027.txt | 10 +++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/pkg/chart/common/util/coalesce.go b/pkg/chart/common/util/coalesce.go index 5994febbc..90e6640e9 100644 --- a/pkg/chart/common/util/coalesce.go +++ b/pkg/chart/common/util/coalesce.go @@ -251,6 +251,12 @@ func coalesceValues(printf printFn, c chart.Charter, v map[string]any, prefix st // If the key is a child chart, coalesce tables with Merge set to true merge := childChartMergeTrue(c, key, merge) + // When coalescing, clean nils from chart defaults before merging + // so they don't leak into the result. + if !merge { + cleanNilValues(src) + } + // Because v has higher precedence than nv, dest values override src // values. coalesceTablesFullKey(printf, dest, src, concatPrefix(subPrefix, key), merge) @@ -258,6 +264,16 @@ func coalesceValues(printf printFn, c chart.Charter, v map[string]any, prefix st } } else { // If the key is not in v, copy it from nv. + // When coalescing, skip chart default nils and clean nils from + // nested maps so they don't shadow globals or produce %!s(). + if !merge { + if val == nil { + continue + } + if sub, ok := val.(map[string]any); ok { + cleanNilValues(sub) + } + } v[key] = val } } @@ -326,7 +342,6 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]any, prefix strin // But if src also has nil (or key not in src), preserve the nil delete(dst, key) } else if !ok { - // key not in user values, preserve src value (including nil) dst[key] = val } else if istable(val) { if istable(dv) { @@ -341,6 +356,18 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]any, prefix strin return dst } +// cleanNilValues recursively removes nil entries from a map so that chart +// default nils don't leak into the coalesced result. +func cleanNilValues(m map[string]any) { + for key, val := range m { + if val == nil { + delete(m, key) + } else if sub, ok := val.(map[string]any); ok { + cleanNilValues(sub) + } + } +} + // istable is a special-purpose function to see if the present thing matches the definition of a YAML table. func istable(v any) bool { _, ok := v.(map[string]any) diff --git a/pkg/cmd/testdata/output/issue-9027.txt b/pkg/cmd/testdata/output/issue-9027.txt index eb19fc383..1227336e4 100644 --- a/pkg/cmd/testdata/output/issue-9027.txt +++ b/pkg/cmd/testdata/output/issue-9027.txt @@ -2,11 +2,15 @@ # Source: issue-9027/charts/subchart/templates/values.yaml global: hash: + key1: 1 + key2: 2 key3: 13 key4: 4 key5: 5 key6: 6 hash: + key1: 1 + key2: 2 key3: 13 key4: 4 key5: 5 @@ -15,17 +19,19 @@ hash: # Source: issue-9027/templates/values.yaml global: hash: - key1: null - key2: null key3: 13 subchart: global: hash: + key1: 1 + key2: 2 key3: 13 key4: 4 key5: 5 key6: 6 hash: + key1: 1 + key2: 2 key3: 13 key4: 4 key5: 5 From 8fbc1965435c2cdba8f0bbca28d5e54368ea8565 Mon Sep 17 00:00:00 2001 From: Johannes Lohmer Date: Mon, 30 Mar 2026 10:15:12 +0200 Subject: [PATCH 4/5] test(values): Add test for nil cleanup in partially overridden subchart maps Signed-off-by: Johannes Lohmer --- pkg/chart/common/util/coalesce_test.go | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/pkg/chart/common/util/coalesce_test.go b/pkg/chart/common/util/coalesce_test.go index f7bf541a2..252ef11ec 100644 --- a/pkg/chart/common/util/coalesce_test.go +++ b/pkg/chart/common/util/coalesce_test.go @@ -883,3 +883,48 @@ func TestCoalesceValuesSubchartNilDoesNotShadowGlobal(t *testing.T) { _, ok = ingress["feature"] is.False(ok, "Expected ingress.feature (nil from chart defaults) to be removed so global can be used via pluck, but it is still present") } + +// TestCoalesceValuesSubchartNilCleanedWhenUserPartiallyOverrides tests that nil +// values in subchart defaults are cleaned even when the user partially overrides +// the same map. Regression test for the coalesceTablesFullKey merge path. +func TestCoalesceValuesSubchartNilCleanedWhenUserPartiallyOverrides(t *testing.T) { + is := assert.New(t) + + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "child"}, + Values: map[string]any{ + "keyMapping": map[string]any{ + "password": nil, + "format": "bcrypt", + }, + }, + } + + parent := withDeps(&chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{}, + }, subchart) + + // User overrides format but doesn't mention password + vals := map[string]any{ + "child": map[string]any{ + "keyMapping": map[string]any{ + "format": "sha256", + }, + }, + } + + v, err := CoalesceValues(parent, vals) + is.NoError(err) + + childVals, ok := v["child"].(map[string]any) + is.True(ok, "child values should be a map") + + keyMapping, ok := childVals["keyMapping"].(map[string]any) + is.True(ok, "keyMapping should be a map") + + is.Equal("sha256", keyMapping["format"], "User override should be preserved") + + _, ok = keyMapping["password"] + is.False(ok, "Expected keyMapping.password (nil from chart defaults) to be removed even when user partially overrides the map") +} From 31c2088b8600677dd969bccbf7dd8265494caad8 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Wed, 1 Apr 2026 19:25:09 +0100 Subject: [PATCH 5/5] Update pkg/chart/common/util/coalesce.go Signed-off-by: Evans Mungai --- pkg/chart/common/util/coalesce.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/chart/common/util/coalesce.go b/pkg/chart/common/util/coalesce.go index 90e6640e9..999eeb208 100644 --- a/pkg/chart/common/util/coalesce.go +++ b/pkg/chart/common/util/coalesce.go @@ -356,7 +356,7 @@ func coalesceTablesFullKey(printf printFn, dst, src map[string]any, prefix strin return dst } -// cleanNilValues recursively removes nil entries from a map so that chart +// cleanNilValues recursively removes nil entries in-place from a map so that chart // default nils don't leak into the coalesced result. func cleanNilValues(m map[string]any) { for key, val := range m {