Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Commit

Permalink
Git Directory Generator only matches directories that contain valid H…
Browse files Browse the repository at this point in the history
…elm/ksonnet/Kustomize artifacts (#132)
  • Loading branch information
jgwest committed Mar 2, 2021
1 parent f72bf48 commit a52be07
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 115 deletions.
1 change: 0 additions & 1 deletion go.mod
Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions pkg/generators/git.go
Expand Up @@ -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)

Expand All @@ -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
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/generators/git_test.go
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down
82 changes: 55 additions & 27 deletions pkg/services/repo_service.go
Expand Up @@ -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,
Expand All @@ -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)
}

Expand All @@ -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
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down
115 changes: 36 additions & 79 deletions pkg/services/repo_service_test.go
Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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())
Expand Down

0 comments on commit a52be07

Please sign in to comment.