Skip to content

Commit

Permalink
fix scmProvider generator filters
Browse files Browse the repository at this point in the history
Signed-off-by: Jedrzej Kotkowski <[email protected]>
  • Loading branch information
jjsiv committed Sep 17, 2023
1 parent 21672a2 commit da433ae
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 117 deletions.
15 changes: 4 additions & 11 deletions applicationset/services/scm_provider/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,8 @@ type Filter struct {
PathsDoNotExist []string
LabelMatch *regexp.Regexp
BranchMatch *regexp.Regexp
FilterType FilterType
// FilterTypeRepo are filters that apply to the repo itself (name, labels)
FilterTypeRepo bool
// FilterTypeBranch are filters that apply to the repo content (path, branch)
FilterTypeBranch bool
}

// A convenience type for indicating where to apply a filter
type FilterType int64

// The enum of filter types
const (
FilterTypeUndefined FilterType = iota
FilterTypeBranch
FilterTypeRepo
)
118 changes: 53 additions & 65 deletions applicationset/services/scm_provider/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,42 +19,38 @@ func compileFilters(filters []argoprojiov1alpha1.SCMProviderGeneratorFilter) ([]
if err != nil {
return nil, fmt.Errorf("error compiling RepositoryMatch regexp %q: %v", *filter.RepositoryMatch, err)
}
outFilter.FilterType = FilterTypeRepo
outFilter.FilterTypeRepo = true
}
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
outFilter.FilterTypeRepo = true
}
if filter.PathsExist != nil {
outFilter.PathsExist = filter.PathsExist
outFilter.FilterType = FilterTypeBranch
outFilter.FilterTypeBranch = true
}
if filter.PathsDoNotExist != nil {
outFilter.PathsDoNotExist = filter.PathsDoNotExist
outFilter.FilterType = FilterTypeBranch
outFilter.FilterTypeBranch = true
}
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.BranchMatch, err)
}
outFilter.FilterType = FilterTypeBranch
outFilter.FilterTypeBranch = true
}
outFilters = append(outFilters, outFilter)
}
return outFilters, nil
}

func matchFilter(ctx context.Context, provider SCMProviderService, repo *Repository, filter *Filter) (bool, error) {
func matchRepoFilter(ctx context.Context, provider SCMProviderService, repo *Repository, filter *Filter) bool {
if filter.RepositoryMatch != nil && !filter.RepositoryMatch.MatchString(repo.Repository) {
return false, nil
}

if filter.BranchMatch != nil && !filter.BranchMatch.MatchString(repo.Branch) {
return false, nil
return false
}

if filter.LabelMatch != nil {
Expand All @@ -66,10 +62,18 @@ func matchFilter(ctx context.Context, provider SCMProviderService, repo *Reposit
}
}
if !found {
return false, nil
return false
}
}

return true
}

func matchBranchFilter(ctx context.Context, provider SCMProviderService, repo *Repository, filter *Filter) (bool, error) {
if filter.BranchMatch != nil && !filter.BranchMatch.MatchString(repo.Branch) {
return false, nil
}

if len(filter.PathsExist) != 0 {
for _, path := range filter.PathsExist {
path = strings.TrimRight(path, "/")
Expand Down Expand Up @@ -107,76 +111,60 @@ func ListRepos(ctx context.Context, provider SCMProviderService, filters []argop
if err != nil {
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
}
filteredRepos := make([]*Repository, 0, len(repos))

filledRepos := make([]*Repository, 0, len(repos))
for _, repo := range repos {
for _, filter := range repoFilters {
matches, err := matchFilter(ctx, provider, repo, filter)

if len(compiledFilters) > 0 {
for _, filter := range compiledFilters {
if filter.FilterTypeRepo {
if matches := matchRepoFilter(ctx, provider, repo, filter); !matches {
continue
}
}

filteredRepoWithBranches, err := getBranches(ctx, provider, repo, filter)
if err != nil {
return nil, err
}

filledRepos = append(filledRepos, filteredRepoWithBranches...)
}
} else {
repoWithBranches, err := getBranches(ctx, provider, repo, nil)
if err != nil {
return nil, err
}
if matches {
filteredRepos = append(filteredRepos, repo)
break
}

filledRepos = append(filledRepos, repoWithBranches...)
}
}

repos, err = getBranches(ctx, provider, filteredRepos, compiledFilters)
return filledRepos, nil
}

func getBranches(ctx context.Context, provider SCMProviderService, repo *Repository, filter *Filter) ([]*Repository, error) {
repoWithBranches := []*Repository{}

repoFilled, err := provider.GetBranches(ctx, repo)
if err != nil {
return nil, err
}
return repos, nil
}
repoWithBranches = append(repoWithBranches, repoFilled...)

func getBranches(ctx context.Context, provider SCMProviderService, repos []*Repository, compiledFilters []*Filter) ([]*Repository, error) {
reposWithBranches := []*Repository{}
for _, repo := range repos {
reposFilled, err := provider.GetBranches(ctx, repo)
if err != nil {
return nil, err
}
reposWithBranches = append(reposWithBranches, reposFilled...)
}
branchFilters := getApplicableFilters(compiledFilters)[FilterTypeBranch]
if len(branchFilters) == 0 {
return reposWithBranches, nil
}
filteredRepos := make([]*Repository, 0, len(reposWithBranches))
for _, repo := range reposWithBranches {
for _, filter := range branchFilters {
matches, err := matchFilter(ctx, provider, repo, filter)
filteredRepoWithBranches := make([]*Repository, 0, len(repoWithBranches))
if filter != nil && filter.FilterTypeBranch {
for _, repoBranch := range repoWithBranches {
matches, err := matchBranchFilter(ctx, provider, repoBranch, filter)
if err != nil {
return nil, err
}
if matches {
filteredRepos = append(filteredRepos, repo)
break
filteredRepoWithBranches = append(filteredRepoWithBranches, repoBranch)
}
}
return filteredRepoWithBranches, nil
}
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
return repoWithBranches, nil
}
60 changes: 19 additions & 41 deletions applicationset/services/scm_provider/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package scm_provider

import (
"context"
"regexp"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -213,6 +212,7 @@ func TestMultiFilterAnd(t *testing.T) {
filters := []argoprojiov1alpha1.SCMProviderGeneratorFilter{
{
RepositoryMatch: strp("w"),
PathsExist: []string{"two"},
LabelMatch: strp("^prod-.*$"),
},
}
Expand All @@ -228,31 +228,46 @@ func TestMultiFilterOr(t *testing.T) {
{
Repository: "one",
Labels: []string{"prod-one", "prod-two", "staging"},
Branch: "devel",
},
{
Repository: "two",
Labels: []string{"prod-two"},
Labels: []string{"prod-one", "prod-two", "staging"},
Branch: "feature/ABC",
},
{
Repository: "two",
Labels: []string{"prod-one", "prod-two", "staging"},
Branch: "main",
},
{
Repository: "three",
Labels: []string{"staging"},
Branch: "feature/XYZ",
},
{
Repository: "four",
Labels: []string{"dev"},
Branch: "feature/IDK",
},
},
}
filters := []argoprojiov1alpha1.SCMProviderGeneratorFilter{
{
RepositoryMatch: strp("e"),
BranchMatch: strp("feature/.*"),
},
{
LabelMatch: strp("^prod-.*$"),
},
}
repos, err := ListRepos(context.Background(), provider, filters, "")
assert.Nil(t, err)
assert.Len(t, repos, 3)
assert.Len(t, repos, 4)
assert.Equal(t, "one", repos[0].Repository)
assert.Equal(t, "two", repos[1].Repository)
assert.Equal(t, "three", repos[2].Repository)
assert.Equal(t, "two", repos[2].Repository)
assert.Equal(t, "three", repos[3].Repository)
}

func TestNoFilters(t *testing.T) {
Expand Down Expand Up @@ -280,40 +295,3 @@ func TestNoFilters(t *testing.T) {
assert.Equal(t, "two", repos[1].Repository)
assert.Equal(t, "three", repos[2].Repository)
}

// tests the getApplicableFilters function, passing in all the filters, and an unset filter, plus an additional
// branch filter
func TestApplicableFilterMap(t *testing.T) {
branchFilter := Filter{
BranchMatch: &regexp.Regexp{},
FilterType: FilterTypeBranch,
}
repoFilter := Filter{
RepositoryMatch: &regexp.Regexp{},
FilterType: FilterTypeRepo,
}
pathExistsFilter := Filter{
PathsExist: []string{"test"},
FilterType: FilterTypeBranch,
}
pathDoesntExistsFilter := Filter{
PathsDoNotExist: []string{"test"},
FilterType: FilterTypeBranch,
}
labelMatchFilter := Filter{
LabelMatch: &regexp.Regexp{},
FilterType: FilterTypeRepo,
}
unsetFilter := Filter{
LabelMatch: &regexp.Regexp{},
}
additionalBranchFilter := Filter{
BranchMatch: &regexp.Regexp{},
FilterType: FilterTypeBranch,
}
filterMap := getApplicableFilters([]*Filter{&branchFilter, &repoFilter,
&pathExistsFilter, &labelMatchFilter, &unsetFilter, &additionalBranchFilter, &pathDoesntExistsFilter})

assert.Len(t, filterMap[FilterTypeRepo], 2)
assert.Len(t, filterMap[FilterTypeBranch], 4)
}

0 comments on commit da433ae

Please sign in to comment.