Skip to content

Commit

Permalink
fix(appset): improve git generator repo credential fallback (#21167)
Browse files Browse the repository at this point in the history 
Signed-off-by: Blake Pettersson <[email protected]>
  • Loading branch information
@blakepettersson
blakepettersson authored Feb 7, 2025
1 parent 4a1d0f3 commit 922dd77
Show file tree
Hide file tree
Showing 16 changed files with 1,790 additions and 1,285 deletions.
24 changes: 18 additions & 6 deletions applicationset/generators/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,17 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic
verifyCommit = len(appProject.Spec.SignatureKeys) > 0 && gpg.IsGPGEnabled()
}

// If the project field is templated, we cannot resolve the project name, so we pass an empty string to the repo-server.
// This means only "globally-scoped" repo credentials can be used for such appsets.
project := resolveProjectName(appSet.Spec.Template.Spec.Project)

var err error
var res []map[string]any
switch {
case len(appSetGenerator.Git.Directories) != 0:
res, err = g.generateParamsForGitDirectories(appSetGenerator, noRevisionCache, verifyCommit, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
res, err = g.generateParamsForGitDirectories(appSetGenerator, noRevisionCache, verifyCommit, appSet.Spec.GoTemplate, project, appSet.Spec.GoTemplateOptions)
case len(appSetGenerator.Git.Files) != 0:
res, err = g.generateParamsForGitFiles(appSetGenerator, noRevisionCache, verifyCommit, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
res, err = g.generateParamsForGitFiles(appSetGenerator, noRevisionCache, verifyCommit, appSet.Spec.GoTemplate, project, appSet.Spec.GoTemplateOptions)
default:
return nil, EmptyAppSetGeneratorError
}
Expand All @@ -98,9 +102,9 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic
return res, nil
}

func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache, verifyCommit bool, useGoTemplate bool, goTemplateOptions []string) ([]map[string]any, error) {
func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache, verifyCommit, useGoTemplate bool, project string, goTemplateOptions []string) ([]map[string]any, 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, project, noRevisionCache, verifyCommit)
if err != nil {
return nil, fmt.Errorf("error getting directories from repo: %w", err)
}
Expand All @@ -123,11 +127,11 @@ func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoproj
return res, nil
}

func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache, verifyCommit bool, useGoTemplate bool, goTemplateOptions []string) ([]map[string]any, error) {
func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, noRevisionCache, verifyCommit, useGoTemplate bool, project string, goTemplateOptions []string) ([]map[string]any, error) {
// 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, project, requestedPath.Path, noRevisionCache, verifyCommit)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -303,3 +307,11 @@ func (g *GitGenerator) generateParamsFromApps(requestedApps []string, appSetGene

return res, nil
}

func resolveProjectName(project string) string {
if strings.Contains(project, "{{") {
return ""
}

return project
}
102 changes: 92 additions & 10 deletions applicationset/generators/git_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/argoproj/argo-cd/v3/applicationset/services/mocks"
Expand Down Expand Up @@ -199,7 +200,6 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) {
"app_3",
"p1/app4",
},
repoError: nil,
expected: []map[string]any{
{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1", "path[0]": "app1"},
{"path": "app2", "path.basename": "app2", "path.basenameNormalized": "app2", "path[0]": "app2"},
Expand Down Expand Up @@ -234,7 +234,6 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) {
"p1/p2/app3",
"p1/p2/p3/app4",
},
repoError: nil,
expected: []map[string]any{
{"path": "p1/app2", "path.basename": "app2", "path[0]": "p1", "path[1]": "app2", "path.basenameNormalized": "app2"},
{"path": "p1/p2/app3", "path.basename": "app3", "path[0]": "p1", "path[1]": "p2", "path[2]": "app3", "path.basenameNormalized": "app3"},
Expand Down Expand Up @@ -322,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 := v1alpha1.ApplicationSet{
Expand Down Expand Up @@ -623,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 := v1alpha1.ApplicationSet{
Expand Down Expand Up @@ -987,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 @@ -1343,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 @@ -1388,6 +1387,7 @@ cluster:
func TestGitGenerator_GenerateParams(t *testing.T) {
cases := []struct {
name string
appProject v1alpha1.AppProject
directories []v1alpha1.GitDirectoryGeneratorItem
pathParamPrefix string
repoApps []string
Expand All @@ -1396,6 +1396,7 @@ func TestGitGenerator_GenerateParams(t *testing.T) {
values map[string]string
expected []map[string]any
expectedError error
expectedProject *string
appset v1alpha1.ApplicationSet
callGetDirectories bool
}{
Expand Down Expand Up @@ -1467,21 +1468,102 @@ func TestGitGenerator_GenerateParams(t *testing.T) {
expected: []map[string]any{{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1", "path[0]": "app1", "values.foo": "bar"}},
expectedError: errors.New("error getting project project: appprojects.argoproj.io \"project\" not found"),
},
{
name: "Project field is not templated - verify that project is passed through to repo-server as-is",
repoApps: []string{
"app1",
},
callGetDirectories: true,
appProject: v1alpha1.AppProject{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Name: "project",
Namespace: "argocd",
},
},
appset: v1alpha1.ApplicationSet{
ObjectMeta: metav1.ObjectMeta{
Name: "set",
Namespace: "namespace",
},
Spec: v1alpha1.ApplicationSetSpec{
Generators: []v1alpha1.ApplicationSetGenerator{{
Git: &v1alpha1.GitGenerator{
RepoURL: "RepoURL",
Revision: "Revision",
Directories: []v1alpha1.GitDirectoryGeneratorItem{{Path: "*"}},
PathParamPrefix: "",
Values: map[string]string{
"foo": "bar",
},
},
}},
Template: v1alpha1.ApplicationSetTemplate{
Spec: v1alpha1.ApplicationSpec{
Project: "project",
},
},
},
},
expected: []map[string]any{{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1", "path[0]": "app1", "values.foo": "bar"}},
expectedProject: ptr.To("project"),
expectedError: nil,
},
{
name: "Project field is templated - verify that project is passed through to repo-server as empty string",
repoApps: []string{
"app1",
},
callGetDirectories: true,
appset: v1alpha1.ApplicationSet{
ObjectMeta: metav1.ObjectMeta{
Name: "set",
Namespace: "namespace",
},
Spec: v1alpha1.ApplicationSetSpec{
Generators: []v1alpha1.ApplicationSetGenerator{{
Git: &v1alpha1.GitGenerator{
RepoURL: "RepoURL",
Revision: "Revision",
Directories: []v1alpha1.GitDirectoryGeneratorItem{{Path: "*"}},
PathParamPrefix: "",
Values: map[string]string{
"foo": "bar",
},
},
}},
Template: v1alpha1.ApplicationSetTemplate{
Spec: v1alpha1.ApplicationSpec{
Project: "{{.project}}",
},
},
},
},
expected: []map[string]any{{"path": "app1", "path.basename": "app1", "path.basenameNormalized": "app1", "path[0]": "app1", "values.foo": "bar"}},
expectedProject: ptr.To(""),
expectedError: nil,
},
}
for _, testCase := range cases {
argoCDServiceMock := mocks.Repos{}

if testCase.callGetDirectories {
argoCDServiceMock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(testCase.repoApps, testCase.repoPathsError)
var project any
if testCase.expectedProject != nil {
project = *testCase.expectedProject
} else {
project = mock.Anything
}

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

scheme := runtime.NewScheme()
err := v1alpha1.AddToScheme(scheme)
require.NoError(t, err)
appProject := v1alpha1.AppProject{}

client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&appProject).Build()
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(&testCase.appProject).Build()

got, err := gitGenerator.GenerateParams(&testCase.appset.Spec.Generators[0], &testCase.appset, client)

Expand Down
2 changes: 1 addition & 1 deletion applicationset/generators/matrix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,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
View file

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

Loading

0 comments on commit 922dd77

Please sign in to comment.