Skip to content

Commit

Permalink
feat(appset): allow usage of project-scoped repos in git generators
Browse files Browse the repository at this point in the history
With #18388, we added the possibility of being able to create a
repository credential with the same URL per AppProject (previously one
could only create one credential globally for the same URL). Sadly, this
caused a regression in regards to ApplicationSets. Now, any credential
that is used for an appset needs to have an unset appproject.

This fixes it by using the same semantics as the API server (used
when doing `argocd repo get` or `argocd repo delete`:

* If there is only one repo credential with the given URL and there is
  no AppProject set on the Git genertor - use that.
* If an AppProject is specified on the Git generator and the single repo
  cred does not match it, return an error.
* If there are multiple repo credentials with the same URL, return an
  error if none matches the given project passed in from the git
generator

This could potentially be split into two PRs; one to allow for a more
flexible retrieval of a _single_ URL, which can be backported to 2.12 -
2.13, and another with the CRD changes which would allow a project to
be specified on the Git generator, which would go in 2.14+.

Signed-off-by: Blake Pettersson <[email protected]>
  • Loading branch information
blakepettersson committed Dec 16, 2024
1 parent 87c853e commit 63c7a0f
Show file tree
Hide file tree
Showing 18 changed files with 841 additions and 728 deletions.
4 changes: 2 additions & 2 deletions applicationset/generators/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic

func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache, verifyCommit bool, useGoTemplate bool, goTemplateOptions []string) ([]map[string]interface{}, error) {
// Directories, not files
allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, noRevisionCache, verifyCommit)
allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, appSetGenerator.Git.Project, noRevisionCache, verifyCommit)
if err != nil {
return nil, fmt.Errorf("error getting directories from repo: %w", err)
}
Expand All @@ -126,7 +126,7 @@ func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1al
// Get all files that match the requested path string, removing duplicates
allFiles := make(map[string][]byte)
for _, requestedPath := range appSetGenerator.Git.Files {
files, err := g.repos.GetFiles(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, requestedPath.Path, noRevisionCache, verifyCommit)
files, err := g.repos.GetFiles(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, appSetGenerator.Git.Project, requestedPath.Path, noRevisionCache, verifyCommit)
if err != nil {
return nil, err
}
Expand Down
10 changes: 5 additions & 5 deletions applicationset/generators/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) {

argoCDServiceMock := mocks.Repos{}

argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)

gitGenerator := NewGitGenerator(&argoCDServiceMock, "")
applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
Expand Down Expand Up @@ -622,7 +622,7 @@ func TestGitGenerateParamsFromDirectoriesGoTemplate(t *testing.T) {

argoCDServiceMock := mocks.Repos{}

argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCaseCopy.repoApps, testCaseCopy.repoError)

gitGenerator := NewGitGenerator(&argoCDServiceMock, "")
applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
Expand Down Expand Up @@ -986,7 +986,7 @@ cluster:
t.Parallel()

argoCDServiceMock := mocks.Repos{}
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(testCaseCopy.repoFileContents, testCaseCopy.repoPathsError)

gitGenerator := NewGitGenerator(&argoCDServiceMock, "")
Expand Down Expand Up @@ -1342,7 +1342,7 @@ cluster:
t.Parallel()

argoCDServiceMock := mocks.Repos{}
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
argoCDServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(testCaseCopy.repoFileContents, testCaseCopy.repoPathsError)

gitGenerator := NewGitGenerator(&argoCDServiceMock, "")
Expand Down Expand Up @@ -1471,7 +1471,7 @@ func TestGitGenerator_GenerateParams(t *testing.T) {
argoCDServiceMock := mocks.Repos{}

if testCase.callGetDirectories {
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCase.repoApps, testCase.repoPathsError)
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCase.repoApps, testCase.repoPathsError)
}
gitGenerator := NewGitGenerator(&argoCDServiceMock, "namespace")

Expand Down
2 changes: 1 addition & 1 deletion applicationset/generators/matrix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ func TestGitGenerator_GenerateParams_list_x_git_matrix_generator(t *testing.T) {
}

repoServiceMock := &mocks.Repos{}
repoServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(map[string][]byte{
repoServiceMock.On("GetFiles", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(map[string][]byte{
"some/path.json": []byte("test: content"),
}, nil)
gitGenerator := NewGitGenerator(repoServiceMock, "")
Expand Down
36 changes: 18 additions & 18 deletions applicationset/services/mocks/Repos.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 34 additions & 11 deletions applicationset/services/repo_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@ package services

import (
"context"
"errors"
"fmt"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"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/git"
"github.com/argoproj/argo-cd/v2/util/io"
"github.com/argoproj/argo-cd/v2/util/repository"
)

type argoCDService struct {
getRepository func(ctx context.Context, url, project string) (*v1alpha1.Repository, error)
listRepositories func(ctx context.Context) ([]*v1alpha1.Repository, error)
storecreds git.CredsStore
submoduleEnabled bool
repoServerClientSet apiclient.Clientset
Expand All @@ -20,25 +25,34 @@ type argoCDService struct {

type Repos interface {
// GetFiles returns content of files (not directories) within the target repo
GetFiles(ctx context.Context, repoURL string, revision string, pattern string, noRevisionCache, verifyCommit bool) (map[string][]byte, error)
GetFiles(ctx context.Context, repoURL, revision, project, pattern string, noRevisionCache, verifyCommit bool) (map[string][]byte, error)

// GetDirectories returns a list of directories (not files) within the target repo
GetDirectories(ctx context.Context, repoURL string, revision string, noRevisionCache, verifyCommit bool) ([]string, error)
GetDirectories(ctx context.Context, repoURL, revision, project string, noRevisionCache, verifyCommit bool) ([]string, error)
}

func NewArgoCDService(getRepository func(ctx context.Context, url, project string) (*v1alpha1.Repository, error), submoduleEnabled bool, repoClientset apiclient.Clientset, newFileGlobbingEnabled bool) (Repos, error) {
func NewArgoCDService(listRepositories func(ctx context.Context) ([]*v1alpha1.Repository, error), submoduleEnabled bool, repoClientset apiclient.Clientset, newFileGlobbingEnabled bool) (Repos, error) {
return &argoCDService{
getRepository: getRepository,
listRepositories: listRepositories,
submoduleEnabled: submoduleEnabled,
repoServerClientSet: repoClientset,
newFileGlobbingEnabled: newFileGlobbingEnabled,
}, nil
}

func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision string, pattern string, noRevisionCache, verifyCommit bool) (map[string][]byte, error) {
repo, err := a.getRepository(ctx, repoURL, "")
func (a *argoCDService) GetFiles(ctx context.Context, repoURL, revision, project, pattern string, noRevisionCache, verifyCommit bool) (map[string][]byte, error) {
repos, err := a.listRepositories(ctx)
if err != nil {
return nil, fmt.Errorf("error in ListRepositories: %w", err)
}

repo, err := repository.FilterRepositoryByProjectAndURL(repos, repoURL, project)
if err != nil {
return nil, fmt.Errorf("error in GetRepository: %w", err)
if errors.Is(err, status.Error(codes.PermissionDenied, "permission denied")) {
repo = &v1alpha1.Repository{Repo: repoURL}
} else {
return nil, fmt.Errorf("error retrieving Git files: %w", err)
}
}

fileRequest := &apiclient.GitFilesRequest{
Expand All @@ -63,10 +77,19 @@ func (a *argoCDService) GetFiles(ctx context.Context, repoURL string, revision s
return fileResponse.GetMap(), nil
}

func (a *argoCDService) GetDirectories(ctx context.Context, repoURL string, revision string, noRevisionCache, verifyCommit bool) ([]string, error) {
repo, err := a.getRepository(ctx, repoURL, "")
func (a *argoCDService) GetDirectories(ctx context.Context, repoURL, revision, project string, noRevisionCache, verifyCommit bool) ([]string, error) {
repos, err := a.listRepositories(ctx)
if err != nil {
return nil, fmt.Errorf("error in ListRepositories: %w", err)
}

repo, err := repository.FilterRepositoryByProjectAndURL(repos, repoURL, project)
if err != nil {
return nil, fmt.Errorf("error in GetRepository: %w", err)
if errors.Is(err, status.Error(codes.PermissionDenied, "permission denied")) {
repo = &v1alpha1.Repository{Repo: repoURL}
} else {
return nil, fmt.Errorf("error retrieving Git files: %w", err)
}
}

dirRequest := &apiclient.GitDirectoriesRequest{
Expand Down
Loading

0 comments on commit 63c7a0f

Please sign in to comment.