RHOAIENG-2974: Restrict DSCI deletion before DSC (opendatahub-io#1094)

* RHOAIENG-2974: Restrict DSCI deletion before DSC

* Added tests and minor enhancements

* Admission allow DSC

* lint fix

(cherry picked from commit 2439104)

Add DELETE operation for webhook in CSV, upstream

60a44c2 ("update: cleanup remove kfdef during uninstallation (opendatahub-io#1100)")

Amend client -> Client due to already applied

03c1abc ("upgrade: controller-runtime and code change accordingly (opendatahub-io#1189)")

bundle/manifests/rhods-operator.clusterserviceversion.yaml

@@ -1808,6 +1808,7 @@ spec:
       operations:
       - CREATE
       - UPDATE
+      - DELETE
       resources:
       - datascienceclusters
       - dscinitializations

config/webhook/manifests.yaml

@@ -22,6 +22,7 @@ webhooks:
     operations:
     - CREATE
     - UPDATE
+    - DELETE
     resources:
     - datascienceclusters
     - dscinitializations

controllers/webhook/webhook.go

@@ -28,11 +28,13 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 )
 
 var log = ctrl.Log.WithName("rhoai-controller-webhook")
 
-//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;update,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1
+//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;update;delete,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1
 //nolint:lll
 
 type OpenDataHubWebhook struct {
@@ -91,12 +93,24 @@ func (w *OpenDataHubWebhook) checkDupCreation(ctx context.Context, req admission
 		fmt.Sprintf("Only one instance of %s object is allowed", req.Kind.Kind))
 }
 
+func (w *OpenDataHubWebhook) checkDeletion(ctx context.Context, req admission.Request) admission.Response {
+	if req.Kind.Kind == "DataScienceCluster" {
+		return admission.Allowed("")
+	}
+
+	// Restrict deletion of DSCI if DSC exists
+	return denyCountGtZero(ctx, w.Client, gvk.DataScienceCluster,
+		fmt.Sprintln("Cannot delete DSCI object when DSC object still exists"))
+}
+
 func (w *OpenDataHubWebhook) Handle(ctx context.Context, req admission.Request) admission.Response {
 	var resp admission.Response
 
 	switch req.Operation {
 	case admissionv1.Create:
 		resp = w.checkDupCreation(ctx, req)
+	case admissionv1.Delete:
+		resp = w.checkDeletion(ctx, req)
 	default:
 		msg := fmt.Sprintf("No logic check by webhook is applied on %v request", req.Operation)
 		log.Info(msg)

controllers/webhook/webhook_suite_test.go

@@ -171,16 +171,41 @@ var _ = Describe("DSC/DSCI webhook", func() {
 		Expect(k8sClient.Create(ctx, desiredDsci)).Should(Succeed())
 		desiredDsci2 := newDSCI(nameBase + "-dsci-2")
 		Expect(k8sClient.Create(ctx, desiredDsci2)).ShouldNot(Succeed())
+		Expect(clearInstance(ctx, desiredDsci)).Should(Succeed())
 	})
 
 	It("Should block creation of second DSC instance", func(ctx context.Context) {
-		dscSpec := newDSC(nameBase+"-dsc-1", namespace)
-		Expect(k8sClient.Create(ctx, dscSpec)).Should(Succeed())
-		dscSpec = newDSC(nameBase+"-dsc-2", namespace)
-		Expect(k8sClient.Create(ctx, dscSpec)).ShouldNot(Succeed())
+		dscSpec1 := newDSC(nameBase+"-dsc-1", namespace)
+		Expect(k8sClient.Create(ctx, dscSpec1)).Should(Succeed())
+		dscSpec2 := newDSC(nameBase+"-dsc-2", namespace)
+		Expect(k8sClient.Create(ctx, dscSpec2)).ShouldNot(Succeed())
+		Expect(clearInstance(ctx, dscSpec1)).Should(Succeed())
+	})
+
+	It("Should block deletion of DSCI instance when DSC instance exist", func(ctx context.Context) {
+		dscInstance := newDSC(nameBase+"-dsc-1", "webhook-test-namespace")
+		Expect(k8sClient.Create(ctx, dscInstance)).Should(Succeed())
+		dsciInstance := newDSCI(nameBase + "-dsci-1")
+		Expect(k8sClient.Create(ctx, dsciInstance)).Should(Succeed())
+		Expect(k8sClient.Delete(ctx, dsciInstance)).ShouldNot(Succeed())
+		Expect(clearInstance(ctx, dscInstance)).Should(Succeed())
+		Expect(clearInstance(ctx, dsciInstance)).Should(Succeed())
+	})
+
+	It("Should allow deletion of DSCI instance when DSC instance does not exist", func(ctx context.Context) {
+		dscInstance := newDSC(nameBase+"-dsc-1", "webhook-test-namespace")
+		Expect(k8sClient.Create(ctx, dscInstance)).Should(Succeed())
+		dsciInstance := newDSCI(nameBase + "-dsci-1")
+		Expect(k8sClient.Create(ctx, dsciInstance)).Should(Succeed())
+		Expect(k8sClient.Delete(ctx, dscInstance)).Should(Succeed())
+		Expect(k8sClient.Delete(ctx, dsciInstance)).Should(Succeed())
 	})
 })
 
+func clearInstance(ctx context.Context, instance client.Object) error {
+	return k8sClient.Delete(ctx, instance)
+}
+
 func newDSCI(appName string) *dsciv1.DSCInitialization {
 	monitoringNS := "monitoring-namespace"
 	return &dsciv1.DSCInitialization{