Skip to content

Commit 7b15eef

Browse files
stevehipwellstevehipwell
committed
fix: Stop repo collaborators drifting on owner
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
1 parent f494cc6 commit 7b15eef

3 files changed

Lines changed: 146 additions & 19 deletions

File tree

‎docs/resources/repository_collaborators.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ resource "github_repository_collaborators" "some_repo_collaborators" {
8686

8787
- `id` (String) The ID of this resource.
8888
- `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.
89+
- `owner_configured` (Boolean) Indicates whether the owner of a personal repository is configured as a collaborator.
8990
- `repository_id` (Number) ID of the repository.
9091

9192
<a id="nestedblock--ignore_team"></a>

‎github/resource_github_repository_collaborators.go‎

Lines changed: 68 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ func resourceGithubRepositoryCollaborators() *schema.Resource {
8989
},
9090
},
9191
},
92+
"owner_configured": {
93+
Type: schema.TypeBool,
94+
Computed: true,
95+
Description: "Indicates whether the owner of a personal repository is configured as a collaborator.",
96+
},
9297
"invitation_ids": {
9398
Type: schema.TypeMap,
9499
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,14 +126,17 @@ func resourceGithubRepositoryCollaborators() *schema.Resource {
121126
}
122127

123128
func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.ResourceDiff, m any) error {
124-
tflog.Debug(ctx, "Diffing user collaborators")
129+
tflog.Debug(ctx, "Diffing collaborators")
130+
131+
meta := m.(*Owner)
132+
125133
if d.HasChange("user") {
126134
users := d.Get("user").(*schema.Set).List()
127135
seen := make(map[string]any)
128136

129137
for _, u := range users {
130138
user := u.(map[string]any)
131-
username := user["username"].(string)
139+
username := strings.ToLower(user["username"].(string))
132140

133141
if _, ok := seen[username]; ok {
134142
return fmt.Errorf("duplicate username %s found in user collaborators", username)
@@ -137,6 +145,32 @@ func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.Re
137145
}
138146
}
139147

148+
if meta.IsOrganization {
149+
if err := d.SetNew("owner_configured", false); err != nil {
150+
return fmt.Errorf("error setting owner_configured: %w", err)
151+
}
152+
} else if d.NewValueKnown("user") {
153+
users := d.Get("user").(*schema.Set).List()
154+
ownerConfigured := false
155+
owner := strings.ToLower(meta.name)
156+
157+
for _, u := range users {
158+
user := u.(map[string]any)
159+
if strings.ToLower(user["username"].(string)) == owner {
160+
ownerConfigured = true
161+
break
162+
}
163+
}
164+
165+
if err := d.SetNew("owner_configured", ownerConfigured); err != nil {
166+
return fmt.Errorf("error setting owner_configured: %w", err)
167+
}
168+
} else {
169+
if err := d.SetNewComputed("owner_configured"); err != nil {
170+
return fmt.Errorf("error setting owner_configured to computed: %w", err)
171+
}
172+
}
173+
140174
if d.HasChange("team") && d.NewValueKnown("team") {
141175
v, diags := d.GetRawConfigAt(cty.GetAttrPath("team"))
142176
if diags.HasError() {
@@ -186,16 +220,18 @@ func resourceGithubRepositoryCollaboratorsCreate(ctx context.Context, d *schema.
186220
teams := d.Get("team").(*schema.Set).List()
187221
ignoreTeams := d.Get("ignore_team").(*schema.Set).List()
188222

189-
tflog.Debug(ctx, "Creating repository collaborators", map[string]any{
190-
"users": users,
191-
"teams": teams,
192-
"ignoreTeams": ignoreTeams,
193-
})
223+
tflog.Debug(ctx, "Creating repository collaborators", map[string]any{"users": users, "teams": teams, "ignoreTeams": ignoreTeams})
224+
194225
inUsers, err := getUserCollaborators(users)
195226
if err != nil {
196227
return diag.FromErr(err)
197228
}
198229

230+
inIgnoreUsers := make([]string, 0)
231+
if !isOrg && !d.Get("owner_configured").(bool) {
232+
inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
233+
}
234+
199235
inTeams, err := getTeamCollaborators(teams)
200236
if err != nil {
201237
return diag.FromErr(err)
@@ -206,7 +242,7 @@ func resourceGithubRepositoryCollaboratorsCreate(ctx context.Context, d *schema.
206242
return diag.FromErr(err)
207243
}
208244

209-
invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers)
245+
invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers, inIgnoreUsers)
210246
if err != nil {
211247
return diag.FromErr(err)
212248
}
@@ -246,6 +282,11 @@ func resourceGithubRepositoryCollaboratorsRead(ctx context.Context, d *schema.Re
246282
teams := d.Get("team").(*schema.Set).List()
247283
ignoreTeams := d.Get("ignore_team").(*schema.Set).List()
248284

285+
inIgnoreUsers := make([]string, 0)
286+
if !isOrg && !d.Get("owner_configured").(bool) {
287+
inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
288+
}
289+
249290
inTeams, err := getTeamCollaborators(teams)
250291
if err != nil {
251292
return diag.FromErr(err)
@@ -256,7 +297,7 @@ func resourceGithubRepositoryCollaboratorsRead(ctx context.Context, d *schema.Re
256297
return diag.FromErr(err)
257298
}
258299

259-
ghUsers, err := listUserCollaborators(ctx, client, owner, repoName)
300+
ghUsers, err := listUserCollaborators(ctx, client, owner, repoName, inIgnoreUsers)
260301
if err != nil {
261302
if err, ok := errors.AsType[*github.ErrorResponse](err); ok && err.Response.StatusCode == 404 {
262303
tflog.Debug(ctx, fmt.Sprintf("Repository %s not found when listing users, removing from state.", repoName))
@@ -316,6 +357,11 @@ func resourceGithubRepositoryCollaboratorsUpdate(ctx context.Context, d *schema.
316357
return diag.FromErr(err)
317358
}
318359

360+
inIgnoreUsers := make([]string, 0)
361+
if !isOrg && !d.Get("owner_configured").(bool) {
362+
inIgnoreUsers = append(inIgnoreUsers, strings.ToLower(owner))
363+
}
364+
319365
inTeams, err := getTeamCollaborators(teams)
320366
if err != nil {
321367
return diag.FromErr(err)
@@ -326,7 +372,7 @@ func resourceGithubRepositoryCollaboratorsUpdate(ctx context.Context, d *schema.
326372
return diag.FromErr(err)
327373
}
328374

329-
invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers)
375+
invitations, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, inUsers, inIgnoreUsers)
330376
if err != nil {
331377
return diag.FromErr(err)
332378
}
@@ -362,14 +408,12 @@ func resourceGithubRepositoryCollaboratorsDelete(ctx context.Context, d *schema.
362408

363409
tflog.Debug(ctx, fmt.Sprintf("Removing all collaborators from repository %s.", repoName))
364410

365-
_, err = updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, nil)
366-
if err != nil {
411+
if _, err := updateUserCollaboratorsAndInvites(ctx, client, owner, repoName, nil, nil); err != nil {
367412
return diag.FromErr(err)
368413
}
369414

370415
if isOrg {
371-
err = updateTeamCollaborators(ctx, client, meta.id, owner, repoName, nil, inIgnoreTeams)
372-
if err != nil {
416+
if err := updateTeamCollaborators(ctx, client, meta.id, owner, repoName, nil, inIgnoreTeams); err != nil {
373417
return diag.FromErr(err)
374418
}
375419
}
@@ -462,7 +506,7 @@ func getTeamCollaborators(col []any) (teamCollaborators, error) {
462506

463507
permission, ok := m["permission"].(string)
464508
if !ok || len(permission) == 0 {
465-
return nil, fmt.Errorf("team input must include 'permission'")
509+
return nil, fmt.Errorf("team input must include permission")
466510
}
467511

468512
collaborators[i] = teamCollaborator{
@@ -498,7 +542,7 @@ func getTeamIdentity(d any) (teamIdentity, error) {
498542

499543
o, ok := m["team_id"]
500544
if !ok {
501-
return teamIdentity{}, fmt.Errorf("team input must include 'team_id'")
545+
return teamIdentity{}, fmt.Errorf("team input must include team_id")
502546
}
503547

504548
id, ok := o.(string)
@@ -509,7 +553,7 @@ func getTeamIdentity(d any) (teamIdentity, error) {
509553
return newLegacyTeamIdentity(id), nil
510554
}
511555

512-
func listUserCollaborators(ctx context.Context, client *github.Client, owner, repoName string) (userCollaborators, error) {
556+
func listUserCollaborators(ctx context.Context, client *github.Client, owner, repoName string, ignoreUsers []string) (userCollaborators, error) {
513557
col := make([]userCollaborator, 0)
514558
tflog.Debug(ctx, "Listing user collaborators", map[string]any{
515559
"owner": owner,
@@ -531,6 +575,10 @@ func listUserCollaborators(ctx context.Context, client *github.Client, owner, re
531575
}
532576

533577
for _, c := range collaborators {
578+
if slices.Contains(ignoreUsers, strings.ToLower(c.GetLogin())) {
579+
continue
580+
}
581+
534582
col = append(col, userCollaborator{
535583
userIdentity: userIdentity{
536584
login: c.GetLogin(),
@@ -642,7 +690,7 @@ func listTeamCollaborators(ctx context.Context, client *github.Client, orgName,
642690
return col, nil
643691
}
644692

645-
func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Client, owner, repoName string, inUsers userCollaborators) (userCollaborators, error) {
693+
func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Client, owner, repoName string, inUsers userCollaborators, ignoreUsers []string) (userCollaborators, error) {
646694
lookup := make(map[string]userCollaborator)
647695
seen := make(map[string]any)
648696
remove := make([]string, 0)
@@ -659,7 +707,7 @@ func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Clien
659707
"remove": remove,
660708
})
661709

662-
ghUsers, err := listUserCollaborators(ctx, client, owner, repoName)
710+
ghUsers, err := listUserCollaborators(ctx, client, owner, repoName, ignoreUsers)
663711
if err != nil {
664712
return nil, err
665713
}
@@ -717,6 +765,7 @@ func updateUserCollaboratorsAndInvites(ctx context.Context, client *github.Clien
717765
if err != nil {
718766
return nil, err
719767
}
768+
720769
// AddCollaborator returns 204 No Content (inv == nil) when the invitee
721770
// is an organization member gaining direct access without an
722771
// invitation. In that case there is no invitation ID to record.

‎github/resource_github_repository_collaborators_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,83 @@ resource "github_repository_collaborators" "test" {
134134
})
135135
})
136136

137+
t.Run("personal_repo_ignores_owner_when_not_configured", func(t *testing.T) {
138+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
139+
repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID)
140+
141+
config := fmt.Sprintf(`
142+
resource "github_repository" "test" {
143+
name = "%s"
144+
visibility = "private"
145+
}
146+
147+
resource "github_repository_collaborators" "test" {
148+
repository = github_repository.test.name
149+
}
150+
`, repoName)
151+
152+
resource.Test(t, resource.TestCase{
153+
PreCheck: func() { skipUnlessMode(t, individual) },
154+
ProviderFactories: providerFactories,
155+
Steps: []resource.TestStep{
156+
{
157+
Config: config,
158+
ConfigStateChecks: []statecheck.StateCheck{
159+
statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetSizeExact(0)),
160+
statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(0)),
161+
statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)),
162+
},
163+
},
164+
{
165+
Config: config,
166+
PlanOnly: true,
167+
ExpectNonEmptyPlan: false,
168+
},
169+
},
170+
})
171+
})
172+
173+
t.Run("personal_repo_keeps_owner_when_configured", func(t *testing.T) {
174+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
175+
repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID)
176+
177+
config := fmt.Sprintf(`
178+
resource "github_repository" "test" {
179+
name = "%s"
180+
visibility = "private"
181+
}
182+
183+
resource "github_repository_collaborators" "test" {
184+
repository = github_repository.test.name
185+
186+
user {
187+
username = "%s"
188+
permission = "admin"
189+
}
190+
}
191+
`, repoName, testAccConf.owner)
192+
193+
resource.Test(t, resource.TestCase{
194+
PreCheck: func() { skipUnlessMode(t, individual) },
195+
ProviderFactories: providerFactories,
196+
Steps: []resource.TestStep{
197+
{
198+
Config: config,
199+
ConfigStateChecks: []statecheck.StateCheck{
200+
statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("user"), knownvalue.SetExact([]knownvalue.Check{
201+
knownvalue.MapExact(map[string]knownvalue.Check{
202+
"username": knownvalue.StringExact(testAccConf.owner),
203+
"permission": knownvalue.StringExact("admin"),
204+
}),
205+
})),
206+
statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("team"), knownvalue.SetSizeExact(0)),
207+
statecheck.ExpectKnownValue("github_repository_collaborators.test", tfjsonpath.New("invitation_ids"), knownvalue.MapSizeExact(0)),
208+
},
209+
},
210+
},
211+
})
212+
})
213+
137214
t.Run("update_teams", func(t *testing.T) {
138215
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
139216
repoName := fmt.Sprintf("%s%s", testResourcePrefix, randomID)

0 commit comments

Comments
 (0)