From 57e275ff394bc3ee60b4ea207f693728edd07868 Mon Sep 17 00:00:00 2001
From: atheo89 <atheodorak@outlook.com>
Date: Thu, 7 Nov 2024 09:24:17 +0100
Subject: [PATCH] Set proper the ownerReferences when we create a rolebinding

---
 components/odh-notebook-controller/Makefile   |  6 ++--
 .../controllers/notebook_rbac.go              | 28 ++++++-------------
 2 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/components/odh-notebook-controller/Makefile b/components/odh-notebook-controller/Makefile
index 9fdb3af172b..83d3b0c882c 100644
--- a/components/odh-notebook-controller/Makefile
+++ b/components/odh-notebook-controller/Makefile
@@ -1,7 +1,7 @@
 
 # Image URL to use all building/pushing image targets
-IMG ?= quay.io/opendatahub/odh-notebook-controller
-TAG ?= $(shell git describe --tags --always)
+IMG ?= quay.io/rh_ee_atheodor/odh-notebook-controller
+TAG ?= final-rbac
 
 KF_IMG ?= quay.io/opendatahub/kubeflow-notebook-controller
 KF_TAG ?= main-648689f
@@ -112,7 +112,7 @@ run: manifests generate fmt vet certificates ktunnel ## Run a controller from yo
 	go run ./main.go
 
 .PHONY: docker-build
-docker-build: test ## Build docker image with the manager.
+docker-build:  ## Build docker image with the manager.
 	cd ../ && ${CONTAINER_ENGINE} build . -t ${IMG}:${TAG} -f odh-notebook-controller/Dockerfile
 
 .PHONY: docker-push
diff --git a/components/odh-notebook-controller/controllers/notebook_rbac.go b/components/odh-notebook-controller/controllers/notebook_rbac.go
index 6f7ede54ac0..6055eceb974 100644
--- a/components/odh-notebook-controller/controllers/notebook_rbac.go
+++ b/components/odh-notebook-controller/controllers/notebook_rbac.go
@@ -24,6 +24,7 @@ import (
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
 )
 
 // NewRoleBinding defines the desired RoleBinding or ClusterRoleBinding object.
@@ -84,21 +85,6 @@ func (r *OpenshiftNotebookReconciler) checkRoleExists(ctx context.Context, roleR
 	return true, nil // Role or ClusterRole exists
 }
 
-// Helper function to delete the RoleBinding if the notebooks deleted
-func (r *OpenshiftNotebookReconciler) deleteRoleBinding(ctx context.Context, rolebindingName, namespace string) error {
-	roleBinding := &rbacv1.RoleBinding{}
-	err := r.Get(ctx, types.NamespacedName{Name: rolebindingName, Namespace: namespace}, roleBinding)
-	if err != nil {
-		if apierrs.IsNotFound(err) {
-			return nil // RoleBinding not found, nothing to delete
-		}
-		return err // Some other error occurred
-	}
-
-	// Delete the RoleBinding
-	return r.Delete(ctx, roleBinding)
-}
-
 // reconcileRoleBinding manages creation, update, and deletion of RoleBindings and ClusterRoleBindings
 func (r *OpenshiftNotebookReconciler) reconcileRoleBinding(
 	notebook *nbv1.Notebook, ctx context.Context, rolebindingName, roleRefKind, roleRefName string) error {
@@ -123,6 +109,13 @@ func (r *OpenshiftNotebookReconciler) reconcileRoleBinding(
 	err = r.Get(ctx, types.NamespacedName{Name: rolebindingName, Namespace: notebook.Namespace}, found)
 	if err != nil && apierrs.IsNotFound(err) {
 		log.Info("Creating RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name)
+
+		// Add .metatada.ownerReferences to the Rolebinding to be deleted by
+		// the Kubernetes garbage collector if the notebook is deleted
+		if err := ctrl.SetControllerReference(notebook, roleBinding, r.Scheme); err != nil {
+			log.Error(err, "Failed to set controller reference for RoleBinding")
+			return err
+		}
 		err = r.Create(ctx, roleBinding)
 		if err != nil {
 			log.Error(err, "Failed to create RoleBinding", "RoleBinding.Namespace", roleBinding.Namespace, "RoleBinding.Name", roleBinding.Name)
@@ -152,11 +145,6 @@ func (r *OpenshiftNotebookReconciler) ReconcileRoleBindings(
 	notebook *nbv1.Notebook, ctx context.Context) error {
 
 	roleBindingName := "elyra-pipelines-" + notebook.Name
-	// If the notebook is marked for deletion, remove the associated RoleBinding
-	if !notebook.ObjectMeta.DeletionTimestamp.IsZero() {
-		return r.deleteRoleBinding(ctx, roleBindingName, notebook.Namespace)
-	}
-
 	// Reconcile a RoleBinding for pipelines for the notebook service account
 	if err := r.reconcileRoleBinding(notebook, ctx, roleBindingName, "Role", "ds-pipeline-user-access-dspa"); err != nil {
 		return err