Git Directory Generator only matches directories that contain valid Helm/ksonnet/Kustomize artifacts

go.mod

@@ -13,7 +13,6 @@ require (
 	github.com/sirupsen/logrus v1.6.0
 	github.com/stretchr/testify v1.6.1
 	github.com/valyala/fasttemplate v1.2.1
-	google.golang.org/grpc v1.29.1
 	k8s.io/api v0.19.2
 	k8s.io/apimachinery v0.19.2
 	k8s.io/client-go v11.0.1-0.20190816222228-6d55c1b1f1ca+incompatible

pkg/generators/git.go

@@ -72,19 +72,21 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic
 }
 
 func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) ([]map[string]string, error) {
-	allApps, err := g.repos.GetApps(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision)
+
+	// Directories, not files
+	allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision)
 	if err != nil {
 		return nil, err
 	}
 
 	log.WithFields(log.Fields{
-		"allAps":   allApps,
-		"total":    len(allApps),
+		"allPaths": allPaths,
+		"total":    len(allPaths),
 		"repoURL":  appSetGenerator.Git.RepoURL,
 		"revision": appSetGenerator.Git.Revision,
 	}).Info("applications result from the repo service")
 
-	requestedApps := g.filterApps(appSetGenerator.Git.Directories, allApps)
+	requestedApps := g.filterApps(appSetGenerator.Git.Directories, allPaths)
 
 	res := g.generateParamsFromApps(requestedApps, appSetGenerator)
 
@@ -96,7 +98,7 @@ func (g *GitGenerator) generateParamsForGitFiles(appSetGenerator *argoprojiov1al
 	// Get all paths that match the requested path string, removing duplicates
 	allPathsMap := make(map[string]bool)
 	for _, requestedPath := range appSetGenerator.Git.Files {
-		paths, err := g.repos.GetPaths(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, requestedPath.Path)
+		paths, err := g.repos.GetFilePaths(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision, requestedPath.Path)
 		if err != nil {
 			return nil, err
 		}

pkg/generators/git_test.go

@@ -29,7 +29,7 @@ func (a argoCDServiceMock) GetApps(ctx context.Context, repoURL string, revision
 	return args.Get(0).([]string), args.Error(1)
 }
 
-func (a argoCDServiceMock) GetPaths(ctx context.Context, repoURL string, revision string, pattern string) ([]string, error) {
+func (a argoCDServiceMock) GetFilePaths(ctx context.Context, repoURL string, revision string, pattern string) ([]string, error) {
 	args := a.mock.Called(ctx, repoURL, revision, pattern)
 
 	return args.Get(0).([]string), args.Error(1)
@@ -41,6 +41,11 @@ func (a argoCDServiceMock) GetFileContent(ctx context.Context, repoURL string, r
 	return args.Get(0).([]byte), args.Error(1)
 }
 
+func (a argoCDServiceMock) GetDirectories(ctx context.Context, repoURL string, revision string) ([]string, error) {
+	args := a.mock.Called(ctx, repoURL, revision)
+	return args.Get(0).([]string), args.Error(1)
+}
+
 func TestGitGenerateParamsFromDirectories(t *testing.T) {
 
 	cases := []struct {
@@ -104,7 +109,8 @@ func TestGitGenerateParamsFromDirectories(t *testing.T) {
 		cc := c
 		t.Run(cc.name, func(t *testing.T) {
 			argoCDServiceMock := argoCDServiceMock{mock: &mock.Mock{}}
-			argoCDServiceMock.mock.On("GetApps", mock.Anything, mock.Anything, mock.Anything).Return(c.repoApps, c.repoError)
+
+			argoCDServiceMock.mock.On("GetDirectories", mock.Anything, mock.Anything, mock.Anything).Return(c.repoApps, c.repoError)
 
 			var gitGenerator = NewGitGenerator(argoCDServiceMock)
 			applicationSetInfo := argoprojiov1alpha1.ApplicationSet{
@@ -239,7 +245,7 @@ func TestGitGenerateParamsFromFiles(t *testing.T) {
 		cc := c
 		t.Run(cc.name, func(t *testing.T) {
 			argoCDServiceMock := argoCDServiceMock{mock: &mock.Mock{}}
-			argoCDServiceMock.mock.On("GetPaths", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(c.repoPaths, c.repoPathsError)
+			argoCDServiceMock.mock.On("GetFilePaths", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(c.repoPaths, c.repoPathsError)
 
 			if c.repoPaths != nil {
 				for _, repoPath := range c.repoPaths {

pkg/services/repo_service.go

@@ -3,15 +3,14 @@ package services
 import (
 	"context"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 
 	"github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	"github.com/argoproj/argo-cd/reposerver/apiclient"
 	"github.com/argoproj/argo-cd/util/db"
 	"github.com/argoproj/argo-cd/util/git"
-	"github.com/argoproj/argo-cd/util/io"
 	"github.com/pkg/errors"
-	log "github.com/sirupsen/logrus"
 )
 
 // RepositoryDB Is a lean facade for ArgoDB,
@@ -26,8 +25,14 @@ type argoCDService struct {
 }
 
 type Repos interface {
-	GetApps(ctx context.Context, repoURL string, revision string) ([]string, error)
-	GetPaths(ctx context.Context, repoURL string, revision string, pattern string) ([]string, error)
+
+	// GetFilePaths returns a list of files (not directories) within the target repo
+	GetFilePaths(ctx context.Context, repoURL string, revision string, pattern string) ([]string, error)
+
+	// GetDirectories returns a list of directories (not files) within the target repo
+	GetDirectories(ctx context.Context, repoURL string, revision string) ([]string, error)
+
+	// GetFileContent returns the contents of a particular repository file
 	GetFileContent(ctx context.Context, repoURL string, revision string, path string) ([]byte, error)
 }
 
@@ -39,45 +44,39 @@ func NewArgoCDService(db db.ArgoDB, repoServerAddress string) Repos {
 	}
 }
 
-func (a *argoCDService) GetApps(ctx context.Context, repoURL string, revision string) ([]string, error) {
+func (a *argoCDService) GetFilePaths(ctx context.Context, repoURL string, revision string, pattern string) ([]string, error) {
 	repo, err := a.repositoriesDB.GetRepository(ctx, repoURL)
 	if err != nil {
-
 		return nil, errors.Wrap(err, "Error in GetRepository")
 	}
 
-	conn, repoClient, err := a.repoClientset.NewRepoServerClient()
-	defer io.Close(conn)
+	gitRepoClient, err := git.NewClient(repo.Repo, repo.GetGitCreds(), repo.IsInsecure(), repo.IsLFSEnabled())
+
 	if err != nil {
-		return nil, errors.Wrap(err, "Error in creating repo service client")
+		return nil, err
 	}
 
-	apps, err := repoClient.ListApps(ctx, &apiclient.ListAppsRequest{
-		Repo:     repo,
-		Revision: revision,
-	})
-	log.Debugf("apps - %#v", apps)
+	err = checkoutRepo(gitRepoClient, revision)
 	if err != nil {
-		return nil, errors.Wrap(err, "Error in ListApps")
+		return nil, err
 	}
 
-	res := []string{}
-
-	for name := range apps.Apps {
-		res = append(res, name)
+	paths, err := gitRepoClient.LsFiles(pattern)
+	if err != nil {
+		return nil, errors.Wrap(err, "Error during listing files of local repo")
 	}
 
-	return res, nil
+	return paths, nil
 }
 
-func (a *argoCDService) GetPaths(ctx context.Context, repoURL string, revision string, pattern string) ([]string, error) {
+func (a *argoCDService) GetDirectories(ctx context.Context, repoURL string, revision string) ([]string, error) {
+
 	repo, err := a.repositoriesDB.GetRepository(ctx, repoURL)
 	if err != nil {
 		return nil, errors.Wrap(err, "Error in GetRepository")
 	}
 
 	gitRepoClient, err := git.NewClient(repo.Repo, repo.GetGitCreds(), repo.IsInsecure(), repo.IsLFSEnabled())
-
 	if err != nil {
 		return nil, err
 	}
@@ -87,12 +86,41 @@ func (a *argoCDService) GetPaths(ctx context.Context, repoURL string, revision s
 		return nil, err
 	}
 
-	paths, err := gitRepoClient.LsFiles(pattern)
-	if err != nil {
-		return nil, errors.Wrap(err, "Error during listing files of local repo")
+	filteredPaths := []string{}
+
+	repoRoot := gitRepoClient.Root()
+
+	if err := filepath.Walk(repoRoot, func(path string, info os.FileInfo, fnErr error) error {
+		if fnErr != nil {
+			return fnErr
+		}
+		if !info.IsDir() { // Skip files: directories only
+			return nil
+		}
+
+		fname := info.Name()
+		if fname == ".git" { // Skip repository metadata
+			return filepath.SkipDir
+		}
+
+		relativePath, err := filepath.Rel(repoRoot, path)
+		if err != nil {
+			return err
+		}
+
+		if relativePath == "." { // Exclude '.' from results
+			return nil
+		}
+
+		filteredPaths = append(filteredPaths, relativePath)
+
+		return nil
+	}); err != nil {
+		return nil, err
 	}
 
-	return paths, nil
+	return filteredPaths, nil
+
 }
 
 func (a *argoCDService) GetFileContent(ctx context.Context, repoURL string, revision string, path string) ([]byte, error) {
@@ -123,7 +151,7 @@ func (a *argoCDService) GetFileContent(ctx context.Context, repoURL string, revi
 func checkoutRepo(gitRepoClient git.Client, revision string) error {
 	err := gitRepoClient.Init()
 	if err != nil {
-		return errors.Wrap(err, "Error during initiliazing repo")
+		return errors.Wrap(err, "Error during initializing repo")
 	}
 
 	err = gitRepoClient.Fetch()

pkg/services/repo_service_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/argoproj/argo-cd/util/io"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
-	"google.golang.org/grpc"
 )
 
 type ArgocdRepositoryMock struct {
@@ -25,33 +24,6 @@ func (a ArgocdRepositoryMock) GetRepository(ctx context.Context, url string) (*v
 
 }
 
-type repoServerClientMock struct {
-	mock *mock.Mock
-}
-
-func (r repoServerClientMock) GenerateManifest(ctx context.Context, in *apiclient.ManifestRequest, opts ...grpc.CallOption) (*apiclient.ManifestResponse, error) {
-	return nil, nil
-}
-func (r repoServerClientMock) ListApps(ctx context.Context, in *apiclient.ListAppsRequest, opts ...grpc.CallOption) (*apiclient.AppList, error) {
-	args := r.mock.Called(ctx, in)
-
-	return args.Get(0).(*apiclient.AppList), args.Error(1)
-}
-func (r repoServerClientMock) ListRefs(ctx context.Context, in *apiclient.ListRefsRequest, opts ...grpc.CallOption) (*apiclient.Refs, error) {
-	args := r.mock.Called(ctx, in)
-
-	return args.Get(0).(*apiclient.Refs), args.Error(1)
-}
-func (r repoServerClientMock) GetAppDetails(ctx context.Context, in *apiclient.RepoServerAppDetailsQuery, opts ...grpc.CallOption) (*apiclient.RepoAppDetailsResponse, error) {
-	return nil, nil
-}
-func (r repoServerClientMock) GetRevisionMetadata(ctx context.Context, in *apiclient.RepoServerRevisionMetadataRequest, opts ...grpc.CallOption) (*v1alpha1.RevisionMetadata, error) {
-	return nil, nil
-}
-func (r repoServerClientMock) GetHelmCharts(ctx context.Context, in *apiclient.HelmChartsRequest, opts ...grpc.CallOption) (*apiclient.HelmChartsResponse, error) {
-	return nil, nil
-}
-
 type closer struct {
 	// mock *mock.Mock
 }
@@ -70,88 +42,73 @@ func (r repoClientsetMock) NewRepoServerClient() (io.Closer, apiclient.RepoServe
 	return closer{}, args.Get(0).(apiclient.RepoServerServiceClient), args.Error(1)
 }
 
-func TestGetApps(t *testing.T) {
+func TestGetDirectories(t *testing.T) {
+
+	// Hardcode a specific revision to changes to argocd-example-apps from regressing this test:
+	//   Author: Alexander Matyushentsev <Alexander_Matyushentsev@intuit.com>
+	//   Date:   Sun Jan 31 09:54:53 2021 -0800
+	//   chore: downgrade kustomize guestbook image tag (#73)
+	exampleRepoRevision := "08f72e2a309beab929d9fd14626071b1a61a47f9"
 
 	for _, c := range []struct {
 		name          string
 		repoURL       string
 		revision      string
 		repoRes       *v1alpha1.Repository
 		repoErr       error
-		appRes        *apiclient.AppList
-		appError      error
 		expected      []string
 		expectedError error
 	}{
 		{
-			"Happy Flow",
-			"repoURL",
-			"revision",
-			&v1alpha1.Repository{},
-			nil,
-			&apiclient.AppList{
-				Apps: map[string]string{
-					"app1": "",
-					"app2": "",
-				},
+			name:     "All child folders should be returned",
+			repoURL:  "https://github.com/argoproj/argocd-example-apps/",
+			revision: exampleRepoRevision,
+			repoRes: &v1alpha1.Repository{
+				Repo: "https://github.com/argoproj/argocd-example-apps/",
 			},
-			nil,
-			[]string{"app1", "app2"},
-			nil,
+			repoErr: nil,
+			expected: []string{"apps", "apps/templates", "blue-green", "blue-green/templates", "guestbook", "helm-dependency",
+				"helm-guestbook", "helm-guestbook/templates", "helm-hooks", "jsonnet-guestbook", "jsonnet-guestbook-tla",
+				"ksonnet-guestbook", "ksonnet-guestbook/components", "ksonnet-guestbook/environments", "ksonnet-guestbook/environments/default",
+				"ksonnet-guestbook/environments/dev", "ksonnet-guestbook/environments/prod", "kustomize-guestbook", "plugins", "plugins/kasane",
+				"plugins/kustomized-helm", "plugins/kustomized-helm/overlays", "pre-post-sync", "sock-shop", "sock-shop/base", "sync-waves"},
 		},
 		{
-			"handles GetRepository error",
-			"repoURL",
-			"revision",
-			&v1alpha1.Repository{},
-			errors.New("error"),
-			&apiclient.AppList{
-				Apps: map[string]string{
-					"app1": "",
-					"app2": "",
-				},
+			name:     "If GetRepository returns an error, it should pass back to caller",
+			repoURL:  "https://github.com/argoproj/argocd-example-apps/",
+			revision: exampleRepoRevision,
+			repoRes: &v1alpha1.Repository{
+				Repo: "https://github.com/argoproj/argocd-example-apps/",
 			},
-			nil,
-			[]string{},
-			errors.New("Error in GetRepository: error"),
+			repoErr:       errors.New("Simulated error from GetRepository"),
+			expected:      nil,
+			expectedError: errors.New("Error in GetRepository: Simulated error from GetRepository"),
 		},
 		{
-			"handles ListApps error",
-			"repoURL",
-			"revision",
-			&v1alpha1.Repository{},
-			nil,
-			&apiclient.AppList{
-				Apps: map[string]string{
-					"app1": "",
-					"app2": "",
-				},
+			name: "Test against repository containing no directories",
+			// Here I picked an arbitrary repository in argoproj-labs, with a commit containing no folders.
+			repoURL:  "https://github.com/argoproj-labs/argo-workflows-operator/",
+			revision: "5f50933a576833b73b7a172909d8545a108685f4",
+			repoRes: &v1alpha1.Repository{
+				Repo: "https://github.com/argoproj-labs/argo-workflows-operator/",
 			},
-			errors.New("error"),
-			[]string{},
-			errors.New("Error in ListApps: error"),
+			repoErr:  nil,
+			expected: []string{},
 		},
 	} {
 		cc := c
 		t.Run(cc.name, func(t *testing.T) {
 			argocdRepositoryMock := ArgocdRepositoryMock{mock: &mock.Mock{}}
-			repoServerClientMock := repoServerClientMock{mock: &mock.Mock{}}
 			repoClientsetMock := repoClientsetMock{mock: &mock.Mock{}}
 
 			argocdRepositoryMock.mock.On("GetRepository", mock.Anything, cc.repoURL).Return(cc.repoRes, cc.repoErr)
 
-			repoServerClientMock.mock.On("ListApps", mock.Anything, &apiclient.ListAppsRequest{
-				Repo:     cc.repoRes,
-				Revision: cc.revision,
-			}).Return(cc.appRes, cc.appError)
-
-			repoClientsetMock.mock.On("NewRepoServerClient").Return(repoServerClientMock, nil)
-
 			argocd := argoCDService{
 				repositoriesDB: argocdRepositoryMock,
 				repoClientset:  repoClientsetMock,
 			}
-			got, err := argocd.GetApps(context.TODO(), cc.repoURL, cc.revision)
+
+			got, err := argocd.GetDirectories(context.TODO(), cc.repoURL, cc.revision)
 
 			if cc.expectedError != nil {
 				assert.EqualError(t, err, cc.expectedError.Error())