diff --git a/prow/cmd/checkconfig/main.go b/prow/cmd/checkconfig/main.go index 1ce479dc5277..97283b1b7665 100644 --- a/prow/cmd/checkconfig/main.go +++ b/prow/cmd/checkconfig/main.go @@ -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 { diff --git a/prow/cmd/checkconfig/main_test.go b/prow/cmd/checkconfig/main_test.go index 4fd9f4d5e110..43d2b76617f8 100644 --- a/prow/cmd/checkconfig/main_test.go +++ b/prow/cmd/checkconfig/main_test.go @@ -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 { diff --git a/prow/plugins/config.go b/prow/plugins/config.go index 6620d4ec0b69..7c38a8166da5 100644 --- a/prow/plugins/config.go +++ b/prow/plugins/config.go @@ -401,6 +401,15 @@ func (l Label) RestrictedLabelsFor(org, repo string) map[string]RestrictedLabel return result } +func (l Label) IsRestrictedLabelInAdditionalLables(restricted string) bool { + 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"`