fix: Stop repo collaborators drifting on owner (#3471)

* fix: Stop repo collaborators drifting on owner

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

* fixup! fix: Stop repo collaborators drifting on owner

* fixup! fix: Stop repo collaborators drifting on owner

* fixup! fix: Stop repo collaborators drifting on owner

---------

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

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
 
@@ -86,6 +84,7 @@ resource "github_repository_collaborators" "some_repo_collaborators" {
 
 - `id` (String) The ID of this resource.
 - `invitation_ids` (Map of String) 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.
+- `owner_configured` (Boolean) Indicates whether the owner of a personal repository is configured as a collaborator.
 - `repository_id` (Number) ID of the repository.
 
 <a id="nestedblock--ignore_team"></a>

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.",
@@ -89,6 +94,11 @@ func resourceGithubRepositoryCollaborators() *schema.Resource {
 					},
 				},
 			},
+			"owner_configured": {
+				Type:        schema.TypeBool,
+				Computed:    true,
+				Description: "Indicates whether the owner of a personal repository is configured as a collaborator.",
+			},
 			"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.",
@@ -121,15 +131,34 @@ func resourceGithubRepositoryCollaborators() *schema.Resource {
 }
 
 func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.ResourceDiff, m any) error {
-	tflog.Debug(ctx, "Diffing user collaborators")
+	tflog.Debug(ctx, "Diffing collaborators")
+
+	meta, _ := m.(*Owner)
+
 	if d.HasChange("user") {
-		users := d.Get("user").(*schema.Set).List()
+		users, ok := d.Get("user").(*schema.Set)
+		if !ok {
+			return fmt.Errorf("error reading user config")
+		}
+
 		seen := make(map[string]any)
+		for _, u := range users.List() {
+			user, ok := u.(map[string]any)
+			if !ok {
+				return fmt.Errorf("error reading user config")
+			}
 
-		for _, u := range users {
-			user := u.(map[string]any)
-			username := user["username"].(string)
+			usernameVal, ok := user["username"]
+			if !ok {
+				return fmt.Errorf("error reading user config")
+			}
 
+			username, ok := usernameVal.(string)
+			if !ok {
+				return fmt.Errorf("error reading user config")
+			}
+
+			username = strings.ToLower(username)
 			if _, ok := seen[username]; ok {
 				return fmt.Errorf("duplicate username %s found in user collaborators", username)
 			}
@@ -162,7 +191,63 @@ 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 := meta.name
+
+		users, ok := d.Get("user").(*schema.Set)
+		if !ok {
+			return fmt.Errorf("error reading user config")
+		}
+
+		for _, u := range users.List() {
+			user, ok := u.(map[string]any)
+			if !ok {
+				return fmt.Errorf("error reading user config")
+			}
+
+			usernameVal, ok := user["username"]
+			if !ok {
+				return fmt.Errorf("error reading user config")
+			}
+
+			username, ok := usernameVal.(string)
+			if !ok {
+				return fmt.Errorf("error reading user config")
+			}
+
+			if strings.EqualFold(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
 	}
 
@@ -186,16 +271,36 @@ func resourceGithubRepositoryCollaboratorsCreate(ctx context.Context, d *schema.
 	teams := d.Get("team").(*schema.Set).List()
 	ignoreTeams := d.Get("ignore_team").(*schema.Set).List()
 
-	tflog.Debug(ctx, "Creating repository collaborators", map[string]any{
-		"users":       users,
-		"teams":       teams,
-		"ignoreTeams": ignoreTeams,
-	})
+	tflog.Debug(ctx, "Creating repository collaborators", map[string]any{"users": users, "teams": teams, "ignoreTeams": ignoreTeams})
+
 	inUsers, err := getUserCollaborators(users)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
+	ownerConfigured := false
+	inIgnoreUsers := make([]string, 0)
+	if !isOrg {
+		ownerConfigured, _ = d.Get("owner_configured").(bool)
+
+		if !ownerConfigured {
+			for _, u := range inUsers {
+				if strings.EqualFold(u.login, owner) {
+					ownerConfigured = true
+					break
+				}
+			}
+
+			if !ownerConfigured {
+				inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
+			}
+
+			if err := d.Set("owner_configured", ownerConfigured); err != nil {
+				return diag.FromErr(err)
+			}
+		}
+	}
+
 	inTeams, err := getTeamCollaborators(teams)
 	if err != nil {
 		return diag.FromErr(err)
@@ -206,7 +311,7 @@ func resourceGithubRepositoryCollaboratorsCreate(ctx context.Context, d *schema.
 		return diag.FromErr(err)
 	}
 
-	invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers)
+	invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers, inIgnoreUsers)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -245,6 +350,12 @@ 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 && !ownerConfigured {
+		inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
+	}
 
 	inTeams, err := getTeamCollaborators(teams)
 	if err != nil {
@@ -256,7 +367,7 @@ func resourceGithubRepositoryCollaboratorsRead(ctx context.Context, d *schema.Re
 		return diag.FromErr(err)
 	}
 
-	ghUsers, err := listUserCollaborators(ctx, client, owner, repoName)
+	ghUsers, err := listUserCollaborators(ctx, client, owner, repoName, inIgnoreUsers)
 	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))
@@ -316,6 +427,29 @@ func resourceGithubRepositoryCollaboratorsUpdate(ctx context.Context, d *schema.
 		return diag.FromErr(err)
 	}
 
+	ownerConfigured := false
+	inIgnoreUsers := make([]string, 0)
+	if !isOrg {
+		ownerConfigured, _ = d.Get("owner_configured").(bool)
+
+		if !ownerConfigured {
+			for _, u := range inUsers {
+				if strings.EqualFold(u.login, owner) {
+					ownerConfigured = true
+					break
+				}
+			}
+
+			if !ownerConfigured {
+				inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
+			}
+
+			if err := d.Set("owner_configured", ownerConfigured); err != nil {
+				return diag.FromErr(err)
+			}
+		}
+	}
+
 	inTeams, err := getTeamCollaborators(teams)
 	if err != nil {
 		return diag.FromErr(err)
@@ -326,7 +460,7 @@ func resourceGithubRepositoryCollaboratorsUpdate(ctx context.Context, d *schema.
 		return diag.FromErr(err)
 	}
 
-	invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers)
+	invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers, inIgnoreUsers)
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -362,14 +496,12 @@ func resourceGithubRepositoryCollaboratorsDelete(ctx context.Context, d *schema.
 
 	tflog.Debug(ctx, fmt.Sprintf("Removing all collaborators from repository %s.", repoName))
 
-	_, err = updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, nil)
-	if err != nil {
+	if _, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, nil, nil); err != nil {
 		return diag.FromErr(err)
 	}
 
 	if isOrg {
-		err = updateTeamCollaborators(ctx, client, meta.id, owner, repoName, nil, inIgnoreTeams)
-		if err != nil {
+		if err := updateTeamCollaborators(ctx, client, meta.id, owner, repoName, nil, inIgnoreTeams); err != nil {
 			return diag.FromErr(err)
 		}
 	}
@@ -462,7 +594,7 @@ func getTeamCollaborators(col []any) (teamCollaborators, error) {
 
 		permission, ok := m["permission"].(string)
 		if !ok || len(permission) == 0 {
-			return nil, fmt.Errorf("team input must include 'permission'")
+			return nil, fmt.Errorf("team input must include permission")
 		}
 
 		collaborators[i] = teamCollaborator{
@@ -498,7 +630,7 @@ func getTeamIdentity(d any) (teamIdentity, error) {
 
 	o, ok := m["team_id"]
 	if !ok {
-		return teamIdentity{}, fmt.Errorf("team input must include 'team_id'")
+		return teamIdentity{}, fmt.Errorf("team input must include team_id")
 	}
 
 	id, ok := o.(string)
@@ -509,7 +641,7 @@ func getTeamIdentity(d any) (teamIdentity, error) {
 	return newLegacyTeamIdentity(id), nil
 }
 
-func listUserCollaborators(ctx context.Context, client *github.Client, owner, repoName string) (userCollaborators, error) {
+func listUserCollaborators(ctx context.Context, client *github.Client, owner, repoName string, ignoreUsers []string) (userCollaborators, error) {
 	col := make([]userCollaborator, 0)
 	tflog.Debug(ctx, "Listing user collaborators", map[string]any{
 		"owner":    owner,
@@ -531,6 +663,10 @@ func listUserCollaborators(ctx context.Context, client *github.Client, owner, re
 			}
 
 			for _, c := range collaborators {
+				if slices.Contains(ignoreUsers, strings.ToLower(c.GetLogin())) {
+					continue
+				}
+
 				col = append(col, userCollaborator{
 					userIdentity: userIdentity{
 						login: c.GetLogin(),
@@ -642,7 +778,7 @@ func listTeamCollaborators(ctx context.Context, client *github.Client, orgName,
 	return col, nil
 }
 
-func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Client, owner, repoName string, inUsers userCollaborators) (userCollaborators, error) {
+func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Client, owner, repoName string, inUsers userCollaborators, ignoreUsers []string) (userCollaborators, error) {
 	lookup := make(map[string]userCollaborator)
 	seen := make(map[string]any)
 	remove := make([]string, 0)
@@ -659,7 +795,7 @@ func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Clien
 		"remove":   remove,
 	})
 
-	ghUsers, err := listUserCollaborators(ctx, client, owner, repoName)
+	ghUsers, err := listUserCollaborators(ctx, client, owner, repoName, ignoreUsers)
 	if err != nil {
 		return nil, err
 	}
@@ -717,6 +853,7 @@ func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Clien
 		if err != nil {
 			return nil, err
 		}
+
 		// AddCollaborator returns 204 No Content (inv == nil) when the invitee
 		// is an organization member gaining direct access without an
 		// invitation. In that case there is no invitation ID to record.