diff --git a/pkg/action/action.go b/pkg/action/action.go index 6e5d8c15a..72eff674e 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -730,6 +730,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 diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index 6fc449c30..f76d901e4 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -164,7 +164,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 } diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index b14305112..c79fe01d4 100644 --- a/pkg/action/uninstall.go +++ b/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", } } @@ -200,7 +206,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 } @@ -227,8 +236,9 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } if !u.DisableHooks { - serverSideApply := true - if err := u.cfg.execHook(rel, release.HookPostDelete, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil { + if serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, rel.ApplyMethod); 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) } } diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index 90f69d639..26a8db508 100644 --- a/pkg/action/uninstall_test.go +++ b/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 { @@ -352,3 +353,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") +} diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 81d51164f..9c9e228b3 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -309,7 +309,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 } @@ -665,15 +665,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) - } -} diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index bd020cbdb..1b5ddb50b 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -637,7 +637,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 @@ -714,7 +714,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) assert.NoError(t, err) assert.Equal(t, tt.expectedServerSideApply, serverSideApply) }) @@ -730,13 +730,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) }) } diff --git a/pkg/cmd/uninstall.go b/pkg/cmd/uninstall.go index bfecd4741..a1ff75375 100644 --- a/pkg/cmd/uninstall.go +++ b/pkg/cmd/uninstall.go @@ -81,6 +81,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