diff --git a/pkg/affected_apps/argocd_matcher.go b/pkg/affected_apps/argocd_matcher.go index 935daeeb..1fd33fc8 100644 --- a/pkg/affected_apps/argocd_matcher.go +++ b/pkg/affected_apps/argocd_matcher.go @@ -36,13 +36,17 @@ func logCounts(repoApps *appdir.AppDirectory) { if repoApps == nil { log.Debug().Msg("found no apps") } else { - log.Debug().Msgf("found %d apps", repoApps.Count()) + log.Debug().Int("apps", repoApps.AppsCount()). + Int("app_files", repoApps.AppFilesCount()). + Int("app_dirs", repoApps.AppDirsCount()). + Msg("mapped apps") } } func getKustomizeApps(vcsToArgoMap appdir.VcsToArgoMap, repo *git.Repo, repoPath string) *appdir.AppDirectory { log.Debug().Msgf("creating fs for %s", repoPath) fs := os.DirFS(repoPath) + log.Debug().Msg("following kustomize apps") kustomizeAppFiles := vcsToArgoMap.WalkKustomizeApps(repo.CloneURL, fs) @@ -76,7 +80,7 @@ func (a *ArgocdMatcher) AffectedApps(_ context.Context, changeList []string, tar } appsSlice := a.appsDirectory.FindAppsBasedOnChangeList(changeList, targetBranch) - appSetsSlice := a.appSetsDirectory.FindAppsBasedOnChangeList(changeList, repo) + appSetsSlice := a.appSetsDirectory.FindAppSetsBasedOnChangeList(changeList, repo) // and return both apps and appSets return AffectedItems{ diff --git a/pkg/affected_apps/argocd_matcher_test.go b/pkg/affected_apps/argocd_matcher_test.go index 0db01456..074aae94 100644 --- a/pkg/affected_apps/argocd_matcher_test.go +++ b/pkg/affected_apps/argocd_matcher_test.go @@ -2,10 +2,15 @@ package affected_apps import ( "context" + "math/rand" + "os" + "path/filepath" + "strconv" "testing" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/zapier/kubechecks/pkg/appdir" "github.com/zapier/kubechecks/pkg/git" ) @@ -41,3 +46,160 @@ func TestFindAffectedAppsWithNilAppsDirectory(t *testing.T) { require.Len(t, items.Applications, 0) require.Len(t, items.ApplicationSets, 0) } + +func TestScenarios(t *testing.T) { + repoURL := "https://github.com/argoproj/argocd-example-apps.git" + + testCases := map[string]struct { + files map[string]string + changedFiles []string + app v1alpha1.Application + expected bool + }{ + "helm - match": { + files: map[string]string{ + "app/Chart.yaml": `apiVersion: v1`, + }, + changedFiles: []string{"app/Chart.yaml"}, + app: v1alpha1.Application{ + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + RepoURL: repoURL, + Path: "app/", + }, + }, + }, + expected: true, + }, + "helm - value file outside of app path": { + files: map[string]string{ + "app/Chart.yaml": `apiVersion: v1`, + "values.yaml": "content", + }, + changedFiles: []string{"values.yaml"}, + app: v1alpha1.Application{ + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + RepoURL: repoURL, + Path: "app/", + Helm: &v1alpha1.ApplicationSourceHelm{ + ValueFiles: []string{ + "../values.yaml", + }, + }, + }, + }, + }, + expected: true, + }, + "helm - file parameter outside of app path": { + files: map[string]string{ + "app/Chart.yaml": `apiVersion: v1`, + "values.yaml": "content", + }, + app: v1alpha1.Application{ + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + RepoURL: repoURL, + Path: "app/", + Helm: &v1alpha1.ApplicationSourceHelm{ + FileParameters: []v1alpha1.HelmFileParameter{{ + Name: "key", Path: "../values.yaml", + }}, + }, + }, + }, + }, + changedFiles: []string{"values.yaml"}, + expected: true, + }, + "kustomize": { + files: map[string]string{ + "app/kustomization.yaml": ` +resources: +- file.yaml`, + "app/file.yaml": "content", + }, + changedFiles: []string{"app/file.yaml"}, + app: v1alpha1.Application{ + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + RepoURL: repoURL, + Path: "app/", + }, + }, + }, + expected: true, + }, + "kustomize - file is outside of app path": { + files: map[string]string{ + "app/kustomization.yaml": ` +resources: +- ../file.yaml`, + "file.yaml": "content", + }, + changedFiles: []string{"file.yaml"}, + app: v1alpha1.Application{ + Spec: v1alpha1.ApplicationSpec{ + Source: &v1alpha1.ApplicationSource{ + RepoURL: repoURL, + Path: "app/", + }, + }, + }, + expected: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + tc.app.Name = name + tc.app.Spec.Source.RepoURL = repoURL + + testRepo := dumpFiles(t, repoURL, "HEAD", tc.files) + + testVCSMap := appdir.NewVcsToArgoMap("vcs-username") + testVCSMap.AddApp(&tc.app) + + m, err := NewArgocdMatcher(testVCSMap, testRepo) + require.NoError(t, err) + + ctx := context.Background() + + result, err := m.AffectedApps(ctx, tc.changedFiles, "main", testRepo) + require.NoError(t, err) + + if tc.expected { + require.Len(t, result.Applications, 1) + app := result.Applications[0] + assert.Equal(t, tc.app.Name, app.Name) + } else { + require.Len(t, result.Applications, 0) + } + }) + } +} + +func dumpFiles(t *testing.T, cloneURL, target string, files map[string]string) *git.Repo { + tempDir := filepath.Join(t.TempDir(), strconv.Itoa(rand.Int())) + repo := &git.Repo{ + BranchName: target, + CloneURL: cloneURL, + Directory: tempDir, + } + + for file, fileContent := range files { + fullfilepath := filepath.Join(tempDir, file) + + // ensure the directories exist + filedir := filepath.Dir(fullfilepath) + err := os.MkdirAll(filedir, 0o755) + require.NoError(t, err) + + // write the file to disk + err = os.WriteFile(fullfilepath, []byte(fileContent), 0o600) + require.NoError(t, err) + } + + return repo +} diff --git a/pkg/app_watcher/app_watcher_test.go b/pkg/app_watcher/app_watcher_test.go index 4af7425c..9f9c7380 100644 --- a/pkg/app_watcher/app_watcher_test.go +++ b/pkg/app_watcher/app_watcher_test.go @@ -86,9 +86,10 @@ func TestApplicationUpdated(t *testing.T) { assert.Equal(t, len(ctrl.vcsToArgoMap.GetMap()), 2) oldAppDirectory := ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") + assert.Equal(t, oldAppDirectory.AppsCount(), 1) + newAppDirectory := ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo-3.git") - assert.Equal(t, oldAppDirectory.Count(), 1) - assert.Equal(t, newAppDirectory.Count(), 0) + assert.Equal(t, newAppDirectory.AppsCount(), 0) // _, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications("default").Update(ctx, &v1alpha1.Application{ ObjectMeta: metav1.ObjectMeta{Name: "test-app-1", Namespace: "default"}, @@ -102,8 +103,8 @@ func TestApplicationUpdated(t *testing.T) { time.Sleep(time.Second * 1) oldAppDirectory = ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") newAppDirectory = ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo-3.git") - assert.Equal(t, oldAppDirectory.Count(), 0) - assert.Equal(t, newAppDirectory.Count(), 1) + assert.Equal(t, oldAppDirectory.AppsCount(), 0) + assert.Equal(t, newAppDirectory.AppsCount(), 1) } func TestApplicationDeleted(t *testing.T) { @@ -119,7 +120,7 @@ func TestApplicationDeleted(t *testing.T) { assert.Equal(t, len(ctrl.vcsToArgoMap.GetMap()), 2) appDirectory := ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") - assert.Equal(t, appDirectory.Count(), 1) + assert.Equal(t, appDirectory.AppsCount(), 1) // err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications("default").Delete(ctx, "test-app-1", metav1.DeleteOptions{}) if err != nil { @@ -128,7 +129,7 @@ func TestApplicationDeleted(t *testing.T) { time.Sleep(time.Second * 1) appDirectory = ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") - assert.Equal(t, appDirectory.Count(), 0) + assert.Equal(t, appDirectory.AppsCount(), 0) } // TestIsGitRepo will test various URLs against the isGitRepo function. diff --git a/pkg/appdir/app_directory.go b/pkg/appdir/app_directory.go index a915fe61..36f41f88 100644 --- a/pkg/appdir/app_directory.go +++ b/pkg/appdir/app_directory.go @@ -23,10 +23,18 @@ func NewAppDirectory() *AppDirectory { } } -func (d *AppDirectory) Count() int { +func (d *AppDirectory) AppsCount() int { return len(d.appsMap) } +func (d *AppDirectory) AppFilesCount() int { + return len(d.appFiles) +} + +func (d *AppDirectory) AppDirsCount() int { + return len(d.appDirs) +} + func (d *AppDirectory) Union(other *AppDirectory) *AppDirectory { var join AppDirectory join.appsMap = mergeMaps(d.appsMap, other.appsMap, takeFirst[v1alpha1.Application]) @@ -35,29 +43,6 @@ func (d *AppDirectory) Union(other *AppDirectory) *AppDirectory { return &join } -func (d *AppDirectory) ProcessApp(app v1alpha1.Application) { - appName := app.Name - - src := app.Spec.GetSource() - - // common data - srcPath := src.Path - d.AddApp(app) - - // handle extra helm paths - if helm := src.Helm; helm != nil { - for _, param := range helm.FileParameters { - path := filepath.Join(srcPath, param.Path) - d.AddFile(appName, path) - } - - for _, valueFilePath := range helm.ValueFiles { - path := filepath.Join(srcPath, valueFilePath) - d.AddFile(appName, path) - } - } -} - // FindAppsBasedOnChangeList receives a list of modified file paths and // returns the list of applications that are affected by the changes. // @@ -155,21 +140,47 @@ func (d *AppDirectory) AddApp(app v1alpha1.Application) { log.Debug().Msgf("app %s already exists", app.Name) return } - log.Debug(). - Str("appName", app.Name). - Str("cluster-name", app.Spec.Destination.Name). - Str("cluster-server", app.Spec.Destination.Server). - Str("source", getSourcePath(app)). - Msg("add app") - d.appsMap[app.Name] = app - d.AddDir(app.Name, getSourcePath(app)) + + for _, src := range getSources(app) { + sourcePath := src.Path + log.Debug(). + Str("appName", app.Name). + Str("cluster-name", app.Spec.Destination.Name). + Str("cluster-server", app.Spec.Destination.Server). + Str("source", sourcePath). + Msg("add app") + + d.appsMap[app.Name] = app + d.addDir(app.Name, sourcePath) + + // handle extra helm paths + if helm := src.Helm; helm != nil { + for _, param := range helm.FileParameters { + path := filepath.Join(sourcePath, param.Path) + d.addFile(app.Name, path) + } + + for _, valueFilePath := range helm.ValueFiles { + path := filepath.Join(sourcePath, valueFilePath) + d.addFile(app.Name, path) + } + } + } +} + +func getSources(app v1alpha1.Application) []v1alpha1.ApplicationSource { + if !app.Spec.HasMultipleSources() { + return []v1alpha1.ApplicationSource{*app.Spec.Source} + } + + return app.Spec.Sources } -func (d *AppDirectory) AddDir(appName, path string) { +func (d *AppDirectory) addDir(appName, path string) { d.appDirs[path] = append(d.appDirs[path], appName) } -func (d *AppDirectory) AddFile(appName, path string) { +func (d *AppDirectory) addFile(appName, path string) { d.appFiles[path] = append(d.appFiles[path], appName) } diff --git a/pkg/appdir/app_directory_test.go b/pkg/appdir/app_directory_test.go index b67f2aa8..20acc034 100644 --- a/pkg/appdir/app_directory_test.go +++ b/pkg/appdir/app_directory_test.go @@ -31,7 +31,7 @@ func TestPathsAreJoinedProperly(t *testing.T) { }, } - rad.ProcessApp(app1) + rad.AddApp(app1) assert.Equal(t, map[string]v1alpha1.Application{ "test-app": app1, diff --git a/pkg/appdir/appset_directory.go b/pkg/appdir/appset_directory.go index e9059371..17e03823 100644 --- a/pkg/appdir/appset_directory.go +++ b/pkg/appdir/appset_directory.go @@ -39,14 +39,14 @@ func (d *AppSetDirectory) Union(other *AppSetDirectory) *AppSetDirectory { return &join } -func (d *AppSetDirectory) ProcessApp(app v1alpha1.ApplicationSet) { +func (d *AppSetDirectory) ProcessAppSet(app v1alpha1.ApplicationSet) { appName := app.GetName() src := app.Spec.Template.Spec.GetSource() // common data srcPath := src.Path - d.AddApp(&app) + d.AddAppSet(&app) // handle extra helm paths if helm := src.Helm; helm != nil { @@ -62,13 +62,13 @@ func (d *AppSetDirectory) ProcessApp(app v1alpha1.ApplicationSet) { } } -// FindAppsBasedOnChangeList receives the modified file path and +// FindAppSetsBasedOnChangeList receives the modified file path and // returns the list of applications that are affected by the changes. // // e.g. changeList = ["/appset/httpdump/httpdump.yaml", "/app/testapp/values.yaml"] // if the changed file is application set file, return it. -func (d *AppSetDirectory) FindAppsBasedOnChangeList(changeList []string, repo *git.Repo) []v1alpha1.ApplicationSet { +func (d *AppSetDirectory) FindAppSetsBasedOnChangeList(changeList []string, repo *git.Repo) []v1alpha1.ApplicationSet { log.Debug().Str("type", "applicationsets").Msgf("checking %d changes", len(changeList)) appsSet := make(map[string]struct{}) @@ -123,7 +123,7 @@ func (d *AppSetDirectory) GetAppSets(filter func(stub v1alpha1.ApplicationSet) b return result } -func (d *AppSetDirectory) AddApp(appSet *v1alpha1.ApplicationSet) { +func (d *AppSetDirectory) AddAppSet(appSet *v1alpha1.ApplicationSet) { if _, exists := d.appSetsMap[appSet.GetName()]; exists { log.Info().Msgf("appset %s already exists", appSet.Name) return @@ -144,7 +144,7 @@ func (d *AppSetDirectory) AddFile(appName, path string) { d.appSetFiles[path] = append(d.appSetFiles[path], appName) } -func (d *AppSetDirectory) RemoveApp(app v1alpha1.ApplicationSet) { +func (d *AppSetDirectory) RemoveAppSet(app v1alpha1.ApplicationSet) { log.Debug(). Str("appName", app.Name). Msg("delete app") diff --git a/pkg/appdir/appset_directory_test.go b/pkg/appdir/appset_directory_test.go index b6aaf6d1..c69c9347 100644 --- a/pkg/appdir/appset_directory_test.go +++ b/pkg/appdir/appset_directory_test.go @@ -62,7 +62,7 @@ func TestAppSetDirectory_ProcessApp(t *testing.T) { appSetFiles: tt.input.appSetFiles, appSetsMap: tt.input.appSetsMap, } - d.ProcessApp(tt.args.app) + d.ProcessAppSet(tt.args.app) assert.Equal(t, tt.expected.appSetDirs, d.appSetDirs) }) } @@ -212,7 +212,7 @@ spec: t.Fatalf("failed to create tmp folder %s", fatalErr) } d := &AppSetDirectory{} - result := d.FindAppsBasedOnChangeList(tt.changeList, &git.Repo{Directory: tempDir}) + result := d.FindAppSetsBasedOnChangeList(tt.changeList, &git.Repo{Directory: tempDir}) assert.Equal(t, tt.expected, result) }) } diff --git a/pkg/appdir/vcstoargomap.go b/pkg/appdir/vcstoargomap.go index eeafc070..574cc9f8 100644 --- a/pkg/appdir/vcstoargomap.go +++ b/pkg/appdir/vcstoargomap.go @@ -132,8 +132,8 @@ func (v2a VcsToArgoMap) AddAppSet(app *v1alpha1.ApplicationSet) { return } - appDirectory := v2a.GetAppSetsInRepo(app.Spec.Template.Spec.GetSource().RepoURL) - appDirectory.ProcessApp(*app) + appSetDirectory := v2a.GetAppSetsInRepo(app.Spec.Template.Spec.GetSource().RepoURL) + appSetDirectory.ProcessAppSet(*app) } func (v2a VcsToArgoMap) UpdateAppSet(old *v1alpha1.ApplicationSet, new *v1alpha1.ApplicationSet) { @@ -143,10 +143,10 @@ func (v2a VcsToArgoMap) UpdateAppSet(old *v1alpha1.ApplicationSet, new *v1alpha1 } oldAppDirectory := v2a.GetAppSetsInRepo(old.Spec.Template.Spec.GetSource().RepoURL) - oldAppDirectory.RemoveApp(*old) + oldAppDirectory.RemoveAppSet(*old) - newAppDirectory := v2a.GetAppSetsInRepo(new.Spec.Template.Spec.GetSource().RepoURL) - newAppDirectory.ProcessApp(*new) + appSetDirectory := v2a.GetAppSetsInRepo(new.Spec.Template.Spec.GetSource().RepoURL) + appSetDirectory.ProcessAppSet(*new) } func (v2a VcsToArgoMap) DeleteAppSet(app *v1alpha1.ApplicationSet) { @@ -155,6 +155,6 @@ func (v2a VcsToArgoMap) DeleteAppSet(app *v1alpha1.ApplicationSet) { return } - oldAppDirectory := v2a.GetAppSetsInRepo(app.Spec.Template.Spec.GetSource().RepoURL) - oldAppDirectory.RemoveApp(*app) + appSetDirectory := v2a.GetAppSetsInRepo(app.Spec.Template.Spec.GetSource().RepoURL) + appSetDirectory.RemoveAppSet(*app) } diff --git a/pkg/appdir/vcstoargomap_test.go b/pkg/appdir/vcstoargomap_test.go index 95a5d088..49be275b 100644 --- a/pkg/appdir/vcstoargomap_test.go +++ b/pkg/appdir/vcstoargomap_test.go @@ -8,7 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// TestAddApp tests the AddApp method from the VcsToArgoMap type. +// TestAddApp tests the AddAppSet method from the VcsToArgoMap type. func TestAddApp(t *testing.T) { // Setup your mocks and expected calls here. @@ -26,7 +26,7 @@ func TestAddApp(t *testing.T) { v2a.AddApp(app1) appDir := v2a.GetAppsInRepo("https://github.com/argoproj/argo-cd.git") - assert.Equal(t, appDir.Count(), 1) + assert.Equal(t, appDir.AppsCount(), 1) assert.Equal(t, len(appDir.appDirs["test-app-1"]), 1) // Assertions to verify the behavior here. @@ -41,7 +41,7 @@ func TestAddApp(t *testing.T) { } v2a.AddApp(app2) - assert.Equal(t, appDir.Count(), 2) + assert.Equal(t, appDir.AppsCount(), 2) assert.Equal(t, len(appDir.appDirs["test-app-2"]), 1) } @@ -73,12 +73,12 @@ func TestDeleteApp(t *testing.T) { v2a.AddApp(app2) appDir := v2a.GetAppsInRepo("https://github.com/argoproj/argo-cd.git") - assert.Equal(t, appDir.Count(), 2) + assert.Equal(t, appDir.AppsCount(), 2) assert.Equal(t, len(appDir.appDirs["test-app-1"]), 1) assert.Equal(t, len(appDir.appDirs["test-app-2"]), 1) v2a.DeleteApp(app2) - assert.Equal(t, appDir.Count(), 1) + assert.Equal(t, appDir.AppsCount(), 1) assert.Equal(t, len(appDir.appDirs["test-app-2"]), 0) } @@ -86,14 +86,13 @@ func TestVcsToArgoMap_AddAppSet(t *testing.T) { type args struct { app *v1alpha1.ApplicationSet } - tests := []struct { + tests := map[string]struct { name string fields VcsToArgoMap args args expectedCount int }{ - { - name: "normal process, expect to get the appset stored in the map", + "normal process, expect to get the appset stored in the map": { fields: NewVcsToArgoMap("dummyuser"), args: args{ app: &v1alpha1.ApplicationSet{ @@ -112,8 +111,7 @@ func TestVcsToArgoMap_AddAppSet(t *testing.T) { }, expectedCount: 1, }, - { - name: "invalid appset", + "invalid appset": { fields: NewVcsToArgoMap("vcs-username"), args: args{ app: &v1alpha1.ApplicationSet{ diff --git a/pkg/appdir/walk_kustomize_files.go b/pkg/appdir/walk_kustomize_files.go index 0ab819a8..b285a2bd 100644 --- a/pkg/appdir/walk_kustomize_files.go +++ b/pkg/appdir/walk_kustomize_files.go @@ -87,11 +87,11 @@ func walkKustomizeFiles(result *AppDirectory, fs fs.FS, appName, dirpath string) } if !stat.IsDir() { - result.AddFile(appName, relPath) + result.addFile(appName, relPath) continue } - result.AddDir(appName, relPath) + result.addDir(appName, relPath) if err = walkKustomizeFiles(result, fs, appName, relPath); err != nil { log.Warn().Err(err).Msgf("failed to read kustomize.yaml from resources in %s", relPath) } @@ -104,7 +104,7 @@ func walkKustomizeFiles(result *AppDirectory, fs fs.FS, appName, dirpath string) } relPath := filepath.Join(dirpath, basePath) - result.AddDir(appName, relPath) + result.addDir(appName, relPath) if err = walkKustomizeFiles(result, fs, appName, relPath); err != nil { log.Warn().Err(err).Msgf("failed to read kustomize.yaml from bases in %s", relPath) } @@ -112,12 +112,12 @@ func walkKustomizeFiles(result *AppDirectory, fs fs.FS, appName, dirpath string) for _, patchFile := range kustomize.PatchesStrategicMerge { relPath := filepath.Join(dirpath, patchFile) - result.AddFile(appName, relPath) + result.addFile(appName, relPath) } for _, patch := range kustomize.PatchesJson6902 { relPath := filepath.Join(dirpath, patch.Path) - result.AddFile(appName, relPath) + result.addFile(appName, relPath) } return nil diff --git a/pkg/events/check.go b/pkg/events/check.go index 94ea4e81..6a6cf433 100644 --- a/pkg/events/check.go +++ b/pkg/events/check.go @@ -58,7 +58,7 @@ type repoManager interface { Clone(ctx context.Context, cloneURL, branchName string) (*git.Repo, error) } -func GenerateMatcher(ce *CheckEvent, repo *git.Repo) error { +func generateMatcher(ce *CheckEvent, repo *git.Repo) error { log.Debug().Msg("using the argocd matcher") m, err := affected_apps.NewArgocdMatcher(ce.ctr.VcsToArgoMap, repo) if err != nil { @@ -265,7 +265,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error { } // Generate a list of affected apps, storing them within the CheckEvent (also returns but discarded here) - if err = ce.GenerateListOfAffectedApps(ctx, repo, ce.pullRequest.BaseRef, GenerateMatcher); err != nil { + if err = ce.GenerateListOfAffectedApps(ctx, repo, ce.pullRequest.BaseRef, generateMatcher); err != nil { return errors.Wrap(err, "failed to generate a list of affected apps") }