fix: enabled flag of ArgoCD workloads does not remove permissions (#1599

) Signed-off-by: Jayendra Parsai <[email protected]> Co-authored-by: Jayendra Parsai <[email protected]>
argoproj-labs · Dec 3, 2024 · 0ca1920 · 0ca1920
1 parent 9174e68
commit 0ca1920
Show file tree

Hide file tree

Showing 24 changed files with 646 additions and 130 deletions.
diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go
@@ -144,9 +144,9 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule
 			}
 			roles = append(roles, role)
 
-			if name == common.ArgoCDDexServerComponent && !UseDex(cr) {
-
-				continue // Dex installation not requested, do nothing
+			if (name == common.ArgoCDDexServerComponent && !UseDex(cr)) ||
+				!UseApplicationController(name, cr) || !UseRedis(name, cr) || !UseServer(name, cr) {
+				continue // Component installation is not requested, do nothing
 			}
 
 			// Only set ownerReferences for roles in same namespace as ArgoCD CR
@@ -161,6 +161,12 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule
 				return nil, err
 			}
 			continue
+		} else {
+			if !UseApplicationController(name, cr) || !UseRedis(name, cr) || !UseServer(name, cr) {
+				if err := r.Client.Delete(context.TODO(), role); err != nil {
+					return nil, err
+				}
+			}
 		}
 
 		// Delete the existing default role if custom role is specified

diff --git a/controllers/argocd/role_test.go b/controllers/argocd/role_test.go
@@ -1181,3 +1181,102 @@ func enableDefaultClusterRoles(t *testing.T, ctx context.Context, a *argoproj.Ar
 	a.Spec.AggregatedClusterRoles = false
 	assert.NoError(t, cl.Update(ctx, a))
 }
+
+func TestReconcileArgoCD_reconcileRole_enable_controller_role(t *testing.T) {
+	logf.SetLogger(ZapLogger(true))
+	a := makeTestArgoCD()
+
+	resObjs := []client.Object{a}
+	subresObjs := []client.Object{a}
+	runtimeObjs := []runtime.Object{}
+	sch := makeTestReconcilerScheme(argoproj.AddToScheme)
+	cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+	r := makeTestReconciler(cl, sch)
+
+	assert.NoError(t, createNamespace(r, a.Namespace, ""))
+
+	componentName := common.ArgoCDApplicationControllerComponent
+
+	_, err := r.reconcileRole(componentName, []v1.PolicyRule{}, a)
+	assert.NoError(t, err)
+
+	expectedName := fmt.Sprintf("%s-%s", a.Name, componentName)
+	reconciledRole := &v1.Role{}
+	assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, reconciledRole))
+
+	flag := false
+	a.Spec.Controller.Enabled = &flag
+
+	_, err = r.reconcileRole(componentName, []v1.PolicyRule{}, a)
+	assert.NoError(t, err)
+
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, reconciledRole)
+	assert.Error(t, err)
+	assertNotFound(t, err)
+}
+
+func TestReconcileArgoCD_reconcileRole_enable_redis_role(t *testing.T) {
+	logf.SetLogger(ZapLogger(true))
+	a := makeTestArgoCD()
+
+	resObjs := []client.Object{a}
+	subresObjs := []client.Object{a}
+	runtimeObjs := []runtime.Object{}
+	sch := makeTestReconcilerScheme(argoproj.AddToScheme)
+	cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+	r := makeTestReconciler(cl, sch)
+
+	assert.NoError(t, createNamespace(r, a.Namespace, ""))
+
+	componentName := common.ArgoCDRedisComponent
+
+	_, err := r.reconcileRole(componentName, []v1.PolicyRule{}, a)
+	assert.NoError(t, err)
+
+	expectedName := fmt.Sprintf("%s-%s", a.Name, componentName)
+	reconciledRole := &v1.Role{}
+	assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, reconciledRole))
+
+	flag := false
+	a.Spec.Redis.Enabled = &flag
+
+	_, err = r.reconcileRole(componentName, []v1.PolicyRule{}, a)
+	assert.NoError(t, err)
+
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, reconciledRole)
+	assert.Error(t, err)
+	assertNotFound(t, err)
+}
+
+func TestReconcileArgoCD_reconcileRole_enable_server_role(t *testing.T) {
+	logf.SetLogger(ZapLogger(true))
+	a := makeTestArgoCD()
+
+	resObjs := []client.Object{a}
+	subresObjs := []client.Object{a}
+	runtimeObjs := []runtime.Object{}
+	sch := makeTestReconcilerScheme(argoproj.AddToScheme)
+	cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+	r := makeTestReconciler(cl, sch)
+
+	assert.NoError(t, createNamespace(r, a.Namespace, ""))
+
+	componentName := common.ArgoCDServerComponent
+
+	_, err := r.reconcileRole(componentName, []v1.PolicyRule{}, a)
+	assert.NoError(t, err)
+
+	expectedName := fmt.Sprintf("%s-%s", a.Name, componentName)
+	reconciledRole := &v1.Role{}
+	assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, reconciledRole))
+
+	flag := false
+	a.Spec.Server.Enabled = &flag
+
+	_, err = r.reconcileRole(componentName, []v1.PolicyRule{}, a)
+	assert.NoError(t, err)
+
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName, Namespace: a.Namespace}, reconciledRole)
+	assert.Error(t, err)
+	assertNotFound(t, err)
+}
diff --git a/controllers/argocd/rolebinding.go b/controllers/argocd/rolebinding.go
@@ -146,8 +146,9 @@ func (r *ReconcileArgoCD) reconcileRoleBinding(name string, rules []v1.PolicyRul
 				return fmt.Errorf("failed to get the rolebinding associated with %s : %s", name, err)
 			}
 
-			if name == common.ArgoCDDexServerComponent && !UseDex(cr) {
-				continue // Dex installation is not requested, do nothing
+			if (name == common.ArgoCDDexServerComponent && !UseDex(cr)) ||
+				!UseApplicationController(name, cr) || !UseRedis(name, cr) || !UseServer(name, cr) {
+				continue // Component installation is not requested, do nothing
 			}
 
 			roleBindingExists = false
@@ -177,7 +178,7 @@ func (r *ReconcileArgoCD) reconcileRoleBinding(name string, rules []v1.PolicyRul
 		}
 
 		if roleBindingExists {
-			if name == common.ArgoCDDexServerComponent && !UseDex(cr) {
+			if (name == common.ArgoCDDexServerComponent && !UseDex(cr)) || !UseApplicationController(name, cr) || !UseRedis(name, cr) || !UseServer(name, cr) {
 				// Delete any existing RoleBinding created for Dex since dex uninstallation is requested
 				log.Info("deleting the existing Dex roleBinding because dex uninstallation is requested")
 				if err = r.Client.Delete(context.TODO(), existingRoleBinding); err != nil {

diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go
@@ -1664,3 +1664,27 @@ func getApplicationSetHTTPServerHost(cr *argoproj.ArgoCD) (string, error) {
 	}
 	return host, nil
 }
+
+// UseApplicationController determines whether Application Controller resources should be created and configured or not
+func UseApplicationController(name string, cr *argoproj.ArgoCD) bool {
+	if name == common.ArgoCDApplicationControllerComponent && cr.Spec.Controller.Enabled != nil {
+		return *cr.Spec.Controller.Enabled
+	}
+	return true
+}
+
+// UseRedis determines whether Redis resources should be created and configured or not
+func UseRedis(name string, cr *argoproj.ArgoCD) bool {
+	if name == common.ArgoCDRedisComponent && cr.Spec.Redis.Enabled != nil {
+		return *cr.Spec.Redis.Enabled
+	}
+	return true
+}
+
+// UseServer determines whether ArgoCD Server resources should be created and configured or not
+func UseServer(name string, cr *argoproj.ArgoCD) bool {
+	if name == common.ArgoCDServerComponent && cr.Spec.Server.Enabled != nil {
+		return *cr.Spec.Server.Enabled
+	}
+	return true
+}
diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-assert.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-assert.yaml
@@ -16,28 +16,12 @@ spec:
     enabled: false
 status:
   phase: Available
-
 ---
-apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
-metadata:
-  name: argocd-test1-argocd-application-controller
-  namespace: test1
-
----
 apiVersion: rbac.authorization.k8s.io/v1
-kind: RoleBinding
 metadata:
-  name: argocd-test1-argocd-application-controller
-  namespace: test1
-
----
-kind: RoleBinding
-apiVersion: rbac.authorization.k8s.io/v1
-metadata:
-  name: argocd-test1-argocd-server
+  name: argocd-test1-argocd-redis-ha
   namespace: test1
-
 ---
 kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1

diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-errors.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-errors.yaml
@@ -4,19 +4,69 @@ kind: Deployment
 metadata:
   name: argocd-test1-redis
   namespace: test1
-
 ---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: argocd-test1-repo-server
   namespace: test1
-
 ---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: argocd-test1-server
   namespace: test1
-
-
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test1-applicationset-controller
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test1-argocd-application-controller
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test1-argocd-application-controller
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test1-argocd-server
+  namespace: test1
+---  
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: argocd-test1-argocd-server
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test1-redis
+  namespace: test1
+---  
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: argocd-test1-redis
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test1-redis-ha
+  namespace: test1
+---  
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: argocd-test1-redis-ha
+  namespace: test1  
diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-install.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-install.yaml
@@ -3,7 +3,6 @@ kind: Namespace
 metadata:
   name: test1
 ---
-
 apiVersion: argoproj.io/v1beta1
 kind: ArgoCD
 metadata:

diff --git a/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-errors.yaml b/tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-errors.yaml
@@ -4,31 +4,69 @@ kind: Deployment
 metadata:
   name: argocd-test1-redis
   namespace: test1
-
 ---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: argocd-test1-repo-server
   namespace: test1
-
 ---
 apiVersion: apps/v1
 kind: Deployment
 metadata:
   name: argocd-test1-server
   namespace: test1
-
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: argocd-test1-applicationset-controller
+  namespace: test1
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
   name: argocd-test1-server
   namespace: test1
-
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test1-server
+  namespace: test1  
+---  
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test1-argocd-redis
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test1-argocd-redis
+  namespace: test1    
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: Role
 metadata:
   name: argocd-test1-repo-server
   namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test1-repo-server
+  namespace: test1 
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: argocd-test1-applicationset-controller
+  namespace: test1
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: RoleBinding
+metadata:
+  name: argocd-test1-applicationset-controller
+  namespace: test1