From cce7ad0f0786a902ccbf8924e7c309fca780f65f Mon Sep 17 00:00:00 2001 From: Steve Hipwell Date: Thu, 26 Feb 2026 23:01:27 +0000 Subject: [PATCH] feat: Refactor repository collaborators Signed-off-by: Steve Hipwell --- github/resource_github_emu_group_mapping.go | 10 +- ...urce_github_emu_group_mapping_migration.go | 5 +- ...esource_github_repository_collaborators.go | 960 ++++++++++-------- ...thub_repository_collaborators_migration.go | 109 ++ ...repository_collaborators_migration_test.go | 40 + ...ce_github_repository_collaborators_test.go | 826 +++++++-------- github/resource_github_team.go | 28 +- github/resource_github_team_members.go | 86 +- github/resource_github_team_members_test.go | 2 +- github/resource_github_team_membership.go | 73 +- .../resource_github_team_membership_test.go | 6 +- github/resource_github_team_repository.go | 95 +- github/util.go | 69 -- github/util_diff.go | 2 +- github/util_team.go | 152 +++ github/util_user.go | 56 + .../r/repository_collaborators.html.markdown | 30 +- 17 files changed, 1446 insertions(+), 1103 deletions(-) create mode 100644 github/resource_github_repository_collaborators_migration.go create mode 100644 github/resource_github_repository_collaborators_migration_test.go create mode 100644 github/util_team.go create mode 100644 github/util_user.go diff --git a/github/resource_github_emu_group_mapping.go b/github/resource_github_emu_group_mapping.go index e7504770dd..76de1d3cb9 100644 --- a/github/resource_github_emu_group_mapping.go +++ b/github/resource_github_emu_group_mapping.go @@ -89,7 +89,7 @@ func resourceGithubEMUGroupMappingCreate(ctx context.Context, d *schema.Resource tflog.Debug(ctx, "Successfully updated connected external group") - teamID, err := lookupTeamID(ctx, meta.(*Owner), teamSlug) + teamID, err := lookupTeamID(ctx, client, orgName, teamSlug) if err != nil { return diag.FromErr(err) } @@ -306,7 +306,11 @@ func resourceGithubEMUGroupMappingDelete(ctx context.Context, d *schema.Resource return nil } -func resourceGithubEMUGroupMappingImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { +func resourceGithubEMUGroupMappingImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) { + meta := m.(*Owner) + client := meta.v3client + orgName := meta.name + importID := d.Id() tflog.Trace(ctx, "Importing EMU group mapping with two-part ID", map[string]any{ "import_id": importID, @@ -329,7 +333,7 @@ func resourceGithubEMUGroupMappingImport(ctx context.Context, d *schema.Resource "team_slug": teamSlug, }) - teamID, err := lookupTeamID(ctx, meta.(*Owner), teamSlug) + teamID, err := lookupTeamID(ctx, client, orgName, teamSlug) if err != nil { return nil, err } diff --git a/github/resource_github_emu_group_mapping_migration.go b/github/resource_github_emu_group_mapping_migration.go index f6aa87fb47..b55828d00c 100644 --- a/github/resource_github_emu_group_mapping_migration.go +++ b/github/resource_github_emu_group_mapping_migration.go @@ -31,11 +31,10 @@ func resourceGithubEMUGroupMappingV0() *schema.Resource { } func resourceGithubEMUGroupMappingStateUpgradeV0(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) { + client := meta.(*Owner).v3client orgName := meta.(*Owner).name tflog.Trace(ctx, "GitHub EMU Group Mapping State before migration", map[string]any{"state": rawState, "owner": orgName}) - client := meta.(*Owner).v3client - teamSlug := rawState["team_slug"].(string) // We need to bypass the etag because we need to get the latest group ctx = context.WithValue(ctx, ctxEtag, nil) @@ -52,7 +51,7 @@ func resourceGithubEMUGroupMappingStateUpgradeV0(ctx context.Context, rawState m } group := groupsList.Groups[0] - teamID, err := lookupTeamID(ctx, meta.(*Owner), teamSlug) + teamID, err := lookupTeamID(ctx, client, orgName, teamSlug) if err != nil { return nil, err } diff --git a/github/resource_github_repository_collaborators.go b/github/resource_github_repository_collaborators.go index 4a8eb80219..d8b7361d0c 100644 --- a/github/resource_github_repository_collaborators.go +++ b/github/resource_github_repository_collaborators.go @@ -2,32 +2,49 @@ package github import ( "context" + "errors" "fmt" - "log" "slices" - "sort" "strconv" + "strings" "github.com/google/go-github/v84/github" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) func resourceGithubRepositoryCollaborators() *schema.Resource { return &schema.Resource{ - Create: resourceGithubRepositoryCollaboratorsCreate, - Read: resourceGithubRepositoryCollaboratorsRead, - Update: resourceGithubRepositoryCollaboratorsUpdate, - Delete: resourceGithubRepositoryCollaboratorsDelete, + SchemaVersion: 1, + StateUpgraders: []schema.StateUpgrader{ + { + Type: resourceGithubRepositoryCollaboratorsV0().CoreConfigSchema().ImpliedType(), + Upgrade: resourceGithubRepositoryCollaboratorsStateUpgradeV0, + Version: 0, + }, + }, + + CreateContext: resourceGithubRepositoryCollaboratorsCreate, + ReadContext: resourceGithubRepositoryCollaboratorsRead, + UpdateContext: resourceGithubRepositoryCollaboratorsUpdate, + DeleteContext: resourceGithubRepositoryCollaboratorsDelete, Importer: &schema.ResourceImporter{ - StateContext: schema.ImportStatePassthroughContext, + StateContext: resourceGithubRepositoryCollaboratorsImport, }, Schema: map[string]*schema.Schema{ "repository": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + Description: "Name of the repository.", + }, + "repository_id": { + Type: schema.TypeInt, + Computed: true, + Description: "ID of the repository.", }, "user": { Type: schema.TypeSet, @@ -35,17 +52,17 @@ func resourceGithubRepositoryCollaborators() *schema.Resource { Description: "List of users.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "permission": { - Type: schema.TypeString, - Optional: true, - Default: "push", - }, "username": { Type: schema.TypeString, Description: "(Required) The user to add to the repository as a collaborator.", Required: true, DiffSuppressFunc: caseInsensitive(), }, + "permission": { + Type: schema.TypeString, + Optional: true, + Default: "push", + }, }, }, }, @@ -55,16 +72,16 @@ func resourceGithubRepositoryCollaborators() *schema.Resource { Description: "List of teams.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "permission": { - Type: schema.TypeString, - Optional: true, - Default: "push", - }, "team_id": { Type: schema.TypeString, Description: "Team ID or slug to add to the repository as a collaborator.", Required: true, }, + "permission": { + Type: schema.TypeString, + Optional: true, + Default: "push", + }, }, }, }, @@ -93,132 +110,415 @@ func resourceGithubRepositoryCollaborators() *schema.Resource { }, CustomizeDiff: customdiff.Sequence( - // If there was a new user added to the list of collaborators, - // it's possible a new invitation id will be created in GitHub. - customdiff.ComputedIf("invitation_ids", func(ctx context.Context, d *schema.ResourceDiff, meta any) bool { - return d.HasChange("user") - }), + diffRepository, + resourceGithubRepositoryCollaboratorsDiff, ), } } -type userCollaborator struct { - permission string - username string -} +func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.ResourceDiff, m any) error { + if d.HasChange("user") { + users := d.Get("user").(*schema.Set).List() + seen := make(map[string]any) + + for _, u := range users { + user := u.(map[string]any) + username := user["username"].(string) + + if _, ok := seen[username]; ok { + return fmt.Errorf("duplicate username %s found in user collaborators", username) + } + seen[username] = nil + } + } + + if d.HasChange("team") && d.NewValueKnown("team") { + v, diags := d.GetRawConfigAt(cty.GetAttrPath("team")) + if diags.HasError() { + return fmt.Errorf("error reading team config: %v", diags) + } + + if !v.IsNull() && v.IsKnown() { + seen := make(map[string]any) + it := v.ElementIterator() + for it.Next() { + _, elem := it.Element() + val := elem.GetAttr("team_id") + if val.IsNull() || !val.IsKnown() { + continue + } + + teamID := val.AsString() + if _, ok := seen[teamID]; ok { + return fmt.Errorf("duplicate team %s found in team collaborators", teamID) + } + seen[teamID] = nil + } + } + } + + if len(d.Id()) == 0 { + return nil + } + + if d.HasChange("user") { + if err := d.SetNewComputed("invitation_ids"); err != nil { + return fmt.Errorf("error setting invitation_ids to computed: %w", err) + } + } -func (c userCollaborator) Empty() bool { - return c == userCollaborator{} + return nil } -type invitedCollaborator struct { - userCollaborator - invitationID int64 +func resourceGithubRepositoryCollaboratorsCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta, _ := m.(*Owner) + client := meta.v3client + owner := meta.name + isOrg := meta.IsOrganization + + repoName := d.Get("repository").(string) + users := d.Get("user").(*schema.Set).List() + teams := d.Get("team").(*schema.Set).List() + ignoreTeams := d.Get("ignore_team").(*schema.Set).List() + + inUsers, err := getUserCollaborators(users) + if err != nil { + return diag.FromErr(err) + } + + inTeams, err := getTeamCollaborators(teams) + if err != nil { + return diag.FromErr(err) + } + + inIgnoreTeams, err := getTeamIdentities(ignoreTeams) + if err != nil { + return diag.FromErr(err) + } + + invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers) + if err != nil { + return diag.FromErr(err) + } + + if isOrg { + err = updateTeamCollaborators(ctx, client, meta.id, owner, repoName, inTeams, inIgnoreTeams) + if err != nil { + return diag.FromErr(err) + } + } + + repo, _, err := client.Repositories.Get(ctx, owner, repoName) + if err != nil { + return diag.FromErr(err) + } + repoID := int(repo.GetID()) + + d.SetId(strconv.FormatInt(repo.GetID(), 10)) + if err := d.Set("repository_id", repoID); err != nil { + return diag.FromErr(err) + } + if err = d.Set("invitation_ids", invitations.flattenInvitations()); err != nil { + return diag.FromErr(err) + } + + return nil } -func flattenUserCollaborator(obj userCollaborator) any { - if obj.Empty() { - return nil +func resourceGithubRepositoryCollaboratorsRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta, _ := m.(*Owner) + client := meta.v3client + owner := meta.name + isOrg := meta.IsOrganization + + repoName := d.Get("repository").(string) + teams := d.Get("team").(*schema.Set).List() + ignoreTeams := d.Get("ignore_team").(*schema.Set).List() + + inTeams, err := getTeamCollaborators(teams) + if err != nil { + return diag.FromErr(err) + } + + inIgnoreTeams, err := getTeamIdentities(ignoreTeams) + if err != nil { + return diag.FromErr(err) + } + + ghUsers, err := listUserCollaborators(ctx, client, owner, repoName) + if err != nil { + if err, ok := errors.AsType[*github.ErrorResponse](err); ok && err.Response.StatusCode == 404 { + tflog.Debug(ctx, fmt.Sprintf("Repository %s not found when listing users, removing from state.", repoName)) + d.SetId("") + return nil + } + return diag.FromErr(err) + } + + if err := d.Set("user", ghUsers.flatten()); err != nil { + return diag.FromErr(err) + } + + if isOrg { + ghTeams, err := listTeamCollaborators(ctx, client, owner, repoName, inTeams, inIgnoreTeams) + if err != nil { + if err, ok := errors.AsType[*github.ErrorResponse](err); ok && err.Response.StatusCode == 404 { + tflog.Debug(ctx, fmt.Sprintf("Repository %s not found when listing teams, removing from state.", repoName)) + d.SetId("") + return nil + } + return diag.FromErr(err) + } + + if err := d.Set("team", ghTeams.flatten()); err != nil { + return diag.FromErr(err) + } } - transformed := map[string]any{ - "permission": obj.permission, - "username": obj.username, + + ghInvitations, err := listInvitations(ctx, client, owner, repoName) + if err != nil { + return diag.FromErr(err) + } + + if err = d.Set("invitation_ids", ghInvitations.flattenInvitations()); err != nil { + return diag.FromErr(err) } - return transformed + return nil } -func flattenUserCollaborators(objs []userCollaborator, invites []invitedCollaborator) []any { - if objs == nil && invites == nil { - return nil +func resourceGithubRepositoryCollaboratorsUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta, _ := m.(*Owner) + client := meta.v3client + owner := meta.name + isOrg := meta.IsOrganization + + repoName := d.Get("repository").(string) + users := d.Get("user").(*schema.Set).List() + teams := d.Get("team").(*schema.Set).List() + ignoreTeams := d.Get("ignore_team").(*schema.Set).List() + + inUsers, err := getUserCollaborators(users) + if err != nil { + return diag.FromErr(err) + } + + inTeams, err := getTeamCollaborators(teams) + if err != nil { + return diag.FromErr(err) } - for _, invite := range invites { - objs = append(objs, invite.userCollaborator) + inIgnoreTeams, err := getTeamIdentities(ignoreTeams) + if err != nil { + return diag.FromErr(err) } - sort.SliceStable(objs, func(i, j int) bool { - return objs[i].username < objs[j].username - }) + invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers) + if err != nil { + return diag.FromErr(err) + } + + if isOrg { + err := updateTeamCollaborators(ctx, client, meta.id, owner, repoName, inTeams, inIgnoreTeams) + if err != nil { + return diag.FromErr(err) + } + } - items := make([]any, len(objs)) - for i, obj := range objs { - items[i] = flattenUserCollaborator(obj) + if err = d.Set("invitation_ids", invitations.flattenInvitations()); err != nil { + return diag.FromErr(err) } - return items + return nil } -type teamCollaborator struct { - permission string - teamID int64 - teamSlug string +func resourceGithubRepositoryCollaboratorsDelete(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta, _ := m.(*Owner) + client := meta.v3client + owner := meta.name + isOrg := meta.IsOrganization + + repoName := d.Get("repository").(string) + ignoreTeams := d.Get("ignore_team").(*schema.Set).List() + + inIgnoreTeams, err := getTeamIdentities(ignoreTeams) + if err != nil { + return diag.FromErr(err) + } + + tflog.Debug(ctx, fmt.Sprintf("Removing all collaborators from repository %s.", repoName)) + + _, err = updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, nil) + if err != nil { + return diag.FromErr(err) + } + + if isOrg { + err = updateTeamCollaborators(ctx, client, meta.id, owner, repoName, nil, inIgnoreTeams) + if err != nil { + return diag.FromErr(err) + } + } + + return nil } -func (c teamCollaborator) Empty() bool { - return c == teamCollaborator{} +func resourceGithubRepositoryCollaboratorsImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) { + meta := m.(*Owner) + client := meta.v3client + owner := meta.name + + repoName := d.Id() + + repo, _, err := client.Repositories.Get(ctx, owner, repoName) + if err != nil { + return nil, err + } + repoID := int(repo.GetID()) + + d.SetId(strconv.FormatInt(repo.GetID(), 10)) + if err := d.Set("repository", repoName); err != nil { + return nil, err + } + if err := d.Set("repository_id", repoID); err != nil { + return nil, err + } + + return []*schema.ResourceData{d}, nil } -func flattenTeamCollaborator(obj teamCollaborator, teamSlugs []string) any { - if obj.Empty() { - return nil +// getUserCollaborators converts a slice of any type to a slice of userCollaborator. +func getUserCollaborators(col []any) (userCollaborators, error) { + collaborators := make([]userCollaborator, len(col)) + + for i, u := range col { + m, ok := u.(map[string]any) + if !ok { + return nil, fmt.Errorf("input invalid") + } + + n, ok := m["username"] + if !ok { + return nil, fmt.Errorf("username missing") + } + + username, ok := n.(string) + if !ok || len(username) == 0 { + return nil, fmt.Errorf("username invalid") + } + + p, ok := m["permission"] + if !ok { + return nil, fmt.Errorf("permission missing") + } + + permission, ok := p.(string) + if !ok || len(permission) == 0 { + return nil, fmt.Errorf("permission invalid") + } + + uc := userCollaborator{ + userIdentity: userIdentity{ + login: username, + }, + permission: permission, + } + + collaborators[i] = uc } - var teamIDString string - if slices.Contains(teamSlugs, obj.teamSlug) { - teamIDString = obj.teamSlug - } else { - teamIDString = strconv.FormatInt(obj.teamID, 10) + return collaborators, nil +} + +// getTeamCollaborators returns a list of team collaborators represented by the input. +func getTeamCollaborators(col []any) (teamCollaborators, error) { + collaborators := make([]teamCollaborator, len(col)) + + for i, t := range col { + m, ok := t.(map[string]any) + if !ok { + return nil, fmt.Errorf("input invalid") + } + + id, err := getTeamIdentity(m) + if err != nil { + return nil, err + } + + permission, ok := m["permission"].(string) + if !ok || len(permission) == 0 { + return nil, fmt.Errorf("team input must include 'permission'") + } + + collaborators[i] = teamCollaborator{ + teamIdentity: id, + permission: permission, + } } - transformed := map[string]any{ - "permission": obj.permission, - "team_id": teamIDString, + return collaborators, nil +} + +// getTeamIdentities returns a list of team identities represented by the input. +func getTeamIdentities(col []any) ([]teamIdentity, error) { + identities := make([]teamIdentity, len(col)) + + for i, t := range col { + id, err := getTeamIdentity(t) + if err != nil { + return nil, err + } + identities[i] = id } - return transformed + return identities, nil } -func flattenTeamCollaborators(objs []teamCollaborator, teamSlugs []string) []any { - if objs == nil { - return nil +// getTeamIdentity returns a team identity represented by the input. +func getTeamIdentity(d any) (teamIdentity, error) { + m, ok := d.(map[string]any) + if !ok { + return teamIdentity{}, fmt.Errorf("team input invalid") } - sort.SliceStable(objs, func(i, j int) bool { - return objs[i].teamID < objs[j].teamID - }) + o, ok := m["team_id"] + if !ok { + return teamIdentity{}, fmt.Errorf("team input must include 'team_id'") + } - items := make([]any, len(objs)) - for i, obj := range objs { - items[i] = flattenTeamCollaborator(obj, teamSlugs) + id, ok := o.(string) + if !ok || len(id) == 0 { + return teamIdentity{}, fmt.Errorf("team_id must be a non-empty string") } - return items + return newLegacyTeamIdentity(id), nil } -func listUserCollaborators(client *github.Client, isOrg bool, ctx context.Context, owner, repoName string) ([]userCollaborator, error) { - userCollaborators := make([]userCollaborator, 0) +func listUserCollaborators(ctx context.Context, client *github.Client, owner, repoName string) (userCollaborators, error) { + col := make([]userCollaborator, 0) + affiliations := []string{"direct", "outside"} for _, affiliation := range affiliations { - opt := &github.ListCollaboratorsOptions{ListOptions: github.ListOptions{ - PerPage: maxPerPage, - }, Affiliation: affiliation} + opt := &github.ListCollaboratorsOptions{ + ListOptions: github.ListOptions{ + PerPage: maxPerPage, + }, + Affiliation: affiliation, + } for { - collaborators, resp, err := client.Repositories.ListCollaborators(ctx, - owner, repoName, opt) + collaborators, resp, err := client.Repositories.ListCollaborators(ctx, owner, repoName, opt) if err != nil { return nil, err } for _, c := range collaborators { - // owners are listed in the collaborators list even though they don't have direct permissions - if !isOrg && c.GetLogin() == owner { - continue - } - permissionName := getPermission(c.GetRoleName()) - - userCollaborators = append(userCollaborators, userCollaborator{permissionName, c.GetLogin()}) + col = append(col, userCollaborator{ + userIdentity: userIdentity{ + login: c.GetLogin(), + }, + permission: getPermission(c.GetRoleName()), + }) } if resp.NextPage == 0 { @@ -227,11 +527,11 @@ func listUserCollaborators(client *github.Client, isOrg bool, ctx context.Contex opt.Page = resp.NextPage } } - return userCollaborators, nil + return col, nil } -func listInvitations(client *github.Client, ctx context.Context, owner, repoName string) ([]invitedCollaborator, error) { - invitedCollaborators := make([]invitedCollaborator, 0) +func listInvitations(ctx context.Context, client *github.Client, owner, repoName string) (userCollaborators, error) { + col := make([]userCollaborator, 0) opt := &github.ListOptions{PerPage: maxPerPage} for { @@ -241,10 +541,14 @@ func listInvitations(client *github.Client, ctx context.Context, owner, repoName } for _, i := range invitations { - permissionName := getPermission(i.GetPermissions()) + id := i.GetID() - invitedCollaborators = append(invitedCollaborators, invitedCollaborator{ - userCollaborator{permissionName, i.GetInvitee().GetLogin()}, i.GetID(), + col = append(col, userCollaborator{ + userIdentity: userIdentity{ + login: i.GetInvitee().GetLogin(), + }, + permission: getPermission(i.GetPermissions()), + invitationID: &id, }) } @@ -253,29 +557,62 @@ func listInvitations(client *github.Client, ctx context.Context, owner, repoName } opt.Page = resp.NextPage } - return invitedCollaborators, nil + + return col, nil } -func listTeams(client *github.Client, isOrg bool, ctx context.Context, owner, repoName string, ignoreTeamIds []int64) ([]teamCollaborator, error) { - allTeams := make([]teamCollaborator, 0) +func listTeamCollaborators(ctx context.Context, client *github.Client, orgName, repoName string, inTeams teamCollaborators, ignoreTeams []teamIdentity) (teamCollaborators, error) { + lookup := make(map[string]teamCollaborator) + ignore := len(ignoreTeams) > 0 + col := make([]teamCollaborator, 0) - if !isOrg { - return allTeams, nil + for _, inTeam := range inTeams { + lookup[inTeam.getTeamID()] = inTeam + } + + opt := &github.ListOptions{ + PerPage: maxPerPage, } - opt := &github.ListOptions{PerPage: maxPerPage} for { - repoTeams, resp, err := client.Repositories.ListTeams(ctx, owner, repoName, opt) + repoTeams, resp, err := client.Repositories.ListTeams(ctx, orgName, repoName, opt) if err != nil { return nil, err } for _, t := range repoTeams { - if slices.Contains(ignoreTeamIds, t.GetID()) { + slug := t.GetSlug() + id := t.GetID() + if ignore && slices.ContainsFunc(ignoreTeams, func(ignore teamIdentity) bool { + if s, ok := ignore.getSlugOK(); ok { + return slug == s + } else { + return id == ignore.getID() + } + }) { continue } - allTeams = append(allTeams, teamCollaborator{permission: getPermission(t.GetPermission()), teamID: t.GetID(), teamSlug: t.GetSlug()}) + var teamID *string + if _, ok := lookup[slug]; ok { + teamID = &slug + } + + if teamID == nil { + idStr := strconv.FormatInt(id, 10) + if _, ok := lookup[idStr]; ok { + teamID = &idStr + } + } + + col = append(col, teamCollaborator{ + teamIdentity: teamIdentity{ + id: &id, + slug: &slug, + teamID: teamID, + }, + permission: getPermission(t.GetPermission()), + }) } if resp.NextPage == 0 { @@ -284,359 +621,160 @@ func listTeams(client *github.Client, isOrg bool, ctx context.Context, owner, re opt.Page = resp.NextPage } - return allTeams, nil + return col, nil } -func listAllCollaborators(client *github.Client, isOrg bool, ctx context.Context, owner, repoName string, ignoreTeamIds []int64) ([]userCollaborator, []invitedCollaborator, []teamCollaborator, error) { - userCollaborators, err := listUserCollaborators(client, isOrg, ctx, owner, repoName) - if err != nil { - return nil, nil, nil, err - } - invitations, err := listInvitations(client, ctx, owner, repoName) - if err != nil { - return nil, nil, nil, err +func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Client, owner, repoName string, inUsers userCollaborators) (userCollaborators, error) { + lookup := make(map[string]userCollaborator) + seen := make(map[string]any) + remove := make([]string, 0) + + for _, inUser := range inUsers { + lookup[inUser.login] = inUser } - teamCollaborators, err := listTeams(client, isOrg, ctx, owner, repoName, ignoreTeamIds) + + ghUsers, err := listUserCollaborators(ctx, client, owner, repoName) if err != nil { - return nil, nil, nil, err + return nil, err } - return userCollaborators, invitations, teamCollaborators, err -} -func matchUserCollaboratorsAndInvites(repoName string, want []any, hasUsers []userCollaborator, hasInvites []invitedCollaborator, meta any) error { - client := meta.(*Owner).v3client + for _, ghUser := range ghUsers { + inUser, ok := lookup[ghUser.login] + if ok { + seen[ghUser.login] = nil - owner := meta.(*Owner).name - ctx := context.Background() - - for _, has := range hasUsers { - var wantPermission string - for _, w := range want { - userData := w.(map[string]any) - if userData["username"] == has.username { - wantPermission = userData["permission"].(string) - break - } - } - if wantPermission == "" { // user should NOT have permission - log.Printf("[DEBUG] Removing user %s from repo: %s.", has.username, repoName) - _, err := client.Repositories.RemoveCollaborator(ctx, owner, repoName, has.username) - if err != nil { - err = handleArchivedRepoDelete(err, "repository collaborator", has.username, owner, repoName) + if ghUser.permission != inUser.permission { + tflog.Info(ctx, fmt.Sprintf("Updating user %s permission from %s to %s for repo %s.", inUser.login, ghUser.permission, inUser.permission, repoName)) + _, _, err := client.Repositories.AddCollaborator(ctx, owner, repoName, inUser.login, &github.RepositoryAddCollaboratorOptions{Permission: inUser.permission}) if err != nil { - return err + return nil, err } } - } else if wantPermission != has.permission { // permission should be updated - log.Printf("[DEBUG] Updating user %s permission from %s to %s for repo: %s.", has.username, has.permission, wantPermission, repoName) - _, _, err := client.Repositories.AddCollaborator( - ctx, owner, repoName, has.username, &github.RepositoryAddCollaboratorOptions{ - Permission: wantPermission, - }, - ) - if err != nil { - return err - } + } else { + remove = append(remove, ghUser.login) } } - for _, has := range hasInvites { - var wantPermission string - for _, u := range want { - userData := u.(map[string]any) - if userData["username"] == has.username { - wantPermission = userData["permission"].(string) - break - } - } - if wantPermission == "" { // user should NOT have permission - log.Printf("[DEBUG] Deleting invite for user %s from repo: %s.", has.username, repoName) - _, err := client.Repositories.DeleteInvitation(ctx, owner, repoName, has.invitationID) - if err != nil { - err = handleArchivedRepoDelete(err, "repository collaborator invitation", has.username, owner, repoName) + ghInvites, err := listInvitations(ctx, client, owner, repoName) + if err != nil { + return nil, err + } + + for _, ghInvite := range ghInvites { + inInvite, ok := lookup[ghInvite.login] + if ok { + seen[ghInvite.login] = nil + + if ghInvite.permission != inInvite.permission { + tflog.Info(ctx, fmt.Sprintf("Updating invite for user %s permission from %s to %s for repo %s.", inInvite.login, ghInvite.permission, inInvite.permission, repoName)) + _, _, err := client.Repositories.UpdateInvitation(ctx, owner, repoName, *ghInvite.invitationID, inInvite.permission) if err != nil { - return err + return nil, err } } - } else if wantPermission != has.permission { // permission should be updated - log.Printf("[DEBUG] Updating invite for user %s permission from %s to %s for repo: %s.", has.username, has.permission, wantPermission, repoName) - _, _, err := client.Repositories.UpdateInvitation(ctx, owner, repoName, has.invitationID, wantPermission) + } else { + tflog.Info(ctx, fmt.Sprintf("Deleting invite for user %s from repo %s.", ghInvite.login, repoName)) + _, err := client.Repositories.DeleteInvitation(ctx, owner, repoName, *ghInvite.invitationID) if err != nil { - return err + return nil, handleArchivedRepoDelete(err, "repository collaborator invitation", ghInvite.login, owner, repoName) } } } - for _, w := range want { - userData := w.(map[string]any) - username := userData["username"].(string) - permission := userData["permission"].(string) - var found bool - for _, has := range hasUsers { - if username == has.username { - found = true - break - } - } - if found { - continue - } - for _, has := range hasInvites { - if username == has.username { - found = true - break - } - } - if found { + for _, inUser := range inUsers { + if _, ok := seen[inUser.login]; ok { continue } - // user needs to be added - log.Printf("[DEBUG] Inviting user %s with permission %s for repo: %s.", username, permission, repoName) - _, _, err := client.Repositories.AddCollaborator( - ctx, owner, repoName, username, &github.RepositoryAddCollaboratorOptions{ - Permission: permission, - }, - ) - if err != nil { - return err - } - } - - return nil -} -func matchTeamCollaborators(repoName string, want []any, has []teamCollaborator, meta any) 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]any) - teamIDString := teamData["team_id"].(string) - teamID, err := getTeamID(teamIDString, meta) - if err != nil { - return err - } - if teamID == hasTeam.teamID { - wantPerm = teamData["permission"].(string) - break - } - } - if wantPerm == "" { // user should NOT have permission - remove = append(remove, hasTeam) - } else if wantPerm != hasTeam.permission { // permission should be updated - 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, - }, - ) - if err != nil { - return err - } + tflog.Info(ctx, fmt.Sprintf("Inviting user %s to repo %s with permission %s.", inUser.login, repoName, inUser.permission)) + inv, _, err := client.Repositories.AddCollaborator(ctx, owner, repoName, inUser.login, &github.RepositoryAddCollaboratorOptions{Permission: inUser.permission}) + if err != nil { + return nil, err } + inUser.invitationID = inv.ID + ghInvites = append(ghInvites, inUser) } - for _, t := range want { - teamData := t.(map[string]any) - teamIDString := teamData["team_id"].(string) - teamID, err := getTeamID(teamIDString, meta) - if err != nil { - return err - } - var found bool - for _, c := range has { - if teamID == c.teamID { - found = true - break - } - } - if found { + for _, l := range remove { + if strings.EqualFold(l, owner) { + tflog.Info(ctx, fmt.Sprintf("Skipping removal of user %s who is the owner of repo %s.", l, repoName)) continue } - permission := teamData["permission"].(string) - // team needs to be added - 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) + tflog.Info(ctx, fmt.Sprintf("Removing user %s from repo %s.", l, repoName)) + _, err := client.Repositories.RemoveCollaborator(ctx, owner, repoName, l) if err != nil { - err = handleArchivedRepoDelete(err, "team repository access", fmt.Sprintf("team %d", team.teamID), owner, repoName) - if err != nil { - return err - } + return nil, handleArchivedRepoDelete(err, "repository collaborator", l, owner, repoName) } } - return nil + return ghInvites, nil } -func resourceGithubRepositoryCollaboratorsCreate(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - - owner := meta.(*Owner).name - isOrg := meta.(*Owner).IsOrganization - users := d.Get("user").(*schema.Set).List() - teams := d.Get("team").(*schema.Set).List() - repoName := d.Get("repository").(string) - ctx := context.Background() - - teamsMap := make(map[string]struct{}, len(teams)) - for _, team := range teams { - teamIDString := team.(map[string]any)["team_id"].(string) - if _, found := teamsMap[teamIDString]; found { - return fmt.Errorf("duplicate set member: %s", teamIDString) - } - teamsMap[teamIDString] = struct{}{} - } - usersMap := make(map[string]struct{}, len(users)) - for _, user := range users { - username := user.(map[string]any)["username"].(string) - if _, found := usersMap[username]; found { - return fmt.Errorf("duplicate set member found: %s", username) - } - usersMap[username] = struct{}{} - } - - ignoreTeamIds, err := getIgnoreTeamIds(d, meta) - if err != nil { - return err - } - - userCollaborators, invitations, teamCollaborators, err := listAllCollaborators(client, isOrg, ctx, owner, repoName, ignoreTeamIds) - if err != nil { - return deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "repository collaborators (%s/%s)", owner, repoName) - } - - err = matchTeamCollaborators(repoName, teams, teamCollaborators, meta) - if err != nil { - return err - } +func updateTeamCollaborators(ctx context.Context, client *github.Client, orgID int64, orgName, repoName string, inTeams teamCollaborators, ignoreTeams []teamIdentity) error { + lookup := make(map[string]teamCollaborator) + seen := make(map[string]any) + remove := make([]string, 0) - err = matchUserCollaboratorsAndInvites(repoName, users, userCollaborators, invitations, meta) - if err != nil { - return err + for _, inTeam := range inTeams { + lookup[inTeam.getTeamID()] = inTeam } - d.SetId(repoName) - - return resourceGithubRepositoryCollaboratorsRead(d, meta) -} - -func resourceGithubRepositoryCollaboratorsRead(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - - owner := meta.(*Owner).name - isOrg := meta.(*Owner).IsOrganization - repoName := d.Id() - ctx := context.WithValue(context.Background(), ctxId, d.Id()) - - ignoreTeamIds, err := getIgnoreTeamIds(d, meta) + ghTeams, err := listTeamCollaborators(ctx, client, orgName, repoName, inTeams, ignoreTeams) if err != nil { return err } - userCollaborators, invitedCollaborators, teamCollaborators, err := listAllCollaborators(client, isOrg, ctx, owner, repoName, ignoreTeamIds) - if err != nil { - return deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "repository collaborators (%s/%s)", owner, repoName) - } - - invitationIds := make(map[string]string, len(invitedCollaborators)) - for _, i := range invitedCollaborators { - invitationIds[i.username] = strconv.FormatInt(i.invitationID, 10) - } + for _, ghTeam := range ghTeams { + slug := ghTeam.getSlug() + if teamID, ok := ghTeam.getTeamIDOK(); ok { + inTeam, ok := lookup[teamID] + if !ok { + continue + } + seen[teamID] = nil - sourceTeams := d.Get("team").(*schema.Set).List() - teamSlugs := make([]string, len(sourceTeams)) - for i, t := range sourceTeams { - teamIdString := t.(map[string]any)["team_id"].(string) - _, parseIntErr := strconv.ParseInt(teamIdString, 10, 64) - if parseIntErr != nil { - teamSlugs[i] = teamIdString + if ghTeam.permission != inTeam.permission { + tflog.Info(ctx, fmt.Sprintf("Updating team %s permission from %s to %s for repo %s.", slug, ghTeam.permission, inTeam.permission, repoName)) + _, err := client.Teams.AddTeamRepoBySlug(ctx, orgName, slug, orgName, repoName, &github.TeamAddTeamRepoOptions{ + Permission: inTeam.permission, + }) + if err != nil { + return err + } + } + } else { + remove = append(remove, slug) } } - err = d.Set("repository", repoName) - if err != nil { - return err - } - err = d.Set("user", flattenUserCollaborators(userCollaborators, invitedCollaborators)) - if err != nil { - return err - } - err = d.Set("team", flattenTeamCollaborators(teamCollaborators, teamSlugs)) - if err != nil { - return err - } - err = d.Set("invitation_ids", invitationIds) - if err != nil { - return err - } - - return nil -} - -func resourceGithubRepositoryCollaboratorsUpdate(d *schema.ResourceData, meta any) error { - return resourceGithubRepositoryCollaboratorsCreate(d, meta) -} - -func resourceGithubRepositoryCollaboratorsDelete(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - - owner := meta.(*Owner).name - isOrg := meta.(*Owner).IsOrganization - repoName := d.Get("repository").(string) - ctx := context.Background() - - ignoreTeamIds, err := getIgnoreTeamIds(d, meta) - if err != nil { - return err - } - - userCollaborators, invitations, teamCollaborators, err := listAllCollaborators(client, isOrg, ctx, owner, repoName, ignoreTeamIds) - if err != nil { - return deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "repository collaborators (%s/%s)", owner, repoName) - } - - log.Printf("[DEBUG] Deleting all users, invites and collaborators for repo: %s.", repoName) + for _, inTeam := range inTeams { + teamID := inTeam.getTeamID() + if _, ok := seen[teamID]; ok { + continue + } - // delete all users - err = matchUserCollaboratorsAndInvites(repoName, nil, userCollaborators, invitations, meta) - if err != nil { - return err + tflog.Info(ctx, fmt.Sprintf("Adding team %s to repo %s with permission %s.", teamID, repoName, inTeam.permission)) + if slug, ok := inTeam.getSlugOK(); ok { + _, err := client.Teams.AddTeamRepoBySlug(ctx, orgName, slug, orgName, repoName, &github.TeamAddTeamRepoOptions{Permission: inTeam.permission}) + if err != nil { + return err + } + } else { + _, err := client.Teams.AddTeamRepoByID(ctx, orgID, inTeam.getID(), orgName, repoName, &github.TeamAddTeamRepoOptions{Permission: inTeam.permission}) + if err != nil { + return err + } + } } - // delete all teams - err = matchTeamCollaborators(repoName, nil, teamCollaborators, meta) - return err -} - -func getIgnoreTeamIds(d *schema.ResourceData, meta any) ([]int64, error) { - ignoreTeams := d.Get("ignore_team").(*schema.Set).List() - ignoreTeamIds := make([]int64, len(ignoreTeams)) - - for i, t := range ignoreTeams { - s := t.(map[string]any)["team_id"].(string) - id, err := getTeamID(s, meta) + for _, s := range remove { + tflog.Info(ctx, fmt.Sprintf("Removing team %s from repo %s.", s, repoName)) + _, err := client.Teams.RemoveTeamRepoBySlug(ctx, orgName, s, orgName, repoName) if err != nil { - return nil, err + return handleArchivedRepoDelete(err, "team repository access", fmt.Sprintf("team %s", s), orgName, repoName) } - ignoreTeamIds[i] = id } - return ignoreTeamIds, nil + return nil } diff --git a/github/resource_github_repository_collaborators_migration.go b/github/resource_github_repository_collaborators_migration.go new file mode 100644 index 0000000000..88ad8e3bff --- /dev/null +++ b/github/resource_github_repository_collaborators_migration.go @@ -0,0 +1,109 @@ +package github + +import ( + "context" + "fmt" + "log" + "strconv" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +func resourceGithubRepositoryCollaboratorsV0() *schema.Resource { + return &schema.Resource{ + SchemaVersion: 0, + + Schema: map[string]*schema.Schema{ + "repository": { + Type: schema.TypeString, + Required: true, + }, + "user": { + Type: schema.TypeSet, + Optional: true, + Description: "List of users.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "permission": { + Type: schema.TypeString, + Optional: true, + Default: "push", + }, + "username": { + Type: schema.TypeString, + Description: "(Required) The user to add to the repository as a collaborator.", + Required: true, + DiffSuppressFunc: caseInsensitive(), + }, + }, + }, + }, + "team": { + Type: schema.TypeSet, + Optional: true, + Description: "List of teams.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "permission": { + Type: schema.TypeString, + Optional: true, + Default: "push", + }, + "team_id": { + Type: schema.TypeString, + Description: "Team ID or slug to add to the repository as a collaborator.", + Required: true, + }, + }, + }, + }, + "invitation_ids": { + Type: schema.TypeMap, + Description: "Map of usernames to invitation ID for any users added", + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + Computed: true, + }, + "ignore_team": { + Type: schema.TypeSet, + Optional: true, + Description: "List of teams to ignore.", + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "team_id": { + Type: schema.TypeString, + Description: "ID or slug of the team to ignore.", + Required: true, + }, + }, + }, + }, + }, + } +} + +func resourceGithubRepositoryCollaboratorsStateUpgradeV0(ctx context.Context, rawState map[string]any, m any) (map[string]any, error) { + meta := m.(*Owner) + client := meta.v3client + owner := meta.name + + log.Printf("[DEBUG] GitHub Repository Collaborators Attributes before migration: %#v", rawState) + + repoName, ok := rawState["repository"].(string) + if !ok { + return nil, fmt.Errorf("repository not found or is not a string") + } + + repo, _, err := client.Repositories.Get(ctx, owner, repoName) + if err != nil { + return nil, fmt.Errorf("failed to retrieve repository %s: %w", repoName, err) + } + + rawState["id"] = strconv.FormatInt(repo.GetID(), 10) + rawState["repository_id"] = int(repo.GetID()) + + log.Printf("[DEBUG] GitHub Repository Collaborators Attributes after migration: %#v", rawState) + + return rawState, nil +} diff --git a/github/resource_github_repository_collaborators_migration_test.go b/github/resource_github_repository_collaborators_migration_test.go new file mode 100644 index 0000000000..23b90f3fdf --- /dev/null +++ b/github/resource_github_repository_collaborators_migration_test.go @@ -0,0 +1,40 @@ +package github + +// TODO: Enable this test once we have a pattern to create a mock client for the test. +// func Test_resourceGithubRepositoryCollaboratorsStateUpgradeV0(t *testing.T) { +// t.Parallel() + +// for _, d := range []struct { +// testName string +// rawState map[string]any +// want map[string]any +// shouldError bool +// }{ +// { +// testName: "migrates v1 to v2", +// rawState: map[string]any{ +// "id": "test-repo", +// "repository": "test-repo", +// }, +// want: map[string]any{ +// "id": "123456", +// "repository": "test-repo", +// "repository_id": "123456", +// }, +// shouldError: false, +// }, +// } { +// t.Run(d.testName, func(t *testing.T) { +// t.Parallel() + +// got, err := resourceGithubRepositoryCollaboratorsStateUpgradeV0(context.Background(), d.rawState, nil) +// if (err != nil) != d.shouldError { +// t.Fatalf("unexpected error state") +// } + +// if !d.shouldError && !reflect.DeepEqual(got, d.want) { +// t.Fatalf("got %+v, want %+v", got, d.want) +// } +// }) +// } +// } diff --git a/github/resource_github_repository_collaborators_test.go b/github/resource_github_repository_collaborators_test.go index 44a24cd339..62eb917734 100644 --- a/github/resource_github_repository_collaborators_test.go +++ b/github/resource_github_repository_collaborators_test.go @@ -2,47 +2,121 @@ package github import ( "fmt" - "strings" + "regexp" "testing" - "github.com/google/go-github/v84/github" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" - "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" ) func TestAccGithubRepositoryCollaborators(t *testing.T) { - if len(testAccConf.testExternalUser) == 0 { - t.Skip("No external user provided") + t.Run("create", func(t *testing.T) { + if len(testAccConf.testOrgUser) == 0 { + t.Skip("No organization user provided") + } + + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) + teamName0 := fmt.Sprintf("%s%s-0", testResourcePrefix, randomID) + teamName1 := fmt.Sprintf("%s%s-1", testResourcePrefix, randomID) + collaboratorUser := testAccConf.testOrgUser + + config := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} + +resource "github_team" "test_0" { + name = "%s" +} + +resource "github_team" "test_1" { + name = "%s" +} + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + user { + username = "%s" + permission = "admin" + } + + team { + team_id = github_team.test_0.id + permission = "pull" } - ctx := t.Context() - meta, err := getTestMeta() - if err != nil { - t.Fatal(err) + team { + team_id = github_team.test_1.slug } +} +`, repoName, teamName0, teamName1, collaboratorUser) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetExact([]knownvalue.Check{ + knownvalue.MapExact(map[string]knownvalue.Check{ + "username": knownvalue.StringExact(collaboratorUser), + "permission": knownvalue.StringExact("admin"), + }), + })), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetExact([]knownvalue.Check{ + knownvalue.MapExact(map[string]knownvalue.Check{ + "team_id": knownvalue.StringRegexp(regexp.MustCompile(`^\d+$`)), + "permission": knownvalue.StringExact("pull"), + }), + knownvalue.MapExact(map[string]knownvalue.Check{ + "team_id": knownvalue.StringExact(teamName1), + "permission": knownvalue.StringExact("push"), + }), + })), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, + }, + { + ResourceName: "github_repository_collaborators.test", + ImportState: true, + ImportStateId: repoName, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"user", "team", "invitation_ids"}, + }, + }, + }) + }) + + t.Run("add_external_user", func(t *testing.T) { + if len(testAccConf.testExternalUser) == 0 { + t.Skip("No external user provided") + } - t.Run("adds user collaborator", func(t *testing.T) { - conn := meta.v3client randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - repoName := fmt.Sprintf("%srepo-collabs-%s", testResourcePrefix, randomID) + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) config := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "push" - } - } - `, repoName, testAccConf.testExternalUser) +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + user { + username = "%s" + permission = "push" + } +} +`, repoName, testAccConf.testExternalUser) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, @@ -50,516 +124,340 @@ func TestAccGithubRepositoryCollaborators(t *testing.T) { Steps: []resource.TestStep{ { Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "user.#"), - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "team.#"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "user.#", "1"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "team.#", "0"), - func(state *terraform.State) error { - owner := testAccConf.owner - - collaborators := state.RootModule().Resources["github_repository_collaborators.test_repo_collaborators"].Primary - for name, val := range collaborators.Attributes { - if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".username") && val != testAccConf.testExternalUser { - return fmt.Errorf("expected user.*.username to be set to %s, was %s", testAccConf.testExternalUser, val) - } - 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) - } - } - - invites, _, err := conn.Repositories.ListInvitations(ctx, owner, repoName, nil) - if err != nil { - return err - } - if len(invites) != 1 { - return fmt.Errorf("expected an invite for %s but not found", testAccConf.testExternalUser) - } - if invites[0].GetInvitee().GetLogin() != testAccConf.testExternalUser { - return fmt.Errorf("expected an invite for %s for repo %s/%s", testAccConf.testExternalUser, owner, repoName) - } - perm := invites[0].GetPermissions() - if perm != "write" { - return fmt.Errorf("expected the invite for %s to have push perms for for %s/%s, found %s", testAccConf.testExternalUser, owner, repoName, perm) - } - return nil - }, - ), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(1)), + }, }, }, }) }) - t.Run("adds team collaborator", func(t *testing.T) { - ctx := t.Context() - conn := meta.v3client + t.Run("update_teams", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - repoName := fmt.Sprintf("%srepo-collabs-%s", testResourcePrefix, randomID) - teamName := fmt.Sprintf("%steam-collabs-%s", testResourcePrefix, randomID) - collaboratorUser := testAccConf.testOrgUser + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) + teamName0 := fmt.Sprintf("%s%s-0", testResourcePrefix, randomID) + teamName1 := fmt.Sprintf("%s%s-1", testResourcePrefix, randomID) + teamName2 := fmt.Sprintf("%s%s-2", testResourcePrefix, randomID) + + configPre := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} - config := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_team" "test" { - name = "%s" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "admin" - } - - team { - team_id = github_team.test.id - permission = "pull" - } - } - `, repoName, teamName, collaboratorUser) +resource "github_team" "test_0" { + name = "%s" +} + +resource "github_team" "test_1" { + name = "%s" +} + +resource "github_team" "test_2" { + name = "%s" +} +`, repoName, teamName0, teamName1, teamName2) + + config0 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + team { + team_id = github_team.test_0.slug + permission = "pull" + } + + team { + team_id = github_team.test_1.slug + permission = "pull" + } +} +`, configPre) + + config1 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + team { + team_id = github_team.test_1.slug + permission = "push" + } + + team { + team_id = github_team.test_2.slug + permission = "push" + } +} +`, configPre) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "user.#"), - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "team.#"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "user.#", "1"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "team.#", "1"), - func(state *terraform.State) error { - owner := testAccConf.owner - - teamAttrs := state.RootModule().Resources["github_team.test"].Primary.Attributes - collaborators := state.RootModule().Resources["github_repository_collaborators.test_repo_collaborators"].Primary - for name, val := range collaborators.Attributes { - if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".username") && val != collaboratorUser { - return fmt.Errorf("expected user.*.username to be set to %s, was %s", collaboratorUser, val) - } - 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["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) - } - } - users, _, err := conn.Repositories.ListCollaborators(ctx, owner, repoName, &github.ListCollaboratorsOptions{Affiliation: "direct"}) - if err != nil { - return err - } - if len(users) != 1 { - return fmt.Errorf("expected 1 collaborator (%s) for repo %s/%s , found %d", collaboratorUser, owner, repoName, len(users)) - } - perm := getPermission(users[0].GetRoleName()) - if perm != "admin" { - return fmt.Errorf("expected %s to have admin perms for repo %s/%s, found %s", collaboratorUser, owner, repoName, perm) - } - teams, _, err := conn.Repositories.ListTeams(ctx, owner, repoName, nil) - if err != nil { - return err - } - if len(teams) != 1 { - return fmt.Errorf("expected team %s to be a collaborator for %s/%s", repoName, owner, repoName) - } - perm = getPermission(teams[0].GetPermission()) - if perm != "pull" { - return fmt.Errorf("expected team %s to have pull perms for repo %s/%s, found %s", repoName, owner, repoName, perm) - } - return nil - }, - ), + Config: config0, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetExact([]knownvalue.Check{ + knownvalue.MapExact(map[string]knownvalue.Check{ + "team_id": knownvalue.StringExact(teamName0), + "permission": knownvalue.StringExact("pull"), + }), + knownvalue.MapExact(map[string]knownvalue.Check{ + "team_id": knownvalue.StringExact(teamName1), + "permission": knownvalue.StringExact("pull"), + }), + })), + }, + }, + { + Config: config1, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetExact([]knownvalue.Check{ + knownvalue.MapExact(map[string]knownvalue.Check{ + "team_id": knownvalue.StringExact(teamName1), + "permission": knownvalue.StringExact("push"), + }), + knownvalue.MapExact(map[string]knownvalue.Check{ + "team_id": knownvalue.StringExact(teamName2), + "permission": knownvalue.StringExact("push"), + }), + })), + }, }, }, }) }) - t.Run("updates user collaborators without error", func(t *testing.T) { - if len(testAccConf.testExternalUser2) == 0 { - t.Skip("No additional external user provided") + t.Run("remove_user", func(t *testing.T) { + if len(testAccConf.testOrgUser) == 0 { + t.Skip("No organization user provided") } - ctx := t.Context() - conn := meta.v3client randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - repoName := fmt.Sprintf("%srepo-collabs-%s", testResourcePrefix, randomID) + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) - config := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "push" - } - } - `, repoName, testAccConf.testExternalUser) - - configUpdate := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "pull" - } - } - `, repoName, testAccConf.testExternalUser2) + configPre := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} +`, repoName) + + config0 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + user { + username = "%s" + permission = "admin" + } +} +`, configPre, testAccConf.testOrgUser) + + config1 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name +} +`, configPre) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnauthenticated(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, + Config: config0, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, }, { - Config: configUpdate, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "user.#"), - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "team.#"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "user.#", "1"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "team.#", "0"), - func(state *terraform.State) error { - owner := testAccConf.owner - - collaborators := state.RootModule().Resources["github_repository_collaborators.test_repo_collaborators"].Primary - for name, val := range collaborators.Attributes { - if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".username") && val != testAccConf.testExternalUser2 { - return fmt.Errorf("expected user.*.username to be set to %s, was %s", testAccConf.testExternalUser, val) - } - if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".permission") && val != "pull" { - return fmt.Errorf("expected user.*.permission to be set to pull, was %s", val) - } - } - - invites, _, err := conn.Repositories.ListInvitations(ctx, owner, repoName, nil) - if err != nil { - return err - } - if len(invites) != 1 { - return fmt.Errorf("expected an invite for %s but not found", testAccConf.testExternalUser) - } - if invites[0].GetInvitee().GetLogin() != testAccConf.testExternalUser2 { - return fmt.Errorf("expected an invite for %s for repo %s/%s", testAccConf.testExternalUser, owner, repoName) - } - perm := getPermission(invites[0].GetPermissions()) - if perm != "pull" { - return fmt.Errorf("expected the invite for %s to have pull perms for for %s/%s, found %s", testAccConf.testExternalUser, owner, repoName, perm) - } - return nil - }, - ), + Config: config1, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, }, }, }) }) - t.Run("updates team collaborators without error", func(t *testing.T) { - if len(testAccConf.testExternalUser2) == 0 { - t.Skip("No additional external user provided") - } - - ctx := t.Context() - conn := meta.v3client + t.Run("change_team_reference", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - repoName := fmt.Sprintf("%srepo-collabs-%s", testResourcePrefix, randomID) - teamName1 := fmt.Sprintf("%steam-collabs-1-%s", testResourcePrefix, randomID) - teamName2 := fmt.Sprintf("%steam-collabs-2-%s", testResourcePrefix, randomID) + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) + teamName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) - config := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_team" "test_0" { - name = "%s" - } - - resource "github_team" "test_1" { - name = "%s" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "admin" - } - user { - username = "%s" - permission = "admin" - } - team { - team_id = github_team.test.id - permission = "pull" - } - team { - team_id = github_team.test2.id - permission = "pull" - } - } - `, repoName, teamName1, teamName2, testAccConf.testExternalUser, testAccConf.testExternalUser2) - - configUpdate := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_team" "test_0" { - name = "%s" - } - - resource "github_team" "test_1" { - name = "%s" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "push" - } - team { - team_id = github_team.test_0.id - permission = "push" - } - } - `, repoName, teamName1, teamName2, testAccConf.testExternalUser) + configPre := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} + +resource "github_team" "test" { + name = "%s" +} +`, repoName, teamName) + + config0 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + team { + team_id = github_team.test.id + permission = "pull" + } +} +`, configPre) + + config1 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + team { + team_id = github_team.test.slug + permission = "pull" + } +} +`, configPre) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, + Config: config0, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, }, { - Config: configUpdate, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "user.#"), - resource.TestCheckResourceAttrSet("github_repository_collaborators.test_repo_collaborators", "team.#"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "user.#", "1"), - resource.TestCheckResourceAttr("github_repository_collaborators.test_repo_collaborators", "team.#", "1"), - func(state *terraform.State) error { - owner := testAccConf.owner - - teamAttrs := state.RootModule().Resources["github_team.test"].Primary.Attributes - collaborators := state.RootModule().Resources["github_repository_collaborators.test_repo_collaborators"].Primary - for name, val := range collaborators.Attributes { - if strings.HasPrefix(name, "user.") && strings.HasSuffix(name, ".username") && val != testAccConf.testExternalUser { - return fmt.Errorf("expected user.*.username to be set to %s, was %s", testAccConf.testExternalUser, val) - } - 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["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) - } - } - - users, _, err := conn.Repositories.ListCollaborators(ctx, owner, repoName, &github.ListCollaboratorsOptions{Affiliation: "direct"}) - if err != nil { - return err - } - if len(users) != 1 { - return fmt.Errorf("expected %s to be a collaborator for repo %s/%s", testAccConf.testExternalUser, owner, repoName) - } - perm := getPermission(users[0].GetRoleName()) - if perm != "push" { - return fmt.Errorf("expected %s to have push perms for repo %s/%s, found %s", testAccConf.testExternalUser, owner, repoName, perm) - } - teams, _, err := conn.Repositories.ListTeams(ctx, owner, repoName, nil) - if err != nil { - return err - } - if len(teams) != 1 { - return fmt.Errorf("expected team %s to be a collaborator for %s/%s", repoName, owner, repoName) - } - perm = getPermission(teams[0].GetPermission()) - if perm != "push" { - return fmt.Errorf("expected team %s to have push perms for repo %s/%s, found %s", repoName, owner, repoName, perm) - } - return nil - }, - ), + Config: config1, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, }, }, }) }) - t.Run("removes user collaborators without error", func(t *testing.T) { - ctx := t.Context() - conn := meta.v3client + t.Run("remove_team", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - repoName := fmt.Sprintf("%srepo-collabs-%s", testResourcePrefix, randomID) + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) + teamName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) - config := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "push" - } - } - `, repoName, testAccConf.testExternalUser) - - configUpdate := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - `, repoName) + configPre := fmt.Sprintf(` +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} + +resource "github_team" "test" { + name = "%s" +} +`, repoName, teamName) + + config0 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + team { + team_id = github_team.test.slug + permission = "pull" + } +} +`, configPre) + + config1 := fmt.Sprintf(` +%s + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name +} +`, configPre) resource.Test(t, resource.TestCase{ - PreCheck: func() { skipUnauthenticated(t) }, + PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, + Config: config0, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, }, { - Config: configUpdate, - Check: resource.ComposeTestCheckFunc( - func(state *terraform.State) error { - owner := testAccConf.owner - - invites, _, err := conn.Repositories.ListInvitations(ctx, owner, repoName, nil) - if err != nil { - return err - } - if len(invites) != 0 { - return fmt.Errorf("expected no invites but not found %d", len(invites)) - } - return nil - }, - ), + Config: config1, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(0)), + statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)), + }, }, }, }) }) - t.Run("removes team collaborators without error", func(t *testing.T) { - if len(testAccConf.testExternalUser2) == 0 { - t.Skip("No additional external user provided") - } - - ctx := t.Context() - conn := meta.v3client + t.Run("duplicate_teams_error", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) - repoName := fmt.Sprintf("%srepo-collabs-%s", testResourcePrefix, randomID) - teamName := fmt.Sprintf("%steam-collabs-%s", testResourcePrefix, randomID) + repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) + teamName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) config := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_team" "test" { - name = "%s" - } - - resource "github_repository_collaborators" "test_repo_collaborators" { - repository = "${github_repository.test.name}" - - user { - username = "%s" - permission = "admin" - } - user { - username = "%s" - permission = "admin" - } - team { - team_id = github_team.test.id - permission = "pull" - } - } - `, repoName, teamName, testAccConf.testExternalUser, testAccConf.testExternalUser2) - - configUpdate := fmt.Sprintf(` - resource "github_repository" "test" { - name = "%s" - auto_init = true - visibility = "private" - } - - resource "github_team" "test" { - name = "%s" - } - `, repoName, teamName) +resource "github_repository" "test" { + name = "%s" + visibility = "private" +} + +resource "github_team" "test" { + name = "%s" +} + +resource "github_repository_collaborators" "test" { + repository = github_repository.test.name + + team { + team_id = github_team.test.slug + permission = "pull" + } + + team { + team_id = github_team.test.slug + permission = "push" + } +} +`, repoName, teamName) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ { - Config: config, - }, - { - Config: configUpdate, - Check: resource.ComposeTestCheckFunc( - func(state *terraform.State) error { - owner := testAccConf.owner - - users, _, err := conn.Repositories.ListCollaborators(ctx, owner, repoName, &github.ListCollaboratorsOptions{Affiliation: "direct"}) - if err != nil { - return err - } - if len(users) != 0 { - return fmt.Errorf("expected no collaborators for repo %s/%s but found %d", owner, repoName, len(users)) - } - teams, _, err := conn.Repositories.ListTeams(ctx, owner, repoName, nil) - if err != nil { - return err - } - if len(teams) != 0 { - return fmt.Errorf("expected no teams to be a collaborator for %s/%s but found %d", owner, repoName, len(teams)) - } - return nil - }, - ), + Config: config, + ExpectError: regexp.MustCompile(`duplicate team .+ found`), }, }, }) diff --git a/github/resource_github_team.go b/github/resource_github_team.go index f20c4f3561..d37bd03378 100644 --- a/github/resource_github_team.go +++ b/github/resource_github_team.go @@ -112,15 +112,16 @@ func resourceGithubTeam() *schema.Resource { } } -func resourceGithubTeamCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { +func resourceGithubTeamCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + ownerName := meta.name + err := checkOrganization(meta) if err != nil { return diag.FromErr(err) } - client := meta.(*Owner).v3client - ownerName := meta.(*Owner).name - name := d.Get("name").(string) newTeam := github.NewTeam{ @@ -135,7 +136,7 @@ func resourceGithubTeamCreate(ctx context.Context, d *schema.ResourceData, meta } if parentTeamID, ok := d.GetOk("parent_team_id"); ok { - teamId, err := getTeamID(parentTeamID.(string), meta) + teamId, err := getTeamID(ctx, meta, parentTeamID.(string)) if err != nil { return diag.FromErr(err) } @@ -306,16 +307,17 @@ func resourceGithubTeamRead(ctx context.Context, d *schema.ResourceData, meta an return nil } -func resourceGithubTeamUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { +func resourceGithubTeamUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id + err := checkOrganization(meta) if err != nil { return diag.FromErr(err) } - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id var removeParentTeam bool - editedTeam := github.NewTeam{ Name: d.Get("name").(string), Description: new(d.Get("description").(string)), @@ -323,7 +325,7 @@ func resourceGithubTeamUpdate(ctx context.Context, d *schema.ResourceData, meta NotificationSetting: new(d.Get("notification_setting").(string)), } if parentTeamID, ok := d.GetOk("parent_team_id"); ok { - teamId, err := getTeamID(parentTeamID.(string), meta) + teamId, err := getTeamID(ctx, meta, parentTeamID.(string)) if err != nil { return diag.FromErr(err) } @@ -434,8 +436,10 @@ func resourceGithubTeamDelete(ctx context.Context, d *schema.ResourceData, meta return nil } -func resourceGithubTeamImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { - teamId, err := getTeamID(d.Id(), meta) +func resourceGithubTeamImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) { + meta := m.(*Owner) + + teamId, err := getTeamID(ctx, meta, d.Id()) if err != nil { return nil, err } diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index e312754525..ce082ea42c 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/google/go-github/v84/github" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/shurcooL/githubv4" ) @@ -18,12 +19,12 @@ type MemberChange struct { func resourceGithubTeamMembers() *schema.Resource { return &schema.Resource{ - Create: resourceGithubTeamMembersCreate, - Read: resourceGithubTeamMembersRead, - Update: resourceGithubTeamMembersUpdate, - Delete: resourceGithubTeamMembersDelete, + CreateContext: resourceGithubTeamMembersCreate, + ReadContext: resourceGithubTeamMembersRead, + UpdateContext: resourceGithubTeamMembersUpdate, + DeleteContext: resourceGithubTeamMembersDelete, Importer: &schema.ResourceImporter{ - State: resourceGithubTeamMembersImport, + StateContext: resourceGithubTeamMembersImport, }, Schema: map[string]*schema.Schema{ @@ -59,16 +60,16 @@ func resourceGithubTeamMembers() *schema.Resource { } } -func resourceGithubTeamMembersCreate(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id +func resourceGithubTeamMembersCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id teamIdString := d.Get("team_id").(string) - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } - ctx := context.Background() members := d.Get("members").(*schema.Set) for _, mMap := range members.List() { @@ -86,25 +87,25 @@ func resourceGithubTeamMembersCreate(d *schema.ResourceData, meta any) error { }, ) if err != nil { - return err + return diag.FromErr(err) } } d.SetId(teamIdString) - return resourceGithubTeamMembersRead(d, meta) + return resourceGithubTeamMembersRead(ctx, d, meta) } -func resourceGithubTeamMembersUpdate(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id +func resourceGithubTeamMembersUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id teamIdString := d.Get("team_id").(string) - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } - ctx := context.Background() o, n := d.GetChange("members") vals := make(map[string]*MemberChange) @@ -146,7 +147,7 @@ func resourceGithubTeamMembersUpdate(d *schema.ResourceData, meta any) error { _, err = client.Teams.RemoveTeamMembershipByID(ctx, orgId, teamId, username) if err != nil { - return err + return diag.FromErr(err) } } @@ -163,39 +164,39 @@ func resourceGithubTeamMembersUpdate(d *schema.ResourceData, meta any) error { }, ) if err != nil { - return err + return diag.FromErr(err) } } } d.SetId(teamIdString) - return resourceGithubTeamMembersRead(d, meta) + return resourceGithubTeamMembersRead(ctx, d, meta) } -func resourceGithubTeamMembersRead(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v4client - orgName := meta.(*Owner).name +func resourceGithubTeamMembersRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v4client + orgName := meta.name + teamIdString := d.Get("team_id").(string) if teamIdString == "" && !d.IsNewResource() { log.Printf("[DEBUG] Importing team with id %q", d.Id()) teamIdString = d.Id() } - teamSlug, err := getTeamSlug(teamIdString, meta) + teamSlug, err := getTeamSlug(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } // We intentionally set these early to allow reconciliation // from an upstream bug which emptied team_id in state // See https://github.com/integrations/terraform-provider-github/issues/323 if err := d.Set("team_id", teamIdString); err != nil { - return err + return diag.FromErr(err) } - ctx := context.WithValue(context.Background(), ctxId, d.Id()) - log.Printf("[DEBUG] Reading team members: %s", teamIdString) var q struct { Organization struct { @@ -225,7 +226,7 @@ func resourceGithubTeamMembersRead(d *schema.ResourceData, meta any) error { var teamMembersAndMaintainers []any for { if err := client.Query(ctx, &q, variables); err != nil { - return err + return diag.FromErr(err) } // Add all members to the list @@ -242,23 +243,24 @@ func resourceGithubTeamMembersRead(d *schema.ResourceData, meta any) error { } if err := d.Set("members", teamMembersAndMaintainers); err != nil { - return err + return diag.FromErr(err) } return nil } -func resourceGithubTeamMembersDelete(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id +func resourceGithubTeamMembersDelete(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id + teamIdString := d.Get("team_id").(string) - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } members := d.Get("members").(*schema.Set) - ctx := context.WithValue(context.Background(), ctxId, d.Id()) for _, member := range members.List() { mem := member.(map[string]any) @@ -268,15 +270,17 @@ func resourceGithubTeamMembersDelete(d *schema.ResourceData, meta any) error { _, err = client.Teams.RemoveTeamMembershipByID(ctx, orgId, teamId, username) if err != nil { - return err + return diag.FromErr(err) } } return nil } -func resourceGithubTeamMembersImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { - teamId, err := getTeamID(d.Id(), meta) +func resourceGithubTeamMembersImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) { + meta := m.(*Owner) + + teamId, err := getTeamID(ctx, meta, d.Id()) if err != nil { return nil, err } diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 132b80ca25..cc0b559311 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -92,7 +92,7 @@ func testAccCheckGithubTeamMembersDestroy(s *terraform.State) error { teamIdString := rs.Primary.ID - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(context.Background(), meta, teamIdString) if err != nil { return unconvertibleIdErr(teamIdString, err) } diff --git a/github/resource_github_team_membership.go b/github/resource_github_team_membership.go index 75f8064f43..f81112c253 100644 --- a/github/resource_github_team_membership.go +++ b/github/resource_github_team_membership.go @@ -8,23 +8,25 @@ import ( "strconv" "github.com/google/go-github/v84/github" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) func resourceGithubTeamMembership() *schema.Resource { return &schema.Resource{ - Create: resourceGithubTeamMembershipCreateOrUpdate, - Read: resourceGithubTeamMembershipRead, - Update: resourceGithubTeamMembershipCreateOrUpdate, - Delete: resourceGithubTeamMembershipDelete, + CreateContext: resourceGithubTeamMembershipCreateOrUpdate, + ReadContext: resourceGithubTeamMembershipRead, + UpdateContext: resourceGithubTeamMembershipCreateOrUpdate, + DeleteContext: resourceGithubTeamMembershipDelete, Importer: &schema.ResourceImporter{ - State: func(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + StateContext: func(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) { + meta := m.(*Owner) teamIdString, username, err := parseTwoPartID(d.Id(), "team_id", "username") if err != nil { return nil, err } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { return nil, err } @@ -63,16 +65,16 @@ func resourceGithubTeamMembership() *schema.Resource { } } -func resourceGithubTeamMembershipCreateOrUpdate(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id +func resourceGithubTeamMembershipCreateOrUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id teamIdString := d.Get("team_id").(string) - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } - ctx := context.Background() username := d.Get("username").(string) role := d.Get("role").(string) @@ -86,38 +88,39 @@ func resourceGithubTeamMembershipCreateOrUpdate(d *schema.ResourceData, meta any }, ) if err != nil { - return err + return diag.FromErr(err) } d.SetId(buildTwoPartID(teamIdString, username)) - return resourceGithubTeamMembershipRead(d, meta) + return resourceGithubTeamMembershipRead(ctx, d, meta) } -func resourceGithubTeamMembershipRead(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id +func resourceGithubTeamMembershipRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id + teamIdString, username, err := parseTwoPartID(d.Id(), "team_id", "username") if err != nil { - return err + return diag.FromErr(err) } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } // We intentionally set these early to allow reconciliation // from an upstream bug which emptied team_id in state // See https://github.com/integrations/terraform-provider-github/issues/323 if err = d.Set("team_id", teamIdString); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("username", username); err != nil { - return err + return diag.FromErr(err) } - ctx := context.WithValue(context.Background(), ctxId, d.Id()) if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } @@ -137,31 +140,35 @@ func resourceGithubTeamMembershipRead(d *schema.ResourceData, meta any) error { return nil } } - return err + return diag.FromErr(err) } if err = d.Set("etag", resp.Header.Get("ETag")); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("role", membership.GetRole()); err != nil { - return err + return diag.FromErr(err) } return nil } -func resourceGithubTeamMembershipDelete(d *schema.ResourceData, meta any) error { - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id +func resourceGithubTeamMembershipDelete(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id + teamIdString := d.Get("team_id").(string) - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } username := d.Get("username").(string) - ctx := context.WithValue(context.Background(), ctxId, d.Id()) _, err = client.Teams.RemoveTeamMembershipByID(ctx, orgId, teamId, username) + if err != nil { + return diag.FromErr(err) + } - return err + return nil } diff --git a/github/resource_github_team_membership_test.go b/github/resource_github_team_membership_test.go index 35072f5d8a..643de342d9 100644 --- a/github/resource_github_team_membership_test.go +++ b/github/resource_github_team_membership_test.go @@ -102,7 +102,7 @@ func testAccCheckGithubTeamMembershipDestroy(s *terraform.State) error { return err } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(context.Background(), meta, teamIdString) if err != nil { return unconvertibleIdErr(teamIdString, err) } @@ -144,7 +144,7 @@ func testAccCheckGithubTeamMembershipExists(ctx context.Context, n string, membe return err } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(context.Background(), meta, teamIdString) if err != nil { return unconvertibleIdErr(teamIdString, err) } @@ -179,7 +179,7 @@ func testAccCheckGithubTeamMembershipRoleState(ctx context.Context, n, expected if err != nil { return err } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(context.Background(), meta, teamIdString) if err != nil { return unconvertibleIdErr(teamIdString, err) } diff --git a/github/resource_github_team_repository.go b/github/resource_github_team_repository.go index 958cc75721..9b4bf8100c 100644 --- a/github/resource_github_team_repository.go +++ b/github/resource_github_team_repository.go @@ -9,23 +9,25 @@ import ( "strconv" "github.com/google/go-github/v84/github" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) func resourceGithubTeamRepository() *schema.Resource { return &schema.Resource{ - Create: resourceGithubTeamRepositoryCreate, - Read: resourceGithubTeamRepositoryRead, - Update: resourceGithubTeamRepositoryUpdate, - Delete: resourceGithubTeamRepositoryDelete, + CreateContext: resourceGithubTeamRepositoryCreate, + ReadContext: resourceGithubTeamRepositoryRead, + UpdateContext: resourceGithubTeamRepositoryUpdate, + DeleteContext: resourceGithubTeamRepositoryDelete, Importer: &schema.ResourceImporter{ - State: func(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + StateContext: func(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) { + meta := m.(*Owner) teamIdString, username, err := parseTwoPartID(d.Id(), "team_id", "username") if err != nil { return nil, err } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { return nil, err } @@ -62,26 +64,26 @@ func resourceGithubTeamRepository() *schema.Resource { } } -func resourceGithubTeamRepositoryCreate(d *schema.ResourceData, meta any) error { +func resourceGithubTeamRepositoryCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id + orgName := meta.name + err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id - // The given team id could be an id or a slug givenTeamId := d.Get("team_id").(string) - teamId, err := getTeamID(givenTeamId, meta) + teamId, err := getTeamID(ctx, meta, givenTeamId) if err != nil { - return err + return diag.FromErr(err) } - orgName := meta.(*Owner).name repoName := d.Get("repository").(string) permission := d.Get("permission").(string) - ctx := context.Background() _, err = client.Teams.AddTeamRepoByID(ctx, orgId, @@ -93,33 +95,34 @@ func resourceGithubTeamRepositoryCreate(d *schema.ResourceData, meta any) error }, ) if err != nil { - return err + return diag.FromErr(err) } d.SetId(buildTwoPartID(strconv.FormatInt(teamId, 10), repoName)) - return resourceGithubTeamRepositoryRead(d, meta) + return resourceGithubTeamRepositoryRead(ctx, d, meta) } -func resourceGithubTeamRepositoryRead(d *schema.ResourceData, meta any) error { +func resourceGithubTeamRepositoryRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + client := meta.v3client + orgId := meta.id + orgName := meta.name + err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } - client := meta.(*Owner).v3client - orgId := meta.(*Owner).id - teamIdString, repoName, err := parseTwoPartID(d.Id(), "team_id", "repository") if err != nil { - return err + return diag.FromErr(err) } - teamId, err := getTeamID(teamIdString, meta) + teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { - return err + return diag.FromErr(err) } - orgName := meta.(*Owner).name - ctx := context.WithValue(context.Background(), ctxId, d.Id()) + if !d.IsNewResource() { ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string)) } @@ -138,33 +141,33 @@ func resourceGithubTeamRepositoryRead(d *schema.ResourceData, meta any) error { return nil } } - return repoErr + return diag.FromErr(repoErr) } if err = d.Set("etag", resp.Header.Get("ETag")); err != nil { - return err + return diag.FromErr(err) } if d.Get("team_id") == "" { // If team_id is empty, that means we are importing the resource. // Set the team_id to be the id of the team. if err = d.Set("team_id", teamIdString); err != nil { - return err + return diag.FromErr(err) } } if err = d.Set("repository", repo.GetName()); err != nil { - return err + return diag.FromErr(err) } if err = d.Set("permission", getPermission(repo.GetRoleName())); err != nil { - return err + return diag.FromErr(err) } return nil } -func resourceGithubTeamRepositoryUpdate(d *schema.ResourceData, meta any) error { +func resourceGithubTeamRepositoryUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } client := meta.(*Owner).v3client @@ -172,15 +175,14 @@ func resourceGithubTeamRepositoryUpdate(d *schema.ResourceData, meta any) error teamIdString, repoName, err := parseTwoPartID(d.Id(), "team_id", "repository") if err != nil { - return err + return diag.FromErr(err) } teamId, err := strconv.ParseInt(teamIdString, 10, 64) if err != nil { - return unconvertibleIdErr(teamIdString, err) + return diag.FromErr(unconvertibleIdErr(teamIdString, err)) } orgName := meta.(*Owner).name permission := d.Get("permission").(string) - ctx := context.WithValue(context.Background(), ctxId, d.Id()) // the go-github library's AddTeamRepo method uses the add/update endpoint from GitHub API _, err = client.Teams.AddTeamRepoByID(ctx, @@ -193,17 +195,17 @@ func resourceGithubTeamRepositoryUpdate(d *schema.ResourceData, meta any) error }, ) if err != nil { - return err + return diag.FromErr(err) } d.SetId(buildTwoPartID(teamIdString, repoName)) - return resourceGithubTeamRepositoryRead(d, meta) + return resourceGithubTeamRepositoryRead(ctx, d, meta) } -func resourceGithubTeamRepositoryDelete(d *schema.ResourceData, meta any) error { +func resourceGithubTeamRepositoryDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { err := checkOrganization(meta) if err != nil { - return err + return diag.FromErr(err) } client := meta.(*Owner).v3client @@ -211,14 +213,13 @@ func resourceGithubTeamRepositoryDelete(d *schema.ResourceData, meta any) error teamIdString, repoName, err := parseTwoPartID(d.Id(), "team_id", "repository") if err != nil { - return err + return diag.FromErr(err) } teamId, err := strconv.ParseInt(teamIdString, 10, 64) if err != nil { - return unconvertibleIdErr(teamIdString, err) + return diag.FromErr(unconvertibleIdErr(teamIdString, err)) } orgName := meta.(*Owner).name - ctx := context.WithValue(context.Background(), ctxId, d.Id()) resp, err := client.Teams.RemoveTeamRepoByID(ctx, orgId, teamId, orgName, repoName) @@ -226,7 +227,7 @@ func resourceGithubTeamRepositoryDelete(d *schema.ResourceData, meta any) error log.Printf("[DEBUG] Failed to find team %s to delete for repo: %s.", teamIdString, repoName) repo, _, err := client.Repositories.Get(ctx, orgName, repoName) if err != nil { - return err + return diag.FromErr(err) } newRepoName := repo.GetName() if newRepoName != repoName { @@ -234,9 +235,9 @@ func resourceGithubTeamRepositoryDelete(d *schema.ResourceData, meta any) error "Try deleting team repository again.", repoName, newRepoName) _, err := client.Teams.RemoveTeamRepoByID(ctx, orgId, teamId, orgName, newRepoName) - return handleArchivedRepoDelete(err, "team repository access", fmt.Sprintf("team %s", teamIdString), orgName, newRepoName) + return diag.FromErr(handleArchivedRepoDelete(err, "team repository access", fmt.Sprintf("team %s", teamIdString), orgName, newRepoName)) } } - return handleArchivedRepoDelete(err, "team repository access", fmt.Sprintf("team %s", teamIdString), orgName, repoName) + return diag.FromErr(handleArchivedRepoDelete(err, "team repository access", fmt.Sprintf("team %s", teamIdString), orgName, repoName)) } diff --git a/github/util.go b/github/util.go index 2ba57a91bd..bd5e67e2f7 100644 --- a/github/util.go +++ b/github/util.go @@ -1,7 +1,6 @@ package github import ( - "context" "crypto/md5" "errors" "fmt" @@ -10,7 +9,6 @@ import ( "regexp" "slices" "sort" - "strconv" "strings" "github.com/google/go-github/v84/github" @@ -215,61 +213,6 @@ func (e *unconvertibleIdError) Error() string { e.OriginalId, e.OriginalError.Error()) } -func getTeamID(teamIDString string, meta any) (int64, error) { - // Given a string that is either a team id or team slug, return the - // id of the team it is referring to. - ctx := context.Background() - client := meta.(*Owner).v3client - orgName := meta.(*Owner).name - - teamId, parseIntErr := strconv.ParseInt(teamIDString, 10, 64) - if parseIntErr == nil { - return teamId, nil - } - - // The given id not an integer, assume it is a team slug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, teamIDString) - if slugErr != nil { - return -1, errors.New(parseIntErr.Error() + slugErr.Error()) - } - return team.GetID(), nil -} - -func getTeamSlug(teamIDString string, meta any) (string, error) { - ctx := context.Background() - return getTeamSlugContext(ctx, teamIDString, meta) -} - -func getTeamSlugContext(ctx context.Context, teamIDString string, meta any) (string, error) { - // Given a string that is either a team id or team slug, return the - // team slug it is referring to. - client := meta.(*Owner).v3client - orgName := meta.(*Owner).name - orgId := meta.(*Owner).id - - teamId, parseIntErr := strconv.ParseInt(teamIDString, 10, 64) - if parseIntErr != nil { - // The given id not an integer, assume it is a team slug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, teamIDString) - if slugErr != nil { - return "", errors.New(parseIntErr.Error() + slugErr.Error()) - } - return team.GetSlug(), nil - } - - // The given id is an integer, assume it is a team id - team, _, teamIdErr := client.Teams.GetTeamByID(ctx, orgId, teamId) - if teamIdErr != nil { - // There isn't a team with the given ID, assume it is a teamslug - team, _, slugErr := client.Teams.GetTeamBySlug(ctx, orgName, teamIDString) - if slugErr != nil { - return "", errors.New(teamIdErr.Error() + slugErr.Error()) - } - return team.GetSlug(), nil - } - return team.GetSlug(), nil -} - // https://docs.github.com/en/actions/reference/encrypted-secrets#naming-your-secrets var secretNameRegexp = regexp.MustCompile("^[a-zA-Z_][a-zA-Z0-9_]*$") @@ -338,15 +281,3 @@ func toInt64(v any) int64 { return 0 } } - -// lookupTeamID looks up the ID of a team by its slug. -func lookupTeamID(ctx context.Context, meta *Owner, slug string) (int64, error) { - client := meta.v3client - owner := meta.name - - team, _, err := client.Teams.GetTeamBySlug(ctx, owner, slug) - if err != nil { - return 0, err - } - return team.GetID(), nil -} diff --git a/github/util_diff.go b/github/util_diff.go index 5afaec878d..4d4fd44e7b 100644 --- a/github/util_diff.go +++ b/github/util_diff.go @@ -148,7 +148,7 @@ func isNewTeamID(ctx context.Context, diff *schema.ResourceDiff, m any) bool { // Resolve new team_slug to team ID via API oldTeamSlug, newTeamSlug := diff.GetChange("team_slug") - newTeamID, err := lookupTeamID(ctx, meta, newTeamSlug.(string)) + newTeamID, err := lookupTeamID(ctx, meta.v3client, meta.name, newTeamSlug.(string)) if err != nil { // If team doesn't exist or API fails, skip ForceNew check and let Read handle it tflog.Debug(ctx, "Unable to resolve new team_slug to team ID, skipping ForceNew check", map[string]any{ diff --git a/github/util_team.go b/github/util_team.go new file mode 100644 index 0000000000..8f674054d3 --- /dev/null +++ b/github/util_team.go @@ -0,0 +1,152 @@ +package github + +import ( + "context" + "strconv" + + "github.com/google/go-github/v84/github" +) + +// teamIdentity represents a GitHub team. +type teamIdentity struct { + id *int64 + slug *string + teamID *string +} + +// newLegacyTeamIdentity creates a new teamIdentity from the given teamID string representing either the team ID or slug. +func newLegacyTeamIdentity(teamID string) teamIdentity { + t := teamIdentity{teamID: &teamID} + + if id, ok := parseTeamID(teamID); ok { + t.id = &id + } else { + t.slug = &teamID + } + + return t +} + +// getID returns the team ID as an int64 if it's available, or 0 if it's not. +func (t teamIdentity) getID() int64 { + id, _ := t.getIDOK() + return id +} + +// getIDOK returns the team ID as an int64 if it's available. +func (t teamIdentity) getIDOK() (int64, bool) { + if t.id != nil { + return *t.id, true + } + return 0, false +} + +// getSlug returns the slug of the team if it's available, or an empty string if it's not. +func (t teamIdentity) getSlug() string { + slug, _ := t.getSlugOK() + return slug +} + +// getSlugOK returns the slug of the team if it's available. +func (t teamIdentity) getSlugOK() (string, bool) { + if t.slug != nil { + return *t.slug, true + } + return "", false +} + +// getTeamID returns the legacy team ID of the team if it's available, or an empty string if it's not. +func (t teamIdentity) getTeamID() string { + teamID, _ := t.getTeamIDOK() + return teamID +} + +// getTeamIDOK returns the legacy team ID of the team if it's available. +func (t teamIdentity) getTeamIDOK() (string, bool) { + if t.teamID != nil { + return *t.teamID, true + } + return "", false +} + +// teamCollaborator represents a GitHub team collaborator with its identity and permission level. +type teamCollaborator struct { + teamIdentity + permission string +} + +// flatten converts the teamCollaborator into a format suitable for Terraform schema. +func (t teamCollaborator) flatten() any { + if teamID, ok := t.getTeamIDOK(); ok { + return map[string]any{ + "team_id": teamID, + "permission": t.permission, + } + } + + return map[string]any{ + "team_id": t.getSlug(), + "permission": t.permission, + } +} + +// teamCollaborators is a slice of teamCollaborator. +type teamCollaborators []teamCollaborator + +// flatten converts the teamCollaborators slice into a format suitable for Terraform schema. +func (tc teamCollaborators) flatten() any { + items := make([]any, len(tc)) + + for i, t := range tc { + items[i] = t.flatten() + } + + return items +} + +// parseTeamID attempts to parse the given string as a team ID (int64). +// It returns the parsed ID and a boolean indicating whether the parsing was successful. +func parseTeamID(s string) (int64, bool) { + if id, err := strconv.ParseInt(s, 10, 64); err == nil { + return id, true + } + return 0, false +} + +// getTeamSlug returns the slug of the team identified by the given string, which may be either a team ID or a team slug. +func getTeamSlug(ctx context.Context, meta *Owner, s string) (string, error) { + if id, ok := parseTeamID(s); ok { + // The given id is an integer, assume it is the team ID and look up the corresponding team slug. + return lookupTeamSlug(ctx, meta.v3client, meta.id, id) + } + // The given id not an integer, assume it is a team slug. + return s, nil +} + +// getTeamID returns the id of the team identified by the given string, which may be either a team ID or a team slug. +func getTeamID(ctx context.Context, meta *Owner, s string) (int64, error) { + if id, ok := parseTeamID(s); ok { + // The given id is an integer, assume it is the team ID. + return id, nil + } + // The given id not an integer, assume it is a team slug and look up the corresponding team ID. + return lookupTeamID(ctx, meta.v3client, meta.name, s) +} + +// lookupTeamSlug looks up the slug of a team by its ID. +func lookupTeamSlug(ctx context.Context, client *github.Client, orgID, id int64) (string, error) { + team, _, err := client.Teams.GetTeamByID(ctx, orgID, id) //nolint:staticcheck + if err != nil { + return "", err + } + return team.GetSlug(), nil +} + +// lookupTeamID looks up the ID of a team by its slug. +func lookupTeamID(ctx context.Context, client *github.Client, orgName, slug string) (int64, error) { + team, _, err := client.Teams.GetTeamBySlug(ctx, orgName, slug) + if err != nil { + return 0, err + } + return team.GetID(), nil +} diff --git a/github/util_user.go b/github/util_user.go new file mode 100644 index 0000000000..5f4b5aeddc --- /dev/null +++ b/github/util_user.go @@ -0,0 +1,56 @@ +package github + +import "strconv" + +// userIdentity represents a GitHub user by their login. +type userIdentity struct { + login string +} + +// userCollaborator represents a GitHub user collaborator with its identity and permission level. +type userCollaborator struct { + userIdentity + permission string + invitationID *int64 +} + +// flatten converts the userCollaborator into a format suitable for Terraform schema. +func (u userCollaborator) flatten() any { + m := map[string]any{ + "username": u.login, + "permission": u.permission, + } + + if u.invitationID != nil { + m["invitation_id"] = *u.invitationID + } + + return m +} + +// userCollaborators is a slice of userCollaborator. +type userCollaborators []userCollaborator + +// flatten converts the userCollaborators slice into a format suitable for Terraform schema. +func (uc userCollaborators) flatten() any { + items := make([]any, len(uc)) + + for i, u := range uc { + items[i] = u.flatten() + } + + return items +} + +// flattenInvitations converts the userCollaborators slice into a format suitable for Terraform schema. +func (uc userCollaborators) flattenInvitations() any { + m := make(map[string]any) + + for _, u := range uc { + if u.invitationID != nil { + m[u.login] = strconv.FormatInt(*u.invitationID, 10) + } + } + + return m +} diff --git a/website/docs/r/repository_collaborators.html.markdown b/website/docs/r/repository_collaborators.html.markdown index 93c01aef91..4f1e527aa0 100644 --- a/website/docs/r/repository_collaborators.html.markdown +++ b/website/docs/r/repository_collaborators.html.markdown @@ -52,12 +52,12 @@ resource "github_repository_collaborators" "some_repo_collaborators" { user { permission = "admin" - username = "SomeUser" + username = "SomeUser" } team { permission = "pull" - team_id = github_team.some_team.slug + team_id = github_team.some_team.slug } } ``` @@ -66,40 +66,40 @@ resource "github_repository_collaborators" "some_repo_collaborators" { The following arguments are supported: -* `repository` - (Required) The GitHub repository. -* `user` - (Optional) List of users to grant access to the repository. -* `team` - (Optional) List of teams to grant access to the repository. -* `ignore_team` - (Optional) List of teams to ignore when checking for repository access. This supports ignoring teams granted access at an organizational level. +- `repository` - (Required) The GitHub repository. +- `user` - (Optional) List of users to grant access to the repository. +- `team` - (Optional) List of teams to grant access to the repository. +- `ignore_team` - (Optional) List of teams to ignore when checking for repository access. This supports ignoring teams granted access at an organizational level. The `user` block supports: -* `username` - (Required) The user to add to the repository as a collaborator. -* `permission` - (Optional) The permission of the outside collaborators for the repository. +- `username` - (Required) The user to add to the repository as a collaborator. +- `permission` - (Optional) The permission of the outside collaborators for the repository. Must be one of `pull`, `push`, `maintain`, `triage` or `admin` or the name of an existing [custom repository role](https://docs.github.com/en/enterprise-cloud@latest/organizations/managing-peoples-access-to-your-organization-with-roles/managing-custom-repository-roles-for-an-organization) within the organization for organization-owned repositories. Must be `push` for personal repositories. Defaults to `push`. The `team` block supports: -* `team_id` - (Required) The GitHub team id or the GitHub team slug. -* `permission` - (Optional) The permission of the outside collaborators for the repository. +- `team_id` - (Required) The GitHub team id or the GitHub team slug. +- `permission` - (Optional) The permission of the outside collaborators for the repository. Must be one of `pull`, `triage`, `push`, `maintain`, `admin` or the name of an existing [custom repository role](https://docs.github.com/en/enterprise-cloud@latest/organizations/managing-peoples-access-to-your-organization-with-roles/managing-custom-repository-roles-for-an-organization) within the organisation. Defaults to `pull`. Must be `push` for personal repositories. Defaults to `push`. The `ignore_team` block supports: -* `team_id` - (Required) The GitHub team id or the GitHub team slug. +- `team_id` - (Required) The GitHub team id or the GitHub team slug. ## Attribute Reference In addition to the above arguments, the following attributes are exported: -* `invitation_ids` - Map of usernames to invitation ID for any users added as part of creation of this resource to +- `invitation_ids` - Map of usernames to invitation ID for any users added as part of creation of this resource to be used in [`github_user_invitation_accepter`](./user_invitation_accepter.html). ## Import -GitHub Repository Collaborators can be imported using the name `name`, e.g. +GitHub Repository Collaborators can be imported using the repository name `name`, e.g. -``` -$ terraform import github_repository_collaborators.collaborators terraform +```hcl +terraform import github_repository_collaborators.collaborators myrepo ```