From 74708c1beb9b86601f6e8ee7c0ac2d3580a3c3ca Mon Sep 17 00:00:00 2001
From: Salem Elrahal <selrahal@redhat.com>
Date: Wed, 26 Jul 2023 10:57:36 +0100
Subject: [PATCH] Move to only adding two roles for managed namespaces

---
 controllers/argocd/role.go             |  4 ++--
 controllers/argocd/role_test.go        | 14 ++++++++++++++
 controllers/argocd/rolebinding.go      |  4 ++--
 controllers/argocd/rolebinding_test.go | 12 ++++++++++++
 4 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go
index 3eec12ff3..eb88d5edb 100644
--- a/controllers/argocd/role.go
+++ b/controllers/argocd/role.go
@@ -121,8 +121,8 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule
 		}
 		// only skip creation of dex and redisHa roles for namespaces that no argocd instance is deployed in
 		if len(list.Items) < 1 {
-			// only create dexServer and redisHa roles for the namespace where the argocd instance is deployed
-			if cr.ObjectMeta.Namespace != namespace.Name && (name == common.ArgoCDDexServerComponent || name == common.ArgoCDRedisHAComponent) {
+			// cr.ObjectMeta.Namespace doesn't contain argocd instance, so don't create any role that isn't the application-controller or the server
+			if cr.ObjectMeta.Namespace != namespace.Name && (name != common.ArgoCDApplicationControllerComponent && name != common.ArgoCDServerComponent) {
 				continue
 			}
 		}
diff --git a/controllers/argocd/role_test.go b/controllers/argocd/role_test.go
index eb84550ec..1efc9b64f 100644
--- a/controllers/argocd/role_test.go
+++ b/controllers/argocd/role_test.go
@@ -74,6 +74,20 @@ func TestReconcileArgoCD_reconcileRole_for_new_namespace(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, expectedNumberOfRoles, len(redisHaRoles))
 	assert.Equal(t, expectedRoleNamespace, redisHaRoles[0].ObjectMeta.Namespace)
+	// check no redis role is created for the new namespace with managed-by label
+	workloadIdentifier = common.ArgoCDRedisComponent
+	expectedRedisRules := policyRuleForRedis(r.Client)
+	redisRoles, err := r.reconcileRole(workloadIdentifier, expectedRedisRules, a)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedNumberOfRoles, len(redisRoles))
+	assert.Equal(t, expectedRoleNamespace, redisRoles[0].ObjectMeta.Namespace)
+	// check no grafana role is created for the new namespace with managed-by label
+	workloadIdentifier = common.ArgoCDOperatorGrafanaComponent
+	expectedGrafanaRules := policyRuleForGrafana(r.Client)
+	grafanaRoles, err := r.reconcileRole(workloadIdentifier, expectedGrafanaRules, a)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedNumberOfRoles, len(grafanaRoles))
+	assert.Equal(t, expectedRoleNamespace, grafanaRoles[0].ObjectMeta.Namespace)
 }
 
 func TestReconcileArgoCD_reconcileClusterRole(t *testing.T) {
diff --git a/controllers/argocd/rolebinding.go b/controllers/argocd/rolebinding.go
index 2547b5bfb..229b92ee8 100644
--- a/controllers/argocd/rolebinding.go
+++ b/controllers/argocd/rolebinding.go
@@ -128,8 +128,8 @@ func (r *ReconcileArgoCD) reconcileRoleBinding(name string, rules []v1.PolicyRul
 		}
 		// only skip creation of dex and redisHa rolebindings for namespaces that no argocd instance is deployed in
 		if len(list.Items) < 1 {
-			// only create dexServer and redisHa rolebindings for the namespace where the argocd instance is deployed
-			if cr.ObjectMeta.Namespace != namespace.Name && (name == common.ArgoCDDexServerComponent || name == common.ArgoCDRedisHAComponent) {
+			// cr.ObjectMeta.Namespace doesn't contain argocd instance, so don't create any role that isn't the application-controller or the server 
+			if cr.ObjectMeta.Namespace != namespace.Name && (name != common.ArgoCDApplicationControllerComponent && name != common.ArgoCDServerComponent) {
 				continue
 			}
 		}
diff --git a/controllers/argocd/rolebinding_test.go b/controllers/argocd/rolebinding_test.go
index 89a5d3767..3917b0804 100644
--- a/controllers/argocd/rolebinding_test.go
+++ b/controllers/argocd/rolebinding_test.go
@@ -67,6 +67,18 @@ func TestReconcileArgoCD_reconcileRoleBinding_for_new_namespace(t *testing.T) {
 	expectedRedisHaRules := policyRuleForRedisHa(r.Client)
 	assert.NoError(t, r.reconcileRoleBinding(workloadIdentifier, expectedRedisHaRules, a))
 	assert.Error(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: "newTestNamespace"}, roleBinding))
+
+	// check no redis rolebinding is created for the new namespace with managed-by label
+	workloadIdentifier = common.ArgoCDRedisComponent
+	expectedRedisRules := policyRuleForRedis(r.Client)
+	assert.NoError(t, r.reconcileRoleBinding(workloadIdentifier, expectedRedisRules, a))
+	assert.Error(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: "newTestNamespace"}, roleBinding))
+
+	// check no grafana rolebinding is created for the new namespace with managed-by label
+	workloadIdentifier = common.ArgoCDOperatorGrafanaComponent
+	expectedGrafanaRules := policyRuleForGrafana(r.Client)
+	assert.NoError(t, r.reconcileRoleBinding(workloadIdentifier, expectedGrafanaRules, a))
+	assert.Error(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: "newTestNamespace"}, roleBinding))
 }
 
 // This test validates the behavior of the operator reconciliation when a managed namespace is not properly terminated