support non-default branches in apps (#89)

pkg/affected_apps/argocd_matcher.go

@@ -53,12 +53,12 @@ func getArgocdApps(vcsToArgoMap config.VcsToArgoMap, repo *repo.Repo) *config.Ap
 	return repoApps
 }
 
-func (a *ArgocdMatcher) AffectedApps(ctx context.Context, changeList []string) (AffectedItems, error) {
+func (a *ArgocdMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
 	if a.appsDirectory == nil {
 		return AffectedItems{}, nil
 	}
 
-	appsSlice := a.appsDirectory.FindAppsBasedOnChangeList(changeList)
+	appsSlice := a.appsDirectory.FindAppsBasedOnChangeList(changeList, targetBranch)
 	return AffectedItems{Applications: appsSlice}, nil
 }
 

pkg/affected_apps/argocd_matcher_test.go

@@ -35,7 +35,7 @@ func TestFindAffectedAppsWithNilAppsDirectory(t *testing.T) {
 	)
 
 	matcher := ArgocdMatcher{}
-	items, err := matcher.AffectedApps(ctx, changeList)
+	items, err := matcher.AffectedApps(ctx, changeList, "main")
 
 	// verify results
 	require.NoError(t, err)

pkg/affected_apps/best_effort.go

@@ -25,7 +25,7 @@ func NewBestEffortMatcher(repoName string, repoFileList []string) *BestEffort {
 	}
 }
 
-func (b *BestEffort) AffectedApps(_ context.Context, changeList []string) (AffectedItems, error) {
+func (b *BestEffort) AffectedApps(_ context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
 	appsMap := make(map[string]string)
 
 	for _, file := range changeList {

pkg/affected_apps/best_effort_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
 	"github.com/zapier/kubechecks/pkg/config"
 )
 
@@ -158,7 +159,7 @@ func TestBestEffortMatcher(t *testing.T) {
 			var err error
 
 			matcher := NewBestEffortMatcher(tt.args.repoName, testRepoFiles)
-			got, err = matcher.AffectedApps(context.TODO(), tt.args.fileList)
+			got, err = matcher.AffectedApps(context.TODO(), tt.args.fileList, "master")
 			require.NoError(t, err)
 
 			assert.Equal(t, len(tt.want.Applications), len(got.Applications))

pkg/affected_apps/config_matcher.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/rs/zerolog/log"
+
 	"github.com/zapier/kubechecks/pkg/argo_client"
 	"github.com/zapier/kubechecks/pkg/config"
 	"github.com/zapier/kubechecks/pkg/repo_config"
@@ -22,7 +23,7 @@ func NewConfigMatcher(cfg *repo_config.Config) *ConfigMatcher {
 	return &ConfigMatcher{cfg: cfg, argoClient: argoClient}
 }
 
-func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string) (AffectedItems, error) {
+func (b *ConfigMatcher) AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error) {
 	appsMap := make(map[string]string)
 	var appSetList []ApplicationSet
 

pkg/affected_apps/matcher.go

@@ -17,7 +17,7 @@ type ApplicationSet struct {
 }
 
 type Matcher interface {
-	AffectedApps(ctx context.Context, changeList []string) (AffectedItems, error)
+	AffectedApps(ctx context.Context, changeList []string, targetBranch string) (AffectedItems, error)
 }
 
 // modifiedDirs filters a list of changed files down to a list

pkg/config/app_directory.go

@@ -9,7 +9,7 @@ import (
 )
 
 type ApplicationStub struct {
-	Name, Path string
+	Name, Path, TargetRevision string
 
 	IsHelm, IsKustomize bool
 }
@@ -51,7 +51,7 @@ func (d *AppDirectory) ProcessApp(app v1alpha1.Application) {
 
 	// common data
 	srcPath := src.Path
-	d.AddAppStub(appName, srcPath, src.IsHelm(), !src.Kustomize.IsZero())
+	d.AddAppStub(appName, srcPath, app.Spec.Source.TargetRevision, src.IsHelm(), !src.Kustomize.IsZero())
 
 	// handle extra helm paths
 	if helm := src.Helm; helm != nil {
@@ -67,10 +67,9 @@ func (d *AppDirectory) ProcessApp(app v1alpha1.Application) {
 	}
 }
 
-func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string) []ApplicationStub {
+func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBranch string) []ApplicationStub {
 	log.Debug().Msgf("checking %d changes", len(changeList))
 
-	appsMap := make(map[string]string)
 	appsSet := make(map[string]struct{})
 	for _, changePath := range changeList {
 		log.Debug().Msgf("change: %s", changePath)
@@ -101,13 +100,41 @@ func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string) []Applicat
 			log.Warn().Msgf("failed to find matched app named '%s'", appName)
 			continue
 		}
+
+		if !shouldInclude(app, targetBranch) {
+			log.Debug().Msgf("target revision of %s is %s and does not match '%s'", appName, app.TargetRevision, targetBranch)
+			continue
+		}
+
 		appsSlice = append(appsSlice, app)
 	}
 
-	log.Debug().Msgf("matched %d files into %d apps", len(appsMap), len(appsSet))
+	log.Debug().Msgf("matched %d files into %d apps", len(changeList), len(appsSet))
 	return appsSlice
 }
 
+func shouldInclude(app ApplicationStub, targetBranch string) bool {
+	if app.TargetRevision == "" {
+		return true
+	}
+
+	if app.TargetRevision == targetBranch {
+		return true
+	}
+
+	if app.TargetRevision == "HEAD" {
+		if targetBranch == "main" {
+			return true
+		}
+
+		if targetBranch == "master" {
+			return true
+		}
+	}
+
+	return false
+}
+
 func (d *AppDirectory) GetApps(filter func(stub ApplicationStub) bool) []ApplicationStub {
 	var result []ApplicationStub
 	for _, value := range d.appsMap {
@@ -119,12 +146,13 @@ func (d *AppDirectory) GetApps(filter func(stub ApplicationStub) bool) []Applica
 	return result
 }
 
-func (d *AppDirectory) AddAppStub(appName, srcPath string, isHelm, isKustomize bool) {
+func (d *AppDirectory) AddAppStub(appName, srcPath, targetRevision string, isHelm, isKustomize bool) {
 	d.appsMap[appName] = ApplicationStub{
-		Name:        appName,
-		Path:        srcPath,
-		IsHelm:      isHelm,
-		IsKustomize: isKustomize,
+		Name:           appName,
+		TargetRevision: targetRevision,
+		Path:           srcPath,
+		IsHelm:         isHelm,
+		IsKustomize:    isKustomize,
 	}
 	d.AddDir(appName, srcPath)
 }

pkg/config/app_directory_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
@@ -49,3 +50,49 @@ func TestPathsAreJoinedProperly(t *testing.T) {
 		"/test1/three.yaml":     {"test-app"},
 	}, rad.appFiles)
 }
+
+func TestShouldInclude(t *testing.T) {
+	testcases := []struct {
+		vcsMergeTarget  string
+		argocdAppBranch string
+		expected        bool
+	}{
+		{
+			vcsMergeTarget:  "some-branch",
+			argocdAppBranch: "some-branch",
+			expected:        true,
+		},
+		{
+			vcsMergeTarget:  "some-branch",
+			argocdAppBranch: "some-other-branch",
+			expected:        false,
+		},
+		{
+			argocdAppBranch: "HEAD",
+			vcsMergeTarget:  "main",
+			expected:        true,
+		},
+		{
+			argocdAppBranch: "HEAD",
+			vcsMergeTarget:  "master",
+			expected:        true,
+		},
+		{
+			argocdAppBranch: "HEAD",
+			vcsMergeTarget:  "other",
+			expected:        false,
+		},
+		{
+			argocdAppBranch: "",
+			vcsMergeTarget:  "branch",
+			expected:        true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(fmt.Sprintf("%v", tc), func(t *testing.T) {
+			actual := shouldInclude(ApplicationStub{TargetRevision: tc.argocdAppBranch}, tc.vcsMergeTarget)
+			assert.Equal(t, tc.expected, actual)
+		})
+	}
+}

pkg/config/walk_kustomize_files_test.go

@@ -74,9 +74,9 @@ resources:
 	)
 
 	appdir := NewAppDirectory()
-	appdir.AddAppStub(kustomizeApp1Name, kustomizeApp1Path, false, true)
-	appdir.AddAppStub(kustomizeApp2Name, kustomizeApp2Path, false, true)
-	appdir.AddAppStub(kustomizeBaseName, kustomizeBasePath, false, true)
+	appdir.AddAppStub(kustomizeApp1Name, kustomizeApp1Path, "HEAD", false, true)
+	appdir.AddAppStub(kustomizeApp2Name, kustomizeApp2Path, "HEAD", false, true)
+	appdir.AddAppStub(kustomizeBaseName, kustomizeBasePath, "HEAD", false, true)
 
 	err = walkKustomizeFiles(appdir, fs, kustomizeApp1Name, kustomizeApp1Path)
 	require.NoError(t, err)

pkg/events/check.go

@@ -142,7 +142,7 @@ func (ce *CheckEvent) GetListOfChangedFiles(ctx context.Context) ([]string, erro
 }
 
 // Walks the repo to find any apps or appsets impacted by the changes in the MR/PR.
-func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context) error {
+func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context, targetBranch string) error {
 	_, span := otel.Tracer("Kubechecks").Start(ctx, "GenerateListOfAffectedApps")
 	defer span.End()
 	var err error
@@ -170,7 +170,7 @@ func (ce *CheckEvent) GenerateListOfAffectedApps(ctx context.Context) error {
 		}
 		matcher = affected_apps.NewBestEffortMatcher(ce.repo.Name, ce.repoFiles)
 	}
-	ce.affectedItems, err = matcher.AffectedApps(ctx, ce.fileList)
+	ce.affectedItems, err = matcher.AffectedApps(ctx, ce.fileList, targetBranch)
 	if err != nil {
 		telemetry.SetError(span, err, "Get Affected Apps")
 		ce.logger.Error().Err(err).Msg("could not get list of affected apps and appsets")

pkg/repo/repo.go

@@ -48,14 +48,14 @@ func (r *Repo) CloneRepoLocal(ctx context.Context, repoDir string) error {
 	// TODO: Look if this is still needed
 	r.RepoDir = repoDir
 
-	cmd := exec.Command("git", "clone", r.CloneURL, repoDir)
+	cmd := execCommand("git", "clone", r.CloneURL, repoDir)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		log.Error().Err(err).Msgf("unable to clone repository, %s", out)
 		return err
 	}
 
-	cmd = exec.Command("git", "remote")
+	cmd = execCommand("git", "remote")
 	cmd.Dir = repoDir
 	pipe, _ := cmd.StdoutPipe()
 	var wg sync.WaitGroup
@@ -109,16 +109,8 @@ func (r *Repo) MergeIntoTarget(ctx context.Context) error {
 		))
 	defer span.End()
 
-	if r.DefaultBranch != "" {
-		if r.BaseRef != r.DefaultBranch {
-			err := fmt.Errorf("target branch (%s) is not default branch (%s)\nfor kubechecks to run target branch must be default branch", r.HeadRef, r.DefaultBranch)
-			telemetry.SetError(span, err, "MergeIntoTarget")
-			return err
-		}
-	}
-
 	log.Debug().Msgf("Merging MR commit %s into a tmp branch off of %s for manifest generation...", r.SHA, r.BaseRef)
-	cmd := exec.Command("git", "fetch", r.Remote, r.BaseRef)
+	cmd := execCommand("git", "fetch", r.Remote, r.BaseRef)
 	cmd.Dir = r.RepoDir
 	err := cmd.Run()
 	if err != nil {
@@ -127,7 +119,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context) error {
 		return err
 	}
 
-	cmd = exec.Command("git", "checkout", "-b", "tmp", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef))
+	cmd = execCommand("git", "checkout", "-b", "tmp", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef))
 	cmd.Dir = r.RepoDir
 	_, err = cmd.Output()
 	if err != nil {
@@ -136,7 +128,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context) error {
 		return err
 	}
 
-	cmd = exec.Command("git", "merge", r.SHA)
+	cmd = execCommand("git", "merge", r.SHA)
 	cmd.Dir = r.RepoDir
 	out, err := cmd.CombinedOutput()
 	if err != nil {
@@ -180,7 +172,7 @@ func (r *Repo) GetListOfChangedFiles(ctx context.Context) ([]string, error) {
 
 	var fileList = []string{}
 
-	cmd := exec.Command("git", "diff", "--name-only", r.BaseRef)
+	cmd := execCommand("git", "diff", "--name-only", fmt.Sprintf("%s/%s", r.Remote, r.BaseRef))
 	cmd.Dir = r.RepoDir
 	pipe, _ := cmd.StdoutPipe()
 	var wg sync.WaitGroup
@@ -220,13 +212,13 @@ func walk(s string, d fs.DirEntry, err error) error {
 
 // InitializeGitSettings ensures Git auth is set up for cloning
 func InitializeGitSettings(user string, email string) error {
-	cmd := exec.Command("git", "config", "--global", "user.email", email)
+	cmd := execCommand("git", "config", "--global", "user.email", email)
 	err := cmd.Run()
 	if err != nil {
 		return errors.Wrap(err, "failed to set git email address")
 	}
 
-	cmd = exec.Command("git", "config", "--global", "user.name", user)
+	cmd = execCommand("git", "config", "--global", "user.name", user)
 	err = cmd.Run()
 	if err != nil {
 		return errors.Wrap(err, "failed to set git user name")
@@ -249,14 +241,14 @@ func InitializeGitSettings(user string, email string) error {
 	}
 	defer outfile.Close()
 
-	cmd = exec.Command("echo", cloneUrl)
+	cmd = execCommand("echo", cloneUrl)
 	cmd.Stdout = outfile
 	err = cmd.Run()
 	if err != nil {
 		return errors.Wrap(err, "unable to set git credentials")
 	}
 
-	cmd = exec.Command("git", "config", "--global", "credential.helper", "store")
+	cmd = execCommand("git", "config", "--global", "credential.helper", "store")
 	err = cmd.Run()
 	if err != nil {
 		return errors.Wrap(err, "unable to set git credential usage")
@@ -287,3 +279,8 @@ func getCloneUrl(user string, cfg *viper.Viper) (string, error) {
 	}
 	return fmt.Sprintf("%s://%s:%s@%s", scheme, user, vcsToken, hostname), nil
 }
+
+func execCommand(name string, args ...string) *exec.Cmd {
+	log.Debug().Strs("args", args).Msg("building command")
+	return exec.Command(name, args...)
+}

pkg/server/hook_handler.go

@@ -165,21 +165,24 @@ func (h *VCSHookHandler) processCheckEvent(ctx context.Context, repo *repo.Repo)
 	err = cEvent.MergeIntoTarget(ctx)
 	if err != nil {
 		// TODO: Cancel if gitlab etc
+		log.Error().Err(err).Msg("failed to merge into target")
 		return
 	}
 
 	// Get the diff between the two branches, storing them within the CheckEvent (also returns but discarded here)
 	_, err = cEvent.GetListOfChangedFiles(ctx)
 	if err != nil {
 		// TODO: Cancel if gitlab etc
+		log.Error().Err(err).Msg("failed to get list of changed files")
 		return
 	}
 
 	// Generate a list of affected apps, storing them within the CheckEvent (also returns but discarded here)
-	err = cEvent.GenerateListOfAffectedApps(ctx)
+	err = cEvent.GenerateListOfAffectedApps(ctx, repo.BaseRef)
 	if err != nil {
 		// TODO: Cancel if gitlab etc
 		//mEvent.CancelEvent(ctx, err, "Generate List of Affected Apps")
+		log.Error().Err(err).Msg("failed to generate a list of affected apps")
 		return
 	}