Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support non-default branches in apps #89

Merged
merged 8 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/affected_apps/argocd_matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/affected_apps/argocd_matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/affected_apps/best_effort.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/affected_apps/best_effort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/zapier/kubechecks/pkg/config"
)

Expand Down Expand Up @@ -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))
Expand Down
3 changes: 2 additions & 1 deletion pkg/affected_apps/config_matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion pkg/affected_apps/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 38 additions & 10 deletions pkg/config/app_directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

type ApplicationStub struct {
Name, Path string
Name, Path, TargetRevision string

IsHelm, IsKustomize bool
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down
47 changes: 47 additions & 0 deletions pkg/config/app_directory_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"fmt"
"testing"

"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
Expand Down Expand Up @@ -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)
})
}
}
6 changes: 3 additions & 3 deletions pkg/config/walk_kustomize_files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/events/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
33 changes: 15 additions & 18 deletions pkg/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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...)
}
5 changes: 4 additions & 1 deletion pkg/server/hook_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down