diff --git a/cmd/deck/main_test.go b/cmd/deck/main_test.go index 453dcd46e8..5e27e9e469 100644 --- a/cmd/deck/main_test.go +++ b/cmd/deck/main_test.go @@ -888,8 +888,9 @@ func TestHandlePluginConfig(t *testing.T) { }}, }, Blunderbuss: plugins.Blunderbuss{ - ExcludeApprovers: true, - }, + BlunderbussConfig: &plugins.BlunderbussConfig{ + ExcludeApprovers: true, + }}, } pluginAgent := &plugins.ConfigAgent{} pluginAgent.Set(&c) diff --git a/pkg/genyaml/genyaml.go b/pkg/genyaml/genyaml.go index f780d1bd59..70b7dd9df6 100644 --- a/pkg/genyaml/genyaml.go +++ b/pkg/genyaml/genyaml.go @@ -296,6 +296,9 @@ func fieldType(field *ast.Field, recurse bool) (string, bool) { case *ast.SelectorExpr: // SelectorExpr are not object types yet reference one, thus continue with DFS. isSelect = true + case *ast.StarExpr: + // Encountered a pointer--overwrite typeName with ast.Expr + typeName = fmt.Sprint(x.X) } return recurse || isSelect diff --git a/pkg/plugins/blunderbuss/blunderbuss.go b/pkg/plugins/blunderbuss/blunderbuss.go index 92ff3ffbbc..d21b0f414a 100644 --- a/pkg/plugins/blunderbuss/blunderbuss.go +++ b/pkg/plugins/blunderbuss/blunderbuss.go @@ -64,18 +64,43 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel two := 2 yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{ Blunderbuss: plugins.Blunderbuss{ - ReviewerCount: &two, - MaxReviewerCount: 3, - ExcludeApprovers: true, - UseStatusAvailability: true, - IgnoreAuthors: []string{}, + 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{}, + }, + }, + }, + }, + }, }, }) 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), }, @@ -137,14 +162,14 @@ func handlePullRequestEvent(pc plugins.Agent, pre github.PullRequestEvent) error pc.GitHubClient, pc.OwnersClient, pc.Logger, - pc.PluginConfig.Blunderbuss, + pc.PluginConfig.BlunderbussFor(pre.Repo.Owner.Login, pre.Repo.Name), pre.Action, &pre.PullRequest, &pre.Repo, ) } -func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.Blunderbuss, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error { +func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.BlunderbussConfig, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error { if !(action == github.PullRequestActionOpened || action == github.PullRequestActionReadyForReview) || assign.CCRegexp.MatchString(pr.Body) { return nil } @@ -176,7 +201,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) pc.GitHubClient, pc.OwnersClient, pc.Logger, - pc.PluginConfig.Blunderbuss, + pc.PluginConfig.BlunderbussFor(ce.Repo.Owner.Login, ce.Repo.Name), ce.Action, ce.IsPR, ce.Number, @@ -186,7 +211,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) ) } -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 { +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 { if action != github.GenericCommentActionCreated || !isPR || issueState == "closed" { return nil } diff --git a/pkg/plugins/blunderbuss/blunderbuss_test.go b/pkg/plugins/blunderbuss/blunderbuss_test.go index 1546264fb3..2b48efdd0f 100644 --- a/pkg/plugins/blunderbuss/blunderbuss_test.go +++ b/pkg/plugins/blunderbuss/blunderbuss_test.go @@ -643,7 +643,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.Blunderbuss{ + c := plugins.BlunderbussConfig{ ReviewerCount: &tc.reviewerCount, MaxReviewerCount: 0, ExcludeApprovers: false, @@ -667,6 +667,70 @@ 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.Set[string]{ + "a.go": sets.New("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{ @@ -740,7 +804,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.Blunderbuss{ + config := plugins.BlunderbussConfig{ ReviewerCount: &tc.reviewerCount, MaxReviewerCount: 0, ExcludeApprovers: false, @@ -762,9 +826,114 @@ 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.Set[string]{ + "a.go": sets.New("al"), + "b.go": sets.New("bob"), + "c.go": sets.New("sarah"), + "d.go": sets.New("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{}, + PluginConfig: &plugins.Configuration{ + Blunderbuss: plugins.Blunderbuss{ + BlunderbussConfig: &plugins.BlunderbussConfig{}, + }}, } ce := github.GenericCommentEvent{} handleGenericCommentEvent(pc, ce) @@ -772,7 +941,10 @@ func TestHandleGenericCommentEvent(t *testing.T) { func TestHandlePullRequestEvent(t *testing.T) { pc := plugins.Agent{ - PluginConfig: &plugins.Configuration{}, + PluginConfig: &plugins.Configuration{ + Blunderbuss: plugins.Blunderbuss{ + BlunderbussConfig: &plugins.BlunderbussConfig{}, + }}, } pre := github.PullRequestEvent{} handlePullRequestEvent(pc, pre) @@ -791,8 +963,11 @@ func TestHelpProvider(t *testing.T) { configInfoIncludes []string }{ { - name: "Empty config", - config: &plugins.Configuration{}, + name: "Empty config", + config: &plugins.Configuration{ + Blunderbuss: plugins.Blunderbuss{ + BlunderbussConfig: &plugins.BlunderbussConfig{}, + }}, enabledRepos: enabledRepos, configInfoIncludes: []string{configString(0)}, }, @@ -800,8 +975,9 @@ func TestHelpProvider(t *testing.T) { name: "ReviewerCount specified", config: &plugins.Configuration{ Blunderbuss: plugins.Blunderbuss{ - ReviewerCount: &[]int{2}[0], - }, + BlunderbussConfig: &plugins.BlunderbussConfig{ + ReviewerCount: &[]int{2}[0], + }}, }, enabledRepos: enabledRepos, configInfoIncludes: []string{configString(2)}, diff --git a/pkg/plugins/config.go b/pkg/plugins/config.go index dbdf735ad9..d6d2db2847 100644 --- a/pkg/plugins/config.go +++ b/pkg/plugins/config.go @@ -146,10 +146,10 @@ type ExternalPlugin struct { Events []string `json:"events,omitempty"` } -// Blunderbuss defines configuration for the blunderbuss plugin. -type Blunderbuss struct { +// BlunderbussConfig defines configuration options for the blunderbuss plugin. +type BlunderbussConfig struct { // ReviewerCount is the minimum number of reviewers to request - // reviews from. Defaults to requesting reviews from 2 reviewers + // reviews from. Defaults to requesting reviews from 2 reviewers. ReviewerCount *int `json:"request_count,omitempty"` // MaxReviewerCount is the maximum number of reviewers to request // reviews from. Defaults to 0 meaning no limit. @@ -173,6 +173,28 @@ type Blunderbuss 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 @@ -885,6 +907,27 @@ 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 @@ -1067,6 +1110,10 @@ func (c *Configuration) setDefaults() { c.ExternalPlugins[repo][i].Endpoint = fmt.Sprintf("http://%s", p.Name) } } + // Instantiate a global Blunderbuss config if it wasn't set in the config + if c.Blunderbuss.BlunderbussConfig == nil { + c.Blunderbuss.BlunderbussConfig = &BlunderbussConfig{} + } if c.Blunderbuss.ReviewerCount == nil { c.Blunderbuss.ReviewerCount = new(int) *c.Blunderbuss.ReviewerCount = defaultBlunderbussReviewerCount @@ -2015,7 +2062,8 @@ 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}, + Lgtm: other.Lgtm, Plugins: other.Plugins, Triggers: other.Triggers, Welcome: other.Welcome, + Blunderbuss: other.Blunderbuss}, config.DefaultDiffOpts...) if diff != "" { @@ -2038,6 +2086,10 @@ 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)) } @@ -2154,6 +2206,69 @@ 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 + + // Handle global configs + if other.BlunderbussConfig != nil { + if b.BlunderbussConfig == nil { + b.BlunderbussConfig = other.BlunderbussConfig + } else if !reflect.DeepEqual(b.BlunderbussConfig, other.BlunderbussConfig) { + // Add error when both configs declare different global defaults + 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 { @@ -2166,11 +2281,11 @@ func getLabelConfigFromRestrictedLabelsSlice(s []RestrictedLabel, label string) func (c *Configuration) HasConfigFor() (global bool, orgs sets.Set[string], repos sets.Set[string]) { equals := reflect.DeepEqual(c, - &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}) + &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}) - if !equals || c.Bugzilla.Default != nil { + if !equals || c.Bugzilla.Default != nil || c.Blunderbuss.BlunderbussConfig != nil { global = true } orgs = sets.Set[string]{} @@ -2183,6 +2298,15 @@ func (c *Configuration) HasConfigFor() (global bool, orgs sets.Set[string], repo } } + 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/pkg/plugins/config_test.go b/pkg/plugins/config_test.go index e14c1b4c36..4f4f7779a0 100644 --- a/pkg/plugins/config_test.go +++ b/pkg/plugins/config_test.go @@ -1728,6 +1728,178 @@ 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 from nil config succeeds", + from: nil, + to: &Blunderbuss{BlunderbussConfig: &BlunderbussConfig{}}, + expected: &Blunderbuss{BlunderbussConfig: &BlunderbussConfig{}}, + }, + { + name: "Merging to nil global config succeeds", + from: &Blunderbuss{BlunderbussConfig: &BlunderbussConfig{}}, + to: &Blunderbuss{BlunderbussConfig: nil}, + expected: &Blunderbuss{BlunderbussConfig: &BlunderbussConfig{}}, + }, + { + name: "Merging from config with nil global config succeeds", + from: &Blunderbuss{BlunderbussConfig: nil}, + 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() @@ -2066,6 +2238,7 @@ func TestHasConfigFor(t *testing.T) { fuzzedConfig.Triggers = nil fuzzedConfig.Welcome = nil fuzzedConfig.ExternalPlugins = nil + fuzzedConfig.Blunderbuss = Blunderbuss{} return fuzzedConfig, !reflect.DeepEqual(fuzzedConfig, &Configuration{}), nil, nil }, }, @@ -2293,6 +2466,20 @@ 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/pkg/plugins/plugin-config-documented.yaml b/pkg/plugins/plugin-config-documented.yaml index 597df00764..d289db2d77 100644 --- a/pkg/plugins/plugin-config-documented.yaml +++ b/pkg/plugins/plugin-config-documented.yaml @@ -54,8 +54,59 @@ 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 allows sharding for repository-specific Blunderbuss settings, provided under keys using + # the format organization/repository. + 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 + # 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 diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index 3898042bdc..cc1d4514a9 100644 --- a/pkg/plugins/plugins_test.go +++ b/pkg/plugins/plugins_test.go @@ -232,8 +232,9 @@ func TestLoad(t *testing.T) { defaultedConfig := func(m ...func(*Configuration)) *Configuration { cfg := &Configuration{ - Owners: Owners{LabelsDenyList: []string{"approved", "lgtm"}}, - Blunderbuss: Blunderbuss{ReviewerCount: func() *int { i := 2; return &i }()}, + Owners: Owners{LabelsDenyList: []string{"approved", "lgtm"}}, + Blunderbuss: Blunderbuss{ + BlunderbussConfig: &BlunderbussConfig{ReviewerCount: func() *int { i := 2; return &i }()}}, CherryPickUnapproved: CherryPickUnapproved{ BranchRegexp: "^release-.*$", BranchRe: regexp.MustCompile("^release-.*$"),