[BUG] Ensure github_emu_group_mapping behaves correctly if mapping changes upstream

github/resource_github_emu_group_mapping_migration.go

@@ -0,0 +1,67 @@
+package github
+
+import (
+	"context"
+	"net/http"
+	"strconv"
+
+	"github.com/hashicorp/terraform-plugin-log/tflog"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+)
+
+func resourceGithubEMUGroupMappingResourceV0() *schema.Resource {
+	return &schema.Resource{
+		Schema: map[string]*schema.Schema{
+			"team_slug": {
+				Type:        schema.TypeString,
+				Required:    true,
+				Description: "Slug of the GitHub team.",
+			},
+			"group_id": {
+				Type:        schema.TypeInt,
+				Required:    true,
+				Description: "Integer corresponding to the external group ID to be linked.",
+			},
+			"etag": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
+		},
+	}
+}
+
+func resourceGithubEMUGroupMappingInstanceStateUpgradeV0(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) {
+	orgName := meta.(*Owner).name
+	tflog.Trace(ctx, "GitHub EMU Group Mapping State before migration", map[string]any{"state": rawState, "owner": orgName})
+
+	client := meta.(*Owner).v3client
+
+	teamSlug := rawState["team_slug"].(string)
+	// We need to bypass the etag because we need to get the latest group
+	ctx = context.WithValue(ctx, ctxEtag, nil)
+	groupsList, resp, err := client.Teams.ListExternalGroupsForTeamBySlug(ctx, orgName, teamSlug)
+	if err != nil {
+		if resp != nil && (resp.StatusCode == http.StatusNotFound) {
+			// If the Group is not found, remove it from state
+			tflog.Info(ctx, "Removing EMU group mapping from state because team no longer exists in GitHub", map[string]any{
+				"resource_id": rawState["id"],
+			})
+			return nil, err
+		}
+		return nil, err
+	}
+	group := groupsList.Groups[0]
+	teamID, err := lookupTeamID(ctx, meta.(*Owner), teamSlug)
+	if err != nil {
+		return nil, err
+	}
+	rawState["team_id"] = teamID
+	resourceID, err := buildID(strconv.FormatInt(teamID, 10), teamSlug, strconv.FormatInt(group.GetGroupID(), 10))
+	if err != nil {
+		return nil, err
+	}
+	rawState["id"] = resourceID
+
+	tflog.Trace(ctx, "GitHub EMU Group Mapping State after migration", map[string]any{"state": rawState})
+	return rawState, nil
+}

github/resource_github_emu_group_mapping_migration_test.go

@@ -0,0 +1,127 @@
+package github
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"strconv"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-github/v82/github"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+)
+
+type (
+	currentStateFunc  func() map[string]any
+	expectedStateFunc func(t *testing.T) map[string]any
+)
+
+var (
+	testTeamID  = 432574718
+	testGroupID = 1234567890
+)
+
+func testResourceGithubEMUGroupMappingInstanceStateDataV0() map[string]any {
+	return map[string]any{
+		"id":        "teams/test-team/external-groups",
+		"team_slug": "test-team",
+		"group_id":  testGroupID,
+	}
+}
+
+func testResourceGithubEMUGroupMappingInstanceStateDataV1(t *testing.T) map[string]any {
+	v0 := testResourceGithubEMUGroupMappingInstanceStateDataV0()
+	v0["team_id"] = int64(testTeamID)
+	resourceID, err := buildID(strconv.Itoa(testTeamID), v0["team_slug"].(string), strconv.Itoa(v0["group_id"].(int)))
+	if err != nil {
+		t.Fatalf("error building resource ID: %s", err)
+	}
+	v0["id"] = resourceID
+	return v0
+}
+
+func buildMockResponsesForMigrationV0toV1() []*mockResponse {
+	return []*mockResponse{
+		{
+			ExpectedUri: fmt.Sprintf("/orgs/%s/teams/%s/external-groups", "test-org", "test-team"),
+			ExpectedHeaders: map[string]string{
+				"Accept": "application/vnd.github.v3+json",
+			},
+			ResponseBody: fmt.Sprintf(`
+{
+	"groups": [
+		{
+			"group_id": %d,
+			"group_name": "test-group",
+			"updated_at": "2021-01-24T11:31:04-06:00"
+		}
+	]
+}`, int64(testGroupID)),
+			StatusCode: 201,
+		},
+		{
+			ExpectedUri: fmt.Sprintf("/orgs/%s/teams/%s", "test-org", "test-team"),
+			ExpectedHeaders: map[string]string{
+				"Accept": "application/vnd.github.v3+json",
+			},
+			ResponseBody: fmt.Sprintf(`
+{
+	"id": %d
+}
+`, testTeamID),
+			StatusCode: 200,
+		},
+	}
+}
+
+func TestGithub_MigrateEMUGroupMappingsState(t *testing.T) {
+	t.Parallel()
+
+	meta := &Owner{
+		name: "test-org",
+	}
+
+	for _, d := range []struct {
+		testName           string
+		migrationFunc      schema.StateUpgradeFunc
+		rawState           currentStateFunc
+		want               expectedStateFunc
+		buildMockResponses func() []*mockResponse
+		shouldError        bool
+	}{
+		{
+			testName:           "migrates v0 to v1",
+			migrationFunc:      resourceGithubEMUGroupMappingInstanceStateUpgradeV0,
+			rawState:           testResourceGithubEMUGroupMappingInstanceStateDataV0,
+			want:               testResourceGithubEMUGroupMappingInstanceStateDataV1,
+			buildMockResponses: buildMockResponsesForMigrationV0toV1,
+			shouldError:        false,
+		},
+	} {
+		t.Run(d.testName, func(t *testing.T) {
+			t.Parallel()
+
+			ts := githubApiMock(d.buildMockResponses())
+			defer ts.Close()
+
+			httpCl := http.DefaultClient
+			httpCl.Transport = http.DefaultTransport
+
+			client := github.NewClient(httpCl)
+			u, _ := url.Parse(ts.URL + "/")
+			client.BaseURL = u
+			meta.v3client = client
+
+			currentState := d.rawState()
+			got, err := d.migrationFunc(t.Context(), currentState, meta)
+			expectedState := d.want(t)
+			if (err != nil) != d.shouldError {
+				t.Fatalf("unexpected error state: %s", err.Error())
+			}
+			if diff := cmp.Diff(expectedState, got); !d.shouldError && diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	}
+}

github/resource_github_emu_group_mapping_test.go

@@ -126,6 +126,36 @@ func TestAccGithubEMUGroupMapping(t *testing.T) {
 			},
 		})
 	})
+
+	t.Run("forces new when switching to different team", func(t *testing.T) {
+		t.Skip("Skipping this test because we don't have terraform-plugin-testing available yet.")
+		randomID := acctest.RandString(5)
+		teamName1 := fmt.Sprintf("%semu1-%s", testResourcePrefix, randomID)
+		teamName2 := fmt.Sprintf("%semu2-%s", testResourcePrefix, randomID)
+
+		resource.Test(t, resource.TestCase{
+			PreCheck:          func() { skipUnlessEnterprise(t) },
+			ProviderFactories: providerFactories,
+			CheckDestroy:      testAccCheckGithubEMUGroupMappingDestroy,
+			Steps: []resource.TestStep{
+				{
+					Config: testAccGithubEMUGroupMappingTwoTeamsConfig(teamName1, teamName2, groupID, "test1"),
+					Check: resource.ComposeTestCheckFunc(
+						resource.TestCheckResourceAttr("github_emu_group_mapping.test", "team_slug", teamName1),
+					),
+				},
+				{
+					Config: testAccGithubEMUGroupMappingTwoTeamsConfig(teamName1, teamName2, groupID, "test2"),
+					// ConfigPlanChecks: resource.ConfigPlanChecks{
+					// 	PreApply: []plancheck.PlanCheck{
+					// 		plancheckExpectKnownValues("github_emu_group_mapping.test", "team_slug", teamName2),
+					// 		plancheck.ExpectResourceAction("github_emu_group_mapping.test", plancheck.ResourceActionDestroyBeforeCreate), // Verify that ForceNew is triggered
+					// 	},
+					// },
+				},
+			},
+		})
+	})
 }
 
 func testAccCheckGithubEMUGroupMappingDestroy(s *terraform.State) error {
@@ -183,3 +213,20 @@ func testAccGithubEMUGroupMappingConfig(teamName string, groupID int) string {
 	}
 	`, teamName, groupID)
 }
+
+func testAccGithubEMUGroupMappingTwoTeamsConfig(teamName1, teamName2 string, groupID int, useTeam string) string {
+	return fmt.Sprintf(`
+	resource "github_team" "test1" {
+		name        = "%s"
+		description = "EMU group mapping test team 1"
+	}
+	resource "github_team" "test2" {
+		name        = "%s"
+		description = "EMU group mapping test team 2"
+	}
+	resource "github_emu_group_mapping" "test" {
+		team_slug = github_team.%s.slug
+		group_id  = %d
+	}
+	`, teamName1, teamName2, useTeam, groupID)
+}

github/util.go

@@ -332,3 +332,43 @@ func deleteResourceOn404AndSwallow304OtherwiseReturnError(err error, d *schema.R
 	}
 	return err
 }
+
+// Helper function to safely convert interface{} to int, handling both int and float64.
+func toInt(v any) int {
+	switch val := v.(type) {
+	case int:
+		return val
+	case float64:
+		return int(val)
+	case int64:
+		return int(val)
+	default:
+		return 0
+	}
+}
+
+// Helper function to safely convert interface{} to int64, handling both int and float64.
+func toInt64(v any) int64 {
+	switch val := v.(type) {
+	case int:
+		return int64(val)
+	case int64:
+		return val
+	case float64:
+		return int64(val)
+	default:
+		return 0
+	}
+}
+
+// lookupTeamID looks up the ID of a team by its slug.
+func lookupTeamID(ctx context.Context, meta *Owner, slug string) (int64, error) {
+	client := meta.v3client
+	owner := meta.name
+
+	team, _, err := client.Teams.GetTeamBySlug(ctx, owner, slug)
+	if err != nil {
+		return 0, err
+	}
+	return team.GetID(), nil
+}

github/util_rules.go

@@ -9,34 +9,6 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
-// Helper function to safely convert interface{} to int, handling both int and float64.
-func toInt(v any) int {
-	switch val := v.(type) {
-	case int:
-		return val
-	case float64:
-		return int(val)
-	case int64:
-		return int(val)
-	default:
-		return 0
-	}
-}
-
-// Helper function to safely convert interface{} to int64, handling both int and float64.
-func toInt64(v any) int64 {
-	switch val := v.(type) {
-	case int:
-		return int64(val)
-	case int64:
-		return val
-	case float64:
-		return int64(val)
-	default:
-		return 0
-	}
-}
-
 func toPullRequestMergeMethods(input any) []github.PullRequestMergeMethod {
 	value, ok := input.([]any)
 	if !ok || len(value) == 0 {

go.mod

@@ -1,9 +1,10 @@
 module github.com/integrations/terraform-provider-github/v6
 
-go 1.24.0
+go 1.24.4
 
 require (
 	github.com/go-jose/go-jose/v3 v3.0.4
+	github.com/google/go-cmp v0.7.0
 	github.com/google/go-github/v82 v82.0.0
 	github.com/google/uuid v1.6.0
 	github.com/hashicorp/go-cty v1.5.0
@@ -23,7 +24,6 @@ require (
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/fatih/color v1.18.0 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
-	github.com/google/go-cmp v0.7.0 // indirect
 	github.com/google/go-querystring v1.2.0 // indirect
 	github.com/hashicorp/errwrap v1.0.0 // indirect
 	github.com/hashicorp/go-checkpoint v0.5.0 // indirect

website/docs/r/emu_group_mapping.html.markdown

@@ -9,8 +9,6 @@ description: |-
 
 This resource manages mappings between external groups for enterprise managed users and GitHub teams. It wraps the [Teams#ExternalGroups API](https://docs.github.com/en/rest/reference/teams#external-groups). Note that this is a distinct resource from `github_team_sync_group_mapping`. `github_emu_group_mapping` is special to the Enterprise Managed User (EMU) external group feature, whereas `github_team_sync_group_mapping` is specific to Identity Provider Groups.
 
-!> **Warning:**: This resources `Read` function has a fundamental bug. It doesn't verify that the group is actually linked to the team. Someone could modify the linked group outside of Terraform and the resource would not detect it.
-
 ## Example Usage
 
 ```hcl
@@ -29,8 +27,8 @@ The following arguments are supported:
 
 ## Import
 
-GitHub EMU External Group Mappings can be imported using the external `group_id` and `team_slug` separated by a colon, e.g.
+GitHub EMU External Group Mappings can be imported using the `team_slug` and external `group_id` separated by a colon, e.g.
 
 ```sh
-$ terraform import github_emu_group_mapping.example_emu_group_mapping 28836:emu-test-team
+$ terraform import github_emu_group_mapping.example_emu_group_mapping emu-test-team:28836
 ```