diff --git a/github/resource_github_repository_collaborators.go b/github/resource_github_repository_collaborators.go index bf05cdddc..cc04f6fc6 100644 --- a/github/resource_github_repository_collaborators.go +++ b/github/resource_github_repository_collaborators.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "slices" "sort" "strconv" @@ -136,6 +137,7 @@ func flattenUserCollaborators(objs []userCollaborator, invites []invitedCollabor type teamCollaborator struct { permission string + teamID int64 teamSlug string } @@ -143,30 +145,38 @@ func (c teamCollaborator) Empty() bool { return c == teamCollaborator{} } -func flattenTeamCollaborator(obj teamCollaborator) interface{} { +func flattenTeamCollaborator(obj teamCollaborator, teamIDs []int64) interface{} { if obj.Empty() { return nil } + + var teamIDString string + if slices.Contains(teamIDs, obj.teamID) { + teamIDString = strconv.FormatInt(obj.teamID, 10) + } else { + teamIDString = obj.teamSlug + } + transformed := map[string]interface{}{ "permission": obj.permission, - "team_id": obj.teamSlug, + "team_id": teamIDString, } return transformed } -func flattenTeamCollaborators(objs []teamCollaborator) []interface{} { +func flattenTeamCollaborators(objs []teamCollaborator, teamIDs []int64) []interface{} { if objs == nil { return nil } sort.SliceStable(objs, func(i, j int) bool { - return objs[i].teamSlug < objs[j].teamSlug + return objs[i].teamID < objs[j].teamID }) items := make([]interface{}, len(objs)) for i, obj := range objs { - items[i] = flattenTeamCollaborator(obj) + items[i] = flattenTeamCollaborator(obj, teamIDs) } return items @@ -248,7 +258,7 @@ func listTeams(client *github.Client, isOrg bool, ctx context.Context, owner, re for _, t := range repoTeams { permissionName := getPermission(t.GetPermission()) - teamCollaborators = append(teamCollaborators, teamCollaborator{permissionName, t.GetSlug()}) + teamCollaborators = append(teamCollaborators, teamCollaborator{permissionName, t.GetID(), t.GetSlug()}) } if resp.NextPage == 0 { @@ -376,33 +386,31 @@ func matchUserCollaboratorsAndInvites( func matchTeamCollaborators( repoName string, want []interface{}, has []teamCollaborator, meta interface{}) error { client := meta.(*Owner).v3client + orgID := meta.(*Owner).id owner := meta.(*Owner).name ctx := context.Background() + remove := make([]teamCollaborator, 0) for _, hasTeam := range has { var wantPerm string for _, w := range want { teamData := w.(map[string]interface{}) teamIDString := teamData["team_id"].(string) - teamSlug, err := getTeamSlug(teamIDString, meta) + teamID, err := getTeamID(teamIDString, meta) if err != nil { return err } - if teamSlug == hasTeam.teamSlug { + if teamID == hasTeam.teamID { wantPerm = teamData["permission"].(string) break } } if wantPerm == "" { // user should NOT have permission - log.Printf("[DEBUG] Removing team %s from repo: %s.", hasTeam.teamSlug, repoName) - _, err := client.Teams.RemoveTeamRepoBySlug(ctx, owner, hasTeam.teamSlug, owner, repoName) - if err != nil { - return err - } + remove = append(remove, hasTeam) } else if wantPerm != hasTeam.permission { // permission should be updated - log.Printf("[DEBUG] Updating team %s permission from %s to %s for repo: %s.", hasTeam.teamSlug, hasTeam.permission, wantPerm, repoName) - _, err := client.Teams.AddTeamRepoBySlug( - ctx, owner, hasTeam.teamSlug, owner, repoName, &github.TeamAddTeamRepoOptions{ + log.Printf("[DEBUG] Updating team %d permission from %s to %s for repo: %s.", hasTeam.teamID, hasTeam.permission, wantPerm, repoName) + _, err := client.Teams.AddTeamRepoByID( + ctx, orgID, hasTeam.teamID, owner, repoName, &github.TeamAddTeamRepoOptions{ Permission: wantPerm, }, ) @@ -415,14 +423,13 @@ func matchTeamCollaborators( for _, t := range want { teamData := t.(map[string]interface{}) teamIDString := teamData["team_id"].(string) - teamSlug, err := getTeamSlug(teamIDString, meta) + teamID, err := getTeamID(teamIDString, meta) if err != nil { return err } - permission := teamData["permission"].(string) var found bool for _, c := range has { - if teamSlug == c.teamSlug { + if teamID == c.teamID { found = true break } @@ -430,10 +437,11 @@ func matchTeamCollaborators( if found { continue } + permission := teamData["permission"].(string) // team needs to be added - log.Printf("[DEBUG] Adding team %s with permission %s for repo: %s.", teamSlug, permission, repoName) - _, err = client.Teams.AddTeamRepoBySlug( - ctx, owner, teamSlug, owner, repoName, &github.TeamAddTeamRepoOptions{ + log.Printf("[DEBUG] Adding team %s with permission %s for repo: %s.", teamIDString, permission, repoName) + _, err = client.Teams.AddTeamRepoByID( + ctx, orgID, teamID, owner, repoName, &github.TeamAddTeamRepoOptions{ Permission: permission, }, ) @@ -441,6 +449,15 @@ func matchTeamCollaborators( return err } } + + for _, team := range remove { + log.Printf("[DEBUG] Removing team %d from repo: %s.", team.teamID, repoName) + _, err := client.Teams.RemoveTeamRepoByID(ctx, orgID, team.teamID, owner, repoName) + if err != nil { + return err + } + } + return nil } @@ -454,6 +471,14 @@ func resourceGithubRepositoryCollaboratorsCreate(d *schema.ResourceData, meta in repoName := d.Get("repository").(string) ctx := context.Background() + teamsMap := make(map[string]struct{}) + for _, team := range teams { + teamIDString := team.(map[string]interface{})["team_id"].(string) + if _, found := teamsMap[teamIDString]; found { + return fmt.Errorf("duplicate set member: %s", teamIDString) + } + teamsMap[teamIDString] = struct{}{} + } usersMap := make(map[string]struct{}) for _, user := range users { username := user.(map[string]interface{})["username"].(string) @@ -462,26 +487,18 @@ func resourceGithubRepositoryCollaboratorsCreate(d *schema.ResourceData, meta in } usersMap[username] = struct{}{} } - teamsMap := make(map[string]struct{}) - for _, team := range teams { - teamID := team.(map[string]interface{})["team_id"].(string) - if _, found := teamsMap[teamID]; found { - return fmt.Errorf("duplicate set member: %s", teamID) - } - teamsMap[teamID] = struct{}{} - } userCollaborators, invitations, teamCollaborators, err := listAllCollaborators(client, isOrg, ctx, owner, repoName) if err != nil { return deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "repository collaborators (%s/%s)", owner, repoName) } - err = matchUserCollaboratorsAndInvites(repoName, users, userCollaborators, invitations, meta) + err = matchTeamCollaborators(repoName, teams, teamCollaborators, meta) if err != nil { return err } - err = matchTeamCollaborators(repoName, teams, teamCollaborators, meta) + err = matchUserCollaboratorsAndInvites(repoName, users, userCollaborators, invitations, meta) if err != nil { return err } @@ -509,6 +526,11 @@ func resourceGithubRepositoryCollaboratorsRead(d *schema.ResourceData, meta inte invitationIds[i.username] = strconv.FormatInt(i.invitationID, 10) } + teamIDs := make([]int64, len(teamCollaborators)) + for i, t := range teamCollaborators { + teamIDs[i] = t.teamID + } + err = d.Set("repository", repoName) if err != nil { return err @@ -517,7 +539,7 @@ func resourceGithubRepositoryCollaboratorsRead(d *schema.ResourceData, meta inte if err != nil { return err } - err = d.Set("team", flattenTeamCollaborators(teamCollaborators)) + err = d.Set("team", flattenTeamCollaborators(teamCollaborators, teamIDs)) if err != nil { return err } diff --git a/github/resource_github_repository_collaborators_test.go b/github/resource_github_repository_collaborators_test.go index 3484ead3a..1fa17ae5e 100644 --- a/github/resource_github_repository_collaborators_test.go +++ b/github/resource_github_repository_collaborators_test.go @@ -64,7 +64,7 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { } resource "github_team" "test" { - name = "%[1]s" + name = "test" } resource "github_repository_collaborators" "test_repo_collaborators" { @@ -75,7 +75,7 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { permission = "admin" } team { - team_id = github_team.test.slug + team_id = github_team.test.id permission = "pull" } } @@ -155,8 +155,8 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".permission") && val != "admin" { return fmt.Errorf("expected user.*.permission to be set to admin, was %s", val) } - if strings.HasPrefix(name, "team.") && strings.HasSuffix(name, ".team_id") && val != teamAttrs["slug"] { - return fmt.Errorf("expected team.*.team_id to be set to %s, was %s", teamAttrs["slug"], val) + if strings.HasPrefix(name, "team.") && strings.HasSuffix(name, ".team_id") && val != teamAttrs["id"] { + return fmt.Errorf("expected team.*.team_id to be set to %s, was %s", teamAttrs["id"], val) } if strings.HasPrefix(name, "team.") && strings.HasSuffix(name, ".permission") && val != "pull" { return fmt.Errorf("expected team.*.permission to be set to pull, was %s", val) @@ -238,7 +238,11 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { } resource "github_team" "test" { - name = "%[1]s" + name = "test" + } + + resource "github_team" "test2" { + name = "test2" } resource "github_repository_collaborators" "test_repo_collaborators" { @@ -253,7 +257,11 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { permission = "admin" } team { - team_id = github_team.test.slug + team_id = github_team.test.id + permission = "pull" + } + team { + team_id = github_team.test2.id permission = "pull" } } @@ -267,7 +275,11 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { } resource "github_team" "test" { - name = "%[1]s" + name = "test" + } + + resource "github_team" "test2" { + name = "test2" } resource "github_repository_collaborators" "test_repo_collaborators" { @@ -278,7 +290,7 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { permission = "push" } team { - team_id = github_team.test.slug + team_id = github_team.test.id permission = "push" } } @@ -361,8 +373,8 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".permission") && val != "push" { return fmt.Errorf("expected user.*.permission to be set to push, was %s", val) } - if strings.HasPrefix(name, "team.") && strings.HasSuffix(name, ".team_id") && val != teamAttrs["slug"] { - return fmt.Errorf("expected team.*.team_id to be set to %s, was %s", teamAttrs["slug"], val) + if strings.HasPrefix(name, "team.") && strings.HasSuffix(name, ".team_id") && val != teamAttrs["id"] { + return fmt.Errorf("expected team.*.team_id to be set to %s, was %s", teamAttrs["id"], val) } if strings.HasPrefix(name, "team.") && strings.HasSuffix(name, ".permission") && val != "push" { return fmt.Errorf("expected team.*.permission to be set to push, was %s", val) @@ -436,7 +448,7 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { } resource "github_team" "test" { - name = "%[1]s" + name = "test" } resource "github_repository_collaborators" "test_repo_collaborators" { @@ -451,7 +463,7 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { permission = "admin" } team { - team_id = github_team.test.slug + team_id = github_team.test.id permission = "pull" } } @@ -465,9 +477,9 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { } resource "github_team" "test" { - name = "%[1]s" + name = "test" } - `, repoName, inOrgUser) + `, repoName) testCase := func(t *testing.T, mode, config, configUpdate string, testCheck func(state *terraform.State) error) { resource.Test(t, resource.TestCase{