Fix: helm apps wouldn't diff if values file was outside of chart (#338)

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{

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
+}

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.

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)
 }
 

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,