From 392066de4064cc127bfd9378293f82ec67331d37 Mon Sep 17 00:00:00 2001
From: Kanika Rana <46766610+ranakan19@users.noreply.github.com>
Date: Thu, 18 Jul 2024 14:24:09 -0400
Subject: [PATCH] SANDBOX-682 add suffix in resource name for role and
 rolebinding (#585)

* add suffix in resource name for role and rolebinding

* update comment
---
 .../useraccount/console_usersettings.go       |  8 ++++
 .../useraccount/console_usersettings_test.go  | 40 +++++++++++++++++++
 .../useraccount/useraccount_controller.go     |  4 +-
 .../useraccount_controller_test.go            | 10 ++++-
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/controllers/useraccount/console_usersettings.go b/controllers/useraccount/console_usersettings.go
index 8ccc8b54..00f53adf 100644
--- a/controllers/useraccount/console_usersettings.go
+++ b/controllers/useraccount/console_usersettings.go
@@ -11,6 +11,8 @@ const (
 	ConsoleUserSettingsUID                = "console.openshift.io/user-settings-uid"
 	UserSettingNS                         = "openshift-console-user-settings"
 	ConsoleUserSettingsResourceNamePrefix = "user-settings-"
+	ConsoleUserSettingsRoleSuffix         = "-role"
+	ConsoleUserSettingsRoleBindingSuffix  = "-rolebinding"
 )
 
 // deleteResource deletes the specified resource associated with a user from console setting.
@@ -21,6 +23,12 @@ const (
 func deleteResource(ctx context.Context, cl client.Client, userUID string, toDelete client.Object) error {
 
 	name := ConsoleUserSettingsResourceNamePrefix + userUID
+	if toDelete.GetObjectKind().GroupVersionKind().Kind == "Role" {
+		name = name + ConsoleUserSettingsRoleSuffix
+	} else if toDelete.GetObjectKind().GroupVersionKind().Kind == "RoleBinding" {
+		name = name + ConsoleUserSettingsRoleBindingSuffix
+	}
+
 	toDelete.SetName(name)
 	toDelete.SetNamespace(UserSettingNS)
 	if err := cl.Delete(ctx, toDelete); err != nil {
diff --git a/controllers/useraccount/console_usersettings_test.go b/controllers/useraccount/console_usersettings_test.go
index f447798f..22fa7afe 100644
--- a/controllers/useraccount/console_usersettings_test.go
+++ b/controllers/useraccount/console_usersettings_test.go
@@ -35,6 +35,46 @@ func TestDeleteConsoleSettingObjects(t *testing.T) {
 		// check that the configmap doesn't exist anymore
 		AssertObjectNotFound(t, cl, UserSettingNS, "user-settings-johnsmith", &corev1.ConfigMap{})
 	})
+	t.Run("Role found by name and deleted", func(t *testing.T) {
+		// given
+		ctx := context.Background()
+		role := &rbac.Role{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "user-settings-johnsmith-role",
+				Namespace: UserSettingNS,
+			},
+		}
+		cl := test.NewFakeClient(t, role)
+
+		// when
+		err := deleteResource(ctx, cl, "johnsmith", &rbac.Role{TypeMeta: metav1.TypeMeta{Kind: "Role"}})
+
+		// then
+		require.NoError(t, err)
+		// check that the role doesn't exist anymore
+		AssertObjectNotFound(t, cl, UserSettingNS, "user-settings-johnsmith-role", &rbac.Role{})
+	})
+
+	t.Run("Rolebinding found by name and deleted", func(t *testing.T) {
+		// given
+		ctx := context.Background()
+		rb := &rbac.RoleBinding{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "user-settings-johnsmith-rolebinding",
+				Namespace: UserSettingNS,
+			},
+		}
+		cl := test.NewFakeClient(t, rb)
+
+		// when
+		err := deleteResource(ctx, cl, "johnsmith", &rbac.RoleBinding{TypeMeta: metav1.TypeMeta{Kind: "RoleBinding"}})
+
+		// then
+		require.NoError(t, err)
+		// check that the rolebinding doesn't exist anymore
+		AssertObjectNotFound(t, cl, UserSettingNS, "user-settings-johnsmith-rolebinding", &rbac.RoleBinding{})
+	})
+
 	t.Run("Object found by label and deletes successfully", func(t *testing.T) {
 		// given
 		cm := &corev1.ConfigMap{
diff --git a/controllers/useraccount/useraccount_controller.go b/controllers/useraccount/useraccount_controller.go
index 6a7fe3a1..ac43932e 100644
--- a/controllers/useraccount/useraccount_controller.go
+++ b/controllers/useraccount/useraccount_controller.go
@@ -507,10 +507,10 @@ func (r *Reconciler) deleteUserResources(ctx context.Context, userUID string) er
 	if err := deleteResource(ctx, r.Client, userUID, &corev1.ConfigMap{}); err != nil {
 		return err
 	}
-	if err := deleteResource(ctx, r.Client, userUID, &rbac.Role{}); err != nil {
+	if err := deleteResource(ctx, r.Client, userUID, &rbac.Role{TypeMeta: metav1.TypeMeta{Kind: "Role"}}); err != nil {
 		return err
 	}
-	return deleteResource(ctx, r.Client, userUID, &rbac.RoleBinding{})
+	return deleteResource(ctx, r.Client, userUID, &rbac.RoleBinding{TypeMeta: metav1.TypeMeta{Kind: "RoleBinding"}})
 }
 
 // deleteIdentity deletes the Identity resources owned by the specified UserAccount.
diff --git a/controllers/useraccount/useraccount_controller_test.go b/controllers/useraccount/useraccount_controller_test.go
index b10e07a7..8b932648 100644
--- a/controllers/useraccount/useraccount_controller_test.go
+++ b/controllers/useraccount/useraccount_controller_test.go
@@ -424,6 +424,10 @@ func TestReconcile(t *testing.T) {
 			},
 		}
 		role := &rbac.Role{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Role",
+				APIVersion: "rbac.authorization.k8s.io/v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      resourceName + "random",
 				Namespace: UserSettingNS,
@@ -434,8 +438,12 @@ func TestReconcile(t *testing.T) {
 			},
 		}
 		rb := &rbac.RoleBinding{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "RoleBinding",
+				APIVersion: "rbac.authorization.k8s.io/v1",
+			},
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      resourceName,
+				Name:      resourceName + ConsoleUserSettingsRoleBindingSuffix,
 				Namespace: UserSettingNS,
 			},
 		}