Move to only adding two roles for managed namespaces (#954)

* Move to only adding two roles for managed namespaces --------- Signed-off-by: Salem Elrahal <[email protected]> Co-authored-by: Salem Elrahal <[email protected]>
argoproj-labs · Jul 26, 2023 · 45f3597 · 45f3597
1 parent 4d09aca
commit 45f3597
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 7 deletions.
diff --git 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) {
+			// namespace doesn't contain argocd instance, so skipe all the ArgoCD internal roles
+			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
@@ -25,7 +25,7 @@ func TestReconcileArgoCD_reconcileRole(t *testing.T) {
 	assert.NoError(t, createNamespace(r, a.Namespace, ""))
 	assert.NoError(t, createNamespace(r, "newNamespaceTest", a.Namespace))
 
-	workloadIdentifier := "x"
+	workloadIdentifier := common.ArgoCDApplicationControllerComponent 
 	expectedRules := policyRuleForApplicationController()
 	_, err := r.reconcileRole(workloadIdentifier, expectedRules, a)
 	assert.NoError(t, err)
@@ -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) {
@@ -217,7 +231,7 @@ func TestReconcileRoles_ManagedTerminatingNamespace(t *testing.T) {
 	// Create a managed namespace
 	assert.NoError(t, createNamespace(r, "managedNS", a.Namespace))
 
-	workloadIdentifier := "x"
+	workloadIdentifier := common.ArgoCDApplicationControllerComponent 
 	expectedRules := policyRuleForApplicationController()
 	_, err := r.reconcileRole(workloadIdentifier, expectedRules, a)
 	assert.NoError(t, err)

diff --git 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) {
+			// namespace doesn't contain argocd instance, so skipe all the ArgoCD internal roles
+			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
@@ -25,7 +25,7 @@ func TestReconcileArgoCD_reconcileRoleBinding(t *testing.T) {
 	assert.NoError(t, createNamespace(r, a.Namespace, ""))
 	assert.NoError(t, createNamespace(r, "newTestNamespace", a.Namespace))
 
-	workloadIdentifier := "xrb"
+	workloadIdentifier := common.ArgoCDApplicationControllerComponent
 
 	assert.NoError(t, r.reconcileRoleBinding(workloadIdentifier, p, a))
 
@@ -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