Skip to content

Commit

Permalink
Move to only adding two roles for managed namespaces
Browse files Browse the repository at this point in the history
  • Loading branch information
Salem Elrahal committed Jul 26, 2023
1 parent 4d09aca commit 74708c1
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 4 deletions.
4 changes: 2 additions & 2 deletions controllers/argocd/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
14 changes: 14 additions & 0 deletions controllers/argocd/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions controllers/argocd/rolebinding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check failure on line 131 in controllers/argocd/rolebinding.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint on PR

File is not `goimports`-ed with -local github.com/argoproj-labs/argocd-operator (goimports)
if cr.ObjectMeta.Namespace != namespace.Name && (name != common.ArgoCDApplicationControllerComponent && name != common.ArgoCDServerComponent) {
continue
}
}
Expand Down
12 changes: 12 additions & 0 deletions controllers/argocd/rolebinding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 74708c1

Please sign in to comment.