feat: only delete cluster if all other finalizers on cluster resource are gone (#85)

* only delete cluster if all other finalizers on cluster resource are gone

* move foreign finalizer detection in helper method

* add unit tests for helper function

* refactor: identifyFinalizers to table driven test

* chore: clean-up go.mod

* fix: remove comments

---------

Co-authored-by: Maximilian Techritz <maximilian.techritz@sap.com>

Author: Diaphteiros

internal/controller/cluster_controller.go

@@ -92,13 +92,20 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 }
 
 func (r *ClusterReconciler) handleDelete(ctx context.Context, cluster *clustersv1alpha1.Cluster) (ctrl.Result, error) {
+	log := logf.FromContext(ctx)
 	requeue := smartrequeue.FromContext(ctx)
 	cluster.Status.Phase = commonapi.StatusPhaseTerminating
 
-	if !controllerutil.ContainsFinalizer(cluster, Finalizer) {
+	// check if there are any foreign finalizers on the Cluster resource
+	foreignFinalizers, found := identifyFinalizers(cluster)
+	if !found {
 		// Nothing to do
 		return ctrl.Result{}, nil
 	}
+	if len(foreignFinalizers) > 0 {
+		log.Info("Postponing cluster deletion until foreign finalizers are removed", "foreignFinalizers", foreignFinalizers)
+		return requeue.Progressing()
+	}
 
 	name := kindName(cluster)
 
@@ -245,3 +252,19 @@ func isClusterProviderResponsible(cluster *clustersv1alpha1.Cluster) bool {
 func runsOnLocalHost() bool {
 	return os.Getenv("KIND_ON_LOCAL_HOST") == "true"
 }
+
+// identifyFinalizers checks two things for the given object:
+// 1. If the 'clusters.openmcp.cloud/finalizer' finalizer is present (second return value).
+// 2. Which other finalizers are present (first return value).
+func identifyFinalizers(obj client.Object) ([]string, bool) {
+	foreignFinalizers := make([]string, 0, len(obj.GetFinalizers()))
+	found := false
+	for _, fin := range obj.GetFinalizers() {
+		if fin != Finalizer {
+			foreignFinalizers = append(foreignFinalizers, fin)
+		} else {
+			found = true
+		}
+	}
+	return foreignFinalizers, found
+}

internal/controller/controller_test.go

@@ -0,0 +1,62 @@
+package controller
+
+import (
+	"reflect"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+)
+
+func TestIdentifyFinalizers(t *testing.T) {
+	tests := []struct {
+		name                      string
+		inputFinalizers           []string
+		expectedForeignFinalizers []string
+		expectedOwnFinalizer      bool
+	}{
+		{
+			name:                      "no finalizers on the object at all",
+			inputFinalizers:           []string{},
+			expectedForeignFinalizers: []string{},
+			expectedOwnFinalizer:      false,
+		},
+		{
+			name:                      "only the own finalizer on the object",
+			inputFinalizers:           []string{Finalizer},
+			expectedForeignFinalizers: []string{},
+			expectedOwnFinalizer:      true,
+		},
+		{
+			name:                      "only other finalizers on the object",
+			inputFinalizers:           []string{"other/finalizer1", "other/finalizer2"},
+			expectedForeignFinalizers: []string{"other/finalizer1", "other/finalizer2"},
+			expectedOwnFinalizer:      false,
+		},
+		{
+			name:                      "both own and other finalizers on the object",
+			inputFinalizers:           []string{"other/finalizer1", Finalizer, "other/finalizer2"},
+			expectedForeignFinalizers: []string{"other/finalizer1", "other/finalizer2"},
+			expectedOwnFinalizer:      true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			obj := &corev1.Namespace{}
+
+			for _, finalizer := range tt.inputFinalizers {
+				controllerutil.AddFinalizer(obj, finalizer)
+			}
+
+			foreignFinalizers, ownFinalizer := identifyFinalizers(obj)
+
+			if !reflect.DeepEqual(foreignFinalizers, tt.expectedForeignFinalizers) {
+				t.Errorf("identifyFinalizers() foreignFinalizers = %v, want %v", foreignFinalizers, tt.expectedForeignFinalizers)
+			}
+			if ownFinalizer != tt.expectedOwnFinalizer {
+				t.Errorf("identifyFinalizers() ownFinalizer = %v, want %v", ownFinalizer, tt.expectedOwnFinalizer)
+			}
+		})
+	}
+}