Skip to content

Commit

Permalink
Added fix and tests for handling labels that have a prefix that can o…
Browse files Browse the repository at this point in the history
…nly exist once per issue

Signed-off-by: Raghav Roy <raghavroy145@gmail.com>
  • Loading branch information
RaghavRoy145 committed May 21, 2022
1 parent 845379f commit 3c7dc01
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 10 deletions.
5 changes: 4 additions & 1 deletion prow/plugins/config.go
Expand Up @@ -391,6 +391,9 @@ type Label struct {
// defines to which repos this applies and can be `*` for global, an org
// or a repo in org/repo notation.
RestrictedLabels map[string][]RestrictedLabel `json:"restricted_labels,omitempty"`
// UniquePrefixList is the list of label prefixes that can only exist
// once per issue,
UniquePrefixes []string `json:"unique_prefixes,omitempty"`
}

func (l Label) RestrictedLabelsFor(org, repo string) map[string]RestrictedLabel {
Expand Down Expand Up @@ -2032,7 +2035,7 @@ func getLabelConfigFromRestrictedLabelsSlice(s []RestrictedLabel, label string)
func (c *Configuration) HasConfigFor() (global bool, orgs sets.String, repos sets.String) {
equals := reflect.DeepEqual(c,
&Configuration{Approve: c.Approve, Bugzilla: c.Bugzilla, ExternalPlugins: c.ExternalPlugins,
Label: Label{RestrictedLabels: c.Label.RestrictedLabels}, Lgtm: c.Lgtm, Plugins: c.Plugins,
Label: Label{RestrictedLabels: c.Label.RestrictedLabels, UniquePrefixes: c.Label.UniquePrefixes}, Lgtm: c.Lgtm, Plugins: c.Plugins,
Triggers: c.Triggers, Welcome: c.Welcome})

if !equals || c.Bugzilla.Default != nil {
Expand Down
57 changes: 49 additions & 8 deletions prow/plugins/label/label.go
@@ -1,12 +1,9 @@
/*
Copyright 2016 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.
Expand Down Expand Up @@ -49,6 +46,20 @@ func init() {
plugins.RegisterPullRequestHandler(PluginName, handlePullRequest, helpProvider)
}

// returns a list of labels for every label prefix that can only exist once per issue
func getExistingUniquePrefixlabels(config plugins.Label, labels []github.Label) map[string][]string {
labelsMap := make(map[string][]string)
prefixList := config.UniquePrefixes
for _, prefix := range prefixList {
for _, label := range labels {
if matched, _ := regexp.MatchString(prefix, label.Name); matched {
labelsMap[prefix] = append(labelsMap[prefix], label.Name)
}
}
}
return labelsMap
}

func configString(labels []string) string {
var formattedLabels []string
for _, label := range labels {
Expand Down Expand Up @@ -168,7 +179,15 @@ func handleComment(gc githubClient, log *logrus.Entry, config plugins.Label, e *
if err != nil {
return err
}

var labelsMap = make(map[string][]string)
for _, label := range labels {
labelSplit := strings.Split(label.Name, "/")
// The second check (labelSplit[0] == "priority") can be removed or extended in the future to generalise
// the functionality of removing labels of the same class
if len(labelSplit) == 2 && labelSplit[0] == "priority" {
labelsMap[labelSplit[0]] = append(labelsMap[labelSplit[0]], labelSplit[1])
}
}
RepoLabelsExisting := sets.String{}
for _, l := range repoLabels {
RepoLabelsExisting.Insert(strings.ToLower(l.Name))
Expand All @@ -180,6 +199,7 @@ func handleComment(gc githubClient, log *logrus.Entry, config plugins.Label, e *
labelsToAdd []string
labelsToRemove []string
nonMemberTriageAccepted bool
labelPrefix string
)

additionalLabelSet := sets.String{}
Expand Down Expand Up @@ -228,12 +248,33 @@ func handleComment(gc githubClient, log *logrus.Entry, config plugins.Label, e *
gc.CreateComment(org, repo, e.Number, plugins.FormatResponseRaw(bodyWithoutComments, e.HTMLURL, e.User.Login, canNotSetLabelReason))
continue
}

if err := gc.AddLabel(org, repo, e.Number, labelToAdd); err != nil {
log.WithError(err).WithField("label", labelToAdd).Error("GitHub failed to add the label")
uniqueLabelsMap := getExistingUniquePrefixlabels(config, labels)
var labelsWithPrefixExist = false
for prefix := range uniqueLabelsMap {
if matched, _ := regexp.MatchString(prefix, labelToAdd); matched {
labelPrefix = prefix
labelsWithPrefixExist = true
}
}
if !github.HasLabel(labelToAdd, labels) {
if labelsWithPrefixExist {
for _, label := range uniqueLabelsMap[labelPrefix] {
if label != labelToAdd && github.HasLabel(label, labels) {
if err := gc.RemoveLabel(org, repo, e.Number, label); err != nil {
log.WithError(err).Errorf("GitHub failed to remove the following label: %s", label)
}
}
}
if err := gc.AddLabel(org, repo, e.Number, labelToAdd); err != nil {
log.WithError(err).WithField("label", labelToAdd).Error("GitHub failed to add the label")
}
} else {
if err := gc.AddLabel(org, repo, e.Number, labelToAdd); err != nil {
log.WithError(err).WithField("label", labelToAdd).Error("GitHub failed to add the label")
}
}
}
}

// Remove labels
for _, labelToRemove := range labelsToRemove {
if !github.HasLabel(labelToRemove, labels) {
Expand Down
14 changes: 13 additions & 1 deletion prow/plugins/label/label_test.go
Expand Up @@ -64,6 +64,7 @@ func TestHandleComment(t *testing.T) {
expectedCommentText string
action github.GenericCommentEventAction
teams map[string]map[string]fakegithub.TeamWithMembers
uniquePrefixList []string
}
testcases := []testCase{
{
Expand Down Expand Up @@ -475,6 +476,17 @@ func TestHandleComment(t *testing.T) {
commenter: orgMember,
action: github.GenericCommentActionCreated,
},
{
name: "Add label that has a prefix that can only exist once per issue",
body: "/priority urgent",
repoLabels: []string{"area/go", "priority/urgent", "priority/critical", "priority/l"},
issueLabels: []string{"area/go", "priority/critical", "priority/l"},
expectedNewLabels: formatWithPRInfo("priority/urgent"),
expectedRemovedLabels: formatWithPRInfo("priority/critical", "priority/l"),
commenter: orgMember,
action: github.GenericCommentActionCreated,
uniquePrefixList: []string{"^priority/"},
},
{
name: "Do nothing with empty /label command",
body: "/label",
Expand Down Expand Up @@ -762,7 +774,7 @@ func TestHandleComment(t *testing.T) {
Repo: github.Repo{Owner: github.User{Login: "org"}, Name: "repo"},
User: github.User{Login: tc.commenter},
}
err := handleComment(fakeClient, logrus.WithField("plugin", PluginName), plugins.Label{AdditionalLabels: tc.extraLabels, RestrictedLabels: tc.restrictedLabels}, e)
err := handleComment(fakeClient, logrus.WithField("plugin", PluginName), plugins.Label{AdditionalLabels: tc.extraLabels, RestrictedLabels: tc.restrictedLabels, UniquePrefixes: tc.uniquePrefixList}, e)
if err != nil {
t.Fatalf("didn't expect error from handle comment test: %v", err)
}
Expand Down
5 changes: 5 additions & 0 deletions prow/plugins/plugin-config-documented.yaml
Expand Up @@ -472,6 +472,11 @@ label:
# or a repo in org/repo notation.
restricted_labels:
"": null

# UniquePrefixList is the list of label prefixes that can only exist
# once per issue.
unique_prefixes:
- ""
lgtm:
- # Repos is either of the form org/repos or just org.
repos:
Expand Down

0 comments on commit 3c7dc01

Please sign in to comment.