Merge 757a9f79c9 into d2de64e64b

pkg/action/action.go

@@ -729,6 +729,22 @@ func determineReleaseSSApplyMethod(serverSideApply bool) release.ApplyMethod {
 	return release.ApplyMethodClientSideApply
 }
 
+// getServerSideApplyValue resolves the server-side apply option string ("auto", "true", "false")
+// against the release's stored apply method. When "auto", it respects the release's previous
+// apply method; otherwise the explicit "true"/"false" value is used.
+func getServerSideApplyValue(serverSideOption string, releaseApplyMethod string) (bool, error) {
+	switch serverSideOption {
+	case "auto":
+		return releaseApplyMethod == string(release.ApplyMethodServerSideApply), nil
+	case "false":
+		return false, nil
+	case "true":
+		return true, nil
+	default:
+		return false, fmt.Errorf("invalid server-side apply option %q (must be one of: auto, true, false)", serverSideOption)
+	}
+}
+
 // isDryRun returns true if the strategy is set to run as a DryRun
 func isDryRun(strategy DryRunStrategy) bool {
 	return strategy == DryRunClient || strategy == DryRunServer

pkg/action/rollback.go

@@ -175,7 +175,7 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele
 		return nil, nil, false, err
 	}
 
-	serverSideApply, err := getUpgradeServerSideValue(r.ServerSideApply, previousRelease.ApplyMethod)
+	serverSideApply, err := getServerSideApplyValue(r.ServerSideApply, previousRelease.ApplyMethod)
 	if err != nil {
 		return nil, nil, false, err
 	}

pkg/action/uninstall.go

@@ -49,12 +49,18 @@ type Uninstall struct {
 	DeletionPropagation string
 	Timeout             time.Duration
 	Description         string
+	// ServerSideApply controls whether pre-delete and post-delete hooks are applied
+	// via Kubernetes server-side apply. Can be the string: "true", "false" or "auto".
+	// When "auto", server-side usage will be based upon the release's previous usage.
+	// See: https://kubernetes.io/docs/reference/using-api/server-side-apply/
+	ServerSideApply string
 }
 
 // NewUninstall creates a new Uninstall object with the given configuration.
 func NewUninstall(cfg *Configuration) *Uninstall {
 	return &Uninstall{
-		cfg: cfg,
+		cfg:             cfg,
+		ServerSideApply: "auto",
 	}
 }
 
@@ -199,7 +205,10 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error)
 	res := &releasei.UninstallReleaseResponse{Release: rel}
 
 	if !u.DisableHooks {
-		serverSideApply := true
+		serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, rel.ApplyMethod)
+		if err != nil {
+			return res, err
+		}
 		if err := u.cfg.execHook(rel, release.HookPreDelete, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {
 			return res, err
 		}
@@ -226,8 +235,9 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error)
 	}
 
 	if !u.DisableHooks {
-		serverSideApply := true
+		if serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, rel.ApplyMethod); err != nil {
-		if err := u.cfg.execHook(rel, release.HookPostDelete, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {
+			errs = append(errs, err)
+		} else if err := u.cfg.execHook(rel, release.HookPostDelete, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil {
 			errs = append(errs, err)
 		}
 	}

pkg/action/uninstall_test.go

@@ -29,6 +29,7 @@ import (
 	"helm.sh/helm/v4/pkg/kube"
 	kubefake "helm.sh/helm/v4/pkg/kube/fake"
 	"helm.sh/helm/v4/pkg/release/common"
+	release "helm.sh/helm/v4/pkg/release/v1"
 )
 
 func uninstallAction(t *testing.T) *Uninstall {
@@ -358,3 +359,111 @@ data:
 	is.Contains(logOutput, "dryrun-unowned-deploy")
 	is.Contains(logOutput, "Deployment")
 }
+
+func TestUninstall_ServerSideApplyDefault(t *testing.T) {
+	unAction := uninstallAction(t)
+	is := assert.New(t)
+	is.Equal("auto", unAction.ServerSideApply, "default ServerSideApply should be 'auto'")
+}
+
+func TestGetServerSideApplyValue_WithUninstallScenarios(t *testing.T) {
+	tests := []struct {
+		name                   string
+		serverSideOption       string
+		releaseApplyMethod     string
+		expectedServerSideApply bool
+		expectError            bool
+		errorContains          string
+		description            string
+	}{
+		{
+			name:                    "auto with CSA release (Helm v3 migration)",
+			serverSideOption:        "auto",
+			releaseApplyMethod:      "csa",
+			expectedServerSideApply: false,
+			description:             "Helm v3 releases using CSA should get client-side apply for pre-delete hooks",
+		},
+		{
+			name:                    "auto with SSA release (Helm v4)",
+			serverSideOption:        "auto",
+			releaseApplyMethod:      "ssa",
+			expectedServerSideApply: true,
+			description:             "Helm v4 releases using SSA should keep server-side apply for pre-delete hooks",
+		},
+		{
+			name:                    "auto with empty apply method",
+			serverSideOption:        "auto",
+			releaseApplyMethod:      "",
+			expectedServerSideApply: false,
+			description:             "empty apply method should default to client-side apply",
+		},
+		{
+			name:                    "explicit true overrides CSA release",
+			serverSideOption:        "true",
+			releaseApplyMethod:      "csa",
+			expectedServerSideApply: true,
+			description:             "user can force SSA even for CSA releases",
+		},
+		{
+			name:                    "explicit false overrides SSA release",
+			serverSideOption:        "false",
+			releaseApplyMethod:      "ssa",
+			expectedServerSideApply: false,
+			description:             "user can force CSA even for SSA releases",
+		},
+		{
+			name:            "invalid option",
+			serverSideOption: "invalid",
+			expectError:     true,
+			errorContains:   "invalid server-side apply option",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result, err := getServerSideApplyValue(tt.serverSideOption, tt.releaseApplyMethod)
+			if tt.expectError {
+				assert.Error(t, err)
+				assert.Contains(t, err.Error(), tt.errorContains)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, tt.expectedServerSideApply, result, tt.description)
+			}
+		})
+	}
+}
+
+// TestUninstall_ServerSideApply_NoHooks verifies that the ServerSideApply option
+// is not consulted when hooks are disabled, avoiding unnecessary errors from
+// invalid ServerSideApply values when hooks won't run anyway.
+func TestUninstall_ServerSideApply_NoHooks_InvalidOption(t *testing.T) {
+	unAction := uninstallAction(t)
+	unAction.DisableHooks = true
+	unAction.ServerSideApply = "invalid-option-should-not-matter"
+
+	rel := releaseStub()
+	rel.Name = "no-hooks-test"
+	rel.ApplyMethod = string(release.ApplyMethodClientSideApply)
+	require.NoError(t, unAction.cfg.Releases.Create(rel))
+
+	// Should succeed because hooks are disabled and SSA option is never evaluated
+	_, err := unAction.Run(rel.Name)
+	assert.NoError(t, err)
+}
+
+// TestUninstall_ServerSideApply_InvalidOption_WithHooks verifies that an invalid
+// ServerSideApply value causes an error when hooks are enabled.
+func TestUninstall_ServerSideApply_InvalidOption_WithHooks(t *testing.T) {
+	unAction := uninstallAction(t)
+	unAction.DisableHooks = false
+	unAction.ServerSideApply = "invalid-option"
+
+	rel := releaseStub()
+	rel.Name = "invalid-ssa-option"
+	rel.ApplyMethod = string(release.ApplyMethodClientSideApply)
+	require.NoError(t, unAction.cfg.Releases.Create(rel))
+
+	_, err := unAction.Run(rel.Name)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "invalid server-side apply option")
+}

pkg/action/upgrade.go

@@ -308,7 +308,7 @@ func (u *Upgrade) prepareUpgrade(ctx context.Context, name string, chart *chartv
 		return nil, nil, false, fmt.Errorf("user supplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels())
 	}
 
-	serverSideApply, err := getUpgradeServerSideValue(u.ServerSideApply, lastRelease.ApplyMethod)
+	serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, lastRelease.ApplyMethod)
 	if err != nil {
 		return nil, nil, false, err
 	}
@@ -664,15 +664,3 @@ func mergeCustomLabels(current, desired map[string]string) map[string]string {
 	return labels
 }
 
-func getUpgradeServerSideValue(serverSideOption string, releaseApplyMethod string) (bool, error) {
-	switch serverSideOption {
-	case "auto":
-		return releaseApplyMethod == "ssa", nil
-	case "false":
-		return false, nil
-	case "true":
-		return true, nil
-	default:
-		return false, fmt.Errorf("invalid/unknown release server-side apply method: %s", serverSideOption)
-	}
-}

pkg/action/upgrade_test.go

@@ -619,7 +619,7 @@ func TestUpgradeRelease_DryRun(t *testing.T) {
 	req.Error(err)
 }
 
-func TestGetUpgradeServerSideValue(t *testing.T) {
+func TestGetServerSideApplyValue(t *testing.T) {
 	tests := []struct {
 		name                    string
 		actionServerSideOption  string
@@ -696,7 +696,7 @@ func TestGetUpgradeServerSideValue(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			serverSideApply, err := getUpgradeServerSideValue(tt.actionServerSideOption, tt.releaseApplyMethod)
+			serverSideApply, err := getServerSideApplyValue(tt.actionServerSideOption, tt.releaseApplyMethod)
 			require.NoError(t, err)
 			assert.Equal(t, tt.expectedServerSideApply, serverSideApply)
 		})
@@ -712,13 +712,13 @@ func TestGetUpgradeServerSideValue(t *testing.T) {
 			name:                   "action invalid option",
 			actionServerSideOption: "invalid",
 			releaseApplyMethod:     "ssa",
-			expectedErrorMsg:       "invalid/unknown release server-side apply method: invalid",
+			expectedErrorMsg:       "invalid server-side apply option \"invalid\" (must be one of: auto, true, false)",
 		},
 	}
 
 	for _, tt := range testsError {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := getUpgradeServerSideValue(tt.actionServerSideOption, tt.releaseApplyMethod)
+			_, err := getServerSideApplyValue(tt.actionServerSideOption, tt.releaseApplyMethod)
 			assert.ErrorContains(t, err, tt.expectedErrorMsg)
 		})
 	}

pkg/cmd/uninstall.go

@@ -82,6 +82,7 @@ func newUninstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command {
 	f.StringVar(&client.DeletionPropagation, "cascade", "background", "Must be \"background\", \"orphan\", or \"foreground\". Selects the deletion cascading strategy for the dependents. Defaults to background. Use \"foreground\" with --wait to ensure resources with finalizers are fully deleted before returning.")
 	f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)")
 	f.StringVar(&client.Description, "description", "", "add a custom description")
+	f.StringVar(&client.ServerSideApply, "server-side", "auto", "must be \"true\", \"false\" or \"auto\". If \"auto\", hooks will use the same server-side apply method as the release was installed with. Set to \"false\" to force client-side apply for pre-delete and post-delete hooks.")
 	AddWaitFlag(cmd, &client.WaitStrategy)
 
 	return cmd