From 39995d388e5c0ffdaa7d8196b6901c0d67f0854e Mon Sep 17 00:00:00 2001 From: BeganovR <446110@niuitmo.ru> Date: Tue, 25 Aug 2026 14:17:45 +0300 Subject: [PATCH 1/2] fix(action): stop resurrected nulled values on the first reuse-values upgrade When --reuse-values is set together with --set key=null, the null correctly drops the key from the persisted config, but chart.Values is set to the fully resolved old release values, so the final render-time coalesce falls back to that stale override and silently brings the key back for that one upgrade. A second, identical upgrade then works, because by then the release's stored config has already lost the key. Prune any key that newVals explicitly nulls from the old values before they're assigned to chart.Values, so an explicit null takes effect on the first upgrade. Signed-off-by: BeganovR <446110@niuitmo.ru> --- pkg/action/upgrade.go | 25 +++++++++++++++++++++++++ pkg/action/upgrade_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7f66ceefb..331b93aaa 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -621,6 +621,14 @@ func (u *Upgrade) reuseValues(chart *chartv2.Chart, current *release.Release, ne return nil, fmt.Errorf("failed to rebuild old values: %w", err) } + // A `--set key=null` cancels key's value from current.Config below, + // but that alone only stops CoalesceTables from copying it over. + // oldVals becomes chart.Values, which the final render-time coalesce + // falls back to for any key newVals doesn't have, so without pruning + // the same key here it would resurface from the release being + // reused instead of the chart's own default. + pruneNilOverrides(oldVals, newVals) + newVals = util.CoalesceTables(newVals, current.Config) chart.Values = oldVals @@ -644,6 +652,23 @@ func (u *Upgrade) reuseValues(chart *chartv2.Chart, current *release.Release, ne return newVals, nil } +// pruneNilOverrides deletes from dst every key that is explicitly nil in +// newVals, recursing into nested tables so an explicit `--set key=null` +// removes the key from dst at whatever depth it appears. +func pruneNilOverrides(dst, newVals map[string]any) { + for key, val := range newVals { + if val == nil { + delete(dst, key) + continue + } + if sub, ok := val.(map[string]any); ok { + if dsub, ok := dst[key].(map[string]any); ok { + pruneNilOverrides(dsub, sub) + } + } + } +} + func validateManifest(c kube.Interface, manifest []byte, openAPIValidation bool) error { _, err := c.Build(bytes.NewReader(manifest), openAPIValidation) return err diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 53419b6a8..cca59cab3 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/resource" + chartcommon "helm.sh/helm/v4/pkg/chart/common" chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" @@ -332,6 +333,35 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { } is.Equal(expectedValues, updatedRes.Config) }) + + t.Run("nulling a previously set value with reuse-values takes effect on the first upgrade", func(t *testing.T) { + is := assert.New(t) + req := require.New(t) + upAction := upgradeAction(t) + + imageTemplate := &chartcommon.File{ + Name: "templates/image", + ModTime: time.Now(), + Data: []byte("image: {{ .Values.image | default \"chart-default\" }}"), + } + ch := buildChartWithTemplates([]*chartcommon.File{imageTemplate}) + + rel := releaseStub() + rel.Name = "nuketown" + rel.Info.Status = common.StatusDeployed + rel.Chart = ch + rel.Config = map[string]any{"image": "old-image"} + req.NoError(upAction.cfg.Releases.Create(rel)) + + upAction.ReuseValues = true + resi, err := upAction.Run(rel.Name, ch, map[string]any{"image": nil}) + req.NoError(err) + res, err := releaserToV1Release(resi) + req.NoError(err) + + is.NotContains(res.Config, "image", "explicit null should remove the key from the persisted config") + is.Contains(res.Manifest, "image: chart-default", "nulling a reused value should fall back to the chart default on the first upgrade, not the second") + }) } func TestUpgradeRelease_ResetThenReuseValues(t *testing.T) { From dca45ba565beb56cce0d797f2052d3c6d898aa4e Mon Sep 17 00:00:00 2001 From: BeganovR <446110@niuitmo.ru> Date: Tue, 25 Aug 2026 14:47:58 +0300 Subject: [PATCH 2/2] test(action): null a nested reuse-values key to match the reported bug The regression test only nulled a top-level key, but the bug (and pruneNilOverrides) is about pruning explicit nil overrides at arbitrary depth. Switch it to null services.myservice.image, matching the nesting in #30765, and also assert the sibling key set alongside it survives. Signed-off-by: BeganovR <446110@niuitmo.ru> --- pkg/action/upgrade_test.go | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index cca59cab3..4d5773afd 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -334,7 +334,7 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { is.Equal(expectedValues, updatedRes.Config) }) - t.Run("nulling a previously set value with reuse-values takes effect on the first upgrade", func(t *testing.T) { + t.Run("nulling a previously set nested value with reuse-values takes effect on the first upgrade", func(t *testing.T) { is := assert.New(t) req := require.New(t) upAction := upgradeAction(t) @@ -342,7 +342,8 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { imageTemplate := &chartcommon.File{ Name: "templates/image", ModTime: time.Now(), - Data: []byte("image: {{ .Values.image | default \"chart-default\" }}"), + Data: []byte(`image: {{ .Values.services.myservice.image | default "chart-default" }} +tag: {{ .Values.services.myservice.tag }}`), } ch := buildChartWithTemplates([]*chartcommon.File{imageTemplate}) @@ -350,17 +351,37 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { rel.Name = "nuketown" rel.Info.Status = common.StatusDeployed rel.Chart = ch - rel.Config = map[string]any{"image": "old-image"} + rel.Config = map[string]any{ + "services": map[string]any{ + "myservice": map[string]any{ + "image": "old-image", + }, + }, + } req.NoError(upAction.cfg.Releases.Create(rel)) upAction.ReuseValues = true - resi, err := upAction.Run(rel.Name, ch, map[string]any{"image": nil}) + resi, err := upAction.Run(rel.Name, ch, map[string]any{ + "services": map[string]any{ + "myservice": map[string]any{ + "image": nil, + "tag": "1.0.0", + }, + }, + }) req.NoError(err) res, err := releaserToV1Release(resi) req.NoError(err) - is.NotContains(res.Config, "image", "explicit null should remove the key from the persisted config") - is.Contains(res.Manifest, "image: chart-default", "nulling a reused value should fall back to the chart default on the first upgrade, not the second") + services, ok := res.Config["services"].(map[string]any) + req.True(ok, "expected services key in persisted config") + myservice, ok := services["myservice"].(map[string]any) + req.True(ok, "expected services.myservice key in persisted config") + is.NotContains(myservice, "image", "explicit null should remove the nested key from the persisted config, at whatever depth it appears") + is.Equal("1.0.0", myservice["tag"]) + + is.Contains(res.Manifest, "image: chart-default", "nulling a reused nested value should fall back to the chart default on the first upgrade, not the second") + is.Contains(res.Manifest, "tag: 1.0.0") }) }