Skip to content

Commit

Permalink
Generalize duplicate repo validation for Approve and Welcome plugins.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjwagner committed Mar 26, 2024
1 parent e774071 commit 3445bc3
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 76 deletions.
33 changes: 0 additions & 33 deletions prow/plugins/approve/approve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,39 +46,6 @@ import (

const prNumber = 1

// TestPluginConfig validates that there are no duplicate repos in the approve plugin config.
func TestPluginConfig(t *testing.T) {
pa := &plugins.ConfigAgent{}

b, err := os.ReadFile("../../../config/prow/plugins.yaml")
if err != nil {
t.Fatalf("Failed to read plugin config: %v.", err)
}
np := &plugins.Configuration{}
if err := yaml.Unmarshal(b, np); err != nil {
t.Fatalf("Failed to unmarshal plugin config: %v.", err)
}
pa.Set(np)

orgs := map[string]bool{}
repos := map[string]bool{}
for _, config := range pa.Config().Approve {
for _, entry := range config.Repos {
if strings.Contains(entry, "/") {
if repos[entry] {
t.Errorf("The repo %q is duplicated in the 'approve' plugin configuration.", entry)
}
repos[entry] = true
} else {
if orgs[entry] {
t.Errorf("The org %q is duplicated in the 'approve' plugin configuration.", entry)
}
orgs[entry] = true
}
}
}
}

func newTestComment(user, body string) github.IssueComment {
return github.IssueComment{User: github.User{Login: user}, Body: body}
}
Expand Down
40 changes: 40 additions & 0 deletions prow/plugins/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ func (a Approve) ConsiderReviewState() bool {
return true
}

func (a Approve) getRepos() []string {
return a.Repos
}

// Lgtm specifies a configuration for a single lgtm.
// The configuration for the lgtm plugin is defined as a list of these structures.
type Lgtm struct {
Expand Down Expand Up @@ -684,6 +688,10 @@ type Welcome struct {
AlwaysPost bool `json:"always_post,omitempty"`
}

func (w Welcome) getRepos() []string {
return w.Repos
}

// Dco is config for the DCO (https://developercertificate.org/) checker plugin.
type Dco struct {
// SkipDCOCheckForMembers is used to skip DCO check for trusted org members
Expand Down Expand Up @@ -1411,11 +1419,43 @@ func (c *Configuration) Validate() error {
if err := validateTrigger(c.Triggers); err != nil {
return err
}
if err := validateRepoDupes(c.Approve); err != nil {
return err
}
if err := validateRepoDupes(c.Welcome); err != nil {
return err
}
validateRepoMilestone(c.RepoMilestone)

return nil
}

type ListableRepos interface {
getRepos() []string
}

func validateRepoDupes[C ListableRepos](configs []C) error {
var errs []error
orgs := map[string]bool{}
repos := map[string]bool{}
for _, config := range configs {
for _, entry := range config.getRepos() {
if strings.Contains(entry, "/") {
if repos[entry] {
errs = append(errs, fmt.Errorf("The repo %q is duplicated in the 'welcome' plugin configuration.", entry))
}
repos[entry] = true
} else {
if orgs[entry] {
errs = append(errs, fmt.Errorf("The org %q is duplicated in the 'welcome' plugin configuration.", entry))
}
orgs[entry] = true
}
}
}
return utilerrors.NewAggregate(errs)
}

func (pluginConfig *ProjectConfig) GetMaintainerTeam(org string, repo string) int {
for orgName, orgConfig := range pluginConfig.Orgs {
if org == orgName {
Expand Down
43 changes: 0 additions & 43 deletions prow/plugins/welcome/welcome_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,11 @@ package welcome
import (
"errors"
"fmt"
"os"
"regexp"
"strings"
"testing"

"github.com/sirupsen/logrus"

"sigs.k8s.io/yaml"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/test-infra/prow/config"
"k8s.io/test-infra/prow/github"
Expand Down Expand Up @@ -448,45 +444,6 @@ func TestWelcomeConfig(t *testing.T) {
}
}

// TestPluginConfig validates that there are no duplicate repos in the welcome plugin config.
func TestPluginConfig(t *testing.T) {
pa := &plugins.ConfigAgent{}

b, err := os.ReadFile("../../../config/prow/plugins.yaml")
if err != nil {
t.Fatalf("Failed to read plugin config: %v.", err)
}
np := &plugins.Configuration{}
if err := yaml.Unmarshal(b, np); err != nil {
t.Fatalf("Failed to unmarshal plugin config: %v.", err)
}
pa.Set(np)

orgs := map[string]bool{}
repos := map[string]bool{}
for _, config := range pa.Config().Welcome {
for _, entry := range config.Repos {
if strings.Contains(entry, "/") {
if repos[entry] {
t.Errorf("The repo %q is duplicated in the 'welcome' plugin configuration.", entry)
}
repos[entry] = true
} else {
if orgs[entry] {
t.Errorf("The org %q is duplicated in the 'welcome' plugin configuration.", entry)
}
orgs[entry] = true
}
}
}
for repo := range repos {
org := strings.Split(repo, "/")[0]
if orgs[org] {
t.Errorf("The repo %q is duplicated with %q in the 'welcome' plugin configuration.", repo, org)
}
}
}

func TestHelpProvider(t *testing.T) {
enabledRepos := []config.OrgRepo{
{Org: "org1", Repo: "repo"},
Expand Down

0 comments on commit 3445bc3

Please sign in to comment.