Remove checking instance type in webhook

Finalizaers are added to all instances no matter what the type is. In
UPSI controller added removing the finalizer when deletion timesamp is
not zero

Author: uzabanov

api/repositories/service_instance_repository.go

@@ -481,7 +481,7 @@ func (r *ServiceInstanceRepo) DeleteServiceInstance(ctx context.Context, authInf
 
 	if message.Purge {
 		if err = k8s.PatchResource(ctx, userClient, serviceInstance, func() {
-			controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName)
+			controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFServiceInstanceFinalizerName)
 		}); err != nil {
 			return ServiceInstanceRecord{}, fmt.Errorf("failed to remove finalizer for service instance: %s, %w", message.GUID, apierrors.FromK8sError(err, ServiceInstanceResourceType))
 		}

api/repositories/service_instance_repository_test.go

@@ -1081,7 +1081,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
 		BeforeEach(func() {
 			serviceInstance = createServiceInstanceCR(ctx, k8sClient, prefixedGUID("service-instance"), space.Name, "the-service-instance", prefixedGUID("secret"))
 
-			serviceInstance.Finalizers = append(serviceInstance.Finalizers, korifiv1alpha1.CFManagedServiceInstanceFinalizerName)
+			serviceInstance.Finalizers = append(serviceInstance.Finalizers, korifiv1alpha1.CFServiceInstanceFinalizerName)
 			Expect(k8sClient.Update(ctx, serviceInstance)).To(Succeed())
 
 			serviceBinding = &korifiv1alpha1.CFServiceBinding{

controllers/api/v1alpha1/cfserviceinstance_types.go

@@ -29,7 +29,7 @@ const (
 	UserProvidedType = "user-provided"
 	ManagedType      = "managed"
 
-	CFManagedServiceInstanceFinalizerName = "managed.cfServiceInstance.korifi.cloudfoundry.org"
+	CFServiceInstanceFinalizerName = "cfServiceInstance.korifi.cloudfoundry.org"
 
 	ProvisioningFailedCondition = "ProvisioningFailed"
 )

controllers/controllers/services/instances/managed/controller.go

@@ -329,7 +329,7 @@ func (r *Reconciler) finalizeCFServiceInstance(
 ) (ctrl.Result, error) {
 	log := logr.FromContextOrDiscard(ctx).WithName("finalizeCFServiceInstance")
 
-	if !controllerutil.ContainsFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName) {
+	if !controllerutil.ContainsFinalizer(serviceInstance, korifiv1alpha1.CFServiceInstanceFinalizerName) {
 		return ctrl.Result{}, nil
 	}
 
@@ -338,7 +338,7 @@ func (r *Reconciler) finalizeCFServiceInstance(
 		log.Error(err, "failed to deprovision service instance with broker")
 	}
 
-	controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName)
+	controllerutil.RemoveFinalizer(serviceInstance, korifiv1alpha1.CFServiceInstanceFinalizerName)
 	log.V(1).Info("finalizer removed")
 
 	return ctrl.Result{}, nil

controllers/controllers/services/instances/managed/controller_test.go

@@ -126,7 +126,7 @@ var _ = Describe("CFServiceInstance", func() {
 				Name:      uuid.NewString(),
 				Namespace: namespace.Name,
 				Finalizers: []string{
-					korifiv1alpha1.CFManagedServiceInstanceFinalizerName,
+					korifiv1alpha1.CFServiceInstanceFinalizerName,
 				},
 			},
 			Spec: korifiv1alpha1.CFServiceInstanceSpec{
@@ -797,7 +797,7 @@ var _ = Describe("CFServiceInstance", func() {
 	When("the service instance is purged", func() {
 		BeforeEach(func() {
 			Expect(k8s.PatchResource(ctx, adminClient, instance, func() {
-				controllerutil.RemoveFinalizer(instance, korifiv1alpha1.CFManagedServiceInstanceFinalizerName)
+				controllerutil.RemoveFinalizer(instance, korifiv1alpha1.CFServiceInstanceFinalizerName)
 			})).To(Succeed())
 		})
 

controllers/controllers/services/instances/upsi/controller.go

@@ -37,6 +37,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -110,6 +111,13 @@ func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceInstance *k
 	cfServiceInstance.Status.ObservedGeneration = cfServiceInstance.Generation
 	log.V(1).Info("set observed generation", "generation", cfServiceInstance.Status.ObservedGeneration)
 
+	if !cfServiceInstance.GetDeletionTimestamp().IsZero() {
+		controllerutil.RemoveFinalizer(cfServiceInstance, korifiv1alpha1.CFServiceInstanceFinalizerName)
+		log.V(1).Info("finalizer removed")
+
+		return ctrl.Result{}, nil
+	}
+
 	credentialsSecret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: cfServiceInstance.Namespace,

controllers/controllers/services/instances/upsi/controller_test.go

@@ -12,6 +12,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -36,6 +37,9 @@ var _ = Describe("CFServiceInstance", func() {
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      uuid.NewString(),
 					Namespace: testNamespace,
+					Finalizers: []string{
+						korifiv1alpha1.CFServiceInstanceFinalizerName,
+					},
 				},
 				Spec: korifiv1alpha1.CFServiceInstanceSpec{
 					DisplayName: "service-instance-name",
@@ -209,6 +213,19 @@ var _ = Describe("CFServiceInstance", func() {
 					}).Should(Succeed())
 				})
 			})
+
+			When("the instance is deleted", func() {
+				JustBeforeEach(func() {
+					Expect(adminClient.Delete(ctx, instance)).To(Succeed())
+				})
+
+				It("is deleted", func() {
+					Eventually(func(g Gomega) {
+						err := adminClient.Get(ctx, client.ObjectKeyFromObject(instance), instance)
+						g.Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+					}).Should(Succeed())
+				})
+			})
 		})
 
 		When("the service instance is managed", func() {

controllers/webhooks/finalizer/webhook.go

@@ -7,8 +7,6 @@ import (
 
 	korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1"
 	"code.cloudfoundry.org/korifi/tools/k8s"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
@@ -25,7 +23,7 @@ func NewControllersFinalizerWebhook() *ControllersFinalizerWebhook {
 			"CFPackage":         {FinalizerName: korifiv1alpha1.CFPackageFinalizerName, SetPolicy: k8s.Always},
 			"CFOrg":             {FinalizerName: korifiv1alpha1.CFOrgFinalizerName, SetPolicy: k8s.Always},
 			"CFDomain":          {FinalizerName: korifiv1alpha1.CFDomainFinalizerName, SetPolicy: k8s.Always},
-			"CFServiceInstance": {FinalizerName: korifiv1alpha1.CFManagedServiceInstanceFinalizerName, SetPolicy: isManagedServiceInstance},
+			"CFServiceInstance": {FinalizerName: korifiv1alpha1.CFServiceInstanceFinalizerName, SetPolicy: k8s.Always},
 			"CFServiceBinding":  {FinalizerName: korifiv1alpha1.CFServiceBindingFinalizerName, SetPolicy: k8s.Always},
 		}),
 	}
@@ -41,16 +39,3 @@ func (r *ControllersFinalizerWebhook) SetupWebhookWithManager(mgr ctrl.Manager)
 func (r *ControllersFinalizerWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {
 	return r.delegate.Handle(ctx, req)
 }
-
-func isManagedServiceInstance(object unstructured.Unstructured) bool {
-	l := ctrl.Log.WithName("isManagedServiceInstance")
-	cfServiceInstance := &korifiv1alpha1.CFServiceInstance{}
-	err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, cfServiceInstance)
-	if err != nil {
-		l.Error(err, "failed to convert to CFServiceInstnace from unstructured", "unstructured", object.Object)
-		return true
-	}
-
-	l.Info("CFServiceInstance converted", "cfserviceinstance", cfServiceInstance)
-	return cfServiceInstance.Spec.Type == korifiv1alpha1.ManagedType
-}

controllers/webhooks/finalizer/webhook_test.go

@@ -119,7 +119,7 @@ var _ = Describe("Controllers Finalizers Webhook", func() {
 					Type: korifiv1alpha1.ManagedType,
 				},
 			},
-			korifiv1alpha1.CFManagedServiceInstanceFinalizerName,
+			korifiv1alpha1.CFServiceInstanceFinalizerName,
 		),
 		Entry("user-provided CF service instance",
 			&korifiv1alpha1.CFServiceInstance{
@@ -131,6 +131,7 @@ var _ = Describe("Controllers Finalizers Webhook", func() {
 					Type: korifiv1alpha1.UserProvidedType,
 				},
 			},
+			korifiv1alpha1.CFServiceInstanceFinalizerName,
 		),
 		Entry("cfservicebinding",
 			&korifiv1alpha1.CFServiceBinding{