feat: project-scoped repository credential improvements (argoproj#18388)

* feat: project-scoped repo cred improvements Implementation of argoproj#18290 Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * fix: missed a test Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * wip project key changes Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * test: update mocks Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * test: fix tests Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * fix: equivalence even if project is empty Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * fix: wip delete Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * refactor: remove repositorydb Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * chore: improve logging Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * fix: pass project to getrepository Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * test: fix failing test Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * fix: compare with project secret instead of app secret Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * fix: get repository needs same logic as delete Need to update the spec accordingly. Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * feat: add project flag to repo rm command Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * docs: make codegen Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * test: fix failing test Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * test: more failing tests Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * chore: minor cleanups Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * chore: propagate project from ui Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * test: add new test cases Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * chore: code review, improve formulation Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> * refactor: address cr feedback Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> --------- Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com> Co-authored-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Signed-off-by: Javier Solana <javier.solana@cabify.com> Signed-off-by: Javier Solana <javier.solana@cabify.com>
jsolana · Jul 24, 2024 · 40d3d4b · 40d3d4b
1 parent 7e87a2a
commit 40d3d4b
Show file tree

Hide file tree

Showing 41 changed files with 992 additions and 500 deletions.
diff --git a/applicationset/services/mocks/RepositoryDB.go b/applicationset/services/mocks/RepositoryDB.go
diff --git a/applicationset/services/repo_service.go b/applicationset/services/repo_service.go
@@ -6,21 +6,12 @@ import (
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
-	"github.com/argoproj/argo-cd/v2/util/db"
 	"github.com/argoproj/argo-cd/v2/util/git"
 	"github.com/argoproj/argo-cd/v2/util/io"
 )
 
-//go:generate go run github.com/vektra/mockery/v2@v2.40.2 --name=RepositoryDB
-
-// RepositoryDB Is a lean facade for ArgoDB,
-// Using a lean interface makes it easier to test the functionality of the git generator
-type RepositoryDB interface {
-	GetRepository(ctx context.Context, url string) (*v1alpha1.Repository, error)
-}
-
 type argoCDService struct {
-	repositoriesDB         RepositoryDB
+	getRepository          func(ctx context.Context, url, project string) (*v1alpha1.Repository, error)
 	storecreds             git.CredsStore
 	submoduleEnabled       bool
 	repoServerClientSet    apiclient.Clientset
@@ -38,17 +29,17 @@ type Repos interface {
 	GetDirectories(ctx context.Context, repoURL string, revision string, noRevisionCache bool) ([]string, error)
 }
 
-func NewArgoCDService(db db.ArgoDB, submoduleEnabled bool, repoClientset apiclient.Clientset, newFileGlobbingEnabled bool) (Repos, error) {
+func NewArgoCDService(getRepository func(ctx context.Context, url, project string) (*v1alpha1.Repository, error), submoduleEnabled bool, repoClientset apiclient.Clientset, newFileGlobbingEnabled bool) (Repos, error) {
 	return &argoCDService{
-		repositoriesDB:         db.(RepositoryDB),
+		getRepository:          getRepository,
 		submoduleEnabled:       submoduleEnabled,
 		repoServerClientSet:    repoClientset,
 		newFileGlobbingEnabled: newFileGlobbingEnabled,
 	}, nil
 }
 
 func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision string, pattern string, noRevisionCache bool) (map[string][]byte, error) {
-	repo, err := a.repositoriesDB.GetRepository(ctx, repoURL)
+	repo, err := a.getRepository(ctx, repoURL, "")
 	if err != nil {
 		return nil, fmt.Errorf("error in GetRepository: %w", err)
 	}
@@ -75,7 +66,7 @@ func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision s
 }
 
 func (a *argoCDService) GetDirectories(ctx context.Context, repoURL string, revision string, noRevisionCache bool) ([]string, error) {
-	repo, err := a.repositoriesDB.GetRepository(ctx, repoURL)
+	repo, err := a.getRepository(ctx, repoURL, "")
 	if err != nil {
 		return nil, fmt.Errorf("error in GetRepository: %w", err)
 	}

diff --git a/applicationset/services/repo_service_test.go b/applicationset/services/repo_service_test.go
@@ -5,10 +5,8 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/argoproj/argo-cd/v2/applicationset/services/mocks"
 	"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
 	repo_mocks "github.com/argoproj/argo-cd/v2/reposerver/apiclient/mocks"
-	db_mocks "github.com/argoproj/argo-cd/v2/util/db/mocks"
 	"github.com/argoproj/argo-cd/v2/util/git"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -19,9 +17,9 @@ import (
 func TestGetDirectories(t *testing.T) {
 
 	type fields struct {
-		repositoriesDBFuncs   []func(*mocks.RepositoryDB)
 		storecreds            git.CredsStore
 		submoduleEnabled      bool
+		getRepository         func(ctx context.Context, url, project string) (*v1alpha1.Repository, error)
 		repoServerClientFuncs []func(*repo_mocks.RepoServerServiceClient)
 	}
 	type args struct {
@@ -38,17 +36,13 @@ func TestGetDirectories(t *testing.T) {
 		wantErr assert.ErrorAssertionFunc
 	}{
 		{name: "ErrorGettingRepos", fields: fields{
-			repositoriesDBFuncs: []func(*mocks.RepositoryDB){
-				func(db *mocks.RepositoryDB) {
-					db.On("GetRepository", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("unable to get repos"))
-				},
+			getRepository: func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+				return nil, fmt.Errorf("unable to get repos")
 			},
 		}, args: args{}, want: nil, wantErr: assert.Error},
 		{name: "ErrorGettingDirs", fields: fields{
-			repositoriesDBFuncs: []func(*mocks.RepositoryDB){
-				func(db *mocks.RepositoryDB) {
-					db.On("GetRepository", mock.Anything, mock.Anything).Return(&v1alpha1.Repository{}, nil)
-				},
+			getRepository: func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+				return &v1alpha1.Repository{}, nil
 			},
 			repoServerClientFuncs: []func(*repo_mocks.RepoServerServiceClient){
 				func(client *repo_mocks.RepoServerServiceClient) {
@@ -57,10 +51,8 @@ func TestGetDirectories(t *testing.T) {
 			},
 		}, args: args{}, want: nil, wantErr: assert.Error},
 		{name: "HappyCase", fields: fields{
-			repositoriesDBFuncs: []func(*mocks.RepositoryDB){
-				func(db *mocks.RepositoryDB) {
-					db.On("GetRepository", mock.Anything, mock.Anything).Return(&v1alpha1.Repository{}, nil)
-				},
+			getRepository: func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+				return &v1alpha1.Repository{}, nil
 			},
 			repoServerClientFuncs: []func(*repo_mocks.RepoServerServiceClient){
 				func(client *repo_mocks.RepoServerServiceClient) {
@@ -73,18 +65,14 @@ func TestGetDirectories(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			mockDb := &mocks.RepositoryDB{}
 			mockRepoClient := &repo_mocks.RepoServerServiceClient{}
 			// decorate the mocks
-			for i := range tt.fields.repositoriesDBFuncs {
-				tt.fields.repositoriesDBFuncs[i](mockDb)
-			}
 			for i := range tt.fields.repoServerClientFuncs {
 				tt.fields.repoServerClientFuncs[i](mockRepoClient)
 			}
 
 			a := &argoCDService{
-				repositoriesDB:      mockDb,
+				getRepository:       tt.fields.getRepository,
 				storecreds:          tt.fields.storecreds,
 				submoduleEnabled:    tt.fields.submoduleEnabled,
 				repoServerClientSet: &repo_mocks.Clientset{RepoServerServiceClient: mockRepoClient},
@@ -100,10 +88,10 @@ func TestGetDirectories(t *testing.T) {
 
 func TestGetFiles(t *testing.T) {
 	type fields struct {
-		repositoriesDBFuncs   []func(*mocks.RepositoryDB)
 		storecreds            git.CredsStore
 		submoduleEnabled      bool
 		repoServerClientFuncs []func(*repo_mocks.RepoServerServiceClient)
+		getRepository         func(ctx context.Context, url, project string) (*v1alpha1.Repository, error)
 	}
 	type args struct {
 		ctx             context.Context
@@ -120,17 +108,13 @@ func TestGetFiles(t *testing.T) {
 		wantErr assert.ErrorAssertionFunc
 	}{
 		{name: "ErrorGettingRepos", fields: fields{
-			repositoriesDBFuncs: []func(*mocks.RepositoryDB){
-				func(db *mocks.RepositoryDB) {
-					db.On("GetRepository", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("unable to get repos"))
-				},
+			getRepository: func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+				return nil, fmt.Errorf("unable to get repos")
 			},
 		}, args: args{}, want: nil, wantErr: assert.Error},
 		{name: "ErrorGettingFiles", fields: fields{
-			repositoriesDBFuncs: []func(*mocks.RepositoryDB){
-				func(db *mocks.RepositoryDB) {
-					db.On("GetRepository", mock.Anything, mock.Anything).Return(&v1alpha1.Repository{}, nil)
-				},
+			getRepository: func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+				return &v1alpha1.Repository{}, nil
 			},
 			repoServerClientFuncs: []func(*repo_mocks.RepoServerServiceClient){
 				func(client *repo_mocks.RepoServerServiceClient) {
@@ -139,10 +123,8 @@ func TestGetFiles(t *testing.T) {
 			},
 		}, args: args{}, want: nil, wantErr: assert.Error},
 		{name: "HappyCase", fields: fields{
-			repositoriesDBFuncs: []func(*mocks.RepositoryDB){
-				func(db *mocks.RepositoryDB) {
-					db.On("GetRepository", mock.Anything, mock.Anything).Return(&v1alpha1.Repository{}, nil)
-				},
+			getRepository: func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+				return &v1alpha1.Repository{}, nil
 			},
 			repoServerClientFuncs: []func(*repo_mocks.RepoServerServiceClient){
 				func(client *repo_mocks.RepoServerServiceClient) {
@@ -161,18 +143,14 @@ func TestGetFiles(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			mockDb := &mocks.RepositoryDB{}
 			mockRepoClient := &repo_mocks.RepoServerServiceClient{}
 			// decorate the mocks
-			for i := range tt.fields.repositoriesDBFuncs {
-				tt.fields.repositoriesDBFuncs[i](mockDb)
-			}
 			for i := range tt.fields.repoServerClientFuncs {
 				tt.fields.repoServerClientFuncs[i](mockRepoClient)
 			}
 
 			a := &argoCDService{
-				repositoriesDB:      mockDb,
+				getRepository:       tt.fields.getRepository,
 				storecreds:          tt.fields.storecreds,
 				submoduleEnabled:    tt.fields.submoduleEnabled,
 				repoServerClientSet: &repo_mocks.Clientset{RepoServerServiceClient: mockRepoClient},
@@ -187,7 +165,9 @@ func TestGetFiles(t *testing.T) {
 }
 
 func TestNewArgoCDService(t *testing.T) {
-	service, err := NewArgoCDService(&db_mocks.ArgoDB{}, false, &repo_mocks.Clientset{}, false)
+	service, err := NewArgoCDService(func(ctx context.Context, url, project string) (*v1alpha1.Repository, error) {
+		return &v1alpha1.Repository{}, nil
+	}, false, &repo_mocks.Clientset{}, false)
 	assert.NoError(t, err, err)
 	assert.NotNil(t, service)
 }
diff --git a/assets/swagger.json b/assets/swagger.json
@@ -3171,7 +3171,7 @@
         "parameters": [
           {
             "type": "string",
-            "description": "URL is the URL that this credentials matches to",
+            "description": "URL is the URL to which these credentials match",
             "name": "creds.url",
             "in": "path",
             "required": true
@@ -3251,6 +3251,12 @@
             "description": "Whether to force a cache refresh on repo's connection state.",
             "name": "forceRefresh",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "App project for query.",
+            "name": "appProject",
+            "in": "query"
           }
         ],
         "responses": {
@@ -3373,6 +3379,12 @@
             "description": "Whether to force a cache refresh on repo's connection state.",
             "name": "forceRefresh",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "App project for query.",
+            "name": "appProject",
+            "in": "query"
           }
         ],
         "responses": {
@@ -3409,6 +3421,12 @@
             "description": "Whether to force a cache refresh on repo's connection state.",
             "name": "forceRefresh",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "App project for query.",
+            "name": "appProject",
+            "in": "query"
           }
         ],
         "responses": {
@@ -3493,6 +3511,12 @@
             "description": "Whether to force a cache refresh on repo's connection state.",
             "name": "forceRefresh",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "App project for query.",
+            "name": "appProject",
+            "in": "query"
           }
         ],
         "responses": {
@@ -3530,6 +3554,12 @@
             "description": "Whether to force a cache refresh on repo's connection state.",
             "name": "forceRefresh",
             "in": "query"
+          },
+          {
+            "type": "string",
+            "description": "App project for query.",
+            "name": "appProject",
+            "in": "query"
           }
         ],
         "responses": {
@@ -8065,7 +8095,7 @@
         },
         "url": {
           "type": "string",
-          "title": "URL is the URL that this credentials matches to"
+          "title": "URL is the URL to which these credentials match"
         },
         "username": {
           "type": "string",
@@ -8151,7 +8181,7 @@
         },
         "project": {
           "type": "string",
-          "title": "Reference between project and repository that allow you automatically to be added as item inside SourceRepos project entity"
+          "title": "Reference between project and repository that allows it to be automatically added as an item inside SourceRepos project entity"
         },
         "proxy": {
           "type": "string",

diff --git a/cmd/argocd-applicationset-controller/commands/applicationset_controller.go b/cmd/argocd-applicationset-controller/commands/applicationset_controller.go
@@ -105,7 +105,7 @@ func NewCommand() *cobra.Command {
 				os.Exit(1)
 			}
 
-			// By default watch all namespace
+			// By default, watch all namespaces
 			var watchedNamespace string = ""
 
 			// If the applicationset-namespaces contains only one namespace it corresponds to the current namespace
@@ -172,7 +172,7 @@ func NewCommand() *cobra.Command {
 			}
 
 			repoClientset := apiclient.NewRepoServerClientset(argocdRepoServer, repoServerTimeoutSeconds, tlsConfig)
-			argoCDService, err := services.NewArgoCDService(argoCDDB, gitSubmoduleEnabled, repoClientset, enableNewGitFileGlobbing)
+			argoCDService, err := services.NewArgoCDService(argoCDDB.GetRepository, gitSubmoduleEnabled, repoClientset, enableNewGitFileGlobbing)
 			errors.CheckError(err)
 
 			terminalGenerators := map[string]generators.Generator{

diff --git a/cmd/argocd/commands/admin/repo.go b/cmd/argocd/commands/admin/repo.go
@@ -157,7 +157,7 @@ func NewGenRepoSpecCommand() *cobra.Command {
 			_, err := argoDB.CreateRepository(ctx, &repoOpts.Repo)
 			errors.CheckError(err)
 
-			secret, err := kubeClientset.CoreV1().Secrets(ArgoCDNamespace).Get(ctx, db.RepoURLToSecretName(repoSecretPrefix, repoOpts.Repo.Repo), v1.GetOptions{})
+			secret, err := kubeClientset.CoreV1().Secrets(ArgoCDNamespace).Get(ctx, db.RepoURLToSecretName(repoSecretPrefix, repoOpts.Repo.Repo, repoOpts.Repo.Project), v1.GetOptions{})
 			errors.CheckError(err)
 
 			errors.CheckError(PrintResources(outputFormat, os.Stdout, secret))