From 19bea9777c43d4b430dc15c4f17ad5a2790edb5e Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Mon, 29 Apr 2019 08:14:47 +0200 Subject: [PATCH 1/5] Implement ConfigTree in approve plugin --- prow/plugins/approve/approve.go | 43 ++-- prow/plugins/approve/approve_test.go | 55 +--- prow/plugins/approve_configtree_test.go | 241 ++++++++++++++++++ prow/plugins/config.go | 117 ++++++--- prow/plugins/config_test.go | 55 ++-- prow/plugins/configtree_template.go | 83 ++++++ prow/plugins/helpers.go | 70 +++++ .../zz_generated_approve_configtree.go | 83 ++++++ 8 files changed, 608 insertions(+), 139 deletions(-) create mode 100644 prow/plugins/approve_configtree_test.go create mode 100644 prow/plugins/configtree_template.go create mode 100644 prow/plugins/helpers.go create mode 100644 prow/plugins/zz_generated_approve_configtree.go diff --git a/prow/plugins/approve/approve.go b/prow/plugins/approve/approve.go index 070bd69c5468..14960dbe1808 100644 --- a/prow/plugins/approve/approve.go +++ b/prow/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.ApproveConfigTree{}, }) 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, @@ -432,7 +423,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 @@ -451,7 +442,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/prow/plugins/approve/approve_test.go b/prow/plugins/approve/approve_test.go index fc443f180c70..2a155073584b 100644 --- a/prow/plugins/approve/approve_test.go +++ b/prow/plugins/approve/approve_test.go @@ -60,23 +60,7 @@ func TestPluginConfig(t *testing.T) { } pa.Set(np) - orgs := map[string]bool{} - repos := map[string]bool{} - for _, config := range pa.Config().Approve { - for _, entry := range config.Repos { - if strings.Contains(entry, "/") { - if repos[entry] { - t.Errorf("The repo %q is duplicated in the 'approve' plugin configuration.", entry) - } - repos[entry] = true - } else { - if orgs[entry] { - t.Errorf("The org %q is duplicated in the 'approve' plugin configuration.", entry) - } - orgs[entry] = true - } - } - } + // no need to check for duplicates as the ConfigTree uses maps } func newTestComment(user, body string) github.IssueComment { @@ -1281,10 +1265,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", @@ -1560,17 +1543,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, ) @@ -1779,19 +1759,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, ) @@ -1935,7 +1912,7 @@ func TestHandlePullRequest(t *testing.T) { Host: "github.com", }, }, - &plugins.Configuration{}, + &plugins.Approve{}, &test.prEvent, ) @@ -1977,13 +1954,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.ApproveConfigTree{ + Orgs: map[string]plugins.ApproveOrg{ + "org2": {}, }, }, }, diff --git a/prow/plugins/approve_configtree_test.go b/prow/plugins/approve_configtree_test.go new file mode 100644 index 000000000000..35520b675f1e --- /dev/null +++ b/prow/plugins/approve_configtree_test.go @@ -0,0 +1,241 @@ +/* +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(` +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: + ignore_review_state: no + kubernetes: + lgtm_acts_as_approve: yes + repos: + kops: + lgtm_acts_as_approve: no + kubernetes: + require_self_approval: yes +`), + expectedApproveBranch: &Approve{ + RequireSelfApproval: yes, + }, + expectedApproveOrg: &Approve{ + LgtmActsAsApprove: yes, + }, + expectedApproveRepo: &Approve{ + RequireSelfApproval: yes, + }, + }, + { + name: "approve full", + config: []byte(` +issue_required: no +require_self_approval: no +lgtm_acts_as_approve: no +ignore_review_state: yes +orgs: + bazelbuild: + ignore_review_state: no + kubernetes: + lgtm_acts_as_approve: yes + repos: + kops: + lgtm_acts_as_approve: no + kubernetes: + 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 ApproveConfigTree + 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/prow/plugins/config.go b/prow/plugins/config.go index 4e19f9eb96cf..600ab29fb0dd 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -63,7 +63,7 @@ type Configuration struct { Owners Owners `json:"owners,omitempty"` // Built-in plugins specific configuration. - Approve []Approve `json:"approve,omitempty"` + Approve ApproveConfigTree `json:"approve,omitempty"` Blockades []Blockade `json:"blockades,omitempty"` Blunderbuss Blunderbuss `json:"blunderbuss,omitempty"` Bugzilla Bugzilla `json:"bugzilla,omitempty"` @@ -299,17 +299,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 requires PR authors to explicitly approve their PRs. // Otherwise the plugin assumes the author of the PR 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. @@ -323,10 +321,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 @@ -796,39 +816,57 @@ 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.NewString(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) ApproveConfigTree { + a := ApproveConfigTree{} + a.Orgs = make(map[string]ApproveOrg) + for _, entry := range old { + for _, repo := range entry.Repos { + s := strings.Split(repo, "/") + ao := a.Orgs[s[0]] + switch len(s) { + case 1: + ao.Approve = *oldToNewApprove(entry) + case 2: + if ao.Repos == nil { + ao.Repos = make(map[string]ApproveRepo) + } + ar := ao.Repos[s[1]] + ar.Approve = *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.NewString(approve.Repos...).Has(org) { - continue - } - return &approve - } +type approveWithoutUnmarshaler ApproveConfigTree - // 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 *ApproveConfigTree) UnmarshalJSON(d []byte) error { + 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") + *a = oldToNewApproveConfig(oldApprove) + return nil } - return a + var target approveWithoutUnmarshaler + err := yaml.Unmarshal(d, &target) + *a = ApproveConfigTree(target) + return err } // LgtmFor finds the Lgtm for a repo, if one exists @@ -1890,7 +1928,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...) @@ -2043,13 +2081,10 @@ func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos set } } - 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/prow/plugins/config_test.go b/prow/plugins/config_test.go index e1fc62adaa41..3780f2114267 100644 --- a/prow/plugins/config_test.go +++ b/prow/plugins/config_test.go @@ -219,24 +219,20 @@ 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: + 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: + 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 @@ -269,7 +265,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) @@ -2003,7 +1999,7 @@ func TestHasConfigFor(t *testing.T) { resultGenerator: func(fuzzedConfig *Configuration) (toCheck *Configuration, expectGlobal bool, expectOrgs sets.String, expectRepos sets.String) { fuzzedConfig.Plugins = nil fuzzedConfig.Bugzilla = Bugzilla{} - fuzzedConfig.Approve = nil + fuzzedConfig.Approve = ApproveConfigTree{} fuzzedConfig.Label.RestrictedLabels = nil fuzzedConfig.Lgtm = nil fuzzedConfig.Triggers = nil @@ -2050,13 +2046,10 @@ func TestHasConfigFor(t *testing.T) { fuzzedConfig = &Configuration{Approve: fuzzedConfig.Approve} expectOrgs, expectRepos = sets.String{}, sets.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) } } @@ -2182,12 +2175,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/prow/plugins/configtree_template.go b/prow/plugins/configtree_template.go new file mode 100644 index 000000000000..2685f51f70a3 --- /dev/null +++ b/prow/plugins/configtree_template.go @@ -0,0 +1,83 @@ +/* +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 + +// GenConfigTree specifies the global generic config for a plugin. +type GenConfigTree struct { + Gen + Orgs map[string]GenOrg `json:"orgs,omitempty"` +} + +// GenOrg holds the default config for an entire org, as well as any repo overrides. +type GenOrg struct { + Gen + Repos map[string]GenRepo `json:"repos,omitempty"` +} + +// GenRepo holds the default config for all branches in a repo, as well as specific branch overrides. +type GenRepo struct { + Gen + Branches map[string]Gen `json:"branches,omitempty"` +} + +// GetOrg returns the org config after merging in any global config. +func (t GenConfigTree) GetOrg(name string) *GenOrg { + o, ok := t.Orgs[name] + if ok { + o.Gen = t.Apply(o.Gen) + } else { + o.Gen = t.Gen + } + return &o +} + +// GetRepo returns the repo config after merging in any org config. +func (o GenOrg) GetRepo(name string) *GenRepo { + r, ok := o.Repos[name] + if ok { + r.Gen = o.Apply(r.Gen) + } else { + r.Gen = o.Gen + } + return &r +} + +// GetBranch returns the branch config after merging in any repo config. +func (r GenRepo) GetBranch(name string) *Gen { + b, ok := r.Branches[name] + if ok { + b = r.Apply(b) + } else { + b = r.Gen + } + return &b +} + +// BranchOptions returns the plugin configuration for a given org/repo/branch. +func (t *GenConfigTree) BranchOptions(org, repo, branch string) *Gen { + return t.GetOrg(org).GetRepo(repo).GetBranch(branch) +} + +// RepoOptions returns the plugin configuration for a given org/repo. +func (t *GenConfigTree) RepoOptions(org, repo string) *Gen { + return &t.GetOrg(org).GetRepo(repo).Gen +} + +// OrgOptions returns the plugin configuration for a given org. +func (t *GenConfigTree) OrgOptions(org string) *Gen { + return &t.GetOrg(org).Gen +} diff --git a/prow/plugins/helpers.go b/prow/plugins/helpers.go new file mode 100644 index 000000000000..41154d677853 --- /dev/null +++ b/prow/plugins/helpers.go @@ -0,0 +1,70 @@ +/* +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 + +// Gen is a generic type used in ConfigTree as a leaf +type Gen interface { + Apply(Gen) Gen +} + +// Apply returns a policy that merges the child into the parent +func (parent Approve) Apply(child Approve) Approve { + new := Approve{ + IssueRequired: child.IssueRequired, + RequireSelfApproval: selectBool(parent.RequireSelfApproval, child.RequireSelfApproval), + LgtmActsAsApprove: child.LgtmActsAsApprove, + IgnoreReviewState: selectBool(parent.IgnoreReviewState, child.IgnoreReviewState), + CommandHelpLink: child.CommandHelpLink, + PrProcessLink: child.PrProcessLink, + } + return new +} + +// Apply returns a policy tree that merges the child into the parent +func (parent ApproveConfigTree) Apply(child ApproveConfigTree) ApproveConfigTree { + parent.Approve = parent.Approve.Apply(child.Approve) + for org, childOrg := range child.Orgs { + if parentOrg, ok := parent.Orgs[org]; ok { + parentOrg.Approve = parentOrg.Approve.Apply(childOrg.Approve) + for repo, childRepo := range childOrg.Repos { + if parentRepo, ok := parentOrg.Repos[repo]; ok { + parentRepo.Approve = parentRepo.Approve.Apply(childRepo.Approve) + 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 +} diff --git a/prow/plugins/zz_generated_approve_configtree.go b/prow/plugins/zz_generated_approve_configtree.go new file mode 100644 index 000000000000..222bcfb6729e --- /dev/null +++ b/prow/plugins/zz_generated_approve_configtree.go @@ -0,0 +1,83 @@ +/* +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 + +// ApproveConfigTree specifies the global generic config for a plugin. +type ApproveConfigTree struct { + Approve + Orgs map[string]ApproveOrg `json:"orgs,omitempty"` +} + +// ApproveOrg holds the default config for an entire org, as well as any repo overrides. +type ApproveOrg struct { + Approve + Repos map[string]ApproveRepo `json:"repos,omitempty"` +} + +// ApproveRepo holds the default config for all branches in a repo, as well as specific branch overrides. +type ApproveRepo struct { + Approve + Branches map[string]Approve `json:"branches,omitempty"` +} + +// GetOrg returns the org config after merging in any global config. +func (t ApproveConfigTree) GetOrg(name string) *ApproveOrg { + o, ok := t.Orgs[name] + if ok { + o.Approve = t.Approve.Apply(o.Approve) + } else { + o.Approve = t.Approve + } + return &o +} + +// GetRepo returns the repo config after merging in any org config. +func (o ApproveOrg) GetRepo(name string) *ApproveRepo { + r, ok := o.Repos[name] + if ok { + r.Approve = o.Apply(r.Approve) + } else { + r.Approve = o.Approve + } + return &r +} + +// GetBranch returns the branch config after merging in any repo config. +func (r ApproveRepo) GetBranch(name string) *Approve { + b, ok := r.Branches[name] + if ok { + b = r.Apply(b) + } else { + b = r.Approve + } + return &b +} + +// BranchOptions returns the plugin configuration for a given org/repo/branch. +func (t *ApproveConfigTree) BranchOptions(org, repo, branch string) *Approve { + return t.GetOrg(org).GetRepo(repo).GetBranch(branch) +} + +// RepoOptions returns the plugin configuration for a given org/repo. +func (t *ApproveConfigTree) RepoOptions(org, repo string) *Approve { + return &t.GetOrg(org).GetRepo(repo).Approve +} + +// OrgOptions returns the plugin configuration for a given org. +func (t *ApproveConfigTree) OrgOptions(org string) *Approve { + return &t.GetOrg(org).Approve +} From 2cf8b5409df75169d0af8ae3ecdde12948512973 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Fri, 13 May 2022 12:23:01 +0200 Subject: [PATCH 2/5] Use generics --- go.mod | 14 ++-- go.sum | 21 +++-- prow/plugins/approve/approve.go | 2 +- prow/plugins/approve/approve_test.go | 4 +- prow/plugins/config.go | 48 ++++++----- prow/plugins/config_test.go | 18 ++-- .../{configtree_template.go => configtree.go} | 56 +++++++------ ..._configtree_test.go => configtree_test.go} | 44 ++++++---- prow/plugins/helpers.go | 27 +++--- .../zz_generated_approve_configtree.go | 83 ------------------- 10 files changed, 129 insertions(+), 188 deletions(-) rename prow/plugins/{configtree_template.go => configtree.go} (51%) rename prow/plugins/{approve_configtree_test.go => configtree_test.go} (89%) delete mode 100644 prow/plugins/zz_generated_approve_configtree.go diff --git a/go.mod b/go.mod index 5961b2897f5b..823e2d3e320e 100644 --- a/go.mod +++ b/go.mod @@ -85,14 +85,14 @@ require ( go.uber.org/zap v1.19.0 go4.org v0.0.0-20201209231011-d4a079459e60 gocloud.dev v0.19.0 - golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b + golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 - golang.org/x/net v0.0.0-20210520170846-37e1c6afe023 + golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c - golang.org/x/text v0.3.6 + golang.org/x/text v0.3.7 golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac - golang.org/x/tools v0.1.5 + golang.org/x/tools v0.1.10 google.golang.org/api v0.44.0 google.golang.org/genproto v0.0.0-20210602131652-f16073e35f0c google.golang.org/protobuf v1.26.0 @@ -187,10 +187,10 @@ require ( go.opencensus.io v0.23.0 // indirect go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect - golang.org/x/mod v0.4.2 // indirect - golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 // indirect + golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect + golang.org/x/sys v0.0.0-20220422013727-9388b58f7150 // indirect golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect - golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect + golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f // indirect gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/grpc v1.38.0 // indirect diff --git a/go.sum b/go.sum index 9e3cc19e2e1e..500ddb8b5414 100644 --- a/go.sum +++ b/go.sum @@ -1520,8 +1520,9 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= -golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b h1:7mWr3k41Qtv8XlltBkDkl8LoP3mpSgBW8BUoxtEdbXg= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg= +golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1554,8 +1555,9 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o= +golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1620,8 +1622,9 @@ golang.org/x/net v0.0.0-20210326060303-6b1517762897/go.mod h1:uSPa2vr4CLtc/ILN5o golang.org/x/net v0.0.0-20210331212208-0fccb6fa2b5c/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= -golang.org/x/net v0.0.0-20210520170846-37e1c6afe023 h1:ADo5wSpq2gqaCGQWzk7S5vd//0iyyLeAratkEoG5dLE= golang.org/x/net v0.0.0-20210520170846-37e1c6afe023/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f h1:OfiFi4JbukWwe3lzw+xunroH1mnC1e2Gy5cxNJApiSY= +golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180724155351-3d292e4d0cdc/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -1748,8 +1751,9 @@ golang.org/x/sys v0.0.0-20210502180810-71e4cd670f79/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 h1:c8PlLMqBbOHoqtjteWm5/kbe6rNY2pbRfbIMVnepueo= golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220422013727-9388b58f7150 h1:xHms4gcpe1YE7A3yIllJXP16CMAGuqwO2lX1mTyyRRc= +golang.org/x/sys v0.0.0-20220422013727-9388b58f7150/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d h1:SZxvLBoTP5yHO3Frd4z4vrF+DBX9vMVanchswa69toE= @@ -1763,8 +1767,9 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -1876,13 +1881,15 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.10 h1:QjFRCZxdOhBJ/UNgnBZLbNV13DlbnK0quyivTnXJM20= +golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f h1:GGU+dLjvlC3qDwqYgL6UgRmHXhOOgns0bZu2Ty5mm6U= +golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= gomodules.xyz/jsonpatch/v2 v2.1.0/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= gomodules.xyz/jsonpatch/v2 v2.2.0 h1:4pT439QV83L+G9FkcCriY6EkpcK6r6bK+A5FBUMI7qY= diff --git a/prow/plugins/approve/approve.go b/prow/plugins/approve/approve.go index 14960dbe1808..dfd18d012e7e 100644 --- a/prow/plugins/approve/approve.go +++ b/prow/plugins/approve/approve.go @@ -114,7 +114,7 @@ func helpProvider(config *plugins.Configuration, enabledRepos []config.OrgRepo) } yamlSnippet, err := plugins.CommentMap.GenYaml(&plugins.Configuration{ - Approve: plugins.ApproveConfigTree{}, + Approve: plugins.ConfigTree[plugins.Approve]{}, }) if err != nil { logrus.WithError(err).Warnf("cannot generate comments for %s plugin", PluginName) diff --git a/prow/plugins/approve/approve_test.go b/prow/plugins/approve/approve_test.go index 2a155073584b..0acea4455307 100644 --- a/prow/plugins/approve/approve_test.go +++ b/prow/plugins/approve/approve_test.go @@ -1954,8 +1954,8 @@ func TestHelpProvider(t *testing.T) { { name: "All configs enabled", config: &plugins.Configuration{ - Approve: plugins.ApproveConfigTree{ - Orgs: map[string]plugins.ApproveOrg{ + Approve: plugins.ConfigTree[plugins.Approve]{ + Orgs: map[string]plugins.Org[plugins.Approve]{ "org2": {}, }, }, diff --git a/prow/plugins/config.go b/prow/plugins/config.go index 600ab29fb0dd..acb376d37b93 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -63,7 +63,7 @@ type Configuration struct { Owners Owners `json:"owners,omitempty"` // Built-in plugins specific configuration. - Approve ApproveConfigTree `json:"approve,omitempty"` + Approve ConfigTree[Approve] `json:"approve,omitempty"` Blockades []Blockade `json:"blockades,omitempty"` Blunderbuss Blunderbuss `json:"blunderbuss,omitempty"` Bugzilla Bugzilla `json:"bugzilla,omitempty"` @@ -816,34 +816,40 @@ func (r RequireMatchingLabel) Describe() string { return str.String() } -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, +func oldToNewConfig[T ProwConfig](old DeprecatedApprove) *T { + a := new(T) + switch v := any(a).(type) { + case *Approve: + *v = Approve{ + IgnoreReviewState: old.IgnoreReviewState, + IssueRequired: old.IssueRequired, + LgtmActsAsApprove: old.LgtmActsAsApprove, + RequireSelfApproval: old.RequireSelfApproval, + CommandHelpLink: old.CommandHelpLink, + PrProcessLink: old.PrProcessLink, + } + default: + logrus.Error("unknown type for ConfigTree object", a) } - return &a + return a } -func oldToNewApproveConfig(old []DeprecatedApprove) ApproveConfigTree { - a := ApproveConfigTree{} - a.Orgs = make(map[string]ApproveOrg) +func oldToConfigTree[T ProwConfig](old []DeprecatedApprove) ConfigTree[T] { + a := ConfigTree[T]{} + a.Orgs = make(map[string]Org[T]) for _, entry := range old { for _, repo := range entry.Repos { s := strings.Split(repo, "/") ao := a.Orgs[s[0]] switch len(s) { case 1: - ao.Approve = *oldToNewApprove(entry) + ao.Config = *oldToNewConfig[T](entry) case 2: if ao.Repos == nil { - ao.Repos = make(map[string]ApproveRepo) + ao.Repos = make(map[string]Repo[T]) } ar := ao.Repos[s[1]] - ar.Approve = *oldToNewApprove(entry) + ar.Config = *oldToNewConfig[T](entry) ao.Repos[s[1]] = ar } a.Orgs[s[0]] = ao @@ -852,20 +858,20 @@ func oldToNewApproveConfig(old []DeprecatedApprove) ApproveConfigTree { return a } -type approveWithoutUnmarshaler ApproveConfigTree +type withoutUnmarshaler[T ProwConfig] ConfigTree[T] var warnTriggerDeprecatedApprove time.Time -func (a *ApproveConfigTree) UnmarshalJSON(d []byte) error { +func (a *ConfigTree[T]) UnmarshalJSON(d []byte) error { 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") - *a = oldToNewApproveConfig(oldApprove) + *a = oldToConfigTree[T](oldApprove) return nil } - var target approveWithoutUnmarshaler + var target withoutUnmarshaler[T] err := yaml.Unmarshal(d, &target) - *a = ApproveConfigTree(target) + *a = ConfigTree[T](target) return err } diff --git a/prow/plugins/config_test.go b/prow/plugins/config_test.go index 3780f2114267..1bc43f78246a 100644 --- a/prow/plugins/config_test.go +++ b/prow/plugins/config_test.go @@ -223,14 +223,16 @@ func TestSetApproveDefaults(t *testing.T) { configYaml := ` --- approve: - commandHelpLink: https://go.k8s.io/bot-commands - pr_process_link: https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process + 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: - commandHelpLink: https://prow.k8s.io/command-help - pr_process_link: https://github.com/kubernetes/community/blob/427ccfbc7d423d8763ed756f3b8c888b7de3cf34/contributors/guide/pull-requests.md + 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 { @@ -1999,7 +2001,7 @@ func TestHasConfigFor(t *testing.T) { resultGenerator: func(fuzzedConfig *Configuration) (toCheck *Configuration, expectGlobal bool, expectOrgs sets.String, expectRepos sets.String) { fuzzedConfig.Plugins = nil fuzzedConfig.Bugzilla = Bugzilla{} - fuzzedConfig.Approve = ApproveConfigTree{} + fuzzedConfig.Approve = ConfigTree[Approve]{} fuzzedConfig.Label.RestrictedLabels = nil fuzzedConfig.Lgtm = nil fuzzedConfig.Triggers = nil @@ -2175,9 +2177,9 @@ func TestMergeFrom(t *testing.T) { }{ { name: "Approve config gets merged", - in: Configuration{Approve: oldToNewApproveConfig([]DeprecatedApprove{{Repos: []string{"foo/bar"}}})}, - supplementalConfigs: []Configuration{{Approve: oldToNewApproveConfig([]DeprecatedApprove{{Repos: []string{"foo/baz"}}})}}, - expected: Configuration{Approve: oldToNewApproveConfig([]DeprecatedApprove{ + in: Configuration{Approve: oldToConfigTree[Approve]([]DeprecatedApprove{{Repos: []string{"foo/bar"}}})}, + supplementalConfigs: []Configuration{{Approve: oldToConfigTree[Approve]([]DeprecatedApprove{{Repos: []string{"foo/baz"}}})}}, + expected: Configuration{Approve: oldToConfigTree[Approve]([]DeprecatedApprove{ {Repos: []string{"foo/bar"}}, {Repos: []string{"foo/baz"}}, })}, diff --git a/prow/plugins/configtree_template.go b/prow/plugins/configtree.go similarity index 51% rename from prow/plugins/configtree_template.go rename to prow/plugins/configtree.go index 2685f51f70a3..19e882fc29c3 100644 --- a/prow/plugins/configtree_template.go +++ b/prow/plugins/configtree.go @@ -16,68 +16,72 @@ limitations under the License. package plugins -// GenConfigTree specifies the global generic config for a plugin. -type GenConfigTree struct { - Gen - Orgs map[string]GenOrg `json:"orgs,omitempty"` +type ProwConfig interface { + Apply(config ProwConfig) ProwConfig } -// GenOrg holds the default config for an entire org, as well as any repo overrides. -type GenOrg struct { - Gen - Repos map[string]GenRepo `json:"repos,omitempty"` +// ConfigTree specifies the global generic config for a plugin. +type ConfigTree[T ProwConfig] struct { + Config T + Orgs map[string]Org[T] `json:"orgs,omitempty"` } -// GenRepo holds the default config for all branches in a repo, as well as specific branch overrides. -type GenRepo struct { - Gen - Branches map[string]Gen `json:"branches,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 GenConfigTree) GetOrg(name string) *GenOrg { +func (t ConfigTree[T]) GetOrg(name string) *Org[T] { o, ok := t.Orgs[name] if ok { - o.Gen = t.Apply(o.Gen) + o.Config = t.Config.Apply(o.Config).(T) } else { - o.Gen = t.Gen + o.Config = t.Config } return &o } // GetRepo returns the repo config after merging in any org config. -func (o GenOrg) GetRepo(name string) *GenRepo { +func (o Org[T]) GetRepo(name string) *Repo[T] { r, ok := o.Repos[name] if ok { - r.Gen = o.Apply(r.Gen) + r.Config = o.Config.Apply(r.Config).(T) } else { - r.Gen = o.Gen + r.Config = o.Config } return &r } // GetBranch returns the branch config after merging in any repo config. -func (r GenRepo) GetBranch(name string) *Gen { +func (r Repo[T]) GetBranch(name string) *T { b, ok := r.Branches[name] if ok { - b = r.Apply(b) + b = r.Config.Apply(b).(T) } else { - b = r.Gen + b = r.Config } return &b } // BranchOptions returns the plugin configuration for a given org/repo/branch. -func (t *GenConfigTree) BranchOptions(org, repo, branch string) *Gen { +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 *GenConfigTree) RepoOptions(org, repo string) *Gen { - return &t.GetOrg(org).GetRepo(repo).Gen +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 *GenConfigTree) OrgOptions(org string) *Gen { - return &t.GetOrg(org).Gen +func (t *ConfigTree[T]) OrgOptions(org string) *T { + return &t.GetOrg(org).Config } diff --git a/prow/plugins/approve_configtree_test.go b/prow/plugins/configtree_test.go similarity index 89% rename from prow/plugins/approve_configtree_test.go rename to prow/plugins/configtree_test.go index 35520b675f1e..e7b3f1619609 100644 --- a/prow/plugins/approve_configtree_test.go +++ b/prow/plugins/configtree_test.go @@ -137,10 +137,11 @@ func TestApproveConfigTree(t *testing.T) { { name: "approve no orgs", config: []byte(` -issue_required: no -require_self_approval: yes -lgtm_acts_as_approve: no -ignore_review_state: yes +config: + issue_required: no + require_self_approval: yes + lgtm_acts_as_approve: no + ignore_review_state: yes `), expectedApproveBranch: &Approve{ IssueRequired: no, @@ -166,14 +167,18 @@ ignore_review_state: yes config: []byte(` orgs: bazelbuild: - ignore_review_state: no + config: + ignore_review_state: no kubernetes: - lgtm_acts_as_approve: yes + config: + lgtm_acts_as_approve: yes repos: kops: - lgtm_acts_as_approve: no + config: + lgtm_acts_as_approve: no kubernetes: - require_self_approval: yes + config: + require_self_approval: yes `), expectedApproveBranch: &Approve{ RequireSelfApproval: yes, @@ -188,20 +193,25 @@ orgs: { name: "approve full", config: []byte(` -issue_required: no -require_self_approval: no -lgtm_acts_as_approve: no -ignore_review_state: yes +config: + issue_required: no + require_self_approval: no + lgtm_acts_as_approve: no + ignore_review_state: yes orgs: bazelbuild: - ignore_review_state: no + config: + ignore_review_state: no kubernetes: - lgtm_acts_as_approve: yes + config: + lgtm_acts_as_approve: yes repos: kops: - lgtm_acts_as_approve: no + config: + lgtm_acts_as_approve: no kubernetes: - require_self_approval: yes + config: + require_self_approval: yes branches: master: require_self_approval: no @@ -223,7 +233,7 @@ orgs: } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - var tree ApproveConfigTree + var tree ConfigTree[Approve] if err := yaml.Unmarshal(c.config, &tree); err != nil { t.Errorf("error unmarshaling config: %v", err) } diff --git a/prow/plugins/helpers.go b/prow/plugins/helpers.go index 41154d677853..e0a5c11ea482 100644 --- a/prow/plugins/helpers.go +++ b/prow/plugins/helpers.go @@ -16,33 +16,28 @@ limitations under the License. package plugins -// Gen is a generic type used in ConfigTree as a leaf -type Gen interface { - Apply(Gen) Gen -} - // Apply returns a policy that merges the child into the parent -func (parent Approve) Apply(child Approve) Approve { +func (parent Approve) Apply(child ProwConfig) ProwConfig { new := Approve{ - IssueRequired: child.IssueRequired, - RequireSelfApproval: selectBool(parent.RequireSelfApproval, child.RequireSelfApproval), - LgtmActsAsApprove: child.LgtmActsAsApprove, - IgnoreReviewState: selectBool(parent.IgnoreReviewState, child.IgnoreReviewState), - CommandHelpLink: child.CommandHelpLink, - PrProcessLink: child.PrProcessLink, + 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 ApproveConfigTree) Apply(child ApproveConfigTree) ApproveConfigTree { - parent.Approve = parent.Approve.Apply(child.Approve) +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.Approve = parentOrg.Approve.Apply(childOrg.Approve) + parentOrg.Config = parentOrg.Config.Apply(childOrg.Config).(T) for repo, childRepo := range childOrg.Repos { if parentRepo, ok := parentOrg.Repos[repo]; ok { - parentRepo.Approve = parentRepo.Approve.Apply(childRepo.Approve) + parentRepo.Config = parentRepo.Config.Apply(childRepo.Config).(T) for branch, childBranch := range childRepo.Branches { if parentBranch, ok := parentRepo.Branches[branch]; ok { parentBranch.Apply(childBranch) diff --git a/prow/plugins/zz_generated_approve_configtree.go b/prow/plugins/zz_generated_approve_configtree.go deleted file mode 100644 index 222bcfb6729e..000000000000 --- a/prow/plugins/zz_generated_approve_configtree.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -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 - -// ApproveConfigTree specifies the global generic config for a plugin. -type ApproveConfigTree struct { - Approve - Orgs map[string]ApproveOrg `json:"orgs,omitempty"` -} - -// ApproveOrg holds the default config for an entire org, as well as any repo overrides. -type ApproveOrg struct { - Approve - Repos map[string]ApproveRepo `json:"repos,omitempty"` -} - -// ApproveRepo holds the default config for all branches in a repo, as well as specific branch overrides. -type ApproveRepo struct { - Approve - Branches map[string]Approve `json:"branches,omitempty"` -} - -// GetOrg returns the org config after merging in any global config. -func (t ApproveConfigTree) GetOrg(name string) *ApproveOrg { - o, ok := t.Orgs[name] - if ok { - o.Approve = t.Approve.Apply(o.Approve) - } else { - o.Approve = t.Approve - } - return &o -} - -// GetRepo returns the repo config after merging in any org config. -func (o ApproveOrg) GetRepo(name string) *ApproveRepo { - r, ok := o.Repos[name] - if ok { - r.Approve = o.Apply(r.Approve) - } else { - r.Approve = o.Approve - } - return &r -} - -// GetBranch returns the branch config after merging in any repo config. -func (r ApproveRepo) GetBranch(name string) *Approve { - b, ok := r.Branches[name] - if ok { - b = r.Apply(b) - } else { - b = r.Approve - } - return &b -} - -// BranchOptions returns the plugin configuration for a given org/repo/branch. -func (t *ApproveConfigTree) BranchOptions(org, repo, branch string) *Approve { - return t.GetOrg(org).GetRepo(repo).GetBranch(branch) -} - -// RepoOptions returns the plugin configuration for a given org/repo. -func (t *ApproveConfigTree) RepoOptions(org, repo string) *Approve { - return &t.GetOrg(org).GetRepo(repo).Approve -} - -// OrgOptions returns the plugin configuration for a given org. -func (t *ApproveConfigTree) OrgOptions(org string) *Approve { - return &t.GetOrg(org).Approve -} From 3a3cf50fce5952571e2de55c12965b4ddff3e115 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Thu, 19 May 2022 16:12:00 +0200 Subject: [PATCH 3/5] go mod tidy --- go.mod | 14 +++++++------- go.sum | 21 +++++++-------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index 823e2d3e320e..5961b2897f5b 100644 --- a/go.mod +++ b/go.mod @@ -85,14 +85,14 @@ require ( go.uber.org/zap v1.19.0 go4.org v0.0.0-20201209231011-d4a079459e60 gocloud.dev v0.19.0 - golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 + golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 - golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f + golang.org/x/net v0.0.0-20210520170846-37e1c6afe023 golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602 golang.org/x/sync v0.0.0-20210220032951-036812b2e83c - golang.org/x/text v0.3.7 + golang.org/x/text v0.3.6 golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac - golang.org/x/tools v0.1.10 + golang.org/x/tools v0.1.5 google.golang.org/api v0.44.0 google.golang.org/genproto v0.0.0-20210602131652-f16073e35f0c google.golang.org/protobuf v1.26.0 @@ -187,10 +187,10 @@ require ( go.opencensus.io v0.23.0 // indirect go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect - golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect - golang.org/x/sys v0.0.0-20220422013727-9388b58f7150 // indirect + golang.org/x/mod v0.4.2 // indirect + golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 // indirect golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect - golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f // indirect + golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/grpc v1.38.0 // indirect diff --git a/go.sum b/go.sum index 500ddb8b5414..9e3cc19e2e1e 100644 --- a/go.sum +++ b/go.sum @@ -1520,9 +1520,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= +golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b h1:7mWr3k41Qtv8XlltBkDkl8LoP3mpSgBW8BUoxtEdbXg= golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= -golang.org/x/crypto v0.0.0-20210921155107-089bfa567519 h1:7I4JAnoQBe7ZtJcBaYHi5UtiO8tQHbUSXxL+pnGRANg= -golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1555,9 +1554,8 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.1-0.20200828183125-ce943fd02449/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o= -golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= golang.org/x/net v0.0.0-20170114055629-f2499483f923/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -1622,9 +1620,8 @@ golang.org/x/net v0.0.0-20210326060303-6b1517762897/go.mod h1:uSPa2vr4CLtc/ILN5o golang.org/x/net v0.0.0-20210331212208-0fccb6fa2b5c/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= +golang.org/x/net v0.0.0-20210520170846-37e1c6afe023 h1:ADo5wSpq2gqaCGQWzk7S5vd//0iyyLeAratkEoG5dLE= golang.org/x/net v0.0.0-20210520170846-37e1c6afe023/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f h1:OfiFi4JbukWwe3lzw+xunroH1mnC1e2Gy5cxNJApiSY= -golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180724155351-3d292e4d0cdc/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -1751,9 +1748,8 @@ golang.org/x/sys v0.0.0-20210502180810-71e4cd670f79/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 h1:c8PlLMqBbOHoqtjteWm5/kbe6rNY2pbRfbIMVnepueo= golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220422013727-9388b58f7150 h1:xHms4gcpe1YE7A3yIllJXP16CMAGuqwO2lX1mTyyRRc= -golang.org/x/sys v0.0.0-20220422013727-9388b58f7150/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d h1:SZxvLBoTP5yHO3Frd4z4vrF+DBX9vMVanchswa69toE= @@ -1767,9 +1763,8 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= -golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -1881,15 +1876,13 @@ golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.5 h1:ouewzE6p+/VEB31YYnTbEJdi8pFqKp4P4n85vwo3DHA= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.1.10 h1:QjFRCZxdOhBJ/UNgnBZLbNV13DlbnK0quyivTnXJM20= -golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f h1:GGU+dLjvlC3qDwqYgL6UgRmHXhOOgns0bZu2Ty5mm6U= -golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.0.1/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= gomodules.xyz/jsonpatch/v2 v2.1.0/go.mod h1:IhYNNY4jnS53ZnfE4PAmpKtDpTCj1JFXc+3mwe7XcUU= gomodules.xyz/jsonpatch/v2 v2.2.0 h1:4pT439QV83L+G9FkcCriY6EkpcK6r6bK+A5FBUMI7qY= From ee06ce08dadd75d1d9266810fe0db063495d4738 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Fri, 20 May 2022 09:57:36 +0200 Subject: [PATCH 4/5] remove extra generics --- prow/plugins/config.go | 51 ++++++++++++++++++------------------- prow/plugins/config_test.go | 6 ++--- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/prow/plugins/config.go b/prow/plugins/config.go index acb376d37b93..204daeb44211 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -816,40 +816,34 @@ func (r RequireMatchingLabel) Describe() string { return str.String() } -func oldToNewConfig[T ProwConfig](old DeprecatedApprove) *T { - a := new(T) - switch v := any(a).(type) { - case *Approve: - *v = Approve{ - IgnoreReviewState: old.IgnoreReviewState, - IssueRequired: old.IssueRequired, - LgtmActsAsApprove: old.LgtmActsAsApprove, - RequireSelfApproval: old.RequireSelfApproval, - CommandHelpLink: old.CommandHelpLink, - PrProcessLink: old.PrProcessLink, - } - default: - logrus.Error("unknown type for ConfigTree object", a) +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 + return &a } -func oldToConfigTree[T ProwConfig](old []DeprecatedApprove) ConfigTree[T] { - a := ConfigTree[T]{} - a.Orgs = make(map[string]Org[T]) +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 = *oldToNewConfig[T](entry) + ao.Config = *oldToNewApprove(entry) case 2: if ao.Repos == nil { - ao.Repos = make(map[string]Repo[T]) + ao.Repos = make(map[string]Repo[Approve]) } ar := ao.Repos[s[1]] - ar.Config = *oldToNewConfig[T](entry) + ar.Config = *oldToNewApprove(entry) ao.Repos[s[1]] = ar } a.Orgs[s[0]] = ao @@ -863,11 +857,16 @@ type withoutUnmarshaler[T ProwConfig] ConfigTree[T] var warnTriggerDeprecatedApprove time.Time func (a *ConfigTree[T]) UnmarshalJSON(d []byte) error { - 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") - *a = oldToConfigTree[T](oldApprove) - return nil + 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) } var target withoutUnmarshaler[T] err := yaml.Unmarshal(d, &target) diff --git a/prow/plugins/config_test.go b/prow/plugins/config_test.go index 1bc43f78246a..7873b103c836 100644 --- a/prow/plugins/config_test.go +++ b/prow/plugins/config_test.go @@ -2177,9 +2177,9 @@ func TestMergeFrom(t *testing.T) { }{ { name: "Approve config gets merged", - in: Configuration{Approve: oldToConfigTree[Approve]([]DeprecatedApprove{{Repos: []string{"foo/bar"}}})}, - supplementalConfigs: []Configuration{{Approve: oldToConfigTree[Approve]([]DeprecatedApprove{{Repos: []string{"foo/baz"}}})}}, - expected: Configuration{Approve: oldToConfigTree[Approve]([]DeprecatedApprove{ + 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"}}, })}, From b7bf1a2f3cf721040bc52c601a160911edcbcd32 Mon Sep 17 00:00:00 2001 From: Matthias Bertschy Date: Fri, 20 May 2022 10:39:13 +0200 Subject: [PATCH 5/5] use pointer receivers --- prow/plugins/config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/prow/plugins/config.go b/prow/plugins/config.go index 204daeb44211..9a6a020d244e 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -333,28 +333,28 @@ var ( warnDependentBugTargetRelease time.Time ) -func (a Approve) AreIssueRequired() bool { +func (a *Approve) AreIssueRequired() bool { if a.IssueRequired != nil { return *a.IssueRequired } return false } -func (a Approve) ShouldLgtmActsAsApprove() bool { +func (a *Approve) ShouldLgtmActsAsApprove() bool { if a.LgtmActsAsApprove != nil { return *a.LgtmActsAsApprove } return false } -func (a Approve) HasSelfApproval() bool { +func (a *Approve) HasSelfApproval() bool { if a.RequireSelfApproval != nil { return !*a.RequireSelfApproval } return true } -func (a Approve) ConsiderReviewState() bool { +func (a *Approve) ConsiderReviewState() bool { if a.IgnoreReviewState != nil { return !*a.IgnoreReviewState }