fix(api): let the garbage collector finalize Applications

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. When the collector came to drop foregroundDeletion or
orphan through the Application, removeFinalizer found no such finalizer
and returned as if it were already gone. The HelmRelease then kept
foregroundDeletion after Helm had uninstalled the release, and the
deletion never completed.

The Application now carries the two garbage collection finalizers, and
an update that adds or removes one of them applies the same change to
the HelmRelease. Controller finalizers such as finalizers.fluxcd.io stay
owned by the live object, so helm uninstall still runs before the
release goes away. The deletion timestamp and grace period are copied
from the live object, which keeps an update of a terminating release
valid.

Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>

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)
+	}
+}