diff --git a/prow/plugins/cla/BUILD.bazel b/prow/plugins/cla/BUILD.bazel index 93d4859c7456..5dc70627f625 100644 --- a/prow/plugins/cla/BUILD.bazel +++ b/prow/plugins/cla/BUILD.bazel @@ -14,6 +14,7 @@ go_test( "//prow/github:go_default_library", "//prow/github/fakegithub:go_default_library", "//prow/labels:go_default_library", + "//prow/plugins:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", ], ) diff --git a/prow/plugins/cla/cla.go b/prow/plugins/cla/cla.go index ab956468b5b1..dc6aa790b3c6 100644 --- a/prow/plugins/cla/cla.go +++ b/prow/plugins/cla/cla.go @@ -33,7 +33,6 @@ import ( const ( pluginName = "cla" - claContextName = "cla/linuxfoundation" cncfclaNotFoundMessage = `Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). :memo: **Please follow instructions at to sign the CLA.** @@ -70,7 +69,7 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel // The {WhoCanUse, Usage, Examples, Config} fields are omitted because this plugin cannot be // manually triggered and is not configurable. pluginHelp := &pluginhelp.PluginHelp{ - Description: "The cla plugin manages the application and removal of the 'cncf-cla' prefixed labels on pull requests as a reaction to the " + claContextName + " github status context. It is also responsible for warning unauthorized PR authors that they need to sign the CNCF CLA before their PR will be merged.", + Description: "The cla plugin manages the application and removal of the 'cncf-cla' prefixed labels on pull requests as a reaction to the EasyCLA / cla-linuxfoundation github status context. It is also responsible for warning unauthorized PR authors that they need to sign the CNCF CLA before their PR will be merged.", } pluginHelp.AddCommand(pluginhelp.Command{ Usage: "/check-cla", @@ -93,7 +92,7 @@ type gitHubClient interface { } func handleStatusEvent(pc plugins.Agent, se github.StatusEvent) error { - return handle(pc.GitHubClient, pc.Logger, se) + return handle(pc.GitHubClient, pc.Logger, se, pc.PluginConfig.CLAConfig) } // 1. Check that the status event received from the webhook is for the CNCF-CLA. @@ -101,12 +100,12 @@ func handleStatusEvent(pc plugins.Agent, se github.StatusEvent) error { // 3. For each issue that matches, check that the PR's HEAD commit hash against the commit hash for which the status // was received. This is because we only care about the status associated with the last (latest) commit in a PR. // 4. Set the corresponding CLA label if needed. -func handle(gc gitHubClient, log *logrus.Entry, se github.StatusEvent) error { +func handle(gc gitHubClient, log *logrus.Entry, se github.StatusEvent, cc plugins.CLAConfig) error { if se.State == "" || se.Context == "" { return fmt.Errorf("invalid status event delivered with empty state/context") } - if se.Context != claContextName { + if !contains(cc.CLAContextNames, se.Context) { // Not the CNCF CLA context, do not process this. return nil } @@ -193,10 +192,10 @@ func handle(gc gitHubClient, log *logrus.Entry, se github.StatusEvent) error { } func handleCommentEvent(pc plugins.Agent, ce github.GenericCommentEvent) error { - return handleComment(pc.GitHubClient, pc.Logger, &ce) + return handleComment(pc.GitHubClient, pc.Logger, &ce, pc.PluginConfig.CLAConfig) } -func handleComment(gc gitHubClient, log *logrus.Entry, e *github.GenericCommentEvent) error { +func handleComment(gc gitHubClient, log *logrus.Entry, e *github.GenericCommentEvent, cc plugins.CLAConfig) error { // Only consider open PRs and new comments. if e.IssueState != "open" || e.Action != github.GenericCommentActionCreated { return nil @@ -240,9 +239,8 @@ func handleComment(gc gitHubClient, log *logrus.Entry, e *github.GenericCommentE } for _, status := range combined.Statuses { - - // Only consider "cla/linuxfoundation" status. - if status.Context == claContextName { + // Only consider "cla/linuxfoundation" or "EasyCLA" status + if contains(cc.CLAContextNames, status.Context) { // Success state implies that the cla exists, so label should be cncf-cla:yes. if status.State == github.StatusSuccess { @@ -278,10 +276,18 @@ func handleComment(gc gitHubClient, log *logrus.Entry, e *github.GenericCommentE } } } - - // No need to consider other contexts once you find the one you need. - break } } return nil } + +// contains checks if a string is present in a slice +func contains(s []string, str string) bool { + for _, v := range s { + if v == str { + return true + } + } + + return false +} diff --git a/prow/plugins/cla/cla_test.go b/prow/plugins/cla/cla_test.go index 48ac237526da..9d28d2bc6234 100644 --- a/prow/plugins/cla/cla_test.go +++ b/prow/plugins/cla/cla_test.go @@ -26,6 +26,7 @@ import ( "k8s.io/test-infra/prow/github" "k8s.io/test-infra/prow/github/fakegithub" "k8s.io/test-infra/prow/labels" + "k8s.io/test-infra/prow/plugins" ) func TestCLALabels(t *testing.T) { @@ -140,6 +141,34 @@ func TestCLALabels(t *testing.T) { addedLabels: []string{fmt.Sprintf("/#3:%s", labels.ClaNo)}, removedLabels: []string{fmt.Sprintf("/#3:%s", labels.ClaYes)}, }, + { + name: "EasyCLA status failure removes \"cncf-cla: yes\" label", + context: "EasyCLA", + state: "failure", + statusSHA: "a", + issues: []github.Issue{ + {Number: 3, State: "open", Labels: []github.Label{{Name: labels.ClaYes}}}, + }, + pullRequests: []github.PullRequest{ + {Number: 3, Head: github.PullRequestBranch{SHA: "a"}}, + }, + addedLabels: []string{fmt.Sprintf("/#3:%s", labels.ClaNo)}, + removedLabels: []string{fmt.Sprintf("/#3:%s", labels.ClaYes)}, + }, + { + name: "EasyCLA status success removes \"cncf-cla: no\" label", + context: "EasyCLA", + state: "success", + statusSHA: "a", + issues: []github.Issue{ + {Number: 3, State: "open", Labels: []github.Label{{Name: labels.ClaNo}}}, + }, + pullRequests: []github.PullRequest{ + {Number: 3, Head: github.PullRequestBranch{SHA: "a"}}, + }, + addedLabels: []string{fmt.Sprintf("/#3:%s", labels.ClaYes)}, + removedLabels: []string{fmt.Sprintf("/#3:%s", labels.ClaNo)}, + }, } for _, tc := range testcases { pullRequests := make(map[int]*github.PullRequest) @@ -161,7 +190,10 @@ func TestCLALabels(t *testing.T) { SHA: tc.statusSHA, State: tc.state, } - if err := handle(fc, logrus.WithField("plugin", pluginName), se); err != nil { + cc := plugins.CLAConfig{ + CLAContextNames: []string{"cla/linuxfoundation", "EasyCLA"}, + } + if err := handle(fc, logrus.WithField("plugin", pluginName), se, cc); err != nil { t.Errorf("For case %s, didn't expect error from cla plugin: %v", tc.name, err) continue } @@ -179,8 +211,7 @@ func TestCLALabels(t *testing.T) { func TestCheckCLA(t *testing.T) { var testcases = []struct { name string - context string - state string + contexts map[string]string issueState string SHA string action string @@ -193,9 +224,10 @@ func TestCheckCLA(t *testing.T) { removedLabel string }{ { - name: "ignore non cla/linuxfoundation context", - context: "random/context", - state: "success", + name: "ignore non cla/linuxfoundation context", + contexts: map[string]string{ + "random/context": "success", + }, issueState: "open", SHA: "sha", action: "created", @@ -205,9 +237,10 @@ func TestCheckCLA(t *testing.T) { }, }, { - name: "ignore non open PRs", - context: "cla/linuxfoundation", - state: "success", + name: "ignore non open PRs", + contexts: map[string]string{ + "cla/linuxfoundation": "success", + }, issueState: "closed", SHA: "sha", action: "created", @@ -217,9 +250,10 @@ func TestCheckCLA(t *testing.T) { }, }, { - name: "ignore non /check-cla comments", - context: "cla/linuxfoundation", - state: "success", + name: "ignore non /check-cla comments", + contexts: map[string]string{ + "cla/linuxfoundation": "success", + }, issueState: "open", SHA: "sha", action: "created", @@ -229,9 +263,10 @@ func TestCheckCLA(t *testing.T) { }, }, { - name: "do nothing on when status state is \"pending\"", - context: "cla/linuxfoundation", - state: "pending", + name: "do nothing on when status state is \"pending\"", + contexts: map[string]string{ + "cla/linuxfoundation": "pending", + }, issueState: "open", SHA: "sha", action: "created", @@ -241,9 +276,10 @@ func TestCheckCLA(t *testing.T) { }, }, { - name: "cla/linuxfoundation status adds the cla-yes label when its state is \"success\"", - context: "cla/linuxfoundation", - state: "success", + name: "EasyCLA status adds the cla-yes label when its state is \"success\"", + contexts: map[string]string{ + "EasyCLA": "success", + }, issueState: "open", SHA: "sha", action: "created", @@ -255,9 +291,10 @@ func TestCheckCLA(t *testing.T) { addedLabel: fmt.Sprintf("/#3:%s", labels.ClaYes), }, { - name: "cla/linuxfoundation status adds the cla-yes label and removes cla-no label when its state is \"success\"", - context: "cla/linuxfoundation", - state: "success", + name: "EasyCLA status adds the cla-yes label and removes cla-no label when its state is \"success\"", + contexts: map[string]string{ + "EasyCLA": "success", + }, issueState: "open", SHA: "sha", action: "created", @@ -271,9 +308,10 @@ func TestCheckCLA(t *testing.T) { removedLabel: fmt.Sprintf("/#3:%s", labels.ClaNo), }, { - name: "cla/linuxfoundation status adds the cla-no label when its state is \"failure\"", - context: "cla/linuxfoundation", - state: "failure", + name: "cla/linuxfoundation status adds the cla-no label when its state is \"failure\"", + contexts: map[string]string{ + "cla/linuxfoundation": "failure", + }, issueState: "open", SHA: "sha", action: "created", @@ -285,9 +323,10 @@ func TestCheckCLA(t *testing.T) { addedLabel: fmt.Sprintf("/#3:%s", labels.ClaNo), }, { - name: "cla/linuxfoundation status adds the cla-no label and removes cla-yes label when its state is \"failure\"", - context: "cla/linuxfoundation", - state: "failure", + name: "EasyCLA status adds the cla-no label and removes cla-yes label when its state is \"failure\"", + contexts: map[string]string{ + "EasyCLA": "failure", + }, issueState: "open", SHA: "sha", action: "created", @@ -301,9 +340,10 @@ func TestCheckCLA(t *testing.T) { removedLabel: fmt.Sprintf("/#3:%s", labels.ClaYes), }, { - name: "cla/linuxfoundation status retains the cla-yes label and removes cla-no label when its state is \"success\"", - context: "cla/linuxfoundation", - state: "success", + name: "cla/linuxfoundation status retains the cla-yes label and removes cla-no label when its state is \"success\"", + contexts: map[string]string{ + "cla/linuxfoundation": "success", + }, issueState: "open", SHA: "sha", action: "created", @@ -317,9 +357,10 @@ func TestCheckCLA(t *testing.T) { removedLabel: fmt.Sprintf("/#3:%s", labels.ClaNo), }, { - name: "cla/linuxfoundation status retains the cla-no label and removes cla-yes label when its state is \"failure\"", - context: "cla/linuxfoundation", - state: "failure", + name: "cla/linuxfoundation status retains the cla-no label and removes cla-yes label when its state is \"failure\"", + contexts: map[string]string{ + "cla/linuxfoundation": "failure", + }, issueState: "open", SHA: "sha", action: "created", @@ -332,6 +373,42 @@ func TestCheckCLA(t *testing.T) { removedLabel: fmt.Sprintf("/#3:%s", labels.ClaYes), }, + { + name: "cla/linuxfoundation status retains the cla-no label and removes cla-yes label when its state is \"failure\"", + contexts: map[string]string{ + "cla/linuxfoundation": "failure", + "EasyCLA": "success", + }, + issueState: "open", + SHA: "sha", + action: "created", + body: "/check-cla", + pullRequests: []github.PullRequest{ + {Number: 3, Head: github.PullRequestBranch{SHA: "sha"}}, + }, + hasCLANo: true, + hasCLAYes: true, + + removedLabel: fmt.Sprintf("/#3:%s", labels.ClaYes), + }, + { + name: "cla/linuxfoundation status retains the cla-yes label and removes cla-no label when its state is \"success\"", + contexts: map[string]string{ + "EasyCLA": "failure", + "cla/linuxfoundation": "success", + }, + issueState: "open", + SHA: "sha", + action: "created", + body: "/check-cla", + pullRequests: []github.PullRequest{ + {Number: 3, Head: github.PullRequestBranch{SHA: "sha"}}, + }, + hasCLANo: true, + hasCLAYes: true, + + removedLabel: fmt.Sprintf("/#3:%s", labels.ClaNo), + }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { @@ -348,11 +425,20 @@ func TestCheckCLA(t *testing.T) { Number: 3, IssueState: tc.issueState, } + cc := plugins.CLAConfig{ + CLAContextNames: []string{"cla/linuxfoundation", "EasyCLA"}, + } + var statuses []github.Status + for context, state := range tc.contexts { + statuses = append(statuses, github.Status{ + State: state, + Context: context, + }) + } + fc.CombinedStatuses = map[string]*github.CombinedStatus{ tc.SHA: { - Statuses: []github.Status{ - {State: tc.state, Context: tc.context}, - }, + Statuses: statuses, }, } if tc.hasCLAYes { @@ -361,7 +447,7 @@ func TestCheckCLA(t *testing.T) { if tc.hasCLANo { fc.IssueLabelsAdded = append(fc.IssueLabelsAdded, fmt.Sprintf("/#3:%s", labels.ClaNo)) } - if err := handleComment(fc, logrus.WithField("plugin", pluginName), e); err != nil { + if err := handleComment(fc, logrus.WithField("plugin", pluginName), e, cc); err != nil { t.Errorf("For case %s, didn't expect error from cla plugin: %v", tc.name, err) } ok := tc.addedLabel == "" diff --git a/prow/plugins/config.go b/prow/plugins/config.go index 6e39ab19fcf8..4d4d55c05aa2 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -90,6 +90,7 @@ type Configuration struct { Welcome []Welcome `json:"welcome,omitempty"` Override Override `json:"override,omitempty"` Help Help `json:"help,omitempty"` + CLAConfig CLAConfig `json:"cla_config,omitempty"` } type Help struct { @@ -482,6 +483,16 @@ type ConfigUpdater struct { GZIP bool `json:"gzip"` } +// CLAConfig contains the list of available context names +type CLAConfig struct { + // CLAContextNames is the list of status contexts to check to decide the + // CLA status of pull requests. When any status in this list fails, the + // "cncf-cla: no" GitHub label is applied. When any status in this list + // passes, the "cncf-cla: yes" GitHub label is applied. The last status in + // the list has priority. Defaults to ["cla/linuxfoundation"] + CLAContextNames []string `json:"cla_context_names,omitempty"` +} + type configUpdatedWithoutUnmarshaler ConfigUpdater func (cu *ConfigUpdater) UnmarshalJSON(d []byte) error { @@ -985,6 +996,9 @@ func (c *Configuration) setDefaults() { c.RequireMatchingLabel[i].GracePeriod = "5s" } } + if len(c.CLAConfig.CLAContextNames) == 0 { + c.CLAConfig.CLAContextNames = []string{"cla/linuxfoundation"} + } } // validatePluginsDupes will return an error if there are duplicated plugins. diff --git a/prow/plugins/plugin-config-documented.yaml b/prow/plugins/plugin-config-documented.yaml index 47c9a2bb096c..84d70f7323b5 100644 --- a/prow/plugins/plugin-config-documented.yaml +++ b/prow/plugins/plugin-config-documented.yaml @@ -316,6 +316,14 @@ cherry_pick_unapproved: # Comment is the comment added by the plugin while adding the # `do-not-merge/cherry-pick-not-approved` label. comment: ' ' +cla_config: + # CLAContextNames is the list of status contexts to check to decide the + # CLA status of pull requests. When any status in this list fails, the + # "cncf-cla: no" GitHub label is applied. When any status in this list + # passes, the "cncf-cla: yes" GitHub label is applied. The last status in + # the list has priority. Defaults to ["cla/linuxfoundation"] + cla_context_names: + - "" config_updater: # ClusterGroups is a map of ClusterGroups that can be used as a target # in the map config. diff --git a/prow/plugins/plugins_test.go b/prow/plugins/plugins_test.go index bc3ff5294010..97ff0814eeba 100644 --- a/prow/plugins/plugins_test.go +++ b/prow/plugins/plugins_test.go @@ -278,6 +278,9 @@ func TestLoad(t *testing.T) { Help: Help{ HelpGuidelinesURL: "https://git.k8s.io/community/contributors/guide/help-wanted.md", }, + CLAConfig: CLAConfig{ + CLAContextNames: []string{"cla/linuxfoundation"}, + }, } for _, modify := range m { modify(cfg)