From d149937ea3c5710449724c28f88c7376af283bfb Mon Sep 17 00:00:00 2001 From: cg49996w11 Date: Mon, 22 Jun 2026 23:53:00 +0530 Subject: [PATCH] helm upgrade: check ownership for deletion case Signed-off-by: cg49996w11 --- pkg/action/upgrade.go | 4 +- pkg/kube/client.go | 34 +++++++++++++- pkg/kube/client_test.go | 89 +++++++++++++++++++++++++++++++++++++ pkg/kube/resource_policy.go | 6 +++ 4 files changed, 130 insertions(+), 3 deletions(-) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 81d51164f..dcf49d1f8 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -467,12 +467,14 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele } upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply // Update client-side field manager if transitioning from client-side to server-side apply + results, err := u.cfg.KubeClient.Update( current, target, kube.ClientUpdateOptionForceReplace(u.ForceReplace), kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), - kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) + kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager), + kube.ClientUpdateOptionOwnership(upgradedRelease.Name, upgradedRelease.Namespace)) if err != nil { u.cfg.recordRelease(originalRelease) u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 7b02f47ef..65a9cb492 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -565,7 +565,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro transformRequests) } -func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc) (*Result, error) { +func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateApplyFunc, updateApplyFunc UpdateApplyFunc, releaseName, releaseNamespace string) (*Result, error) { updateErrors := []error{} res := &Result{} @@ -674,6 +674,21 @@ func (c *Client) update(originals, targets ResourceList, createApplyFunc CreateA c.Logger().Debug("skipping delete due to annotation", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, "annotation", ResourcePolicyAnno, "value", KeepPolicy) continue } + if releaseName != "" && annotations != nil { + annoReleaseName := annotations[ReleaseNameAnnotation] + annoReleaseNS := annotations[ReleaseNamespaceAnnotation] + ownedByDifferentRelease := (annoReleaseName != "" && annoReleaseName != releaseName) || + (releaseNamespace != "" && annoReleaseNS != "" && annoReleaseNS != releaseNamespace) + if ownedByDifferentRelease { + c.Logger().Warn("skipping delete of resource not owned by this release", + slog.String("namespace", info.Namespace), + slog.String("name", info.Name), + slog.String("kind", info.Mapping.GroupVersionKind.Kind), + slog.String("release", releaseName), + ) + continue + } + } if err := deleteResource(info, metav1.DeletePropagationBackground); err != nil { c.Logger().Debug( "failed to delete resource", @@ -706,6 +721,8 @@ type clientUpdateOptions struct { dryRun bool fieldValidationDirective FieldValidationDirective upgradeClientSideFieldManager bool + releaseName string + releaseNamespace string } type ClientUpdateOption func(*clientUpdateOptions) error @@ -789,6 +806,19 @@ func ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManag } } +// ClientUpdateOptionOwnership specifies the release name and namespace that owns the resources being updated. +// When set, orphaned resources (present in the original list but not in the target list) will only be deleted +// if their meta.helm.sh/release-name and meta.helm.sh/release-namespace annotations match the specified +// release. Resources annotated as belonging to a different release will be skipped. +func ClientUpdateOptionOwnership(releaseName, releaseNamespace string) ClientUpdateOption { + return func(o *clientUpdateOptions) error { + o.releaseName = releaseName + o.releaseNamespace = releaseNamespace + + return nil + } +} + // Update takes the current list of objects and target list of objects and // creates resources that don't already exist, updates resources that have been // modified in the target configuration, and deletes resources from the current @@ -895,7 +925,7 @@ func (c *Client) Update(originals, targets ResourceList, options ...ClientUpdate } } - return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc()) + return c.update(originals, targets, createApplyFunc, makeUpdateApplyFunc(), updateOptions.releaseName, updateOptions.releaseNamespace) } // Delete deletes Kubernetes resources specified in the resources list with diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index 131695dc2..655651c86 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -574,6 +574,95 @@ func TestUpdate(t *testing.T) { } } +func TestUpdateOwnershipCheck(t *testing.T) { + // Verify that resources owned by a different release are skipped during deletion. + // "keep" is in originals but not in targets; it is annotated as owned by "other-release". + // "squid" is in originals but not in targets, and has no ownership annotations — it must be deleted. + const ( + currentRelease = "my-release" + currentNamespace = "my-ns" + differentRelease = "other-release" + differentNS = "other-ns" + ) + + keepPod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "keep", + Namespace: v1.NamespaceDefault, + SelfLink: "/api/v1/namespaces/default/pods/keep", + Annotations: map[string]string{ + ReleaseNameAnnotation: differentRelease, + ReleaseNamespaceAnnotation: differentNS, + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + Name: "app:v4", + Image: "abc/app:v4", + Ports: []v1.ContainerPort{{Name: "http", ContainerPort: 80}}, + }}, + }, + } + + originalPods := newPodList("starfish", "squid") + originalPods.Items = append(originalPods.Items, keepPod) + targetPods := newPodList("starfish") + + cb := func(_ []RequestResponseAction, req *http.Request) (*http.Response, error) { + p, m := req.URL.Path, req.Method + switch { + case p == "/namespaces/default/pods/starfish" && m == http.MethodGet: + return newResponse(http.StatusOK, &originalPods.Items[0]) + case p == "/namespaces/default/pods/starfish" && m == http.MethodPatch: + return newResponse(http.StatusOK, &targetPods.Items[0]) + case p == "/namespaces/default/pods/squid" && m == http.MethodGet: + return newResponse(http.StatusOK, &originalPods.Items[1]) + case p == "/namespaces/default/pods/squid" && m == http.MethodDelete: + return newResponse(http.StatusOK, &originalPods.Items[1]) + case p == "/namespaces/default/pods/keep" && m == http.MethodGet: + return newResponse(http.StatusOK, &keepPod) + case p == "/namespaces/default/pods/keep" && m == http.MethodDelete: + t.Errorf("DELETE called on resource owned by different release") + return newResponse(http.StatusOK, &keepPod) + } + t.Logf("Unhandled request: %s %s", m, p) + t.FailNow() + return nil, nil + } + + c := newTestClient(t) + client := NewRequestResponseLogClient(t, cb) + c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{ + NegotiatedSerializer: unstructuredSerializer, + Client: fake.CreateHTTPClient(client.Do), + } + + originals, err := c.Build(objBody(&originalPods), false) + require.NoError(t, err) + + targets, err := c.Build(objBody(&targetPods), false) + require.NoError(t, err) + + result, err := c.Update( + originals, + targets, + ClientUpdateOptionServerSideApply(true, false), + ClientUpdateOptionOwnership(currentRelease, currentNamespace), + ) + require.NoError(t, err) + + // "squid" should be deleted; "keep" (owned by other-release) should not + assert.Len(t, result.Deleted, 1, "expected 1 resource deleted (squid), got %d", len(result.Deleted)) + assert.Equal(t, "squid", result.Deleted[0].Name) + + // Verify DELETE was never called for "keep" + for _, action := range client.Actions { + if action.Request.URL.Path == "/namespaces/default/pods/keep" { + assert.NotEqual(t, http.MethodDelete, action.Request.Method, "DELETE should not be called for resource owned by a different release") + } + } +} + func TestBuild(t *testing.T) { tests := []struct { name string diff --git a/pkg/kube/resource_policy.go b/pkg/kube/resource_policy.go index 3a271877c..77914ec69 100644 --- a/pkg/kube/resource_policy.go +++ b/pkg/kube/resource_policy.go @@ -25,3 +25,9 @@ const ResourcePolicyAnno = "helm.sh/resource-policy" // // during an uninstallRelease action. const KeepPolicy = "keep" + +// ReleaseNameAnnotation is the annotation that tracks which release owns a resource +const ReleaseNameAnnotation = "meta.helm.sh/release-name" + +// ReleaseNamespaceAnnotation is the annotation that tracks which release namespace owns a resource +const ReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace"