feat(redis): Fix channel config in users.acl

Signed-off-by: Oliver Gondža <ogondza@gmail.com>

Author: olivergondza

controllers/argocd/deployment_test.go

@@ -2374,16 +2374,6 @@ func repoServerDefaultVolumes() []corev1.Volume {
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					SecretName: "argocd-redis-initial-password",
-					Items: []corev1.KeyToPath{
-						{
-							Key:  "admin.password",
-							Path: "auth",
-						},
-						{
-							Key:  "users.acl",
-							Path: "users.acl",
-						},
-					},
 				},
 			},
 		},
@@ -2487,16 +2477,6 @@ func serverDefaultVolumes() []corev1.Volume {
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					SecretName: "argocd-redis-initial-password",
-					Items: []corev1.KeyToPath{
-						{
-							Key:  "admin.password",
-							Path: "auth",
-						},
-						{
-							Key:  "users.acl",
-							Path: "users.acl",
-						},
-					},
 				},
 			},
 		},

controllers/argocd/secret.go

@@ -746,13 +746,24 @@ func (r *ReconcileArgoCD) getClusterSecrets(cr *argoproj.ArgoCD) (*corev1.Secret
 // reconcileRedisInitialPasswordSecret will ensure that the redis Secret is present for the cluster.
 func (r *ReconcileArgoCD) reconcileRedisInitialPasswordSecret(cr *argoproj.ArgoCD) error {
 	secret := argoutil.NewSecretWithSuffix(cr, "redis-initial-password")
+	existed := false
 
-	secretExists, err := argoutil.IsObjectFound(r.Client, cr.Namespace, secret.Name, secret)
-	if err != nil {
+	// Recreate if the secret or some of its keys are missing
+	err := argoutil.FetchObject(r.Client, cr.Namespace, secret.Name, secret)
+	if err != nil && !apierrors.IsNotFound(err) {
 		return err
 	}
-	if secretExists {
-		return nil // Secret found, do nothing
+	if secret.Data != nil {
+		_, hasPwd := secret.Data[common.ArgoCDKeyAdminPassword]
+		_, hasAuth := secret.Data["auth"]
+		_, hasUsername := secret.Data["auth_username"]
+		_, hasAcl := secret.Data["users.acl"]
+		if hasPwd && hasAuth && hasUsername && hasAcl {
+			return nil // Healthy - keep it
+		}
+		// Drop unsettable fields from FetchObject
+		secret = argoutil.NewSecretWithSuffix(cr, "redis-initial-password")
+		existed = true
 	}
 
 	redisInitialPassword, err := generateRedisAdminPassword()
@@ -761,18 +772,26 @@ func (r *ReconcileArgoCD) reconcileRedisInitialPasswordSecret(cr *argoproj.ArgoC
 	}
 
 	pw := strings.TrimRight(string(redisInitialPassword), "\n")
-	usersACL := fmt.Sprintf(`user default on >%s ~* +@all`, pw)
+	usersACL := fmt.Sprintf("user default on >%s allchannels allkeys allcommands\n", pw)
 
 	secret.Data = map[string][]byte{
-		"immutable":                   []byte("true"),
+		"immutable": []byte("true"),
+		// Mapping the legacy key-name, the operator customers can depend on.
 		common.ArgoCDKeyAdminPassword: redisInitialPassword,
 		// Provide ACL file content so redis-server can use file-based ACLs
-		"users.acl": []byte(usersACL),
+		"auth":          redisInitialPassword,
+		"auth_username": []byte("default"),
+		"users.acl":     []byte(usersACL),
 	}
 
 	if err := controllerutil.SetControllerReference(cr, secret, r.Scheme); err != nil {
 		return err
 	}
+
+	if existed {
+		argoutil.LogResourceUpdate(log, secret)
+		return r.Update(context.TODO(), secret)
+	}
 	argoutil.LogResourceCreation(log, secret)
 	return r.Create(context.TODO(), secret)
 }

controllers/argocd/secret_test.go

@@ -10,10 +10,10 @@ import (
 	"testing"
 
 	argopass "github.com/argoproj/argo-cd/v3/util/password"
-
 	configv1 "github.com/openshift/api/config/v1"
 	routev1 "github.com/openshift/api/route/v1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	corev1 "k8s.io/api/core/v1"
 	testclient "k8s.io/client-go/kubernetes/fake"
@@ -362,6 +362,77 @@ func Test_ReconcileArgoCD_ReconcileShouldNotChangeWhenUpdatedAdminPass(t *testin
 	assert.True(t, argoutil.IsTrackedByOperator(testSecret.Labels))
 }
 
+func Test_ReconcileArgoCD_ReconcileRedisInitialPasswordSecret(t *testing.T) {
+	const suffix = "redis-initial-password"
+	argocd := &argoproj.ArgoCD{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "argocd",
+			Namespace: "argocd-operator",
+		},
+	}
+	secretName := argoutil.NewSecretWithSuffix(argocd, suffix).Name
+	secretNN := types.NamespacedName{Name: secretName, Namespace: "argocd-operator"}
+
+	resObjs := []client.Object{argocd}
+	subresObjs := []client.Object{argocd}
+	runtimeObjs := []runtime.Object{}
+	sch := makeTestReconcilerScheme(argoproj.AddToScheme)
+	cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+	r := makeTestReconciler(cl, sch, testclient.NewSimpleClientset())
+
+	var actual corev1.Secret
+	fetchSecret := func() error {
+		return r.Get(t.Context(), secretNN, &actual)
+	}
+	assertSecretValid := func() string {
+		assert.Equal(t, "true", string(actual.Data["immutable"]))
+		assert.Equal(t, "default", string(actual.Data["auth_username"]))
+		assert.Contains(t, string(actual.Data["users.acl"]), "user default on >")
+		assert.Contains(t, string(actual.Data["users.acl"]), " allchannels allkeys allcommands")
+		actualPwd := string(actual.Data["auth"])
+		assert.NotEqual(t, "", actualPwd)
+		assert.Contains(t, string(actual.Data["users.acl"]), actualPwd, "Password is mentioned in the ACL file")
+		return actualPwd
+	}
+
+	t.Run("Create when does not exist", func(t *testing.T) {
+		require.ErrorContains(t, fetchSecret(), fmt.Sprintf(`secrets "%s" not found`, secretName))
+
+		require.NoError(t, r.reconcileRedisInitialPasswordSecret(argocd))
+
+		require.NoError(t, fetchSecret())
+		assertSecretValid()
+	})
+
+	t.Run("Update keys and regenerate on operator upgrade", func(t *testing.T) {
+		const oldPwd = "asdfghjkl"
+		secret := argoutil.NewSecretWithSuffix(argocd, suffix)
+		secret.Data = map[string][]byte{
+			"immutable":                   []byte("true"),
+			common.ArgoCDKeyAdminPassword: []byte(oldPwd),
+		}
+		require.NoError(t, r.Update(t.Context(), secret))
+
+		require.NoError(t, r.reconcileRedisInitialPasswordSecret(argocd))
+
+		require.NoError(t, fetchSecret())
+		actualPwd := assertSecretValid()
+		assert.NotEqual(t, oldPwd, actualPwd)
+	})
+
+	t.Run("Keep untouched if healthy", func(t *testing.T) {
+		require.NoError(t, fetchSecret())
+		assertSecretValid()
+		oldVersion := actual.ResourceVersion
+
+		require.NoError(t, r.reconcileRedisInitialPasswordSecret(argocd))
+
+		require.NoError(t, fetchSecret())
+		assertSecretValid()
+		assert.Equal(t, oldVersion, actual.ResourceVersion, "Resource version should not change")
+	})
+}
+
 func Test_ReconcileArgoCD_ReconcileRedisTLSSecret(t *testing.T) {
 	argocd := &argoproj.ArgoCD{
 		ObjectMeta: metav1.ObjectMeta{

controllers/argocd/statefulset_test.go

@@ -92,16 +92,6 @@ func controllerDefaultVolumes() []corev1.Volume {
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					SecretName: "argocd-redis-initial-password",
-					Items: []corev1.KeyToPath{
-						{
-							Key:  "admin.password",
-							Path: "auth",
-						},
-						{
-							Key:  "users.acl",
-							Path: "users.acl",
-						},
-					},
 				},
 			},
 		},

controllers/argocdagent/deployment_test.go

@@ -434,11 +434,7 @@ func TestReconcilePrincipalDeployment_VerifyDeploymentSpec(t *testing.T) {
 	assert.NotNil(t, redisAuthVolume.Secret)
 	assert.Equal(t, "argocd-redis-initial-password", redisAuthVolume.Secret.SecretName)
 	assert.NotEqual(t, ptr.To(true), redisAuthVolume.Secret.Optional)
-	assert.Len(t, redisAuthVolume.Secret.Items, 2)
-	assert.Equal(t, "admin.password", redisAuthVolume.VolumeSource.Secret.Items[0].Key)
-	assert.Equal(t, "auth", redisAuthVolume.VolumeSource.Secret.Items[0].Path)
-	assert.Equal(t, "users.acl", redisAuthVolume.VolumeSource.Secret.Items[1].Key)
-	assert.Equal(t, "users.acl", redisAuthVolume.VolumeSource.Secret.Items[1].Path)
+	assert.Len(t, redisAuthVolume.Secret.Items, 0)
 }
 
 func TestReconcilePrincipalDeployment_CustomImage(t *testing.T) {

controllers/argoutil/redis.go

@@ -24,19 +24,6 @@ func MountRedisAuthToArgo(cr *argoproj.ArgoCD) (volume corev1.Volume, mount core
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
 				SecretName: GetSecretNameWithSuffix(cr, "redis-initial-password"),
-				Items: []corev1.KeyToPath{
-					{
-						// Mapping the legacy key-name, the operator customers can depend on, to the expected file name for argo-cd
-						// Ref.: https://argo-cd.readthedocs.io/en/latest/faq/#using-file-based-redis-credentials-via-redis_creds_dir_path
-						Key:  "admin.password",
-						Path: "auth",
-					},
-					{
-						// ACL file used by redis-server
-						Key:  "users.acl",
-						Path: "users.acl",
-					},
-				},
 			},
 		},
 	}

tests/ginkgo/sequential/1-053_validate_argocd_agent_principal_connected_test.go

@@ -370,6 +370,7 @@ var _ = Describe("GitOps Operator Sequential E2E Tests", func() {
 			Eventually(func() bool {
 				for {
 					// drain channel looking for name of new pod
+					GinkgoWriter.Println("Awaiting message")
 					select {
 					case msg := <-msgChan:
 						GinkgoWriter.Println("Processing message:", msg)