This commit is contained in:
Abhay Chaurasiya 2026-04-03 14:14:28 -07:00 committed by GitHub
commit 0fe430db30
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 110 additions and 0 deletions

View file

@ -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

View file

@ -25,6 +25,7 @@ import (
"text/template"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"helm.sh/helm/v4/pkg/chart/common"
chart "helm.sh/helm/v4/pkg/chart/v2"
@ -732,6 +733,81 @@ 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) {
// 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)
require.NoError(t, err)
sub, ok := v["mysubchart"].(map[string]any)
require.True(t, ok, "mysubchart is not a map")
// "other" should still be present (subchart default)
assert.Equal(t, "subchart_default", sub["other"])
// "key" should be removed — user explicitly set it to null to erase the parent default
_, present := sub["key"]
assert.False(t, 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) {
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)
require.NoError(t, err)
sub, ok := v["mysubchart"].(map[string]any)
require.True(t, ok, "mysubchart is not a map")
assert.Equal(t, "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) {