From 8b8f5f489c9f3234ad279da3ca807b8119ac7e89 Mon Sep 17 00:00:00 2001 From: Nitin Jain Date: Sat, 30 May 2026 21:50:03 +0200 Subject: [PATCH 1/2] fix: skip github_team_members deletion when parent team is already gone When a github_team is destroyed alongside github_team_members, the team deletion removes all members server-side. Subsequent per-member API calls return 404 and fail loudly. Handle 404 from both getTeamID (slug-based lookup) and RemoveTeamMembershipByID (numeric ID path) by treating them as a no-op and returning early, consistent with how other resources handle missing parents. Fixes #3411 --- github/resource_github_team_members.go | 25 ++++++++++- github/resource_github_team_members_test.go | 49 +++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index e30c9ed1ec..adea478697 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -2,12 +2,15 @@ package github import ( "context" + "errors" "log" + "net/http" "reflect" "strconv" "strings" "github.com/google/go-github/v88/github" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/shurcooL/githubv4" @@ -187,6 +190,11 @@ func resourceGithubTeamMembersRead(ctx context.Context, d *schema.ResourceData, teamSlug, err := getTeamSlug(ctx, meta, teamIdString) if err != nil { + if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response.StatusCode == http.StatusNotFound { + tflog.Info(ctx, "Team no longer exists, removing from state", map[string]any{"team_id": teamIdString}) + d.SetId("") + return nil + } return diag.FromErr(err) } @@ -201,7 +209,8 @@ func resourceGithubTeamMembersRead(ctx context.Context, d *schema.ResourceData, var q struct { Organization struct { Team struct { - Members struct { + DatabaseId int + Members struct { Edges []struct { Node struct { Login string @@ -229,6 +238,12 @@ func resourceGithubTeamMembersRead(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } + if q.Organization.Team.DatabaseId == 0 { + tflog.Info(ctx, "Team no longer exists, removing from state", map[string]any{"team_id": teamIdString}) + d.SetId("") + return nil + } + // Add all members to the list for _, member := range q.Organization.Team.Members.Edges { teamMembersAndMaintainers = append(teamMembersAndMaintainers, map[string]any{ @@ -257,6 +272,10 @@ func resourceGithubTeamMembersDelete(ctx context.Context, d *schema.ResourceData teamIdString := d.Get("team_id").(string) teamId, err := getTeamID(ctx, meta, teamIdString) if err != nil { + if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response.StatusCode == http.StatusNotFound { + tflog.Info(ctx, "Team no longer exists, skipping member deletion", map[string]any{"team_id": teamIdString}) + return nil + } return diag.FromErr(err) } @@ -270,6 +289,10 @@ func resourceGithubTeamMembersDelete(ctx context.Context, d *schema.ResourceData _, err = client.Teams.RemoveTeamMembershipByID(ctx, orgId, teamId, username) if err != nil { + if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response.StatusCode == http.StatusNotFound { + tflog.Info(ctx, "Team no longer exists, skipping remaining member deletions", map[string]any{"team_id": teamIdString}) + return nil + } return diag.FromErr(err) } } diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index cc0b559311..9bd20f6b99 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -75,6 +75,55 @@ resource "github_team_members" "test" { }, }) }) + + t.Run("succeeds when parent team is deleted out-of-band before destroy", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + teamName := fmt.Sprintf("%steam-members-%s", testResourcePrefix, randomID) + + config := fmt.Sprintf(` +resource "github_team" "test" { + name = "%s" +} + +resource "github_team_members" "test" { + team_id = github_team.test.id + + members { + username = "%s" + role = "maintainer" + } +} +`, teamName, testAccConf.testOrgUser) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("github_team_members.test", "members.#", "1"), + ), + }, + { + // Delete the team out-of-band so it is gone before Terraform runs its destroy. + // The github_team_members delete should no-op instead of returning a 404 error. + PreConfig: func() { + meta, err := getTestMeta() + if err != nil { + t.Fatal(err.Error()) + } + ctx := context.Background() + if _, err := meta.v3client.Teams.DeleteTeamBySlug(ctx, meta.name, teamName); err != nil { + t.Fatalf("failed to delete team out-of-band: %s", err) + } + }, + Config: config, + Destroy: true, + }, + }, + }) + }) } func testAccCheckGithubTeamMembersDestroy(s *terraform.State) error { From 9e1e98fa5cab403c9d8dacadf08edd60ec928ea1 Mon Sep 17 00:00:00 2001 From: Nitin Jain Date: Thu, 4 Jun 2026 11:44:51 +0200 Subject: [PATCH 2/2] fix: address Copilot review feedback on team members delete and test coverage - Delete: on 404 from RemoveTeamMembershipByID, verify whether the team itself is gone before short-circuiting; if team still exists the membership was removed out-of-band so skip that user and continue the loop rather than skipping all remaining members - Test: add RefreshState step after out-of-band team deletion to assert github_team_members is removed from state by Read, covering the new getTeamSlug 404 and DatabaseId==0 guard paths --- github/resource_github_team_members.go | 14 +++++++-- github/resource_github_team_members_test.go | 35 +++++++++++++-------- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index adea478697..1ab7901566 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -290,8 +290,18 @@ func resourceGithubTeamMembersDelete(ctx context.Context, d *schema.ResourceData _, err = client.Teams.RemoveTeamMembershipByID(ctx, orgId, teamId, username) if err != nil { if ghErr, ok := errors.AsType[*github.ErrorResponse](err); ok && ghErr.Response.StatusCode == http.StatusNotFound { - tflog.Info(ctx, "Team no longer exists, skipping remaining member deletions", map[string]any{"team_id": teamIdString}) - return nil + // 404 can mean the membership was already removed out-of-band (drift) + // or the team itself is gone. Check the team to distinguish. + _, _, teamErr := client.Teams.GetTeamByID(ctx, orgId, teamId) //nolint:staticcheck + if teamErr != nil { + if teamGhErr, ok := errors.AsType[*github.ErrorResponse](teamErr); ok && teamGhErr.Response.StatusCode == http.StatusNotFound { + tflog.Info(ctx, "Team no longer exists, skipping remaining member deletions", map[string]any{"team_id": teamIdString}) + return nil + } + } + // Team still exists — this membership was removed out-of-band, continue. + tflog.Info(ctx, "Team membership no longer exists, skipping", map[string]any{"team_id": teamIdString, "username": username}) + continue } return diag.FromErr(err) } diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 9bd20f6b99..009cc8001b 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -76,7 +76,7 @@ resource "github_team_members" "test" { }) }) - t.Run("succeeds when parent team is deleted out-of-band before destroy", func(t *testing.T) { + t.Run("removes from state and succeeds destroy when parent team is deleted out-of-band", func(t *testing.T) { randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) teamName := fmt.Sprintf("%steam-members-%s", testResourcePrefix, randomID) @@ -95,6 +95,17 @@ resource "github_team_members" "test" { } `, teamName, testAccConf.testOrgUser) + deleteTeamOutOfBand := func() { + meta, err := getTestMeta() + if err != nil { + t.Fatal(err.Error()) + } + ctx := context.Background() + if _, err := meta.v3client.Teams.DeleteTeamBySlug(ctx, meta.name, teamName); err != nil { + t.Fatalf("failed to delete team out-of-band: %s", err) + } + } + resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, @@ -106,18 +117,16 @@ resource "github_team_members" "test" { ), }, { - // Delete the team out-of-band so it is gone before Terraform runs its destroy. - // The github_team_members delete should no-op instead of returning a 404 error. - PreConfig: func() { - meta, err := getTestMeta() - if err != nil { - t.Fatal(err.Error()) - } - ctx := context.Background() - if _, err := meta.v3client.Teams.DeleteTeamBySlug(ctx, meta.name, teamName); err != nil { - t.Fatalf("failed to delete team out-of-band: %s", err) - } - }, + // Delete the team out-of-band then refresh — Read should detect the + // missing team and remove github_team_members from state without error. + // Plan is non-empty because config still defines both resources. + PreConfig: deleteTeamOutOfBand, + RefreshState: true, + ExpectNonEmptyPlan: true, + }, + { + // Destroy step: resource already removed from state by Read above, + // destroy should complete cleanly with no 404 errors. Config: config, Destroy: true, },