From 83a159a61274990f593c419766a4cd810c10b2c7 Mon Sep 17 00:00:00 2001 From: ayushi-work Date: Thu, 18 Jun 2026 00:22:00 +0530 Subject: [PATCH 1/3] feat(uninstall): add --server-side flag to control hook apply method Defaults to "auto" which respects the release's previous apply method (CSA or SSA) instead of always forcing server-side apply. Signed-off-by: ayushi-work --- pkg/action/action.go | 16 +++++ pkg/action/rollback.go | 2 +- pkg/action/uninstall.go | 18 +++++- pkg/action/uninstall_test.go | 109 +++++++++++++++++++++++++++++++++++ pkg/action/upgrade.go | 14 +---- pkg/action/upgrade_test.go | 6 +- pkg/cmd/uninstall.go | 1 + 7 files changed, 146 insertions(+), 20 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 4ed718371..bf036dc8f 100644 --- a/pkg/action/action.go +++ b/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 == "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) + } +} + // 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 459569781..855574530 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..47b5cbd04 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 enables changes to be 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,7 +236,10 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } if !u.DisableHooks { - serverSideApply := true + serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, rel.ApplyMethod) + if err != nil { + errs = append(errs, err) + } 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..deb166317 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/unknown release server-side apply method", + }, + } + + 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/unknown release server-side apply method") +} diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 0d7b1b148..0e10245c3 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -309,7 +309,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str 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 ab19d82b7..92f152c50 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) }) @@ -736,7 +736,7 @@ func TestGetUpgradeServerSideValue(t *testing.T) { 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 From 5b9cf6dedbe497b18c6bf670a53074bf6a2aeb10 Mon Sep 17 00:00:00 2001 From: ayushi-work Date: Thu, 18 Jun 2026 00:26:12 +0530 Subject: [PATCH 2/3] fix(uninstall): skip post-delete hook when SSA option is invalid - Guard execHook call to avoid running hooks with a bad server-side apply value - Clarify ServerSideApply field comment to note it affects hooks specifically Signed-off-by: ayushi-work --- pkg/action/uninstall.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 47b5cbd04..c79fe01d4 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -49,10 +49,10 @@ type Uninstall struct { DeletionPropagation string Timeout time.Duration Description string - // ServerSideApply enables changes to be 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 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 } @@ -236,11 +236,9 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } if !u.DisableHooks { - serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, rel.ApplyMethod) - if err != nil { + if serverSideApply, err := getServerSideApplyValue(u.ServerSideApply, rel.ApplyMethod); err != nil { errs = append(errs, err) - } - if err := u.cfg.execHook(rel, release.HookPostDelete, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil { + } else if err := u.cfg.execHook(rel, release.HookPostDelete, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil { errs = append(errs, err) } } From f979194ea7b6faa3b3ea38bdd00bd5f1dd8842a2 Mon Sep 17 00:00:00 2001 From: ayushi-work Date: Thu, 18 Jun 2026 00:40:59 +0530 Subject: [PATCH 3/3] fix: improve SSA option error message and use constant over magic string - Change error to "invalid server-side apply option" (was misleadingly referencing "release server-side apply method") - Use string(release.ApplyMethodServerSideApply) constant instead of "ssa" - Update test assertions to match new error format Signed-off-by: ayushi-work --- pkg/action/action.go | 4 ++-- pkg/action/uninstall_test.go | 4 ++-- pkg/action/upgrade_test.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index bf036dc8f..505bde144 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -735,13 +735,13 @@ func determineReleaseSSApplyMethod(serverSideApply bool) release.ApplyMethod { func getServerSideApplyValue(serverSideOption string, releaseApplyMethod string) (bool, error) { switch serverSideOption { case "auto": - return releaseApplyMethod == "ssa", nil + return releaseApplyMethod == string(release.ApplyMethodServerSideApply), 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) + return false, fmt.Errorf("invalid server-side apply option %q (must be one of: auto, true, false)", serverSideOption) } } diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index deb166317..26a8db508 100644 --- a/pkg/action/uninstall_test.go +++ b/pkg/action/uninstall_test.go @@ -409,7 +409,7 @@ func TestGetServerSideApplyValue_WithUninstallScenarios(t *testing.T) { name: "invalid option", serverSideOption: "invalid", expectError: true, - errorContains: "invalid/unknown release server-side apply method", + errorContains: "invalid server-side apply option", }, } @@ -459,5 +459,5 @@ func TestUninstall_ServerSideApply_InvalidOption_WithHooks(t *testing.T) { _, err := unAction.Run(rel.Name) assert.Error(t, err) - assert.Contains(t, err.Error(), "invalid/unknown release server-side apply method") + assert.Contains(t, err.Error(), "invalid server-side apply option") } diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 92f152c50..b62573395 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -730,7 +730,7 @@ func TestGetServerSideApplyValue(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)", }, }