Skip to content

Commit

Permalink
remove application set revision check. (#271)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Greyeye authored Sep 9, 2024
1 parent ebf376b commit e17621c
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 38 deletions.
2 changes: 1 addition & 1 deletion pkg/affected_apps/argocd_matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
36 changes: 2 additions & 34 deletions pkg/appdir/appset_directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -97,53 +97,21 @@ 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{}{}
appSets = append(appSets, *appSet)
}
}

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 {
Expand Down
4 changes: 1 addition & 3 deletions pkg/appdir/appset_directory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -180,7 +179,6 @@ spec:
changeList: []string{
"invalid-appset.yaml",
},
targetBranch: "main",
mockFiles: map[string]string{
"appsets/httpdump/invalid-appset.yaml": "invalid yaml content",
},
Expand Down Expand Up @@ -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)
})
}
Expand Down

0 comments on commit e17621c

Please sign in to comment.