diff --git a/pkg/genyaml/genyaml.go b/pkg/genyaml/genyaml.go index b6d99fca1687..70b7dd9df6f7 100644 --- a/pkg/genyaml/genyaml.go +++ b/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 diff --git a/prow/cmd/deck/main_test.go b/prow/cmd/deck/main_test.go index e8382d524854..363917ee8504 100644 --- a/prow/cmd/deck/main_test.go +++ b/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, }}, } diff --git a/prow/plugins/blunderbuss/blunderbuss.go b/prow/plugins/blunderbuss/blunderbuss.go index 329e6e1e9ba4..b579f0bf9235 100644 --- a/prow/plugins/blunderbuss/blunderbuss.go +++ b/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, diff --git a/prow/plugins/blunderbuss/blunderbuss_test.go b/prow/plugins/blunderbuss/blunderbuss_test.go index 171e629afe42..7e211704659a 100644 --- a/prow/plugins/blunderbuss/blunderbuss_test.go +++ b/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,7 +922,10 @@ 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) @@ -930,7 +933,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) @@ -949,8 +955,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)}, }, @@ -958,7 +967,7 @@ func TestHelpProvider(t *testing.T) { name: "ReviewerCount specified", config: &plugins.Configuration{ Blunderbuss: plugins.Blunderbuss{ - BlunderbussConfig: plugins.BlunderbussConfig{ + BlunderbussConfig: &plugins.BlunderbussConfig{ ReviewerCount: &[]int{2}[0], }}, }, diff --git a/prow/plugins/config.go b/prow/plugins/config.go index a2ca7b674aea..8d5e199fee4d 100644 --- a/prow/plugins/config.go +++ b/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{} diff --git a/prow/plugins/config_test.go b/prow/plugins/config_test.go index 0004163b16cc..a369c0025325 100644 --- a/prow/plugins/config_test.go +++ b/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 }, }, diff --git a/prow/plugins/plugin-config-documented.yaml b/prow/plugins/plugin-config-documented.yaml index 7bafdea28b32..f943cf6aa101 100644 --- a/prow/plugins/plugin-config-documented.yaml +++ b/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 diff --git a/prow/plugins/plugins_test.go b/prow/plugins/plugins_test.go index e739dcb4e628..7bd699f69a0e 100644 --- a/prow/plugins/plugins_test.go +++ b/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-.*$"),