feat: Add support for custom ClusterRoles (controller,server) for clu…

…ster scoped instances (#1357) * feat: Enable use of alternate cluster roles for cluster scoped instances Signed-off-by: Jayendra Parsai <[email protected]> * Update role/binding unit tests, add a new line to the doc Signed-off-by: Jonathan West <[email protected]> --------- Signed-off-by: Jayendra Parsai <[email protected]> Signed-off-by: Jonathan West <[email protected]> Co-authored-by: Jayendra Parsai <[email protected]> Co-authored-by: Jonathan West <[email protected]>
argoproj-labs · May 24, 2024 · 4047a69 · 4047a69
1 parent 5f3f5b5
commit 4047a69
Show file tree

Hide file tree

Showing 14 changed files with 212 additions and 8 deletions.
diff --git a/api/v1alpha1/argocd_conversion.go b/api/v1alpha1/argocd_conversion.go
@@ -93,6 +93,7 @@ func (src *ArgoCD) ConvertTo(dstRaw conversion.Hub) error {
 	dst.Spec.UsersAnonymousEnabled = src.Spec.UsersAnonymousEnabled
 	dst.Spec.Version = src.Spec.Version
 	dst.Spec.Banner = (*v1beta1.Banner)(src.Spec.Banner)
+	dst.Spec.DefaultClusterScopedRoleDisabled = src.Spec.DefaultClusterScopedRoleDisabled
 
 	// Status conversion
 	dst.Status = v1beta1.ArgoCDStatus(src.Status)
@@ -160,6 +161,7 @@ func (dst *ArgoCD) ConvertFrom(srcRaw conversion.Hub) error {
 	dst.Spec.UsersAnonymousEnabled = src.Spec.UsersAnonymousEnabled
 	dst.Spec.Version = src.Spec.Version
 	dst.Spec.Banner = (*Banner)(src.Spec.Banner)
+	dst.Spec.DefaultClusterScopedRoleDisabled = src.Spec.DefaultClusterScopedRoleDisabled
 
 	// Status conversion
 	dst.Status = ArgoCDStatus(src.Status)

diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go
@@ -824,6 +824,9 @@ type ArgoCDSpec struct {
 	// Deprecated field. Support dropped in v1beta1 version.
 	// Dex defines the Dex server options for ArgoCD.
 	Dex *ArgoCDDexSpec `json:"dex,omitempty"`
+
+	// DefaultClusterScopedRoleDisabled will disable creation of default ClusterRoles for a cluster scoped instance.
+	DefaultClusterScopedRoleDisabled bool `json:"defaultClusterScopedRoleDisabled,omitempty"`
 }
 
 // ArgoCDStatus defines the observed state of ArgoCD

diff --git a/api/v1beta1/argocd_types.go b/api/v1beta1/argocd_types.go
@@ -855,6 +855,9 @@ type ArgoCDSpec struct {
 
 	// Banner defines an additional banner to be displayed in Argo CD UI
 	Banner *Banner `json:"banner,omitempty"`
+
+	// DefaultClusterScopedRoleDisabled will disable creation of default ClusterRoles for a cluster scoped instance.
+	DefaultClusterScopedRoleDisabled bool `json:"defaultClusterScopedRoleDisabled,omitempty"`
 }
 
 // ArgoCDStatus defines the observed state of ArgoCD

diff --git a/bundle/manifests/argoproj.io_argocds.yaml b/bundle/manifests/argoproj.io_argocds.yaml
@@ -630,6 +630,10 @@ spec:
                         type: integer
                     type: object
                 type: object
+              defaultClusterScopedRoleDisabled:
+                description: DefaultClusterScopedRoleDisabled will disable creation
+                  of default ClusterRoles for a cluster scoped instance.
+                type: boolean
               dex:
                 description: Deprecated field. Support dropped in v1beta1 version.
                   Dex defines the Dex server options for ArgoCD.
@@ -7593,6 +7597,10 @@ spec:
                         type: integer
                     type: object
                 type: object
+              defaultClusterScopedRoleDisabled:
+                description: DefaultClusterScopedRoleDisabled will disable creation
+                  of default ClusterRoles for a cluster scoped instance.
+                type: boolean
               disableAdmin:
                 description: DisableAdmin will disable the admin user.
                 type: boolean

diff --git a/config/crd/bases/argoproj.io_argocds.yaml b/config/crd/bases/argoproj.io_argocds.yaml
@@ -621,6 +621,10 @@ spec:
                         type: integer
                     type: object
                 type: object
+              defaultClusterScopedRoleDisabled:
+                description: DefaultClusterScopedRoleDisabled will disable creation
+                  of default ClusterRoles for a cluster scoped instance.
+                type: boolean
               dex:
                 description: Deprecated field. Support dropped in v1beta1 version.
                   Dex defines the Dex server options for ArgoCD.
@@ -7584,6 +7588,10 @@ spec:
                         type: integer
                     type: object
                 type: object
+              defaultClusterScopedRoleDisabled:
+                description: DefaultClusterScopedRoleDisabled will disable creation
+                  of default ClusterRoles for a cluster scoped instance.
+                type: boolean
               disableAdmin:
                 description: DisableAdmin will disable the admin user.
                 type: boolean

diff --git a/controllers/argocd/configmap_test.go b/controllers/argocd/configmap_test.go
@@ -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))

diff --git a/controllers/argocd/role.go b/controllers/argocd/role.go
@@ -280,10 +280,29 @@ func (r *ReconcileArgoCD) reconcileRoleForApplicationSourceNamespaces(name strin
 }
 
 func (r *ReconcileArgoCD) reconcileClusterRole(name string, policyRules []v1.PolicyRule, cr *argoproj.ArgoCD) (*v1.ClusterRole, error) {
+
 	allowed := false
 	if allowedNamespace(cr.Namespace, os.Getenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES")) {
 		allowed = true
 	}
+
+	// Check if it is cluster-scoped instance namespace and user doesn't want to use default ClusterRole
+	if allowed && cr.Spec.DefaultClusterScopedRoleDisabled {
+
+		// In case DefaultClusterScopedRoleDisabled was false earlier and default ClusterRole was created, then delete it.
+		existingClusterRole := &v1.ClusterRole{}
+		if err := r.Client.Get(context.TODO(), types.NamespacedName{Name: GenerateUniqueResourceName(name, cr)}, existingClusterRole); err == nil {
+
+			// Default ClusterRole exists, now delete it
+			if err := r.Client.Delete(context.TODO(), existingClusterRole); err != nil {
+				return nil, fmt.Errorf("failed to delete existing cluster role for the service account associated with %s : %s", name, err)
+			}
+		}
+
+		// Don't create a default ClusterRole
+		return nil, nil
+	}
+
 	clusterRole := newClusterRole(name, policyRules, cr)
 	if err := applyReconcilerHook(cr, clusterRole, ""); err != nil {
 		return nil, err

diff --git a/controllers/argocd/role_test.go b/controllers/argocd/role_test.go
@@ -151,6 +151,67 @@ 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_disabled(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)
+
+	workloadIdentifier := common.ArgoCDApplicationControllerComponent
+	clusterRoleName := GenerateUniqueResourceName(workloadIdentifier, a)
+	expectedRules := policyRuleForApplicationController()
+
+	// Set the namespace to be cluster-scoped
+	t.Setenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES", a.Namespace)
+
+	// Disable creation of default ClusterRole
+	a.Spec.DefaultClusterScopedRoleDisabled = true
+
+	err := cl.Update(context.Background(), a)
+	assert.NoError(t, err)
+
+	// Reconcile ClusterRole
+	_, err = r.reconcileClusterRole(workloadIdentifier, expectedRules, a)
+	assert.NoError(t, err)
+
+	// Ensure default ClusterRole is not created
+	reconciledClusterRole := &v1.ClusterRole{}
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole)
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "not found")
+
+	// Now enable creation of default ClusterRole
+	a.Spec.DefaultClusterScopedRoleDisabled = false
+	err = cl.Update(context.Background(), a)
+	assert.NoError(t, err)
+
+	// Again reconcile ClusterRole
+	_, err = r.reconcileClusterRole(workloadIdentifier, expectedRules, a)
+	assert.NoError(t, err)
+
+	// Ensure default ClusterRole is created now
+	assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole))
+
+	// Once again disable creation of default ClusterRole
+	a.Spec.DefaultClusterScopedRoleDisabled = true
+	err = cl.Update(context.Background(), a)
+	assert.NoError(t, err)
+
+	// Once again reconcile ClusterRole
+	_, err = r.reconcileClusterRole(workloadIdentifier, expectedRules, a)
+	assert.NoError(t, err)
+
+	// Ensure default ClusterRole is deleted again
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole)
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "not found")
+}
+
 func TestReconcileArgoCD_reconcileRoleForApplicationSourceNamespaces(t *testing.T) {
 	logf.SetLogger(ZapLogger(true))
 	sourceNamespace := "newNamespaceTest"

diff --git a/controllers/argocd/rolebinding.go b/controllers/argocd/rolebinding.go
@@ -340,6 +340,23 @@ func newRoleBindingWithNameForApplicationSourceNamespaces(namespace string, cr *
 
 func (r *ReconcileArgoCD) reconcileClusterRoleBinding(name string, role *v1.ClusterRole, cr *argoproj.ArgoCD) error {
 
+	// Check if user doesn't want to use default ClusterRole, hence default ClusterRoleBinding is also not required
+	if cr.Spec.DefaultClusterScopedRoleDisabled {
+
+		// In case DefaultClusterScopedRoleDisabled was false earlier and default ClusterRoleBinding was created, then delete it.
+		existingClusterRoleBinding := &v1.ClusterRoleBinding{}
+		if err := r.Client.Get(context.TODO(), types.NamespacedName{Name: GenerateUniqueResourceName(name, cr)}, existingClusterRoleBinding); err == nil {
+
+			// Default ClusterRoleBinding exists, now delete it
+			if err := r.Client.Delete(context.TODO(), existingClusterRoleBinding); err != nil {
+				return fmt.Errorf("failed to delete existing cluster role binding for the service account associated with %s : %s", name, err)
+			}
+		}
+
+		// Don't create a default ClusterRoleBinding
+		return nil
+	}
+
 	// get expected name
 	roleBinding := newClusterRoleBindingWithname(name, cr)
 	// fetch existing rolebinding by name

diff --git a/controllers/argocd/rolebinding_test.go b/controllers/argocd/rolebinding_test.go
@@ -184,6 +184,58 @@ func TestReconcileArgoCD_reconcileClusterRoleBinding(t *testing.T) {
 	assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName}, clusterRoleBinding))
 }
 
+func TestReconcileArgoCD_reconcileClusterRoleBinding_disabled(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)
+	workloadIdentifier := "x"
+	expectedClusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: workloadIdentifier}}
+
+	// Disable creation of default ClusterRole, hence RoleBinding won't be created either.
+	a.Spec.DefaultClusterScopedRoleDisabled = true
+	err := cl.Update(context.Background(), a)
+	assert.NoError(t, err)
+
+	// Reconcile ClusterRoleBinding
+	assert.NoError(t, r.reconcileClusterRoleBinding(workloadIdentifier, expectedClusterRole, a))
+
+	// Ensure default ClusterRoleBinding is not created
+	clusterRoleBinding := &rbacv1.ClusterRoleBinding{}
+	expectedName := fmt.Sprintf("%s-%s-%s", a.Name, a.Namespace, workloadIdentifier)
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName}, clusterRoleBinding)
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "not found")
+
+	// Now enable creation of default ClusterRole, hence RoleBinding should be created aw well.
+	a.Spec.DefaultClusterScopedRoleDisabled = false
+	err = cl.Update(context.Background(), a)
+	assert.NoError(t, err)
+
+	// Again reconcile ClusterRoleBinding
+	assert.NoError(t, r.reconcileClusterRoleBinding(workloadIdentifier, expectedClusterRole, a))
+
+	// Ensure default ClusterRoleBinding is created now
+	assert.NoError(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName}, clusterRoleBinding))
+
+	// Once again disable creation of default ClusterRole
+	a.Spec.DefaultClusterScopedRoleDisabled = true
+	err = cl.Update(context.Background(), a)
+	assert.NoError(t, err)
+
+	// Once again reconcile ClusterRoleBinding
+	assert.NoError(t, r.reconcileClusterRoleBinding(workloadIdentifier, expectedClusterRole, a))
+
+	// Ensure default ClusterRoleBinding is deleted again
+	err = r.Client.Get(context.TODO(), types.NamespacedName{Name: expectedName}, clusterRoleBinding)
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "not found")
+}
+
 func TestReconcileArgoCD_reconcileRoleBinding_custom_role(t *testing.T) {
 	logf.SetLogger(ZapLogger(true))
 	a := makeTestArgoCD()

diff --git a/controllers/argocd/statefulset.go b/controllers/argocd/statefulset.go
@@ -33,7 +33,7 @@ import (
 	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
 )
 
-func getRedisHAReplicas(cr *argoproj.ArgoCD) *int32 {
+func getRedisHAReplicas() *int32 {
 	replicas := common.ArgoCDDefaultRedisHAReplicas
 	// TODO: Allow override of this value through CR?
 	return &replicas
@@ -107,7 +107,7 @@ func (r *ReconcileArgoCD) reconcileRedisStatefulSet(cr *argoproj.ArgoCD) error {
 	})
 
 	ss.Spec.PodManagementPolicy = appsv1.OrderedReadyPodManagement
-	ss.Spec.Replicas = getRedisHAReplicas(cr)
+	ss.Spec.Replicas = getRedisHAReplicas()
 	ss.Spec.Selector = &metav1.LabelSelector{
 		MatchLabels: map[string]string{
 			common.ArgoCDKeyName: nameWithSuffix("redis-ha", cr),

diff --git a/controllers/argocd/util_test.go b/controllers/argocd/util_test.go
@@ -549,8 +549,8 @@ func TestGetArgoApplicationControllerCommand(t *testing.T) {
 func TestGetArgoApplicationContainerEnv(t *testing.T) {
 
 	sync60s := []v1.EnvVar{
-		v1.EnvVar{Name: "HOME", Value: "/home/argocd", ValueFrom: (*v1.EnvVarSource)(nil)},
-		v1.EnvVar{Name: "REDIS_PASSWORD", Value: "",
+		{Name: "HOME", Value: "/home/argocd", ValueFrom: (*v1.EnvVarSource)(nil)},
+		{Name: "REDIS_PASSWORD", Value: "",
 			ValueFrom: &corev1.EnvVarSource{
 				SecretKeyRef: &corev1.SecretKeySelector{
 					LocalObjectReference: corev1.LocalObjectReference{
@@ -559,7 +559,7 @@ func TestGetArgoApplicationContainerEnv(t *testing.T) {
 					Key: "admin.password",
 				},
 			}},
-		v1.EnvVar{Name: "ARGOCD_RECONCILIATION_TIMEOUT", Value: "60s", ValueFrom: (*v1.EnvVarSource)(nil)}}
+		{Name: "ARGOCD_RECONCILIATION_TIMEOUT", Value: "60s", ValueFrom: (*v1.EnvVarSource)(nil)}}
 
 	cmdTests := []struct {
 		name string
@@ -994,7 +994,7 @@ func TestGenerateRandomString(t *testing.T) {
 	assert.Len(t, b, 20)
 }
 
-func generateEncodedPEM(t *testing.T, host string) []byte {
+func generateEncodedPEM(t *testing.T) []byte {
 	key, err := argoutil.NewPrivateKey()
 	assert.NoError(t, err)
 

diff --git a/deploy/olm-catalog/argocd-operator/0.9.0/argoproj.io_argocds.yaml b/deploy/olm-catalog/argocd-operator/0.9.0/argoproj.io_argocds.yaml
@@ -630,6 +630,10 @@ spec:
                         type: integer
                     type: object
                 type: object
+              defaultClusterScopedRoleDisabled:
+                description: DefaultClusterScopedRoleDisabled will disable creation
+                  of default ClusterRoles for a cluster scoped instance.
+                type: boolean
               dex:
                 description: Deprecated field. Support dropped in v1beta1 version.
                   Dex defines the Dex server options for ArgoCD.
@@ -7593,6 +7597,10 @@ spec:
                         type: integer
                     type: object
                 type: object
+              defaultClusterScopedRoleDisabled:
+                description: DefaultClusterScopedRoleDisabled will disable creation
+                  of default ClusterRoles for a cluster scoped instance.
+                type: boolean
               disableAdmin:
                 description: DisableAdmin will disable the admin user.
                 type: boolean

diff --git a/docs/usage/custom_roles.md b/docs/usage/custom_roles.md
@@ -37,4 +37,27 @@ spec:
             value: custom-controller-role
           - name: SERVER_CLUSTER_ROLE
             value: custom-server-role
-```
+```
+
+## Cluster Scoped Roles
+
+When the administrative user deploys Argo CD as a cluster scoped instance, the operator creates additional ClusterRoles and ClusterRoleBindings for the
+application-controller and server components. These provide the additional permissions that Argo CD requires to operate at the cluster level.
+
+Specifying alternate ClusterRoles enables the administrative user to add or remove permissions
+as needed and have them applied across all cluster scoped instances. For example, features such as the [Auto Respect RBAC For Controller](https://argo-cd.readthedocs.io/en/stable/operator-manual/declarative-setup/#auto-respect-rbac-for-controller) enables specifying more granular permissions for the application-controller service account.
+
+These customized ClusterRoles need to be created and referred in ClusterRoleBinding by admin. A user can disable creation of default ClusterRoles by setting `ArgoCD.Spec.DefaultClusterScopedRoleDisabled` field to `true`.
+
+```yaml
+apiVersion: argoproj.io/v1alpha1
+kind: ArgoCD
+metadata:
+  name: example-argocd
+  labels:
+    example: basic
+spec:
+  defaultClusterScopedRoleDisabled: true
+```
+
+When `defaultClusterScopedRoleDisabled` is `true`, the default ClusterRole/ClusterRoleBindings for the Argo CD instance will not be created, and the administrative user is free to create and customize these independent of the operator. The field can later be set to `false`, to recreate these resources, if needed.