Skip to content

Commit 9556627

Browse files
dhaiducekdhaiducek
committed
Add Orgs sharding for Blunderbuss
Using a pointer lets sharding determine whether a config had been provided so they can be properly compared. If one is not provided, this also instantiates one in `setDefaults()`, which is called following the sharding logic. Signed-off-by: Dale Haiducek <[email protected]>
1 parent 3e50f22 commit 9556627

File tree

8 files changed

+630
-40
lines changed

8 files changed

+630
-40
lines changed

‎cmd/deck/main_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -888,8 +888,9 @@ func TestHandlePluginConfig(t *testing.T) {
888888
}},
889889
},
890890
Blunderbuss: plugins.Blunderbuss{
891-
ExcludeApprovers: true,
892-
},
891+
BlunderbussConfig: &plugins.BlunderbussConfig{
892+
ExcludeApprovers: true,
893+
}},
893894
}
894895
pluginAgent := &plugins.ConfigAgent{}
895896
pluginAgent.Set(&c)

‎pkg/genyaml/genyaml.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,9 @@ func fieldType(field *ast.Field, recurse bool) (string, bool) {
295295
case *ast.SelectorExpr:
296296
// SelectorExpr are not object types yet reference one, thus continue with DFS.
297297
isSelect = true
298+
case *ast.StarExpr:
299+
// Encountered a pointer--overwrite typeName with ast.Expr
300+
typeName = fmt.Sprint(x.X)
298301
}
299302

300303
return recurse || isSelect

‎pkg/plugins/blunderbuss/blunderbuss.go‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,43 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel
6565
two := 2
6666
yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{
6767
Blunderbuss: plugins.Blunderbuss{
68-
ReviewerCount: &two,
69-
MaxReviewerCount: 3,
70-
ExcludeApprovers: true,
71-
UseStatusAvailability: true,
72-
IgnoreAuthors: []string{},
68+
BlunderbussConfig: &plugins.BlunderbussConfig{
69+
ReviewerCount: &two,
70+
MaxReviewerCount: 3,
71+
ExcludeApprovers: true,
72+
UseStatusAvailability: true,
73+
IgnoreAuthors: []string{},
74+
},
75+
Orgs: map[string]plugins.BlunderbussOrgConfig{
76+
"": {
77+
BlunderbussConfig: &plugins.BlunderbussConfig{
78+
ReviewerCount: &two,
79+
MaxReviewerCount: 3,
80+
ExcludeApprovers: true,
81+
UseStatusAvailability: true,
82+
IgnoreAuthors: []string{},
83+
},
84+
Repos: map[string]plugins.BlunderbussRepoConfig{
85+
"": {
86+
BlunderbussConfig: plugins.BlunderbussConfig{
87+
ReviewerCount: &two,
88+
MaxReviewerCount: 3,
89+
ExcludeApprovers: true,
90+
UseStatusAvailability: true,
91+
IgnoreAuthors: []string{},
92+
},
93+
},
94+
},
95+
},
96+
},
7397
},
7498
})
7599
if err != nil {
76100
logrus.WithError(err).Warnf("cannot generate comments for %s plugin", PluginName)
77101
}
78102
pluginHelp := &pluginhelp.PluginHelp{
79-
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.",
103+
Description: "The blunderbuss plugin automatically requests reviews from reviewers when a new PR is created. " +
104+
"The reviewers are selected based on the reviewers specified in the OWNERS files that apply to the files modified by the PR.",
80105
Config: map[string]string{
81106
"": configString(reviewCount),
82107
},
@@ -139,14 +164,14 @@ func handlePullRequestEvent(pc plugins.Agent, pre github.PullRequestEvent) error
139164
pc.GitHubClient,
140165
pc.OwnersClient,
141166
pc.Logger,
142-
pc.PluginConfig.Blunderbuss,
167+
pc.PluginConfig.BlunderbussFor(pre.Repo.Owner.Login, pre.Repo.Name),
143168
pre.Action,
144169
&pre.PullRequest,
145170
&pre.Repo,
146171
)
147172
}
148173

149-
func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.Blunderbuss, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error {
174+
func handlePullRequest(ghc githubClient, roc repoownersClient, log *logrus.Entry, config plugins.BlunderbussConfig, action github.PullRequestEventAction, pr *github.PullRequest, repo *github.Repo) error {
150175
// Ignore pull request event if:
151176
// - not an opened or ready for review event
152177
// - if they have a /cc command in the description (will be handled elsewhere)
@@ -182,7 +207,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent)
182207
pc.GitHubClient,
183208
pc.OwnersClient,
184209
pc.Logger,
185-
pc.PluginConfig.Blunderbuss,
210+
pc.PluginConfig.BlunderbussFor(ce.Repo.Owner.Login, ce.Repo.Name),
186211
ce.Action,
187212
ce.IsPR,
188213
ce.Number,
@@ -192,7 +217,7 @@ func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent)
192217
)
193218
}
194219

195-
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 {
220+
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 {
196221
if action != github.GenericCommentActionCreated || !isPR || issueState == "closed" {
197222
return nil
198223
}

‎pkg/plugins/blunderbuss/blunderbuss_test.go‎

Lines changed: 195 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ func TestHandlePullRequest(t *testing.T) {
685685
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}, Body: tc.body, Draft: tc.draft}
686686
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
687687
fghc := newFakeGitHubClient(&pr, tc.filesChanged)
688-
c := plugins.Blunderbuss{
688+
c := plugins.BlunderbussConfig{
689689
ReviewerCount: &tc.reviewerCount,
690690
MaxReviewerCount: 0,
691691
ExcludeApprovers: false,
@@ -710,6 +710,70 @@ func TestHandlePullRequest(t *testing.T) {
710710
}
711711
}
712712

713+
func TestHandlePullRequestShardedConfig(t *testing.T) {
714+
froc := &fakeRepoownersClient{
715+
foc: &fakeOwnersClient{
716+
owners: map[string]string{
717+
"a.go": "1",
718+
},
719+
leafReviewers: map[string]sets.Set[string]{
720+
"a.go": sets.New("al"),
721+
},
722+
},
723+
}
724+
725+
var tc = struct {
726+
action github.PullRequestEventAction
727+
body string
728+
filesChanged []string
729+
reviewerCount int
730+
expectedRequested []string
731+
draft bool
732+
ignoreDrafts bool
733+
ignoreAuthors []string
734+
}{
735+
action: github.PullRequestActionOpened,
736+
filesChanged: []string{"a.go"},
737+
draft: false,
738+
ignoreDrafts: true,
739+
reviewerCount: 1,
740+
}
741+
742+
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}, Body: tc.body, Draft: tc.draft}
743+
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
744+
fghc := newFakeGitHubClient(&pr, tc.filesChanged)
745+
c := &plugins.Configuration{
746+
Blunderbuss: plugins.Blunderbuss{
747+
BlunderbussConfig: &plugins.BlunderbussConfig{
748+
ReviewerCount: &tc.reviewerCount,
749+
MaxReviewerCount: 0,
750+
ExcludeApprovers: false,
751+
IgnoreDrafts: tc.ignoreDrafts,
752+
IgnoreAuthors: tc.ignoreAuthors,
753+
},
754+
Orgs: map[string]plugins.BlunderbussOrgConfig{
755+
"org": {
756+
Repos: map[string]plugins.BlunderbussRepoConfig{
757+
"org/repo": {
758+
BlunderbussConfig: plugins.BlunderbussConfig{
759+
IgnoreAuthors: []string{"author"},
760+
}}}}}}}
761+
bc := c.BlunderbussFor(repo.Owner.Login, repo.Name)
762+
763+
if err := handlePullRequest(
764+
fghc, froc, logrus.WithField("plugin", PluginName),
765+
bc, tc.action, &pr, &repo,
766+
); err != nil {
767+
t.Fatalf("unexpected error from handle: %v", err)
768+
}
769+
770+
sort.Strings(fghc.requested)
771+
sort.Strings(tc.expectedRequested)
772+
if !reflect.DeepEqual(fghc.requested, tc.expectedRequested) {
773+
t.Fatalf("expected the requested reviewers to be %q, but got %q.", tc.expectedRequested, fghc.requested)
774+
}
775+
}
776+
713777
func TestHandleGenericComment(t *testing.T) {
714778
froc := &fakeRepoownersClient{
715779
foc: &fakeOwnersClient{
@@ -783,7 +847,7 @@ func TestHandleGenericComment(t *testing.T) {
783847
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}}
784848
fghc := newFakeGitHubClient(&pr, tc.filesChanged)
785849
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
786-
config := plugins.Blunderbuss{
850+
config := plugins.BlunderbussConfig{
787851
ReviewerCount: &tc.reviewerCount,
788852
MaxReviewerCount: 0,
789853
ExcludeApprovers: false,
@@ -805,6 +869,108 @@ func TestHandleGenericComment(t *testing.T) {
805869
}
806870
}
807871

872+
func TestHandleGenericCommentShardedConfig(t *testing.T) {
873+
froc := &fakeRepoownersClient{
874+
foc: &fakeOwnersClient{
875+
owners: map[string]string{
876+
"a.go": "1",
877+
"b.go": "2",
878+
},
879+
leafReviewers: map[string]sets.Set[string]{
880+
"a.go": sets.New("al"),
881+
"b.go": sets.New("bob"),
882+
"c.go": sets.New("sarah"),
883+
"d.go": sets.New("busy-user"),
884+
},
885+
},
886+
}
887+
888+
overrideOrgReviewerCount := 2
889+
overrideRepoReviewerCount := 3
890+
var testcases = []struct {
891+
name string
892+
orgConfig map[string]plugins.BlunderbussOrgConfig
893+
expectedRequested int
894+
}{
895+
{
896+
name: "overrides default config with org config",
897+
orgConfig: map[string]plugins.BlunderbussOrgConfig{
898+
"org": {
899+
BlunderbussConfig: &plugins.BlunderbussConfig{
900+
ReviewerCount: &overrideOrgReviewerCount,
901+
MaxReviewerCount: overrideOrgReviewerCount,
902+
}},
903+
},
904+
expectedRequested: 2,
905+
},
906+
{
907+
name: "overrides default and org config with repo config",
908+
orgConfig: map[string]plugins.BlunderbussOrgConfig{
909+
"org": {
910+
BlunderbussConfig: &plugins.BlunderbussConfig{
911+
ReviewerCount: &overrideOrgReviewerCount,
912+
MaxReviewerCount: overrideOrgReviewerCount,
913+
},
914+
Repos: map[string]plugins.BlunderbussRepoConfig{
915+
"org/repo": {
916+
BlunderbussConfig: plugins.BlunderbussConfig{
917+
ReviewerCount: &overrideRepoReviewerCount,
918+
MaxReviewerCount: overrideRepoReviewerCount,
919+
}}}},
920+
},
921+
expectedRequested: 3,
922+
},
923+
{
924+
name: "Uses org config with invalid repo config key",
925+
orgConfig: map[string]plugins.BlunderbussOrgConfig{
926+
"org": {
927+
BlunderbussConfig: &plugins.BlunderbussConfig{
928+
ReviewerCount: &overrideOrgReviewerCount,
929+
MaxReviewerCount: overrideOrgReviewerCount,
930+
},
931+
Repos: map[string]plugins.BlunderbussRepoConfig{
932+
"repo": {
933+
BlunderbussConfig: plugins.BlunderbussConfig{
934+
ReviewerCount: &overrideRepoReviewerCount,
935+
MaxReviewerCount: overrideRepoReviewerCount,
936+
}}}},
937+
},
938+
expectedRequested: 2,
939+
},
940+
}
941+
942+
for _, tc := range testcases {
943+
t.Run(tc.name, func(t *testing.T) {
944+
pr := github.PullRequest{Number: 5, User: github.User{Login: "author"}}
945+
fghc := newFakeGitHubClient(&pr, []string{"a.go", "b.go", "c.go", "d.go"})
946+
defaultReviewerCount := 1
947+
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
948+
949+
config := &plugins.Configuration{
950+
Blunderbuss: plugins.Blunderbuss{
951+
BlunderbussConfig: &plugins.BlunderbussConfig{
952+
IgnoreAuthors: []string{"bob"},
953+
ReviewerCount: &defaultReviewerCount,
954+
UseStatusAvailability: false,
955+
},
956+
Orgs: tc.orgConfig,
957+
}}
958+
bc := config.BlunderbussFor(repo.Owner.Login, repo.Name)
959+
960+
if err := handleGenericComment(
961+
fghc, froc, logrus.WithField("plugin", PluginName), bc,
962+
github.GenericCommentActionCreated, true, pr.Number, "open", &repo, "/auto-cc",
963+
); err != nil {
964+
t.Fatalf("unexpected error from handle: %v", err)
965+
}
966+
967+
if tc.expectedRequested != len(fghc.requested) {
968+
t.Fatalf("expected the requested reviewers to be %d, but got %d.", tc.expectedRequested, len(fghc.requested))
969+
}
970+
})
971+
}
972+
}
973+
808974
func TestHandleStatus(t *testing.T) {
809975
froc := &fakeRepoownersClient{
810976
foc: &fakeOwnersClient{
@@ -951,15 +1117,17 @@ func TestHandleStatus(t *testing.T) {
9511117
repo := github.Repo{Owner: github.User{Login: "org"}, Name: "repo"}
9521118
descriptionPattern := "Not mergeable. (PullRequest is missing sufficient approving GitHub review\\(s\\)|Needs (lgtm|approved|approved, lgtm) labels?)\\.?"
9531119
config := plugins.Blunderbuss{
954-
ReviewerCount: &tc.reviewerCount,
955-
MaxReviewerCount: 0,
956-
ExcludeApprovers: false,
957-
IgnoreAuthors: []string{"ignoreme"},
958-
WaitForStatus: &plugins.ContextMatch{
959-
Context: "tide",
960-
Description: descriptionPattern,
961-
DescriptionRe: regexp.MustCompile(descriptionPattern),
962-
State: "pending",
1120+
BlunderbussConfig: &plugins.BlunderbussConfig{
1121+
ReviewerCount: &tc.reviewerCount,
1122+
MaxReviewerCount: 0,
1123+
ExcludeApprovers: false,
1124+
IgnoreAuthors: []string{"ignoreme"},
1125+
WaitForStatus: &plugins.ContextMatch{
1126+
Context: "tide",
1127+
Description: descriptionPattern,
1128+
DescriptionRe: regexp.MustCompile(descriptionPattern),
1129+
State: "pending",
1130+
},
9631131
},
9641132
}
9651133

@@ -981,15 +1149,21 @@ func TestHandleStatus(t *testing.T) {
9811149

9821150
func TestHandleGenericCommentEvent(t *testing.T) {
9831151
pc := plugins.Agent{
984-
PluginConfig: &plugins.Configuration{},
1152+
PluginConfig: &plugins.Configuration{
1153+
Blunderbuss: plugins.Blunderbuss{
1154+
BlunderbussConfig: &plugins.BlunderbussConfig{},
1155+
}},
9851156
}
9861157
ce := github.GenericCommentEvent{}
9871158
handleGenericCommentEvent(pc, ce)
9881159
}
9891160

9901161
func TestHandlePullRequestEvent(t *testing.T) {
9911162
pc := plugins.Agent{
992-
PluginConfig: &plugins.Configuration{},
1163+
PluginConfig: &plugins.Configuration{
1164+
Blunderbuss: plugins.Blunderbuss{
1165+
BlunderbussConfig: &plugins.BlunderbussConfig{},
1166+
}},
9931167
}
9941168
pre := github.PullRequestEvent{}
9951169
handlePullRequestEvent(pc, pre)
@@ -1016,17 +1190,21 @@ func TestHelpProvider(t *testing.T) {
10161190
configInfoIncludes []string
10171191
}{
10181192
{
1019-
name: "Empty config",
1020-
config: &plugins.Configuration{},
1193+
name: "Empty config",
1194+
config: &plugins.Configuration{
1195+
Blunderbuss: plugins.Blunderbuss{
1196+
BlunderbussConfig: &plugins.BlunderbussConfig{},
1197+
}},
10211198
enabledRepos: enabledRepos,
10221199
configInfoIncludes: []string{configString(0)},
10231200
},
10241201
{
10251202
name: "ReviewerCount specified",
10261203
config: &plugins.Configuration{
10271204
Blunderbuss: plugins.Blunderbuss{
1028-
ReviewerCount: &[]int{2}[0],
1029-
},
1205+
BlunderbussConfig: &plugins.BlunderbussConfig{
1206+
ReviewerCount: &[]int{2}[0],
1207+
}},
10301208
},
10311209
enabledRepos: enabledRepos,
10321210
configInfoIncludes: []string{configString(2)},

0 commit comments

Comments
 (0)