Skip to content

Commit

Permalink
k/test-infra: Carry pull requests (#258)
Browse files Browse the repository at this point in the history 
- org: Add test case for ignoring invitees
   ref: kubernetes/test-infra#27793
- Remove deprecated `sets` objects
   ref: kubernetes/test-infra#30041

---------

Signed-off-by: Stephen Augustus <[email protected]>
  • Loading branch information
@justaugustus
justaugustus authored Sep 5, 2023
1 parent 5932a39 commit 19662c2
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 157 deletions.
40 changes: 20 additions & 20 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,18 +91,18 @@ func loadOrg(dir string) (*org.Config, error) {
return &cfg, nil
}

func testDuplicates(list sets.String) error {
found := sets.String{}
dups := sets.String{}
all := list.List()
func testDuplicates(list sets.Set[string]) error {
found := sets.Set[string]{}
dups := sets.Set[string]{}
all := sets.List(list)
for _, i := range all {
if found.Has(i) {
dups.Insert(i)
}
found.Insert(i)
}
if n := len(dups); n > 0 {
return fmt.Errorf("%d duplicate names: %s", n, strings.Join(dups.List(), ", "))
return fmt.Errorf("%d duplicate names: %s", n, strings.Join(sets.List(dups), ", "))
}
return nil
}
Expand All @@ -116,8 +116,8 @@ func isSorted(list []string) bool {
return sort.StringsAreSorted(items)
}

func normalize(s sets.String) sets.String {
out := sets.String{}
func normalize(s sets.Set[string]) sets.Set[string] {
out := sets.Set[string]{}
for i := range s {
out.Insert(github.NormLogin(i))
}
Expand All @@ -126,11 +126,11 @@ func normalize(s sets.String) sets.String {

// testTeamMembers ensures that a user is not a maintainer and member at the same time,
// there are no duplicate names in the list and all users are org members.
func testTeamMembers(teams map[string]org.Team, admins sets.String, orgMembers sets.String, orgName string) []error {
func testTeamMembers(teams map[string]org.Team, admins sets.Set[string], orgMembers sets.Set[string], orgName string) []error {
var errs []error
for teamName, team := range teams {
teamMaintainers := sets.NewString(team.Maintainers...)
teamMembers := sets.NewString(team.Members...)
teamMaintainers := sets.New[string](team.Maintainers...)
teamMembers := sets.New[string](team.Members...)

teamMaintainers = normalize(teamMaintainers)
teamMembers = normalize(teamMembers)
Expand All @@ -142,12 +142,12 @@ func testTeamMembers(teams map[string]org.Team, admins sets.String, orgMembers s

// check for non-admins in maintainers list
if nonAdminMaintainers := teamMaintainers.Difference(admins); len(nonAdminMaintainers) > 0 {
errs = append(errs, fmt.Errorf("The team %s in org %s has non-admins listed as maintainers; these users should be in the members list instead: %s", teamName, orgName, strings.Join(nonAdminMaintainers.List(), ",")))
errs = append(errs, fmt.Errorf("The team %s in org %s has non-admins listed as maintainers; these users should be in the members list instead: %s", teamName, orgName, strings.Join(sets.List(nonAdminMaintainers), ",")))
}

// check for users in both maintainers and members
if both := teamMaintainers.Intersection(teamMembers); len(both) > 0 {
errs = append(errs, fmt.Errorf("The team %s in org %s has users in both maintainer admin and member roles: %s", teamName, orgName, strings.Join(both.List(), ", ")))
errs = append(errs, fmt.Errorf("The team %s in org %s has users in both maintainer admin and member roles: %s", teamName, orgName, strings.Join(sets.List(both), ", ")))
}

// check for duplicates
Expand All @@ -160,12 +160,12 @@ func testTeamMembers(teams map[string]org.Team, admins sets.String, orgMembers s

// check if all are org members
if missing := teamMembers.Difference(orgMembers); len(missing) > 0 {
errs = append(errs, fmt.Errorf("The following members of team %s are not %s org members: %s", teamName, orgName, strings.Join(missing.List(), ", ")))
errs = append(errs, fmt.Errorf("The following members of team %s are not %s org members: %s", teamName, orgName, strings.Join(sets.List(missing), ", ")))
}

// check if admins are a regular member of team
if adminTeamMembers := teamMembers.Intersection(admins); len(adminTeamMembers) > 0 {
errs = append(errs, fmt.Errorf("The team %s in org %s has org admins listed as members; these users should be in the maintainers list instead, and cannot be on the members list: %s", teamName, orgName, strings.Join(adminTeamMembers.List(), ", ")))
errs = append(errs, fmt.Errorf("The team %s in org %s has org admins listed as members; these users should be in the maintainers list instead, and cannot be on the members list: %s", teamName, orgName, strings.Join(sets.List(adminTeamMembers), ", ")))
}

// check if lists are sorted
Expand Down Expand Up @@ -194,12 +194,12 @@ func testOrg(targetDir string, t *testing.T) {
t.Fatalf("failed to load OWNERS: %v", err)
}
members := normalize(sets.NewString(cfg.Members...))
admins := normalize(sets.NewString(cfg.Admins...))
members := normalize(sets.New[string](cfg.Members...))
admins := normalize(sets.New[string](cfg.Admins...))
allOrgMembers := members.Union(admins)
reviewers := normalize(sets.NewString(own.Reviewers...))
approvers := normalize(sets.NewString(own.Approvers...))
reviewers := normalize(sets.New[string](own.Reviewers...))
approvers := normalize(sets.New[string](own.Approvers...))
if n := len(approvers); n < 5 {
t.Errorf("Require at least 5 approvers, found %d: %s", n, strings.Join(approvers.List(), ", "))
Expand Down Expand Up @@ -245,8 +245,8 @@ func TestAllOrgs(t *testing.T) {
}
for _, org := range cfg.Orgs {
members := normalize(sets.NewString(org.Members...))
admins := normalize(sets.NewString(org.Admins...))
members := normalize(sets.New[string](org.Members...))
admins := normalize(sets.New[string](org.Admins...))
allOrgMembers := members.Union(admins)
if both := admins.Intersection(members); len(both) > 0 {
Expand Down
34 changes: 17 additions & 17 deletions org/members.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type inviteClient interface {
ListOrgInvitations(org string) ([]github.OrgInvitation, error)
}

func orgInvitations(opt root.Options, client inviteClient, orgName string) (sets.String, error) {
invitees := sets.String{}
func orgInvitations(opt root.Options, client inviteClient, orgName string) (sets.Set[string], error) {
invitees := sets.Set[string]{}
if (!opt.FixOrgMembers && !opt.FixTeamMembers) || opt.IgnoreInvitees {
return invitees, nil
}
Expand All @@ -58,10 +58,10 @@ type orgClient interface {
UpdateOrgMembership(org, user string, admin bool) (*github.OrgMembership, error)
}

func configureOrgMembers(opt root.Options, client orgClient, orgName string, orgConfig org.Config, invitees sets.String) error {
func configureOrgMembers(opt root.Options, client orgClient, orgName string, orgConfig org.Config, invitees sets.Set[string]) error {
// Get desired state
wantAdmins := sets.NewString(orgConfig.Admins...)
wantMembers := sets.NewString(orgConfig.Members...)
wantAdmins := sets.New[string](orgConfig.Admins...)
wantMembers := sets.New[string](orgConfig.Members...)

// Sanity desired state
if n := len(wantAdmins); n < opt.MinAdmins {
Expand All @@ -85,8 +85,8 @@ func configureOrgMembers(opt root.Options, client orgClient, orgName string, org
}

// Get current state
haveAdmins := sets.String{}
haveMembers := sets.String{}
haveAdmins := sets.Set[string]{}
haveMembers := sets.Set[string]{}
ms, err := client.ListOrgMembers(orgName, github.RoleAdmin)
if err != nil {
return fmt.Errorf("failed to list %s admins: %w", orgName, err)
Expand All @@ -113,9 +113,9 @@ func configureOrgMembers(opt root.Options, client orgClient, orgName string, org
return fmt.Errorf("cannot delete %d memberships or %.3f of %s (exceeds limit of %.3f)", len(remove), d, orgName, opt.MaxDelta)
}

teamMembers := sets.String{}
teamNames := sets.String{}
duplicateTeamNames := sets.String{}
teamMembers := sets.Set[string]{}
teamNames := sets.Set[string]{}
duplicateTeamNames := sets.Set[string]{}
for name, team := range orgConfig.Teams {
teamMembers.Insert(team.Members...)
teamMembers.Insert(team.Maintainers...)
Expand All @@ -133,11 +133,11 @@ func configureOrgMembers(opt root.Options, client orgClient, orgName string, org

teamMembers = normalize(teamMembers)
if outside := teamMembers.Difference(want.all()); len(outside) > 0 {
return fmt.Errorf("all team members/maintainers must also be org members: %s", strings.Join(outside.List(), ", "))
return fmt.Errorf("all team members/maintainers must also be org members: %s", strings.Join(sets.List(outside), ", "))
}

if n := len(duplicateTeamNames); n > 0 {
return fmt.Errorf("team names must be unique (including previous names), %d duplicated names: %s", n, strings.Join(duplicateTeamNames.List(), ", "))
return fmt.Errorf("team names must be unique (including previous names), %d duplicated names: %s", n, strings.Join(sets.List(duplicateTeamNames), ", "))
}

adder := func(user string, super bool) error {
Expand Down Expand Up @@ -177,11 +177,11 @@ func configureOrgMembers(opt root.Options, client orgClient, orgName string, org
}

type memberships struct {
members sets.String
super sets.String
members sets.Set[string]
super sets.Set[string]
}

func (m memberships) all() sets.String {
func (m memberships) all() sets.Set[string] {
return m.members.Union(m.super)
}

Expand All @@ -190,11 +190,11 @@ func (m *memberships) normalize() {
m.super = normalize(m.super)
}

func configureMembers(have, want memberships, invitees sets.String, adder func(user string, super bool) error, remover func(user string) error) error {
func configureMembers(have, want memberships, invitees sets.Set[string], adder func(user string, super bool) error, remover func(user string) error) error {
have.normalize()
want.normalize()
if both := want.super.Intersection(want.members); len(both) > 0 {
return fmt.Errorf("users in both roles: %s", strings.Join(both.List(), ", "))
return fmt.Errorf("users in both roles: %s", strings.Join(sets.List(both), ", "))
}
havePlusInvites := have.all().Union(invitees)
remove := havePlusInvites.Difference(want.all())
Expand Down
4 changes: 2 additions & 2 deletions org/org.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func updateBool(have, want *bool) bool {
return true
}

func normalize(s sets.String) sets.String {
out := sets.String{}
func normalize(s sets.Set[string]) sets.Set[string] {
out := sets.Set[string]{}
for i := range s {
out.Insert(github.NormLogin(i))
}
Expand Down
Loading

0 comments on commit 19662c2

Please sign in to comment.