test: fix flaky impersonation test (#25641)

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

test/e2e/sync_with_impersonate_test.go

@@ -41,10 +41,10 @@ func TestSyncWithNoDestinationServiceAccountsInProject(t *testing.T) {
 	Given(t).
 		Path("guestbook").
 		When().
+		WithImpersonationEnabled("", nil).
 		CreateFromFile(func(app *v1alpha1.Application) {
 			app.Spec.SyncPolicy = &v1alpha1.SyncPolicy{Automated: &v1alpha1.SyncPolicyAutomated{}}
 		}).
-		WithImpersonationEnabled("", nil).
 		Then().
 		// With the impersonation feature enabled, Application sync must fail
 		// when there are no destination service accounts configured in AppProject

util/db/db.go

@@ -3,9 +3,7 @@ package db
 import (
 	"context"
 	"math"
-	"strings"
 
-	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -141,22 +139,6 @@ func NewDB(namespace string, settingsMgr *settings.SettingsManager, kubeclientse
 	}
 }
 
-func (db *db) getSecret(name string, cache map[string]*corev1.Secret) (*corev1.Secret, error) {
-	if _, ok := cache[name]; !ok {
-		secret, err := db.settingsMgr.GetSecretByName(name)
-		if err != nil {
-			return nil, err
-		}
-		cache[name] = secret
-	}
-	return cache[name], nil
-}
-
-// StripCRLFCharacter strips the trailing CRLF characters
-func StripCRLFCharacter(input string) string {
-	return strings.TrimSpace(input)
-}
-
 // GetApplicationControllerReplicas gets the replicas of application controller
 func (db *db) GetApplicationControllerReplicas() int {
 	// get the replicas from application controller deployment, if the application controller deployment does not exist, check for environment variable

util/db/repository_secrets.go

@@ -64,12 +64,8 @@ func (s *secretsRepositoryBackend) CreateRepository(ctx context.Context, reposit
 // the label is found and false otherwise. Will return false if no secret is found with the given
 // name.
 func (s *secretsRepositoryBackend) hasRepoTypeLabel(secretName string) (bool, error) {
-	noCache := make(map[string]*corev1.Secret)
-	sec, err := s.db.getSecret(secretName, noCache)
+	sec, err := s.db.settingsMgr.GetSecretByName(secretName)
 	if err != nil {
-		if apierrors.IsNotFound(err) {
-			return false, nil
-		}
 		return false, err
 	}
 	_, ok := sec.GetLabels()[common.LabelKeySecretType]
@@ -80,7 +76,7 @@ func (s *secretsRepositoryBackend) hasRepoTypeLabel(secretName string) (bool, er
 }
 
 func (s *secretsRepositoryBackend) GetRepoCredsBySecretName(_ context.Context, name string) (*appsv1.RepoCreds, error) {
-	secret, err := s.db.getSecret(name, map[string]*corev1.Secret{})
+	secret, err := s.db.settingsMgr.GetSecretByName(name)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get secret %s: %w", name, err)
 	}
@@ -414,6 +410,8 @@ func secretToRepository(secret *corev1.Secret) (*appsv1.Repository, error) {
 	return repository, nil
 }
 
+// repositoryToSecret updates the given secret with the data from the repository object. It adds the appropriate
+// labels/annotations, but it does not add any name or namespace metadata.
 func (s *secretsRepositoryBackend) repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) *corev1.Secret {
 	secretCopy := secret.DeepCopy()
 

util/db/repository_secrets_test.go

@@ -11,13 +11,9 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/watch"
 	"k8s.io/client-go/kubernetes/fake"
-	k8stesting "k8s.io/client-go/testing"
 
 	"github.com/argoproj/argo-cd/v3/common"
 	appsv1 "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
@@ -39,8 +35,8 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) {
 		InsecureIgnoreHostKey: false,
 		EnableLFS:             true,
 	}
-	setupWithK8sObjects := func(objects ...runtime.Object) *fixture {
-		clientset := getClientset(objects...)
+	setup := func() *fixture {
+		clientset := getClientset()
 		settingsMgr := settings.NewSettingsManager(t.Context(), clientset, testNamespace)
 		repoBackend := &secretsRepositoryBackend{db: &db{
 			ns:            testNamespace,
@@ -55,7 +51,7 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) {
 	t.Run("will create repository successfully", func(t *testing.T) {
 		// given
 		t.Parallel()
-		f := setupWithK8sObjects()
+		f := setup()
 
 		// when
 		output, err := f.repoBackend.CreateRepository(t.Context(), repo)
@@ -85,21 +81,26 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) {
 	t.Run("will return proper error if secret does not have expected label", func(t *testing.T) {
 		// given
 		t.Parallel()
-		secret := &corev1.Secret{}
-		s := secretsRepositoryBackend{}
-		updatedSecret := s.repositoryToSecret(repo, secret)
-		delete(updatedSecret.Labels, common.LabelKeySecretType)
-		f := setupWithK8sObjects(updatedSecret)
-		f.clientSet.ReactionChain = nil
-		f.clientSet.AddReactor("create", "secrets", func(_ k8stesting.Action) (handled bool, ret runtime.Object, err error) {
-			gr := schema.GroupResource{
-				Group:    "v1",
-				Resource: "secrets",
-			}
-			return true, nil, apierrors.NewAlreadyExists(gr, "already exists")
-		})
+		f := setup()
 
 		// when
+		_, err := f.repoBackend.CreateRepository(t.Context(), repo)
+
+		// then
+		require.NoError(t, err)
+
+		// given - remove the label from the secret
+		secret, err := f.clientSet.CoreV1().Secrets(testNamespace).Get(
+			t.Context(),
+			RepoURLToSecretName(repoSecretPrefix, repo.Repo, ""),
+			metav1.GetOptions{},
+		)
+		require.NoError(t, err)
+		delete(secret.Labels, common.LabelKeySecretType)
+		_, err = f.clientSet.CoreV1().Secrets(testNamespace).Update(t.Context(), secret, metav1.UpdateOptions{})
+		require.NoError(t, err)
+
+		// when - try to create the same repository again
 		output, err := f.repoBackend.CreateRepository(t.Context(), repo)
 
 		// then
@@ -107,41 +108,20 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) {
 		assert.Nil(t, output)
 		status, ok := status.FromError(err)
 		assert.True(t, ok)
-		assert.Equal(t, codes.InvalidArgument, status.Code())
+		assert.Equal(t, codes.InvalidArgument, status.Code(), "got unexpected error: %v", err)
 	})
-	t.Run("will return proper error if secret already exists", func(t *testing.T) {
+	t.Run("will return proper error if secret already exists and does have the proper label", func(t *testing.T) {
 		// given
 		t.Parallel()
-		secName := RepoURLToSecretName(repoSecretPrefix, repo.Repo, "")
-		secret := &corev1.Secret{
-			TypeMeta: metav1.TypeMeta{
-				Kind:       "Secret",
-				APIVersion: "v1",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      secName,
-				Namespace: "default",
-			},
-		}
-		s := secretsRepositoryBackend{}
-		updatedSecret := s.repositoryToSecret(repo, secret)
-		f := setupWithK8sObjects(updatedSecret)
-		f.clientSet.ReactionChain = nil
-		f.clientSet.WatchReactionChain = nil
-		f.clientSet.AddReactor("create", "secrets", func(_ k8stesting.Action) (handled bool, ret runtime.Object, err error) {
-			gr := schema.GroupResource{
-				Group:    "v1",
-				Resource: "secrets",
-			}
-			return true, nil, apierrors.NewAlreadyExists(gr, "already exists")
-		})
-		watcher := watch.NewFakeWithChanSize(1, true)
-		watcher.Add(updatedSecret)
-		f.clientSet.AddWatchReactor("secrets", func(_ k8stesting.Action) (handled bool, ret watch.Interface, err error) {
-			return true, watcher, nil
-		})
+		f := setup()
 
 		// when
+		_, err := f.repoBackend.CreateRepository(t.Context(), repo)
+
+		// then
+		require.NoError(t, err)
+
+		// when - try to create the same repository again
 		output, err := f.repoBackend.CreateRepository(t.Context(), repo)
 
 		// then
@@ -149,7 +129,7 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) {
 		assert.Nil(t, output)
 		status, ok := status.FromError(err)
 		assert.True(t, ok)
-		assert.Equal(t, codes.AlreadyExists, status.Code())
+		assert.Equal(t, codes.AlreadyExists, status.Code(), "got unexpected error: %v", err)
 	})
 }