Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report problem when label added as both restricted and additional #24267

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions prow/cmd/checkconfig/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,22 +1053,33 @@ func verifyOwnersPlugin(cfg *plugins.Configuration) error {
}

func verifyLabelPlugin(label plugins.Label) error {
var orgRepos []string
var orgReposWithEmptyLabelConfig []string
var errs []error
restrictedAndAdditionalLabels := make(map[string][]string)
for orgRepo, restrictedLabels := range label.RestrictedLabels {
for _, restrictedLabel := range restrictedLabels {
if label.IsRestrictedLabelInAdditionalLables(restrictedLabel.Label) {
restrictedAndAdditionalLabels[restrictedLabel.Label] = append(restrictedAndAdditionalLabels[restrictedLabel.Label], orgRepo)
}
if restrictedLabel.Label == "" {
orgRepos = append(orgRepos, orgRepo)
orgReposWithEmptyLabelConfig = append(orgReposWithEmptyLabelConfig, orgRepo)
}
}
}

if len(orgRepos) > 0 {
sort.Strings(orgRepos)
return fmt.Errorf("the following orgs or repos have configuration of %s plugin using the empty string as label name in restricted labels: %s",
labelplugin.PluginName, strings.Join(orgRepos, ", "),
)
for label, repos := range restrictedAndAdditionalLabels {
sort.Strings(repos)
errs = append(errs,
fmt.Errorf("the following orgs or repos have configuration of label plugin using the restricted label %s which is also configured as an additional label: %s", label, strings.Join(repos, ", ")))
}
return nil

if len(orgReposWithEmptyLabelConfig) > 0 {
sort.Strings(orgReposWithEmptyLabelConfig)
errs = append(errs, fmt.Errorf("the following orgs or repos have configuration of %s plugin using the empty string as label name in restricted labels: %s",
labelplugin.PluginName, strings.Join(orgReposWithEmptyLabelConfig, ", "),
))
}
return utilerrors.NewAggregate(errs)
}

func validateTriggers(cfg *config.Config, pcfg *plugins.Configuration) error {
Expand Down
39 changes: 39 additions & 0 deletions prow/cmd/checkconfig/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2281,6 +2281,45 @@ func TestVerifyLabelPlugin(t *testing.T) {
},
expectedErrorMsg: "the following orgs or repos have configuration of label plugin using the empty string as label name in restricted labels: orgRepo1, orgRepo2",
},
{
name: "invalid when additional and restricted labels are the same",
label: plugins.Label{
AdditionalLabels: []string{"cherry-pick-approved"},
RestrictedLabels: map[string][]plugins.RestrictedLabel{
"orgRepo1": {
{
Label: "cherry-pick-approved",
},
},
},
},
expectedErrorMsg: "the following orgs or repos have configuration of label plugin using the restricted label cherry-pick-approved which is also configured as an additional label: orgRepo1",
},
{
name: "invalid when additional and restricted labels are the same in multiple orgRepos and empty string",
label: plugins.Label{
AdditionalLabels: []string{"cherry-pick-approved"},
RestrictedLabels: map[string][]plugins.RestrictedLabel{
"orgRepo1": {
{
Label: "cherry-pick-approved",
},
},
"orgRepo2": {
{
Label: "",
},
},
"orgRepo3": {
{
Label: "cherry-pick-approved",
},
},
},
},
expectedErrorMsg: "[the following orgs or repos have configuration of label plugin using the restricted label cherry-pick-approved which is also configured as an additional label: orgRepo1, orgRepo3, " +
"the following orgs or repos have configuration of label plugin using the empty string as label name in restricted labels: orgRepo2]",
},
}

for _, tc := range testCases {
Expand Down
9 changes: 9 additions & 0 deletions prow/plugins/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,15 @@ func (l Label) RestrictedLabelsFor(org, repo string) map[string]RestrictedLabel
return result
}

func (l Label) IsRestrictedLabelInAdditionalLables(restricted string) bool {
jmguzik marked this conversation as resolved.
Show resolved Hide resolved
for _, additional := range l.AdditionalLabels {
if restricted == additional {
return true
}
}
return false
}

type RestrictedLabel struct {
Label string `json:"label"`
AllowedTeams []string `json:"allowed_teams,omitempty"`
Expand Down