fix: Stop repo collaborators drifting on owner#3471
Open
stevehipwell wants to merge 1 commit into
Open
Conversation
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
Contributor
There was a problem hiding this comment.
Pull request overview
These provider review instructions are being used.
This PR aims to eliminate perpetual drift in github_repository_collaborators for personal repositories by ignoring the implicit repository owner collaborator when the owner isn’t configured, and introduces an owner_configured computed attribute to track that behavior.
Changes:
- Adds an
owner_configuredcomputed attribute to thegithub_repository_collaboratorsschema and sets it during diffing. - Updates collaborator listing/reconciliation to optionally ignore the repository owner (to prevent owner-related drift on personal repos).
- Adds acceptance tests for the personal-repo owner drift scenarios and updates the generated docs schema output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
github/resource_github_repository_collaborators.go |
Adds owner_configured and uses an ignore list to filter the owner from collaborator reads/updates to prevent drift. |
github/resource_github_repository_collaborators_test.go |
Adds acceptance coverage for personal repositories to validate owner drift behavior. |
docs/resources/repository_collaborators.md |
Documents the new owner_configured read-only attribute in the generated schema section. |
Comment on lines
+285
to
+289
| inIgnoreUsers := make([]string, 0) | ||
| if !isOrg && !d.Get("owner_configured").(bool) { | ||
| inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner)) | ||
| } | ||
|
|
Comment on lines
+158
to
+162
| 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)), | ||
| }, |
Comment on lines
+196
to
+210
| 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(testAccConf.owner), | ||
| "permission": knownvalue.StringExact("admin"), | ||
| }), | ||
| })), | ||
| 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)), | ||
| }, | ||
| }, | ||
| }, |
Comment on lines
86
to
90
|
|
||
| - `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. |
deiga
requested changes
Jun 3, 2026
Comment on lines
+148
to
+152
| 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") { |
Collaborator
There was a problem hiding this comment.
suggestion: I think both of these code paths would benefit from a few lines of comments explaining what is going on here 🤔
Comment on lines
+360
to
+363
| inIgnoreUsers := make([]string, 0) | ||
| if !isOrg && !d.Get("owner_configured").(bool) { | ||
| inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner)) | ||
| } |
Collaborator
There was a problem hiding this comment.
suggestion: We use this only to determine what to pass into listUserCollaborators, but we have to do it 3 times. What if we move this logic into listUserCollaborators and only pass in owner_configured?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #3434
Before the change?
github_repository_collaboratorsresource drifts when the rpo is personally and the owner hasn't been configuredAfter the change?
github_repository_collaboratorsresource ignores the owner if no configuredowner_configuredcomputed attribute togithub_repository_collaboratorsresourcePull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!