From cfa26ecc250208bb06e53a2fde217e8766e3d06b Mon Sep 17 00:00:00 2001 From: Petr Muller Date: Thu, 5 Aug 2021 14:05:16 +0200 Subject: [PATCH] Respect stricter feature branch names in config resolver --- pkg/load/agents/configAgent.go | 17 +++++---- pkg/load/agents/configAgent_test.go | 54 ++++++++++++++++++++++------- pkg/prowgen/prowgen.go | 12 +++---- 3 files changed, 58 insertions(+), 25 deletions(-) diff --git a/pkg/load/agents/configAgent.go b/pkg/load/agents/configAgent.go index aa8a4455c64..7e8e3084f9b 100644 --- a/pkg/load/agents/configAgent.go +++ b/pkg/load/agents/configAgent.go @@ -12,6 +12,7 @@ import ( "github.com/openshift/ci-tools/pkg/api" "github.com/openshift/ci-tools/pkg/load" + "github.com/openshift/ci-tools/pkg/prowgen" ) type IndexDelta struct { @@ -134,13 +135,17 @@ func (a *configAgent) GetMatchingConfig(metadata api.Metadata) (api.ReleaseBuild } var matchingConfigs []api.ReleaseBuildConfiguration for _, config := range repoConfigs { - r, err := regexp.Compile(config.Metadata.Branch) - if err != nil { - return api.ReleaseBuildConfiguration{}, fmt.Errorf("could not compile regex for %s/%s@%s: %w", metadata.Org, metadata.Repo, config.Metadata.Branch, err) - } - if r.MatchString(metadata.Branch) && config.Metadata.Variant == metadata.Variant { - matchingConfigs = append(matchingConfigs, config) + for _, f := range []func(string) string{prowgen.ExactlyBranch, prowgen.FeatureBranch} { + r, err := regexp.Compile(f(config.Metadata.Branch)) + if err != nil { + return api.ReleaseBuildConfiguration{}, fmt.Errorf("could not compile regex for %s/%s@%s: %w", metadata.Org, metadata.Repo, config.Metadata.Branch, err) + } + if r.MatchString(metadata.Branch) && config.Metadata.Variant == metadata.Variant { + matchingConfigs = append(matchingConfigs, config) + break + } } + } switch len(matchingConfigs) { case 0: diff --git a/pkg/load/agents/configAgent_test.go b/pkg/load/agents/configAgent_test.go index 69377e78b54..de976c02d26 100644 --- a/pkg/load/agents/configAgent_test.go +++ b/pkg/load/agents/configAgent_test.go @@ -330,32 +330,60 @@ func TestConfigAgent_GetMatchingConfig(t *testing.T) { }}, {Metadata: api.Metadata{ Org: "org", Repo: "repo", - Branch: "branch-fo", + Branch: "branch-foo", }}}, }, }, meta: api.Metadata{ Org: "org", Repo: "repo", - Branch: "branch-foo", + Branch: "branch-foo-bar", }, expectedErr: true, }, + { + name: "no error on simple substring", + input: load.ByOrgRepo{ + "org": map[string][]api.ReleaseBuildConfiguration{ + "repo": {{Metadata: api.Metadata{ + Org: "org", + Repo: "repo", + Branch: "release-4.1", + }}, {Metadata: api.Metadata{ + Org: "org", + Repo: "repo", + Branch: "release-4.10", + }}}, + }, + }, + meta: api.Metadata{ + Org: "org", + Repo: "repo", + Branch: "release-4.10", + }, + expected: api.ReleaseBuildConfiguration{Metadata: api.Metadata{ + Org: "org", + Repo: "repo", + Branch: "release-4.10", + }}, + }, } for _, testCase := range testCases { - agent := &configAgent{lock: &sync.RWMutex{}, configs: testCase.input} - actual, actualErr := agent.GetMatchingConfig(testCase.meta) - if testCase.expectedErr && actualErr == nil { - t.Errorf("%s: expected an error but got none", testCase.name) - } - if !testCase.expectedErr && actualErr != nil { - t.Errorf("%s: expected no error but got one: %v", testCase.name, actualErr) - } + t.Run(testCase.name, func(t *testing.T) { + agent := &configAgent{lock: &sync.RWMutex{}, configs: testCase.input} + actual, actualErr := agent.GetMatchingConfig(testCase.meta) + if testCase.expectedErr && actualErr == nil { + t.Errorf("%s: expected an error but got none", testCase.name) + } + if !testCase.expectedErr && actualErr != nil { + t.Errorf("%s: expected no error but got one: %v", testCase.name, actualErr) + } - if diff := cmp.Diff(actual, testCase.expected); diff != "" { - t.Errorf("%s: got incorrect config: %v", testCase.name, diff) - } + if diff := cmp.Diff(actual, testCase.expected); !testCase.expectedErr && diff != "" { + t.Errorf("%s: got incorrect config: %v", testCase.name, diff) + } + }) } } diff --git a/pkg/prowgen/prowgen.go b/pkg/prowgen/prowgen.go index 9af37574209..8a0690ce3fe 100644 --- a/pkg/prowgen/prowgen.go +++ b/pkg/prowgen/prowgen.go @@ -445,7 +445,7 @@ func generatePresubmitForTest(name string, info *ProwgenInfo, podSpec *corev1.Po return &prowconfig.Presubmit{ JobBase: base, AlwaysRun: true, - Brancher: prowconfig.Brancher{Branches: sets.NewString(exactlyBranch(info.Branch), featureBranch(info.Branch)).List()}, + Brancher: prowconfig.Brancher{Branches: sets.NewString(ExactlyBranch(info.Branch), FeatureBranch(info.Branch)).List()}, Reporter: prowconfig.Reporter{ Context: fmt.Sprintf("ci/prow/%s", shortName), }, @@ -462,7 +462,7 @@ func generatePostsubmitForTest(name string, info *ProwgenInfo, podSpec *corev1.P base := generateJobBase(name, jc.PostsubmitPrefix, info, podSpec, false, pathAlias, jobRelease, skipCloning) return &prowconfig.Postsubmit{ JobBase: base, - Brancher: prowconfig.Brancher{Branches: []string{exactlyBranch(info.Branch)}}, + Brancher: prowconfig.Brancher{Branches: []string{ExactlyBranch(info.Branch)}}, } } @@ -618,19 +618,19 @@ func generateJobBase(name, prefix string, info *ProwgenInfo, podSpec *corev1.Pod return base } -// exactlyBranch returns a regex string that matches exactly the given branch name: I.e. returns +// ExactlyBranch returns a regex string that matches exactly the given branch name: I.e. returns // '^master$' for 'master'. If the given branch name already looks like a regex, return it unchanged. -func exactlyBranch(branch string) string { +func ExactlyBranch(branch string) string { if !jc.SimpleBranchRegexp.MatchString(branch) { return branch } return fmt.Sprintf("^%s$", regexp.QuoteMeta(branch)) } -// featureBranch returns a regex string that matches feature branch prefixes for the given branch name: +// FeatureBranch returns a regex string that matches feature branch prefixes for the given branch name: // I.e. returns '^master-' for 'master'. If the given branch name already looks like a regex, // return it unchanged. -func featureBranch(branch string) string { +func FeatureBranch(branch string) string { if !jc.SimpleBranchRegexp.MatchString(branch) { return branch }