Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
prow: allow org members to add triage/accepted label
  • Loading branch information
nikhita and justaugustus committed Sep 18, 2020
1 parent a88c0ef commit 3d7d537
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 10 deletions.
1 change: 1 addition & 0 deletions label_sync/labels.md
Expand Up @@ -113,6 +113,7 @@ larger set of contributors to apply/remove them.
| <a id="sig/ui" href="#sig/ui">`sig/ui`</a> | Categorizes an issue or PR as relevant to SIG UI.| anyone | [label](https://git.k8s.io/test-infra/prow/plugins/label) |
| <a id="sig/usability" href="#sig/usability">`sig/usability`</a> | Categorizes an issue or PR as relevant to SIG Usability.| anyone | [label](https://git.k8s.io/test-infra/prow/plugins/label) |
| <a id="sig/windows" href="#sig/windows">`sig/windows`</a> | Categorizes an issue or PR as relevant to SIG Windows.| anyone | [label](https://git.k8s.io/test-infra/prow/plugins/label) |
| <a id="triage/accepted" href="#triage/accepted">`triage/accepted`</a> | Indicates an issue or PR is ready to be actively worked on.| org members | [label](https://git.k8s.io/test-infra/prow/plugins/label) |
| <a id="triage/duplicate" href="#triage/duplicate">`triage/duplicate`</a> | Indicates an issue is a duplicate of other open issue. <br><br> This was previously `close/duplicate`, `duplicate`, | humans | |
| <a id="triage/needs-information" href="#triage/needs-information">`triage/needs-information`</a> | Indicates an issue needs more information in order to work on it. <br><br> This was previously `close/needs-information`, | humans | |
| <a id="triage/not-reproducible" href="#triage/not-reproducible">`triage/not-reproducible`</a> | Indicates an issue can not be reproduced as described. <br><br> This was previously `close/not-reproducible`, | humans | |
Expand Down
6 changes: 6 additions & 0 deletions label_sync/labels.yaml
Expand Up @@ -97,6 +97,12 @@ default:
addedBy: humans
previously:
- name: cherrypick-approved
- color: 8fc951
description: Indicates an issue or PR is ready to be actively worked on.
name: triage/accepted
target: both
prowPlugin: label
addedBy: org members
- color: d455d0
description: Indicates an issue is a duplicate of other open issue.
name: triage/duplicate
Expand Down
1 change: 1 addition & 0 deletions prow/labels/labels.go
Expand Up @@ -46,6 +46,7 @@ const (
NeedsSig = "needs-sig"
OkToTest = "ok-to-test"
Shrug = \\_(ツ)_/¯"
TriageAccepted = "triage/accepted"
WorkInProgress = "do-not-merge/work-in-progress"
ValidBug = "bugzilla/valid-bug"
)
1 change: 1 addition & 0 deletions prow/plugins/label/BUILD.bazel
Expand Up @@ -27,6 +27,7 @@ go_library(
deps = [
"//prow/config:go_default_library",
"//prow/github:go_default_library",
"//prow/labels:go_default_library",
"//prow/pluginhelp:go_default_library",
"//prow/plugins:go_default_library",
"@com_github_sirupsen_logrus//:go_default_library",
Expand Down
32 changes: 26 additions & 6 deletions prow/plugins/label/label.go
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/test-infra/prow/config"
"k8s.io/test-infra/prow/github"
prowlabels "k8s.io/test-infra/prow/labels"
"k8s.io/test-infra/prow/pluginhelp"
"k8s.io/test-infra/prow/plugins"
)
Expand Down Expand Up @@ -67,7 +68,7 @@ func helpProvider(config *plugins.Configuration, _ []config.OrgRepo) (*pluginhel
Usage: "/[remove-](area|committee|kind|language|priority|sig|triage|wg|label) <target>",
Description: "Applies or removes a label from one of the recognized types of labels.",
Featured: false,
WhoCanUse: "Anyone can trigger this command on a PR.",
WhoCanUse: "Anyone can trigger this command on issues and PRs. `triage/accepted` can only be added by org members.",
Examples: []string{"/kind bug", "/remove-area prow", "/sig testing", "/language zh", "/label foo-bar-baz"},
})
return pluginHelp, nil
Expand All @@ -80,6 +81,7 @@ func handleGenericComment(pc plugins.Agent, e github.GenericCommentEvent) error
type githubClient interface {
CreateComment(owner, repo string, number int, comment string) error
AddLabel(owner, repo string, number int, label string) error
IsMember(org, user string) (bool, error)
RemoveLabel(owner, repo string, number int, label string) error
GetRepoLabels(owner, repo string) ([]github.Label, error)
GetIssueLabels(org, repo string, number int) ([]github.Label, error)
Expand Down Expand Up @@ -137,6 +139,7 @@ func handle(gc githubClient, log *logrus.Entry, additionalLabels []string, e *gi

org := e.Repo.Owner.Login
repo := e.Repo.Name
user := e.User.Login

repoLabels, err := gc.GetRepoLabels(org, repo)
if err != nil {
Expand All @@ -152,11 +155,12 @@ func handle(gc githubClient, log *logrus.Entry, additionalLabels []string, e *gi
RepoLabelsExisting.Insert(strings.ToLower(l.Name))
}
var (
nonexistent []string
noSuchLabelsInRepo []string
noSuchLabelsOnIssue []string
labelsToAdd []string
labelsToRemove []string
nonexistent []string
noSuchLabelsInRepo []string
noSuchLabelsOnIssue []string
labelsToAdd []string
labelsToRemove []string
nonMemberTriageAccepted bool
)
// Get labels to add and labels to remove from regexp matches
labelsToAdd = append(getLabelsFromREMatches(labelMatches), getLabelsFromGenericMatches(customLabelMatches, additionalLabels, &nonexistent)...)
Expand All @@ -172,6 +176,17 @@ func handle(gc githubClient, log *logrus.Entry, additionalLabels []string, e *gi
continue
}

// only org members can add triage/accepted
if labelToAdd == prowlabels.TriageAccepted {
if member, err := gc.IsMember(org, user); err != nil {
log.WithError(err).Errorf("error in IsMember(%s): %v", org, err)
continue
} else if !member {
nonMemberTriageAccepted = true
continue
}
}

if err := gc.AddLabel(org, repo, e.Number, labelToAdd); err != nil {
log.WithError(err).Errorf("GitHub failed to add the following label: %s", labelToAdd)
}
Expand Down Expand Up @@ -211,5 +226,10 @@ func handle(gc githubClient, log *logrus.Entry, additionalLabels []string, e *gi
return gc.CreateComment(org, repo, e.Number, plugins.FormatResponseRaw(bodyWithoutComments, e.HTMLURL, e.User.Login, msg))
}

if nonMemberTriageAccepted {
msg := fmt.Sprintf("The label `%s` cannot be applied. Only GitHub organization members can add the label.", prowlabels.TriageAccepted)
return gc.CreateComment(org, repo, e.Number, plugins.FormatResponseRaw(bodyWithoutComments, e.HTMLURL, e.User.Login, msg))
}

return nil
}
52 changes: 48 additions & 4 deletions prow/plugins/label/label_test.go
Expand Up @@ -132,6 +132,38 @@ func TestLabel(t *testing.T) {
commenter: orgMember,
action: github.GenericCommentActionCreated,
},
{
name: "Org member can add triage/accepted label",
body: "/triage accepted",
repoLabels: []string{"triage/accepted"},
issueLabels: []string{},
expectedNewLabels: formatLabels("triage/accepted"),
expectedRemovedLabels: []string{},
commenter: orgMember,
action: github.GenericCommentActionCreated,
},
{
name: "Non org member cannot add triage/accepted label",
body: "/triage accepted",
repoLabels: []string{"triage/accepted", "kind/bug"},
issueLabels: []string{"kind/bug"},
expectedNewLabels: formatLabels(),
expectedRemovedLabels: []string{},
commenter: nonOrgMember,
expectedBotComment: true,
expectedCommentText: "The label `triage/accepted` cannot be applied. Only GitHub organization members can add the label.",
action: github.GenericCommentActionCreated,
},
{
name: "Non org member can add triage/needs-information label",
body: "/triage needs-information",
repoLabels: []string{"area/infra", "triage/needs-information"},
issueLabels: []string{"area/infra"},
expectedNewLabels: formatLabels("triage/needs-information"),
expectedRemovedLabels: []string{},
commenter: nonOrgMember,
action: github.GenericCommentActionCreated,
},
{
name: "Adding Labels is Case Insensitive",
body: "/kind BuG",
Expand Down Expand Up @@ -274,6 +306,18 @@ func TestLabel(t *testing.T) {
expectedCommentText: "The label(s) `committee/calamity` cannot be applied, because the repository doesn't have them",
action: github.GenericCommentActionCreated,
},
{
name: "Non org member adds multiple triage labels (some valid)",
body: "/triage needs-information accepted",
repoLabels: []string{"triage/needs-information", "triage/accepted"},
issueLabels: []string{},
expectedNewLabels: formatLabels("triage/needs-information"),
expectedRemovedLabels: []string{},
commenter: nonOrgMember,
expectedBotComment: true,
expectedCommentText: "The label `triage/accepted` cannot be applied. Only GitHub organization members can add the label.",
action: github.GenericCommentActionCreated,
},
{
name: "Add Multiple Types of Labels Different Lines",
body: "/priority urgent\n/area infra",
Expand Down Expand Up @@ -368,11 +412,11 @@ func TestLabel(t *testing.T) {
},
{
name: "Remove Triage Label",
body: "/remove-triage needs-information",
repoLabels: []string{"area/infra", "triage/needs-information"},
issueLabels: []string{"area/infra", "triage/needs-information"},
body: "/remove-triage needs-information accepted",
repoLabels: []string{"area/infra", "triage/needs-information", "triage/accepted"},
issueLabels: []string{"area/infra", "triage/needs-information", "triage/accepted"},
expectedNewLabels: []string{},
expectedRemovedLabels: formatLabels("triage/needs-information"),
expectedRemovedLabels: formatLabels("triage/needs-information", "triage/accepted"),
commenter: orgMember,
action: github.GenericCommentActionCreated,
},
Expand Down

0 comments on commit 3d7d537

Please sign in to comment.