helm upgrade: check ownership for deletion case

Signed-off-by: cg49996w11 <cg49996w11@gmail.com>
pull/32257/head
cg49996w11 3 months ago
parent 74fa4fceb8
commit d149937ea3

@ -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 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( results, err := u.cfg.KubeClient.Update(
current, current,
target, target,
kube.ClientUpdateOptionForceReplace(u.ForceReplace), kube.ClientUpdateOptionForceReplace(u.ForceReplace),
kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts), kube.ClientUpdateOptionServerSideApply(serverSideApply, u.ForceConflicts),
kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager)) kube.ClientUpdateOptionUpgradeClientSideFieldManager(upgradeClientSideFieldManager),
kube.ClientUpdateOptionOwnership(upgradedRelease.Name, upgradedRelease.Namespace))
if err != nil { if err != nil {
u.cfg.recordRelease(originalRelease) u.cfg.recordRelease(originalRelease)
u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err) u.reportToPerformUpgrade(c, upgradedRelease, results.Created, err)

@ -565,7 +565,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro
transformRequests) 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{} updateErrors := []error{}
res := &Result{} 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) c.Logger().Debug("skipping delete due to annotation", "namespace", info.Namespace, "name", info.Name, "kind", info.Mapping.GroupVersionKind.Kind, "annotation", ResourcePolicyAnno, "value", KeepPolicy)
continue 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 { if err := deleteResource(info, metav1.DeletePropagationBackground); err != nil {
c.Logger().Debug( c.Logger().Debug(
"failed to delete resource", "failed to delete resource",
@ -706,6 +721,8 @@ type clientUpdateOptions struct {
dryRun bool dryRun bool
fieldValidationDirective FieldValidationDirective fieldValidationDirective FieldValidationDirective
upgradeClientSideFieldManager bool upgradeClientSideFieldManager bool
releaseName string
releaseNamespace string
} }
type ClientUpdateOption func(*clientUpdateOptions) error 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 // 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 // creates resources that don't already exist, updates resources that have been
// modified in the target configuration, and deletes resources from the current // 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 // Delete deletes Kubernetes resources specified in the resources list with

@ -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) { func TestBuild(t *testing.T) {
tests := []struct { tests := []struct {
name string name string

@ -25,3 +25,9 @@ const ResourcePolicyAnno = "helm.sh/resource-policy"
// //
// during an uninstallRelease action. // during an uninstallRelease action.
const KeepPolicy = "keep" 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"

Loading…
Cancel
Save