fixup! fix: Stop repo collaborators drifting on owner

Author: stevehipwell

.golangci.yml

@@ -12,7 +12,7 @@ linters:
     - errcheck
     - errname
     - errorlint
-    # - forcetypeassert TODO: Re-enable when we can fix the issues
+    # - forcetypeassert # TODO: Re-enable when we can fix the issues
     - godot
     - govet
     - ineffassign

docs/resources/repository_collaborators.md

@@ -25,9 +25,7 @@ Teams will be added to the repository on apply, and removed if removed from the
 
 ## Personal Repositories
 
-For personal repositories, collaborators can only be granted [write](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/repository-access-and-collaboration/permission-levels-for-a-personal-account-repository#collaborator-access-for-a-repository-owned-by-a-personal-account) permission.
-
-!> If the repository owner is not added as a collaborator with admin access, the provider will churn this resource on every plan/apply. To prevent this, ensure that the repository owner is included in the set of user collaborators.
+For personal repositories, non-owner collaborators can only be granted [write](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/repository-access-and-collaboration/permission-levels-for-a-personal-account-repository#collaborator-access-for-a-repository-owned-by-a-personal-account) permission. Owners will be ignored unless they are explicitly added, in which case they must be granted `admin` permission.
 
 ## Users
 
@@ -90,14 +88,15 @@ resource "github_repository_collaborators" "some_repo_collaborators" {
 - `repository_id` (Number) ID of the repository.
 
 <a id="nestedblock--ignore_team"></a>
+
 ### Nested Schema for `ignore_team`
 
 Required:
 
 - `team_id` (String) ID or slug of the team to ignore.
 
-
 <a id="nestedblock--team"></a>
+
 ### Nested Schema for `team`
 
 Required:
@@ -108,8 +107,8 @@ Optional:
 
 - `permission` (String) Permission to grant to the team. 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 organization. Defaults to `push`.
 
-
 <a id="nestedblock--user"></a>
+
 ### Nested Schema for `user`
 
 Required:

github/resource_github_repository_collaborators.go

@@ -18,13 +18,18 @@ import (
 
 func resourceGithubRepositoryCollaborators() *schema.Resource {
 	return &schema.Resource{
-		SchemaVersion: 1,
+		SchemaVersion: 2,
 		StateUpgraders: []schema.StateUpgrader{
 			{
 				Type:    resourceGithubRepositoryCollaboratorsV0().CoreConfigSchema().ImpliedType(),
 				Upgrade: resourceGithubRepositoryCollaboratorsStateUpgradeV0,
 				Version: 0,
 			},
+			{
+				Type:    resourceGithubRepositoryCollaboratorsV1().CoreConfigSchema().ImpliedType(),
+				Upgrade: resourceGithubRepositoryCollaboratorsStateUpgradeV1,
+				Version: 1,
+			},
 		},
 
 		Description: "Manage the complete set of collaborators (users and teams) for a GitHub repository.",
@@ -145,32 +150,6 @@ func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.Re
 		}
 	}
 
-	if meta.IsOrganization {
-		if err := d.SetNew("owner_configured", false); err != nil {
-			return fmt.Errorf("error setting owner_configured: %w", err)
-		}
-	} else if d.NewValueKnown("user") {
-		users := d.Get("user").(*schema.Set).List()
-		ownerConfigured := false
-		owner := strings.ToLower(meta.name)
-
-		for _, u := range users {
-			user := u.(map[string]any)
-			if strings.ToLower(user["username"].(string)) == owner {
-				ownerConfigured = true
-				break
-			}
-		}
-
-		if err := d.SetNew("owner_configured", ownerConfigured); err != nil {
-			return fmt.Errorf("error setting owner_configured: %w", err)
-		}
-	} else {
-		if err := d.SetNewComputed("owner_configured"); err != nil {
-			return fmt.Errorf("error setting owner_configured to computed: %w", err)
-		}
-	}
-
 	if d.HasChange("team") && d.NewValueKnown("team") {
 		v, diags := d.GetRawConfigAt(cty.GetAttrPath("team"))
 		if diags.HasError() {
@@ -196,7 +175,61 @@ func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.Re
 		}
 	}
 
-	if len(d.Id()) == 0 {
+	if meta.IsOrganization {
+		// If the repository belongs to an organization the owner cannot be a,
+		// collaborator, so owner_configured is always false.
+
+		if err := d.SetNew("owner_configured", false); err != nil {
+			return fmt.Errorf("error setting owner_configured: %w", err)
+		}
+	} else if d.NewValueKnown("user") {
+		// If the repository belongs to a user and we know the new value of user,
+		// then we can determine the value of owner_configured by checking if
+		// the owner is included in the list of users.
+
+		ownerConfigured := false
+		owner := strings.ToLower(meta.name)
+
+		usersVal := d.Get("user")
+		if users, ok := usersVal.(*schema.Set); ok {
+			for _, u := range users.List() {
+				user, ok := u.(map[string]any)
+				if !ok {
+					continue
+				}
+
+				usernameVal, ok := user["username"]
+				if !ok {
+					continue
+				}
+
+				username, ok := usernameVal.(string)
+				if !ok {
+					continue
+				}
+
+				if strings.ToLower(username) == owner {
+					ownerConfigured = true
+					break
+				}
+			}
+		}
+
+		if err := d.SetNew("owner_configured", ownerConfigured); err != nil {
+			return fmt.Errorf("error setting owner_configured: %w", err)
+		}
+	} else {
+		// If the repository belongs to a user but we don't know the new value of user,
+		// then we don't know if the owner is configured as a collaborator or not,
+		// so we set owner_configured to computed to indicate that Terraform should
+		// determine the value during apply.
+
+		if err := d.SetNewComputed("owner_configured"); err != nil {
+			return fmt.Errorf("error setting owner_configured to computed: %w", err)
+		}
+	}
+
+	if d.Id() == "" {
 		return nil
 	}
 
@@ -281,9 +314,10 @@ func resourceGithubRepositoryCollaboratorsRead(ctx context.Context, d *schema.Re
 	repoName := d.Get("repository").(string)
 	teams := d.Get("team").(*schema.Set).List()
 	ignoreTeams := d.Get("ignore_team").(*schema.Set).List()
+	ownerConfigured, _ := d.Get("owner_configured").(bool)
 
 	inIgnoreUsers := make([]string, 0)
-	if !isOrg && !d.Get("owner_configured").(bool) {
+	if !isOrg && !ownerConfigured {
 		inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
 	}
 

github/resource_github_repository_collaborators_migration.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log"
 	"strconv"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
@@ -88,7 +89,7 @@ func resourceGithubRepositoryCollaboratorsStateUpgradeV0(ctx context.Context, ra
 	client := meta.v3client
 	owner := meta.name
 
-	log.Printf("[DEBUG] GitHub Repository Collaborators Attributes before migration: %#v", rawState)
+	log.Printf("[DEBUG] GitHub Repository Collaborators Attributes before migration to v1: %#v", rawState)
 
 	repoName, ok := rawState["repository"].(string)
 	if !ok {
@@ -103,7 +104,141 @@ func resourceGithubRepositoryCollaboratorsStateUpgradeV0(ctx context.Context, ra
 	rawState["id"] = strconv.FormatInt(repo.GetID(), 10)
 	rawState["repository_id"] = int(repo.GetID())
 
-	log.Printf("[DEBUG] GitHub Repository Collaborators Attributes after migration: %#v", rawState)
+	log.Printf("[DEBUG] GitHub Repository Collaborators Attributes after migration to v1: %#v", rawState)
+
+	return rawState, nil
+}
+
+func resourceGithubRepositoryCollaboratorsV1() *schema.Resource {
+	return &schema.Resource{
+		SchemaVersion: 1,
+
+		Schema: map[string]*schema.Schema{
+			"repository": {
+				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,
+				Optional:    true,
+				Description: "Users to grant access to the repository.",
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"username": {
+							Type:             schema.TypeString,
+							Description:      "Login for the user to add to the repository as a collaborator.",
+							Required:         true,
+							DiffSuppressFunc: caseInsensitive(),
+						},
+						"permission": {
+							Type:        schema.TypeString,
+							Description: "Permission to grant to the user. 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 organization. Must be `push` for personal repositories. Defaults to `push`.",
+							Optional:    true,
+							Default:     "push",
+						},
+					},
+				},
+			},
+			"team": {
+				Type:        schema.TypeSet,
+				Optional:    true,
+				Description: "Teams to grant access to the repository.",
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"team_id": {
+							Type:        schema.TypeString,
+							Description: "ID or slug of the team to add to the repository as a collaborator.",
+							Required:    true,
+						},
+						"permission": {
+							Type:        schema.TypeString,
+							Description: "Permission to grant to the team. 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 organization. Defaults to `push`.",
+							Optional:    true,
+							Default:     "push",
+						},
+					},
+				},
+			},
+			"invitation_ids": {
+				Type:        schema.TypeMap,
+				Description: "Map of usernames to invitation ID for users that haven't yet accepted their invitation to become a collaborator. This is only set on read, and is used internally to track pending invitations for users that aren't yet collaborators.",
+				Elem: &schema.Schema{
+					Type: schema.TypeString,
+				},
+				Computed: true,
+			},
+			"ignore_team": {
+				Type:        schema.TypeSet,
+				Optional:    true,
+				Description: "Teams to ignore when managing repository collaborators.",
+				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 resourceGithubRepositoryCollaboratorsStateUpgradeV1(_ context.Context, rawState map[string]any, m any) (map[string]any, error) {
+	meta, _ := m.(*Owner)
+
+	log.Printf("[DEBUG] GitHub Repository Collaborators Attributes before migration to v2: %#v", rawState)
+
+	if meta.IsOrganization {
+		// If the repository belongs to an organization the owner cannot be a,
+		// collaborator, so owner_configured is always false.
+
+		rawState["owner_configured"] = false
+	} else {
+		// If the repository belongs to a user and we know the new value of user
+		// we can determine the value of owner_configured by checking if the owner
+		// is included in the list of users.
+
+		ownerConfigured := false
+		owner := strings.ToLower(meta.name)
+
+		if usersVal, ok := rawState["user"]; ok {
+			if users, ok := usersVal.([]any); ok {
+				for _, userVal := range users {
+					user, ok := userVal.(map[string]any)
+					if !ok {
+						continue
+					}
+
+					usernameVal, ok := user["username"]
+					if !ok {
+						continue
+					}
+
+					username, ok := usernameVal.(string)
+					if !ok {
+						continue
+					}
+
+					if strings.ToLower(username) == owner {
+						ownerConfigured = true
+						break
+					}
+				}
+			}
+		}
+
+		rawState["owner_configured"] = ownerConfigured
+	}
+
+	log.Printf("[DEBUG] GitHub Repository Collaborators Attributes after migration to v2: %#v", rawState)
 
 	return rawState, nil
 }