Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions controllers/argocd/role.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
}
18 changes: 1 addition & 17 deletions tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-assert.yaml
View file
Open in desktop
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
58 changes: 54 additions & 4 deletions tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-errors.yaml
View file
Open in desktop
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
1 change: 0 additions & 1 deletion tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/01-install.yaml
View file
Open in desktop
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
46 changes: 42 additions & 4 deletions tests/k8s/1-034_validate_applicationset_reconcile_enabled_set_false/02-errors.yaml
View file
Open in desktop
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