diff --git a/pkg/plugins/approve/approve.go b/pkg/plugins/approve/approve.go index dbcc267838..55ca6a6942 100644 --- a/pkg/plugins/approve/approve.go +++ b/pkg/plugins/approve/approve.go @@ -109,21 +109,12 @@ func helpProvider(config *plugins.Configuration, enabledRepos []config.OrgRepo) approveConfig := map[string]string{} for _, repo := range enabledRepos { - opts := config.ApproveFor(repo.Org, repo.Repo) - approveConfig[repo.String()] = fmt.Sprintf("Pull requests %s require an associated issue.
Pull request authors %s implicitly approve their own PRs.
The /lgtm [cancel] command(s) %s act as approval.
A GitHub approved or changes requested review %s act as approval or cancel respectively.", doNot(opts.IssueRequired), doNot(opts.HasSelfApproval()), willNot(opts.LgtmActsAsApprove), willNot(opts.ConsiderReviewState())) + opts := config.Approve.RepoOptions(repo.Org, repo.Repo) + approveConfig[repo.String()] = fmt.Sprintf("Pull requests %s require an associated issue.
Pull request authors %s implicitly approve their own PRs.
The /lgtm [cancel] command(s) %s act as approval.
A GitHub approved or changes requested review %s act as approval or cancel respectively.", doNot(opts.AreIssueRequired()), doNot(opts.HasSelfApproval()), willNot(opts.ShouldLgtmActsAsApprove()), willNot(opts.ConsiderReviewState())) } yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{ - Approve: []plugins.Approve{ - { - Repos: []string{ - "ORGANIZATION", - "ORGANIZATION/REPOSITORY", - }, - RequireSelfApproval: new(bool), - IgnoreReviewState: new(bool), - }, - }, + Approve: plugins.ConfigTree[plugins.Approve]{}, }) if err != nil { logrus.WithError(err).Warnf("cannot generate comments for %s plugin", PluginName) @@ -148,17 +139,18 @@ func helpProvider(config *plugins.Configuration, enabledRepos []config.OrgRepo) } func handleGenericCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) error { + opts := pc.PluginConfig.Approve.BranchOptions(ce.Repo.Owner.Login, ce.Repo.Name, ce.Repo.DefaultBranch) return handleGenericComment( pc.Logger, pc.GitHubClient, pc.OwnersClient, pc.Config.GitHubOptions, - pc.PluginConfig, + opts, &ce, ) } -func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, config *plugins.Configuration, ce *github.GenericCommentEvent) error { +func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, opts *plugins.Approve, ce *github.GenericCommentEvent) error { funcStart := time.Now() defer func() { log.WithField("duration", time.Since(funcStart).String()).Debug("Completed handleGenericComment") @@ -173,8 +165,7 @@ func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient, return err } - opts := config.ApproveFor(ce.Repo.Owner.Login, ce.Repo.Name) - if !isApprovalCommand(botUserChecker, opts.LgtmActsAsApprove, &comment{Body: ce.Body, Author: ce.User.Login}) { + if !isApprovalCommand(botUserChecker, *opts.LgtmActsAsApprove, &comment{Body: ce.Body, Author: ce.User.Login}) { log.Debug("Comment does not constitute approval, skipping event.") return nil } @@ -213,17 +204,18 @@ func handleGenericComment(log *logrus.Entry, ghc githubClient, oc ownersClient, // handleReviewEvent should only handle reviews that have no approval command. // Reviews with approval commands will be handled by handleGenericCommentEvent. func handleReviewEvent(pc plugins.Agent, re github.ReviewEvent) error { + opts := pc.PluginConfig.Approve.BranchOptions(re.Repo.Owner.Login, re.Repo.Name, re.Repo.DefaultBranch) return handleReview( pc.Logger, pc.GitHubClient, pc.OwnersClient, pc.Config.GitHubOptions, - pc.PluginConfig, + opts, &re, ) } -func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, config *plugins.Configuration, re *github.ReviewEvent) error { +func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, opts *plugins.Approve, re *github.ReviewEvent) error { funcStart := time.Now() defer func() { log.WithField("duration", time.Since(funcStart).String()).Debug("Completed handleReview") @@ -238,12 +230,10 @@ func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubCo return err } - opts := config.ApproveFor(re.Repo.Owner.Login, re.Repo.Name) - // Check for an approval command is in the body. If one exists, let the // genericCommentEventHandler handle this event. Approval commands override // review state. - if isApprovalCommand(botUserChecker, opts.LgtmActsAsApprove, &comment{Body: re.Review.Body, Author: re.Review.User.Login}) { + if isApprovalCommand(botUserChecker, *opts.LgtmActsAsApprove, &comment{Body: re.Review.Body, Author: re.Review.User.Login}) { log.Debug("Review constitutes approval, skipping event.") return nil } @@ -282,17 +272,18 @@ func handleReview(log *logrus.Entry, ghc githubClient, oc ownersClient, githubCo } func handlePullRequestEvent(pc plugins.Agent, pre github.PullRequestEvent) error { + opts := pc.PluginConfig.Approve.BranchOptions(pre.Repo.Owner.Login, pre.Repo.Name, pre.Repo.DefaultBranch) return handlePullRequest( pc.Logger, pc.GitHubClient, pc.OwnersClient, pc.Config.GitHubOptions, - pc.PluginConfig, + opts, &pre, ) } -func handlePullRequest(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, config *plugins.Configuration, pre *github.PullRequestEvent) error { +func handlePullRequest(log *logrus.Entry, ghc githubClient, oc ownersClient, githubConfig config.GitHubOptions, opts *plugins.Approve, pre *github.PullRequestEvent) error { funcStart := time.Now() defer func() { log.WithField("duration", time.Since(funcStart).String()).Debug("Completed handlePullRequest") @@ -325,7 +316,7 @@ func handlePullRequest(log *logrus.Entry, ghc githubClient, oc ownersClient, git ghc, repo, githubConfig, - config.ApproveFor(pre.Repo.Owner.Login, pre.Repo.Name), + opts, &state{ org: pre.Repo.Owner.Login, repo: pre.Repo.Name, @@ -433,7 +424,7 @@ func handle(log *logrus.Entry, ghc githubClient, repo approvers.Repo, githubConf if err != nil { log.WithError(err).Errorf("Failed to find associated issue from PR body: %v", err) } - approversHandler.RequireIssue = opts.IssueRequired + approversHandler.RequireIssue = *opts.IssueRequired approversHandler.ManuallyApproved = humanAddedApproved(ghc, log, pr.org, pr.repo, pr.number, hasApprovedLabel) // Author implicitly approves their own PR if config allows it @@ -452,7 +443,7 @@ func handle(log *logrus.Entry, ghc githubClient, repo approvers.Repo, githubConf sort.SliceStable(comments, func(i, j int) bool { return comments[i].CreatedAt.Before(comments[j].CreatedAt) }) - approveComments := filterComments(comments, approvalMatcher(botUserChecker, opts.LgtmActsAsApprove, opts.ConsiderReviewState())) + approveComments := filterComments(comments, approvalMatcher(botUserChecker, *opts.LgtmActsAsApprove, opts.ConsiderReviewState())) addApprovers(&approversHandler, approveComments, pr.author, opts.ConsiderReviewState()) log.WithField("duration", time.Since(start).String()).Debug("Completed filtering approval comments in handle") diff --git a/pkg/plugins/approve/approve_test.go b/pkg/plugins/approve/approve_test.go index 1b181a6ce6..36152a4b1b 100644 --- a/pkg/plugins/approve/approve_test.go +++ b/pkg/plugins/approve/approve_test.go @@ -1261,10 +1261,9 @@ Approvers can cancel approval by writing ` + "`/approve cancel`" + ` in a commen LinkURL: test.githubLinkURL, }, &plugins.Approve{ - Repos: []string{"org/repo"}, RequireSelfApproval: &rsa, - IssueRequired: test.needsIssue, - LgtmActsAsApprove: test.lgtmActsAsApprove, + IssueRequired: &test.needsIssue, + LgtmActsAsApprove: &test.lgtmActsAsApprove, IgnoreReviewState: &irs, CommandHelpLink: "https://go.k8s.io/bot-commands", PrProcessLink: "https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process", @@ -1548,17 +1547,14 @@ func TestHandleGenericComment(t *testing.T) { Host: "github.com", }, } - config := &plugins.Configuration{} - config.Approve = append(config.Approve, plugins.Approve{ - Repos: []string{test.commentEvent.Repo.Owner.Login}, - LgtmActsAsApprove: test.lgtmActsAsApprove, - }) err := handleGenericComment( logrus.WithField("plugin", "approve"), fghc, fakeOwnersClient{}, githubConfig, - config, + &plugins.Approve{ + LgtmActsAsApprove: &test.lgtmActsAsApprove, + }, &test.commentEvent, ) @@ -1767,19 +1763,16 @@ func TestHandleReview(t *testing.T) { Host: "github.com", }, } - config := &plugins.Configuration{} irs := !test.reviewActsAsApprove - config.Approve = append(config.Approve, plugins.Approve{ - Repos: []string{test.reviewEvent.Repo.Owner.Login}, - LgtmActsAsApprove: test.lgtmActsAsApprove, - IgnoreReviewState: &irs, - }) err := handleReview( logrus.WithField("plugin", "approve"), fghc, fakeOwnersClient{}, githubConfig, - config, + &plugins.Approve{ + LgtmActsAsApprove: &test.lgtmActsAsApprove, + IgnoreReviewState: &irs, + }, &test.reviewEvent, ) @@ -1923,7 +1916,7 @@ func TestHandlePullRequest(t *testing.T) { Host: "github.com", }, }, - &plugins.Configuration{}, + &plugins.Approve{}, &test.prEvent, ) @@ -1965,13 +1958,9 @@ func TestHelpProvider(t *testing.T) { { name: "All configs enabled", config: &plugins.Configuration{ - Approve: []plugins.Approve{ - { - Repos: []string{"org2/repo"}, - IssueRequired: true, - RequireSelfApproval: &[]bool{true}[0], - LgtmActsAsApprove: true, - IgnoreReviewState: &[]bool{true}[0], + Approve: plugins.ConfigTree[plugins.Approve]{ + Orgs: map[string]plugins.Org[plugins.Approve]{ + "org2": {}, }, }, }, diff --git a/pkg/plugins/config.go b/pkg/plugins/config.go index 29ada6b7a8..dcb42cf2d8 100644 --- a/pkg/plugins/config.go +++ b/pkg/plugins/config.go @@ -66,7 +66,7 @@ type Configuration struct { Owners Owners `json:"owners,omitempty"` // Built-in plugins specific configuration. - Approve []Approve `json:"approve,omitempty"` + Approve ConfigTree[Approve] `json:"approve,omitempty"` Blockades []Blockade `json:"blockades,omitempty"` Blunderbuss Blunderbuss `json:"blunderbuss,omitempty"` Bugzilla Bugzilla `json:"bugzilla,omitempty"` @@ -309,17 +309,15 @@ type Blockade struct { // // The configuration for the approve plugin is defined as a list of these structures. type Approve struct { - // Repos is either of the form org/repos or just org. - Repos []string `json:"repos,omitempty"` // IssueRequired indicates if an associated issue is required for approval in // the specified repos. - IssueRequired bool `json:"issue_required,omitempty"` + IssueRequired *bool `json:"issue_required,omitempty"` // RequireSelfApproval disables automatic approval from PR authors with approval rights. // Otherwise the plugin assumes the author of the PR with approval rights approves the changes in the PR. RequireSelfApproval *bool `json:"require_self_approval,omitempty"` // LgtmActsAsApprove indicates that the lgtm command should be used to // indicate approval - LgtmActsAsApprove bool `json:"lgtm_acts_as_approve,omitempty"` + LgtmActsAsApprove *bool `json:"lgtm_acts_as_approve,omitempty"` // 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. @@ -333,10 +331,32 @@ type Approve struct { PrProcessLink string `json:"pr_process_link,omitempty"` } +// DeprecatedApprove is a composed type for compatibility with the old config format +type DeprecatedApprove struct { + // Repos is either of the form org/repos or just org. + Repos []string `json:"repos,omitempty"` + + Approve +} + var ( warnDependentBugTargetRelease time.Time ) +func (a Approve) AreIssueRequired() bool { + if a.IssueRequired != nil { + return *a.IssueRequired + } + return false +} + +func (a Approve) ShouldLgtmActsAsApprove() bool { + if a.LgtmActsAsApprove != nil { + return *a.LgtmActsAsApprove + } + return false +} + func (a Approve) HasSelfApproval() bool { if a.RequireSelfApproval != nil { return !*a.RequireSelfApproval @@ -351,10 +371,6 @@ func (a Approve) ConsiderReviewState() bool { return true } -func (a Approve) getRepos() []string { - return a.Repos -} - // Lgtm specifies a configuration for a single lgtm. // The configuration for the lgtm plugin is defined as a list of these structures. type Lgtm struct { @@ -846,39 +862,62 @@ func (r RequireMatchingLabel) Describe() string { return str.String() } -// ApproveFor finds the Approve for a repo, if one exists. -// Approval configuration can be listed for a repository -// or an organization. -func (c *Configuration) ApproveFor(org, repo string) *Approve { - fullName := fmt.Sprintf("%s/%s", org, repo) - - a := func() *Approve { - // First search for repo config - for _, approve := range c.Approve { - if !sets.New[string](approve.Repos...).Has(fullName) { - continue +func oldToNewApprove(old DeprecatedApprove) *Approve { + a := Approve{ + IgnoreReviewState: old.IgnoreReviewState, + IssueRequired: old.IssueRequired, + LgtmActsAsApprove: old.LgtmActsAsApprove, + RequireSelfApproval: old.RequireSelfApproval, + CommandHelpLink: old.CommandHelpLink, + PrProcessLink: old.PrProcessLink, + } + return &a +} + +func oldToNewApproveConfig(old []DeprecatedApprove) ConfigTree[Approve] { + a := ConfigTree[Approve]{} + a.Orgs = make(map[string]Org[Approve]) + for _, entry := range old { + for _, repo := range entry.Repos { + s := strings.Split(repo, "/") + ao := a.Orgs[s[0]] + switch len(s) { + case 1: + ao.Config = *oldToNewApprove(entry) + case 2: + if ao.Repos == nil { + ao.Repos = make(map[string]Repo[Approve]) + } + ar := ao.Repos[s[1]] + ar.Config = *oldToNewApprove(entry) + ao.Repos[s[1]] = ar } - return &approve + a.Orgs[s[0]] = ao } + } + return a +} - // If you don't find anything, loop again looking for an org config - for _, approve := range c.Approve { - if !sets.New[string](approve.Repos...).Has(org) { - continue - } - return &approve - } +type withoutUnmarshaler[T ProwConfig] ConfigTree[T] - // Return an empty config, and use plugin defaults - return &Approve{} - }() - if a.CommandHelpLink == "" { - a.CommandHelpLink = "https://go.k8s.io/bot-commands" - } - if a.PrProcessLink == "" { - a.PrProcessLink = "https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process" +var warnTriggerDeprecatedApprove time.Time + +func (a *ConfigTree[T]) UnmarshalJSON(d []byte) error { + switch v := any(a).(type) { + case *ConfigTree[Approve]: + var oldApprove []DeprecatedApprove + if err := yaml.Unmarshal(d, &oldApprove); err == nil { + logrusutil.ThrottledWarnf(&warnTriggerDeprecatedApprove, time.Hour, "Approve plugin uses a deprecated config style, please migrate to a ConfigTree based config") + *v = oldToNewApproveConfig(oldApprove) + return nil + } + default: + return fmt.Errorf("unknown type for ConfigTree object %v", v) } - return a + var target withoutUnmarshaler[T] + err := yaml.Unmarshal(d, &target) + *a = ConfigTree[T](target) + return err } // LgtmFor finds the Lgtm for a repo, if one exists @@ -1419,9 +1458,7 @@ func (c *Configuration) Validate() error { if err := validateTrigger(c.Triggers); err != nil { return err } - if err := validateRepoDupes(c.Approve); err != nil { - return err - } + // no need to check for repo duplicates in Approve as the ConfigTree uses maps if err := validateRepoDupes(c.Welcome); err != nil { return err } @@ -2029,7 +2066,7 @@ func (c *Configuration) mergeFrom(other *Configuration) error { errs = append(errs, fmt.Errorf("failed to merge .bugzilla from supplemental config: %w", err)) } - c.Approve = append(c.Approve, other.Approve...) + c.Approve = c.Approve.Apply(other.Approve) c.Lgtm = append(c.Lgtm, other.Lgtm...) c.Triggers = append(c.Triggers, other.Triggers...) c.Welcome = append(c.Welcome, other.Welcome...) @@ -2188,13 +2225,10 @@ func (c *Configuration) HasConfigFor() (global bool, orgs sets.Set[string], repo } } - for _, approveConfig := range c.Approve { - for _, orgOrRepo := range approveConfig.Repos { - if strings.Contains(orgOrRepo, "/") { - repos.Insert(orgOrRepo) - } else { - orgs.Insert(orgOrRepo) - } + for org, approveOrg := range c.Approve.Orgs { + orgs.Insert(org) + for repo := range approveOrg.Repos { + repos.Insert(repo) } } diff --git a/pkg/plugins/config_test.go b/pkg/plugins/config_test.go index e14c1b4c36..425491fac2 100644 --- a/pkg/plugins/config_test.go +++ b/pkg/plugins/config_test.go @@ -276,24 +276,22 @@ func TestTriggerFor(t *testing.T) { } func TestSetApproveDefaults(t *testing.T) { - c := &Configuration{ - Approve: []Approve{ - { - Repos: []string{ - "kubernetes/kubernetes", - "kubernetes-client", - }, - }, - { - Repos: []string{ - "kubernetes-sigs/cluster-api", - }, - CommandHelpLink: "https://prow.k8s.io/command-help", - PrProcessLink: "https://github.com/kubernetes/community/blob/427ccfbc7d423d8763ed756f3b8c888b7de3cf34/contributors/guide/pull-requests.md", - }, - }, - } - + var c Configuration + configYaml := ` +--- +approve: + config: + commandHelpLink: https://go.k8s.io/bot-commands + pr_process_link: https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process + orgs: + kubernetes-sigs: + repos: + cluster-api: + config: + commandHelpLink: https://prow.k8s.io/command-help + pr_process_link: https://github.com/kubernetes/community/blob/427ccfbc7d423d8763ed756f3b8c888b7de3cf34/contributors/guide/pull-requests.md +` + yaml.Unmarshal([]byte(configYaml), &c) tests := []struct { name string org string @@ -326,7 +324,7 @@ func TestSetApproveDefaults(t *testing.T) { for _, test := range tests { - a := c.ApproveFor(test.org, test.repo) + a := c.Approve.RepoOptions(test.org, test.repo) if a.CommandHelpLink != test.expectedCommandHelpLink { t.Errorf("unexpected commandHelpLink: %s, expected: %s", a.CommandHelpLink, test.expectedCommandHelpLink) @@ -2060,7 +2058,7 @@ func TestHasConfigFor(t *testing.T) { resultGenerator: func(fuzzedConfig *Configuration) (toCheck *Configuration, expectGlobal bool, expectOrgs sets.Set[string], expectRepos sets.Set[string]) { fuzzedConfig.Plugins = nil fuzzedConfig.Bugzilla = Bugzilla{} - fuzzedConfig.Approve = nil + fuzzedConfig.Approve = ConfigTree[Approve]{} fuzzedConfig.Label.RestrictedLabels = nil fuzzedConfig.Lgtm = nil fuzzedConfig.Triggers = nil @@ -2108,13 +2106,10 @@ func TestHasConfigFor(t *testing.T) { fuzzedConfig = &Configuration{Approve: fuzzedConfig.Approve} expectOrgs, expectRepos = sets.Set[string]{}, sets.Set[string]{} - for _, approveConfig := range fuzzedConfig.Approve { - for _, orgOrRepo := range approveConfig.Repos { - if strings.Contains(orgOrRepo, "/") { - expectRepos.Insert(orgOrRepo) - } else { - expectOrgs.Insert(orgOrRepo) - } + for org, approveOrg := range fuzzedConfig.Approve.Orgs { + expectOrgs.Insert(org) + for repo := range approveOrg.Repos { + expectRepos.Insert(repo) } } @@ -2259,12 +2254,12 @@ func TestMergeFrom(t *testing.T) { }{ { name: "Approve config gets merged", - in: Configuration{Approve: []Approve{{Repos: []string{"foo/bar"}}}}, - supplementalConfigs: []Configuration{{Approve: []Approve{{Repos: []string{"foo/baz"}}}}}, - expected: Configuration{Approve: []Approve{ + in: Configuration{Approve: oldToNewApproveConfig([]DeprecatedApprove{{Repos: []string{"foo/bar"}}})}, + supplementalConfigs: []Configuration{{Approve: oldToNewApproveConfig([]DeprecatedApprove{{Repos: []string{"foo/baz"}}})}}, + expected: Configuration{Approve: oldToNewApproveConfig([]DeprecatedApprove{ {Repos: []string{"foo/bar"}}, {Repos: []string{"foo/baz"}}, - }}, + })}, }, { name: "LGTM config gets merged", diff --git a/pkg/plugins/configtree.go b/pkg/plugins/configtree.go new file mode 100644 index 0000000000..19e882fc29 --- /dev/null +++ b/pkg/plugins/configtree.go @@ -0,0 +1,87 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugins + +type ProwConfig interface { + Apply(config ProwConfig) ProwConfig +} + +// ConfigTree specifies the global generic config for a plugin. +type ConfigTree[T ProwConfig] struct { + Config T + Orgs map[string]Org[T] `json:"orgs,omitempty"` +} + +// Org holds the default config for an entire org, as well as any repo overrides. +type Org[T ProwConfig] struct { + Config T + Repos map[string]Repo[T] `json:"repos,omitempty"` +} + +// Repo holds the default config for all branches in a repo, as well as specific branch overrides. +type Repo[T ProwConfig] struct { + Config T + Branches map[string]T `json:"branches,omitempty"` +} + +// GetOrg returns the org config after merging in any global config. +func (t ConfigTree[T]) GetOrg(name string) *Org[T] { + o, ok := t.Orgs[name] + if ok { + o.Config = t.Config.Apply(o.Config).(T) + } else { + o.Config = t.Config + } + return &o +} + +// GetRepo returns the repo config after merging in any org config. +func (o Org[T]) GetRepo(name string) *Repo[T] { + r, ok := o.Repos[name] + if ok { + r.Config = o.Config.Apply(r.Config).(T) + } else { + r.Config = o.Config + } + return &r +} + +// GetBranch returns the branch config after merging in any repo config. +func (r Repo[T]) GetBranch(name string) *T { + b, ok := r.Branches[name] + if ok { + b = r.Config.Apply(b).(T) + } else { + b = r.Config + } + return &b +} + +// BranchOptions returns the plugin configuration for a given org/repo/branch. +func (t *ConfigTree[T]) BranchOptions(org, repo, branch string) *T { + return t.GetOrg(org).GetRepo(repo).GetBranch(branch) +} + +// RepoOptions returns the plugin configuration for a given org/repo. +func (t *ConfigTree[T]) RepoOptions(org, repo string) *T { + return &t.GetOrg(org).GetRepo(repo).Config +} + +// OrgOptions returns the plugin configuration for a given org. +func (t *ConfigTree[T]) OrgOptions(org string) *T { + return &t.GetOrg(org).Config +} diff --git a/pkg/plugins/configtree_test.go b/pkg/plugins/configtree_test.go new file mode 100644 index 0000000000..e7b3f16196 --- /dev/null +++ b/pkg/plugins/configtree_test.go @@ -0,0 +1,251 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugins + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + + "sigs.k8s.io/yaml" +) + +var ( + y = true + n = false + yes = &y + no = &n +) + +func TestApproveConfigTreeApply(t *testing.T) { + var cases = []struct { + name string + child Approve + expected Approve + parent Approve + }{ + { + name: "all empty", + child: Approve{}, + expected: Approve{}, + parent: Approve{}, + }, + { + name: "empty child", + child: Approve{}, + expected: Approve{ + RequireSelfApproval: yes, + IgnoreReviewState: yes, + }, + parent: Approve{ + IssueRequired: yes, + RequireSelfApproval: yes, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + }, + { + name: "empty parent", + child: Approve{ + IssueRequired: yes, + RequireSelfApproval: yes, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + expected: Approve{ + IssueRequired: yes, + RequireSelfApproval: yes, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + parent: Approve{}, + }, + { + name: "all yes", + child: Approve{ + IssueRequired: yes, + RequireSelfApproval: yes, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + expected: Approve{ + IssueRequired: yes, + RequireSelfApproval: yes, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + parent: Approve{ + IssueRequired: yes, + RequireSelfApproval: yes, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + }, + { + name: "all no", + child: Approve{ + IssueRequired: no, + RequireSelfApproval: no, + LgtmActsAsApprove: no, + IgnoreReviewState: no, + }, + expected: Approve{ + IssueRequired: no, + RequireSelfApproval: no, + LgtmActsAsApprove: no, + IgnoreReviewState: no, + }, + parent: Approve{ + IssueRequired: no, + RequireSelfApproval: no, + LgtmActsAsApprove: no, + IgnoreReviewState: no, + }, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if diff := cmp.Diff(c.expected, c.parent.Apply(c.child)); diff != "" { + t.Error("returned config does not match expected for kubernetes\n", diff) + } + }) + } +} + +func TestApproveConfigTree(t *testing.T) { + var cases = []struct { + name string + config []byte + expectedApproveBranch *Approve + expectedApproveOrg *Approve + expectedApproveRepo *Approve + }{ + { + name: "approve no orgs", + config: []byte(` +config: + issue_required: no + require_self_approval: yes + lgtm_acts_as_approve: no + ignore_review_state: yes +`), + expectedApproveBranch: &Approve{ + IssueRequired: no, + RequireSelfApproval: yes, + LgtmActsAsApprove: no, + IgnoreReviewState: yes, + }, + expectedApproveOrg: &Approve{ + IssueRequired: no, + RequireSelfApproval: yes, + LgtmActsAsApprove: no, + IgnoreReviewState: yes, + }, + expectedApproveRepo: &Approve{ + IssueRequired: no, + RequireSelfApproval: yes, + LgtmActsAsApprove: no, + IgnoreReviewState: yes, + }, + }, + { + name: "approve no default", + config: []byte(` +orgs: + bazelbuild: + config: + ignore_review_state: no + kubernetes: + config: + lgtm_acts_as_approve: yes + repos: + kops: + config: + lgtm_acts_as_approve: no + kubernetes: + config: + require_self_approval: yes +`), + expectedApproveBranch: &Approve{ + RequireSelfApproval: yes, + }, + expectedApproveOrg: &Approve{ + LgtmActsAsApprove: yes, + }, + expectedApproveRepo: &Approve{ + RequireSelfApproval: yes, + }, + }, + { + name: "approve full", + config: []byte(` +config: + issue_required: no + require_self_approval: no + lgtm_acts_as_approve: no + ignore_review_state: yes +orgs: + bazelbuild: + config: + ignore_review_state: no + kubernetes: + config: + lgtm_acts_as_approve: yes + repos: + kops: + config: + lgtm_acts_as_approve: no + kubernetes: + config: + require_self_approval: yes + branches: + master: + require_self_approval: no +`), + expectedApproveBranch: &Approve{ + RequireSelfApproval: no, + IgnoreReviewState: yes, + }, + expectedApproveOrg: &Approve{ + RequireSelfApproval: no, + LgtmActsAsApprove: yes, + IgnoreReviewState: yes, + }, + expectedApproveRepo: &Approve{ + RequireSelfApproval: yes, + IgnoreReviewState: yes, + }, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var tree ConfigTree[Approve] + if err := yaml.Unmarshal(c.config, &tree); err != nil { + t.Errorf("error unmarshaling config: %v", err) + } + if diff := cmp.Diff(c.expectedApproveOrg, tree.OrgOptions("kubernetes")); diff != "" { + t.Error("returned config does not match expected for kubernetes\n", diff) + } + if diff := cmp.Diff(c.expectedApproveRepo, tree.RepoOptions("kubernetes", "kubernetes")); diff != "" { + t.Error("returned config does not match expected for kubernetes/kubernetes\n", diff) + } + if diff := cmp.Diff(c.expectedApproveBranch, tree.BranchOptions("kubernetes", "kubernetes", "master")); diff != "" { + t.Error("returned config does not match expected for kubernetes/kubernetes:master\n", diff) + } + }) + } +} diff --git a/pkg/plugins/helpers.go b/pkg/plugins/helpers.go new file mode 100644 index 0000000000..e0a5c11ea4 --- /dev/null +++ b/pkg/plugins/helpers.go @@ -0,0 +1,65 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugins + +// Apply returns a policy that merges the child into the parent +func (parent Approve) Apply(child ProwConfig) ProwConfig { + new := Approve{ + IssueRequired: child.(Approve).IssueRequired, + RequireSelfApproval: selectBool(parent.RequireSelfApproval, child.(Approve).RequireSelfApproval), + LgtmActsAsApprove: child.(Approve).LgtmActsAsApprove, + IgnoreReviewState: selectBool(parent.IgnoreReviewState, child.(Approve).IgnoreReviewState), + CommandHelpLink: child.(Approve).CommandHelpLink, + PrProcessLink: child.(Approve).PrProcessLink, + } + return new +} + +// Apply returns a policy tree that merges the child into the parent +func (parent ConfigTree[T]) Apply(child ConfigTree[T]) ConfigTree[T] { + parent.Config = parent.Config.Apply(child.Config).(T) + for org, childOrg := range child.Orgs { + if parentOrg, ok := parent.Orgs[org]; ok { + parentOrg.Config = parentOrg.Config.Apply(childOrg.Config).(T) + for repo, childRepo := range childOrg.Repos { + if parentRepo, ok := parentOrg.Repos[repo]; ok { + parentRepo.Config = parentRepo.Config.Apply(childRepo.Config).(T) + for branch, childBranch := range childRepo.Branches { + if parentBranch, ok := parentRepo.Branches[branch]; ok { + parentBranch.Apply(childBranch) + } else { + parentRepo.Branches[branch] = childBranch + } + } + } else { + parentOrg.Repos[repo] = childRepo + } + } + } else { + parent.Orgs[org] = childOrg + } + } + return parent +} + +// selectBool returns the child argument if set, otherwise the parent +func selectBool(parent, child *bool) *bool { + if child != nil { + return child + } + return parent +}