[BUG]: fix lowercase permadiff team members

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")
+	}
+}

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 := diffDuplicateUsernameCheck(ctx, d, "user"); err != nil {
+		return fmt.Errorf("error diffing user config: %w", err)
 	}
 
 	if d.HasChange("team") && d.NewValueKnown("team") {

github/resource_github_team_members.go

@@ -12,7 +12,6 @@ import (
 	"github.com/google/go-github/v92/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(diffLegacyTeamID, diffLegacyTeam),
+		CustomizeDiff: resourceGithubTeamMembersDiff,
 
 		SchemaVersion: 1,
 		StateUpgraders: []schema.StateUpgrader{
@@ -63,6 +62,13 @@ func resourceGithubTeamMembers() *schema.Resource {
 				Type:        schema.TypeSet,
 				Required:    true,
 				Description: "List of users that should be members of the team.",
+				// Set identity ignores username case so a case-only config change is a no-op;
+				// duplicates are rejected by diffDuplicateUsernameCheck.
+				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": {
@@ -85,6 +91,24 @@ func resourceGithubTeamMembers() *schema.Resource {
 	}
 }
 
+func resourceGithubTeamMembersDiff(ctx context.Context, d *schema.ResourceDiff, m any) error {
+	tflog.Debug(ctx, "diffing team members")
+
+	if err := diffDuplicateUsernameCheck(ctx, d, "members"); err != nil {
+		return fmt.Errorf("error diffing members config: %w", err)
+	}
+
+	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
+}
+
 func resourceGithubTeamMembersCreate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
 	meta, _ := m.(*Owner)
 	client := meta.v3client
@@ -400,11 +424,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)
 			}
 		}
 	}

github/resource_github_team_members_test.go

@@ -2,6 +2,7 @@ package github
 
 import (
 	"fmt"
+	"regexp"
 	"strconv"
 	"testing"
 
@@ -19,7 +20,7 @@ func TestAccGithubTeamMembers(t *testing.T) {
 	skipUnlessHasOrgs(t)
 	skipUnlessHasOrgUser1(t)
 
-	t.Run("team_by_slug", func(t *testing.T) {
+	t.Run("imports_team_by_slug", func(t *testing.T) {
 		t.Parallel()
 
 		team := mustCreateTestTeam(t)
@@ -57,7 +58,42 @@ resource "github_team_members" "test" {
 		})
 	})
 
-	t.Run("team_by_id_as_slug", func(t *testing.T) {
+	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(), testAccConf.testOrgUser1)
+
+		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("imports_team_by_id_as_slug", func(t *testing.T) {
 		t.Parallel()
 
 		team := mustCreateTestTeam(t)
@@ -96,7 +132,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)
@@ -266,12 +302,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)),
 					},
 				},
 			},
@@ -430,4 +548,72 @@ resource "github_team_members" "test" {
 			},
 		})
 	})
+
+	t.Run("is_case_insensitive", func(t *testing.T) {
+		t.Parallel()
+
+		team := mustCreateTestTeam(t, nil)
+		flippedCaseUsername := flipUsernameCase(testAccConf.testOrgUser1)
+
+		config := fmt.Sprintf(`
+resource "github_team_members" "test" {
+  team_slug = "%s"
+
+  members {
+    username = "%%s"
+    role     = "maintainer"
+  }
+}
+`, team.GetSlug())
+
+		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{
+						statecheck.ExpectKnownValue("github_team_members.test", tfjsonpath.New("members"), knownvalue.SetSizeExact(1)),
+					},
+				},
+				{
+					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)),
+					},
+				},
+			},
+		})
+	})
 }

github/util_diff.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/google/go-github/v92/github"
+	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
@@ -337,3 +338,33 @@ func suppressUnorderedListDiff(fieldKey string, f func(a, b any) int) schema.Sch
 		return reflect.DeepEqual(oldList, newList)
 	}
 }
+
+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)})
+		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
+}