diff --git a/pkg/services/scm_provider/types.go b/pkg/services/scm_provider/types.go index b7f90bd8..eb8cf847 100644 --- a/pkg/services/scm_provider/types.go +++ b/pkg/services/scm_provider/types.go @@ -28,7 +28,39 @@ type Filter struct { PathsExist []string LabelMatch *regexp.Regexp BranchMatch *regexp.Regexp - FilterType FilterType +} + +// HasRepoFilter returns true if a Filter includes a filter which can exclude a repo based on knowledge about the repo +// only (no need for knowledge of any branches). A Filter with a "repo filter" may also include branch-specific filters. +func (f *Filter) HasRepoFilter() bool { + return f.RepositoryMatch != nil || f.LabelMatch != nil +} + +// HasBranchFilter returns true if a Filter includes a filter which can exclude a branch. +func (f *Filter) HasBranchFilter() bool { + return f.BranchMatch != nil || f.PathsExist != nil +} + +type Filters []*Filter + +func (f Filters) GetRepoFilters() Filters { + var repoFilters Filters + for _, filter := range f { + if filter.HasRepoFilter() { + repoFilters = append(repoFilters, filter) + } + } + return repoFilters +} + +func (f Filters) GetBranchFilters() Filters { + var branchFilters Filters + for _, filter := range f { + if filter.HasBranchFilter() { + branchFilters = append(branchFilters, filter) + } + } + return branchFilters } // A convenience type for indicating where to apply a filter diff --git a/pkg/services/scm_provider/utils.go b/pkg/services/scm_provider/utils.go index 07f29a59..3a280c6a 100644 --- a/pkg/services/scm_provider/utils.go +++ b/pkg/services/scm_provider/utils.go @@ -8,8 +8,8 @@ import ( argoprojiov1alpha1 "github.com/argoproj/applicationset/api/v1alpha1" ) -func compileFilters(filters []argoprojiov1alpha1.SCMProviderGeneratorFilter) ([]*Filter, error) { - outFilters := make([]*Filter, 0, len(filters)) +func compileFilters(filters []argoprojiov1alpha1.SCMProviderGeneratorFilter) (Filters, error) { + outFilters := make(Filters, 0, len(filters)) for _, filter := range filters { outFilter := &Filter{} var err error @@ -18,25 +18,21 @@ func compileFilters(filters []argoprojiov1alpha1.SCMProviderGeneratorFilter) ([] if err != nil { return nil, fmt.Errorf("error compiling RepositoryMatch regexp %q: %v", *filter.RepositoryMatch, err) } - outFilter.FilterType = FilterTypeRepo } if filter.LabelMatch != nil { outFilter.LabelMatch, err = regexp.Compile(*filter.LabelMatch) if err != nil { return nil, fmt.Errorf("error compiling LabelMatch regexp %q: %v", *filter.LabelMatch, err) } - outFilter.FilterType = FilterTypeRepo } if filter.PathsExist != nil { outFilter.PathsExist = filter.PathsExist - outFilter.FilterType = FilterTypeBranch } if filter.BranchMatch != nil { outFilter.BranchMatch, err = regexp.Compile(*filter.BranchMatch) if err != nil { return nil, fmt.Errorf("error compiling BranchMatch regexp %q: %v", *filter.LabelMatch, err) } - outFilter.FilterType = FilterTypeBranch } outFilters = append(outFilters, outFilter) } @@ -91,25 +87,21 @@ func ListRepos(ctx context.Context, provider SCMProviderService, filters []argop return nil, err } - repoFilters := getApplicableFilters(compiledFilters)[FilterTypeRepo] - if len(repoFilters) == 0 { - repos, err := getBranches(ctx, provider, repos, compiledFilters) - if err != nil { - return nil, err - } - return repos, nil - } - + repoFilters := compiledFilters.GetRepoFilters() filteredRepos := make([]*Repository, 0, len(repos)) - for _, repo := range repos { - for _, filter := range repoFilters { - matches, err := matchFilter(ctx, provider, repo, filter) - if err != nil { - return nil, err - } - if matches { - filteredRepos = append(filteredRepos, repo) - break + if len(repoFilters) == 0 { + filteredRepos = repos + } else { + for _, repo := range repos { + for _, filter := range repoFilters { + matches, err := matchFilter(ctx, provider, repo, filter) + if err != nil { + return nil, err + } + if matches { + filteredRepos = append(filteredRepos, repo) + break + } } } } @@ -121,7 +113,7 @@ func ListRepos(ctx context.Context, provider SCMProviderService, filters []argop return repos, nil } -func getBranches(ctx context.Context, provider SCMProviderService, repos []*Repository, compiledFilters []*Filter) ([]*Repository, error) { +func getBranches(ctx context.Context, provider SCMProviderService, repos []*Repository, compiledFilters Filters) ([]*Repository, error) { reposWithBranches := []*Repository{} for _, repo := range repos { reposFilled, err := provider.GetBranches(ctx, repo) @@ -130,7 +122,7 @@ func getBranches(ctx context.Context, provider SCMProviderService, repos []*Repo } reposWithBranches = append(reposWithBranches, reposFilled...) } - branchFilters := getApplicableFilters(compiledFilters)[FilterTypeBranch] + branchFilters := compiledFilters.GetBranchFilters() if len(branchFilters) == 0 { return reposWithBranches, nil } @@ -149,19 +141,3 @@ func getBranches(ctx context.Context, provider SCMProviderService, repos []*Repo } return filteredRepos, nil } - -// getApplicableFilters returns a map of filters separated by type. -func getApplicableFilters(filters []*Filter) map[FilterType][]*Filter { - filterMap := map[FilterType][]*Filter{ - FilterTypeBranch: {}, - FilterTypeRepo: {}, - } - for _, filter := range filters { - if filter.FilterType == FilterTypeBranch { - filterMap[FilterTypeBranch] = append(filterMap[FilterTypeBranch], filter) - } else if filter.FilterType == FilterTypeRepo { - filterMap[FilterTypeRepo] = append(filterMap[FilterTypeRepo], filter) - } - } - return filterMap -} diff --git a/pkg/services/scm_provider/utils_test.go b/pkg/services/scm_provider/utils_test.go index 1bf68e96..33a31bde 100644 --- a/pkg/services/scm_provider/utils_test.go +++ b/pkg/services/scm_provider/utils_test.go @@ -257,35 +257,36 @@ func TestNoFilters(t *testing.T) { assert.Equal(t, "three", repos[2].Repository) } -// tests the getApplicableFilters function, passing in all the filters, and an unset filter, plus an additional +// tests the filters segmentation functions, passing in all the filters, and an unset filter, plus an additional // branch filter func TestApplicableFilterMap(t *testing.T) { branchFilter := Filter{ BranchMatch: ®exp.Regexp{}, - FilterType: FilterTypeBranch, } repoFilter := Filter{ RepositoryMatch: ®exp.Regexp{}, - FilterType: FilterTypeRepo, } pathExistsFilter := Filter{ PathsExist: []string{"test"}, - FilterType: FilterTypeBranch, } labelMatchFilter := Filter{ LabelMatch: ®exp.Regexp{}, - FilterType: FilterTypeRepo, } unsetFilter := Filter{ LabelMatch: ®exp.Regexp{}, } additionalBranchFilter := Filter{ BranchMatch: ®exp.Regexp{}, - FilterType: FilterTypeBranch, } - filterMap := getApplicableFilters([]*Filter{&branchFilter, &repoFilter, - &pathExistsFilter, &labelMatchFilter, &unsetFilter, &additionalBranchFilter}) + bothFilter := Filter{ + RepositoryMatch: ®exp.Regexp{}, + PathsExist: []string{"test"}, + } + filters := Filters{&branchFilter, &repoFilter, + &pathExistsFilter, &labelMatchFilter, &unsetFilter, &additionalBranchFilter, &bothFilter} + repoFilters := filters.GetRepoFilters() + branchFilters := filters.GetBranchFilters() - assert.Len(t, filterMap[FilterTypeRepo], 2) - assert.Len(t, filterMap[FilterTypeBranch], 3) + assert.Len(t, repoFilters, 4) + assert.Len(t, branchFilters, 4) }