From abb354b5e32e82cf1fc13daa9cb4818f9630a5b8 Mon Sep 17 00:00:00 2001 From: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com> Date: Sun, 14 Dec 2025 07:29:58 -0500 Subject: [PATCH] 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 | 2 +- util/db/db.go | 18 ------ util/db/repository_secrets.go | 10 ++-- util/db/repository_secrets_test.go | 82 ++++++++++---------------- 4 files changed, 36 insertions(+), 76 deletions(-) diff --git a/test/e2e/sync_with_impersonate_test.go b/test/e2e/sync_with_impersonate_test.go index 65919a69dd..b55dda8fa6 100644 --- a/test/e2e/sync_with_impersonate_test.go +++ b/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 diff --git a/util/db/db.go b/util/db/db.go index 9a905c7b3b..479342352f 100644 --- a/util/db/db.go +++ b/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 diff --git a/util/db/repository_secrets.go b/util/db/repository_secrets.go index 37410bc0b5..c0a1b1f189 100644 --- a/util/db/repository_secrets.go +++ b/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() diff --git a/util/db/repository_secrets_test.go b/util/db/repository_secrets_test.go index 9f90038caa..87ab3b638d 100644 --- a/util/db/repository_secrets_test.go +++ b/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) }) }