From 4c3495ca027a4b3b16ba79cb2f5766e260c54607 Mon Sep 17 00:00:00 2001
From: Sunny <darkowlzz@protonmail.com>
Date: Wed, 26 Jul 2023 18:02:04 +0000
Subject: [PATCH] Handle delete before adding finalizer

In Reconcile() method, move the object deletion above add finalizer.
Finalizers can't be set when an object is being deleted.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
---
 .../controller/kustomization_controller.go    | 12 ++--
 .../kustomization_controller_test.go          | 68 +++++++++++++++++++
 2 files changed, 75 insertions(+), 5 deletions(-)
 create mode 100644 internal/controller/kustomization_controller_test.go

diff --git a/internal/controller/kustomization_controller.go b/internal/controller/kustomization_controller.go
index 3def3efc..1248604b 100644
--- a/internal/controller/kustomization_controller.go
+++ b/internal/controller/kustomization_controller.go
@@ -196,18 +196,20 @@ func (r *KustomizationReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 	}()
 
+	// Prune managed resources if the object is under deletion.
+	if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
+		return r.finalize(ctx, obj)
+	}
+
 	// Add finalizer first if it doesn't exist to avoid the race condition
 	// between init and delete.
+	// Note: Finalizers in general can only be added when the deletionTimestamp
+	// is not set.
 	if !controllerutil.ContainsFinalizer(obj, kustomizev1.KustomizationFinalizer) {
 		controllerutil.AddFinalizer(obj, kustomizev1.KustomizationFinalizer)
 		return ctrl.Result{Requeue: true}, nil
 	}
 
-	// Prune managed resources if the object is under deletion.
-	if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
-		return r.finalize(ctx, obj)
-	}
-
 	// Skip reconciliation if the object is suspended.
 	if obj.Spec.Suspend {
 		log.Info("Reconciliation is suspended for this object")
diff --git a/internal/controller/kustomization_controller_test.go b/internal/controller/kustomization_controller_test.go
new file mode 100644
index 00000000..90c03bbd
--- /dev/null
+++ b/internal/controller/kustomization_controller_test.go
@@ -0,0 +1,68 @@
+/*
+Copyright 2023 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/record"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1"
+)
+
+func TestKustomizationReconciler_deleteBeforeFinalizer(t *testing.T) {
+	g := NewWithT(t)
+
+	namespaceName := "kust-" + randStringRunes(5)
+	namespace := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{Name: namespaceName},
+	}
+	g.Expect(k8sClient.Create(ctx, namespace)).ToNot(HaveOccurred())
+	t.Cleanup(func() {
+		g.Expect(k8sClient.Delete(ctx, namespace)).NotTo(HaveOccurred())
+	})
+
+	kustomization := &kustomizev1.Kustomization{}
+	kustomization.Name = "test-kust"
+	kustomization.Namespace = namespaceName
+	kustomization.Spec = kustomizev1.KustomizationSpec{
+		Interval: metav1.Duration{Duration: interval},
+		Prune:    true,
+		SourceRef: kustomizev1.CrossNamespaceSourceReference{
+			Kind: "Bucket",
+			Name: "foo",
+		},
+	}
+	// Add a test finalizer to prevent the object from getting deleted.
+	kustomization.SetFinalizers([]string{"test-finalizer"})
+	g.Expect(k8sClient.Create(ctx, kustomization)).NotTo(HaveOccurred())
+	// Add deletion timestamp by deleting the object.
+	g.Expect(k8sClient.Delete(ctx, kustomization)).NotTo(HaveOccurred())
+
+	r := &KustomizationReconciler{
+		Client:        k8sClient,
+		EventRecorder: record.NewFakeRecorder(32),
+	}
+	// NOTE: Only a real API server responds with an error in this scenario.
+	_, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(kustomization)})
+	g.Expect(err).NotTo(HaveOccurred())
+}