From 76a42b3e2d3cc9a5e22152ecf198555df7d0bbfb Mon Sep 17 00:00:00 2001 From: Gates Wang <9372086+SetagGnaw@users.noreply.github.com> Date: Mon, 20 Jul 2026 21:04:39 -0400 Subject: [PATCH] fix(action): only ignore not-found on uninstall --ignore-not-found The non-dry-run uninstall path suppressed every error returned by Releases.History when IgnoreNotFound was set, so a genuine storage failure was reported as a successful uninstall (exit 0) even though the release still existed and nothing was deleted. Guard the suppression with errors.Is(err, driver.ErrReleaseNotFound), matching the dry-run path, so only a missing release is ignored and any other error propagates. Add a regression test that injects a non-sentinel storage error and asserts it surfaces. Signed-off-by: Gates Wang <9372086+SetagGnaw@users.noreply.github.com> --- pkg/action/uninstall.go | 2 +- pkg/action/uninstall_test.go | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 47b2e47de..f5db2f3f7 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -164,7 +164,7 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) relsi, err := u.cfg.Releases.History(name) if err != nil { - if u.IgnoreNotFound { + if u.IgnoreNotFound && errors.Is(err, driver.ErrReleaseNotFound) { return nil, nil } return nil, fmt.Errorf("uninstall: Release not loaded: %s: %w", name, err) diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index 913ade1a5..dc5b3c494 100644 --- a/pkg/action/uninstall_test.go +++ b/pkg/action/uninstall_test.go @@ -19,6 +19,7 @@ package action import ( "bytes" "errors" + "fmt" "io" "log/slog" "testing" @@ -28,7 +29,10 @@ import ( "helm.sh/helm/v4/pkg/kube" kubefake "helm.sh/helm/v4/pkg/kube/fake" + "helm.sh/helm/v4/pkg/release" "helm.sh/helm/v4/pkg/release/common" + "helm.sh/helm/v4/pkg/storage" + "helm.sh/helm/v4/pkg/storage/driver" ) func uninstallAction(t *testing.T) *Uninstall { @@ -59,6 +63,46 @@ func TestUninstallRelease_ignoreNotFound(t *testing.T) { is.Nil(res) is.NoError(err) } + +// queryFailingDriver wraps a storage driver but forces Query (the method +// Storage.History calls) to fail with a supplied error. It lets a test +// simulate a storage-backend failure instead of a genuine "release not found". +type queryFailingDriver struct { + driver.Driver + queryErr error +} + +func (d *queryFailingDriver) Query(_ map[string]string) ([]release.Releaser, error) { + return nil, d.queryErr +} + +// Regression test: uninstall --ignore-not-found must ignore only a genuine +// driver.ErrReleaseNotFound. Any other storage failure surfaces as a wrapped, +// non-sentinel error and must propagate, not be swallowed into a false success. +// See pkg/action/uninstall.go. +func TestUninstallRelease_ignoreNotFound_realStorageError(t *testing.T) { + is := assert.New(t) + + unAction := uninstallAction(t) + unAction.DryRun = false + unAction.IgnoreNotFound = true + + // Mirror how the Secrets driver reports a backend failure: a wrapped error + // that is NOT driver.ErrReleaseNotFound. + backendErr := errors.New("forbidden: user cannot list secrets") + failingDriver := &queryFailingDriver{ + Driver: driver.NewMemory(), + queryErr: fmt.Errorf("query: failed to query with labels: %w", backendErr), + } + unAction.cfg.Releases = storage.Init(failingDriver) + + res, err := unAction.Run("release-non-exist") + is.Nil(res) + is.Error(err) + is.ErrorContains(err, "forbidden: user cannot list secrets") + // The failure must not be misclassified as a not-found and swallowed. + is.False(errors.Is(err, driver.ErrReleaseNotFound)) +} func TestUninstallRelease_deleteRelease(t *testing.T) { is := assert.New(t) req := require.New(t)