Repository: argoproj-labs/applicationset (go)

Issue:
Git Directory Generator only matches directories that contain valid Helm/ksonnet/Kustomize artifacts
The Git Directory generator is currently calling Argo CD repo-server's `ListApps` function, in order to discover the list of directories that are available in the repository.

However, that function only returns directories that contain Argo CD applications (and it excludes directory-only applications), specifically those that [match the set of rules here](https://github.com/argoproj/argo-cd/blob/1d1fbf0fb17d5c47ff269b3232d090dc507e0b8d/util/app/discovery/discovery.go#L14), and [described in the docs here](https://argoproj.github.io/argo-cd/user-guide/tool_detection/).

While this works well for Argo CD, it doesn't fully fit our needs... as per the design doc, Git Directory should consider all directories, not just those that contain kssonet, Helm, or Kustomize artifacts.

So a couple better solutions are:
1) Either: Use `repo_service.go`'s `GetPaths` instead of `GetApps`, in order to gather the list of directories
2) Or: add a new function to Argo CD repo service to return a full list of paths.

IMHO I prefer option 1 (reserving the right to implement option 2 in the future, if we find that too much CPU/network resources are consumed by option 1).


Pull request description:
Git Directory Generator only matches directories that contain valid Helm/ksonnet/Kustomize artifacts
See parent issue for details.

Fixes #121 

Patch:
diff --git a/go.mod b/go.mod
index 6a2bed8..73b4ca4 100644
--- a/go.mod
+++ b/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
diff --git a/pkg/generators/git.go b/pkg/generators/git.go
index 93e22ce..1e16c2d 100644
--- a/pkg/generators/git.go
+++ b/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
 		}
diff --git a/pkg/services/repo_service.go b/pkg/services/repo_service.go
index 3c22303..f8728fe 100644
--- a/pkg/services/repo_service.go
+++ b/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()


Test patch:
diff --git a/pkg/generators/git_test.go b/pkg/generators/git_test.go
index ea49916..8d383b2 100644
--- a/pkg/generators/git_test.go
+++ b/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 {
diff --git a/pkg/services/repo_service_test.go b/pkg/services/repo_service_test.go
index a53707b..1186968 100644
--- a/pkg/services/repo_service_test.go
+++ b/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,7 +42,13 @@ 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
@@ -78,80 +56,59 @@ func TestGetApps(t *testing.T) {
 		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())