diff --git a/pkg/chart/common/util/coalesce.go b/pkg/chart/common/util/coalesce.go index 5994febbc..45e3870e6 100644 --- a/pkg/chart/common/util/coalesce.go +++ b/pkg/chart/common/util/coalesce.go @@ -129,6 +129,19 @@ func coalesceDeps(printf printFn, chrt chart.Charter, dest map[string]any, prefi if err != nil { return dest, err } + // When coalescing, erase any remaining nil values in the subchart result + // that were user-supplied null overrides of parent-chart defaults. + // These are not erased at subchart level because the subchart has no + // default for the key; without this pass they would persist as nil. + if !merge { + if subMap, ok := dest[sub.Name()].(map[string]any); ok { + if parentVals, ok := ch.Values()[sub.Name()]; ok { + if parentValsMap, ok := parentVals.(map[string]any); ok { + eraseNullsWithParentDefaults(subMap, parentValsMap) + } + } + } + } } } return dest, nil @@ -189,6 +202,25 @@ func coalesceGlobals(printf printFn, dest, src map[string]any, prefix string, _ dest[common.GlobalKey] = dg } +// eraseNullsWithParentDefaults removes nil-valued keys from dst where the +// corresponding parent chart default was non-nil. This handles the case where +// a user sets a subchart key to null to erase a parent-defined default, but +// the subchart itself has no default for that key (so subchart-level null +// erasure never runs). +func eraseNullsWithParentDefaults(dst, parentDefs map[string]any) { + for key, dv := range dst { + pdv, inParent := parentDefs[key] + if !inParent { + continue + } + if dv == nil && pdv != nil { + delete(dst, key) + } else if istable(dv) && istable(pdv) { + eraseNullsWithParentDefaults(dv.(map[string]any), pdv.(map[string]any)) + } + } +} + func copyMap(src map[string]any) map[string]any { m := make(map[string]any, len(src)) maps.Copy(m, src) @@ -249,6 +281,8 @@ func coalesceValues(printf printFn, c chart.Charter, v map[string]any, prefix st } } else { // If the key is a child chart, coalesce tables with Merge set to true + // to preserve user-supplied null values so they can be erased at the + // subchart level (where subchart defaults are processed). merge := childChartMergeTrue(c, key, merge) // Because v has higher precedence than nv, dest values override src diff --git a/pkg/chart/common/util/coalesce_test.go b/pkg/chart/common/util/coalesce_test.go index 1d0baa84d..1a4acc72c 100644 --- a/pkg/chart/common/util/coalesce_test.go +++ b/pkg/chart/common/util/coalesce_test.go @@ -732,6 +732,85 @@ func TestConcatPrefix(t *testing.T) { assert.Equal(t, "a.b", concatPrefix("a", "b")) } +// TestCoalesceValuesNullErasureInSubchart tests that a user-supplied null value +// erases a key defined in the parent chart's values.yaml for a subchart, +// even when that key has no default in the subchart's own values.yaml. +// Regression test for https://github.com/helm/helm/issues/31919 +func TestCoalesceValuesNullErasureInSubchart(t *testing.T) { + is := assert.New(t) + + // Parent chart defines "key" for subchart in its own values.yaml. + // The subchart itself does NOT define "key" in its values.yaml. + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "mysubchart"}, + Values: map[string]any{ + "other": "subchart_default", + }, + } + parent := withDeps(&chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{ + "mysubchart": map[string]any{ + "key": "parent_defined_value", + }, + }, + }, subchart) + + // User passes null for mysubchart.key to erase the parent-defined default. + vals := map[string]any{ + "mysubchart": map[string]any{ + "key": nil, + }, + } + + v, err := CoalesceValues(parent, vals) + is.NoError(err) + + sub, ok := v["mysubchart"].(map[string]any) + is.True(ok, "mysubchart is not a map") + + // "other" should still be present (subchart default) + is.Equal("subchart_default", sub["other"]) + + // "key" should be removed — user explicitly set it to null to erase the parent default + _, present := sub["key"] + is.False(present, "Expected mysubchart.key to be erased by user null, but it is still present") +} + +// TestCoalesceValuesNullErasurePreservesUserValues ensures that when a parent chart +// defines a null for a subchart key, the user's non-nil value is still preserved. +func TestCoalesceValuesNullErasurePreservesUserValues(t *testing.T) { + is := assert.New(t) + + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "mysubchart"}, + Values: map[string]any{}, + } + parent := withDeps(&chart.Chart{ + Metadata: &chart.Metadata{Name: "parent"}, + Values: map[string]any{ + "mysubchart": map[string]any{ + "key": nil, // parent chart defaults this to null + }, + }, + }, subchart) + + // User provides a real value — should not be erased by the parent's null default + vals := map[string]any{ + "mysubchart": map[string]any{ + "key": "user_value", + }, + } + + v, err := CoalesceValues(parent, vals) + is.NoError(err) + + sub, ok := v["mysubchart"].(map[string]any) + is.True(ok, "mysubchart is not a map") + + is.Equal("user_value", sub["key"], "user value should not be overwritten by parent null default") +} + // TestCoalesceValuesEmptyMapWithNils tests the full CoalesceValues scenario // from issue #31643 where chart has data: {} and user provides data: {foo: bar, baz: ~} func TestCoalesceValuesEmptyMapWithNils(t *testing.T) {