Skip to content

Commit 614a772

Browse files
fix: read immediate github_team_members only, not child-team members (#3498)
The Read path (getTeamMembers) used the REST list-team-members endpoint, which always includes members inherited from child teams and has no immediate-only option. For organizations with nested teams this surfaced inherited members as drift, and broke the github_team_members member set that is meant to be authoritative. Switch getTeamMembers to the GraphQL API with membership:IMMEDIATE so only direct members are returned. A null team node is mapped to a 404 github.ErrorResponse so the existing Read/Delete handling still removes the resource from state when the team is deleted out-of-band. getTeamMembers and updateTeamMembers now take *Owner so they can reach the v4 (GraphQL) client for reads while still using the v3 client for membership add/remove. Fixes the team_with_child_team regression test added in #3499. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 64da252 commit 614a772

1 file changed

Lines changed: 63 additions & 19 deletions

File tree

github/resource_github_team_members.go

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
1616
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1717
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
18+
"github.com/shurcooL/githubv4"
1819
)
1920

2021
func resourceGithubTeamMembers() *schema.Resource {
@@ -124,7 +125,7 @@ func resourceGithubTeamMembersCreate(ctx context.Context, d *schema.ResourceData
124125
return diag.FromErr(fmt.Errorf("failed to parse team members: %w", err))
125126
}
126127

127-
if err := updateTeamMembers(ctx, client, orgName, slug, teamMembers); err != nil {
128+
if err := updateTeamMembers(ctx, meta, slug, teamMembers); err != nil {
128129
return diag.FromErr(err)
129130
}
130131

@@ -167,7 +168,7 @@ func resourceGithubTeamMembersRead(ctx context.Context, d *schema.ResourceData,
167168

168169
tflog.Debug(ctx, "Reading team members.", map[string]any{"team_slug": slug})
169170

170-
teamMembers, err := getTeamMembers(ctx, client, orgName, slug)
171+
teamMembers, err := getTeamMembers(ctx, meta, slug)
171172
if err != nil {
172173
if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response.StatusCode == http.StatusNotFound {
173174
tflog.Info(ctx, "Team not found during read, removing from state.", map[string]any{"team_slug": slug})
@@ -209,7 +210,7 @@ func resourceGithubTeamMembersUpdate(ctx context.Context, d *schema.ResourceData
209210
return diag.FromErr(fmt.Errorf("failed to parse team members: %w", err))
210211
}
211212

212-
if err := updateTeamMembers(ctx, client, orgName, slug, teamMembers); err != nil {
213+
if err := updateTeamMembers(ctx, meta, slug, teamMembers); err != nil {
213214
return diag.FromErr(err)
214215
}
215216

@@ -218,14 +219,12 @@ func resourceGithubTeamMembersUpdate(ctx context.Context, d *schema.ResourceData
218219

219220
func resourceGithubTeamMembersDelete(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
220221
meta := m.(*Owner)
221-
client := meta.v3client
222-
orgName := meta.name
223222

224223
slug, _ := d.Get("team_slug").(string)
225224

226225
tflog.Debug(ctx, "Removing all members from team.", map[string]any{"team_slug": slug})
227226

228-
if err := updateTeamMembers(ctx, client, orgName, slug, nil); err != nil {
227+
if err := updateTeamMembers(ctx, meta, slug, nil); err != nil {
229228
if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response.StatusCode == http.StatusNotFound {
230229
tflog.Info(ctx, "Team not found during delete, assuming already removed.", map[string]any{"team_slug": slug})
231230
return nil
@@ -271,7 +270,7 @@ func resourceGithubTeamMembersImport(ctx context.Context, d *schema.ResourceData
271270
}
272271
}
273272

274-
teamMembers, err := getTeamMembers(ctx, meta.v3client, meta.name, slug)
273+
teamMembers, err := getTeamMembers(ctx, meta, slug)
275274
if err != nil {
276275
return nil, err
277276
}
@@ -289,36 +288,81 @@ func resourceGithubTeamMembersImport(ctx context.Context, d *schema.ResourceData
289288
return []*schema.ResourceData{d}, nil
290289
}
291290

292-
func getTeamMembers(ctx context.Context, client *github.Client, orgName, slug string) (userMembers, error) {
291+
func getTeamMembers(ctx context.Context, meta *Owner, slug string) (userMembers, error) {
293292
tflog.Debug(ctx, "Getting team members.", map[string]any{"team_slug": slug})
294293

294+
// Use the GraphQL API with membership:IMMEDIATE so that only direct members
295+
// of the team are returned. The REST list-members endpoint always includes
296+
// members inherited from child teams (it has no immediate-only option), which
297+
// would otherwise surface as drift for organizations that use nested teams.
298+
// See https://github.com/integrations/terraform-provider-github/issues/3497
299+
var query struct {
300+
Organization struct {
301+
Team *struct {
302+
Members struct {
303+
Edges []struct {
304+
Node struct {
305+
Login string
306+
}
307+
Role string
308+
}
309+
PageInfo struct {
310+
EndCursor githubv4.String
311+
HasNextPage bool
312+
}
313+
} `graphql:"members(membership: IMMEDIATE, first: 100, after: $after)"`
314+
} `graphql:"team(slug: $slug)"`
315+
} `graphql:"organization(login: $login)"`
316+
}
317+
318+
variables := map[string]any{
319+
"login": githubv4.String(meta.name),
320+
"slug": githubv4.String(slug),
321+
"after": (*githubv4.String)(nil),
322+
}
323+
295324
var teamMembers userMembers
296-
for _, role := range []string{"maintainer", "member"} {
297-
opts := &github.TeamListTeamMembersOptions{
298-
Role: role,
299-
ListOptions: github.ListOptions{PerPage: maxPerPage},
325+
for {
326+
if err := meta.v4client.Query(ctx, &query, variables); err != nil {
327+
return nil, err
300328
}
301329

302-
for member, err := range client.Teams.ListTeamMembersBySlugIter(ctx, orgName, slug, opts) {
303-
if err != nil {
304-
return nil, err
330+
// A null team means it no longer exists (e.g. deleted out-of-band). The
331+
// GraphQL API returns a null node rather than an error in this case, so
332+
// surface a 404 to match the REST endpoints and let callers remove the
333+
// resource from state.
334+
if query.Organization.Team == nil {
335+
return nil, &github.ErrorResponse{
336+
Response: &http.Response{StatusCode: http.StatusNotFound},
337+
Message: fmt.Sprintf("team %q not found in organization %q", slug, meta.name),
305338
}
339+
}
306340

341+
for _, edge := range query.Organization.Team.Members.Edges {
307342
teamMembers = append(teamMembers, userMember{
308343
userIdentity: userIdentity{
309-
login: strings.ToLower(member.GetLogin()),
344+
login: strings.ToLower(edge.Node.Login),
310345
},
311-
role: role,
346+
role: strings.ToLower(edge.Role),
312347
})
313348
}
349+
350+
if !query.Organization.Team.Members.PageInfo.HasNextPage {
351+
break
352+
}
353+
354+
variables["after"] = query.Organization.Team.Members.PageInfo.EndCursor
314355
}
315356

316357
tflog.Debug(ctx, "Got team members.", map[string]any{"team_slug": slug, "team_members": teamMembers})
317358

318359
return teamMembers, nil
319360
}
320361

321-
func updateTeamMembers(ctx context.Context, client *github.Client, orgName, slug string, wantMembers userMembers) error {
362+
func updateTeamMembers(ctx context.Context, meta *Owner, slug string, wantMembers userMembers) error {
363+
client := meta.v3client
364+
orgName := meta.name
365+
322366
tflog.Debug(ctx, "Updating team members.", map[string]any{"team_slug": slug, "members": wantMembers.flatten()})
323367

324368
roleLookup := map[string]int{
@@ -330,7 +374,7 @@ func updateTeamMembers(ctx context.Context, client *github.Client, orgName, slug
330374
return roleLookup[a.role] - roleLookup[b.role]
331375
})
332376

333-
currentMembers, err := getTeamMembers(ctx, client, orgName, slug)
377+
currentMembers, err := getTeamMembers(ctx, meta, slug)
334378
if err != nil {
335379
return err
336380
}

0 commit comments

Comments
 (0)