Skip to content

Commit

Permalink
feat: Enable use of alternate cluster roles for cluster scoped instances
Browse files Browse the repository at this point in the history
Signed-off-by: Jayendra Parsai <[email protected]>
  • Loading branch information
Jayendra Parsai committed May 16, 2024
1 parent b300521 commit 484e6ad
Show file tree
Hide file tree
Showing 11 changed files with 289 additions and 99 deletions.
6 changes: 6 additions & 0 deletions common/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ const (
// ArgoCDServerClusterRoleEnvName is an environment variable to specify a custom cluster role for Argo CD server
ArgoCDServerClusterRoleEnvName = "SERVER_CLUSTER_ROLE"

// ArgoCDControllerClusterScopeRoleEnvName is an environment variable to specify a custom controller cluster role to replace the default cluster role used when Argo CD is deployed in cluster scoped mode.
ArgoCDControllerClusterScopeRoleEnvName = "CONTROLLER_CLUSTER_SCOPE_ROLE"

// ArgoCDServerClusterScopeRoleEnvName is an environment variable to specify a custom server cluster role to replace the default cluster role used when Argo CD is deployed in cluster scoped mode.
ArgoCDServerClusterScopeRoleEnvName = "SERVER_CLUSTER_SCOPE_ROLE"

// ArgoCDDexSecretKey is used to reference Dex secret from Argo CD secret into Argo CD configmap
ArgoCDDexSecretKey = "oidc.dex.clientSecret"

Expand Down
2 changes: 1 addition & 1 deletion controllers/argocd/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func TestReconcileArgoCD_reconcileTLSCerts_configMapUpdate(t *testing.T) {
}

// update a new cert in argocd-tls-certs-cm
testPEM := generateEncodedPEM(t, "example.com")
testPEM := generateEncodedPEM(t)

configMap.Data["example.com"] = string(testPEM)
assert.NoError(t, r.Client.Update(context.TODO(), configMap))
Expand Down
44 changes: 22 additions & 22 deletions controllers/argocd/policyrule.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,46 +317,46 @@ func policyRuleForServerClusterRole() []v1.PolicyRule {
}

func getPolicyRuleList(client client.Client) []struct {
name string
policyRule []v1.PolicyRule
componentName string
policyRule []v1.PolicyRule
} {
return []struct {
name string
policyRule []v1.PolicyRule
componentName string
policyRule []v1.PolicyRule
}{
{
name: common.ArgoCDApplicationControllerComponent,
policyRule: policyRuleForApplicationController(),
componentName: common.ArgoCDApplicationControllerComponent,
policyRule: policyRuleForApplicationController(),
}, {
name: common.ArgoCDDexServerComponent,
policyRule: policyRuleForDexServer(),
componentName: common.ArgoCDDexServerComponent,
policyRule: policyRuleForDexServer(),
}, {
name: common.ArgoCDServerComponent,
policyRule: policyRuleForServer(),
componentName: common.ArgoCDServerComponent,
policyRule: policyRuleForServer(),
}, {
name: common.ArgoCDRedisHAComponent,
policyRule: policyRuleForRedisHa(client),
componentName: common.ArgoCDRedisHAComponent,
policyRule: policyRuleForRedisHa(client),
}, {
name: common.ArgoCDRedisComponent,
policyRule: policyRuleForRedis(client),
componentName: common.ArgoCDRedisComponent,
policyRule: policyRuleForRedis(client),
},
}
}

func getPolicyRuleClusterRoleList() []struct {
name string
policyRule []v1.PolicyRule
componentName string
policyRule []v1.PolicyRule
} {
return []struct {
name string
policyRule []v1.PolicyRule
componentName string
policyRule []v1.PolicyRule
}{
{
name: common.ArgoCDApplicationControllerComponent,
policyRule: policyRuleForApplicationController(),
componentName: common.ArgoCDApplicationControllerComponent,
policyRule: policyRuleForApplicationController(),
}, {
name: common.ArgoCDServerComponent,
policyRule: policyRuleForServerClusterRole(),
componentName: common.ArgoCDServerComponent,
policyRule: policyRuleForServerClusterRole(),
},
}
}
Expand Down
79 changes: 54 additions & 25 deletions controllers/argocd/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,15 @@ func newClusterRole(name string, rules []v1.PolicyRule, cr *argoproj.ArgoCD) *v1
// reconcileRoles will ensure that all ArgoCD Service Accounts are configured.
func (r *ReconcileArgoCD) reconcileRoles(cr *argoproj.ArgoCD) error {
params := getPolicyRuleList(r.Client)

for _, param := range params {
if _, err := r.reconcileRole(param.name, param.policyRule, cr); err != nil {
if _, err := r.reconcileRole(param.componentName, param.policyRule, cr); err != nil {
return err
}
}

clusterParams := getPolicyRuleClusterRoleList()

for _, clusterParam := range clusterParams {
if _, err := r.reconcileClusterRole(clusterParam.name, clusterParam.policyRule, cr); err != nil {
if _, err := r.reconcileClusterRole(clusterParam.componentName, clusterParam.policyRule, cr); err != nil {
return err
}
}
Expand All @@ -98,7 +96,7 @@ func (r *ReconcileArgoCD) reconcileRoles(cr *argoproj.ArgoCD) error {

// reconcileRole, reconciles the policy rules for different ArgoCD components, for each namespace
// Managed by a single instance of ArgoCD.
func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule, cr *argoproj.ArgoCD) ([]*v1.Role, error) {
func (r *ReconcileArgoCD) reconcileRole(componentName string, policyRules []v1.PolicyRule, cr *argoproj.ArgoCD) ([]*v1.Role, error) {
var roles []*v1.Role

// create policy rules for each namespace
Expand All @@ -121,13 +119,13 @@ 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 {
// 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) {
// namespace doesn't contain argocd instance, so skip all the ArgoCD internal roles
if cr.ObjectMeta.Namespace != namespace.Name && (componentName != common.ArgoCDApplicationControllerComponent && componentName != common.ArgoCDServerComponent) {
continue
}
}
customRole := getCustomRoleName(name)
role := newRole(name, policyRules, cr)
customRole := getCustomRoleName(componentName, cr)
role := newRole(componentName, policyRules, cr)
if err := applyReconcilerHook(cr, role, ""); err != nil {
return nil, err
}
Expand All @@ -136,14 +134,14 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: role.Name, Namespace: role.Namespace}, &existingRole)
if err != nil {
if !errors.IsNotFound(err) {
return nil, fmt.Errorf("failed to reconcile the role for the service account associated with %s : %s", name, err)
return nil, fmt.Errorf("failed to reconcile the role for the service account associated with %s : %s", componentName, err)
}
if customRole != "" {
continue // skip creating default role if custom cluster role is provided
}
roles = append(roles, role)

if name == common.ArgoCDDexServerComponent && !UseDex(cr) {
if componentName == common.ArgoCDDexServerComponent && !UseDex(cr) {

continue // Dex installation not requested, do nothing
}
Expand All @@ -165,7 +163,7 @@ func (r *ReconcileArgoCD) reconcileRole(name string, policyRules []v1.PolicyRule
// Delete the existing default role if custom role is specified
// or if there is an existing Role created for Dex but dex is disabled or not configured
if customRole != "" ||
(name == common.ArgoCDDexServerComponent && !UseDex(cr)) {
(componentName == common.ArgoCDDexServerComponent && !UseDex(cr)) {

log.Info("deleting the existing Dex role because dex is not configured")
if err := r.Client.Delete(context.TODO(), &existingRole); err != nil {
Expand Down Expand Up @@ -279,41 +277,72 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin
return nil
}

func (r *ReconcileArgoCD) reconcileClusterRole(name string, policyRules []v1.PolicyRule, cr *argoproj.ArgoCD) (*v1.ClusterRole, error) {
func (r *ReconcileArgoCD) reconcileClusterRole(componentName string, policyRules []v1.PolicyRule, cr *argoproj.ArgoCD) (*v1.ClusterRole, error) {
allowed := false
if allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) {
allowed = true
}
clusterRole := newClusterRole(name, policyRules, cr)
clusterRole := newClusterRole(componentName, policyRules, cr)
if err := applyReconcilerHook(cr, clusterRole, ""); err != nil {
return nil, err
}

// fetch the name of custom cluster role provided for the component
customClusterRoleName := getCustomClusterRoleName(componentName, cr)
customClusterRole := &v1.ClusterRole{}
if customClusterRoleName != "" {
err := r.Client.Get(context.TODO(), types.NamespacedName{Name: customClusterRoleName}, customClusterRole)
if err != nil {
return nil, fmt.Errorf("failed to find the specified cluster role %s for the service account associated with %s : %s", customClusterRoleName, componentName, err)
}
}

existingClusterRoleExists := false
existingClusterRole := &v1.ClusterRole{}
err := r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRole.Name}, existingClusterRole)
if err != nil {
if !errors.IsNotFound(err) {
return nil, fmt.Errorf("failed to reconcile the cluster role for the service account associated with %s : %s", name, err)
return nil, fmt.Errorf("failed to reconcile the cluster role for the service account associated with %s : %s", componentName, err)
}
if !allowed {
// Do Nothing
return nil, nil
}
return clusterRole, r.Client.Create(context.TODO(), clusterRole)
}

if !allowed {
return nil, r.Client.Delete(context.TODO(), existingClusterRole)
// custom cluster role is not given, create default one
if customClusterRoleName == "" {
return clusterRole, r.Client.Create(context.TODO(), clusterRole)
}
} else {
existingClusterRoleExists = true
// Role exists but there is no custom role, check if allowed and update rules if needed
if !allowed {
return nil, r.Client.Delete(context.TODO(), existingClusterRole)
}

// if the Rules differ, update the Role
if !reflect.DeepEqual(existingClusterRole.Rules, clusterRole.Rules) {
existingClusterRole.Rules = clusterRole.Rules
if err := r.Client.Update(context.TODO(), existingClusterRole); err != nil {
return nil, err
}
}
}

// if the Rules differ, update the Role
if !reflect.DeepEqual(existingClusterRole.Rules, clusterRole.Rules) {
existingClusterRole.Rules = clusterRole.Rules
if err := r.Client.Update(context.TODO(), existingClusterRole); err != nil {
return nil, err
// Return either custom or existing role
if customClusterRoleName != "" {
log.Info("Checking to see if we need to delete existing cluster role")
if existingClusterRoleExists {
log.Info("Deleting existing cluster role")
err = r.Client.Delete(context.TODO(), existingClusterRole)
if err != nil {
log.Error(err, fmt.Sprintf("failed to delete default ClusterRole [%s]", existingClusterRole.Name))
}
}
return customClusterRole, nil
} else {
return existingClusterRole, nil
}
return existingClusterRole, nil
}

func deleteClusterRoles(c client.Client, clusterRoleList *v1.ClusterRoleList) error {
Expand Down
57 changes: 56 additions & 1 deletion controllers/argocd/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,61 @@ func TestReconcileArgoCD_reconcileClusterRole(t *testing.T) {
assert.Contains(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole).Error(), "not found")
}

func TestReconcileArgoCD_reconcileClusterRole_custom_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)

argoComponentName := common.ArgoCDApplicationControllerComponent
clusterRoleName := GenerateUniqueResourceName(argoComponentName, a)
expectedRules := policyRuleForApplicationController()

// Reconcile cluster role
_, err := r.reconcileClusterRole(argoComponentName, expectedRules, a)
assert.NoError(t, err)

// Set the namespace to be cluster scoped
t.Setenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES", a.Namespace)

// Reconcile cluster role
_, err = r.reconcileClusterRole(argoComponentName, expectedRules, a)
assert.NoError(t, err)

// Confirm default cluster role is created, since custom cluster role name is not given
reconciledClusterRole := &v1.ClusterRole{}
assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole))
assert.Equal(t, expectedRules, reconciledClusterRole.Rules)

// Set the custom cluster role name, since it doesn't exist error should be raised
a.Spec.Controller.Env = append(a.Spec.Controller.Env, corev1.EnvVar{Name: common.ArgoCDControllerClusterScopeRoleEnvName, Value: "custom-cluster-scope-role"})
_, err = r.reconcileClusterRole(argoComponentName, expectedRules, a)
assert.Error(t, err)
assert.Contains(t, err.Error(), "not found")

// Create custom-cluster-scope-role
customClusterRole := &v1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Name: "custom-cluster-scope-role",
},
}
assert.NoError(t, r.Client.Create(context.TODO(), customClusterRole))

// Reconcile cluster role, custer cluster role name is set and it also exists, default role created earlier should be deleted
_, err = r.reconcileClusterRole(argoComponentName, expectedRules, a)
assert.NoError(t, err)

// Confirm default cluster role has been deleted
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole)
assert.Error(t, err)
assert.Contains(t, err.Error(), "not found")
}

func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing.T) {
logf.SetLogger(ZapLogger(true))
sourceNamespace := "newNamespaceTest"
Expand Down Expand Up @@ -271,7 +326,7 @@ func TestReconcileArgoCD_reconcileRole_custom_role(t *testing.T) {
assert.Equal(t, expectedRules, reconciledRole.Rules)

// set the custom role as env variable
t.Setenv(common.ArgoCDControllerClusterRoleEnvName, "custom-role")
a.Spec.Controller.Env = append(a.Spec.Controller.Env, corev1.EnvVar{Name: common.ArgoCDControllerClusterRoleEnvName, Value: "custom-role"})

_, err = r.reconcileRole(workloadIdentifier, expectedRules, a)
assert.NoError(t, err)
Expand Down
Loading

0 comments on commit 484e6ad

Please sign in to comment.