fix(api): let the garbage collector finalize Applications (#4610)

## What this PR does

An application deleted with foreground or orphan propagation (`kubectl
delete --cascade=foreground`, or any client that sends that
`propagationPolicy`) could stay in deletion forever. Helm uninstalled
the release and helm-controller removed `finalizers.fluxcd.io`, but the
HelmRelease kept `foregroundDeletion` in `metadata.finalizers`, so the
HelmRelease and its Application stayed terminating. We hit this on
v1.6.4 with a Bucket and an OpenBAO application.

An Application and its HelmRelease share one UID, so the garbage
collector keeps a single graph node for both and may finalize it through
the Application endpoint. The API never returned finalizers on an
Application, and every update restored the HelmRelease finalizers from
the live object.
[`removeFinalizer`](https://github.com/kubernetes/kubernetes/blob/v1.33.1/pkg/controller/garbagecollector/operations.go#L85-L123)
reads the owner, does not find the finalizer there and returns as if it
had already been removed, so nothing ever took it off the HelmRelease.

This PR:

- returns `foregroundDeletion` and `orphan` in `metadata.finalizers` of
an Application, and only those two;
- carries an add or a remove of either of them from an Application
update to the HelmRelease, as a change against the finalizers the update
started from, so the result is the same when helm-controller or the
garbage collector changes the list during the conflict retry;
- keeps controller finalizers such as `finalizers.fluxcd.io` owned by
the live object, so `helm uninstall` still runs before the release goes
away;
- copies the deletion timestamp and grace period from the live object
into the rebuilt HelmRelease, so an update of a terminating release
stays valid.

`rest_garbage_collection_test.go` covers both finalizers with Helm
finishing first and with the garbage collector finishing first,
concurrent finalizer changes during the conflict retry, controller
finalizers surviving an update, the cached HelmRelease not being mutated
by the conversion, and an `UpdatedObject` that edits the object in
place. All five tests fail on the current `main` without this change.
`go test -race ./pkg/registry/...` passes. The tests use the
controller-runtime fake client, so this has not been run against a live
garbage collector with a patched cozystack-api yet.

The same bug is in release-1.6. There the Update path has no finalizer
carry-over at all, because #2914 is not backported yet, and its
automatic backport #4577 currently carries conflict markers. A 1.6
backport of this fix should go on top of a working backport of #2914.

### Screenshots

Not a UI change.

### Downstream repositories

Walked the trigger map against the diff: it only changes how
cozystack-api handles `metadata.finalizers` of Applications in
`pkg/registry/apps/application`. No package, values, schema, CRD,
`hack/` file or convention is touched.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
fix(api): an application deleted with foreground or orphan propagation no longer leaves its HelmRelease stuck with the foregroundDeletion or orphan finalizer
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Application updates now preserve Kubernetes deletion behavior,
including foreground deletion and orphaning, even when concurrent
changes occur.
* Helm-managed protection metadata is retained during updates, and
deletion timing details remain intact.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

pkg/registry/apps/application/rest.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"maps"
 	"net/http"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -523,6 +524,8 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje
 		return nil, false, err
 	}
 
+	previousFinalizers := slices.Clone(oldObj.(*appsv1alpha1.Application).Finalizers)
+
 	// Update the Application object
 	newObj, err := objInfo.UpdatedObject(ctx, oldObj)
 	if err != nil {
@@ -587,7 +590,7 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje
 	if helmRelease.ResourceVersion == "" {
 		helmRelease.SetResourceVersion(cur.GetResourceVersion())
 	}
-	r.applyLiveMetadata(helmRelease, cur, app)
+	r.applyLiveMetadata(helmRelease, cur, app, previousFinalizers)
 
 	// Suspension is not part of the Application, so the rebuilt object always
 	// says suspend=false, and sending it as is resumes a release that an
@@ -628,7 +631,7 @@ func (r *REST) Update(ctx context.Context, name string, objInfo rest.UpdatedObje
 				return getErr
 			}
 			helmRelease = converted.DeepCopy()
-			r.applyLiveMetadata(helmRelease, cur, app)
+			r.applyLiveMetadata(helmRelease, cur, app, previousFinalizers)
 			helmRelease.SetResourceVersion(cur.GetResourceVersion())
 			helmRelease.Spec.Suspend = cur.Spec.Suspend
 		}
@@ -1220,10 +1223,36 @@ func filterPrefixedMap(original map[string]string, prefix string) map[string]str
 	return processed
 }
 
+func garbageCollectionFinalizers(finalizers []string) []string {
+	var result []string
+	for _, finalizer := range finalizers {
+		if finalizer == metav1.FinalizerDeleteDependents || finalizer == metav1.FinalizerOrphanDependents {
+			result = append(result, finalizer)
+		}
+	}
+	return result
+}
+
+func mergeGarbageCollectionFinalizers(current, previous, desired []string) []string {
+	result := slices.Clone(current)
+	for _, finalizer := range []string{metav1.FinalizerDeleteDependents, metav1.FinalizerOrphanDependents} {
+		before, after := slices.Contains(previous, finalizer), slices.Contains(desired, finalizer)
+		if before == after {
+			continue
+		}
+		if !after {
+			result = slices.DeleteFunc(result, func(f string) bool { return f == finalizer })
+		} else if !slices.Contains(result, finalizer) {
+			result = append(result, finalizer)
+		}
+	}
+	return result
+}
+
 // applyLiveMetadata assembles the metadata of a HelmRelease rebuilt from an
 // Application, given the live object it is about to replace. The PUT is a full
 // replace, so anything not restated here is dropped:
-//   - finalizers: helm-controller's finalizers.fluxcd.io guarantees
+//   - controller finalizers: helm-controller's finalizers.fluxcd.io guarantees
 //     `helm uninstall` runs on deletion. Stripping it lets a subsequent delete
 //     remove the HelmRelease instantly, orphaning every resource of the release.
 //   - ownerReferences: garbage collection and lineage.
@@ -1234,8 +1263,10 @@ func filterPrefixedMap(original map[string]string, prefix string) map[string]str
 //
 // hr must be a fresh conversion output: the function overlays the live metadata
 // and is not idempotent against its own result.
-func (r *REST) applyLiveMetadata(hr, cur *helmv2.HelmRelease, app *appsv1alpha1.Application) {
-	hr.Finalizers = cur.Finalizers
+func (r *REST) applyLiveMetadata(hr, cur *helmv2.HelmRelease, app *appsv1alpha1.Application, previousFinalizers []string) {
+	hr.Finalizers = mergeGarbageCollectionFinalizers(cur.Finalizers, previousFinalizers, app.Finalizers)
+	hr.DeletionTimestamp = cur.DeletionTimestamp.DeepCopy()
+	hr.DeletionGracePeriodSeconds = cur.DeletionGracePeriodSeconds
 	hr.OwnerReferences = cur.OwnerReferences
 	hr.Labels = mergeMaps(omitPrefixedMap(cur.Labels, LabelPrefix), hr.Labels)
 	hr.Annotations = mergeMaps(omitPrefixedMap(cur.Annotations, AnnotationPrefix), hr.Annotations)
@@ -1513,6 +1544,7 @@ func (r *REST) convertHelmReleaseToApplication(ctx context.Context, hr *helmv2.H
 			ResourceVersion:   hr.GetResourceVersion(),
 			CreationTimestamp: hr.CreationTimestamp,
 			DeletionTimestamp: hr.DeletionTimestamp,
+			Finalizers:        garbageCollectionFinalizers(hr.Finalizers),
 			Labels:            filterPrefixedMap(hr.Labels, LabelPrefix),
 			Annotations:       filterPrefixedMap(hr.Annotations, AnnotationPrefix),
 		},
@@ -1744,6 +1776,7 @@ func (r *REST) convertApplicationToHelmRelease(app *appsv1alpha1.Application) (*
 			Annotations:     addPrefixedMap(app.Annotations, AnnotationPrefix),
 			ResourceVersion: app.ResourceVersion,
 			UID:             app.UID,
+			Finalizers:      garbageCollectionFinalizers(app.Finalizers),
 		},
 		Spec: helmv2.HelmReleaseSpec{
 			ChartRef: &helmv2.CrossNamespaceSourceReference{

pkg/registry/apps/application/rest_garbage_collection_test.go

@@ -0,0 +1,196 @@
+package application
+
+import (
+	"context"
+	"errors"
+	"slices"
+	"testing"
+
+	helmv2 "github.com/fluxcd/helm-controller/api/v2"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/apiserver/pkg/registry/rest"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+
+	appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1"
+)
+
+func TestApplicationGarbageCollection(t *testing.T) {
+	for _, finalizer := range []string{metav1.FinalizerDeleteDependents, metav1.FinalizerOrphanDependents} {
+		for _, helmFirst := range []bool{true, false} {
+			order := "gc-first"
+			if helmFirst {
+				order = "helm-first"
+			}
+			t.Run(finalizer+"/"+order, func(t *testing.T) {
+				hr := optionsTestHelmRelease()
+				hr.UID = "release-uid"
+				now := metav1.Now()
+				hr.DeletionTimestamp = &now
+				hr.Finalizers = []string{finalizer}
+				if !helmFirst {
+					hr.Finalizers = append(hr.Finalizers, "finalizers.fluxcd.io")
+				}
+				r := newOptionsTestREST(t, interceptor.Funcs{}, hr)
+				ctx := request.WithNamespace(context.Background(), optionsTestNamespace)
+				obj, err := r.Get(ctx, "example", &metav1.GetOptions{})
+				if err != nil {
+					t.Fatal(err)
+				}
+				app := obj.(*appsv1alpha1.Application)
+				if app.UID != hr.UID || app.DeletionTimestamp == nil {
+					t.Fatalf("deletion identity lost: %v", app.ObjectMeta)
+				}
+				if !slices.Contains(app.Finalizers, finalizer) {
+					t.Fatalf("garbage collector cannot see %q: %v", finalizer, app.Finalizers)
+				}
+				app.Finalizers = slices.DeleteFunc(app.Finalizers, func(f string) bool { return f == finalizer })
+				if _, _, err := r.Update(ctx, app.Name, rest.DefaultUpdatedObjectInfo(app), nil, nil, false, &metav1.UpdateOptions{}); err != nil {
+					t.Fatal(err)
+				}
+				live := &helmv2.HelmRelease{}
+				err = r.c.Get(ctx, client.ObjectKeyFromObject(hr), live)
+				if helmFirst {
+					if !apierrors.IsNotFound(err) {
+						t.Fatalf("release still exists after cleanup: err=%v, finalizers=%v", err, live.Finalizers)
+					}
+					return
+				}
+				if err != nil {
+					t.Fatal(err)
+				}
+				if !slices.Equal(live.Finalizers, []string{"finalizers.fluxcd.io"}) {
+					t.Fatalf("Helm cleanup protection changed: %v", live.Finalizers)
+				}
+				live.Finalizers = nil
+				if err := r.c.Update(ctx, live); err != nil {
+					t.Fatal(err)
+				}
+				if err := r.c.Get(ctx, client.ObjectKeyFromObject(hr), live); !apierrors.IsNotFound(err) {
+					t.Fatalf("release still exists after Helm cleanup: %v", err)
+				}
+			})
+		}
+	}
+}
+
+func TestApplicationGarbageCollectionConflict(t *testing.T) {
+	for _, concurrent := range []struct {
+		name      string
+		initial   []string
+		requested []string
+		updated   []string
+		expected  []string
+	}{
+		{"helm-completes", []string{metav1.FinalizerDeleteDependents, "finalizers.fluxcd.io", "test.example/hold"}, nil, []string{metav1.FinalizerDeleteDependents, "test.example/hold"}, []string{"test.example/hold"}},
+		{"controller-adds-finalizer", []string{metav1.FinalizerDeleteDependents}, nil, []string{metav1.FinalizerDeleteDependents, "finalizers.fluxcd.io"}, []string{"finalizers.fluxcd.io"}},
+		{"gc-starts-during-update", []string{"finalizers.fluxcd.io"}, nil, []string{"finalizers.fluxcd.io", metav1.FinalizerDeleteDependents}, []string{"finalizers.fluxcd.io", metav1.FinalizerDeleteDependents}},
+		{"gc-finishes-during-update", []string{"finalizers.fluxcd.io", metav1.FinalizerDeleteDependents}, []string{metav1.FinalizerDeleteDependents}, []string{"finalizers.fluxcd.io"}, []string{"finalizers.fluxcd.io"}},
+	} {
+		t.Run(concurrent.name, func(t *testing.T) {
+			hr := optionsTestHelmRelease()
+			hr.Finalizers = concurrent.initial
+			updates := 0
+			r := newOptionsTestREST(t, interceptor.Funcs{
+				Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
+					updates++
+					if updates > 1 {
+						return c.Update(ctx, obj, opts...)
+					}
+					live := &helmv2.HelmRelease{}
+					if err := c.Get(ctx, client.ObjectKeyFromObject(hr), live); err != nil {
+						return err
+					}
+					live.Finalizers = slices.Clone(concurrent.updated)
+					if err := c.Update(ctx, live); err != nil {
+						return err
+					}
+					return apierrors.NewConflict(helmv2.GroupVersion.WithResource("helmreleases").GroupResource(), hr.Name, errors.New("concurrent controller update"))
+				},
+			}, hr)
+			ctx := request.WithNamespace(context.Background(), optionsTestNamespace)
+			obj, err := r.Get(ctx, "example", &metav1.GetOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			app := obj.(*appsv1alpha1.Application)
+			app.Finalizers = concurrent.requested
+			if _, _, err := r.Update(ctx, app.Name, rest.DefaultUpdatedObjectInfo(app), nil, nil, false, &metav1.UpdateOptions{}); err != nil {
+				t.Fatal(err)
+			}
+			live := &helmv2.HelmRelease{}
+			if err := r.c.Get(ctx, client.ObjectKeyFromObject(hr), live); err != nil {
+				t.Fatal(err)
+			}
+			if updates != 2 {
+				t.Fatalf("update calls = %d, want 2", updates)
+			}
+			if !slices.Equal(live.Finalizers, concurrent.expected) {
+				t.Fatalf("finalizers = %v, want %v", live.Finalizers, concurrent.expected)
+			}
+		})
+	}
+}
+
+func TestApplicationFinalizersKeepControllerProtection(t *testing.T) {
+	hr := optionsTestHelmRelease()
+	hr.Finalizers = []string{"finalizers.fluxcd.io", metav1.FinalizerDeleteDependents}
+	r := newOptionsTestREST(t, interceptor.Funcs{}, hr)
+	ctx := request.WithNamespace(context.Background(), optionsTestNamespace)
+	obj, err := r.Get(ctx, "example", &metav1.GetOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	app := obj.(*appsv1alpha1.Application)
+	if !slices.Equal(app.Finalizers, []string{metav1.FinalizerDeleteDependents}) {
+		t.Fatalf("unexpected application finalizers: %v", app.Finalizers)
+	}
+	app.Finalizers = []string{"other.example/ignored"}
+	if _, _, err := r.Update(ctx, app.Name, rest.DefaultUpdatedObjectInfo(app), nil, nil, false, &metav1.UpdateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+	live := &helmv2.HelmRelease{}
+	if err := r.c.Get(ctx, client.ObjectKeyFromObject(hr), live); err != nil {
+		t.Fatal(err)
+	}
+	if !slices.Equal(live.Finalizers, []string{"finalizers.fluxcd.io"}) {
+		t.Fatalf("controller finalizers changed: %v", live.Finalizers)
+	}
+}
+
+func TestApplicationFinalizersDoNotMutateCachedHelmRelease(t *testing.T) {
+	hr := optionsTestHelmRelease()
+	hr.Finalizers = []string{metav1.FinalizerDeleteDependents, "finalizers.fluxcd.io"}
+	r := newOptionsTestREST(t, interceptor.Funcs{})
+	app, err := r.ConvertHelmReleaseToApplication(context.Background(), hr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	app.Finalizers[0] = metav1.FinalizerOrphanDependents
+	if !slices.Equal(hr.Finalizers, []string{metav1.FinalizerDeleteDependents, "finalizers.fluxcd.io"}) {
+		t.Fatalf("conversion changed cached finalizers: %v", hr.Finalizers)
+	}
+}
+
+func TestApplicationGarbageCollectionWithInPlaceUpdate(t *testing.T) {
+	hr := optionsTestHelmRelease()
+	hr.Finalizers = []string{metav1.FinalizerDeleteDependents}
+	now := metav1.Now()
+	hr.DeletionTimestamp = &now
+	r := newOptionsTestREST(t, interceptor.Funcs{}, hr)
+	ctx := request.WithNamespace(context.Background(), optionsTestNamespace)
+	update := rest.DefaultUpdatedObjectInfo(nil, func(_ context.Context, _, old runtime.Object) (runtime.Object, error) {
+		app := old.(*appsv1alpha1.Application)
+		app.Finalizers = nil
+		return app, nil
+	})
+	if _, _, err := r.Update(ctx, "example", update, nil, nil, false, &metav1.UpdateOptions{}); err != nil {
+		t.Fatal(err)
+	}
+	if err := r.c.Get(ctx, client.ObjectKeyFromObject(hr), &helmv2.HelmRelease{}); !apierrors.IsNotFound(err) {
+		t.Fatalf("release still exists after in-place finalizer removal: %v", err)
+	}
+}