Skip to content

Commit

Permalink
fix: enabled flag of ArgoCD workloads does not remove permissions
Browse files Browse the repository at this point in the history
Signed-off-by: Jayendra Parsai <[email protected]>
  • Loading branch information
Jayendra Parsai committed Nov 8, 2024
1 parent 46ecfa9 commit 9607c55
Show file tree
Hide file tree
Showing 24 changed files with 646 additions and 130 deletions.
12 changes: 9 additions & 3 deletions controllers/argocd/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
99 changes: 99 additions & 0 deletions controllers/argocd/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 4 additions & 3 deletions controllers/argocd/rolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 24 additions & 0 deletions controllers/argocd/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ kind: Namespace
metadata:
name: test1
---

apiVersion: argoproj.io/v1beta1
kind: ArgoCD
metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit 9607c55

Please sign in to comment.