Skip to content

Commit

Permalink
Merge pull request #32416 from Yunsang-Jeong/f-aws_emr_studio_session…
Browse files Browse the repository at this point in the history 
…_mapping

Fix bug reading emr studio session mapping
  • Loading branch information
@ewbankkit
ewbankkit authored Jul 28, 2023
2 parents 0b08c9f + 466b84e commit d1a767a
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 24 deletions.
3 changes: 3 additions & 0 deletions .changelog/32416.txt
View file
Original file line number Diff line number Diff line change
@@ -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`
```
11 changes: 8 additions & 3 deletions internal/service/emr/find.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 21 additions & 5 deletions internal/service/emr/id.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
29 changes: 20 additions & 9 deletions internal/service/emr/studio_session_mapping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)...)
}
Expand All @@ -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)
}
Expand All @@ -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)
Expand All @@ -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("")
Expand All @@ -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())
Expand Down
92 changes: 85 additions & 7 deletions internal/service/emr/studio_session_mapping_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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"),
),
},
},
})
}
Expand All @@ -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,
Expand All @@ -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,
},
},
})
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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" {}
Expand All @@ -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 = "/"
Expand Down Expand Up @@ -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" {
Expand Down Expand Up @@ -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))
}

0 comments on commit d1a767a

Please sign in to comment.