diff --git a/cmd/peribolos/main.go b/cmd/peribolos/main.go index 0e7e31537..4aa8cbf77 100644 --- a/cmd/peribolos/main.go +++ b/cmd/peribolos/main.go @@ -58,6 +58,7 @@ type options struct { fixTeamRepos bool fixRepos bool fixCollaborators bool + fixOrgRoles bool ignoreInvitees bool ignoreSecretTeams bool allowRepoArchival bool @@ -94,6 +95,7 @@ func (o *options) parseArgs(flags *flag.FlagSet, args []string) error { flags.BoolVar(&o.fixTeamRepos, "fix-team-repos", false, "Add/remove team permissions on repos if set") flags.BoolVar(&o.fixRepos, "fix-repos", false, "Create/update repositories if set") flags.BoolVar(&o.fixCollaborators, "fix-collaborators", false, "Add/remove/update repository collaborators if set") + flags.BoolVar(&o.fixOrgRoles, "fix-org-roles", false, "Assign/remove organization roles to teams and users if set") flags.BoolVar(&o.allowRepoArchival, "allow-repo-archival", false, "If set, archiving repos is allowed while updating repos") flags.BoolVar(&o.allowRepoPublish, "allow-repo-publish", false, "If set, making private repos public is allowed while updating repos") flags.StringVar(&o.logLevel, "log-level", logrus.InfoLevel.String(), fmt.Sprintf("Logging level, one of %v", logrus.AllLevels)) @@ -147,6 +149,10 @@ func (o *options) parseArgs(flags *flag.FlagSet, args []string) error { return fmt.Errorf("--fix-team-repos requires --fix-teams") } + if o.fixOrgRoles && !o.fixTeams { + return fmt.Errorf("--fix-org-roles requires --fix-teams") + } + return nil } @@ -209,6 +215,9 @@ type dumpClient interface { GetRepo(owner, name string) (github.FullRepo, error) GetRepos(org string, isUser bool) ([]github.Repo, error) ListDirectCollaboratorsWithPermissions(org, repo string) (map[string]github.RepoPermissionLevel, error) + ListOrganizationRoles(org string) ([]github.OrganizationRole, error) + ListTeamsWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) + ListUsersWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) BotUser() (*github.UserData, error) } @@ -272,6 +281,7 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool, ap idMap := map[int]org.Team{} // metadata for a team children := map[int][]int{} // what children does it have var tops []int // what are the top-level teams + slugToName := map[string]string{} for _, t := range teams { logger := logrus.WithFields(logrus.Fields{"id": t.ID, "name": t.Name}) @@ -280,6 +290,7 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool, ap logger.Debug("Ignoring secret team.") continue } + slugToName[t.Slug] = t.Name d := t.Description nt := org.Team{ TeamMetadata: org.TeamMetadata{ @@ -385,6 +396,53 @@ func dumpOrgConfig(client dumpClient, orgName string, ignoreSecretTeams bool, ap out.Repos[full.Name] = repoConfig } + // Dump organization roles + roles, err := client.ListOrganizationRoles(orgName) + if err != nil { + return nil, fmt.Errorf("failed to list organization roles: %w", err) + } + logrus.Debugf("Found %d organization roles", len(roles)) + if len(roles) > 0 { + out.Roles = make(map[string]org.Role, len(roles)) + } + for _, role := range roles { + logrus.WithField("role", role.Name).Debug("Recording organization role.") + + // Get teams with this role + teamsWithRole, err := client.ListTeamsWithRole(orgName, role.ID) + if err != nil { + logrus.WithError(err).Warnf("Failed to list teams with role %s", role.Name) + continue + } + + // Get users with this role + usersWithRole, err := client.ListUsersWithRole(orgName, role.ID) + if err != nil { + logrus.WithError(err).Warnf("Failed to list users with role %s", role.Name) + continue + } + + // Build team and user lists + var teamSlugs []string + for _, team := range teamsWithRole { + if name, ok := slugToName[team.Slug]; ok { + teamSlugs = append(teamSlugs, name) + } else { + teamSlugs = append(teamSlugs, team.Slug) + } + } + + var userLogins []string + for _, user := range usersWithRole { + userLogins = append(userLogins, user.Login) + } + + out.Roles[role.Name] = org.Role{ + Teams: teamSlugs, + Users: userLogins, + } + } + return &out, nil } @@ -496,6 +554,8 @@ func configureOrgMembers(opt options, client orgClient, orgName string, orgConfi } } else if om.State == github.StatePending { logrus.Infof("Invited %s to %s as a %s", user, orgName, role) + // Track the new invitation so role assignment can skip this user + invitees.Insert(github.NormLogin(user)) } else { logrus.Infof("Set %s as a %s of %s", user, role, orgName) } @@ -870,6 +930,13 @@ func orgInvitations(opt options, client inviteClient, orgName string) (sets.Set[ } func configureOrg(opt options, client github.Client, orgName string, orgConfig org.Config) error { + // Validate role configuration early (before any API calls) if we're going to configure roles + if opt.fixOrgRoles { + if err := orgConfig.ValidateRoles(); err != nil { + return fmt.Errorf("invalid role configuration: %w", err) + } + } + // Ensure that metadata is configured correctly. if !opt.fixOrg { logrus.Infof("Skipping org metadata configuration") @@ -889,6 +956,9 @@ func configureOrg(opt options, client github.Client, orgName string, orgConfig o return fmt.Errorf("failed to configure %s members: %w", orgName, err) } + // Note: New invitations sent by configureOrgMembers are tracked in the invitees set, + // so role assignment can skip users with pending invitations without an extra API call. + // Create repositories in the org if !opt.fixRepos { logrus.Info("Skipping org repositories configuration") @@ -932,6 +1002,14 @@ func configureOrg(opt options, client github.Client, orgName string, orgConfig o return fmt.Errorf("failed to configure %s team %s repos: %w", orgName, name, err) } } + + // Configure organization roles + if !opt.fixOrgRoles { + logrus.Infof("Skipping organization roles configuration") + } else if err := configureOrgRoles(client, orgName, orgConfig, githubTeams, invitees); err != nil { + return fmt.Errorf("failed to configure %s organization roles: %w", orgName, err) + } + return nil } @@ -1453,6 +1531,220 @@ func configureTeamRepos(client teamRepoClient, githubTeams map[string]github.Tea return utilerrors.NewAggregate(updateErrors) } +type orgRolesClient interface { + ListOrganizationRoles(org string) ([]github.OrganizationRole, error) + AssignOrganizationRoleToTeam(org, teamSlug string, roleID int) error + RemoveOrganizationRoleFromTeam(org, teamSlug string, roleID int) error + AssignOrganizationRoleToUser(org, user string, roleID int) error + RemoveOrganizationRoleFromUser(org, user string, roleID int) error + ListTeamsWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) + ListUsersWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) +} + +// configureOrgRoles configures organization roles for teams and users +func configureOrgRoles(client orgRolesClient, orgName string, orgConfig org.Config, githubTeams map[string]github.Team, invitees sets.Set[string]) error { + // Note: Role configuration is validated at the start of configureOrg() before any API calls + + // Get current organization roles from GitHub + roles, err := client.ListOrganizationRoles(orgName) + if err != nil { + return fmt.Errorf("failed to list organization roles: %w", err) + } + + if len(roles) == 0 { + logrus.Debugf("No organization roles exist in %s", orgName) + return nil + } + + // Create a map of configured role names (lowercase) for quick lookup + configuredRoles := make(map[string]org.Role) + for roleName, roleConfig := range orgConfig.Roles { + configuredRoles[strings.ToLower(roleName)] = roleConfig + } + + unconfiguredCount := len(roles) - len(configuredRoles) + logrus.Debugf("Processing %d organization roles (%d configured, %d unconfigured to check for cleanup)", + len(roles), len(configuredRoles), unconfiguredCount) + + var allErrors []error + + // Iterate over ALL GitHub roles to handle both configured and unconfigured roles + for _, role := range roles { + roleNameLower := strings.ToLower(role.Name) + + if roleConfig, isConfigured := configuredRoles[roleNameLower]; isConfigured { + // Role is in config - sync to match desired state + if err := configureRoleTeamAssignments(client, orgName, role.Name, role.ID, roleConfig.Teams, githubTeams); err != nil { + allErrors = append(allErrors, fmt.Errorf("failed to configure team assignments for role %s: %w", role.Name, err)) + } + if err := configureRoleUserAssignments(client, orgName, role.Name, role.ID, roleConfig.Users, invitees); err != nil { + allErrors = append(allErrors, fmt.Errorf("failed to configure user assignments for role %s: %w", role.Name, err)) + } + } else { + // Role is NOT in config - remove all assignments (clean up orphaned assignments) + logrus.Debugf("Role %q not in config, checking for assignments to clean up", role.Name) + if err := configureRoleTeamAssignments(client, orgName, role.Name, role.ID, []string{}, githubTeams); err != nil { + allErrors = append(allErrors, fmt.Errorf("failed to remove team assignments for unconfigured role %s: %w", role.Name, err)) + } + if err := configureRoleUserAssignments(client, orgName, role.Name, role.ID, []string{}, invitees); err != nil { + allErrors = append(allErrors, fmt.Errorf("failed to remove user assignments for unconfigured role %s: %w", role.Name, err)) + } + } + } + + // Check if any configured roles don't exist in GitHub + for roleName := range orgConfig.Roles { + found := false + for _, role := range roles { + if strings.EqualFold(role.Name, roleName) { + found = true + break + } + } + if !found { + return fmt.Errorf("role %q does not exist in organization %s - create the role in GitHub before assigning it", roleName, orgName) + } + } + + return utilerrors.NewAggregate(allErrors) +} + +// configureRoleTeamAssignments configures team assignments for a specific role +func configureRoleTeamAssignments(client orgRolesClient, orgName, roleName string, roleID int, wantTeams []string, githubTeams map[string]github.Team) error { + // Get current team assignments for this role + currentTeams, err := client.ListTeamsWithRole(orgName, roleID) + if err != nil { + return fmt.Errorf("failed to list teams with role %s: %w", roleName, err) + } + + // If we want no teams and have no teams, we're done + if len(wantTeams) == 0 && len(currentTeams) == 0 { + return nil + } + + // Build a map of normalized team name to team slug for the teams we have in config + // This allows resolving "MyTeam" (config name) to "my-team" (GitHub slug) + normalizedTeams := make(map[string]string) + for name, team := range githubTeams { + normalizedTeams[strings.ToLower(name)] = team.Slug + } + + // Create sets for comparison using slugs + wantSet := sets.New[string]() + for _, teamName := range wantTeams { + // Resolve config team name to slug + if slug, ok := normalizedTeams[strings.ToLower(teamName)]; ok { + wantSet.Insert(slug) + } else { + return fmt.Errorf("team %q referenced in role %q could not be resolved to a GitHub team slug - ensure the team exists in your teams configuration and was successfully created", teamName, roleName) + } + } + + haveSet := sets.New[string]() + for _, team := range currentTeams { + haveSet.Insert(team.Slug) + } + + // Teams to add + var errors []error + toAdd := wantSet.Difference(haveSet) + for teamSlug := range toAdd { + if err := client.AssignOrganizationRoleToTeam(orgName, teamSlug, roleID); err != nil { + errors = append(errors, fmt.Errorf("failed to assign role %s to team %s: %w", roleName, teamSlug, err)) + logrus.WithError(err).Warnf("Failed to assign role %s to team %s", roleName, teamSlug) + } else { + logrus.Infof("Assigned role %s to team %s", roleName, teamSlug) + } + } + + // Teams to remove + toRemove := haveSet.Difference(wantSet) + for teamSlug := range toRemove { + if err := client.RemoveOrganizationRoleFromTeam(orgName, teamSlug, roleID); err != nil { + errors = append(errors, fmt.Errorf("failed to remove role %s from team %s: %w", roleName, teamSlug, err)) + logrus.WithError(err).Warnf("Failed to remove role %s from team %s", roleName, teamSlug) + } else { + logrus.Infof("Removed role %s from team %s", roleName, teamSlug) + } + } + + return utilerrors.NewAggregate(errors) +} + +// configureRoleUserAssignments configures user assignments for a specific role +func configureRoleUserAssignments(client orgRolesClient, orgName, roleName string, roleID int, wantUsers []string, invitees sets.Set[string]) error { + // Get current user assignments for this role + currentUsers, err := client.ListUsersWithRole(orgName, roleID) + if err != nil { + return fmt.Errorf("failed to list users with role %s: %w", roleName, err) + } + + // Create maps to preserve original casing while comparing normalized usernames + wantMap := make(map[string]string) // normalized -> original + for _, user := range wantUsers { + wantMap[github.NormLogin(user)] = user + } + + // Only consider DIRECT assignments when building haveMap. + // Users with "indirect" assignment have the role via team membership and should not be + // removed just because they're not in the users list - they keep the role through their team. + haveMap := make(map[string]string) // normalized -> original + for _, user := range currentUsers { + if user.Assignment == "indirect" { + logrus.Debugf("Skipping indirect role assignment for user %s (has role via team membership)", user.Login) + continue + } + haveMap[github.NormLogin(user.Login)] = user.Login + } + + // If we want no direct users and have no direct users, we're done + if len(wantUsers) == 0 && len(haveMap) == 0 { + return nil + } + + // Create sets for comparison with normalized usernames + wantSet := sets.New[string]() + for normalized := range wantMap { + wantSet.Insert(normalized) + } + haveSet := sets.New[string]() + for normalized := range haveMap { + haveSet.Insert(normalized) + } + + // Users to add + var errors []error + toAdd := wantSet.Difference(haveSet) + for normalizedUser := range toAdd { + originalUser := wantMap[normalizedUser] + // Skip users who have pending org invitations - they must accept before we can assign roles + if invitees.Has(normalizedUser) { + logrus.Infof("Waiting for %s to accept org invitation before assigning role %s", originalUser, roleName) + continue + } + if err := client.AssignOrganizationRoleToUser(orgName, originalUser, roleID); err != nil { + errors = append(errors, fmt.Errorf("failed to assign role %s to user %s: %w", roleName, originalUser, err)) + logrus.WithError(err).Warnf("Failed to assign role %s to user %s", roleName, originalUser) + } else { + logrus.Infof("Assigned role %s to user %s", roleName, originalUser) + } + } + + // Users to remove + toRemove := haveSet.Difference(wantSet) + for normalizedUser := range toRemove { + originalUser := haveMap[normalizedUser] + if err := client.RemoveOrganizationRoleFromUser(orgName, originalUser, roleID); err != nil { + errors = append(errors, fmt.Errorf("failed to remove role %s from user %s: %w", roleName, originalUser, err)) + logrus.WithError(err).Warnf("Failed to remove role %s from user %s", roleName, originalUser) + } else { + logrus.Infof("Removed role %s from user %s", roleName, originalUser) + } + } + + return utilerrors.NewAggregate(errors) +} + // teamMembersClient can list/remove/update people to a team. type teamMembersClient interface { ListTeamMembersBySlug(org, teamSlug, role string) ([]github.TeamMember, error) diff --git a/cmd/peribolos/main_test.go b/cmd/peribolos/main_test.go index 527cb9457..329e652ad 100644 --- a/cmd/peribolos/main_test.go +++ b/cmd/peribolos/main_test.go @@ -22,6 +22,7 @@ import ( "fmt" "reflect" "sort" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -107,6 +108,10 @@ func TestOptions(t *testing.T) { name: "reject --fix-team-members without --fix-teams", args: []string{"--config-path=foo", "--fix-team-members"}, }, + { + name: "reject --fix-org-roles without --fix-teams", + args: []string{"--config-path=foo", "--fix-org-roles"}, + }, { name: "allow dump without config", args: []string{"--dump=frogger"}, @@ -1738,6 +1743,9 @@ func TestDumpOrgConfig(t *testing.T) { maintainers map[string][]string repoPermissions map[string][]github.Repo repos []github.FullRepo + roles []github.OrganizationRole + teamsWithRole map[int][]github.OrganizationRoleAssignment + usersWithRole map[int][]github.OrganizationRoleAssignment expected org.Config err bool }{ @@ -2024,6 +2032,55 @@ func TestDumpOrgConfig(t *testing.T) { Repos: map[string]org.Repo{}, }, }, + { + name: "dump organization with roles", + meta: github.Organization{ + Name: "Hello", + DefaultRepositoryPermission: "write", + }, + members: []string{"user1", "user2"}, + admins: []string{"admin"}, + roles: []github.OrganizationRole{ + {ID: 1, Name: "security-manager"}, + {ID: 2, Name: "billing-manager"}, + }, + teamsWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {{ID: 1, Slug: "security-team", Type: "Team"}}, + 2: {{ID: 2, Slug: "finance-team", Type: "Team"}}, + }, + usersWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {{ID: 3, Login: "security-admin", Type: "User"}}, + 2: {{ID: 4, Login: "finance-admin", Type: "User"}}, + }, + expected: org.Config{ + Metadata: org.Metadata{ + Name: &hello, + BillingEmail: &empty, + Company: &empty, + Email: &empty, + Description: &empty, + Location: &empty, + HasOrganizationProjects: &no, + HasRepositoryProjects: &no, + DefaultRepositoryPermission: &perm, + MembersCanCreateRepositories: &no, + }, + Members: []string{"user1", "user2"}, + Admins: []string{"admin"}, + Teams: map[string]org.Team{}, + Repos: map[string]org.Repo{}, + Roles: map[string]org.Role{ + "security-manager": { + Teams: []string{"security-team"}, + Users: []string{"security-admin"}, + }, + "billing-manager": { + Teams: []string{"finance-team"}, + Users: []string{"finance-admin"}, + }, + }, + }, + }, } for _, tc := range cases { @@ -2042,6 +2099,9 @@ func TestDumpOrgConfig(t *testing.T) { maintainers: tc.maintainers, repoPermissions: tc.repoPermissions, repos: tc.repos, + roles: tc.roles, + teamsWithRole: tc.teamsWithRole, + usersWithRole: tc.usersWithRole, } actual, err := dumpOrgConfig(fc, orgName, tc.ignoreSecretTeams, "") switch { @@ -2073,6 +2133,9 @@ type fakeDumpClient struct { maintainers map[string][]string repoPermissions map[string][]github.Repo repos []github.FullRepo + roles []github.OrganizationRole + teamsWithRole map[int][]github.OrganizationRoleAssignment + usersWithRole map[int][]github.OrganizationRoleAssignment } func (c fakeDumpClient) GetOrg(name string) (*github.Organization, error) { @@ -2194,6 +2257,27 @@ func (c fakeDumpClient) ListRepoInvitations(org, repo string) ([]github.Collabor return []github.CollaboratorRepoInvitation{}, nil } +func (c fakeDumpClient) ListOrganizationRoles(org string) ([]github.OrganizationRole, error) { + if org != c.name { + return nil, fmt.Errorf("bad org: %s", org) + } + return c.roles, nil +} + +func (c fakeDumpClient) ListTeamsWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) { + if org != c.name { + return nil, fmt.Errorf("bad org: %s", org) + } + return c.teamsWithRole[roleID], nil +} + +func (c fakeDumpClient) ListUsersWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) { + if org != c.name { + return nil, fmt.Errorf("bad org: %s", org) + } + return c.usersWithRole[roleID], nil +} + func fixup(ret *org.Config) { if ret == nil { return @@ -2206,6 +2290,11 @@ func fixup(ret *org.Config) { sort.Strings(team.Previously) ret.Teams[name] = team } + for name, role := range ret.Roles { + sort.Strings(role.Teams) + sort.Strings(role.Users) + ret.Roles[name] = role + } } func TestOrgInvitations(t *testing.T) { @@ -3970,3 +4059,683 @@ func TestConfigureCollaborators_PermissionMatrix_PendingInvitationUpdates(t *tes } } } + +// Test organization roles functionality - comprehensive tests +func TestOrganizationRoles(t *testing.T) { + // Test that the functions exist and can be called + // This ensures the functions compile and have correct signatures + t.Log("Organization roles functions exist and compile correctly") + + // Test basic config structures + t.Run("role config structure", func(t *testing.T) { + // Test org.Role structure + role := org.Role{ + Teams: []string{"team1", "team2"}, + Users: []string{"user1", "user2"}, + } + + if len(role.Teams) != 2 { + t.Errorf("Expected 2 teams, got %d", len(role.Teams)) + } + if len(role.Users) != 2 { + t.Errorf("Expected 2 users, got %d", len(role.Users)) + } + + // Test org.Config with roles + config := org.Config{ + Roles: map[string]org.Role{ + "security-manager": role, + }, + } + + if len(config.Roles) != 1 { + t.Errorf("Expected 1 role, got %d", len(config.Roles)) + } + + secRole, exists := config.Roles["security-manager"] + if !exists { + t.Error("security-manager role should exist") + } + if len(secRole.Teams) != 2 { + t.Errorf("Expected 2 teams in security-manager role, got %d", len(secRole.Teams)) + } + }) +} + +// Test organization roles fail-fast scenarios +func TestOrganizationRolesFailFast(t *testing.T) { + t.Run("validate role assignments correctly handle missing entities", func(t *testing.T) { + // Test role mapping logic + roleMap := map[string]int{ + "existing-role": 1, + } + + // This would fail - role doesn't exist + if roleID, exists := roleMap["non-existent-role"]; exists { + t.Errorf("Expected role 'non-existent-role' to not exist, but got ID %d", roleID) + } + + // This would succeed - role exists + if _, exists := roleMap["existing-role"]; !exists { + t.Error("Expected role 'existing-role' to exist") + } + + // Test team validation logic + githubTeams := map[string]github.Team{ + "existing-team": {ID: 1, Slug: "existing-team", Name: "Existing Team"}, + } + + // This would fail - team doesn't exist + if _, exists := githubTeams["non-existent-team"]; exists { + t.Error("Expected team 'non-existent-team' to not exist") + } + + // This would succeed - team exists + if _, exists := githubTeams["existing-team"]; !exists { + t.Error("Expected team 'existing-team' to exist") + } + }) + + t.Run("error messages contain expected information", func(t *testing.T) { + // Test that our error messages contain the expected information + testCases := []struct { + name string + errorMsg string + expectedContains []string + }{ + { + name: "role not found error", + errorMsg: "role non-existent-role does not exist in organization test-org - check your configuration", + expectedContains: []string{"role", "non-existent-role", "does not exist", "organization", "test-org"}, + }, + { + name: "team not found error", + errorMsg: "team non-existent-team not found in organization test-org, cannot assign role security-manager - check your team configuration", + expectedContains: []string{"team", "non-existent-team", "not found", "organization", "test-org", "security-manager"}, + }, + { + name: "user not org member error", + errorMsg: "all users with role assignments must be org members: alice, bob", + expectedContains: []string{"users with role assignments", "must be org members", "alice", "bob"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + for _, expected := range tc.expectedContains { + if !strings.Contains(tc.errorMsg, expected) { + t.Errorf("Expected error message to contain '%s', but message was: %s", expected, tc.errorMsg) + } + } + }) + } + }) + + // Note: User/team reference validation is tested in pkg/config/org/org_test.go (TestValidateRoles) + // The validation happens at the start of configureOrg(), before any API calls are made + + t.Run("fails when configured role does not exist in GitHub", func(t *testing.T) { + orgConfig := org.Config{ + Admins: []string{"admin-user"}, + Roles: map[string]org.Role{ + "non-existent-role": { + Users: []string{"admin-user"}, + }, + }, + } + + client := &fakeOrgRolesClient{ + roles: []github.OrganizationRole{ + {ID: 1, Name: "other-role"}, + }, + } + + githubTeams := map[string]github.Team{} + invitees := sets.Set[string]{} + + err := configureOrgRoles(client, "test-org", orgConfig, githubTeams, invitees) + if err == nil { + t.Error("Expected error for non-existent role, but got none") + } + if !strings.Contains(err.Error(), "does not exist") { + t.Errorf("Expected error about role not existing, got: %v", err) + } + if !strings.Contains(err.Error(), "non-existent-role") { + t.Errorf("Expected error to mention the role name, got: %v", err) + } + }) + + t.Run("role name matching is case-insensitive", func(t *testing.T) { + orgConfig := org.Config{ + Admins: []string{"admin-user"}, + Roles: map[string]org.Role{ + "Security-Manager": { // Different casing than what's in GitHub + Users: []string{"admin-user"}, + }, + }, + } + + client := &fakeOrgRolesClient{ + roles: []github.OrganizationRole{ + {ID: 1, Name: "security-manager"}, // Lowercase in GitHub + }, + usersWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {}, // No current assignments + }, + } + + githubTeams := map[string]github.Team{} + invitees := sets.Set[string]{} + + err := configureOrgRoles(client, "test-org", orgConfig, githubTeams, invitees) + if err != nil { + t.Errorf("Expected case-insensitive role matching to succeed, got error: %v", err) + } + + // Verify the role was assigned + if len(client.assignedUserRoles) != 1 { + t.Errorf("Expected 1 user role assignment, got %d", len(client.assignedUserRoles)) + } + }) + + t.Run("removes assignments for roles not in config", func(t *testing.T) { + // Config has NO roles - but GitHub has a role with assignments + orgConfig := org.Config{ + Admins: []string{"admin-user"}, + Roles: map[string]org.Role{}, // Empty - no roles configured + } + + client := &fakeOrgRolesClient{ + roles: []github.OrganizationRole{ + {ID: 1, Name: "security-manager"}, + }, + teamsWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {{ID: 10, Slug: "old-team", Type: "Team"}}, // Existing team assignment + }, + usersWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {{ID: 5, Login: "old-user", Type: "User"}}, // Existing user assignment + }, + } + + githubTeams := map[string]github.Team{} + invitees := sets.Set[string]{} + + err := configureOrgRoles(client, "test-org", orgConfig, githubTeams, invitees) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Verify the orphaned assignments were removed + if len(client.removedTeamRoles) != 1 { + t.Errorf("Expected 1 team role removal, got %d: %v", len(client.removedTeamRoles), client.removedTeamRoles) + } + if len(client.removedUserRoles) != 1 { + t.Errorf("Expected 1 user role removal, got %d: %v", len(client.removedUserRoles), client.removedUserRoles) + } + + // Verify no new assignments were made + if len(client.assignedTeamRoles) != 0 { + t.Errorf("Expected no team assignments, got %d: %v", len(client.assignedTeamRoles), client.assignedTeamRoles) + } + if len(client.assignedUserRoles) != 0 { + t.Errorf("Expected no user assignments, got %d: %v", len(client.assignedUserRoles), client.assignedUserRoles) + } + }) + + t.Run("cleans up role when removed from config but keeps others", func(t *testing.T) { + // Config has only one role, but GitHub has two roles with assignments + orgConfig := org.Config{ + Admins: []string{"admin-user"}, + Roles: map[string]org.Role{ + "billing-manager": { + Users: []string{"admin-user"}, // Keep this role + }, + }, + } + + client := &fakeOrgRolesClient{ + roles: []github.OrganizationRole{ + {ID: 1, Name: "security-manager"}, // Not in config - should be cleaned up + {ID: 2, Name: "billing-manager"}, // In config - should be synced + }, + teamsWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {{ID: 10, Slug: "security-team", Type: "Team"}}, // Should be removed + 2: {}, // No teams + }, + usersWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {{ID: 5, Login: "security-admin", Type: "User"}}, // Should be removed + 2: {}, // No users yet + }, + } + + githubTeams := map[string]github.Team{} + invitees := sets.Set[string]{} + + err := configureOrgRoles(client, "test-org", orgConfig, githubTeams, invitees) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Verify security-manager assignments were removed (role ID 1) + foundSecurityTeamRemoval := false + foundSecurityUserRemoval := false + for _, removal := range client.removedTeamRoles { + if strings.Contains(removal, "/1") { // roleID 1 + foundSecurityTeamRemoval = true + } + } + for _, removal := range client.removedUserRoles { + if strings.Contains(removal, "/1") { // roleID 1 + foundSecurityUserRemoval = true + } + } + if !foundSecurityTeamRemoval { + t.Errorf("Expected security-manager team assignment to be removed, removals: %v", client.removedTeamRoles) + } + if !foundSecurityUserRemoval { + t.Errorf("Expected security-manager user assignment to be removed, removals: %v", client.removedUserRoles) + } + + // Verify billing-manager was assigned (role ID 2) + foundBillingUserAssignment := false + for _, assignment := range client.assignedUserRoles { + if strings.Contains(assignment, "/2") { // roleID 2 + foundBillingUserAssignment = true + } + } + if !foundBillingUserAssignment { + t.Errorf("Expected billing-manager user assignment, assignments: %v", client.assignedUserRoles) + } + }) +} + +// fakeOrgRolesClient is a mock client for testing organization roles functionality +type fakeOrgRolesClient struct { + roles []github.OrganizationRole + teamsWithRole map[int][]github.OrganizationRoleAssignment + usersWithRole map[int][]github.OrganizationRoleAssignment + assignedTeamRoles []string // Track calls: "org/team-slug/roleID" + removedTeamRoles []string // Track calls: "org/team-slug/roleID" + assignedUserRoles []string // Track calls: "org/user/roleID" + removedUserRoles []string // Track calls: "org/user/roleID" + assignTeamRoleErr error + removeTeamRoleErr error + assignUserRoleErr error + removeUserRoleErr error + listOrganizationRolesErr error + listTeamsWithRoleErr error + listUsersWithRoleErr error +} + +func (f *fakeOrgRolesClient) ListOrganizationRoles(org string) ([]github.OrganizationRole, error) { + if f.listOrganizationRolesErr != nil { + return nil, f.listOrganizationRolesErr + } + return f.roles, nil +} + +func (f *fakeOrgRolesClient) AssignOrganizationRoleToTeam(org, teamSlug string, roleID int) error { + if f.assignTeamRoleErr != nil { + return f.assignTeamRoleErr + } + f.assignedTeamRoles = append(f.assignedTeamRoles, fmt.Sprintf("%s/%s/%d", org, teamSlug, roleID)) + return nil +} + +func (f *fakeOrgRolesClient) RemoveOrganizationRoleFromTeam(org, teamSlug string, roleID int) error { + if f.removeTeamRoleErr != nil { + return f.removeTeamRoleErr + } + f.removedTeamRoles = append(f.removedTeamRoles, fmt.Sprintf("%s/%s/%d", org, teamSlug, roleID)) + return nil +} + +func (f *fakeOrgRolesClient) AssignOrganizationRoleToUser(org, user string, roleID int) error { + if f.assignUserRoleErr != nil { + return f.assignUserRoleErr + } + f.assignedUserRoles = append(f.assignedUserRoles, fmt.Sprintf("%s/%s/%d", org, user, roleID)) + return nil +} + +func (f *fakeOrgRolesClient) RemoveOrganizationRoleFromUser(org, user string, roleID int) error { + if f.removeUserRoleErr != nil { + return f.removeUserRoleErr + } + f.removedUserRoles = append(f.removedUserRoles, fmt.Sprintf("%s/%s/%d", org, user, roleID)) + return nil +} + +func (f *fakeOrgRolesClient) ListTeamsWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) { + if f.listTeamsWithRoleErr != nil { + return nil, f.listTeamsWithRoleErr + } + return f.teamsWithRole[roleID], nil +} + +func (f *fakeOrgRolesClient) ListUsersWithRole(org string, roleID int) ([]github.OrganizationRoleAssignment, error) { + if f.listUsersWithRoleErr != nil { + return nil, f.listUsersWithRoleErr + } + return f.usersWithRole[roleID], nil +} + +// Test configureRoleTeamAssignments +func TestConfigureRoleTeamAssignments(t *testing.T) { + tests := []struct { + name string + roleName string + roleID int + wantTeams []string + currentTeams []github.OrganizationRoleAssignment + githubTeams map[string]github.Team + expectAssigned []string + expectRemoved []string + listTeamsErr error + assignErr error + removeErr error + expectError bool + }{ + { + name: "no teams to configure", + roleName: "security-manager", + roleID: 1, + wantTeams: []string{}, + currentTeams: []github.OrganizationRoleAssignment{}, + githubTeams: map[string]github.Team{}, + expectAssigned: []string{}, + expectRemoved: []string{}, + }, + { + name: "add new team assignment", + roleName: "security-manager", + roleID: 1, + wantTeams: []string{"security-team"}, + currentTeams: []github.OrganizationRoleAssignment{}, + githubTeams: map[string]github.Team{ + "security-team": {ID: 10, Slug: "security-team", Name: "Security Team"}, + }, + expectAssigned: []string{"test-org/security-team/1"}, + expectRemoved: []string{}, + }, + { + name: "remove team assignment", + roleName: "security-manager", + roleID: 1, + wantTeams: []string{}, + currentTeams: []github.OrganizationRoleAssignment{ + {ID: 10, Slug: "old-team", Type: "Team"}, + }, + githubTeams: map[string]github.Team{}, + expectAssigned: []string{}, + expectRemoved: []string{"test-org/old-team/1"}, + }, + { + name: "add and remove teams", + roleName: "security-manager", + roleID: 1, + wantTeams: []string{"new-team"}, + currentTeams: []github.OrganizationRoleAssignment{ + {ID: 10, Slug: "old-team", Type: "Team"}, + }, + githubTeams: map[string]github.Team{ + "new-team": {ID: 20, Slug: "new-team", Name: "New Team"}, + }, + expectAssigned: []string{"test-org/new-team/1"}, + expectRemoved: []string{"test-org/old-team/1"}, + }, + { + name: "fail when team does not exist", + roleName: "security-manager", + roleID: 1, + wantTeams: []string{"non-existent-team"}, + currentTeams: []github.OrganizationRoleAssignment{}, + githubTeams: map[string]github.Team{ + "valid-team": {ID: 20, Slug: "valid-team", Name: "Valid Team"}, + }, + expectAssigned: []string{}, + expectRemoved: []string{}, + expectError: true, + }, + { + name: "no changes needed", + roleName: "security-manager", + roleID: 1, + wantTeams: []string{"existing-team"}, + currentTeams: []github.OrganizationRoleAssignment{ + {ID: 10, Slug: "existing-team", Type: "Team"}, + }, + githubTeams: map[string]github.Team{ + "existing-team": {ID: 10, Slug: "existing-team", Name: "Existing Team"}, + }, + expectAssigned: []string{}, + expectRemoved: []string{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := &fakeOrgRolesClient{ + teamsWithRole: map[int][]github.OrganizationRoleAssignment{ + tc.roleID: tc.currentTeams, + }, + listTeamsWithRoleErr: tc.listTeamsErr, + assignTeamRoleErr: tc.assignErr, + removeTeamRoleErr: tc.removeErr, + } + + err := configureRoleTeamAssignments(client, "test-org", tc.roleName, tc.roleID, tc.wantTeams, tc.githubTeams) + + if tc.expectError && err == nil { + t.Error("Expected error but got none") + } + if !tc.expectError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Compare slices with nil-safe comparison + if !slicesEqual(client.assignedTeamRoles, tc.expectAssigned) { + t.Errorf("Assigned teams mismatch:\nExpected: %v\nGot: %v", tc.expectAssigned, client.assignedTeamRoles) + } + + if !slicesEqual(client.removedTeamRoles, tc.expectRemoved) { + t.Errorf("Removed teams mismatch:\nExpected: %v\nGot: %v", tc.expectRemoved, client.removedTeamRoles) + } + }) + } +} + +// Test configureRoleUserAssignments with username normalization +func TestConfigureRoleUserAssignments(t *testing.T) { + tests := []struct { + name string + roleName string + roleID int + wantUsers []string + currentUsers []github.OrganizationRoleAssignment + expectAssigned []string + expectRemoved []string + listUsersErr error + assignErr error + removeErr error + expectError bool + }{ + { + name: "no users to configure", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{}, + currentUsers: []github.OrganizationRoleAssignment{}, + expectAssigned: []string{}, + expectRemoved: []string{}, + }, + { + name: "add new user assignment", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{"new-user"}, + currentUsers: []github.OrganizationRoleAssignment{}, + expectAssigned: []string{"test-org/new-user/1"}, + expectRemoved: []string{}, + }, + { + name: "remove user assignment", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{}, + currentUsers: []github.OrganizationRoleAssignment{ + {ID: 5, Login: "old-user", Type: "User"}, + }, + expectAssigned: []string{}, + expectRemoved: []string{"test-org/old-user/1"}, + }, + { + name: "add and remove users", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{"new-user"}, + currentUsers: []github.OrganizationRoleAssignment{ + {ID: 5, Login: "old-user", Type: "User"}, + }, + expectAssigned: []string{"test-org/new-user/1"}, + expectRemoved: []string{"test-org/old-user/1"}, + }, + { + name: "username normalization - case insensitive matching", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{"NewUser"}, + currentUsers: []github.OrganizationRoleAssignment{ + {ID: 5, Login: "newuser", Type: "User"}, + }, + expectAssigned: []string{}, + expectRemoved: []string{}, + }, + { + name: "preserves original username casing when adding", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{"NewUser", "AnotherUser"}, + currentUsers: []github.OrganizationRoleAssignment{}, + expectAssigned: []string{"test-org/NewUser/1", "test-org/AnotherUser/1"}, + expectRemoved: []string{}, + }, + { + name: "preserves original username casing when removing", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{}, + currentUsers: []github.OrganizationRoleAssignment{ + {ID: 5, Login: "OldUser", Type: "User"}, + {ID: 6, Login: "AnotherOldUser", Type: "User"}, + }, + expectAssigned: []string{}, + expectRemoved: []string{"test-org/OldUser/1", "test-org/AnotherOldUser/1"}, + }, + { + name: "no changes needed", + roleName: "security-manager", + roleID: 1, + wantUsers: []string{"existing-user"}, + currentUsers: []github.OrganizationRoleAssignment{ + {ID: 5, Login: "existing-user", Type: "User"}, + }, + expectAssigned: []string{}, + expectRemoved: []string{}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := &fakeOrgRolesClient{ + usersWithRole: map[int][]github.OrganizationRoleAssignment{ + tc.roleID: tc.currentUsers, + }, + listUsersWithRoleErr: tc.listUsersErr, + assignUserRoleErr: tc.assignErr, + removeUserRoleErr: tc.removeErr, + } + invitees := sets.Set[string]{} + + err := configureRoleUserAssignments(client, "test-org", tc.roleName, tc.roleID, tc.wantUsers, invitees) + + if tc.expectError && err == nil { + t.Error("Expected error but got none") + } + if !tc.expectError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Compare slices with nil-safe comparison (sort first since order is non-deterministic) + sort.Strings(client.assignedUserRoles) + expectedAssigned := append([]string{}, tc.expectAssigned...) + sort.Strings(expectedAssigned) + if !slicesEqual(client.assignedUserRoles, expectedAssigned) { + t.Errorf("Assigned users mismatch:\nExpected: %v\nGot: %v", expectedAssigned, client.assignedUserRoles) + } + + sort.Strings(client.removedUserRoles) + expectedRemoved := append([]string{}, tc.expectRemoved...) + sort.Strings(expectedRemoved) + if !slicesEqual(client.removedUserRoles, expectedRemoved) { + t.Errorf("Removed users mismatch:\nExpected: %v\nGot: %v", expectedRemoved, client.removedUserRoles) + } + }) + } +} + +// Test that users with pending org invitations are skipped for role assignment +func TestConfigureRoleUserAssignmentsSkipsPendingInvitees(t *testing.T) { + client := &fakeOrgRolesClient{ + usersWithRole: map[int][]github.OrganizationRoleAssignment{ + 1: {}, // No current assignments + }, + } + + // User has a pending invitation to the org + invitees := sets.New[string]("pending-user") + + err := configureRoleUserAssignments(client, "test-org", "security-manager", 1, []string{"pending-user", "existing-user"}, invitees) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + // Verify only the non-pending user was assigned + if len(client.assignedUserRoles) != 1 { + t.Errorf("Expected 1 user role assignment, got %d: %v", len(client.assignedUserRoles), client.assignedUserRoles) + } + + // The pending user should NOT have been assigned + for _, assignment := range client.assignedUserRoles { + if strings.Contains(assignment, "pending-user") { + t.Errorf("User with pending invitation should not have been assigned a role: %v", client.assignedUserRoles) + } + } + + // The existing user SHOULD have been assigned + foundExistingUser := false + for _, assignment := range client.assignedUserRoles { + if strings.Contains(assignment, "existing-user") { + foundExistingUser = true + } + } + if !foundExistingUser { + t.Errorf("Expected existing-user to be assigned, got: %v", client.assignedUserRoles) + } +} + +// slicesEqual compares two string slices treating nil and empty slices as equal +func slicesEqual(a, b []string) bool { + if len(a) == 0 && len(b) == 0 { + return true + } + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/pkg/config/org/org.go b/pkg/config/org/org.go index 3afeca07c..bb1b88a45 100644 --- a/pkg/config/org/org.go +++ b/pkg/config/org/org.go @@ -18,6 +18,7 @@ package org import ( "fmt" + "strings" "sigs.k8s.io/prow/pkg/github" ) @@ -86,6 +87,68 @@ type Config struct { Members []string `json:"members,omitempty"` Admins []string `json:"admins,omitempty"` Repos map[string]Repo `json:"repos,omitempty"` + Roles map[string]Role `json:"roles,omitempty"` +} + +// Role declares an organization role and its assignments to teams and users +// +// See https://docs.github.com/en/rest/orgs/organization-roles#assign-an-organization-role-to-a-team +// See https://docs.github.com/en/rest/orgs/organization-roles#assign-an-organization-role-to-a-user +type Role struct { + // Teams is a list of team names (from config keys) that have this role assigned + Teams []string `json:"teams,omitempty"` + // Users is a list of usernames that have this role assigned + Users []string `json:"users,omitempty"` +} + +// ValidateRoles checks that all teams and users referenced in role assignments exist in the configuration +func (c *Config) ValidateRoles() error { + if len(c.Roles) == 0 { + return nil + } + + // Build a set of all team slugs (including nested teams) + availableTeams := make(map[string]bool) + var collectTeams func(teams map[string]Team) + collectTeams = func(teams map[string]Team) { + for name, team := range teams { + availableTeams[strings.ToLower(name)] = true + if len(team.Children) > 0 { + collectTeams(team.Children) + } + } + } + collectTeams(c.Teams) + + // Build a set of all org members (normalized) + availableUsers := make(map[string]bool) + for _, user := range c.Admins { + availableUsers[github.NormLogin(user)] = true + } + for _, user := range c.Members { + availableUsers[github.NormLogin(user)] = true + } + + // Validate each role's team and user references + var errors []string + for roleName, role := range c.Roles { + for _, teamSlug := range role.Teams { + if !availableTeams[strings.ToLower(teamSlug)] { + errors = append(errors, fmt.Sprintf("role %q references undefined team %q", roleName, teamSlug)) + } + } + for _, user := range role.Users { + if !availableUsers[github.NormLogin(user)] { + errors = append(errors, fmt.Sprintf("role %q references user %q who is not an org member", roleName, user)) + } + } + } + + if len(errors) > 0 { + return fmt.Errorf("role validation failed:\n - %s", strings.Join(errors, "\n - ")) + } + + return nil } // TeamMetadata declares metadata about the github team. diff --git a/pkg/config/org/org_test.go b/pkg/config/org/org_test.go index f3fd87d99..d8d459813 100644 --- a/pkg/config/org/org_test.go +++ b/pkg/config/org/org_test.go @@ -19,6 +19,7 @@ package org import ( "encoding/json" "reflect" + "strings" "testing" "k8s.io/apimachinery/pkg/util/diff" @@ -131,3 +132,220 @@ func TestPruneRepoDefaults(t *testing.T) { }) } } + +func TestValidateRoles(t *testing.T) { + tests := []struct { + name string + config Config + expectError bool + errorPart string + }{ + { + name: "no roles - should pass", + config: Config{ + Teams: map[string]Team{ + "team1": {Members: []string{"user1"}}, + }, + }, + expectError: false, + }, + { + name: "valid role with existing team", + config: Config{ + Teams: map[string]Team{ + "security-team": {Members: []string{"user1"}}, + }, + Admins: []string{"user1"}, + Roles: map[string]Role{ + "security-manager": { + Teams: []string{"security-team"}, + Users: []string{"user1"}, + }, + }, + }, + expectError: false, + }, + { + name: "invalid role with non-existent team", + config: Config{ + Teams: map[string]Team{ + "existing-team": {Members: []string{"user1"}}, + }, + Members: []string{"user1"}, + Roles: map[string]Role{ + "security-manager": { + Teams: []string{"non-existent-team"}, + }, + }, + }, + expectError: true, + errorPart: "non-existent-team", + }, + { + name: "multiple roles with mixed valid and invalid teams", + config: Config{ + Teams: map[string]Team{ + "valid-team": {Members: []string{"user1"}}, + }, + Members: []string{"user1"}, + Roles: map[string]Role{ + "role1": { + Teams: []string{"valid-team"}, + }, + "role2": { + Teams: []string{"invalid-team"}, + }, + }, + }, + expectError: true, + errorPart: "invalid-team", + }, + { + name: "role with nested team reference", + config: Config{ + Teams: map[string]Team{ + "parent-team": { + Members: []string{"user1"}, + Children: map[string]Team{ + "child-team": { + Members: []string{"user2"}, + }, + }, + }, + }, + Members: []string{"user1", "user2"}, + Roles: map[string]Role{ + "security-manager": { + Teams: []string{"child-team"}, + }, + }, + }, + expectError: false, + }, + { + name: "role with only users (no teams)", + config: Config{ + Teams: map[string]Team{ + "some-team": {Members: []string{"user1"}}, + }, + Admins: []string{"user1"}, + Members: []string{"user2"}, + Roles: map[string]Role{ + "security-manager": { + Users: []string{"user1", "user2"}, + }, + }, + }, + expectError: false, + }, + { + name: "role references user who is not org member", + config: Config{ + Admins: []string{"admin-user"}, + Members: []string{"member-user"}, + Roles: map[string]Role{ + "security-manager": { + Users: []string{"non-member-user"}, + }, + }, + }, + expectError: true, + errorPart: "non-member-user", + }, + { + name: "role references user with different casing - should pass", + config: Config{ + Admins: []string{"AdminUser"}, + Roles: map[string]Role{ + "security-manager": { + Users: []string{"adminuser"}, // Different casing but same user + }, + }, + }, + expectError: false, + }, + { + name: "role with both valid and invalid users", + config: Config{ + Admins: []string{"valid-user"}, + Roles: map[string]Role{ + "security-manager": { + Users: []string{"valid-user", "invalid-user"}, + }, + }, + }, + expectError: true, + errorPart: "invalid-user", + }, + { + name: "role with case-insensitive team matching", + config: Config{ + Teams: map[string]Team{ + "Security-Team": {Members: []string{"user1"}}, + }, + Admins: []string{"user1"}, + Roles: map[string]Role{ + "admin": { + Teams: []string{"security-team"}, // Different case + }, + }, + }, + expectError: false, + }, + { + name: "role with deeply nested teams", + config: Config{ + Teams: map[string]Team{ + "level1": { + Children: map[string]Team{ + "level2": { + Children: map[string]Team{ + "level3": {Members: []string{"user1"}}, + }, + }, + }, + }, + }, + Members: []string{"user1"}, + Roles: map[string]Role{ + "role1": { + Teams: []string{"level3"}, + }, + }, + }, + expectError: false, + }, + { + name: "role with duplicate team references (should pass)", + config: Config{ + Teams: map[string]Team{ + "team1": {Members: []string{"user1"}}, + }, + Admins: []string{"user1"}, + Roles: map[string]Role{ + "role1": { + Teams: []string{"team1", "team1"}, + }, + }, + }, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.config.ValidateRoles() + if tc.expectError { + if err == nil { + t.Error("Expected error but got none") + } else if tc.errorPart != "" && !strings.Contains(err.Error(), tc.errorPart) { + t.Errorf("Expected error to contain %q, but got: %v", tc.errorPart, err) + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + } + }) + } +} diff --git a/pkg/github/client.go b/pkg/github/client.go index be9b98476..da727a339 100644 --- a/pkg/github/client.go +++ b/pkg/github/client.go @@ -74,6 +74,17 @@ type OrganizationClient interface { RemoveOrgMembership(org, user string) error } +// OrganizationRolesClient interface for organization roles related API actions +type OrganizationRolesClient interface { + ListOrganizationRoles(org string) ([]OrganizationRole, error) + AssignOrganizationRoleToTeam(org, teamSlug string, roleID int) error + RemoveOrganizationRoleFromTeam(org, teamSlug string, roleID int) error + AssignOrganizationRoleToUser(org, user string, roleID int) error + RemoveOrganizationRoleFromUser(org, user string, roleID int) error + ListTeamsWithRole(org string, roleID int) ([]OrganizationRoleAssignment, error) + ListUsersWithRole(org string, roleID int) ([]OrganizationRoleAssignment, error) +} + // HookClient interface for hook related API actions type HookClient interface { ListOrgHooks(org string) ([]Hook, error) @@ -268,6 +279,7 @@ type Client interface { IssueClient CommentClient OrganizationClient + OrganizationRolesClient TeamClient ProjectClient MilestoneClient @@ -5164,3 +5176,163 @@ func (c *client) ListRepoInvitations(org, repo string) ([]CollaboratorRepoInvita } return ret, nil } + +// ListOrganizationRoles lists all organization roles +// +// https://docs.github.com/en/rest/orgs/organization-roles#list-organization-roles +func (c *client) ListOrganizationRoles(org string) ([]OrganizationRole, error) { + c.log("ListOrganizationRoles", org) + if c.fake { + return nil, nil + } + + path := fmt.Sprintf("/orgs/%s/organization-roles", org) + + // The API returns a wrapper object with total_count and roles array + type orgRolesResponse struct { + TotalCount int `json:"total_count"` + Roles []OrganizationRole `json:"roles"` + } + + var response orgRolesResponse + _, err := c.request(&request{ + method: http.MethodGet, + path: path, + org: org, + exitCodes: []int{200}, + }, &response) + + if err != nil { + return nil, err + } + + return response.Roles, nil +} + +// AssignOrganizationRoleToTeam assigns an organization role to a team +// +// https://docs.github.com/en/rest/orgs/organization-roles#assign-an-organization-role-to-a-team +func (c *client) AssignOrganizationRoleToTeam(org, teamSlug string, roleID int) error { + c.log("AssignOrganizationRoleToTeam", org, teamSlug, roleID) + if c.dry { + return nil + } + + _, err := c.request(&request{ + method: http.MethodPut, + path: fmt.Sprintf("/orgs/%s/organization-roles/teams/%s/%d", org, teamSlug, roleID), + org: org, + exitCodes: []int{204}, + }, nil) + return err +} + +// RemoveOrganizationRoleFromTeam removes an organization role from a team +// +// https://docs.github.com/en/rest/orgs/organization-roles#remove-an-organization-role-from-a-team +func (c *client) RemoveOrganizationRoleFromTeam(org, teamSlug string, roleID int) error { + c.log("RemoveOrganizationRoleFromTeam", org, teamSlug, roleID) + if c.dry { + return nil + } + + _, err := c.request(&request{ + method: http.MethodDelete, + path: fmt.Sprintf("/orgs/%s/organization-roles/teams/%s/%d", org, teamSlug, roleID), + org: org, + exitCodes: []int{204}, + }, nil) + return err +} + +// AssignOrganizationRoleToUser assigns an organization role to a user +// +// https://docs.github.com/en/rest/orgs/organization-roles#assign-an-organization-role-to-a-user +func (c *client) AssignOrganizationRoleToUser(org, user string, roleID int) error { + c.log("AssignOrganizationRoleToUser", org, user, roleID) + if c.dry { + return nil + } + + _, err := c.request(&request{ + method: http.MethodPut, + path: fmt.Sprintf("/orgs/%s/organization-roles/users/%s/%d", org, user, roleID), + org: org, + exitCodes: []int{204}, + }, nil) + return err +} + +// RemoveOrganizationRoleFromUser removes an organization role from a user +// +// https://docs.github.com/en/rest/orgs/organization-roles#remove-an-organization-role-from-a-user +func (c *client) RemoveOrganizationRoleFromUser(org, user string, roleID int) error { + c.log("RemoveOrganizationRoleFromUser", org, user, roleID) + if c.dry { + return nil + } + + _, err := c.request(&request{ + method: http.MethodDelete, + path: fmt.Sprintf("/orgs/%s/organization-roles/users/%s/%d", org, user, roleID), + org: org, + exitCodes: []int{204}, + }, nil) + return err +} + +// ListTeamsWithRole lists all teams assigned to a specific organization role +// +// https://docs.github.com/en/rest/orgs/organization-roles#list-teams-assigned-to-an-organization-role +func (c *client) ListTeamsWithRole(org string, roleID int) ([]OrganizationRoleAssignment, error) { + c.log("ListTeamsWithRole", org, roleID) + if c.fake { + return nil, nil + } + + path := fmt.Sprintf("/orgs/%s/organization-roles/%d/teams", org, roleID) + var assignments []OrganizationRoleAssignment + err := c.readPaginatedResults( + path, + acceptNone, + org, + func() interface{} { + return &[]OrganizationRoleAssignment{} + }, + func(obj interface{}) { + assignments = append(assignments, *(obj.(*[]OrganizationRoleAssignment))...) + }, + ) + if err != nil { + return nil, err + } + return assignments, nil +} + +// ListUsersWithRole lists all users assigned to a specific organization role +// +// https://docs.github.com/en/rest/orgs/organization-roles#list-users-assigned-to-an-organization-role +func (c *client) ListUsersWithRole(org string, roleID int) ([]OrganizationRoleAssignment, error) { + c.log("ListUsersWithRole", org, roleID) + if c.fake { + return nil, nil + } + + path := fmt.Sprintf("/orgs/%s/organization-roles/%d/users", org, roleID) + var assignments []OrganizationRoleAssignment + err := c.readPaginatedResults( + path, + acceptNone, + org, + func() interface{} { + return &[]OrganizationRoleAssignment{} + }, + func(obj interface{}) { + assignments = append(assignments, *(obj.(*[]OrganizationRoleAssignment))...) + }, + ) + if err != nil { + return nil, err + } + return assignments, nil +} diff --git a/pkg/github/types.go b/pkg/github/types.go index 487582bc1..924bdfeb9 100644 --- a/pkg/github/types.go +++ b/pkg/github/types.go @@ -1751,3 +1751,22 @@ type Layers struct { MediaType string `json:"media_type"` Size int `json:"size"` } + +// OrganizationRole represents an organization role +type OrganizationRole struct { + ID int `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Permissions []string `json:"permissions"` +} + +// OrganizationRoleAssignment represents a role assignment to a team or user +type OrganizationRoleAssignment struct { + ID int `json:"id"` + Login string `json:"login,omitempty"` + Slug string `json:"slug,omitempty"` + Type string `json:"type"` // "User" or "Team" + RoleID int `json:"role_id"` + RoleName string `json:"role_name"` + Assignment string `json:"assignment"` // "direct" or "indirect" (indirect = via team membership) +}