diff --git a/prow/cmd/deck/main_test.go b/prow/cmd/deck/main_test.go index e8382d524854..594a6b70094d 100644 --- a/prow/cmd/deck/main_test.go +++ b/prow/cmd/deck/main_test.go @@ -831,9 +831,8 @@ func TestHandlePluginConfig(t *testing.T) { }}, }, Blunderbuss: plugins.Blunderbuss{ - BlunderbussConfig: plugins.BlunderbussConfig{ - ExcludeApprovers: true, - }}, + ExcludeApprovers: true, + }, } pluginAgent := &plugins.ConfigAgent{} pluginAgent.Set(&c) diff --git a/prow/plugins/blunderbuss/blunderbuss.go b/prow/plugins/blunderbuss/blunderbuss.go index 329e6e1e9ba4..34d37b43e29a 100644 --- a/prow/plugins/blunderbuss/blunderbuss.go +++ b/prow/plugins/blunderbuss/blunderbuss.go @@ -64,43 +64,18 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel two := 2 yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{ Blunderbuss: plugins.Blunderbuss{ - BlunderbussConfig: plugins.BlunderbussConfig{ - ReviewerCount: &two, - MaxReviewerCount: 3, - ExcludeApprovers: true, - UseStatusAvailability: true, - IgnoreAuthors: []string{}, - }, - Orgs: map[string]plugins.BlunderbussOrgConfig{ - "": { - BlunderbussConfig: &plugins.BlunderbussConfig{ - ReviewerCount: &two, - MaxReviewerCount: 3, - ExcludeApprovers: true, - UseStatusAvailability: true, - IgnoreAuthors: []string{}, - }, - Repos: map[string]plugins.BlunderbussRepoConfig{ - "": { - BlunderbussConfig: plugins.BlunderbussConfig{ - ReviewerCount: &two, - MaxReviewerCount: 3, - ExcludeApprovers: true, - UseStatusAvailability: true, - IgnoreAuthors: []string{}, - }, - }, - }, - }, - }, + ReviewerCount: &two, + MaxReviewerCount: 3, + ExcludeApprovers: true, + UseStatusAvailability: true, + IgnoreAuthors: []string{}, }, }) if err != nil { logrus.WithError(err).Warnf("cannot generate comments for %s plugin", PluginName) } pluginHelp := &pluginhelp.PluginHelp{ - Description: "The blunderbuss plugin automatically requests reviews from reviewers when a new PR is created. " + - "The reviewers are selected based on the reviewers specified in the OWNERS files that apply to the files modified by the PR.", + Description: "The blunderbuss plugin automatically requests reviews from reviewers when a new PR is created. The reviewers are selected based on the reviewers specified in the OWNERS files that apply to the files modified by the PR.", Config: map[string]string{ "": configString(reviewCount), }, @@ -162,18 +137,17 @@ func handlePullRequestEvent(pc plugins.Agent, pre github.PullRequestEvent) error pc.GitHubClient, pc.OwnersClient, pc.Logger, - pc.PluginConfig.BlunderbussFor(pre.Repo.Owner.Login, pre.Repo.Name), + pc.PluginConfig.Blunderbuss, pre.Action, &pre.PullRequest, &pre.Repo, ) } -func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.BlunderbussConfig, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error { +func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.Blunderbuss, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error { if !(action == github.PullRequestActionOpened || action == github.PullRequestActionReadyForReview) || assign.CCRegexp.MatchString(pr.Body) { return nil } - if pr.Draft && config.IgnoreDrafts { // ignore Draft PR when IgnoreDrafts is true return nil @@ -202,7 +176,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) pc.GitHubClient, pc.OwnersClient, pc.Logger, - pc.PluginConfig.BlunderbussFor(ce.Repo.Owner.Login, ce.Repo.Name), + pc.PluginConfig.Blunderbuss, ce.Action, ce.IsPR, ce.Number, @@ -212,7 +186,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) ) } -func handleGenericComment(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.BlunderbussConfig, action github.GenericCommentEventAction, isPR bool, prNumber int, issueState string, repo *github.Repo, body string) error { +func handleGenericComment(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.Blunderbuss, action github.GenericCommentEventAction, isPR bool, prNumber int, issueState string, repo *github.Repo, body string) error { if action != github.GenericCommentActionCreated || !isPR || issueState == "closed" { return nil } diff --git a/prow/plugins/blunderbuss/blunderbuss_test.go b/prow/plugins/blunderbuss/blunderbuss_test.go index 171e629afe42..ecb582401ed5 100644 --- a/prow/plugins/blunderbuss/blunderbuss_test.go +++ b/prow/plugins/blunderbuss/blunderbuss_test.go @@ -635,7 +635,7 @@ func TestHandlePullRequest(t *testing.T) { pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}, Body: tc.body, Draft: tc.draft} repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"} fghc := newFakeGitHubClient(&pr, tc.filesChanged) - c := plugins.BlunderbussConfig{ + c := plugins.Blunderbuss{ ReviewerCount: &tc.reviewerCount, MaxReviewerCount: 0, ExcludeApprovers: false, @@ -659,70 +659,6 @@ func TestHandlePullRequest(t *testing.T) { } } -func TestHandlePullRequestShardedConfig(t *testing.T) { - froc := &fakeRepoownersClient{ - foc: &fakeOwnersClient{ - owners: map[string]string{ - "a.go": "1", - }, - leafReviewers: map[string]sets.String{ - "a.go": sets.NewString("al"), - }, - }, - } - - var tc = struct { - action github.PullRequestEventAction - body string - filesChanged []string - reviewerCount int - expectedRequested []string - draft bool - ignoreDrafts bool - ignoreAuthors []string - }{ - action: github.PullRequestActionOpened, - filesChanged: []string{"a.go"}, - draft: false, - ignoreDrafts: true, - reviewerCount: 1, - } - - pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}, Body: tc.body, Draft: tc.draft} - repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"} - fghc := newFakeGitHubClient(&pr, tc.filesChanged) - c := &plugins.Configuration{ - Blunderbuss: plugins.Blunderbuss{ - BlunderbussConfig: plugins.BlunderbussConfig{ - ReviewerCount: &tc.reviewerCount, - MaxReviewerCount: 0, - ExcludeApprovers: false, - IgnoreDrafts: tc.ignoreDrafts, - IgnoreAuthors: tc.ignoreAuthors, - }, - Orgs: map[string]plugins.BlunderbussOrgConfig{ - "org": { - Repos: map[string]plugins.BlunderbussRepoConfig{ - "org/repo": { - BlunderbussConfig: plugins.BlunderbussConfig{ - IgnoreAuthors: []string{"author"}, - }}}}}}} - bc := c.BlunderbussFor(repo.Owner.Login, repo.Name) - - if err := handlePullRequest( - fghc, froc, logrus.WithField("plugin", PluginName), - bc, tc.action, &pr, &repo, - ); err != nil { - t.Fatalf("unexpected error from handle: %v", err) - } - - sort.Strings(fghc.requested) - sort.Strings(tc.expectedRequested) - if !reflect.DeepEqual(fghc.requested, tc.expectedRequested) { - t.Fatalf("expected the requested reviewers to be %q, but got %q.", tc.expectedRequested, fghc.requested) - } -} - func TestHandleGenericComment(t *testing.T) { froc := &fakeRepoownersClient{ foc: &fakeOwnersClient{ @@ -796,7 +732,7 @@ func TestHandleGenericComment(t *testing.T) { pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}} fghc := newFakeGitHubClient(&pr, tc.filesChanged) repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"} - config := plugins.BlunderbussConfig{ + config := plugins.Blunderbuss{ ReviewerCount: &tc.reviewerCount, MaxReviewerCount: 0, ExcludeApprovers: false, @@ -818,108 +754,6 @@ func TestHandleGenericComment(t *testing.T) { } } -func TestHandleGenericCommentShardedConfig(t *testing.T) { - froc := &fakeRepoownersClient{ - foc: &fakeOwnersClient{ - owners: map[string]string{ - "a.go": "1", - "b.go": "2", - }, - leafReviewers: map[string]sets.String{ - "a.go": sets.NewString("al"), - "b.go": sets.NewString("bob"), - "c.go": sets.NewString("sarah"), - "d.go": sets.NewString("busy-user"), - }, - }, - } - - overrideOrgReviewerCount := 2 - overrideRepoReviewerCount := 3 - var testcases = []struct { - name string - orgConfig map[string]plugins.BlunderbussOrgConfig - expectedRequested int - }{ - { - name: "overrides default config with org config", - orgConfig: map[string]plugins.BlunderbussOrgConfig{ - "org": { - BlunderbussConfig: &plugins.BlunderbussConfig{ - ReviewerCount: &overrideOrgReviewerCount, - MaxReviewerCount: overrideOrgReviewerCount, - }}, - }, - expectedRequested: 2, - }, - { - name: "overrides default and org config with repo config", - orgConfig: map[string]plugins.BlunderbussOrgConfig{ - "org": { - BlunderbussConfig: &plugins.BlunderbussConfig{ - ReviewerCount: &overrideOrgReviewerCount, - MaxReviewerCount: overrideOrgReviewerCount, - }, - Repos: map[string]plugins.BlunderbussRepoConfig{ - "org/repo": { - BlunderbussConfig: plugins.BlunderbussConfig{ - ReviewerCount: &overrideRepoReviewerCount, - MaxReviewerCount: overrideRepoReviewerCount, - }}}}, - }, - expectedRequested: 3, - }, - { - name: "Uses org config with invalid repo config key", - orgConfig: map[string]plugins.BlunderbussOrgConfig{ - "org": { - BlunderbussConfig: &plugins.BlunderbussConfig{ - ReviewerCount: &overrideOrgReviewerCount, - MaxReviewerCount: overrideOrgReviewerCount, - }, - Repos: map[string]plugins.BlunderbussRepoConfig{ - "repo": { - BlunderbussConfig: plugins.BlunderbussConfig{ - ReviewerCount: &overrideRepoReviewerCount, - MaxReviewerCount: overrideRepoReviewerCount, - }}}}, - }, - expectedRequested: 2, - }, - } - - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}} - fghc := newFakeGitHubClient(&pr, []string{"a.go", "b.go", "c.go", "d.go"}) - defaultReviewerCount := 1 - repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"} - - config := &plugins.Configuration{ - Blunderbuss: plugins.Blunderbuss{ - BlunderbussConfig: plugins.BlunderbussConfig{ - IgnoreAuthors: []string{"bob"}, - ReviewerCount: &defaultReviewerCount, - UseStatusAvailability: false, - }, - Orgs: tc.orgConfig, - }} - bc := config.BlunderbussFor(repo.Owner.Login, repo.Name) - - if err := handleGenericComment( - fghc, froc, logrus.WithField("plugin", PluginName), bc, - github.GenericCommentActionCreated, true, pr.Number, "open", &repo, "/auto-cc", - ); err != nil { - t.Fatalf("unexpected error from handle: %v", err) - } - - if tc.expectedRequested != len(fghc.requested) { - t.Fatalf("expected the requested reviewers to be %d, but got %d.", tc.expectedRequested, len(fghc.requested)) - } - }) - } -} - func TestHandleGenericCommentEvent(t *testing.T) { pc := plugins.Agent{ PluginConfig: &plugins.Configuration{}, @@ -958,9 +792,8 @@ func TestHelpProvider(t *testing.T) { name: "ReviewerCount specified", config: &plugins.Configuration{ Blunderbuss: plugins.Blunderbuss{ - BlunderbussConfig: plugins.BlunderbussConfig{ - ReviewerCount: &[]int{2}[0], - }}, + ReviewerCount: &[]int{2}[0], + }, }, enabledRepos: enabledRepos, configInfoIncludes: []string{configString(2)}, diff --git a/prow/plugins/config.go b/prow/plugins/config.go index a2ca7b674aea..8025bc8d0fcc 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -145,8 +145,8 @@ type ExternalPlugin struct { Events []string `json:"events,omitempty"` } -// BlunderbussConfig defines configuration options for the blunderbuss plugin. -type BlunderbussConfig struct { +// Blunderbuss defines configuration for the blunderbuss plugin. +type Blunderbuss struct { // ReviewerCount is the minimum number of reviewers to request // reviews from. Defaults to requesting reviews from 2 reviewers ReviewerCount *int `json:"request_count,omitempty"` @@ -172,28 +172,6 @@ type BlunderbussConfig struct { IgnoreAuthors []string `json:"ignore_authors,omitempty"` } -// BlunderbussRepoConfig defines repository-specific configuration for the blunderbuss plugin. -type BlunderbussRepoConfig struct { - BlunderbussConfig `json:",inline,omitempty"` -} - -// BlunderbussOrgConfig defines organization-specific configuration for the blunderbuss plugin. -type BlunderbussOrgConfig struct { - *BlunderbussConfig `json:",inline,omitempty"` - // Repos allows sharding for repository-specific Blunderbuss settings, provided under keys using - // the format organization/repository. - Repos map[string]BlunderbussRepoConfig `json:"repos,omitempty"` -} - -// Blunderbuss defines overall configuration for the blunderbuss plugin. -type Blunderbuss struct { - BlunderbussConfig `json:",inline,omitempty"` - // Orgs allows sharding for organization-specific Blunderbuss settings, provided under keys with - // the name of the organization. For repository-specific settings, provide - // organization/repository keys under orgs[].repos. - Orgs map[string]BlunderbussOrgConfig `json:"orgs,omitempty"` -} - // Owners contains configuration related to handling OWNERS files. type Owners struct { // MDYAMLRepos is a list of org and org/repo strings specifying the repos that support YAML @@ -873,27 +851,6 @@ func (c *Configuration) ApproveFor(org, repo string) *Approve { return a } -// BlunderbussFor finds the BlunderbussConfig for an org or repo, if one exists. -// Blunderbuss configuration can be listed for a repository or an organization. -func (c *Configuration) BlunderbussFor(org, repo string) BlunderbussConfig { - fullName := fmt.Sprintf("%s/%s", org, repo) - - if orgConfig, ok := c.Blunderbuss.Orgs[org]; ok { - if repoConfig, ok := orgConfig.Repos[fullName]; ok { - // Return repo configuration - return repoConfig.BlunderbussConfig - } - - // Return org configuration if defined - if orgConfig.BlunderbussConfig != nil { - return *orgConfig.BlunderbussConfig - } - } - - // Return base config - return c.Blunderbuss.BlunderbussConfig -} - // LgtmFor finds the Lgtm for a repo, if one exists // a trigger can be listed for the repo itself or for the // owning organization @@ -1962,8 +1919,7 @@ func (c *Configuration) mergeFrom(other *Configuration) error { diff := cmp.Diff(other, &Configuration{Approve: other.Approve, Bugzilla: other.Bugzilla, ExternalPlugins: other.ExternalPlugins, Label: Label{RestrictedLabels: other.Label.RestrictedLabels}, - Lgtm: other.Lgtm, Plugins: other.Plugins, Triggers: other.Triggers, Welcome: other.Welcome, - Blunderbuss: other.Blunderbuss}, + Lgtm: other.Lgtm, Plugins: other.Plugins, Triggers: other.Triggers, Welcome: other.Welcome}, config.DefaultDiffOpts...) if diff != "" { @@ -1986,10 +1942,6 @@ func (c *Configuration) mergeFrom(other *Configuration) error { c.Triggers = append(c.Triggers, other.Triggers...) c.Welcome = append(c.Welcome, other.Welcome...) - if err := c.Blunderbuss.mergeFrom(&other.Blunderbuss); err != nil { - errs = append(errs, fmt.Errorf("failed to merge .blunderbuss from supplemental config: %w", err)) - } - if err := c.mergeExternalPluginsFrom(other.ExternalPlugins); err != nil { errs = append(errs, fmt.Errorf("failed to merge .external-plugins from supplemental config: %w", err)) } @@ -2106,64 +2058,6 @@ func (l *Label) mergeFrom(other *Label) error { return utilerrors.NewAggregate(errs) } -// Merge two Blunderbuss configurations, returning an aggregated error for conflicts -func (b *Blunderbuss) mergeFrom(other *Blunderbuss) error { - // No config actually specified, so no action required - if other == nil { - return nil - } - - var errs []error - - // Add error when both configs declare different global defaults - if !reflect.DeepEqual(b.BlunderbussConfig, other.BlunderbussConfig) { - errs = append(errs, fmt.Errorf("global configurations for blunderbuss do not match")) - } - - // Initialize the Orgs map if it's empty to accept incoming Orgs configs - if other.Orgs != nil && b.Orgs == nil { - b.Orgs = map[string]BlunderbussOrgConfig{} - } - - // Merge Orgs configs, skipping conflicts and adding a message to the aggregated error message - for org, otherOrgConfig := range other.Orgs { - if orgConfig, ok := b.Orgs[org]; ok { - // Use incoming org if current org config is empty, otherwise verify they're the same - if otherOrgConfig.BlunderbussConfig != nil { - if orgConfig.BlunderbussConfig == nil { - orgConfig.BlunderbussConfig = otherOrgConfig.BlunderbussConfig - b.Orgs[org] = orgConfig - } else if !reflect.DeepEqual(&orgConfig.BlunderbussConfig, &otherOrgConfig.BlunderbussConfig) { - errs = append(errs, fmt.Errorf("found conflicting config for blunderbuss.orgs[\"%s\"]", org)) - continue - } - } - // Initialize Repos map if it's empty to accept incoming Repos configs - if otherOrgConfig.Repos != nil && orgConfig.Repos == nil { - orgConfig.Repos = map[string]BlunderbussRepoConfig{} - b.Orgs[org] = orgConfig - } - // Merge Repos configs, skipping conflicts and adding a message to the aggregated error message - for repo, otherRepoConfig := range otherOrgConfig.Repos { - if repoConfig, ok := orgConfig.Repos[repo]; ok { - // Verify the repo configurations are the same - if !reflect.DeepEqual(&repoConfig.BlunderbussConfig, &otherRepoConfig.BlunderbussConfig) { - errs = append(errs, fmt.Errorf( - "found conflicting config for blunderbuss.orgs[\"%s\"].repos[\"%s\"]", org, repo)) - continue - } - } else { - b.Orgs[org].Repos[repo] = otherRepoConfig - } - } - } else { - b.Orgs[org] = otherOrgConfig - } - } - - return utilerrors.NewAggregate(errs) -} - func getLabelConfigFromRestrictedLabelsSlice(s []RestrictedLabel, label string) int { for idx, item := range s { if item.Label == label { @@ -2176,9 +2070,9 @@ func getLabelConfigFromRestrictedLabelsSlice(s []RestrictedLabel, label string) func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos sets.String) { equals := reflect.DeepEqual(c, - &Configuration{Approve: c.Approve, Blunderbuss: c.Blunderbuss, Bugzilla: c.Bugzilla, - ExternalPlugins: c.ExternalPlugins, Label: Label{RestrictedLabels: c.Label.RestrictedLabels}, - Lgtm: c.Lgtm, Plugins: c.Plugins, Triggers: c.Triggers, Welcome: c.Welcome}) + &Configuration{Approve: c.Approve, Bugzilla: c.Bugzilla, ExternalPlugins: c.ExternalPlugins, + Label: Label{RestrictedLabels: c.Label.RestrictedLabels}, Lgtm: c.Lgtm, Plugins: c.Plugins, + Triggers: c.Triggers, Welcome: c.Welcome}) if !equals || c.Bugzilla.Default != nil { global = true @@ -2193,15 +2087,6 @@ func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos set } } - for org, orgConfig := range c.Blunderbuss.Orgs { - if orgConfig.BlunderbussConfig != nil { - orgs.Insert(org) - } - for repo := range orgConfig.Repos { - repos.Insert(repo) - } - } - for org, orgConfig := range c.Bugzilla.Orgs { if orgConfig.Default != nil { orgs.Insert(org) diff --git a/prow/plugins/config_test.go b/prow/plugins/config_test.go index 0004163b16cc..10cac84b6485 100644 --- a/prow/plugins/config_test.go +++ b/prow/plugins/config_test.go @@ -1671,160 +1671,6 @@ func TestPluginsMergeFrom(t *testing.T) { } } -func TestBlunderbussMergeFrom(t *testing.T) { - t.Parallel() - - testCases := []struct { - name string - from *Blunderbuss - to *Blunderbuss - expected *Blunderbuss - expectedErrMsg string - }{ - { - name: "Merging for two different repos in an org", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-2": {}}}}}, - expected: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{ - "org/repo-1": {}, - "org/repo-2": {}}}}}, - }, - { - name: "Merging org and repo in org", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-2": {}}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - BlunderbussConfig: &BlunderbussConfig{}}}}, - expected: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - BlunderbussConfig: &BlunderbussConfig{}, - Repos: map[string]BlunderbussRepoConfig{"org/repo-2": {}}}}}, - }, - { - name: "Merging 2 orgs", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org-2": { - Repos: map[string]BlunderbussRepoConfig{"org-2/repo-1": {}}}}}, - expected: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{ - "org": { - Repos: map[string]BlunderbussRepoConfig{ - "org/repo-1": {}}}, - "org-2": { - Repos: map[string]BlunderbussRepoConfig{ - "org-2/repo-1": {}}}}}, - }, - { - name: "Merging global config with org/repo config succeeds", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - to: &Blunderbuss{ - BlunderbussConfig: BlunderbussConfig{}}, - expected: &Blunderbuss{ - BlunderbussConfig: BlunderbussConfig{}, - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - }, - { - name: "Merging org/repo config with global config succeeds", - from: &Blunderbuss{ - BlunderbussConfig: BlunderbussConfig{}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - expected: &Blunderbuss{ - BlunderbussConfig: BlunderbussConfig{}, - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - }, - { - name: "Merging identical global configs succeeds", - from: &Blunderbuss{BlunderbussConfig: BlunderbussConfig{}}, - to: &Blunderbuss{BlunderbussConfig: BlunderbussConfig{}}, - expected: &Blunderbuss{BlunderbussConfig: BlunderbussConfig{}}, - }, - { - name: "Merging differing global configs fails", - from: &Blunderbuss{BlunderbussConfig: BlunderbussConfig{MaxReviewerCount: 1}}, - to: &Blunderbuss{BlunderbussConfig: BlunderbussConfig{MaxReviewerCount: 2}}, - expectedErrMsg: "global configurations for blunderbuss do not match", - }, - { - name: "Merging identical organization configs succeeds", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": {BlunderbussConfig: &BlunderbussConfig{}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": {BlunderbussConfig: &BlunderbussConfig{}}}}, - expected: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": {BlunderbussConfig: &BlunderbussConfig{}}}}, - }, - { - name: "Merging differing organization configs fails", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": {BlunderbussConfig: &BlunderbussConfig{MaxReviewerCount: 1}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": {BlunderbussConfig: &BlunderbussConfig{MaxReviewerCount: 2}}}}, - expectedErrMsg: "found conflicting config for blunderbuss.orgs[\"org\"]", - }, - { - name: "Merging identical repository configs succeeds", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - expected: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}}, - }, - { - name: "Merging differing repository configs fails", - from: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": { - BlunderbussConfig: BlunderbussConfig{MaxReviewerCount: 1}}}}}}, - to: &Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"org": { - Repos: map[string]BlunderbussRepoConfig{"org/repo-1": { - BlunderbussConfig: BlunderbussConfig{MaxReviewerCount: 2}}}}}}, - expectedErrMsg: "found conflicting config for blunderbuss.orgs[\"org\"].repos[\"org/repo-1\"]", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - var errMsg string - err := tc.to.mergeFrom(tc.from) - if err != nil { - errMsg = err.Error() - } - if tc.expectedErrMsg != errMsg { - t.Fatalf("expected error message %q, got %q", tc.expectedErrMsg, errMsg) - } - if err != nil { - return - } - - if diff := cmp.Diff(tc.expected, tc.to); diff != "" { - t.Errorf("expected config differs from actual: %s", diff) - } - }) - } -} - func TestBugzillaMergeFrom(t *testing.T) { t.Parallel() @@ -2140,7 +1986,7 @@ func TestBugzillaMergeFrom(t *testing.T) { } if diff := cmp.Diff(tc.expected, tc.to); diff != "" { - t.Errorf("expected config differs from actual: %s", diff) + t.Errorf("expexcted config differs from actual: %s", diff) } }) } @@ -2163,7 +2009,6 @@ func TestHasConfigFor(t *testing.T) { fuzzedConfig.Triggers = nil fuzzedConfig.Welcome = nil fuzzedConfig.ExternalPlugins = nil - fuzzedConfig.Blunderbuss = Blunderbuss{BlunderbussConfig: BlunderbussConfig{}} return fuzzedConfig, !reflect.DeepEqual(fuzzedConfig, &Configuration{}), nil, nil }, }, @@ -2391,20 +2236,6 @@ func TestMergeFrom(t *testing.T) { {Repos: []string{"foo/baz"}}, }}, }, - { - name: "Blunderbuss config gets merged", - in: Configuration{Blunderbuss: Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"foo": { - Repos: map[string]BlunderbussRepoConfig{"foo/bar": {}}}}}}, - supplementalConfigs: []Configuration{{Blunderbuss: Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"foo": { - Repos: map[string]BlunderbussRepoConfig{"foo/baz": {}}}}}}}, - expected: Configuration{Blunderbuss: Blunderbuss{ - Orgs: map[string]BlunderbussOrgConfig{"foo": { - Repos: map[string]BlunderbussRepoConfig{ - "foo/bar": {}, - "foo/baz": {}}}}}}, - }, { name: "ExternalPlugins get merged", in: Configuration{ diff --git a/prow/plugins/plugin-config-documented.yaml b/prow/plugins/plugin-config-documented.yaml index 7bafdea28b32..72402ad12782 100644 --- a/prow/plugins/plugin-config-documented.yaml +++ b/prow/plugins/plugin-config-documented.yaml @@ -54,55 +54,6 @@ blunderbuss: # IgnoreDrafts instructs the plugin to ignore assigning reviewers # to the PR that is in Draft state. Default it's false. ignore_drafts: true - # Orgs allows sharding for organization-specific Blunderbuss settings, provided under keys with - # the name of the organization. For repository-specific settings, provide - # organization/repository keys under orgs[].repos. - orgs: - "": - # ExcludeApprovers controls whether approvers are considered to be - # reviewers. By default, approvers are considered as reviewers if - # insufficient reviewers are available. If ExcludeApprovers is true, - # approvers will never be considered as reviewers. - exclude_approvers: true - # IgnoreAuthors skips requesting reviewers for specified users. - # This is useful when a bot user or admin opens a PR that will be - # merged regardless of approvals. - ignore_authors: - - "" - # IgnoreDrafts instructs the plugin to ignore assigning reviewers - # to the PR that is in Draft state. Default it's false. - ignore_drafts: true - repos: - "": - # ExcludeApprovers controls whether approvers are considered to be - # reviewers. By default, approvers are considered as reviewers if - # insufficient reviewers are available. If ExcludeApprovers is true, - # approvers will never be considered as reviewers. - exclude_approvers: true - # IgnoreAuthors skips requesting reviewers for specified users. - # This is useful when a bot user or admin opens a PR that will be - # merged regardless of approvals. - ignore_authors: - - "" - # IgnoreDrafts instructs the plugin to ignore assigning reviewers - # to the PR that is in Draft state. Default it's false. - ignore_drafts: true - # ReviewerCount is the minimum number of reviewers to request - # reviews from. Defaults to requesting reviews from 2 reviewers - request_count: 0 - # UseStatusAvailability controls whether blunderbuss will consider GitHub's - # status availability when requesting reviews for users. This will use at one - # additional token per successful reviewer (and potentially more depending on - # how many busy reviewers it had to pass over). - use_status_availability: true - # ReviewerCount is the minimum number of reviewers to request - # reviews from. Defaults to requesting reviews from 2 reviewers - request_count: 0 - # UseStatusAvailability controls whether blunderbuss will consider GitHub's - # status availability when requesting reviews for users. This will use at one - # additional token per successful reviewer (and potentially more depending on - # how many busy reviewers it had to pass over). - use_status_availability: true # ReviewerCount is the minimum number of reviewers to request # reviews from. Defaults to requesting reviews from 2 reviewers request_count: 0 diff --git a/prow/plugins/plugins_test.go b/prow/plugins/plugins_test.go index e739dcb4e628..d823f3cf72d9 100644 --- a/prow/plugins/plugins_test.go +++ b/prow/plugins/plugins_test.go @@ -232,9 +232,8 @@ func TestLoad(t *testing.T) { defaultedConfig := func(m ...func(*Configuration)) *Configuration { cfg := &Configuration{ - Owners: Owners{LabelsDenyList: []string{"approved", "lgtm"}}, - Blunderbuss: Blunderbuss{ - BlunderbussConfig: BlunderbussConfig{ReviewerCount: func() *int { i := 2; return &i }()}}, + Owners: Owners{LabelsDenyList: []string{"approved", "lgtm"}}, + Blunderbuss: Blunderbuss{ReviewerCount: func() *int { i := 2; return &i }()}, CherryPickUnapproved: CherryPickUnapproved{ BranchRegexp: "^release-.*$", BranchRe: regexp.MustCompile("^release-.*$"),