Repository navigation
fix: check ownership before delete in upgrade/rollback/install (#32218) - #32793
Open
silence-coding wants to merge 1 commit into
Open
silence-coding wants to merge 1 commit into
silence-coding wants to merge 1 commit into
Conversation
This was referenced Oct 21, 2026
Closed
silence-coding
force-pushed
the
fix/ownership-check-delete-path
branch
2 times, most recently
from
October 21, 2026 14:17
58ad34b to
9d59131
Compare
The delete phase of Update (pkg/kube/client.go) is annotation-blind: a resource present in the original release but absent from the target is deleted without consulting meta.helm.sh/release-name. When a resource has been adopted by another release (via --take-ownership), the source release still deletes it on its next upgrade/rollback, corrupting the adopter's state. This is helm#32218. The uninstall path already guards against this via verifyOwnershipBeforeDelete (helm#31584). This extends the same principle to the shared update() delete loop, which is reached by upgrade, rollback, and install. Add an opt-in ClientUpdateOptionOwnership(releaseName). When set, the delete loop reads the live resource's release-name annotation and skips deletion when it belongs to a different release. Resources owned by the operating release, or carrying no annotation, are deleted as before. When releaseName is empty the safeguard is disabled and behavior is unchanged. Wire the option into all three Update call sites: - pkg/action/upgrade.go (releasingUpgrade) - pkg/action/rollback.go (performRollback) - pkg/action/install.go (performInstall) Compared to the stalled draft helm#32257 (upgrade-only), this also covers rollback and install, and checks only release-name (not namespace) to avoid edge-case false skips across namespaces. Verified on a k3s cluster with a cross-release transfer scenario: after release B adopts a resource from A via --take-ownership, upgrading A no longer deletes it. The unmodified build deletes it. A bidirectional swap (A->B and B->A via upgrade) keeps both resources alive with correct ownership on the fixed build; the unmodified build loses one. Regular upgrades that remove a release's own resources still delete them. Signed-off-by: silence-coding <32766901+silence-coding@users.noreply.github.com>
silence-coding
force-pushed
the
fix/ownership-check-delete-path
branch
from
October 21, 2026 14:20
9d59131 to
71f840c
Compare
Author
|
@TerryHowe Could you take a look at this when you have a moment? I figured you'd be familiar with this area. This addresses #32218. I'm happy to make any changes based on your feedback. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Fixes #32218.
The delete phase of
Update(pkg/kube/client.go) is annotation-blind: a resource present in the original release but absent from the target is deleted without consultingmeta.helm.sh/release-name. When a resource has been adopted by another release via--take-ownership, the source release still deletes it on its next upgrade/rollback, corrupting the adopter's state.The uninstall path already guards against this via
verifyOwnershipBeforeDelete(#31584). This PR extends the same principle to the sharedupdate()delete loop, which is reached by upgrade, rollback, and install.Changes:
ClientUpdateOptionOwnership(releaseName)topkg/kube/client.go. When set, the delete loop reads the live resource'smeta.helm.sh/release-nameannotation and skips deletion when it belongs to a different release. Resources owned by the operating release, or carrying no annotation, are deleted as before. WhenreleaseNameis empty the safeguard is disabled and behavior is unchanged.Updatecall sites:upgrade.go(releasingUpgrade),rollback.go(performRollback),install.go(performInstall).release-nameis checked (notnamespace) to avoid edge-case false skips across namespaces. The existing private constant inpkg/action/validate.gois left untouched;pkg/kubeuses its own package-private constant since it cannot importpkg/action.Special notes for your reviewer:
This is an alternative to the stalled draft #32257 (upgrade-only). Differences:
Updatecall sites — upgrade, rollback, and install — not just upgrade. They share the sameupdate()delete loop.meta.helm.sh/release-name(notrelease-namespace) to avoid edge-case false skips for cluster-scoped or cross-namespace resources.ClientUpdateOptionOwnership(releaseName)option.The three action call sites always pass the release name, so the safeguard is always active in practice. Keeping it an explicit option preserves the
pkg/kubeAPI contract (HIP-0004):Update's option list is additive, andclientUpdateOptions.releaseNamedefaults to empty (disabled), so any direct caller ofpkg/kube.Client.Updatesees no behavior change.A true non-destructive transfer (adopt-before-delete, no delete/recreate) is a larger enhancement and is out of scope for this fix. This PR targets the correctness goal in #32218: a release must not delete a resource owned by another release.
If applicable:
docs neededlabel should be applied if so)User-facing change:
helm upgrade/rollback/installno longer deletes a resource that has been adopted by another release (via--take-ownership), and logsskipping delete of resource not owned by this releasewhen skipping. No docs update needed — the existing--take-ownershipdocumentation already describes the intended behavior ("Subsequent upgrades and uninstalls then treat those objects as part of the release"); this fix makes the actual behavior match that description for the source release.Test plan
go build ./...passesgo vet ./pkg/kube/ ./pkg/action/passesgo test ./pkg/kube/... ./pkg/action/...passespkg/kube/client_test.go:TestUpdateOwnershipCheck: foreign-owned resource skipped; owned + no-annotation resources deleted; option omitted → all deleted (regression)shared-cm, B adopts via--take-ownership, upgrade A removes it. Fixed build:shared-cmsurvives (owned by b). Unmodified: deleted.Backward compatibility: the
pkg/kube.Client.Updatesignature is unchanged; only a newClientUpdateOptionis added. Direct callers that do not passClientUpdateOptionOwnershipsee identical behavior. CLI flags and workflows are unchanged. This conforms to HIP-0004.