Fix bug reading emr studio session mapping

.changelog/32416.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+resource/aws_emr_studio_session_mapping: Fix `InvalidRequestException: IdentityId is invalid` errors reading resources created with `identity_name`
+```

internal/service/emr/find.go

@@ -87,16 +87,21 @@ func FindStudioByID(ctx context.Context, conn *emr.EMR, id string) (*emr.Studio,
 	return output.Studio, nil
 }
 
-func FindStudioSessionMappingByID(ctx context.Context, conn *emr.EMR, id string) (*emr.SessionMappingDetail, error) {
-	studioId, identityType, identityId, err := readStudioSessionMapping(id)
+func FindStudioSessionMappingByIDOrName(ctx context.Context, conn *emr.EMR, id string) (*emr.SessionMappingDetail, error) {
+	studioId, identityType, identityIdOrName, err := readStudioSessionMapping(id)
 	if err != nil {
 		return nil, err
 	}
 
 	input := &emr.GetStudioSessionMappingInput{
 		StudioId:     aws.String(studioId),
 		IdentityType: aws.String(identityType),
-		IdentityId:   aws.String(identityId),
+	}
+
+	if isIdentityId(identityIdOrName) {
+		input.IdentityId = aws.String(identityIdOrName)
+	} else {
+		input.IdentityName = aws.String(identityIdOrName)
 	}
 
 	output, err := conn.GetStudioSessionMappingWithContext(ctx, input)

internal/service/emr/id.go

@@ -5,13 +5,29 @@ package emr
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 )
 
-func readStudioSessionMapping(id string) (studioId, identityType, identityId string, err error) {
-	idParts := strings.Split(id, ":")
-	if len(idParts) != 3 {
-		return "", "", "", fmt.Errorf("expected ID in format studio-id:identity-type:identity-id, received: %s", id)
+const IdentityIdPattern = `([0-9a-f]{10}-|)[A-Fa-f0-9]{8}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{4}-[A-Fa-f0-9]{12}`
+
+var IdentityIdPatternRegexp = regexp.MustCompile(IdentityIdPattern)
+
+func isIdentityId(identityIdOrName string) bool {
+	return IdentityIdPatternRegexp.MatchString(identityIdOrName)
+}
+
+func readStudioSessionMapping(id string) (studioId, identityType, identityIdOrName string, err error) {
+	idOrNameParts := strings.Split(id, ":")
+	if len(idOrNameParts) == 3 {
+		return idOrNameParts[0], idOrNameParts[1], idOrNameParts[2], nil
 	}
-	return idParts[0], idParts[1], idParts[2], nil
+
+	if isIdentityId(identityIdOrName) {
+		err = fmt.Errorf("expected ID in format studio-id:identity-type:identity-id, received: %s", identityIdOrName)
+	} else {
+		err = fmt.Errorf("expected ID in format studio-id:identity-type:identity-name, received: %s", identityIdOrName)
+	}
+
+	return "", "", "", err
 }

internal/service/emr/studio_session_mapping.go

@@ -70,7 +70,7 @@ func resourceStudioSessionMappingCreate(ctx context.Context, d *schema.ResourceD
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).EMRConn(ctx)
 
-	var id string
+	var idOrName string
 	studioId := d.Get("studio_id").(string)
 	identityType := d.Get("identity_type").(string)
 	input := &emr.CreateStudioSessionMappingInput{
@@ -81,20 +81,20 @@ func resourceStudioSessionMappingCreate(ctx context.Context, d *schema.ResourceD
 
 	if v, ok := d.GetOk("identity_id"); ok {
 		input.IdentityId = aws.String(v.(string))
-		id = v.(string)
+		idOrName = v.(string)
 	}
 
 	if v, ok := d.GetOk("identity_name"); ok {
 		input.IdentityName = aws.String(v.(string))
-		id = v.(string)
+		idOrName = v.(string)
 	}
 
 	_, err := conn.CreateStudioSessionMappingWithContext(ctx, input)
 	if err != nil {
 		return sdkdiag.AppendErrorf(diags, "creating EMR Studio Session Mapping: %s", err)
 	}
 
-	d.SetId(fmt.Sprintf("%s:%s:%s", studioId, identityType, id))
+	d.SetId(fmt.Sprintf("%s:%s:%s", studioId, identityType, idOrName))
 
 	return append(diags, resourceStudioSessionMappingRead(ctx, d, meta)...)
 }
@@ -103,7 +103,7 @@ func resourceStudioSessionMappingUpdate(ctx context.Context, d *schema.ResourceD
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).EMRConn(ctx)
 
-	studioId, identityType, identityId, err := readStudioSessionMapping(d.Id())
+	studioId, identityType, identityIdOrName, err := readStudioSessionMapping(d.Id())
 	if err != nil {
 		return sdkdiag.AppendErrorf(diags, "updating EMR Studio Session Mapping (%s): %s", d.Id(), err)
 	}
@@ -112,7 +112,12 @@ func resourceStudioSessionMappingUpdate(ctx context.Context, d *schema.ResourceD
 		SessionPolicyArn: aws.String(d.Get("session_policy_arn").(string)),
 		IdentityType:     aws.String(identityType),
 		StudioId:         aws.String(studioId),
-		IdentityId:       aws.String(identityId),
+	}
+
+	if isIdentityId(identityIdOrName) {
+		input.IdentityId = aws.String(identityIdOrName)
+	} else {
+		input.IdentityName = aws.String(identityIdOrName)
 	}
 
 	_, err = conn.UpdateStudioSessionMappingWithContext(ctx, input)
@@ -127,7 +132,7 @@ func resourceStudioSessionMappingRead(ctx context.Context, d *schema.ResourceDat
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).EMRConn(ctx)
 
-	mapping, err := FindStudioSessionMappingByID(ctx, conn, d.Id())
+	mapping, err := FindStudioSessionMappingByIDOrName(ctx, conn, d.Id())
 	if !d.IsNewResource() && tfresource.NotFound(err) {
 		log.Printf("[WARN] EMR Studio Session Mapping (%s) not found, removing from state", d.Id())
 		d.SetId("")
@@ -150,15 +155,21 @@ func resourceStudioSessionMappingRead(ctx context.Context, d *schema.ResourceDat
 func resourceStudioSessionMappingDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).EMRConn(ctx)
-	studioId, identityType, identityId, err := readStudioSessionMapping(d.Id())
+
+	studioId, identityType, identityIdOrName, err := readStudioSessionMapping(d.Id())
 	if err != nil {
 		return sdkdiag.AppendErrorf(diags, "deleting EMR Studio Session Mapping (%s): %s", d.Id(), err)
 	}
 
 	input := &emr.DeleteStudioSessionMappingInput{
 		IdentityType: aws.String(identityType),
 		StudioId:     aws.String(studioId),
-		IdentityId:   aws.String(identityId),
+	}
+
+	if isIdentityId(identityIdOrName) {
+		input.IdentityId = aws.String(identityIdOrName)
+	} else {
+		input.IdentityName = aws.String(identityIdOrName)
 	}
 
 	log.Printf("[INFO] Deleting EMR Studio Session Mapping: %s", d.Id())

internal/service/emr/studio_session_mapping_test.go

@@ -26,11 +26,13 @@ func TestAccEMRStudioSessionMapping_basic(t *testing.T) {
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	updatedName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	uName := os.Getenv("AWS_IDENTITY_STORE_USER_ID")
+	gName := os.Getenv("AWS_IDENTITY_STORE_GROUP_NAME")
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck: func() {
 			acctest.PreCheck(ctx, t)
 			testAccPreCheckUserID(t)
+			testAccPreCheckGroupName(t)
 		},
 		ErrorCheck:               acctest.ErrorCheck(t, emr.EndpointsID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
@@ -46,6 +48,16 @@ func TestAccEMRStudioSessionMapping_basic(t *testing.T) {
 					resource.TestCheckResourceAttrPair(resourceName, "session_policy_arn", "aws_iam_policy.test", "arn"),
 				),
 			},
+			{
+				Config: testAccStudioSessionMappingConfig_basic2(rName, gName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStudioSessionMappingExists(ctx, resourceName, &studio),
+					resource.TestCheckResourceAttr(resourceName, "identity_name", gName),
+					resource.TestCheckResourceAttr(resourceName, "identity_type", "GROUP"),
+					resource.TestCheckResourceAttrPair(resourceName, "studio_id", "aws_emr_studio.test", "id"),
+					resource.TestCheckResourceAttrPair(resourceName, "session_policy_arn", "aws_iam_policy.test", "arn"),
+				),
+			},
 			{
 				ResourceName:      resourceName,
 				ImportState:       true,
@@ -61,6 +73,16 @@ func TestAccEMRStudioSessionMapping_basic(t *testing.T) {
 					resource.TestCheckResourceAttrPair(resourceName, "session_policy_arn", "aws_iam_policy.test2", "arn"),
 				),
 			},
+			{
+				Config: testAccStudioSessionMappingConfig_updated2(rName, gName, updatedName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStudioSessionMappingExists(ctx, resourceName, &studio),
+					resource.TestCheckResourceAttr(resourceName, "identity_name", gName),
+					resource.TestCheckResourceAttr(resourceName, "identity_type", "GROUP"),
+					resource.TestCheckResourceAttrPair(resourceName, "studio_id", "aws_emr_studio.test", "id"),
+					resource.TestCheckResourceAttrPair(resourceName, "session_policy_arn", "aws_iam_policy.test2", "arn"),
+				),
+			},
 		},
 	})
 }
@@ -71,11 +93,13 @@ func TestAccEMRStudioSessionMapping_disappears(t *testing.T) {
 	resourceName := "aws_emr_studio_session_mapping.test"
 	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
 	uName := os.Getenv("AWS_IDENTITY_STORE_USER_ID")
+	gName := os.Getenv("AWS_IDENTITY_STORE_GROUP_NAME")
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck: func() {
 			acctest.PreCheck(ctx, t)
 			testAccPreCheckUserID(t)
+			testAccPreCheckGroupName(t)
 		},
 		ErrorCheck:               acctest.ErrorCheck(t, emr.EndpointsID),
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
@@ -90,6 +114,15 @@ func TestAccEMRStudioSessionMapping_disappears(t *testing.T) {
 				),
 				ExpectNonEmptyPlan: true,
 			},
+			{
+				Config: testAccStudioSessionMappingConfig_basic2(rName, gName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStudioSessionMappingExists(ctx, resourceName, &studio),
+					acctest.CheckResourceDisappears(ctx, acctest.Provider, tfemr.ResourceStudioSessionMapping(), resourceName),
+					acctest.CheckResourceDisappears(ctx, acctest.Provider, tfemr.ResourceStudioSessionMapping(), resourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
 		},
 	})
 }
@@ -103,7 +136,7 @@ func testAccCheckStudioSessionMappingExists(ctx context.Context, resourceName st
 
 		conn := acctest.Provider.Meta().(*conns.AWSClient).EMRConn(ctx)
 
-		output, err := tfemr.FindStudioSessionMappingByID(ctx, conn, rs.Primary.ID)
+		output, err := tfemr.FindStudioSessionMappingByIDOrName(ctx, conn, rs.Primary.ID)
 		if err != nil {
 			return err
 		}
@@ -127,7 +160,7 @@ func testAccCheckStudioSessionMappingDestroy(ctx context.Context) resource.TestC
 				continue
 			}
 
-			_, err := tfemr.FindStudioSessionMappingByID(ctx, conn, rs.Primary.ID)
+			_, err := tfemr.FindStudioSessionMappingByIDOrName(ctx, conn, rs.Primary.ID)
 			if tfresource.NotFound(err) {
 				continue
 			}
@@ -149,6 +182,13 @@ func testAccPreCheckUserID(t *testing.T) {
 	}
 }
 
+func testAccPreCheckGroupName(t *testing.T) {
+	if os.Getenv("AWS_IDENTITY_STORE_GROUP_NAME") == "" {
+		t.Skip("AWS_IDENTITY_STORE_GROUP_NAME env var must be set for AWS Identity Store Group acceptance test. " +
+			"This is required until ListGroups API returns results without filtering by name.")
+	}
+}
+
 func testAccStudioSessionMappingConfigBase(rName string) string {
 	return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
 data "aws_partition" "current" {}
@@ -168,11 +208,6 @@ resource "aws_s3_bucket" "test" {
   force_destroy = true
 }
 
-resource "aws_s3_bucket_acl" "test" {
-  bucket = aws_s3_bucket.test.id
-  acl    = "private"
-}
-
 resource "aws_iam_role" "test" {
   name               = %[1]q
   path               = "/"
@@ -264,6 +299,17 @@ resource "aws_emr_studio_session_mapping" "test" {
 `, uName))
 }
 
+func testAccStudioSessionMappingConfig_basic2(rName, gName string) string {
+	return acctest.ConfigCompose(testAccStudioSessionMappingConfigBase(rName), fmt.Sprintf(`
+resource "aws_emr_studio_session_mapping" "test" {
+  studio_id          = aws_emr_studio.test.id
+  identity_type      = "GROUP"
+  identity_name      = %[1]q
+  session_policy_arn = aws_iam_policy.test.arn
+}
+`, gName))
+}
+
 func testAccStudioSessionMappingConfig_updated(rName, uName, updatedName string) string {
 	return acctest.ConfigCompose(testAccStudioSessionMappingConfigBase(rName), fmt.Sprintf(`
 resource "aws_iam_policy" "test2" {
@@ -295,3 +341,35 @@ resource "aws_emr_studio_session_mapping" "test" {
 }
 `, uName, updatedName))
 }
+
+func testAccStudioSessionMappingConfig_updated2(rName, gName, updatedName string) string {
+	return acctest.ConfigCompose(testAccStudioSessionMappingConfigBase(rName), fmt.Sprintf(`
+resource "aws_iam_policy" "test2" {
+  name   = %[2]q
+  policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": [
+        "s3:*"
+      ],
+      "Resource": [
+        "${aws_s3_bucket.test.arn}/*",
+		"${aws_s3_bucket.test.arn}"
+      ]
+    }
+  ]
+}
+EOF
+}
+
+resource "aws_emr_studio_session_mapping" "test" {
+  studio_id          = aws_emr_studio.test.id
+  identity_type      = "GROUP"
+  identity_name      = %[1]q
+  session_policy_arn = aws_iam_policy.test2.arn
+}
+`, gName, updatedName))
+}