From 06e12f34155ece92ba9f6e47f7435d80fb64c1cd Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 14 Jul 2026 17:18:18 +0300 Subject: [PATCH 01/15] Ensure that set elements are only considered different if username changes Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index e56bf0ac38..63a6899d45 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -63,6 +63,10 @@ func resourceGithubTeamMembers() *schema.Resource { Type: schema.TypeSet, Required: true, Description: "List of users that should be members of the team.", + Set: func(v any) int { + username := v.(map[string]any)["username"].(string) + return schema.HashString(strings.ToLower(username)) + }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "username": { From d6b35ddea9e1a81461ffb061884186b60ae6bf1a Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 14 Jul 2026 17:20:42 +0300 Subject: [PATCH 02/15] Replace diff suppression with statefunc which stores username as lowercase Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index 63a6899d45..af659f5121 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -64,16 +64,19 @@ func resourceGithubTeamMembers() *schema.Resource { Required: true, Description: "List of users that should be members of the team.", Set: func(v any) int { - username := v.(map[string]any)["username"].(string) + username, _ := v.(map[string]any)["username"].(string) return schema.HashString(strings.ToLower(username)) }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, - DiffSuppressFunc: caseInsensitive(), - Description: "User to add to the team.", + Type: schema.TypeString, + Required: true, + StateFunc: func(v any) string { + val, _ := v.(string) + return strings.ToLower(val) + }, + Description: "User to add to the team.", }, "role": { Type: schema.TypeString, From cb100c8fdc358615e1137ba37e3bdc834ebae509 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 14 Jul 2026 21:11:08 +0300 Subject: [PATCH 03/15] Add test for case insensitivity Signed-off-by: Timo Sand --- github/resource_github_team_members_test.go | 64 +++++++++++++++++---- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 3911bce9fa..61e82353c0 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -3,6 +3,7 @@ package github import ( "fmt" "strconv" + "strings" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/resource" @@ -18,6 +19,7 @@ func TestAccGithubTeamMembers(t *testing.T) { skipUnlessHasOrgs(t) skipUnlessHasOrgUser1(t) + flippedCaseUsername := flipUsernameCase(testAccConf.testOrgUser1) t.Run("team_by_slug", func(t *testing.T) { t.Parallel() @@ -33,7 +35,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -71,7 +73,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -110,7 +112,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetID(), testAccConf.testOrgUser1) +`, team.GetID(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -148,7 +150,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) configMigrate := fmt.Sprintf(` resource "github_team_members" "test" { @@ -159,7 +161,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -201,7 +203,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetID(), testAccConf.testOrgUser1) +`, team.GetID(), flippedCaseUsername) configMigrate := fmt.Sprintf(` resource "github_team_members" "test" { @@ -212,7 +214,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -257,7 +259,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -296,7 +298,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -326,7 +328,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, testAccConf.testOrgUser1) +`, flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -364,7 +366,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), testAccConf.testOrgUser1) +`, team.GetSlug(), flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -403,7 +405,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, testAccConf.testOrgUser1) +`, flippedCaseUsername) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -430,4 +432,42 @@ resource "github_team_members" "test" { }, }) }) + + t.Run("is_case_insensitive", func(t *testing.T) { + t.Parallel() + + team := mustCreateTestTeam(t, nil) + + config := fmt.Sprintf(` +resource "github_team_members" "test" { + team_slug = "%s" + + members { + username = "%%s" + role = "maintainer" + } +} +`, team.GetSlug()) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, flippedCaseUsername), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(testAccConf.testOrgUser1))), + }, + }, + { + Config: fmt.Sprintf(config, testAccConf.testOrgUser1), + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(testAccConf.testOrgUser1))), + }, + }, + }, + }) + }) } From b67a82cfd72fec32dfa9b18109413df70ba11516 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 14 Jul 2026 21:12:58 +0300 Subject: [PATCH 04/15] re-adds DiffSuppressFunc Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 5 +++-- github/resource_github_team_members_test.go | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index af659f5121..c11aff9a89 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -70,8 +70,9 @@ func resourceGithubTeamMembers() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "username": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Required: true, + DiffSuppressFunc: caseInsensitive(), StateFunc: func(v any) string { val, _ := v.(string) return strings.ToLower(val) diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 61e82353c0..9be8883584 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -462,6 +462,11 @@ resource "github_team_members" "test" { }, { Config: fmt.Sprintf(config, testAccConf.testOrgUser1), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("github_team_members.test", plancheck.ResourceActionNoop), + }, + }, ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(testAccConf.testOrgUser1))), From b3a09a3eb45cfb10d3f658ab762cff328f417819 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 14 Jul 2026 21:17:27 +0300 Subject: [PATCH 05/15] Adds username comparer to test Signed-off-by: Timo Sand --- github/resource_github_team_members_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 9be8883584..68d0d47d99 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/hashicorp/terraform-plugin-testing/compare" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/knownvalue" "github.com/hashicorp/terraform-plugin-testing/plancheck" @@ -449,6 +450,7 @@ resource "github_team_members" "test" { } `, team.GetSlug()) + usernameIsSameComparer := statecheck.CompareValue(compare.ValuesSame()) resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, @@ -456,6 +458,7 @@ resource "github_team_members" "test" { { Config: fmt.Sprintf(config, flippedCaseUsername), ConfigStateChecks: []statecheck.StateCheck{ + usernameIsSameComparer.AddStateValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username")), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(testAccConf.testOrgUser1))), }, @@ -468,8 +471,8 @@ resource "github_team_members" "test" { }, }, ConfigStateChecks: []statecheck.StateCheck{ + usernameIsSameComparer.AddStateValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username")), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), - statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(testAccConf.testOrgUser1))), }, }, }, From 6064d0c7c0fdc28d4431cb472056b758ef153c06 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Wed, 22 Jul 2026 22:42:43 +0300 Subject: [PATCH 06/15] Adds test to verify that role updates still work (they didn't) Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 3 +- github/resource_github_team_members_test.go | 35 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index c11aff9a89..c72ca2f9d8 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -65,7 +65,8 @@ func resourceGithubTeamMembers() *schema.Resource { Description: "List of users that should be members of the team.", Set: func(v any) int { username, _ := v.(map[string]any)["username"].(string) - return schema.HashString(strings.ToLower(username)) + role, _ := v.(map[string]any)["role"].(string) + return schema.HashString(strings.ToLower(username) + ":" + strings.ToLower(role)) }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 68d0d47d99..fc0812491b 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -60,6 +60,41 @@ resource "github_team_members" "test" { }) }) + t.Run("updates_team_member_role", func(t *testing.T) { + t.Parallel() + + team := mustCreateTestTeam(t, nil) + + config := fmt.Sprintf(` +resource "github_team_members" "test" { + team_slug = "%s" + + members { + username = "%s" + role = "%%s" + } +} +`, team.GetSlug(), flippedCaseUsername) + + resource.Test(t, resource.TestCase{ + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, "maintainer"), + }, + { + Config: fmt.Sprintf(config, "member"), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("github_team_members.test", plancheck.ResourceActionUpdate), + plancheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("role"), knownvalue.StringExact("member")), + }, + }, + }, + }, + }) + }) + t.Run("team_by_id_as_slug", func(t *testing.T) { t.Parallel() From c2088ccdff828949131b4661aa976c91a281585d Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 24 Jul 2026 09:01:51 +0300 Subject: [PATCH 07/15] Renames import tests for clarity Signed-off-by: Timo Sand --- github/resource_github_team_members_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index fc0812491b..399f542e8e 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -22,7 +22,7 @@ func TestAccGithubTeamMembers(t *testing.T) { skipUnlessHasOrgUser1(t) flippedCaseUsername := flipUsernameCase(testAccConf.testOrgUser1) - t.Run("team_by_slug", func(t *testing.T) { + t.Run("imports_team_by_slug", func(t *testing.T) { t.Parallel() team := mustCreateTestTeam(t) @@ -95,7 +95,7 @@ resource "github_team_members" "test" { }) }) - t.Run("team_by_id_as_slug", func(t *testing.T) { + t.Run("imports_team_by_id_as_slug", func(t *testing.T) { t.Parallel() team := mustCreateTestTeam(t) @@ -134,7 +134,7 @@ resource "github_team_members" "test" { }) }) - t.Run("team_by_id", func(t *testing.T) { + t.Run("imports_team_by_id", func(t *testing.T) { t.Parallel() team := mustCreateTestTeam(t) From 8bddfa22f5e9d3a5db1fc21721dada4436c8518c Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 24 Jul 2026 09:04:53 +0300 Subject: [PATCH 08/15] Remove `Set` function as it doesn't provide enough benefit here Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index c72ca2f9d8..a1a2d40700 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -63,11 +63,6 @@ func resourceGithubTeamMembers() *schema.Resource { Type: schema.TypeSet, Required: true, Description: "List of users that should be members of the team.", - Set: func(v any) int { - username, _ := v.(map[string]any)["username"].(string) - role, _ := v.(map[string]any)["role"].(string) - return schema.HashString(strings.ToLower(username) + ":" + strings.ToLower(role)) - }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "username": { From 04fb53d844ce55ac570594a54dbff6bfee2fea8e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 24 Jul 2026 09:05:41 +0300 Subject: [PATCH 09/15] Add explanation for the need of `StateFunc` Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index a1a2d40700..8ebf986f9c 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -69,11 +69,13 @@ func resourceGithubTeamMembers() *schema.Resource { Type: schema.TypeString, Required: true, DiffSuppressFunc: caseInsensitive(), + Description: "User to add to the team.", + // This seems to be the only way to ensure that the username is in lowercase. + // Without this the tests fail because the value is compared in a case-sensitive manner. StateFunc: func(v any) string { val, _ := v.(string) return strings.ToLower(val) }, - Description: "User to add to the team.", }, "role": { Type: schema.TypeString, From 9038b057f728052787866645c89b0dc16a6e00b0 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 24 Jul 2026 09:06:56 +0300 Subject: [PATCH 10/15] Ensure removing team members also lowercases username Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index 8ebf986f9c..8d1f3029c2 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -406,11 +406,12 @@ func updateTeamMembers(ctx context.Context, meta *Owner, slug string, wantMember } for _, member := range currentMembers { - if _, ok := want[member.login]; !ok { - tflog.Debug(ctx, "Removing team member.", map[string]any{"team_slug": slug, "username": member.login}) + login := strings.ToLower(member.login) + if _, ok := want[login]; !ok { + tflog.Debug(ctx, "Removing team member.", map[string]any{"team_slug": slug, "username": login}) - if _, err := client.Teams.RemoveTeamMembershipBySlug(ctx, orgName, slug, member.login); err != nil { - return fmt.Errorf("could not remove existing team member %q: %w", member.login, err) + if _, err := client.Teams.RemoveTeamMembershipBySlug(ctx, orgName, slug, login); err != nil { + return fmt.Errorf("could not remove existing team member %q: %w", login, err) } } } From 0b54d7b73e3e24e39da5cd81e0682f456c767d34 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 24 Jul 2026 09:10:52 +0300 Subject: [PATCH 11/15] Only use `flipUsernameCase` in `is_case_insensitive` test Signed-off-by: Timo Sand --- github/resource_github_team_members_test.go | 30 ++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 399f542e8e..866121f2ff 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -20,7 +20,6 @@ func TestAccGithubTeamMembers(t *testing.T) { skipUnlessHasOrgs(t) skipUnlessHasOrgUser1(t) - flippedCaseUsername := flipUsernameCase(testAccConf.testOrgUser1) t.Run("imports_team_by_slug", func(t *testing.T) { t.Parallel() @@ -36,7 +35,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -74,7 +73,7 @@ resource "github_team_members" "test" { role = "%%s" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -109,7 +108,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -148,7 +147,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetID(), flippedCaseUsername) +`, team.GetID(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -186,7 +185,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) configMigrate := fmt.Sprintf(` resource "github_team_members" "test" { @@ -197,7 +196,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -239,7 +238,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetID(), flippedCaseUsername) +`, team.GetID(), testAccConf.testOrgUser1) configMigrate := fmt.Sprintf(` resource "github_team_members" "test" { @@ -250,7 +249,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -295,7 +294,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -334,7 +333,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -364,7 +363,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, flippedCaseUsername) +`, testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -402,7 +401,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, team.GetSlug(), flippedCaseUsername) +`, team.GetSlug(), testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -441,7 +440,7 @@ resource "github_team_members" "test" { role = "maintainer" } } -`, flippedCaseUsername) +`, testAccConf.testOrgUser1) resource.Test(t, resource.TestCase{ ProviderFactories: providerFactories, @@ -473,6 +472,7 @@ resource "github_team_members" "test" { t.Parallel() team := mustCreateTestTeam(t, nil) + flippedCaseUsername := flipUsernameCase(testAccConf.testOrgUser1) config := fmt.Sprintf(` resource "github_team_members" "test" { @@ -495,7 +495,7 @@ resource "github_team_members" "test" { ConfigStateChecks: []statecheck.StateCheck{ usernameIsSameComparer.AddStateValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username")), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), - statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(testAccConf.testOrgUser1))), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(flippedCaseUsername))), }, }, { From 2fca26a545c2f877cb9f61719d0a363b50be3deb Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Fri, 24 Jul 2026 09:14:04 +0300 Subject: [PATCH 12/15] Add tests for member changes Signed-off-by: Timo Sand --- github/acc_test.go | 10 +-- github/resource_github_team_members_test.go | 82 +++++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/github/acc_test.go b/github/acc_test.go index f984be6c76..98df8d069d 100644 --- a/github/acc_test.go +++ b/github/acc_test.go @@ -407,8 +407,8 @@ func skipUnlessHasOrgUser2(t *testing.T) { } } -// func skipUnlessHasOrgUser3(t *testing.T) { -// if testAccConf.testOrgUser3 == "" { -// t.Skip("Skipping as no test org user 3 is configured") -// } -// } +func skipUnlessHasOrgUser3(t *testing.T) { + if testAccConf.testOrgUser3 == "" { + t.Skip("Skipping as no test org user 3 is configured") + } +} diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 866121f2ff..9b7c8abcb2 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -303,12 +303,94 @@ resource "github_team_members" "test" { Config: config, ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), SetAbsent([]knownvalue.Check{ + knownvalue.MapPartial(map[string]knownvalue.Check{ + "username": knownvalue.StringExact(testAccConf.testOrgUser2), + }), + })), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetExact([]knownvalue.Check{ + knownvalue.MapPartial(map[string]knownvalue.Check{ + "username": knownvalue.StringExact(testAccConf.testOrgUser1), + }), + })), }, }, { PreConfig: func() { mustAddTeamMember(t, team, testAccConf.testOrgUser2) }, Config: config, ConfigStateChecks: []statecheck.StateCheck{ statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), SetAbsent([]knownvalue.Check{ + knownvalue.MapPartial(map[string]knownvalue.Check{ + "username": knownvalue.StringExact(testAccConf.testOrgUser2), + }), + })), + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetExact([]knownvalue.Check{ + knownvalue.MapPartial(map[string]knownvalue.Check{ + "username": knownvalue.StringExact(testAccConf.testOrgUser1), + }), + })), + }, + }, + }, + }) + }) + + t.Run("updates_team_members_changes", func(t *testing.T) { + t.Parallel() + + skipUnlessHasOrgUser2(t) + skipUnlessHasOrgUser3(t) + + team := mustCreateTestTeam(t, nil) + flippedCaseUsername2 := flipUsernameCase(testAccConf.testOrgUser2) + flippedCaseUsername3 := flipUsernameCase(testAccConf.testOrgUser3) + + memberConfig := ` + members { + username = "%s" + role = "%s" + } +` + + baseConfig := ` +resource "github_team_members" "test" { + team_slug = "%s" + + %s + %s + %s +}` + initialConfig := fmt.Sprintf(baseConfig, team.GetSlug(), fmt.Sprintf(memberConfig, testAccConf.testOrgUser1, "maintainer"), "", "") + + resource.Test(t, resource.TestCase{ + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: initialConfig, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), + }, + }, + { + Config: fmt.Sprintf(baseConfig, team.GetSlug(), fmt.Sprintf(memberConfig, testAccConf.testOrgUser1, "maintainer"), fmt.Sprintf(memberConfig, flippedCaseUsername2, "member"), fmt.Sprintf(memberConfig, flippedCaseUsername3, "member")), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("github_team_members.test", plancheck.ResourceActionUpdate), + }, + }, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(3)), + }, + }, + { + Config: fmt.Sprintf(baseConfig, team.GetSlug(), fmt.Sprintf(memberConfig, testAccConf.testOrgUser1, "maintainer"), fmt.Sprintf(memberConfig, flippedCaseUsername2, "member"), ""), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("github_team_members.test", plancheck.ResourceActionUpdate), + }, + }, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(2)), }, }, }, From 2a32e578917d4e6febb97ddf9209b81012e5ff8e Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 28 Jul 2026 19:33:43 +0300 Subject: [PATCH 13/15] Use same duplicate username check as in repo collaborators resource Signed-off-by: Timo Sand --- ...esource_github_repository_collaborators.go | 25 ++------------- github/resource_github_team_members.go | 22 ++++++++----- github/util_diff.go | 31 +++++++++++++++++++ 3 files changed, 48 insertions(+), 30 deletions(-) diff --git a/github/resource_github_repository_collaborators.go b/github/resource_github_repository_collaborators.go index b82322c4f8..bd018b3784 100644 --- a/github/resource_github_repository_collaborators.go +++ b/github/resource_github_repository_collaborators.go @@ -137,29 +137,8 @@ func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.Re meta, _ := m.(*Owner) - if d.HasChange("user") && d.NewValueKnown("user") { - v, diags := d.GetRawConfigAt(cty.GetAttrPath("user")) - if diags.HasError() { - return fmt.Errorf("error reading user config: %v", diags) - } - - if !v.IsNull() && v.IsKnown() { - seen := make(map[string]struct{}) - it := v.ElementIterator() - for it.Next() { - _, elem := it.Element() - val := elem.GetAttr("username") - if val.IsNull() || !val.IsKnown() { - continue - } - - username := strings.ToLower(val.AsString()) - if _, ok := seen[username]; ok { - return fmt.Errorf("duplicate user %s found in user collaborators", username) - } - seen[username] = struct{}{} - } - } + if err := diffNestedUsernameCheck(ctx, d, "user"); err != nil { + return fmt.Errorf("error diffing user config: %w", err) } if d.HasChange("team") && d.NewValueKnown("team") { diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index 8d1f3029c2..d4fa7ed3dd 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -28,7 +28,7 @@ func resourceGithubTeamMembers() *schema.Resource { StateContext: resourceGithubTeamMembersImport, }, - CustomizeDiff: customdiff.Sequence(diffLegacyTeamID, diffLegacyTeam), + CustomizeDiff: customdiff.Sequence(resourceGithubTeamMembersDiff, diffLegacyTeamID, diffLegacyTeam), SchemaVersion: 1, StateUpgraders: []schema.StateUpgrader{ @@ -70,12 +70,6 @@ func resourceGithubTeamMembers() *schema.Resource { Required: true, DiffSuppressFunc: caseInsensitive(), Description: "User to add to the team.", - // This seems to be the only way to ensure that the username is in lowercase. - // Without this the tests fail because the value is compared in a case-sensitive manner. - StateFunc: func(v any) string { - val, _ := v.(string) - return strings.ToLower(val) - }, }, "role": { Type: schema.TypeString, @@ -91,6 +85,20 @@ func resourceGithubTeamMembers() *schema.Resource { } } +func resourceGithubTeamMembersDiff(ctx context.Context, d *schema.ResourceDiff, m any) error { + tflog.Debug(ctx, "diffing team members") + + if err := diffNestedUsernameCheck(ctx, d, "members"); err != nil { + return fmt.Errorf("error diffing members config: %w", err) + } + + if d.Id() == "" { + return nil + } + + return nil +} + func resourceGithubTeamMembersCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { meta, _ := m.(*Owner) client := meta.v3client diff --git a/github/util_diff.go b/github/util_diff.go index fedf54898a..aaa03bbbe5 100644 --- a/github/util_diff.go +++ b/github/util_diff.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/google/go-github/v89/github" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -332,3 +333,33 @@ func suppressUnorderedListDiff(fieldKey string, f func(a, b any) int) schema.Sch return reflect.DeepEqual(oldList, newList) } } + +func diffNestedUsernameCheck(ctx context.Context, d *schema.ResourceDiff, fieldKey string) error { + tflog.Debug(ctx, "diffing nested username check", map[string]any{"field": fieldKey}) + if d.HasChange(fieldKey) && d.NewValueKnown(fieldKey) { + tflog.Trace(ctx, "field is changed and it's new value is known", map[string]any{"field": fieldKey, "new_value": d.Get(fieldKey)}) + v, diags := d.GetRawConfigAt(cty.GetAttrPath(fieldKey)) + if diags.HasError() { + return fmt.Errorf("error reading '%s' config: %v", fieldKey, diags) + } + + if !v.IsNull() && v.IsKnown() { + seen := make(map[string]struct{}) + it := v.ElementIterator() + for it.Next() { + _, elem := it.Element() + val := elem.GetAttr("username") + if val.IsNull() || !val.IsKnown() { + continue + } + + username := strings.ToLower(val.AsString()) + if _, ok := seen[username]; ok { + return fmt.Errorf("duplicate user '%s' found in '%s' collection", username, fieldKey) + } + seen[username] = struct{}{} + } + } + } + return nil +} From 1c08b0c65268a29c84132c279ef428fafc2ff915 Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Tue, 28 Jul 2026 19:34:47 +0300 Subject: [PATCH 14/15] Ensure that `members` set elements are considered equal based on lowercased username and role Signed-off-by: Timo Sand --- github/resource_github_team_members.go | 6 ++++ github/resource_github_team_members_test.go | 33 +++++++++++++++++---- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index d4fa7ed3dd..b4d06b2510 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -63,6 +63,12 @@ func resourceGithubTeamMembers() *schema.Resource { Type: schema.TypeSet, Required: true, Description: "List of users that should be members of the team.", + // The Set hash function ensures that the same user cannot be added to the team multiple times with different case. + Set: func(v any) int { + username, _ := v.(map[string]any)["username"].(string) + role, _ := v.(map[string]any)["role"].(string) + return schema.HashString("username:" + strings.ToLower(username) + ";role:" + strings.ToLower(role)) + }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "username": { diff --git a/github/resource_github_team_members_test.go b/github/resource_github_team_members_test.go index 9b7c8abcb2..4c1a6acdec 100644 --- a/github/resource_github_team_members_test.go +++ b/github/resource_github_team_members_test.go @@ -2,11 +2,10 @@ package github import ( "fmt" + "regexp" "strconv" - "strings" "testing" - "github.com/hashicorp/terraform-plugin-testing/compare" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/knownvalue" "github.com/hashicorp/terraform-plugin-testing/plancheck" @@ -567,17 +566,40 @@ resource "github_team_members" "test" { } `, team.GetSlug()) - usernameIsSameComparer := statecheck.CompareValue(compare.ValuesSame()) + duplicateConfig := fmt.Sprintf(` +resource "github_team_members" "test" { + team_slug = "%s" + + members { + username = "%%s" + role = "maintainer" + } + + members { + username = "%%s" + role = "member" + } +} +`, team.GetSlug()) + resource.Test(t, resource.TestCase{ PreCheck: func() { skipUnlessHasOrgs(t) }, ProviderFactories: providerFactories, Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(duplicateConfig, testAccConf.testOrgUser1, flippedCaseUsername), + PlanOnly: true, + ExpectError: regexp.MustCompile("duplicate user '.*?' found in 'members' collection"), + }, { Config: fmt.Sprintf(config, flippedCaseUsername), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("github_team_members.test", plancheck.ResourceActionCreate), + }, + }, ConfigStateChecks: []statecheck.StateCheck{ - usernameIsSameComparer.AddStateValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username")), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), - statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username"), knownvalue.StringExact(strings.ToLower(flippedCaseUsername))), }, }, { @@ -588,7 +610,6 @@ resource "github_team_members" "test" { }, }, ConfigStateChecks: []statecheck.StateCheck{ - usernameIsSameComparer.AddStateValue("github_team_members.test", tfjsonpath.New("members").AtSliceIndex(0).AtMapKey("username")), statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)), }, }, From 517eff0a1b684ea8c86acb2c59f1990c1ca6e38b Mon Sep 17 00:00:00 2001 From: Timo Sand Date: Sat, 22 Aug 2026 14:11:24 +0300 Subject: [PATCH 15/15] Addresses review comments Signed-off-by: Timo Sand --- ...esource_github_repository_collaborators.go | 2 +- github/resource_github_team_members.go | 19 ++++++++----------- github/util_diff.go | 2 +- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/github/resource_github_repository_collaborators.go b/github/resource_github_repository_collaborators.go index bd018b3784..4c6253dca5 100644 --- a/github/resource_github_repository_collaborators.go +++ b/github/resource_github_repository_collaborators.go @@ -137,7 +137,7 @@ func resourceGithubRepositoryCollaboratorsDiff(ctx context.Context, d *schema.Re meta, _ := m.(*Owner) - if err := diffNestedUsernameCheck(ctx, d, "user"); err != nil { + if err := diffDuplicateUsernameCheck(ctx, d, "user"); err != nil { return fmt.Errorf("error diffing user config: %w", err) } diff --git a/github/resource_github_team_members.go b/github/resource_github_team_members.go index b4d06b2510..d08efc98ba 100644 --- a/github/resource_github_team_members.go +++ b/github/resource_github_team_members.go @@ -12,7 +12,6 @@ import ( "github.com/google/go-github/v89/github" "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/shurcooL/githubv4" @@ -28,7 +27,7 @@ func resourceGithubTeamMembers() *schema.Resource { StateContext: resourceGithubTeamMembersImport, }, - CustomizeDiff: customdiff.Sequence(resourceGithubTeamMembersDiff, diffLegacyTeamID, diffLegacyTeam), + CustomizeDiff: resourceGithubTeamMembersDiff, SchemaVersion: 1, StateUpgraders: []schema.StateUpgrader{ @@ -63,12 +62,6 @@ func resourceGithubTeamMembers() *schema.Resource { Type: schema.TypeSet, Required: true, Description: "List of users that should be members of the team.", - // The Set hash function ensures that the same user cannot be added to the team multiple times with different case. - Set: func(v any) int { - username, _ := v.(map[string]any)["username"].(string) - role, _ := v.(map[string]any)["role"].(string) - return schema.HashString("username:" + strings.ToLower(username) + ";role:" + strings.ToLower(role)) - }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "username": { @@ -94,12 +87,16 @@ func resourceGithubTeamMembers() *schema.Resource { func resourceGithubTeamMembersDiff(ctx context.Context, d *schema.ResourceDiff, m any) error { tflog.Debug(ctx, "diffing team members") - if err := diffNestedUsernameCheck(ctx, d, "members"); err != nil { + if err := diffDuplicateUsernameCheck(ctx, d, "members"); err != nil { return fmt.Errorf("error diffing members config: %w", err) } - if d.Id() == "" { - return nil + if err := diffLegacyTeamID(ctx, d, m); err != nil { + return fmt.Errorf("error diffing legacy team ID: %w", err) + } + + if err := diffLegacyTeam(ctx, d, m); err != nil { + return fmt.Errorf("error diffing legacy team: %w", err) } return nil diff --git a/github/util_diff.go b/github/util_diff.go index aaa03bbbe5..8dd06281af 100644 --- a/github/util_diff.go +++ b/github/util_diff.go @@ -334,7 +334,7 @@ func suppressUnorderedListDiff(fieldKey string, f func(a, b any) int) schema.Sch } } -func diffNestedUsernameCheck(ctx context.Context, d *schema.ResourceDiff, fieldKey string) error { +func diffDuplicateUsernameCheck(ctx context.Context, d *schema.ResourceDiff, fieldKey string) error { tflog.Debug(ctx, "diffing nested username check", map[string]any{"field": fieldKey}) if d.HasChange(fieldKey) && d.NewValueKnown(fieldKey) { tflog.Trace(ctx, "field is changed and it's new value is known", map[string]any{"field": fieldKey, "new_value": d.Get(fieldKey)})