Merge ccce0b3b22 into 2b128dcd27

pkg/action/upgrade.go

@@ -466,12 +466,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)

pkg/action/validate.go

@@ -34,8 +34,6 @@ var accessor = meta.NewAccessor()
 const (
 	appManagedByLabel = "app.kubernetes.io/managed-by"
 	appManagedByHelm  = "Helm"
-	helmReleaseNameAnnotation      = "meta.helm.sh/release-name"
-	helmReleaseNamespaceAnnotation = "meta.helm.sh/release-namespace"
 )
 
 // requireAdoption returns the subset of resources that already exist in the cluster.
@@ -181,10 +179,10 @@ func checkOwnership(obj runtime.Object, releaseName, releaseNamespace string) er
 	if err := requireValue(lbls, appManagedByLabel, appManagedByHelm); err != nil {
 		errs = append(errs, fmt.Errorf("label validation error: %w", err))
 	}
-	if err := requireValue(annos, helmReleaseNameAnnotation, releaseName); err != nil {
+	if err := requireValue(annos, kube.ReleaseNameAnnotation, releaseName); err != nil {
 		errs = append(errs, fmt.Errorf("annotation validation error: %w", err))
 	}
-	if err := requireValue(annos, helmReleaseNamespaceAnnotation, releaseNamespace); err != nil {
+	if err := requireValue(annos, kube.ReleaseNamespaceAnnotation, releaseNamespace); err != nil {
 		errs = append(errs, fmt.Errorf("annotation validation error: %w", err))
 	}
 
@@ -231,8 +229,8 @@ func setMetadataVisitor(releaseName, releaseNamespace string, forceOwnership boo
 		}
 
 		if err := mergeAnnotations(info.Object, map[string]string{
-			helmReleaseNameAnnotation:      releaseName,
-			helmReleaseNamespaceAnnotation: releaseNamespace,
+			kube.ReleaseNameAnnotation:      releaseName,
+			kube.ReleaseNamespaceAnnotation: releaseNamespace,
 		}); err != nil {
 			return fmt.Errorf(
 				"%s annotations could not be updated: %w",

pkg/kube/client.go

@@ -572,7 +572,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{}
 
@@ -681,6 +681,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",
@@ -713,6 +728,8 @@ type clientUpdateOptions struct {
 	dryRun                        bool
 	fieldValidationDirective      FieldValidationDirective
 	upgradeClientSideFieldManager bool
+	releaseName                   string
+	releaseNamespace              string
 }
 
 type ClientUpdateOption func(*clientUpdateOptions) error
@@ -796,6 +813,22 @@ 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 {
+		if releaseName == "" {
+			return errors.New("releaseName must not be empty for ownership check")
+		}
+		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
@@ -902,7 +935,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

pkg/kube/client_test.go

@@ -592,6 +592,111 @@ 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 TestClientUpdateOptionOwnershipValidation(t *testing.T) {
+	// An empty releaseName must be rejected so callers cannot silently disable
+	// the ownership check by passing a zero-value string.
+	opt := ClientUpdateOptionOwnership("", "some-ns")
+	var o clientUpdateOptions
+	err := opt(&o)
+	assert.ErrorContains(t, err, "releaseName must not be empty")
+
+	// Non-empty releaseName with empty namespace is allowed (cluster-scoped resources).
+	opt2 := ClientUpdateOptionOwnership("my-release", "")
+	err = opt2(&o)
+	assert.NoError(t, err)
+	assert.Equal(t, "my-release", o.releaseName)
+	assert.Equal(t, "", o.releaseNamespace)
+}
+
 func TestBuild(t *testing.T) {
 	tests := []struct {
 		name      string

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"