Fix sharding by using a pointer for global Blunderbuss config

pkg/genyaml/genyaml.go

@@ -19,6 +19,7 @@ limitations under the License.
 // from the AST of a given file.
 //
 // Example:
+//
 //	cm, err := NewCommentMap("example_config.go")
 //
 //	yamlSnippet, err := cm.GenYaml(&plugins.Configuration{
@@ -40,8 +41,7 @@ limitations under the License.
 //
 // yamlSnippet, err := cm.GenYaml(PopulateStruct(&plugins.Configuration{}))
 //
-//
-// 	yamlSnippet will be assigned a string containing the following YAML:
+//	yamlSnippet will be assigned a string containing the following YAML:
 //
 //	# Approve is the configuration for the Approve plugin.
 //	approve:
@@ -61,7 +61,6 @@ limitations under the License.
 //
 //		# IgnoreReviewState causes the approve plugin to ignore the GitHub review state. Otherwise: * an APPROVE github review is equivalent to leaving an \"/approve\" message. * A REQUEST_CHANGES github review is equivalent to leaving an /approve cancel\" message.
 //		ignore_review_state: false
-//
 package genyaml
 
 import (
@@ -297,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

prow/cmd/deck/main_test.go

@@ -831,7 +831,7 @@ func TestHandlePluginConfig(t *testing.T) {
 			}},
 		},
 		Blunderbuss: plugins.Blunderbuss{
-			BlunderbussConfig: plugins.BlunderbussConfig{
+			BlunderbussConfig: &plugins.BlunderbussConfig{
 				ExcludeApprovers: true,
 			}},
 	}

prow/plugins/blunderbuss/blunderbuss.go

@@ -64,7 +64,7 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel
 	two := 2
 	yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{
 		Blunderbuss: plugins.Blunderbuss{
-			BlunderbussConfig: plugins.BlunderbussConfig{
+			BlunderbussConfig: &plugins.BlunderbussConfig{
 				ReviewerCount:         &two,
 				MaxReviewerCount:      3,
 				ExcludeApprovers:      true,

prow/plugins/blunderbuss/blunderbuss_test.go

@@ -693,7 +693,7 @@ func TestHandlePullRequestShardedConfig(t *testing.T) {
 	fghc := newFakeGitHubClient(&pr, tc.filesChanged)
 	c := &plugins.Configuration{
 		Blunderbuss: plugins.Blunderbuss{
-			BlunderbussConfig: plugins.BlunderbussConfig{
+			BlunderbussConfig: &plugins.BlunderbussConfig{
 				ReviewerCount:    &tc.reviewerCount,
 				MaxReviewerCount: 0,
 				ExcludeApprovers: false,
@@ -897,7 +897,7 @@ func TestHandleGenericCommentShardedConfig(t *testing.T) {
 
 			config := &plugins.Configuration{
 				Blunderbuss: plugins.Blunderbuss{
-					BlunderbussConfig: plugins.BlunderbussConfig{
+					BlunderbussConfig: &plugins.BlunderbussConfig{
 						IgnoreAuthors:         []string{"bob"},
 						ReviewerCount:         &defaultReviewerCount,
 						UseStatusAvailability: false,
@@ -922,15 +922,21 @@ func TestHandleGenericCommentShardedConfig(t *testing.T) {
 
 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)
 }
 
 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)
@@ -949,16 +955,19 @@ 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)},
 		},
 		{
 			name: "ReviewerCount specified",
 			config: &plugins.Configuration{
 				Blunderbuss: plugins.Blunderbuss{
-					BlunderbussConfig: plugins.BlunderbussConfig{
+					BlunderbussConfig: &plugins.BlunderbussConfig{
 						ReviewerCount: &[]int{2}[0],
 					}},
 			},

prow/plugins/config.go

@@ -187,7 +187,7 @@ type BlunderbussOrgConfig struct {
 
 // Blunderbuss defines overall configuration for the blunderbuss plugin.
 type Blunderbuss struct {
-	BlunderbussConfig `json:",inline,omitempty"`
+	*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.
@@ -891,7 +891,7 @@ func (c *Configuration) BlunderbussFor(org, repo string) BlunderbussConfig {
 	}
 
 	// Return base config
-	return c.Blunderbuss.BlunderbussConfig
+	return *c.Blunderbuss.BlunderbussConfig
 }
 
 // LgtmFor finds the Lgtm for a repo, if one exists
@@ -1068,6 +1068,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
@@ -2115,9 +2119,14 @@ func (b *Blunderbuss) mergeFrom(other *Blunderbuss) error {
 
 	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"))
+	// 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
@@ -2180,7 +2189,7 @@ func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos set
 			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.String{}

prow/plugins/config_test.go

@@ -1731,34 +1731,52 @@ func TestBlunderbussMergeFrom(t *testing.T) {
 				Orgs: map[string]BlunderbussOrgConfig{"org": {
 					Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}},
 			to: &Blunderbuss{
-				BlunderbussConfig: BlunderbussConfig{}},
+				BlunderbussConfig: &BlunderbussConfig{}},
 			expected: &Blunderbuss{
-				BlunderbussConfig: BlunderbussConfig{},
+				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{}},
+				BlunderbussConfig: &BlunderbussConfig{}},
 			to: &Blunderbuss{
 				Orgs: map[string]BlunderbussOrgConfig{"org": {
 					Repos: map[string]BlunderbussRepoConfig{"org/repo-1": {}}}}},
 			expected: &Blunderbuss{
-				BlunderbussConfig: BlunderbussConfig{},
+				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{}},
+			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}},
+			from:           &Blunderbuss{BlunderbussConfig: &BlunderbussConfig{MaxReviewerCount: 1}},
+			to:             &Blunderbuss{BlunderbussConfig: &BlunderbussConfig{MaxReviewerCount: 2}},
 			expectedErrMsg: "global configurations for blunderbuss do not match",
 		},
 		{
@@ -2163,7 +2181,7 @@ func TestHasConfigFor(t *testing.T) {
 				fuzzedConfig.Triggers = nil
 				fuzzedConfig.Welcome = nil
 				fuzzedConfig.ExternalPlugins = nil
-				fuzzedConfig.Blunderbuss = Blunderbuss{BlunderbussConfig: BlunderbussConfig{}}
+				fuzzedConfig.Blunderbuss = Blunderbuss{}
 				return fuzzedConfig, !reflect.DeepEqual(fuzzedConfig, &Configuration{}), nil, nil
 			},
 		},

prow/plugins/plugin-config-documented.yaml

@@ -72,6 +72,8 @@ blunderbuss:
             # 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

prow/plugins/plugins_test.go

@@ -234,7 +234,7 @@ func TestLoad(t *testing.T) {
 		cfg := &Configuration{
 			Owners: Owners{LabelsDenyList: []string{"approved", "lgtm"}},
 			Blunderbuss: Blunderbuss{
-				BlunderbussConfig: BlunderbussConfig{ReviewerCount: func() *int { i := 2; return &i }()}},
+				BlunderbussConfig: &BlunderbussConfig{ReviewerCount: func() *int { i := 2; return &i }()}},
 			CherryPickUnapproved: CherryPickUnapproved{
 				BranchRegexp: "^release-.*$",
 				BranchRe:     regexp.MustCompile("^release-.*$"),