From e17621c6d8431124dac83a0d12115e297b598f90 Mon Sep 17 00:00:00 2001 From: James Hong Date: Tue, 10 Sep 2024 09:57:51 +1000 Subject: [PATCH] remove application set revision check. (#271) applicationsets was checking the manifest was targeting main, however there are cases where targetRevision may be using template e.g. `{{ .values.target }}` This causes the check to fail and does not perform applicationset checks. This PR removes the check as this check is only required on the applications not applicationsets. --- pkg/affected_apps/argocd_matcher.go | 2 +- pkg/appdir/appset_directory.go | 36 ++--------------------------- pkg/appdir/appset_directory_test.go | 4 +--- 3 files changed, 4 insertions(+), 38 deletions(-) diff --git a/pkg/affected_apps/argocd_matcher.go b/pkg/affected_apps/argocd_matcher.go index cb0c132f..0c1b7ded 100644 --- a/pkg/affected_apps/argocd_matcher.go +++ b/pkg/affected_apps/argocd_matcher.go @@ -77,7 +77,7 @@ func (a *ArgocdMatcher) AffectedApps(_ context.Context, changeList []string, tar } appsSlice := a.appsDirectory.FindAppsBasedOnChangeList(changeList, targetBranch) - appSetsSlice := a.appSetsDirectory.FindAppsBasedOnChangeList(changeList, targetBranch, repo) + appSetsSlice := a.appSetsDirectory.FindAppsBasedOnChangeList(changeList, repo) // and return both apps and appSets return AffectedItems{ diff --git a/pkg/appdir/appset_directory.go b/pkg/appdir/appset_directory.go index 88eec123..e9059371 100644 --- a/pkg/appdir/appset_directory.go +++ b/pkg/appdir/appset_directory.go @@ -68,7 +68,7 @@ func (d *AppSetDirectory) ProcessApp(app v1alpha1.ApplicationSet) { // 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, targetBranch string, repo *git.Repo) []v1alpha1.ApplicationSet { +func (d *AppSetDirectory) FindAppsBasedOnChangeList(changeList []string, repo *git.Repo) []v1alpha1.ApplicationSet { log.Debug().Str("type", "applicationsets").Msgf("checking %d changes", len(changeList)) appsSet := make(map[string]struct{}) @@ -97,11 +97,6 @@ func (d *AppSetDirectory) FindAppsBasedOnChangeList(changeList []string, targetB continue } - if !appSetShouldInclude(appSet, targetBranch) { - log.Debug().Msgf("target revision of %s is %s and does not match '%s'", appSet, appSetGetTargetRevision(appSet), targetBranch) - continue - } - // Store the unique ApplicationSet if _, exists := appsSet[appSet.Name]; !exists { appsSet[appSet.Name] = struct{}{} @@ -109,41 +104,14 @@ func (d *AppSetDirectory) FindAppsBasedOnChangeList(changeList []string, targetB } } - log.Debug().Str("source", "appset_directory").Msgf("matched %d files into %d appset", len(changeList), len(appsSet)) + log.Debug().Str("source", "appset_directory").Msgf("matched %d files into %d appset", len(changeList), len(appSets)) return appSets } -func appSetGetTargetRevision(app *v1alpha1.ApplicationSet) string { - return app.Spec.Template.Spec.GetSource().TargetRevision -} - func appSetGetSourcePath(app *v1alpha1.ApplicationSet) string { return app.Spec.Template.Spec.GetSource().Path } -func appSetShouldInclude(app *v1alpha1.ApplicationSet, targetBranch string) bool { - targetRevision := appSetGetTargetRevision(app) - if targetRevision == "" { - return true - } - - if targetRevision == targetBranch { - return true - } - - if targetRevision == "HEAD" { - if targetBranch == "main" { - return true - } - - if targetBranch == "master" { - return true - } - } - - return false -} - func (d *AppSetDirectory) GetAppSets(filter func(stub v1alpha1.ApplicationSet) bool) []v1alpha1.ApplicationSet { var result []v1alpha1.ApplicationSet for _, value := range d.appSetsMap { diff --git a/pkg/appdir/appset_directory_test.go b/pkg/appdir/appset_directory_test.go index c2fa48c3..b6aaf6d1 100644 --- a/pkg/appdir/appset_directory_test.go +++ b/pkg/appdir/appset_directory_test.go @@ -82,7 +82,6 @@ func TestAppSetDirectory_FindAppsBasedOnChangeList(t *testing.T) { changeList: []string{ "appsets/httpdump/valid-appset.yaml", }, - targetBranch: "main", mockFiles: map[string]string{ "appsets/httpdump/valid-appset.yaml": ` apiVersion: argoproj.io/v1alpha1 @@ -180,7 +179,6 @@ spec: changeList: []string{ "invalid-appset.yaml", }, - targetBranch: "main", mockFiles: map[string]string{ "appsets/httpdump/invalid-appset.yaml": "invalid yaml content", }, @@ -214,7 +212,7 @@ spec: t.Fatalf("failed to create tmp folder %s", fatalErr) } d := &AppSetDirectory{} - result := d.FindAppsBasedOnChangeList(tt.changeList, tt.targetBranch, &git.Repo{Directory: tempDir}) + result := d.FindAppsBasedOnChangeList(tt.changeList, &git.Repo{Directory: tempDir}) assert.Equal(t, tt.expected, result) }) }