fix: Refactored github_repository_collaborators for team id

Signed-off-by: Steve Hipwell <[email protected]>

github/resource_github_repository_collaborators.go

@@ -4,6 +4,7 @@ import (
  "context"
  "fmt"
  "log"
+ "slices"
  "sort"
  "strconv"
 
@@ -136,37 +137,46 @@ func flattenUserCollaborators(objs []userCollaborator, invites []invitedCollabor
 
 type teamCollaborator struct {
  permission string
+ teamID int64
  teamSlug string
 }
 
 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,32 +423,41 @@ 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
  }
  }
  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,
  },
  )
  if err != nil {
  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
  }

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{