From 2911c68474ce342510cc10b68298571e73f1d7dc Mon Sep 17 00:00:00 2001 From: Johannes Lohmer Date: Thu, 30 Apr 2026 14:44:09 +0200 Subject: [PATCH] test(values): Values merge nil handling tests when the subchart has unrelated user-defined values Signed-off-by: Johannes Lohmer --- pkg/chart/v2/util/dependencies_test.go | 110 +++++++++++++++++++++++++ pkg/engine/engine_test.go | 22 +++-- 2 files changed, 126 insertions(+), 6 deletions(-) diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index 0e4df8528..8ed80e98e 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -21,7 +21,11 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/pkg/chart/common" + commonutil "helm.sh/helm/v4/pkg/chart/common/util" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/loader" ) @@ -568,3 +572,109 @@ func TestChartWithDependencyAliasedTwiceAndDoublyReferencedSubDependency(t *test } validateDependencyTree(t, c) } + +// Metadata.Dependencies must be set so processImportValues doesn't early-exit +// before contaminating parent.Values with subchart nil defaults. +func newParentWithNilSubchart() *chart.Chart { + subchart := &chart.Chart{ + Metadata: &chart.Metadata{Name: "child"}, + Values: map[string]any{ + "keyMapping": map[string]any{ + "password": nil, + }, + "ingress": map[string]any{ + "configureCertmanager": nil, + }, + }, + } + parent := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{{Name: "child"}}, + }, + Values: map[string]any{}, + } + parent.AddDependency(subchart) + return parent +} + +func processAndCoalesce(t *testing.T, parent *chart.Chart, vals common.Values) common.Values { + t.Helper() + require.NoError(t, ProcessDependencies(parent, vals)) + coalesced, err := commonutil.CoalesceValues(parent, vals) + require.NoError(t, err) + return coalesced +} + +// Baseline: with no user subchart values, nil cleanup goes through the +// !merge else-branch in coalesceValues and works. +func TestProcessDependenciesNilCleanedNoUserSubchartVals(t *testing.T) { + is := assert.New(t) + coalesced := processAndCoalesce(t, newParentWithNilSubchart(), common.Values{}) + + _, err := common.Values(coalesced).PathValue("child.keyMapping.password") + is.ErrorAs(err, &common.ErrNoValue{}, "child.keyMapping.password should be absent") + + _, err = common.Values(coalesced).PathValue("child.ingress.configureCertmanager") + is.ErrorAs(err, &common.ErrNoValue{}, "child.ingress.configureCertmanager should be absent") +} + +// Any user value under "child" routes coalescing through merge=true and +// bypasses cleanNilValues. Covers #31919 and #31971. +func TestProcessDependenciesNilCleanedWithUnrelatedSubchartVals(t *testing.T) { + is := assert.New(t) + coalesced := processAndCoalesce(t, newParentWithNilSubchart(), common.Values{ + "child": map[string]any{ + "someOtherKey": "someValue", + }, + "global": map[string]any{ + "ingress": map[string]any{ + "configureCertmanager": true, + }, + }, + }) + + _, err := common.Values(coalesced).PathValue("child.keyMapping.password") + is.ErrorAs(err, &common.ErrNoValue{}, "child.keyMapping.password should be absent") + + _, err = common.Values(coalesced).PathValue("child.ingress.configureCertmanager") + is.ErrorAs(err, &common.ErrNoValue{}, "child.ingress.configureCertmanager should be absent so pluck falls through to global") +} + +// Stricter case: user overrides a sibling key in the same map containing the nil. +func TestProcessDependenciesNilCleanedWithPartialSameMapVals(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 := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{{Name: "child"}}, + }, + Values: map[string]any{}, + } + parent.AddDependency(subchart) + + coalesced := processAndCoalesce(t, parent, common.Values{ + "child": map[string]any{ + "keyMapping": map[string]any{ + "format": "sha256", + }, + }, + }) + + v, err := common.Values(coalesced).PathValue("child.keyMapping.format") + is.NoError(err) + is.Equal("sha256", v) + + _, err = common.Values(coalesced).PathValue("child.keyMapping.password") + is.ErrorAs(err, &common.ErrNoValue{}, "child.keyMapping.password should be absent") +} diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 869b5d202..857a9e558 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -37,6 +37,7 @@ import ( "helm.sh/helm/v4/pkg/chart/common" "helm.sh/helm/v4/pkg/chart/common/util" chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" ) func TestSortTemplates(t *testing.T) { @@ -1512,7 +1513,6 @@ func TestTraceableError_NoTemplateForm(t *testing.T) { 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{ @@ -1524,19 +1524,29 @@ func TestRenderSubchartDefaultNilNoStringify(t *testing.T) { }, Values: map[string]any{ "keyMapping": map[string]any{ - "password": nil, // nil in chart defaults + "password": nil, }, }, } parent := &chart.Chart{ - Metadata: &chart.Metadata{Name: "parent"}, - Values: map[string]any{}, + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{{Name: "child"}}, + }, + Values: map[string]any{}, } parent.AddDependency(subchart) - // Parent user values don't set keyMapping - injValues := map[string]any{} + injValues := map[string]any{ + "child": map[string]any{ + "someOtherKey": "someValue", + }, + } + + if err := chartutil.ProcessDependencies(parent, injValues); err != nil { + t.Fatalf("Failed to process dependencies: %s", err) + } tmp, err := util.CoalesceValues(parent, injValues) if err != nil {