Repository: argoproj-labs/argocd-operator
Pull request #847 (2023-02-02 19:08:06)

feat: read dex clientSecret from argocd-secret
Signed-off-by: iam-veeramalla <abhishek.veeramalla@gmail.com>

**What type of PR is this?**
> /kind enhancement

**What does this PR do / why we need it**:
This PR will introduce a behavioral change in the way operator configures Dex for OpenShift connector.

Currently, Operator configures dex openshift connector to the Argo CD Instance when `.spec.dex.OpenShiftOAuth: true` && `.spec.provider: dex`. While this configuration works as expected, the oidc configuration which includes the `clientSecret` is placed in the configmap as plain text. This can be a security risk.

Once this PR is merged, operator will provide the reference of the `clientSecret` in the configmap and not the actual secret. Argo CD Server will read this `clientSecret` reference from `argocd-secret`.

The new configuration will look something like below
`kubectl get cm argocd-cm -oyaml`

```
data:
  .....
  dex.config: |
    connectors:
    - config:
        ....
        clientSecret: $oidc.dex.clientSecret
       .....
       .....
```

**Have you updated the necessary documentation?**
NA

**How to test changes / Special notes to the reviewer**:
1. Run the operator locally using `make install run`
2. Use the below Argo CD CR to create Argo CD Instance with Dex configuration.
```
apiVersion: argoproj.io/v1alpha1
kind: ArgoCD
metadata:
  name: example-argocd
  labels:
    example: route
spec:
  sso:
   provider: dex
   dex:
    openShiftOAuth: true
  server:
    route:
      enabled: true
```
3. Login into the Argo CD UI using the route url.
4. Verify that the login is working fine with the new implementation.
5. Verify in the Argo CD Configmap that `clientSecret` is not comprised but is just a reference to the `oidc.dex.clientSecret` in `argocd-secret`.



Patch:
diff --git a/common/keys.go b/common/keys.go
index 04aa48a6..65f2c1bb 100644
--- a/common/keys.go
+++ b/common/keys.go
@@ -228,4 +228,7 @@ const (
 
 	// ArgoCDServerClusterRoleEnvName is an environment variable to specify a custom cluster role for Argo CD server
 	ArgoCDServerClusterRoleEnvName = "SERVER_CLUSTER_ROLE"
+
+	// ArgoCDDexSecretKey is used to reference Dex secret from Argo CD secret into Argo CD configmap
+	ArgoCDDexSecretKey = "oidc.dex.clientSecret"
 )
diff --git a/controllers/argocd/dex.go b/controllers/argocd/dex.go
index a93a9388..b81b9010 100644
--- a/controllers/argocd/dex.go
+++ b/controllers/argocd/dex.go
@@ -143,10 +143,6 @@ func (r *ReconcileArgoCD) reconcileDexConfiguration(cm *corev1.ConfigMap, cr *ar
 
 // getOpenShiftDexConfig will return the configuration for the Dex server running on OpenShift.
 func (r *ReconcileArgoCD) getOpenShiftDexConfig(cr *argoprojv1a1.ArgoCD) (string, error) {
-	clientSecret, err := r.getDexOAuthClientSecret(cr)
-	if err != nil {
-		return "", err
-	}
 
 	groups := []string{}
 
@@ -164,7 +160,7 @@ func (r *ReconcileArgoCD) getOpenShiftDexConfig(cr *argoprojv1a1.ArgoCD) (string
 		Config: map[string]interface{}{
 			"issuer":       "https://kubernetes.default.svc", // TODO: Should this be hard-coded?
 			"clientID":     getDexOAuthClientID(cr),
-			"clientSecret": *clientSecret,
+			"clientSecret": "$oidc.dex.clientSecret",
 			"redirectURI":  r.getDexOAuthRedirectURI(cr),
 			"insecureCA":   true, // TODO: Configure for openshift CA,
 			"groups":       groups,
diff --git a/controllers/argocd/secret.go b/controllers/argocd/secret.go
index 50c48252..7b577980 100644
--- a/controllers/argocd/secret.go
+++ b/controllers/argocd/secret.go
@@ -29,6 +29,7 @@ import (
 	argopass "github.com/argoproj/argo-cd/v2/util/password"
 	tlsutil "github.com/operator-framework/operator-sdk/pkg/tls"
 
+	"github.com/argoproj-labs/argocd-operator/api/v1alpha1"
 	argoprojv1a1 "github.com/argoproj-labs/argocd-operator/api/v1alpha1"
 	"github.com/argoproj-labs/argocd-operator/common"
 	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
@@ -161,7 +162,7 @@ func (r *ReconcileArgoCD) reconcileArgoSecret(cr *argoprojv1a1.ArgoCD) error {
 	}
 
 	if argoutil.IsObjectFound(r.Client, cr.Namespace, secret.Name, secret) {
-		return r.reconcileExistingArgoSecret(secret, clusterSecret, tlsSecret)
+		return r.reconcileExistingArgoSecret(cr, secret, clusterSecret, tlsSecret)
 	}
 
 	// Secret not found, create it...
@@ -183,6 +184,14 @@ func (r *ReconcileArgoCD) reconcileArgoSecret(cr *argoprojv1a1.ArgoCD) error {
 		common.ArgoCDKeyTLSPrivateKey:      tlsSecret.Data[common.ArgoCDKeyTLSPrivateKey],
 	}
 
+	if cr.Spec.SSO != nil && cr.Spec.SSO.Provider == v1alpha1.SSOProviderTypeDex {
+		dexOIDCClientSecret, err := r.getDexOAuthClientSecret(cr)
+		if err != nil {
+			return nil
+		}
+		secret.Data[common.ArgoCDDexSecretKey] = []byte(*dexOIDCClientSecret)
+	}
+
 	if err := controllerutil.SetControllerReference(cr, secret, r.Scheme); err != nil {
 		return err
 	}
@@ -290,7 +299,7 @@ func (r *ReconcileArgoCD) reconcileClusterSecrets(cr *argoprojv1a1.ArgoCD) error
 }
 
 // reconcileExistingArgoSecret will ensure that the Argo CD Secret is up to date.
-func (r *ReconcileArgoCD) reconcileExistingArgoSecret(secret *corev1.Secret, clusterSecret *corev1.Secret, tlsSecret *corev1.Secret) error {
+func (r *ReconcileArgoCD) reconcileExistingArgoSecret(cr *argoprojv1a1.ArgoCD, secret *corev1.Secret, clusterSecret *corev1.Secret, tlsSecret *corev1.Secret) error {
 	changed := false
 
 	if secret.Data == nil {
@@ -325,6 +334,17 @@ func (r *ReconcileArgoCD) reconcileExistingArgoSecret(secret *corev1.Secret, clu
 		changed = true
 	}
 
+	if cr.Spec.SSO != nil && cr.Spec.SSO.Provider == v1alpha1.SSOProviderTypeDex {
+		if secret.Data[common.ArgoCDDexSecretKey] == nil {
+			dexOIDCClientSecret, err := r.getDexOAuthClientSecret(cr)
+			if err != nil {
+				return nil
+			}
+			secret.Data[common.ArgoCDDexSecretKey] = []byte(*dexOIDCClientSecret)
+			changed = true
+		}
+	}
+
 	if changed {
 		log.Info("updating argo secret")
 		if err := r.Client.Update(context.TODO(), secret); err != nil {
diff --git a/examples/argocd-openshift-dex.yaml b/examples/argocd-openshift-dex.yaml
new file mode 100644
index 00000000..2f012b3d
--- /dev/null
+++ b/examples/argocd-openshift-dex.yaml
@@ -0,0 +1,22 @@
+apiVersion: argoproj.io/v1alpha1
+kind: ArgoCD
+metadata:
+  name: example-argocd
+  labels:
+    example: route
+spec:
+  sso:
+   provider: dex
+   dex:
+    openShiftOAuth: true
+  grafana:
+    enabled: true
+    route:
+      enabled: true
+  prometheus:
+    enabled: true
+    route:
+      enabled: true
+  server:
+    route:
+      enabled: true


Test patch:
diff --git a/controllers/argocd/secret_test.go b/controllers/argocd/secret_test.go
index 6f46d351..f9527ac6 100644
--- a/controllers/argocd/secret_test.go
+++ b/controllers/argocd/secret_test.go
@@ -222,6 +222,7 @@ func Test_ReconcileArgoCD_ReconcileExistingArgoSecret(t *testing.T) {
 	r := makeTestReconciler(t, argocd)
 	r.Client.Create(context.TODO(), clusterSecret)
 	r.Client.Create(context.TODO(), tlsSecret)
+
 	err := r.reconcileArgoSecret(argocd)
 
 	assert.NoError(t, err)
@@ -234,7 +235,7 @@ func Test_ReconcileArgoCD_ReconcileExistingArgoSecret(t *testing.T) {
 	testSecret.Data = nil
 	r.Client.Update(context.TODO(), testSecret)
 
-	_ = r.reconcileExistingArgoSecret(testSecret, clusterSecret, tlsSecret)
+	_ = r.reconcileExistingArgoSecret(argocd, testSecret, clusterSecret, tlsSecret)
 	_ = r.Client.Get(context.TODO(), types.NamespacedName{Name: "argocd-secret", Namespace: "argocd-operator"}, testSecret)
 
 	if testSecret.Data == nil {