Merge dca45ba565 into 87268ee077

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

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,56 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) {
 		}
 		is.Equal(expectedValues, updatedRes.Config)
 	})
+
+	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)
+
+		imageTemplate := &chartcommon.File{
+			Name:    "templates/image",
+			ModTime: time.Now(),
+			Data: []byte(`image: {{ .Values.services.myservice.image | default "chart-default" }}
+tag: {{ .Values.services.myservice.tag }}`),
+		}
+		ch := buildChartWithTemplates([]*chartcommon.File{imageTemplate})
+
+		rel := releaseStub()
+		rel.Name = "nuketown"
+		rel.Info.Status = common.StatusDeployed
+		rel.Chart = ch
+		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{
+			"services": map[string]any{
+				"myservice": map[string]any{
+					"image": nil,
+					"tag":   "1.0.0",
+				},
+			},
+		})
+		req.NoError(err)
+		res, err := releaserToV1Release(resi)
+		req.NoError(err)
+
+		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")
+	})
 }
 
 func TestUpgradeRelease_ResetThenReuseValues(t *testing.T) {