From 16358fb4362c6ebd51bc9c5c576bc78f6c2da5a7 Mon Sep 17 00:00:00 2001 From: Adrien Frediani Date: Fri, 26 Jan 2024 16:16:33 +0100 Subject: [PATCH 1/5] fix #35491 --- internal/service/cognitoidp/user_group.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/cognitoidp/user_group.go b/internal/service/cognitoidp/user_group.go index 0e7bd0dcd65..36221d8871b 100644 --- a/internal/service/cognitoidp/user_group.go +++ b/internal/service/cognitoidp/user_group.go @@ -180,7 +180,7 @@ func resourceUserGroupDelete(ctx context.Context, d *schema.ResourceData, meta i } func resourceUserGroupImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - idSplit := strings.Split(d.Id(), "/") + idSplit := strings.SplitN(d.Id(), "/", 2) if len(idSplit) != 2 { return nil, errors.New("Error importing Cognito User Group. Must specify user_pool_id/group_name") } From 75cabfe69e7f84d343a68f71a0b3f0879aea126a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 31 Jan 2024 08:04:54 -0500 Subject: [PATCH 2/5] Add CHANGELOG entry. --- .changelog/35501.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/35501.txt diff --git a/.changelog/35501.txt b/.changelog/35501.txt new file mode 100644 index 00000000000..30cf593bb4e --- /dev/null +++ b/.changelog/35501.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_cognito_user_group: Allow import of user groups with names containing `/` +``` \ No newline at end of file From 99074301f13c191bbf1d7429fdd28a280b3c3d53 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 31 Jan 2024 08:19:41 -0500 Subject: [PATCH 3/5] r/aws_cognito_user_group: Tidy up. --- internal/service/cognitoidp/consts.go | 1 - internal/service/cognitoidp/exports_test.go | 3 + .../service/cognitoidp/service_package_gen.go | 3 +- internal/service/cognitoidp/user_group.go | 139 ++++++++++++------ 4 files changed, 100 insertions(+), 46 deletions(-) diff --git a/internal/service/cognitoidp/consts.go b/internal/service/cognitoidp/consts.go index 355dd1d7418..009281f3c71 100644 --- a/internal/service/cognitoidp/consts.go +++ b/internal/service/cognitoidp/consts.go @@ -11,7 +11,6 @@ const ( ResNameIdentityProvider = "Identity Provider" ResNameResourceServer = "Resource Server" ResNameRiskConfiguration = "Risk Configuration" - ResNameUserGroup = "User Group" ResNameUserPoolClient = "User Pool Client" ResNameUserPoolDomain = "User Pool Domain" ResNameUserPool = "User Pool" diff --git a/internal/service/cognitoidp/exports_test.go b/internal/service/cognitoidp/exports_test.go index 7746cfae13c..9cbdaee8484 100644 --- a/internal/service/cognitoidp/exports_test.go +++ b/internal/service/cognitoidp/exports_test.go @@ -5,6 +5,9 @@ package cognitoidp // Exports for use in tests only. var ( + ResourceUserGroup = resourceUserGroup ResourceUserPoolClient = newResourceUserPoolClient ResourceManagedUserPoolClient = newResourceManagedUserPoolClient + + FindGroupByTwoPartKey = findGroupByTwoPartKey ) diff --git a/internal/service/cognitoidp/service_package_gen.go b/internal/service/cognitoidp/service_package_gen.go index f3f2acbbb89..2b0cb49f183 100644 --- a/internal/service/cognitoidp/service_package_gen.go +++ b/internal/service/cognitoidp/service_package_gen.go @@ -70,8 +70,9 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka TypeName: "aws_cognito_user", }, { - Factory: ResourceUserGroup, + Factory: resourceUserGroup, TypeName: "aws_cognito_user_group", + Name: "User Group", }, { Factory: ResourceUserInGroup, diff --git a/internal/service/cognitoidp/user_group.go b/internal/service/cognitoidp/user_group.go index 36221d8871b..49e8c6e20f2 100644 --- a/internal/service/cognitoidp/user_group.go +++ b/internal/service/cognitoidp/user_group.go @@ -12,19 +12,19 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cognitoidentityprovider" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" - "github.com/hashicorp/terraform-provider-aws/internal/create" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" - "github.com/hashicorp/terraform-provider-aws/names" ) -// @SDKResource("aws_cognito_user_group") -func ResourceUserGroup() *schema.Resource { +// @SDKResource("aws_cognito_user_group", name="User Group") +func resourceUserGroup() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceUserGroupCreate, ReadWithoutTimeout: resourceUserGroupRead, @@ -71,31 +71,31 @@ func resourceUserGroupCreate(ctx context.Context, d *schema.ResourceData, meta i var diags diag.Diagnostics conn := meta.(*conns.AWSClient).CognitoIDPConn(ctx) - params := &cognitoidentityprovider.CreateGroupInput{ - GroupName: aws.String(d.Get("name").(string)), + name := d.Get("name").(string) + input := &cognitoidentityprovider.CreateGroupInput{ + GroupName: aws.String(name), UserPoolId: aws.String(d.Get("user_pool_id").(string)), } if v, ok := d.GetOk("description"); ok { - params.Description = aws.String(v.(string)) + input.Description = aws.String(v.(string)) } if v, ok := d.GetOk("precedence"); ok { - params.Precedence = aws.Int64(int64(v.(int))) + input.Precedence = aws.Int64(int64(v.(int))) } if v, ok := d.GetOk("role_arn"); ok { - params.RoleArn = aws.String(v.(string)) + input.RoleArn = aws.String(v.(string)) } - log.Print("[DEBUG] Creating Cognito User Group") + output, err := conn.CreateGroupWithContext(ctx, input) - resp, err := conn.CreateGroupWithContext(ctx, params) if err != nil { - return sdkdiag.AppendErrorf(diags, "creating Cognito User Group: %s", err) + return sdkdiag.AppendErrorf(diags, "creating Cognito User Group (%s): %s", name, err) } - d.SetId(fmt.Sprintf("%s/%s", *resp.Group.UserPoolId, *resp.Group.GroupName)) + d.SetId(userGroupCreateResourceID(aws.StringValue(output.Group.UserPoolId), aws.StringValue(output.Group.GroupName))) return append(diags, resourceUserGroupRead(ctx, d, meta)...) } @@ -104,27 +104,26 @@ func resourceUserGroupRead(ctx context.Context, d *schema.ResourceData, meta int var diags diag.Diagnostics conn := meta.(*conns.AWSClient).CognitoIDPConn(ctx) - params := &cognitoidentityprovider.GetGroupInput{ - GroupName: aws.String(d.Get("name").(string)), - UserPoolId: aws.String(d.Get("user_pool_id").(string)), + userPoolID, groupName, err := userGroupParseResourceID(d.Id()) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) } - log.Print("[DEBUG] Reading Cognito User Group") + group, err := findGroupByTwoPartKey(ctx, conn, userPoolID, groupName) - resp, err := conn.GetGroupWithContext(ctx, params) if !d.IsNewResource() && tfresource.NotFound(err) { - create.LogNotFoundRemoveState(names.CognitoIDP, create.ErrActionReading, ResNameUserGroup, d.Get("name").(string)) + log.Printf("[WARN] Cognito User Group %s not found, removing from state", d.Id()) d.SetId("") return diags } if err != nil { - return create.AppendDiagError(diags, names.CognitoIDP, create.ErrActionReading, ResNameUserGroup, d.Get("name").(string), err) + return sdkdiag.AppendErrorf(diags, "reading Cognito User Group (%s): %s", d.Id(), err) } - d.Set("description", resp.Group.Description) - d.Set("precedence", resp.Group.Precedence) - d.Set("role_arn", resp.Group.RoleArn) + d.Set("description", group.Description) + d.Set("precedence", group.Precedence) + d.Set("role_arn", group.RoleArn) return diags } @@ -133,28 +132,32 @@ func resourceUserGroupUpdate(ctx context.Context, d *schema.ResourceData, meta i var diags diag.Diagnostics conn := meta.(*conns.AWSClient).CognitoIDPConn(ctx) - params := &cognitoidentityprovider.UpdateGroupInput{ - GroupName: aws.String(d.Get("name").(string)), - UserPoolId: aws.String(d.Get("user_pool_id").(string)), + userPoolID, groupName, err := userGroupParseResourceID(d.Id()) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + + input := &cognitoidentityprovider.UpdateGroupInput{ + GroupName: aws.String(groupName), + UserPoolId: aws.String(userPoolID), } if d.HasChange("description") { - params.Description = aws.String(d.Get("description").(string)) + input.Description = aws.String(d.Get("description").(string)) } if d.HasChange("precedence") { - params.Precedence = aws.Int64(int64(d.Get("precedence").(int))) + input.Precedence = aws.Int64(int64(d.Get("precedence").(int))) } if d.HasChange("role_arn") { - params.RoleArn = aws.String(d.Get("role_arn").(string)) + input.RoleArn = aws.String(d.Get("role_arn").(string)) } - log.Print("[DEBUG] Updating Cognito User Group") + _, err = conn.UpdateGroupWithContext(ctx, input) - _, err := conn.UpdateGroupWithContext(ctx, params) if err != nil { - return sdkdiag.AppendErrorf(diags, "updating Cognito User Group: %s", err) + return sdkdiag.AppendErrorf(diags, "updating Cognito User Group (%s): %s", d.Id(), err) } return append(diags, resourceUserGroupRead(ctx, d, meta)...) @@ -164,29 +167,77 @@ func resourceUserGroupDelete(ctx context.Context, d *schema.ResourceData, meta i var diags diag.Diagnostics conn := meta.(*conns.AWSClient).CognitoIDPConn(ctx) - params := &cognitoidentityprovider.DeleteGroupInput{ - GroupName: aws.String(d.Get("name").(string)), - UserPoolId: aws.String(d.Get("user_pool_id").(string)), + userPoolID, groupName, err := userGroupParseResourceID(d.Id()) + if err != nil { + return sdkdiag.AppendFromErr(diags, err) } - log.Print("[DEBUG] Deleting Cognito User Group") + log.Printf("[DEBUG] Deleting Cognito User Group: %s", d.Id()) + _, err = conn.DeleteGroupWithContext(ctx, &cognitoidentityprovider.DeleteGroupInput{ + GroupName: aws.String(groupName), + UserPoolId: aws.String(userPoolID), + }) - _, err := conn.DeleteGroupWithContext(ctx, params) if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting Cognito User Group: %s", err) + return sdkdiag.AppendErrorf(diags, "deleting Cognito User Group (%s): %s", d.Id(), err) } return diags } func resourceUserGroupImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { - idSplit := strings.SplitN(d.Id(), "/", 2) - if len(idSplit) != 2 { + parts := strings.SplitN(d.Id(), "/", 2) + if len(parts) != 2 { return nil, errors.New("Error importing Cognito User Group. Must specify user_pool_id/group_name") } - userPoolId := idSplit[0] - name := idSplit[1] - d.Set("user_pool_id", userPoolId) - d.Set("name", name) + + d.Set("user_pool_id", parts[0]) + d.Set("name", parts[1]) + return []*schema.ResourceData{d}, nil } + +const userGroupResourceIDSeparator = "/" + +func userGroupCreateResourceID(userPoolID, groupName string) string { + parts := []string{userPoolID, groupName} + id := strings.Join(parts, userGroupResourceIDSeparator) + + return id +} + +func userGroupParseResourceID(id string) (string, string, error) { + parts := strings.Split(id, userGroupResourceIDSeparator) + + if len(parts) == 2 && parts[0] != "" && parts[1] != "" { + return parts[0], parts[1], nil + } + + return "", "", fmt.Errorf("unexpected format for ID (%[1]s), expected USERPOOLID%[2]sGROUPNAME", id, userGroupResourceIDSeparator) +} + +func findGroupByTwoPartKey(ctx context.Context, conn *cognitoidentityprovider.CognitoIdentityProvider, userPoolID, groupName string) (*cognitoidentityprovider.GroupType, error) { + input := &cognitoidentityprovider.GetGroupInput{ + GroupName: aws.String(groupName), + UserPoolId: aws.String(userPoolID), + } + + output, err := conn.GetGroupWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, cognitoidentityprovider.ErrCodeResourceNotFoundException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.Group == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.Group, nil +} From d1b277c475f252290d4499eb28aa1b8e138ad0d7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 31 Jan 2024 08:30:35 -0500 Subject: [PATCH 4/5] r/aws_cognito_user_group: Additional acceptance tests. --- .../service/cognitoidp/user_group_test.go | 146 +++++++++--------- 1 file changed, 73 insertions(+), 73 deletions(-) diff --git a/internal/service/cognitoidp/user_group_test.go b/internal/service/cognitoidp/user_group_test.go index 09f8aba1b73..da053a2665f 100644 --- a/internal/service/cognitoidp/user_group_test.go +++ b/internal/service/cognitoidp/user_group_test.go @@ -5,26 +5,25 @@ package cognitoidp_test import ( "context" - "errors" "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cognitoidentityprovider" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfcognitoidp "github.com/hashicorp/terraform-provider-aws/internal/service/cognitoidp" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccCognitoIDPUserGroup_basic(t *testing.T) { ctx := acctest.Context(t) - poolName := fmt.Sprintf("tf-acc-%s", sdkacctest.RandString(10)) - groupName := fmt.Sprintf("tf-acc-%s", sdkacctest.RandString(10)) - updatedGroupName := fmt.Sprintf("tf-acc-%s", sdkacctest.RandString(10)) - resourceName := "aws_cognito_user_group.main" + poolName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + groupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + updatedGroupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_cognito_user_group.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) }, @@ -55,12 +54,36 @@ func TestAccCognitoIDPUserGroup_basic(t *testing.T) { }) } +func TestAccCognitoIDPUserGroup_disappears(t *testing.T) { + ctx := acctest.Context(t) + poolName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + groupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_cognito_user_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, cognitoidentityprovider.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckUserGroupDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccUserGroupConfig_basic(poolName, groupName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckUserGroupExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfcognitoidp.ResourceUserGroup(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccCognitoIDPUserGroup_complex(t *testing.T) { ctx := acctest.Context(t) - poolName := fmt.Sprintf("tf-acc-%s", sdkacctest.RandString(10)) - groupName := fmt.Sprintf("tf-acc-%s", sdkacctest.RandString(10)) - updatedGroupName := fmt.Sprintf("tf-acc-%s", sdkacctest.RandString(10)) - resourceName := "aws_cognito_user_group.main" + poolName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + groupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + updatedGroupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_cognito_user_group.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) }, @@ -99,8 +122,8 @@ func TestAccCognitoIDPUserGroup_complex(t *testing.T) { func TestAccCognitoIDPUserGroup_roleARN(t *testing.T) { ctx := acctest.Context(t) - rName := sdkacctest.RandomWithPrefix("tf-acc") - resourceName := "aws_cognito_user_group.main" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_cognito_user_group.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheckIdentityProvider(ctx, t) }, @@ -131,37 +154,17 @@ func TestAccCognitoIDPUserGroup_roleARN(t *testing.T) { }) } -func testAccCheckUserGroupExists(ctx context.Context, name string) resource.TestCheckFunc { +func testAccCheckUserGroupExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", name) - } - - id := rs.Primary.ID - name := rs.Primary.Attributes["name"] - userPoolId := rs.Primary.Attributes["user_pool_id"] - - if name == "" { - return errors.New("No Cognito User Group Name set") - } - - if userPoolId == "" { - return errors.New("No Cognito User Pool Id set") - } - - if id != fmt.Sprintf("%s/%s", userPoolId, name) { - return fmt.Errorf(fmt.Sprintf("ID should be user_pool_id/name. ID was %s. name was %s, user_pool_id was %s", id, name, userPoolId)) + return fmt.Errorf("Not found: %s", n) } conn := acctest.Provider.Meta().(*conns.AWSClient).CognitoIDPConn(ctx) - params := &cognitoidentityprovider.GetGroupInput{ - GroupName: aws.String(rs.Primary.Attributes["name"]), - UserPoolId: aws.String(rs.Primary.Attributes["user_pool_id"]), - } + _, err := tfcognitoidp.FindGroupByTwoPartKey(ctx, conn, rs.Primary.Attributes["user_pool_id"], rs.Primary.Attributes["name"]) - _, err := conn.GetGroupWithContext(ctx, params) return err } } @@ -175,20 +178,17 @@ func testAccCheckUserGroupDestroy(ctx context.Context) resource.TestCheckFunc { continue } - params := &cognitoidentityprovider.GetGroupInput{ - GroupName: aws.String(rs.Primary.ID), - UserPoolId: aws.String(rs.Primary.Attributes["user_pool_id"]), - } - - _, err := conn.GetGroupWithContext(ctx, params) + _, err := tfcognitoidp.FindGroupByTwoPartKey(ctx, conn, rs.Primary.Attributes["user_pool_id"], rs.Primary.Attributes["name"]) - if tfawserr.ErrCodeEquals(err, cognitoidentityprovider.ErrCodeResourceNotFoundException) { + if tfresource.NotFound(err) { continue } if err != nil { return err } + + return fmt.Errorf("Cognito User Group %s still exists", rs.Primary.ID) } return nil @@ -197,27 +197,27 @@ func testAccCheckUserGroupDestroy(ctx context.Context) resource.TestCheckFunc { func testAccUserGroupConfig_basic(poolName, groupName string) string { return fmt.Sprintf(` -resource "aws_cognito_user_pool" "main" { - name = "%s" +resource "aws_cognito_user_pool" "test" { + name = %[1]q } -resource "aws_cognito_user_group" "main" { - name = "%s" - user_pool_id = aws_cognito_user_pool.main.id +resource "aws_cognito_user_group" "test" { + name = %[2]q + user_pool_id = aws_cognito_user_pool.test.id } `, poolName, groupName) } func testAccUserGroupConfig_complex(poolName, groupName, groupDescription string, precedence int) string { return fmt.Sprintf(` -resource "aws_cognito_user_pool" "main" { - name = "%[1]s" +resource "aws_cognito_user_pool" "test" { + name = %[1]q } data "aws_region" "current" {} -resource "aws_iam_role" "group_role" { - name = "%[2]s" +resource "aws_iam_role" "test" { + name = %[2]q assume_role_policy = < Date: Wed, 31 Jan 2024 08:50:28 -0500 Subject: [PATCH 5/5] Fixes. --- internal/service/cognitoidp/user_group.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/service/cognitoidp/user_group.go b/internal/service/cognitoidp/user_group.go index 49e8c6e20f2..9accccc73d3 100644 --- a/internal/service/cognitoidp/user_group.go +++ b/internal/service/cognitoidp/user_group.go @@ -178,6 +178,10 @@ func resourceUserGroupDelete(ctx context.Context, d *schema.ResourceData, meta i UserPoolId: aws.String(userPoolID), }) + if tfawserr.ErrCodeEquals(err, cognitoidentityprovider.ErrCodeResourceNotFoundException) { + return diags + } + if err != nil { return sdkdiag.AppendErrorf(diags, "deleting Cognito User Group (%s): %s", d.Id(), err) } @@ -207,7 +211,7 @@ func userGroupCreateResourceID(userPoolID, groupName string) string { } func userGroupParseResourceID(id string) (string, string, error) { - parts := strings.Split(id, userGroupResourceIDSeparator) + parts := strings.SplitN(id, userGroupResourceIDSeparator, 2) if len(parts) == 2 && parts[0] != "" && parts[1] != "" { return parts[0], parts[1], nil