From 54ffafca75cc9e93d0d764aaa8a571178982f81e Mon Sep 17 00:00:00 2001 From: Frank Lichtenheld Date: Mon, 13 Nov 2023 17:56:10 +0100 Subject: [PATCH 01/20] service/iam: retry attaching policy on ConcurrentModificationException Fixes #34371 Signed-off-by: Frank Lichtenheld --- .../service/iam/group_policy_attachment.go | 10 +++++---- internal/service/iam/policy_attachment.go | 22 ++++++++++++++----- .../service/iam/role_policy_attachment.go | 10 +++++---- .../service/iam/user_policy_attachment.go | 10 +++++---- 4 files changed, 34 insertions(+), 18 deletions(-) diff --git a/internal/service/iam/group_policy_attachment.go b/internal/service/iam/group_policy_attachment.go index 4686bb34238..a3299076d1e 100644 --- a/internal/service/iam/group_policy_attachment.go +++ b/internal/service/iam/group_policy_attachment.go @@ -150,10 +150,12 @@ func resourceGroupPolicyAttachmentImport(ctx context.Context, d *schema.Resource } func attachPolicyToGroup(ctx context.Context, conn *iam.IAM, group string, arn string) error { - _, err := conn.AttachGroupPolicyWithContext(ctx, &iam.AttachGroupPolicyInput{ - GroupName: aws.String(group), - PolicyArn: aws.String(arn), - }) + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.AttachGroupPolicyWithContext(ctx, &iam.AttachGroupPolicyInput{ + GroupName: aws.String(group), + PolicyArn: aws.String(arn), + }) + }, iam.ErrCodeConcurrentModificationException) return err } diff --git a/internal/service/iam/policy_attachment.go b/internal/service/iam/policy_attachment.go index 5cab1ff5ca1..ef079c7c042 100644 --- a/internal/service/iam/policy_attachment.go +++ b/internal/service/iam/policy_attachment.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) // @SDKResource("aws_iam_policy_attachment") @@ -206,10 +207,13 @@ func composeErrors(desc string, uErr error, rErr error, gErr error) diag.Diagnos func attachPolicyToUsers(ctx context.Context, conn *iam.IAM, users []*string, arn string) error { for _, u := range users { - _, err := conn.AttachUserPolicyWithContext(ctx, &iam.AttachUserPolicyInput{ + input := &iam.AttachUserPolicyInput{ UserName: u, PolicyArn: aws.String(arn), - }) + } + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.AttachUserPolicyWithContext(ctx, input) + }, iam.ErrCodeConcurrentModificationException) if err != nil { return err } @@ -218,10 +222,13 @@ func attachPolicyToUsers(ctx context.Context, conn *iam.IAM, users []*string, ar } func attachPolicyToRoles(ctx context.Context, conn *iam.IAM, roles []*string, arn string) error { for _, r := range roles { - _, err := conn.AttachRolePolicyWithContext(ctx, &iam.AttachRolePolicyInput{ + input := &iam.AttachRolePolicyInput{ RoleName: r, PolicyArn: aws.String(arn), - }) + } + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.AttachRolePolicyWithContext(ctx, input) + }, iam.ErrCodeConcurrentModificationException) if err != nil { return err } @@ -230,10 +237,13 @@ func attachPolicyToRoles(ctx context.Context, conn *iam.IAM, roles []*string, ar } func attachPolicyToGroups(ctx context.Context, conn *iam.IAM, groups []*string, arn string) error { for _, g := range groups { - _, err := conn.AttachGroupPolicyWithContext(ctx, &iam.AttachGroupPolicyInput{ + input := &iam.AttachGroupPolicyInput{ GroupName: g, PolicyArn: aws.String(arn), - }) + } + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.AttachGroupPolicyWithContext(ctx, input) + }, iam.ErrCodeConcurrentModificationException) if err != nil { return err } diff --git a/internal/service/iam/role_policy_attachment.go b/internal/service/iam/role_policy_attachment.go index 92835771597..2faee20b16f 100644 --- a/internal/service/iam/role_policy_attachment.go +++ b/internal/service/iam/role_policy_attachment.go @@ -157,10 +157,12 @@ func resourceRolePolicyAttachmentImport(ctx context.Context, d *schema.ResourceD } func attachPolicyToRole(ctx context.Context, conn *iam.IAM, role string, arn string) error { - _, err := conn.AttachRolePolicyWithContext(ctx, &iam.AttachRolePolicyInput{ - RoleName: aws.String(role), - PolicyArn: aws.String(arn), - }) + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.AttachRolePolicyWithContext(ctx, &iam.AttachRolePolicyInput{ + RoleName: aws.String(role), + PolicyArn: aws.String(arn), + }) + }, iam.ErrCodeConcurrentModificationException) return err } diff --git a/internal/service/iam/user_policy_attachment.go b/internal/service/iam/user_policy_attachment.go index 13f642d5498..bb080962965 100644 --- a/internal/service/iam/user_policy_attachment.go +++ b/internal/service/iam/user_policy_attachment.go @@ -153,10 +153,12 @@ func resourceUserPolicyAttachmentImport(ctx context.Context, d *schema.ResourceD } func attachPolicyToUser(ctx context.Context, conn *iam.IAM, user string, arn string) error { - _, err := conn.AttachUserPolicyWithContext(ctx, &iam.AttachUserPolicyInput{ - UserName: aws.String(user), - PolicyArn: aws.String(arn), - }) + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.AttachUserPolicyWithContext(ctx, &iam.AttachUserPolicyInput{ + UserName: aws.String(user), + PolicyArn: aws.String(arn), + }) + }, iam.ErrCodeConcurrentModificationException) return err } From 0ce4f410fdff23eb46a42e65a6b88d31b5a20104 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 12:07:51 -0500 Subject: [PATCH 02/20] Add CHANGELOG entries. --- .changelog/34378.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .changelog/34378.txt diff --git a/.changelog/34378.txt b/.changelog/34378.txt new file mode 100644 index 00000000000..6b2feea1025 --- /dev/null +++ b/.changelog/34378.txt @@ -0,0 +1,15 @@ +```release-note:bug +resource/aws_iam_group_policy_attachment: Retry `ConcurrentModificationException` errors on create +``` + +```release-note:bug +resource/aws_iam_policy_attachment: Retry `ConcurrentModificationException` errors on create +``` + +```release-note:bug +resource/aws_iam_role_policy_attachment: Retry `ConcurrentModificationException` errors on create +``` + +```release-note:bug +resource/aws_iam_user_policy_attachment: Retry `ConcurrentModificationException` errors on create +``` \ No newline at end of file From 72bf55790045367af43390f1eb5d5badf0a12b4c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 13:10:28 -0500 Subject: [PATCH 03/20] r/aws_iam_group_policy_attachment: Add plan-time validation of `policy_arn`. --- .changelog/34378.txt | 4 ++++ internal/service/iam/group_policy_attachment.go | 9 ++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 6b2feea1025..0c40a1522ff 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -12,4 +12,8 @@ resource/aws_iam_role_policy_attachment: Retry `ConcurrentModificationException` ```release-note:bug resource/aws_iam_user_policy_attachment: Retry `ConcurrentModificationException` errors on create +``` + +```release-note:enhancement +resource/aws_iam_group_policy_attachment: Add plan-time validation of `policy_arn` ``` \ No newline at end of file diff --git a/internal/service/iam/group_policy_attachment.go b/internal/service/iam/group_policy_attachment.go index a3299076d1e..b451719e504 100644 --- a/internal/service/iam/group_policy_attachment.go +++ b/internal/service/iam/group_policy_attachment.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "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" ) // @SDKResource("aws_iam_group_policy_attachment") @@ -27,6 +28,7 @@ func ResourceGroupPolicyAttachment() *schema.Resource { CreateWithoutTimeout: resourceGroupPolicyAttachmentCreate, ReadWithoutTimeout: resourceGroupPolicyAttachmentRead, DeleteWithoutTimeout: resourceGroupPolicyAttachmentDelete, + Importer: &schema.ResourceImporter{ StateContext: resourceGroupPolicyAttachmentImport, }, @@ -38,9 +40,10 @@ func ResourceGroupPolicyAttachment() *schema.Resource { ForceNew: true, }, "policy_arn": { - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, }, }, } From 30278997b9278037320d4c230974d2609bb13abc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 13:13:05 -0500 Subject: [PATCH 04/20] r/aws_iam_policy_attachment: Add plan-time validation of `policy_arn`. --- .changelog/34378.txt | 4 ++++ internal/service/iam/policy_attachment.go | 25 +++++++++++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 0c40a1522ff..8f2220ddb79 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -16,4 +16,8 @@ resource/aws_iam_user_policy_attachment: Retry `ConcurrentModificationException` ```release-note:enhancement resource/aws_iam_group_policy_attachment: Add plan-time validation of `policy_arn` +``` + +```release-note:enhancement +resource/aws_iam_policy_attachment: Add plan-time validation of `policy_arn` ``` \ No newline at end of file diff --git a/internal/service/iam/policy_attachment.go b/internal/service/iam/policy_attachment.go index ef079c7c042..349cb76fe9b 100644 --- a/internal/service/iam/policy_attachment.go +++ b/internal/service/iam/policy_attachment.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/verify" ) // @SDKResource("aws_iam_policy_attachment") @@ -29,34 +30,32 @@ func ResourcePolicyAttachment() *schema.Resource { DeleteWithoutTimeout: resourcePolicyAttachmentDelete, Schema: map[string]*schema.Schema{ + "groups": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, "name": { Type: schema.TypeString, Required: true, ForceNew: true, ValidateFunc: validation.NoZeroValues, }, - "users": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, + "policy_arn": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, }, "roles": { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, - "groups": { + "users": { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, - }, - "policy_arn": { - Type: schema.TypeString, - Required: true, - ForceNew: true, }, }, } From fab3687c277844df1f84fd257282faab96295520 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 13:16:39 -0500 Subject: [PATCH 05/20] r/aws_iam_role_policy_attachment: Add plan-time validation of `policy_arn`. --- .changelog/34378.txt | 4 ++++ internal/service/iam/role_policy_attachment.go | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 8f2220ddb79..6acb71febcd 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -20,4 +20,8 @@ resource/aws_iam_group_policy_attachment: Add plan-time validation of `policy_ar ```release-note:enhancement resource/aws_iam_policy_attachment: Add plan-time validation of `policy_arn` +``` + +```release-note:enhancement +resource/aws_iam_role_policy_attachment: Add plan-time validation of `policy_arn` ``` \ No newline at end of file diff --git a/internal/service/iam/role_policy_attachment.go b/internal/service/iam/role_policy_attachment.go index 2faee20b16f..453023ff698 100644 --- a/internal/service/iam/role_policy_attachment.go +++ b/internal/service/iam/role_policy_attachment.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "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" ) // @SDKResource("aws_iam_role_policy_attachment") @@ -27,17 +28,19 @@ func ResourceRolePolicyAttachment() *schema.Resource { CreateWithoutTimeout: resourceRolePolicyAttachmentCreate, ReadWithoutTimeout: resourceRolePolicyAttachmentRead, DeleteWithoutTimeout: resourceRolePolicyAttachmentDelete, + Importer: &schema.ResourceImporter{ StateContext: resourceRolePolicyAttachmentImport, }, Schema: map[string]*schema.Schema{ - "role": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, "policy_arn": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, + }, + "role": { Type: schema.TypeString, Required: true, ForceNew: true, From 7e40676e0b217838aa28566416f9dee94252f754 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 13:18:06 -0500 Subject: [PATCH 06/20] r/aws_iam_user_policy_attachment: Add plan-time validation of `policy_arn`. --- .changelog/34378.txt | 4 ++++ internal/service/iam/user_policy_attachment.go | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 6acb71febcd..8243b94b439 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -24,4 +24,8 @@ resource/aws_iam_policy_attachment: Add plan-time validation of `policy_arn` ```release-note:enhancement resource/aws_iam_role_policy_attachment: Add plan-time validation of `policy_arn` +``` + +```release-note:enhancement +resource/aws_iam_user_policy_attachment: Add plan-time validation of `policy_arn` ``` \ No newline at end of file diff --git a/internal/service/iam/user_policy_attachment.go b/internal/service/iam/user_policy_attachment.go index bb080962965..adeceaea442 100644 --- a/internal/service/iam/user_policy_attachment.go +++ b/internal/service/iam/user_policy_attachment.go @@ -19,6 +19,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" "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" ) // @SDKResource("aws_iam_user_policy_attachment") @@ -27,21 +28,23 @@ func ResourceUserPolicyAttachment() *schema.Resource { CreateWithoutTimeout: resourceUserPolicyAttachmentCreate, ReadWithoutTimeout: resourceUserPolicyAttachmentRead, DeleteWithoutTimeout: resourceUserPolicyAttachmentDelete, + Importer: &schema.ResourceImporter{ StateContext: resourceUserPolicyAttachmentImport, }, Schema: map[string]*schema.Schema{ + "policy_arn": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: verify.ValidARN, + }, "user": { Type: schema.TypeString, ForceNew: true, Required: true, }, - "policy_arn": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, }, } } From 42e0b2606351212d9b8e7160c86a7f69c7de0ae8 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 13:37:34 -0500 Subject: [PATCH 07/20] r/aws_iam_group_policy_attachment: Tidy up. --- .changelog/34378.txt | 2 +- internal/service/iam/find.go | 34 ---- .../service/iam/group_policy_attachment.go | 146 +++++++++++------- internal/service/iam/service_package_gen.go | 1 + 4 files changed, 90 insertions(+), 93 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 8243b94b439..fe26bb64b29 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -1,5 +1,5 @@ ```release-note:bug -resource/aws_iam_group_policy_attachment: Retry `ConcurrentModificationException` errors on create +resource/aws_iam_group_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete ``` ```release-note:bug diff --git a/internal/service/iam/find.go b/internal/service/iam/find.go index b9533f6da35..2f25b42d4af 100644 --- a/internal/service/iam/find.go +++ b/internal/service/iam/find.go @@ -14,40 +14,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// FindGroupAttachedPolicy returns the AttachedPolicy corresponding to the specified group and policy ARN. -func FindGroupAttachedPolicy(ctx context.Context, conn *iam.IAM, groupName string, policyARN string) (*iam.AttachedPolicy, error) { - input := &iam.ListAttachedGroupPoliciesInput{ - GroupName: aws.String(groupName), - } - - var result *iam.AttachedPolicy - - err := conn.ListAttachedGroupPoliciesPagesWithContext(ctx, input, func(page *iam.ListAttachedGroupPoliciesOutput, lastPage bool) bool { - if page == nil { - return !lastPage - } - - for _, attachedPolicy := range page.AttachedPolicies { - if attachedPolicy == nil { - continue - } - - if aws.StringValue(attachedPolicy.PolicyArn) == policyARN { - result = attachedPolicy - return false - } - } - - return !lastPage - }) - - if err != nil { - return nil, err - } - - return result, nil -} - // FindUserAttachedPolicy returns the AttachedPolicy corresponding to the specified user and policy ARN. func FindUserAttachedPolicy(ctx context.Context, conn *iam.IAM, userName string, policyARN string) (*iam.AttachedPolicy, error) { input := &iam.ListAttachedUserPoliciesInput{ diff --git a/internal/service/iam/group_policy_attachment.go b/internal/service/iam/group_policy_attachment.go index b451719e504..5b11cc14f1e 100644 --- a/internal/service/iam/group_policy_attachment.go +++ b/internal/service/iam/group_policy_attachment.go @@ -18,11 +18,12 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_iam_group_policy_attachment") +// @SDKResource("aws_iam_group_policy_attachment", name="Group Policy Attachment") func ResourceGroupPolicyAttachment() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceGroupPolicyAttachmentCreate, @@ -54,11 +55,11 @@ func resourceGroupPolicyAttachmentCreate(ctx context.Context, d *schema.Resource conn := meta.(*conns.AWSClient).IAMConn(ctx) group := d.Get("group").(string) - arn := d.Get("policy_arn").(string) + policyARN := d.Get("policy_arn").(string) - err := attachPolicyToGroup(ctx, conn, group, arn) + err := attachPolicyToGroup(ctx, conn, group, policyARN) if err != nil { - return sdkdiag.AppendErrorf(diags, "attaching policy %s to IAM group %s: %v", arn, group, err) + return sdkdiag.AppendFromErr(diags, err) } //lintignore:R016 // Allow legacy unstable ID usage in managed resource @@ -70,57 +71,24 @@ func resourceGroupPolicyAttachmentCreate(ctx context.Context, d *schema.Resource func resourceGroupPolicyAttachmentRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) + group := d.Get("group").(string) - arn := d.Get("policy_arn").(string) + policyARN := d.Get("policy_arn").(string) // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s:%s", group, arn) - - var attachedPolicy *iam.AttachedPolicy - - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - - attachedPolicy, err = FindGroupAttachedPolicy(ctx, conn, group, arn) + id := fmt.Sprintf("%s:%s", group, policyARN) - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } + _, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { + return FindAttachedGroupPolicyByTwoPartKey(ctx, conn, group, policyARN) + }, d.IsNewResource()) - if d.IsNewResource() && attachedPolicy == nil { - return retry.RetryableError(&retry.NotFoundError{ - LastError: fmt.Errorf("IAM Group Managed Policy Attachment (%s) not found", id), - }) - } - - return nil - }) - - if tfresource.TimedOut(err) { - attachedPolicy, err = FindGroupAttachedPolicy(ctx, conn, group, arn) - } - - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - log.Printf("[WARN] IAM User Managed Policy Attachment (%s) not found, removing from state", id) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] IAM Group Policy Attachment (%s) not found, removing from state", id) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM Group Managed Policy Attachment (%s): %s", id, err) - } - - if attachedPolicy == nil { - if d.IsNewResource() { - return sdkdiag.AppendErrorf(diags, "reading IAM User Managed Policy Attachment (%s): not found after creation", id) - } - - log.Printf("[WARN] IAM Group Managed Policy Attachment (%s) not found, removing from state", id) - d.SetId("") - return diags + return sdkdiag.AppendErrorf(diags, "reading IAM Group Policy Attachment (%s): %s", id, err) } return diags @@ -129,13 +97,12 @@ func resourceGroupPolicyAttachmentRead(ctx context.Context, d *schema.ResourceDa func resourceGroupPolicyAttachmentDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - group := d.Get("group").(string) - arn := d.Get("policy_arn").(string) - err := detachPolicyFromGroup(ctx, conn, group, arn) + err := detachPolicyFromGroup(ctx, conn, d.Get("group").(string), d.Get("policy_arn").(string)) if err != nil { - return sdkdiag.AppendErrorf(diags, "removing policy %s from IAM Group %s: %v", arn, group, err) + return sdkdiag.AppendFromErr(diags, err) } + return diags } @@ -152,20 +119,83 @@ func resourceGroupPolicyAttachmentImport(ctx context.Context, d *schema.Resource return []*schema.ResourceData{d}, nil } -func attachPolicyToGroup(ctx context.Context, conn *iam.IAM, group string, arn string) error { +func attachPolicyToGroup(ctx context.Context, conn *iam.IAM, group, policyARN string) error { _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { return conn.AttachGroupPolicyWithContext(ctx, &iam.AttachGroupPolicyInput{ GroupName: aws.String(group), - PolicyArn: aws.String(arn), + PolicyArn: aws.String(policyARN), + }) + }, iam.ErrCodeConcurrentModificationException) + + if err != nil { + return fmt.Errorf("attaching IAM Policy (%s) to IAM Group (%s): %w", policyARN, group, err) + } + + return nil +} + +func detachPolicyFromGroup(ctx context.Context, conn *iam.IAM, group, policyARN string) error { + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.DetachGroupPolicyWithContext(ctx, &iam.DetachGroupPolicyInput{ + GroupName: aws.String(group), + PolicyArn: aws.String(policyARN), }) }, iam.ErrCodeConcurrentModificationException) - return err + + if err != nil { + return fmt.Errorf("dtaching IAM Policy (%s) from IAM Group (%s): %w", policyARN, group, err) + } + + return nil } -func detachPolicyFromGroup(ctx context.Context, conn *iam.IAM, group string, arn string) error { - _, err := conn.DetachGroupPolicyWithContext(ctx, &iam.DetachGroupPolicyInput{ - GroupName: aws.String(group), - PolicyArn: aws.String(arn), +func FindAttachedGroupPolicyByTwoPartKey(ctx context.Context, conn *iam.IAM, groupName, policyARN string) (*iam.AttachedPolicy, error) { + input := &iam.ListAttachedGroupPoliciesInput{ + GroupName: aws.String(groupName), + } + + return findAttachedGroupPolicy(ctx, conn, input, func(v *iam.AttachedPolicy) bool { + return aws.StringValue(v.PolicyArn) == policyARN }) - return err +} + +func findAttachedGroupPolicy(ctx context.Context, conn *iam.IAM, input *iam.ListAttachedGroupPoliciesInput, filter tfslices.Predicate[*iam.AttachedPolicy]) (*iam.AttachedPolicy, error) { + output, err := findAttachedGroupPolicies(ctx, conn, input, filter) + + if err != nil { + return nil, err + } + + return tfresource.AssertSinglePtrResult(output) +} + +func findAttachedGroupPolicies(ctx context.Context, conn *iam.IAM, input *iam.ListAttachedGroupPoliciesInput, filter tfslices.Predicate[*iam.AttachedPolicy]) ([]*iam.AttachedPolicy, error) { + var output []*iam.AttachedPolicy + + err := conn.ListAttachedGroupPoliciesPagesWithContext(ctx, input, func(page *iam.ListAttachedGroupPoliciesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.AttachedPolicies { + if v != nil && filter(v) { + output = append(output, v) + } + } + + return !lastPage + }) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + return output, nil } diff --git a/internal/service/iam/service_package_gen.go b/internal/service/iam/service_package_gen.go index 6d2303a2c66..00ce060449c 100644 --- a/internal/service/iam/service_package_gen.go +++ b/internal/service/iam/service_package_gen.go @@ -125,6 +125,7 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka { Factory: ResourceGroupPolicyAttachment, TypeName: "aws_iam_group_policy_attachment", + Name: "Group Policy Attachment", }, { Factory: ResourceInstanceProfile, From b6fb2c5009a31a184330f4f9986393fca8a0cd21 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 14:06:33 -0500 Subject: [PATCH 08/20] r/aws_iam_group_policy_attachment: Tidy up acceptance tests. --- internal/service/iam/exports_test.go | 12 ++ .../service/iam/group_policy_attachment.go | 12 +- .../iam/group_policy_attachment_test.go | 197 ++++++++++-------- internal/service/iam/service_package_gen.go | 2 +- 4 files changed, 136 insertions(+), 87 deletions(-) create mode 100644 internal/service/iam/exports_test.go diff --git a/internal/service/iam/exports_test.go b/internal/service/iam/exports_test.go new file mode 100644 index 00000000000..879596d4fa3 --- /dev/null +++ b/internal/service/iam/exports_test.go @@ -0,0 +1,12 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package iam + +// Exports for use in tests only. +var ( + ResourceGroupPolicyAttachment = resourceGroupPolicyAttachment + + FindAttachedGroupPolicies = findAttachedGroupPolicies + FindAttachedGroupPolicyByTwoPartKey = findAttachedGroupPolicyByTwoPartKey +) diff --git a/internal/service/iam/group_policy_attachment.go b/internal/service/iam/group_policy_attachment.go index 5b11cc14f1e..0cc699e7752 100644 --- a/internal/service/iam/group_policy_attachment.go +++ b/internal/service/iam/group_policy_attachment.go @@ -24,7 +24,7 @@ import ( ) // @SDKResource("aws_iam_group_policy_attachment", name="Group Policy Attachment") -func ResourceGroupPolicyAttachment() *schema.Resource { +func resourceGroupPolicyAttachment() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceGroupPolicyAttachmentCreate, ReadWithoutTimeout: resourceGroupPolicyAttachmentRead, @@ -74,11 +74,11 @@ func resourceGroupPolicyAttachmentRead(ctx context.Context, d *schema.ResourceDa group := d.Get("group").(string) policyARN := d.Get("policy_arn").(string) - // Human friendly ID for error messages since d.Id() is non-descriptive + // Human friendly ID for error messages since d.Id() is non-descriptive. id := fmt.Sprintf("%s:%s", group, policyARN) _, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { - return FindAttachedGroupPolicyByTwoPartKey(ctx, conn, group, policyARN) + return findAttachedGroupPolicyByTwoPartKey(ctx, conn, group, policyARN) }, d.IsNewResource()) if !d.IsNewResource() && tfresource.NotFound(err) { @@ -142,6 +142,10 @@ func detachPolicyFromGroup(ctx context.Context, conn *iam.IAM, group, policyARN }) }, iam.ErrCodeConcurrentModificationException) + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil + } + if err != nil { return fmt.Errorf("dtaching IAM Policy (%s) from IAM Group (%s): %w", policyARN, group, err) } @@ -149,7 +153,7 @@ func detachPolicyFromGroup(ctx context.Context, conn *iam.IAM, group, policyARN return nil } -func FindAttachedGroupPolicyByTwoPartKey(ctx context.Context, conn *iam.IAM, groupName, policyARN string) (*iam.AttachedPolicy, error) { +func findAttachedGroupPolicyByTwoPartKey(ctx context.Context, conn *iam.IAM, groupName, policyARN string) (*iam.AttachedPolicy, error) { input := &iam.ListAttachedGroupPoliciesInput{ GroupName: aws.String(groupName), } diff --git a/internal/service/iam/group_policy_attachment_test.go b/internal/service/iam/group_policy_attachment_test.go index f6a7f9e12c7..aa7a309ec7e 100644 --- a/internal/service/iam/group_policy_attachment_test.go +++ b/internal/service/iam/group_policy_attachment_test.go @@ -6,46 +6,46 @@ package iam_test import ( "context" "fmt" - "strings" "testing" - "github.com/aws/aws-sdk-go-v2/aws/arn" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/iam" 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" + tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMGroupPolicyAttachment_basic(t *testing.T) { ctx := acctest.Context(t) - var out iam.ListAttachedGroupPoliciesOutput - - rString := sdkacctest.RandString(8) - groupName := fmt.Sprintf("tf-acc-group-gpa-basic-%s", rString) - policyName := fmt.Sprintf("tf-acc-policy-gpa-basic-%s", rString) - policyName2 := fmt.Sprintf("tf-acc-policy-gpa-basic-2-%s", rString) - policyName3 := fmt.Sprintf("tf-acc-policy-gpa-basic-3-%s", rString) + groupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName3 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_group_policy_attachment.test1" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckGroupPolicyAttachmentDestroy, + CheckDestroy: testAccCheckGroupPolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccGroupPolicyAttachmentConfig_attach(groupName, policyName), + Config: testAccGroupPolicyAttachmentConfig_attach(groupName, policyName1), Check: resource.ComposeTestCheckFunc( - testAccCheckGroupPolicyAttachmentExists(ctx, "aws_iam_group_policy_attachment.test-attach", 1, &out), - testAccCheckGroupPolicyAttachmentAttributes([]string{policyName}, &out), + testAccCheckGroupPolicyAttachmentExists(ctx, resourceName), + testAccCheckGroupPolicyAttachmentCount(ctx, groupName, 1), ), }, { - ResourceName: "aws_iam_group_policy_attachment.test-attach", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccGroupPolicyAttachmentImportStateIdFunc("aws_iam_group_policy_attachment.test-attach"), + ImportStateIdFunc: testAccGroupPolicyAttachmentImportStateIdFunc(resourceName), // We do not have a way to align IDs since the Create function uses id.PrefixedUniqueId() // Failed state verification, resource with ID GROUP-POLICYARN not found // ImportStateVerify: true, @@ -61,77 +61,120 @@ func TestAccIAMGroupPolicyAttachment_basic(t *testing.T) { }, }, { - Config: testAccGroupPolicyAttachmentConfig_attachUpdate(groupName, policyName, policyName2, policyName3), + Config: testAccGroupPolicyAttachmentConfig_attachUpdate(groupName, policyName1, policyName2, policyName3), + Check: resource.ComposeTestCheckFunc( + testAccCheckGroupPolicyAttachmentExists(ctx, resourceName), + testAccCheckGroupPolicyAttachmentCount(ctx, groupName, 2), + ), + }, + }, + }) +} + +func TestAccIAMGroupPolicyAttachment_disappears(t *testing.T) { + ctx := acctest.Context(t) + groupName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_group_policy_attachment.test1" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckGroupPolicyAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccGroupPolicyAttachmentConfig_attach(groupName, policyName), Check: resource.ComposeTestCheckFunc( - testAccCheckGroupPolicyAttachmentExists(ctx, "aws_iam_group_policy_attachment.test-attach", 2, &out), - testAccCheckGroupPolicyAttachmentAttributes([]string{policyName2, policyName3}, &out), + testAccCheckGroupPolicyAttachmentExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceGroupPolicyAttachment(), resourceName), ), + ExpectNonEmptyPlan: true, }, }, }) } -func testAccCheckGroupPolicyAttachmentDestroy(s *terraform.State) error { - return nil +func testAccCheckGroupPolicyAttachmentDestroy(ctx context.Context) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_iam_group_policy_attachment" { + continue + } + + _, err := tfiam.FindAttachedGroupPolicyByTwoPartKey(ctx, conn, rs.Primary.Attributes["group"], rs.Primary.Attributes["policy_arn"]) + + if tfresource.NotFound(err) { + continue + } + + if err != nil { + return err + } + + return fmt.Errorf("IAM Group Policy Attachment %s still exists", rs.Primary.ID) + } + + return nil + } } -func testAccCheckGroupPolicyAttachmentExists(ctx context.Context, n string, c int, out *iam.ListAttachedGroupPoliciesOutput) resource.TestCheckFunc { +func testAccCheckGroupPolicyAttachmentExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) } - if rs.Primary.ID == "" { - return fmt.Errorf("No policy name is set") - } + conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) + + _, err := tfiam.FindAttachedGroupPolicyByTwoPartKey(ctx, conn, rs.Primary.Attributes["group"], rs.Primary.Attributes["policy_arn"]) + + return err + } +} +func testAccCheckGroupPolicyAttachmentCount(ctx context.Context, groupName string, want int) resource.TestCheckFunc { + return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) - group := rs.Primary.Attributes["group"] - attachedPolicies, err := conn.ListAttachedGroupPoliciesWithContext(ctx, &iam.ListAttachedGroupPoliciesInput{ - GroupName: aws.String(group), - }) + input := &iam.ListAttachedGroupPoliciesInput{ + GroupName: aws.String(groupName), + } + output, err := tfiam.FindAttachedGroupPolicies(ctx, conn, input, tfslices.PredicateTrue[*iam.AttachedPolicy]()) + if err != nil { - return fmt.Errorf("Error: Failed to get attached policies for group %s (%s)", group, n) + return err } - if c != len(attachedPolicies.AttachedPolicies) { - return fmt.Errorf("Error: Group (%s) has wrong number of policies attached on initial creation", n) + + if got := len(output); got != want { + return fmt.Errorf("GroupPolicyAttachmentCount(%q) = %v, want %v", groupName, got, want) } - *out = *attachedPolicies - return nil + return err } } -func testAccCheckGroupPolicyAttachmentAttributes(policies []string, out *iam.ListAttachedGroupPoliciesOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - matched := 0 - - for _, p := range policies { - for _, ap := range out.AttachedPolicies { - // *ap.PolicyArn like arn:aws:iam::111111111111:policy/test-policy - parts := strings.Split(*ap.PolicyArn, "/") - if len(parts) == 2 && p == parts[1] { - matched++ - } - } - } - if matched != len(policies) || matched != len(out.AttachedPolicies) { - return fmt.Errorf("Error: Number of attached policies was incorrect: expected %d matched policies, matched %d of %d", len(policies), matched, len(out.AttachedPolicies)) +func testAccGroupPolicyAttachmentImportStateIdFunc(resourceName string) resource.ImportStateIdFunc { + return func(s *terraform.State) (string, error) { + rs, ok := s.RootModule().Resources[resourceName] + if !ok { + return "", fmt.Errorf("Not found: %s", resourceName) } - return nil + return fmt.Sprintf("%s/%s", rs.Primary.Attributes["group"], rs.Primary.Attributes["policy_arn"]), nil } } func testAccGroupPolicyAttachmentConfig_attach(groupName, policyName string) string { return fmt.Sprintf(` -resource "aws_iam_group" "group" { - name = "%s" +resource "aws_iam_group" "test" { + name = %[1]q } -resource "aws_iam_policy" "policy" { - name = "%s" +resource "aws_iam_policy" "test1" { + name = %[2]q description = "A test policy" policy = < Date: Tue, 14 Nov 2023 14:07:47 -0500 Subject: [PATCH 09/20] Acceptance test output: % make testacc TESTARGS='-run=TestAccIAMGroupPolicyAttachment_' PKG=iam ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run=TestAccIAMGroupPolicyAttachment_ -timeout 360m === RUN TestAccIAMGroupPolicyAttachment_basic === PAUSE TestAccIAMGroupPolicyAttachment_basic === RUN TestAccIAMGroupPolicyAttachment_disappears === PAUSE TestAccIAMGroupPolicyAttachment_disappears === CONT TestAccIAMGroupPolicyAttachment_basic === CONT TestAccIAMGroupPolicyAttachment_disappears --- PASS: TestAccIAMGroupPolicyAttachment_disappears (21.23s) --- PASS: TestAccIAMGroupPolicyAttachment_basic (38.74s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/iam 44.687s --- internal/service/iam/group_policy_attachment.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/service/iam/group_policy_attachment.go b/internal/service/iam/group_policy_attachment.go index 0cc699e7752..94c2940634c 100644 --- a/internal/service/iam/group_policy_attachment.go +++ b/internal/service/iam/group_policy_attachment.go @@ -57,8 +57,7 @@ func resourceGroupPolicyAttachmentCreate(ctx context.Context, d *schema.Resource group := d.Get("group").(string) policyARN := d.Get("policy_arn").(string) - err := attachPolicyToGroup(ctx, conn, group, policyARN) - if err != nil { + if err := attachPolicyToGroup(ctx, conn, group, policyARN); err != nil { return sdkdiag.AppendFromErr(diags, err) } @@ -98,8 +97,7 @@ func resourceGroupPolicyAttachmentDelete(ctx context.Context, d *schema.Resource var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - err := detachPolicyFromGroup(ctx, conn, d.Get("group").(string), d.Get("policy_arn").(string)) - if err != nil { + if err := detachPolicyFromGroup(ctx, conn, d.Get("group").(string), d.Get("policy_arn").(string)); err != nil { return sdkdiag.AppendFromErr(diags, err) } @@ -147,7 +145,7 @@ func detachPolicyFromGroup(ctx context.Context, conn *iam.IAM, group, policyARN } if err != nil { - return fmt.Errorf("dtaching IAM Policy (%s) from IAM Group (%s): %w", policyARN, group, err) + return fmt.Errorf("detaching IAM Policy (%s) from IAM Group (%s): %w", policyARN, group, err) } return nil From 46b8b5eb33da8d772a997ac5c7ddb325deb67c7d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 14:21:53 -0500 Subject: [PATCH 10/20] r/aws_iam_user_policy_attachment: Tidy up. --- .changelog/34378.txt | 2 +- internal/service/iam/find.go | 34 ---- internal/service/iam/service_package_gen.go | 2 +- internal/service/iam/sweep.go | 2 +- .../service/iam/user_policy_attachment.go | 154 +++++++++++------- 5 files changed, 96 insertions(+), 98 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index fe26bb64b29..4bcac297d7b 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -11,7 +11,7 @@ resource/aws_iam_role_policy_attachment: Retry `ConcurrentModificationException` ``` ```release-note:bug -resource/aws_iam_user_policy_attachment: Retry `ConcurrentModificationException` errors on create +resource/aws_iam_user_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete ``` ```release-note:enhancement diff --git a/internal/service/iam/find.go b/internal/service/iam/find.go index 2f25b42d4af..b84b59a7a6d 100644 --- a/internal/service/iam/find.go +++ b/internal/service/iam/find.go @@ -14,40 +14,6 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) -// FindUserAttachedPolicy returns the AttachedPolicy corresponding to the specified user and policy ARN. -func FindUserAttachedPolicy(ctx context.Context, conn *iam.IAM, userName string, policyARN string) (*iam.AttachedPolicy, error) { - input := &iam.ListAttachedUserPoliciesInput{ - UserName: aws.String(userName), - } - - var result *iam.AttachedPolicy - - err := conn.ListAttachedUserPoliciesPagesWithContext(ctx, input, func(page *iam.ListAttachedUserPoliciesOutput, lastPage bool) bool { - if page == nil { - return !lastPage - } - - for _, attachedPolicy := range page.AttachedPolicies { - if attachedPolicy == nil { - continue - } - - if aws.StringValue(attachedPolicy.PolicyArn) == policyARN { - result = attachedPolicy - return false - } - } - - return !lastPage - }) - - if err != nil { - return nil, err - } - - return result, nil -} - func FindUsers(ctx context.Context, conn *iam.IAM, nameRegex, pathPrefix string) ([]*iam.User, error) { input := &iam.ListUsersInput{} diff --git a/internal/service/iam/service_package_gen.go b/internal/service/iam/service_package_gen.go index 77e1337b12f..2bc276d3b65 100644 --- a/internal/service/iam/service_package_gen.go +++ b/internal/service/iam/service_package_gen.go @@ -213,7 +213,7 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka TypeName: "aws_iam_user_policy", }, { - Factory: ResourceUserPolicyAttachment, + Factory: resourceUserPolicyAttachment, TypeName: "aws_iam_user_policy_attachment", }, { diff --git a/internal/service/iam/sweep.go b/internal/service/iam/sweep.go index ebfe9961bb1..5b7fe4cad46 100644 --- a/internal/service/iam/sweep.go +++ b/internal/service/iam/sweep.go @@ -683,7 +683,7 @@ func sweepUsers(region string) error { log.Printf("[DEBUG] Detaching IAM User (%s) attached policy: %s", username, policyARN) - if err := DetachPolicyFromUser(ctx, conn, username, policyARN); err != nil { + if err := detachPolicyFromUser(ctx, conn, username, policyARN); err != nil { sweeperErr := fmt.Errorf("error detaching IAM User (%s) attached policy (%s): %s", username, policyARN, err) log.Printf("[ERROR] %s", sweeperErr) sweeperErrs = multierror.Append(sweeperErrs, sweeperErr) diff --git a/internal/service/iam/user_policy_attachment.go b/internal/service/iam/user_policy_attachment.go index adeceaea442..64b6537a59d 100644 --- a/internal/service/iam/user_policy_attachment.go +++ b/internal/service/iam/user_policy_attachment.go @@ -18,12 +18,13 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) // @SDKResource("aws_iam_user_policy_attachment") -func ResourceUserPolicyAttachment() *schema.Resource { +func resourceUserPolicyAttachment() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceUserPolicyAttachmentCreate, ReadWithoutTimeout: resourceUserPolicyAttachmentRead, @@ -54,11 +55,10 @@ func resourceUserPolicyAttachmentCreate(ctx context.Context, d *schema.ResourceD conn := meta.(*conns.AWSClient).IAMConn(ctx) user := d.Get("user").(string) - arn := d.Get("policy_arn").(string) + policyARN := d.Get("policy_arn").(string) - err := attachPolicyToUser(ctx, conn, user, arn) - if err != nil { - return sdkdiag.AppendErrorf(diags, "attaching policy %s to IAM User %s: %v", arn, user, err) + if err := attachPolicyToUser(ctx, conn, user, policyARN); err != nil { + return sdkdiag.AppendFromErr(diags, err) } //lintignore:R016 // Allow legacy unstable ID usage in managed resource @@ -70,57 +70,24 @@ func resourceUserPolicyAttachmentCreate(ctx context.Context, d *schema.ResourceD func resourceUserPolicyAttachmentRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - user := d.Get("user").(string) - arn := d.Get("policy_arn").(string) - // Human friendly ID for error messages since d.Id() is non-descriptive - id := fmt.Sprintf("%s:%s", user, arn) - - var attachedPolicy *iam.AttachedPolicy - - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - - attachedPolicy, err = FindUserAttachedPolicy(ctx, conn, user, arn) - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } - - if d.IsNewResource() && attachedPolicy == nil { - return retry.RetryableError(&retry.NotFoundError{ - LastError: fmt.Errorf("IAM User Managed Policy Attachment (%s) not found", id), - }) - } - return nil - }) + user := d.Get("user").(string) + policyARN := d.Get("policy_arn").(string) + // Human friendly ID for error messages since d.Id() is non-descriptive. + id := fmt.Sprintf("%s:%s", user, policyARN) - if tfresource.TimedOut(err) { - attachedPolicy, err = FindUserAttachedPolicy(ctx, conn, user, arn) - } + _, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { + return findAttachedUserPolicyByTwoPartKey(ctx, conn, user, policyARN) + }, d.IsNewResource()) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - log.Printf("[WARN] IAM User Managed Policy Attachment (%s) not found, removing from state", id) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] IAM User Policy Attachment (%s) not found, removing from state", id) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM User Managed Policy Attachment (%s): %s", id, err) - } - - if attachedPolicy == nil { - if d.IsNewResource() { - return sdkdiag.AppendErrorf(diags, "reading IAM User Managed Policy Attachment (%s): not found after creation", id) - } - - log.Printf("[WARN] IAM User Managed Policy Attachment (%s) not found, removing from state", id) - d.SetId("") - return diags + return sdkdiag.AppendErrorf(diags, "reading IAM User Policy Attachment (%s): %s", id, err) } return diags @@ -129,13 +96,11 @@ func resourceUserPolicyAttachmentRead(ctx context.Context, d *schema.ResourceDat func resourceUserPolicyAttachmentDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - user := d.Get("user").(string) - arn := d.Get("policy_arn").(string) - err := DetachPolicyFromUser(ctx, conn, user, arn) - if err != nil { - return sdkdiag.AppendErrorf(diags, "removing policy %s from IAM User %s: %v", arn, user, err) + if err := detachPolicyFromUser(ctx, conn, d.Get("user").(string), d.Get("policy_arn").(string)); err != nil { + return sdkdiag.AppendFromErr(diags, err) } + return diags } @@ -155,20 +120,87 @@ func resourceUserPolicyAttachmentImport(ctx context.Context, d *schema.ResourceD return []*schema.ResourceData{d}, nil } -func attachPolicyToUser(ctx context.Context, conn *iam.IAM, user string, arn string) error { +func attachPolicyToUser(ctx context.Context, conn *iam.IAM, user, policyARN string) error { _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { return conn.AttachUserPolicyWithContext(ctx, &iam.AttachUserPolicyInput{ UserName: aws.String(user), - PolicyArn: aws.String(arn), + PolicyArn: aws.String(policyARN), + }) + }, iam.ErrCodeConcurrentModificationException) + + if err != nil { + return fmt.Errorf("attaching IAM Policy (%s) to IAM User (%s): %w", policyARN, user, err) + } + + return nil +} + +func detachPolicyFromUser(ctx context.Context, conn *iam.IAM, user, policyARN string) error { + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.DetachUserPolicyWithContext(ctx, &iam.DetachUserPolicyInput{ + UserName: aws.String(user), + PolicyArn: aws.String(policyARN), }) }, iam.ErrCodeConcurrentModificationException) - return err + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil + } + + if err != nil { + return fmt.Errorf("detaching IAM Policy (%s) from IAM User (%s): %w", policyARN, user, err) + } + + return nil +} + +func findAttachedUserPolicyByTwoPartKey(ctx context.Context, conn *iam.IAM, userName, policyARN string) (*iam.AttachedPolicy, error) { + input := &iam.ListAttachedUserPoliciesInput{ + UserName: aws.String(userName), + } + + return findAttachedUserPolicy(ctx, conn, input, func(v *iam.AttachedPolicy) bool { + return aws.StringValue(v.PolicyArn) == policyARN + }) +} + +func findAttachedUserPolicy(ctx context.Context, conn *iam.IAM, input *iam.ListAttachedUserPoliciesInput, filter tfslices.Predicate[*iam.AttachedPolicy]) (*iam.AttachedPolicy, error) { + output, err := findAttachedUserPolicies(ctx, conn, input, filter) + + if err != nil { + return nil, err + } + + return tfresource.AssertSinglePtrResult(output) } -func DetachPolicyFromUser(ctx context.Context, conn *iam.IAM, user string, arn string) error { - _, err := conn.DetachUserPolicyWithContext(ctx, &iam.DetachUserPolicyInput{ - UserName: aws.String(user), - PolicyArn: aws.String(arn), +func findAttachedUserPolicies(ctx context.Context, conn *iam.IAM, input *iam.ListAttachedUserPoliciesInput, filter tfslices.Predicate[*iam.AttachedPolicy]) ([]*iam.AttachedPolicy, error) { + var output []*iam.AttachedPolicy + + err := conn.ListAttachedUserPoliciesPagesWithContext(ctx, input, func(page *iam.ListAttachedUserPoliciesOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + for _, v := range page.AttachedPolicies { + if v != nil && filter(v) { + output = append(output, v) + } + } + + return !lastPage }) - return err + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + return output, nil } From 2af0899d9933ab1e6f0398b7c8f918ffc0ac69d0 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 14:32:27 -0500 Subject: [PATCH 11/20] r/aws_iam_user_policy_attachment: Tidy up acceptance tests. --- internal/service/iam/exports_test.go | 3 + .../iam/user_policy_attachment_test.go | 173 +++++++++++------- 2 files changed, 107 insertions(+), 69 deletions(-) diff --git a/internal/service/iam/exports_test.go b/internal/service/iam/exports_test.go index 879596d4fa3..0728525119d 100644 --- a/internal/service/iam/exports_test.go +++ b/internal/service/iam/exports_test.go @@ -6,7 +6,10 @@ package iam // Exports for use in tests only. var ( ResourceGroupPolicyAttachment = resourceGroupPolicyAttachment + ResourceUserPolicyAttachment = resourceUserPolicyAttachment FindAttachedGroupPolicies = findAttachedGroupPolicies FindAttachedGroupPolicyByTwoPartKey = findAttachedGroupPolicyByTwoPartKey + FindAttachedUserPolicies = findAttachedUserPolicies + FindAttachedUserPolicyByTwoPartKey = findAttachedUserPolicyByTwoPartKey ) diff --git a/internal/service/iam/user_policy_attachment_test.go b/internal/service/iam/user_policy_attachment_test.go index ff0359ff3b7..a51b3b0f6e1 100644 --- a/internal/service/iam/user_policy_attachment_test.go +++ b/internal/service/iam/user_policy_attachment_test.go @@ -6,7 +6,6 @@ package iam_test import ( "context" "fmt" - "strings" "testing" "github.com/aws/aws-sdk-go/aws" @@ -17,33 +16,36 @@ import ( "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" + tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMUserPolicyAttachment_basic(t *testing.T) { ctx := acctest.Context(t) - var out iam.ListAttachedUserPoliciesOutput - rName := sdkacctest.RandString(10) - policyName1 := fmt.Sprintf("test-policy-%s", sdkacctest.RandString(10)) - policyName2 := fmt.Sprintf("test-policy-%s", sdkacctest.RandString(10)) - policyName3 := fmt.Sprintf("test-policy-%s", sdkacctest.RandString(10)) + userName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName3 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_user_policy_attachment.test1" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckUserPolicyAttachmentDestroy, + CheckDestroy: testAccCheckUserPolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccUserPolicyAttachmentConfig_attach(rName, policyName1), + Config: testAccUserPolicyAttachmentConfig_attach(userName, policyName1), Check: resource.ComposeTestCheckFunc( - testAccCheckUserPolicyAttachmentExists(ctx, "aws_iam_user_policy_attachment.test-attach", 1, &out), - testAccCheckUserPolicyAttachmentAttributes([]string{policyName1}, &out), + testAccCheckUserPolicyAttachmentExists(ctx, resourceName), + testAccCheckUserPolicyAttachmentCount(ctx, userName, 1), ), }, { - ResourceName: "aws_iam_user_policy_attachment.test-attach", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccUserPolicyAttachmentImportStateIdFunc("aws_iam_user_policy_attachment.test-attach"), + ImportStateIdFunc: testAccUserPolicyAttachmentImportStateIdFunc(resourceName), // We do not have a way to align IDs since the Create function uses id.PrefixedUniqueId() // Failed state verification, resource with ID USER-POLICYARN not found // ImportStateVerify: true, @@ -62,66 +64,99 @@ func TestAccIAMUserPolicyAttachment_basic(t *testing.T) { }, }, { - Config: testAccUserPolicyAttachmentConfig_attachUpdate(rName, policyName1, policyName2, policyName3), + Config: testAccUserPolicyAttachmentConfig_attachUpdate(userName, policyName1, policyName2, policyName3), Check: resource.ComposeTestCheckFunc( - testAccCheckUserPolicyAttachmentExists(ctx, "aws_iam_user_policy_attachment.test-attach", 2, &out), - testAccCheckUserPolicyAttachmentAttributes([]string{policyName2, policyName3}, &out), + testAccCheckUserPolicyAttachmentExists(ctx, resourceName), + testAccCheckUserPolicyAttachmentCount(ctx, userName, 2), ), }, }, }) } -func testAccCheckUserPolicyAttachmentDestroy(s *terraform.State) error { - return nil +func TestAccIAMUserPolicyAttachment_disappears(t *testing.T) { + ctx := acctest.Context(t) + userName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_user_policy_attachment.test1" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckUserPolicyAttachmentDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccUserPolicyAttachmentConfig_attach(userName, policyName), + Check: resource.ComposeTestCheckFunc( + testAccCheckUserPolicyAttachmentExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceUserPolicyAttachment(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func testAccCheckUserPolicyAttachmentDestroy(ctx context.Context) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_iam_user_policy_attachment" { + continue + } + + _, err := tfiam.FindAttachedUserPolicyByTwoPartKey(ctx, conn, rs.Primary.Attributes["user"], rs.Primary.Attributes["policy_arn"]) + + if tfresource.NotFound(err) { + continue + } + + if err != nil { + return err + } + + return fmt.Errorf("IAM User Policy Attachment %s still exists", rs.Primary.ID) + } + + return nil + } } -func testAccCheckUserPolicyAttachmentExists(ctx context.Context, n string, c int, out *iam.ListAttachedUserPoliciesOutput) resource.TestCheckFunc { +func testAccCheckUserPolicyAttachmentExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) } - if rs.Primary.ID == "" { - return fmt.Errorf("No policy name is set") - } - conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) - user := rs.Primary.Attributes["user"] - attachedPolicies, err := conn.ListAttachedUserPoliciesWithContext(ctx, &iam.ListAttachedUserPoliciesInput{ - UserName: aws.String(user), - }) - if err != nil { - return fmt.Errorf("Error: Failed to get attached policies for user %s (%s)", user, n) - } - if c != len(attachedPolicies.AttachedPolicies) { - return fmt.Errorf("Error: User (%s) has wrong number of policies attached on initial creation", n) - } + _, err := tfiam.FindAttachedUserPolicyByTwoPartKey(ctx, conn, rs.Primary.Attributes["user"], rs.Primary.Attributes["policy_arn"]) - *out = *attachedPolicies - return nil + return err } } -func testAccCheckUserPolicyAttachmentAttributes(policies []string, out *iam.ListAttachedUserPoliciesOutput) resource.TestCheckFunc { +func testAccCheckUserPolicyAttachmentCount(ctx context.Context, userName string, want int) resource.TestCheckFunc { return func(s *terraform.State) error { - matched := 0 - - for _, p := range policies { - for _, ap := range out.AttachedPolicies { - // *ap.PolicyArn like arn:aws:iam::111111111111:policy/test-policy - parts := strings.Split(*ap.PolicyArn, "/") - if len(parts) == 2 && p == parts[1] { - matched++ - } - } + conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) + + input := &iam.ListAttachedUserPoliciesInput{ + UserName: aws.String(userName), } - if matched != len(policies) || matched != len(out.AttachedPolicies) { - return fmt.Errorf("Error: Number of attached policies was incorrect: expected %d matched policies, matched %d of %d", len(policies), matched, len(out.AttachedPolicies)) + output, err := tfiam.FindAttachedUserPolicies(ctx, conn, input, tfslices.PredicateTrue[*iam.AttachedPolicy]()) + + if err != nil { + return err } - return nil + + if got := len(output); got != want { + return fmt.Errorf("UserPolicyAttachmentCount(%q) = %v, want %v", userName, got, want) + } + + return err } } @@ -138,12 +173,12 @@ func testAccUserPolicyAttachmentImportStateIdFunc(resourceName string) resource. func testAccUserPolicyAttachmentConfig_attach(rName, policyName string) string { return fmt.Sprintf(` -resource "aws_iam_user" "user" { - name = "test-user-%s" +resource "aws_iam_user" "test" { + name = %[1]q } -resource "aws_iam_policy" "policy" { - name = "%s" +resource "aws_iam_policy" "test1" { + name = %[2]q description = "A test policy" policy = < Date: Tue, 14 Nov 2023 14:35:24 -0500 Subject: [PATCH 12/20] Acceptance test output: % make testacc TESTARGS='-run=TestAccIAMUserPolicyAttachment_' PKG=iam ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run=TestAccIAMUserPolicyAttachment_ -timeout 360m === RUN TestAccIAMUserPolicyAttachment_basic === PAUSE TestAccIAMUserPolicyAttachment_basic === RUN TestAccIAMUserPolicyAttachment_disappears === PAUSE TestAccIAMUserPolicyAttachment_disappears === CONT TestAccIAMUserPolicyAttachment_basic === CONT TestAccIAMUserPolicyAttachment_disappears --- PASS: TestAccIAMUserPolicyAttachment_disappears (23.71s) --- PASS: TestAccIAMUserPolicyAttachment_basic (41.76s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/iam 47.241s From f02e51775f1dfff36be96f064f4dca66b0b34989 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 14:53:30 -0500 Subject: [PATCH 13/20] r/aws_iam_role_policy_attachment: Tidy up. --- .changelog/34378.txt | 2 +- .../service/iam/group_policy_attachment.go | 3 + .../service/iam/role_policy_attachment.go | 152 +++++++++--------- internal/service/iam/service_package_gen.go | 4 +- .../service/iam/user_policy_attachment.go | 2 +- 5 files changed, 88 insertions(+), 75 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 4bcac297d7b..2ea76fad577 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -7,7 +7,7 @@ resource/aws_iam_policy_attachment: Retry `ConcurrentModificationException` erro ``` ```release-note:bug -resource/aws_iam_role_policy_attachment: Retry `ConcurrentModificationException` errors on create +resource/aws_iam_role_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete ``` ```release-note:bug diff --git a/internal/service/iam/group_policy_attachment.go b/internal/service/iam/group_policy_attachment.go index 94c2940634c..fb22b733f87 100644 --- a/internal/service/iam/group_policy_attachment.go +++ b/internal/service/iam/group_policy_attachment.go @@ -109,11 +109,14 @@ func resourceGroupPolicyAttachmentImport(ctx context.Context, d *schema.Resource if len(idParts) != 2 || idParts[0] == "" || idParts[1] == "" { return nil, fmt.Errorf("unexpected format of ID (%q), expected /", d.Id()) } + groupName := idParts[0] policyARN := idParts[1] + d.Set("group", groupName) d.Set("policy_arn", policyARN) d.SetId(fmt.Sprintf("%s-%s", groupName, policyARN)) + return []*schema.ResourceData{d}, nil } diff --git a/internal/service/iam/role_policy_attachment.go b/internal/service/iam/role_policy_attachment.go index 453023ff698..70342aa6152 100644 --- a/internal/service/iam/role_policy_attachment.go +++ b/internal/service/iam/role_policy_attachment.go @@ -18,12 +18,13 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_iam_role_policy_attachment") -func ResourceRolePolicyAttachment() *schema.Resource { +// @SDKResource("aws_iam_role_policy_attachment", name="Role Policy Attachment") +func resourceRolePolicyAttachment() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceRolePolicyAttachmentCreate, ReadWithoutTimeout: resourceRolePolicyAttachmentRead, @@ -54,11 +55,10 @@ func resourceRolePolicyAttachmentCreate(ctx context.Context, d *schema.ResourceD conn := meta.(*conns.AWSClient).IAMConn(ctx) role := d.Get("role").(string) - arn := d.Get("policy_arn").(string) + policyARN := d.Get("policy_arn").(string) - err := attachPolicyToRole(ctx, conn, role, arn) - if err != nil { - return sdkdiag.AppendErrorf(diags, "attaching policy %s to IAM Role %s: %v", arn, role, err) + if err := attachPolicyToRole(ctx, conn, role, policyARN); err != nil { + return sdkdiag.AppendFromErr(diags, err) } //lintignore:R016 // Allow legacy unstable ID usage in managed resource @@ -70,76 +70,37 @@ func resourceRolePolicyAttachmentCreate(ctx context.Context, d *schema.ResourceD func resourceRolePolicyAttachmentRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) + role := d.Get("role").(string) policyARN := d.Get("policy_arn").(string) - // Human friendly ID for error messages since d.Id() is non-descriptive + // Human friendly ID for error messages since d.Id() is non-descriptive. id := fmt.Sprintf("%s:%s", role, policyARN) - var hasPolicyAttachment bool - - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { - var err error - - hasPolicyAttachment, err = RoleHasPolicyARNAttachment(ctx, conn, role, policyARN) - - if d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return retry.RetryableError(err) - } - - if err != nil { - return retry.NonRetryableError(err) - } - - if d.IsNewResource() && !hasPolicyAttachment { - return retry.RetryableError(&retry.NotFoundError{ - LastError: fmt.Errorf("IAM Role Managed Policy Attachment (%s) not found", id), - }) - } - - return nil - }) - - if tfresource.TimedOut(err) { - hasPolicyAttachment, err = RoleHasPolicyARNAttachment(ctx, conn, role, policyARN) - } + _, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { + return findAttachedRolePolicyByTwoPartKey(ctx, conn, role, policyARN) + }, d.IsNewResource()) - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - log.Printf("[WARN] IAM Role Managed Policy Attachment (%s) not found, removing from state", id) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] IAM Role Policy Attachment (%s) not found, removing from state", id) d.SetId("") return diags } if err != nil { - return sdkdiag.AppendErrorf(diags, "reading IAM Role Managed Policy Attachment (%s): %s", id, err) + return sdkdiag.AppendErrorf(diags, "reading IAM Role Policy Attachment (%s): %s", id, err) } - if !d.IsNewResource() && !hasPolicyAttachment { - log.Printf("[WARN] IAM Role Managed Policy Attachment (%s) not found, removing from state", id) - d.SetId("") - return diags - } - - d.Set("policy_arn", policyARN) - d.Set("role", role) - return diags } func resourceRolePolicyAttachmentDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - role := d.Get("role").(string) - arn := d.Get("policy_arn").(string) - - err := DetachPolicyFromRole(ctx, conn, role, arn) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - return diags + if err := detachPolicyFromRole(ctx, conn, d.Get("role").(string), d.Get("policy_arn").(string)); err != nil { + return sdkdiag.AppendFromErr(diags, err) } - if err != nil { - return sdkdiag.AppendErrorf(diags, "removing policy %s from IAM Role %s: %v", arn, role, err) - } return diags } @@ -159,40 +120,87 @@ func resourceRolePolicyAttachmentImport(ctx context.Context, d *schema.ResourceD return []*schema.ResourceData{d}, nil } -func attachPolicyToRole(ctx context.Context, conn *iam.IAM, role string, arn string) error { +func attachPolicyToRole(ctx context.Context, conn *iam.IAM, role, policyARN string) error { _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { return conn.AttachRolePolicyWithContext(ctx, &iam.AttachRolePolicyInput{ + PolicyArn: aws.String(policyARN), + RoleName: aws.String(role), + }) + }, iam.ErrCodeConcurrentModificationException) + + if err != nil { + return fmt.Errorf("attaching IAM Policy (%s) to IAM Role (%s): %w", policyARN, role, err) + } + + return nil +} + +func detachPolicyFromRole(ctx context.Context, conn *iam.IAM, role, policyARN string) error { + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { + return conn.DetachRolePolicyWithContext(ctx, &iam.DetachRolePolicyInput{ RoleName: aws.String(role), - PolicyArn: aws.String(arn), + PolicyArn: aws.String(policyARN), }) }, iam.ErrCodeConcurrentModificationException) - return err + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil + } + + if err != nil { + return fmt.Errorf("detaching IAM Policy (%s) from IAM Role (%s): %w", policyARN, role, err) + } + + return nil } -func DetachPolicyFromRole(ctx context.Context, conn *iam.IAM, role string, arn string) error { - _, err := conn.DetachRolePolicyWithContext(ctx, &iam.DetachRolePolicyInput{ - RoleName: aws.String(role), - PolicyArn: aws.String(arn), +func findAttachedRolePolicyByTwoPartKey(ctx context.Context, conn *iam.IAM, roleName, policyARN string) (*iam.AttachedPolicy, error) { + input := &iam.ListAttachedRolePoliciesInput{ + RoleName: aws.String(roleName), + } + + return findAttachedRolePolicy(ctx, conn, input, func(v *iam.AttachedPolicy) bool { + return aws.StringValue(v.PolicyArn) == policyARN }) - return err } -func RoleHasPolicyARNAttachment(ctx context.Context, conn *iam.IAM, role string, policyARN string) (bool, error) { - hasPolicyAttachment := false - input := &iam.ListAttachedRolePoliciesInput{ - RoleName: aws.String(role), +func findAttachedRolePolicy(ctx context.Context, conn *iam.IAM, input *iam.ListAttachedRolePoliciesInput, filter tfslices.Predicate[*iam.AttachedPolicy]) (*iam.AttachedPolicy, error) { + output, err := findAttachedRolePolicies(ctx, conn, input, filter) + + if err != nil { + return nil, err } + return tfresource.AssertSinglePtrResult(output) +} + +func findAttachedRolePolicies(ctx context.Context, conn *iam.IAM, input *iam.ListAttachedRolePoliciesInput, filter tfslices.Predicate[*iam.AttachedPolicy]) ([]*iam.AttachedPolicy, error) { + var output []*iam.AttachedPolicy + err := conn.ListAttachedRolePoliciesPagesWithContext(ctx, input, func(page *iam.ListAttachedRolePoliciesOutput, lastPage bool) bool { - for _, p := range page.AttachedPolicies { - if aws.StringValue(p.PolicyArn) == policyARN { - hasPolicyAttachment = true - return false + if page == nil { + return !lastPage + } + + for _, v := range page.AttachedPolicies { + if v != nil && filter(v) { + output = append(output, v) } } return !lastPage }) - return hasPolicyAttachment, err + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + return output, nil } diff --git a/internal/service/iam/service_package_gen.go b/internal/service/iam/service_package_gen.go index 2bc276d3b65..b67b8e30108 100644 --- a/internal/service/iam/service_package_gen.go +++ b/internal/service/iam/service_package_gen.go @@ -160,8 +160,9 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka TypeName: "aws_iam_role_policy", }, { - Factory: ResourceRolePolicyAttachment, + Factory: resourceRolePolicyAttachment, TypeName: "aws_iam_role_policy_attachment", + Name: "Role Policy Attachment", }, { Factory: ResourceSAMLProvider, @@ -215,6 +216,7 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka { Factory: resourceUserPolicyAttachment, TypeName: "aws_iam_user_policy_attachment", + Name: "User Policy Attachment", }, { Factory: ResourceUserSSHKey, diff --git a/internal/service/iam/user_policy_attachment.go b/internal/service/iam/user_policy_attachment.go index 64b6537a59d..a091c29db9b 100644 --- a/internal/service/iam/user_policy_attachment.go +++ b/internal/service/iam/user_policy_attachment.go @@ -23,7 +23,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_iam_user_policy_attachment") +// @SDKResource("aws_iam_user_policy_attachment", name="User Policy Attachment") func resourceUserPolicyAttachment() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceUserPolicyAttachmentCreate, From 6c2809de9b98e2efc3d33803b999f99eb246c298 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 15:05:14 -0500 Subject: [PATCH 14/20] r/aws_iam_role_policy_attachment: Tidy up acceptance tests. --- internal/service/iam/exports_test.go | 3 + internal/service/iam/role.go | 7 +- .../iam/role_policy_attachment_test.go | 206 ++++++------------ 3 files changed, 74 insertions(+), 142 deletions(-) diff --git a/internal/service/iam/exports_test.go b/internal/service/iam/exports_test.go index 0728525119d..644969fd3c8 100644 --- a/internal/service/iam/exports_test.go +++ b/internal/service/iam/exports_test.go @@ -6,10 +6,13 @@ package iam // Exports for use in tests only. var ( ResourceGroupPolicyAttachment = resourceGroupPolicyAttachment + ResourceRolePolicyAttachment = resourceRolePolicyAttachment ResourceUserPolicyAttachment = resourceUserPolicyAttachment FindAttachedGroupPolicies = findAttachedGroupPolicies FindAttachedGroupPolicyByTwoPartKey = findAttachedGroupPolicyByTwoPartKey + FindAttachedRolePolicies = findAttachedRolePolicies + FindAttachedRolePolicyByTwoPartKey = findAttachedRolePolicyByTwoPartKey FindAttachedUserPolicies = findAttachedUserPolicies FindAttachedUserPolicyByTwoPartKey = findAttachedUserPolicyByTwoPartKey ) diff --git a/internal/service/iam/role.go b/internal/service/iam/role.go index 4ca2bc1c435..c1b7d41be58 100644 --- a/internal/service/iam/role.go +++ b/internal/service/iam/role.go @@ -870,16 +870,15 @@ func addRoleInlinePolicies(ctx context.Context, policies []*iam.PutRolePolicyInp func addRoleManagedPolicies(ctx context.Context, roleName string, policies []*string, meta interface{}) error { conn := meta.(*conns.AWSClient).IAMConn(ctx) + var errs []error - var errs *multierror.Error for _, arn := range policies { if err := attachPolicyToRole(ctx, conn, roleName, aws.StringValue(arn)); err != nil { - newErr := fmt.Errorf("attaching managed policy (%s): %w", aws.StringValue(arn), err) - errs = multierror.Append(errs, newErr) + errs = append(errs, err) } } - return errs.ErrorOrNil() + return errors.Join(errs...) } func readRoleInlinePolicies(ctx context.Context, roleName string, meta interface{}) ([]*iam.PutRolePolicyInput, error) { diff --git a/internal/service/iam/role_policy_attachment_test.go b/internal/service/iam/role_policy_attachment_test.go index 13bada1cbd2..0e1de00ff2a 100644 --- a/internal/service/iam/role_policy_attachment_test.go +++ b/internal/service/iam/role_policy_attachment_test.go @@ -6,28 +6,28 @@ package iam_test import ( "context" "fmt" - "strings" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/iam" - "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" tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMRolePolicyAttachment_basic(t *testing.T) { ctx := acctest.Context(t) - var out iam.ListAttachedRolePoliciesOutput - rInt := sdkacctest.RandInt() - testPolicy := fmt.Sprintf("tf-acctest-%d", rInt) - testPolicy2 := fmt.Sprintf("tf-acctest2-%d", rInt) - testPolicy3 := fmt.Sprintf("tf-acctest3-%d", rInt) + roleName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName3 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_role_policy_attachment.test1" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -36,16 +36,16 @@ func TestAccIAMRolePolicyAttachment_basic(t *testing.T) { CheckDestroy: testAccCheckRolePolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccRolePolicyAttachmentConfig_attach(rInt), + Config: testAccRolePolicyAttachmentConfig_attach(roleName, policyName1), Check: resource.ComposeTestCheckFunc( - testAccCheckRolePolicyAttachmentExists(ctx, "aws_iam_role_policy_attachment.test-attach", 1, &out), - testAccCheckRolePolicyAttachmentAttributes([]string{testPolicy}, &out), + testAccCheckRolePolicyAttachmentExists(ctx, resourceName), + testAccCheckRolePolicyAttachmentCount(ctx, roleName, 1), ), }, { - ResourceName: "aws_iam_role_policy_attachment.test-attach", + ResourceName: resourceName, ImportState: true, - ImportStateIdFunc: testAccRolePolicyAttachmentImportStateIdFunc("aws_iam_role_policy_attachment.test-attach"), + ImportStateIdFunc: testAccRolePolicyAttachmentImportStateIdFunc(resourceName), // We do not have a way to align IDs since the Create function uses id.PrefixedUniqueId() // Failed state verification, resource with ID ROLE-POLICYARN not found // ImportStateVerify: true, @@ -64,10 +64,10 @@ func TestAccIAMRolePolicyAttachment_basic(t *testing.T) { }, }, { - Config: testAccRolePolicyAttachmentConfig_attachUpdate(rInt), + Config: testAccRolePolicyAttachmentConfig_attachUpdate(roleName, policyName1, policyName2, policyName3), Check: resource.ComposeTestCheckFunc( - testAccCheckRolePolicyAttachmentExists(ctx, "aws_iam_role_policy_attachment.test-attach", 2, &out), - testAccCheckRolePolicyAttachmentAttributes([]string{testPolicy2, testPolicy3}, &out), + testAccCheckRolePolicyAttachmentExists(ctx, resourceName), + testAccCheckRolePolicyAttachmentCount(ctx, roleName, 2), ), }, }, @@ -76,10 +76,9 @@ func TestAccIAMRolePolicyAttachment_basic(t *testing.T) { func TestAccIAMRolePolicyAttachment_disappears(t *testing.T) { ctx := acctest.Context(t) - var attachedRolePolicies iam.ListAttachedRolePoliciesOutput - - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - resourceName := "aws_iam_role_policy_attachment.test" + roleName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_role_policy_attachment.test1" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -88,10 +87,10 @@ func TestAccIAMRolePolicyAttachment_disappears(t *testing.T) { CheckDestroy: testAccCheckRolePolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccRolePolicyAttachmentConfig_basic(rName), + Config: testAccRolePolicyAttachmentConfig_attach(roleName, policyName), Check: resource.ComposeTestCheckFunc( - testAccCheckRolePolicyAttachmentExists(ctx, resourceName, 1, &attachedRolePolicies), - testAccCheckRolePolicyAttachmentDisappears(ctx, resourceName), + testAccCheckRolePolicyAttachmentExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceRolePolicyAttachment(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -101,12 +100,10 @@ func TestAccIAMRolePolicyAttachment_disappears(t *testing.T) { func TestAccIAMRolePolicyAttachment_Disappears_role(t *testing.T) { ctx := acctest.Context(t) - var attachedRolePolicies iam.ListAttachedRolePoliciesOutput - var role iam.Role - - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + roleName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_role_policy_attachment.test1" iamRoleResourceName := "aws_iam_role.test" - resourceName := "aws_iam_role_policy_attachment.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -115,12 +112,11 @@ func TestAccIAMRolePolicyAttachment_Disappears_role(t *testing.T) { CheckDestroy: testAccCheckRolePolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccRolePolicyAttachmentConfig_basic(rName), + Config: testAccRolePolicyAttachmentConfig_attach(roleName, policyName), Check: resource.ComposeTestCheckFunc( - testAccCheckRoleExists(ctx, iamRoleResourceName, &role), - testAccCheckRolePolicyAttachmentExists(ctx, resourceName, 1, &attachedRolePolicies), + testAccCheckRolePolicyAttachmentExists(ctx, resourceName), // DeleteConflict: Cannot delete entity, must detach all policies first. - testAccCheckRolePolicyAttachmentDisappears(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceRolePolicyAttachment(), resourceName), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourceRole(), iamRoleResourceName), ), ExpectNonEmptyPlan: true, @@ -138,12 +134,9 @@ func testAccCheckRolePolicyAttachmentDestroy(ctx context.Context) resource.TestC continue } - policyARN := rs.Primary.Attributes["policy_arn"] - role := rs.Primary.Attributes["role"] + _, err := tfiam.FindAttachedRolePolicyByTwoPartKey(ctx, conn, rs.Primary.Attributes["role"], rs.Primary.Attributes["policy_arn"]) - hasPolicyAttachment, err := tfiam.RoleHasPolicyARNAttachment(ctx, conn, role, policyARN) - - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + if tfresource.NotFound(err) { continue } @@ -151,78 +144,46 @@ func testAccCheckRolePolicyAttachmentDestroy(ctx context.Context) resource.TestC return err } - if hasPolicyAttachment { - return fmt.Errorf("IAM Role (%s) Policy Attachment (%s) still exists", role, policyARN) - } + return fmt.Errorf("IAM Role Policy Attachment %s still exists", rs.Primary.ID) } return nil } } -func testAccCheckRolePolicyAttachmentExists(ctx context.Context, n string, c int, out *iam.ListAttachedRolePoliciesOutput) resource.TestCheckFunc { +func testAccCheckRolePolicyAttachmentExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) } - if rs.Primary.ID == "" { - return fmt.Errorf("No policy name is set") - } - conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) - role := rs.Primary.Attributes["role"] - attachedPolicies, err := conn.ListAttachedRolePoliciesWithContext(ctx, &iam.ListAttachedRolePoliciesInput{ - RoleName: aws.String(role), - }) - if err != nil { - return fmt.Errorf("Error: Failed to get attached policies for role %s (%s)", role, n) - } - if c != len(attachedPolicies.AttachedPolicies) { - return fmt.Errorf("Error: Role (%s) has wrong number of policies attached on initial creation", n) - } + _, err := tfiam.FindAttachedRolePolicyByTwoPartKey(ctx, conn, rs.Primary.Attributes["role"], rs.Primary.Attributes["policy_arn"]) - *out = *attachedPolicies - return nil + return err } } -func testAccCheckRolePolicyAttachmentAttributes(policies []string, out *iam.ListAttachedRolePoliciesOutput) resource.TestCheckFunc { - return func(s *terraform.State) error { - matched := 0 - - for _, p := range policies { - for _, ap := range out.AttachedPolicies { - // *ap.PolicyArn like arn:aws:iam::111111111111:policy/test-policy - parts := strings.Split(*ap.PolicyArn, "/") - if len(parts) == 2 && p == parts[1] { - matched++ - } - } - } - if matched != len(policies) || matched != len(out.AttachedPolicies) { - return fmt.Errorf("Error: Number of attached policies was incorrect: expected %d matched policies, matched %d of %d", len(policies), matched, len(out.AttachedPolicies)) - } - return nil - } -} - -func testAccCheckRolePolicyAttachmentDisappears(ctx context.Context, resourceName string) resource.TestCheckFunc { +func testAccCheckRolePolicyAttachmentCount(ctx context.Context, roleName string, want int) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) - rs, ok := s.RootModule().Resources[resourceName] + input := &iam.ListAttachedRolePoliciesInput{ + RoleName: aws.String(roleName), + } + output, err := tfiam.FindAttachedRolePolicies(ctx, conn, input, tfslices.PredicateTrue[*iam.AttachedPolicy]()) - if !ok { - return fmt.Errorf("Not found: %s", resourceName) + if err != nil { + return err } - policyARN := rs.Primary.Attributes["policy_arn"] - role := rs.Primary.Attributes["role"] + if got := len(output); got != want { + return fmt.Errorf("RolePolicyAttachmentCount(%q) = %v, want %v", roleName, got, want) + } - return tfiam.DetachPolicyFromRole(ctx, conn, role, policyARN) + return err } } @@ -237,10 +198,10 @@ func testAccRolePolicyAttachmentImportStateIdFunc(resourceName string) resource. } } -func testAccRolePolicyAttachmentConfig_attach(rInt int) string { +func testAccRolePolicyAttachmentConfig_attach(roleName, policyName string) string { return fmt.Sprintf(` -resource "aws_iam_role" "role" { - name = "test-role-%d" +resource "aws_iam_role" "test" { + name = %[1]q assume_role_policy = < Date: Tue, 14 Nov 2023 15:09:24 -0500 Subject: [PATCH 15/20] Acceptance test output: % make testacc TESTARGS='-run=TestAccIAMRolePolicyAttachment_' PKG=iam ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run=TestAccIAMRolePolicyAttachment_ -timeout 360m === RUN TestAccIAMRolePolicyAttachment_basic === PAUSE TestAccIAMRolePolicyAttachment_basic === RUN TestAccIAMRolePolicyAttachment_disappears === PAUSE TestAccIAMRolePolicyAttachment_disappears === RUN TestAccIAMRolePolicyAttachment_Disappears_role === PAUSE TestAccIAMRolePolicyAttachment_Disappears_role === CONT TestAccIAMRolePolicyAttachment_basic === CONT TestAccIAMRolePolicyAttachment_Disappears_role === CONT TestAccIAMRolePolicyAttachment_disappears --- PASS: TestAccIAMRolePolicyAttachment_Disappears_role (28.46s) --- PASS: TestAccIAMRolePolicyAttachment_disappears (28.48s) --- PASS: TestAccIAMRolePolicyAttachment_basic (47.56s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/iam 53.610s From 55b6ee9423409979cf97bc7fb558eb073ee6cae1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 15:55:05 -0500 Subject: [PATCH 16/20] r/aws_iam_policy_attachment: Tidy up. --- .changelog/34378.txt | 2 +- internal/service/iam/exports_test.go | 1 + internal/service/iam/policy.go | 4 +- internal/service/iam/policy_attachment.go | 436 ++++++++---------- internal/service/iam/policy_data_source.go | 2 +- .../service/iam/role_policy_attachment.go | 2 +- internal/service/iam/service_package_gen.go | 3 +- .../service/iam/user_policy_attachment.go | 4 +- 8 files changed, 215 insertions(+), 239 deletions(-) diff --git a/.changelog/34378.txt b/.changelog/34378.txt index 2ea76fad577..61d8b12dc44 100644 --- a/.changelog/34378.txt +++ b/.changelog/34378.txt @@ -3,7 +3,7 @@ resource/aws_iam_group_policy_attachment: Retry `ConcurrentModificationException ``` ```release-note:bug -resource/aws_iam_policy_attachment: Retry `ConcurrentModificationException` errors on create +resource/aws_iam_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete ``` ```release-note:bug diff --git a/internal/service/iam/exports_test.go b/internal/service/iam/exports_test.go index 644969fd3c8..6f90daa8667 100644 --- a/internal/service/iam/exports_test.go +++ b/internal/service/iam/exports_test.go @@ -15,4 +15,5 @@ var ( FindAttachedRolePolicyByTwoPartKey = findAttachedRolePolicyByTwoPartKey FindAttachedUserPolicies = findAttachedUserPolicies FindAttachedUserPolicyByTwoPartKey = findAttachedUserPolicyByTwoPartKey + FindPolicyByARN = findPolicyByARN ) diff --git a/internal/service/iam/policy.go b/internal/service/iam/policy.go index 68302a637f6..310891229e2 100644 --- a/internal/service/iam/policy.go +++ b/internal/service/iam/policy.go @@ -162,7 +162,7 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf outputRaw, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) { iamPolicy := &policyWithVersion{} - if v, err := FindPolicyByARN(ctx, conn, d.Id()); err == nil { + if v, err := findPolicyByARN(ctx, conn, d.Id()); err == nil { iamPolicy.policy = v } else { return nil, err @@ -348,7 +348,7 @@ func policyDeleteVersion(ctx context.Context, conn *iam.IAM, arn, versionID stri return nil } -func FindPolicyByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.Policy, error) { +func findPolicyByARN(ctx context.Context, conn *iam.IAM, arn string) (*iam.Policy, error) { input := &iam.GetPolicyInput{ PolicyArn: aws.String(arn), } diff --git a/internal/service/iam/policy_attachment.go b/internal/service/iam/policy_attachment.go index 349cb76fe9b..1f50997cc26 100644 --- a/internal/service/iam/policy_attachment.go +++ b/internal/service/iam/policy_attachment.go @@ -5,24 +5,26 @@ package iam import ( "context" - "fmt" + "errors" "log" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" "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/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" + tfslices "github.com/hashicorp/terraform-provider-aws/internal/slices" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) -// @SDKResource("aws_iam_policy_attachment") -func ResourcePolicyAttachment() *schema.Resource { +// @SDKResource("aws_iam_policy_attachment", name="Policy Attachment") +func resourcePolicyAttachment() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourcePolicyAttachmentCreate, ReadWithoutTimeout: resourcePolicyAttachmentRead, @@ -31,9 +33,10 @@ func ResourcePolicyAttachment() *schema.Resource { Schema: map[string]*schema.Schema{ "groups": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + AtLeastOneOf: []string{"groups", "roles", "users"}, }, "name": { Type: schema.TypeString, @@ -48,14 +51,16 @@ func ResourcePolicyAttachment() *schema.Resource { ValidateFunc: verify.ValidARN, }, "roles": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + AtLeastOneOf: []string{"groups", "roles", "users"}, }, "users": { - Type: schema.TypeSet, - Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + AtLeastOneOf: []string{"groups", "roles", "users"}, }, }, } @@ -65,84 +70,51 @@ func resourcePolicyAttachmentCreate(ctx context.Context, d *schema.ResourceData, var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - name := d.Get("name").(string) - arn := d.Get("policy_arn").(string) - users := flex.ExpandStringSet(d.Get("users").(*schema.Set)) - roles := flex.ExpandStringSet(d.Get("roles").(*schema.Set)) - groups := flex.ExpandStringSet(d.Get("groups").(*schema.Set)) - - if len(users) == 0 && len(roles) == 0 && len(groups) == 0 { - return sdkdiag.AppendErrorf(diags, "No Users, Roles, or Groups specified for IAM Policy Attachment %s", name) - } else { - var userErr, roleErr, groupErr error - if users != nil { - userErr = attachPolicyToUsers(ctx, conn, users, arn) - } - if roles != nil { - roleErr = attachPolicyToRoles(ctx, conn, roles, arn) - } - if groups != nil { - groupErr = attachPolicyToGroups(ctx, conn, groups, arn) - } - if userErr != nil || roleErr != nil || groupErr != nil { - return composeErrors(fmt.Sprint("attaching policy with IAM Policy Attachment ", name, ":"), userErr, roleErr, groupErr) - } + policyARN := d.Get("policy_arn").(string) + var groups, roles, users []string + if v, ok := d.GetOk("groups"); ok && v.(*schema.Set).Len() > 0 { + groups = flex.ExpandStringValueSet(v.(*schema.Set)) + } + if v, ok := d.GetOk("roles"); ok && v.(*schema.Set).Len() > 0 { + roles = flex.ExpandStringValueSet(v.(*schema.Set)) + } + if v, ok := d.GetOk("users"); ok && v.(*schema.Set).Len() > 0 { + users = flex.ExpandStringValueSet(v.(*schema.Set)) + } + + diags = sdkdiag.AppendFromErr(diags, attachPolicyToGroups(ctx, conn, groups, policyARN)) + diags = sdkdiag.AppendFromErr(diags, attachPolicyToRoles(ctx, conn, roles, policyARN)) + diags = sdkdiag.AppendFromErr(diags, attachPolicyToUsers(ctx, conn, users, policyARN)) + + if diags.HasError() { + return diags } + d.SetId(d.Get("name").(string)) + return append(diags, resourcePolicyAttachmentRead(ctx, d, meta)...) } func resourcePolicyAttachmentRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - arn := d.Get("policy_arn").(string) - name := d.Get("name").(string) - - _, err := conn.GetPolicyWithContext(ctx, &iam.GetPolicyInput{ - PolicyArn: aws.String(arn), - }) - if err != nil { - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - log.Printf("[WARN] IAM Policy Attachment (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags - } - return sdkdiag.AppendErrorf(diags, "reading IAM Policy Attachment (%s): %s", d.Id(), err) - } + policyARN := d.Get("policy_arn").(string) + groups, roles, users, err := findEntitiesForPolicyByARN(ctx, conn, policyARN) - ul := make([]string, 0) - rl := make([]string, 0) - gl := make([]string, 0) - - args := iam.ListEntitiesForPolicyInput{ - PolicyArn: aws.String(arn), + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] IAM Policy Attachment (%s) not found, removing from state", d.Id()) + d.SetId("") + return diags } - err = conn.ListEntitiesForPolicyPagesWithContext(ctx, &args, func(page *iam.ListEntitiesForPolicyOutput, lastPage bool) bool { - for _, u := range page.PolicyUsers { - ul = append(ul, *u.UserName) - } - for _, r := range page.PolicyRoles { - rl = append(rl, *r.RoleName) - } - - for _, g := range page.PolicyGroups { - gl = append(gl, *g.GroupName) - } - return true - }) if err != nil { return sdkdiag.AppendErrorf(diags, "reading IAM Policy Attachment (%s): %s", d.Id(), err) } - userErr := d.Set("users", ul) - roleErr := d.Set("roles", rl) - groupErr := d.Set("groups", gl) - - if userErr != nil || roleErr != nil || groupErr != nil { - return composeErrors(fmt.Sprint("setting user, role, or group list from IAM Policy Attachment ", name, ":"), userErr, roleErr, groupErr) - } + d.Set("groups", groups) + d.Set("roles", roles) + d.Set("users", users) return diags } @@ -150,213 +122,215 @@ func resourcePolicyAttachmentRead(ctx context.Context, d *schema.ResourceData, m func resourcePolicyAttachmentUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - name := d.Get("name").(string) - var userErr, roleErr, groupErr error - if d.HasChange("users") { - userErr = updateUsers(ctx, conn, d) + if d.HasChange("groups") { + diags = sdkdiag.AppendFromErr(diags, updateGroups(ctx, conn, d)) } if d.HasChange("roles") { - roleErr = updateRoles(ctx, conn, d) + diags = sdkdiag.AppendFromErr(diags, updateRoles(ctx, conn, d)) } - if d.HasChange("groups") { - groupErr = updateGroups(ctx, conn, d) + if d.HasChange("users") { + diags = sdkdiag.AppendFromErr(diags, updateUsers(ctx, conn, d)) } - if userErr != nil || roleErr != nil || groupErr != nil { - return composeErrors(fmt.Sprint("updating user, role, or group list from IAM Policy Attachment ", name, ":"), userErr, roleErr, groupErr) + + if diags.HasError() { + return diags } + return append(diags, resourcePolicyAttachmentRead(ctx, d, meta)...) } func resourcePolicyAttachmentDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics conn := meta.(*conns.AWSClient).IAMConn(ctx) - name := d.Get("name").(string) - arn := d.Get("policy_arn").(string) - users := flex.ExpandStringSet(d.Get("users").(*schema.Set)) - roles := flex.ExpandStringSet(d.Get("roles").(*schema.Set)) - groups := flex.ExpandStringSet(d.Get("groups").(*schema.Set)) - - var userErr, roleErr, groupErr error - if len(users) != 0 { - userErr = detachPolicyFromUsers(ctx, conn, users, arn) - } - if len(roles) != 0 { - roleErr = detachPolicyFromRoles(ctx, conn, roles, arn) + + policyARN := d.Get("policy_arn").(string) + var groups, roles, users []string + if v, ok := d.GetOk("groups"); ok && v.(*schema.Set).Len() > 0 { + groups = flex.ExpandStringValueSet(v.(*schema.Set)) } - if len(groups) != 0 { - groupErr = detachPolicyFromGroups(ctx, conn, groups, arn) + if v, ok := d.GetOk("roles"); ok && v.(*schema.Set).Len() > 0 { + roles = flex.ExpandStringValueSet(v.(*schema.Set)) } - if userErr != nil || roleErr != nil || groupErr != nil { - return append(diags, composeErrors(fmt.Sprint("removing user, role, or group list from IAM Policy Detach ", name, ":"), userErr, roleErr, groupErr)...) + if v, ok := d.GetOk("users"); ok && v.(*schema.Set).Len() > 0 { + users = flex.ExpandStringValueSet(v.(*schema.Set)) } + + diags = sdkdiag.AppendFromErr(diags, detachPolicyFromGroups(ctx, conn, groups, policyARN)) + diags = sdkdiag.AppendFromErr(diags, detachPolicyFromRoles(ctx, conn, roles, policyARN)) + diags = sdkdiag.AppendFromErr(diags, detachPolicyFromUsers(ctx, conn, users, policyARN)) + return diags } -func composeErrors(desc string, uErr error, rErr error, gErr error) diag.Diagnostics { - errMsg := fmt.Sprint(desc) - errs := []error{uErr, rErr, gErr} - for _, e := range errs { - if e != nil { - errMsg = errMsg + "\n– " + e.Error() - } +func attachPolicyToGroups(ctx context.Context, conn *iam.IAM, groups []string, policyARN string) error { + var errs []error + + for _, group := range groups { + errs = append(errs, attachPolicyToGroup(ctx, conn, group, policyARN)) } - return diag.Errorf(errMsg) + + return errors.Join(errs...) } -func attachPolicyToUsers(ctx context.Context, conn *iam.IAM, users []*string, arn string) error { - for _, u := range users { - input := &iam.AttachUserPolicyInput{ - UserName: u, - PolicyArn: aws.String(arn), - } - _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { - return conn.AttachUserPolicyWithContext(ctx, input) - }, iam.ErrCodeConcurrentModificationException) - if err != nil { - return err - } +func attachPolicyToRoles(ctx context.Context, conn *iam.IAM, roles []string, policyARN string) error { + var errs []error + + for _, role := range roles { + errs = append(errs, attachPolicyToRole(ctx, conn, role, policyARN)) } - return nil + + return errors.Join(errs...) } -func attachPolicyToRoles(ctx context.Context, conn *iam.IAM, roles []*string, arn string) error { - for _, r := range roles { - input := &iam.AttachRolePolicyInput{ - RoleName: r, - PolicyArn: aws.String(arn), - } - _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { - return conn.AttachRolePolicyWithContext(ctx, input) - }, iam.ErrCodeConcurrentModificationException) - if err != nil { - return err - } + +func attachPolicyToUsers(ctx context.Context, conn *iam.IAM, users []string, policyARN string) error { + var errs []error + + for _, user := range users { + errs = append(errs, attachPolicyToUser(ctx, conn, user, policyARN)) + } + + return errors.Join(errs...) +} + +func updateGroups(ctx context.Context, conn *iam.IAM, d *schema.ResourceData) error { + policyARN := d.Get("policy_arn").(string) + o, n := d.GetChange("groups") + os, ns := o.(*schema.Set), n.(*schema.Set) + add, del := flex.ExpandStringValueSet(ns.Difference(os)), flex.ExpandStringValueSet(os.Difference(ns)) + + if err := detachPolicyFromGroups(ctx, conn, del, policyARN); err != nil { + return err } + if err := attachPolicyToGroups(ctx, conn, add, policyARN); err != nil { + return err + } + return nil } -func attachPolicyToGroups(ctx context.Context, conn *iam.IAM, groups []*string, arn string) error { - for _, g := range groups { - input := &iam.AttachGroupPolicyInput{ - GroupName: g, - PolicyArn: aws.String(arn), - } - _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { - return conn.AttachGroupPolicyWithContext(ctx, input) - }, iam.ErrCodeConcurrentModificationException) - if err != nil { - return err - } + +func updateRoles(ctx context.Context, conn *iam.IAM, d *schema.ResourceData) error { + policyARN := d.Get("policy_arn").(string) + o, n := d.GetChange("roles") + os, ns := o.(*schema.Set), n.(*schema.Set) + add, del := flex.ExpandStringValueSet(ns.Difference(os)), flex.ExpandStringValueSet(os.Difference(ns)) + + if err := detachPolicyFromRoles(ctx, conn, del, policyARN); err != nil { + return err + } + if err := attachPolicyToRoles(ctx, conn, add, policyARN); err != nil { + return err } + return nil } + func updateUsers(ctx context.Context, conn *iam.IAM, d *schema.ResourceData) error { - arn := d.Get("policy_arn").(string) + policyARN := d.Get("policy_arn").(string) o, n := d.GetChange("users") - if o == nil { - o = new(schema.Set) - } - if n == nil { - n = new(schema.Set) - } - os := o.(*schema.Set) - ns := n.(*schema.Set) - remove := flex.ExpandStringSet(os.Difference(ns)) - add := flex.ExpandStringSet(ns.Difference(os)) + os, ns := o.(*schema.Set), n.(*schema.Set) + add, del := flex.ExpandStringValueSet(ns.Difference(os)), flex.ExpandStringValueSet(os.Difference(ns)) - if rErr := detachPolicyFromUsers(ctx, conn, remove, arn); rErr != nil { - return rErr + if err := detachPolicyFromUsers(ctx, conn, del, policyARN); err != nil { + return err } - if aErr := attachPolicyToUsers(ctx, conn, add, arn); aErr != nil { - return aErr + if err := attachPolicyToUsers(ctx, conn, add, policyARN); err != nil { + return err } + return nil } -func updateRoles(ctx context.Context, conn *iam.IAM, d *schema.ResourceData) error { - arn := d.Get("policy_arn").(string) - o, n := d.GetChange("roles") - if o == nil { - o = new(schema.Set) - } - if n == nil { - n = new(schema.Set) - } - os := o.(*schema.Set) - ns := n.(*schema.Set) - remove := flex.ExpandStringSet(os.Difference(ns)) - add := flex.ExpandStringSet(ns.Difference(os)) - if rErr := detachPolicyFromRoles(ctx, conn, remove, arn); rErr != nil { - return rErr +func detachPolicyFromGroups(ctx context.Context, conn *iam.IAM, groups []string, policyARN string) error { + var errs []error + + for _, group := range groups { + errs = append(errs, detachPolicyFromGroup(ctx, conn, group, policyARN)) } - if aErr := attachPolicyToRoles(ctx, conn, add, arn); aErr != nil { - return aErr + + return errors.Join(errs...) +} + +func detachPolicyFromRoles(ctx context.Context, conn *iam.IAM, roles []string, policyARN string) error { + var errs []error + + for _, role := range roles { + errs = append(errs, detachPolicyFromRole(ctx, conn, role, policyARN)) } - return nil + + return errors.Join(errs...) } -func updateGroups(ctx context.Context, conn *iam.IAM, d *schema.ResourceData) error { - arn := d.Get("policy_arn").(string) - o, n := d.GetChange("groups") - if o == nil { - o = new(schema.Set) + +func detachPolicyFromUsers(ctx context.Context, conn *iam.IAM, users []string, policyARN string) error { + var errs []error + + for _, user := range users { + errs = append(errs, detachPolicyFromUser(ctx, conn, user, policyARN)) } - if n == nil { - n = new(schema.Set) + + return errors.Join(errs...) +} + +func findEntitiesForPolicyByARN(ctx context.Context, conn *iam.IAM, arn string) ([]string, []string, []string, error) { + input := &iam.ListEntitiesForPolicyInput{ + PolicyArn: aws.String(arn), } - os := o.(*schema.Set) - ns := n.(*schema.Set) - remove := flex.ExpandStringSet(os.Difference(ns)) - add := flex.ExpandStringSet(ns.Difference(os)) + groups, roles, users, err := findEntitiesForPolicy(ctx, conn, input) - if rErr := detachPolicyFromGroups(ctx, conn, remove, arn); rErr != nil { - return rErr + if err != nil { + return nil, nil, nil, err } - if aErr := attachPolicyToGroups(ctx, conn, add, arn); aErr != nil { - return aErr + + if len(groups) == 0 && len(roles) == 0 && len(users) == 0 { + return nil, nil, nil, tfresource.NewEmptyResultError(input) } - return nil + + groupName := tfslices.ApplyToAll(groups, func(v *iam.PolicyGroup) string { return aws.StringValue(v.GroupName) }) + roleNames := tfslices.ApplyToAll(roles, func(v *iam.PolicyRole) string { return aws.StringValue(v.RoleName) }) + userNames := tfslices.ApplyToAll(users, func(v *iam.PolicyUser) string { return aws.StringValue(v.UserName) }) + + return groupName, roleNames, userNames, nil } -func detachPolicyFromUsers(ctx context.Context, conn *iam.IAM, users []*string, arn string) error { - for _, u := range users { - _, err := conn.DetachUserPolicyWithContext(ctx, &iam.DetachUserPolicyInput{ - UserName: u, - PolicyArn: aws.String(arn), - }) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - continue - } - if err != nil { - return err + +func findEntitiesForPolicy(ctx context.Context, conn *iam.IAM, input *iam.ListEntitiesForPolicyInput) ([]*iam.PolicyGroup, []*iam.PolicyRole, []*iam.PolicyUser, error) { + var groups []*iam.PolicyGroup + var roles []*iam.PolicyRole + var users []*iam.PolicyUser + + err := conn.ListEntitiesForPolicyPagesWithContext(ctx, input, func(page *iam.ListEntitiesForPolicyOutput, lastPage bool) bool { + if page == nil { + return !lastPage } - } - return nil -} -func detachPolicyFromRoles(ctx context.Context, conn *iam.IAM, roles []*string, arn string) error { - for _, r := range roles { - _, err := conn.DetachRolePolicyWithContext(ctx, &iam.DetachRolePolicyInput{ - RoleName: r, - PolicyArn: aws.String(arn), - }) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - continue + + for _, v := range page.PolicyGroups { + if v != nil { + groups = append(groups, v) + } } - if err != nil { - return err + for _, v := range page.PolicyRoles { + if v != nil { + roles = append(roles, v) + } } - } - return nil -} -func detachPolicyFromGroups(ctx context.Context, conn *iam.IAM, groups []*string, arn string) error { - for _, g := range groups { - _, err := conn.DetachGroupPolicyWithContext(ctx, &iam.DetachGroupPolicyInput{ - GroupName: g, - PolicyArn: aws.String(arn), - }) - if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { - continue + for _, v := range page.PolicyUsers { + if v != nil { + users = append(users, v) + } } - if err != nil { - return err + + return !lastPage + }) + + if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) { + return nil, nil, nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, } } - return nil + + if err != nil { + return nil, nil, nil, err + } + + return groups, roles, users, nil } diff --git a/internal/service/iam/policy_data_source.go b/internal/service/iam/policy_data_source.go index 9e3696bee1a..dbc8644dc9d 100644 --- a/internal/service/iam/policy_data_source.go +++ b/internal/service/iam/policy_data_source.go @@ -87,7 +87,7 @@ func dataSourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta inte } // We need to make a call to `iam.GetPolicy` because `iam.ListPolicies` doesn't return all values - policy, err := FindPolicyByARN(ctx, conn, arn) + policy, err := findPolicyByARN(ctx, conn, arn) if err != nil { return sdkdiag.AppendErrorf(diags, "reading IAM Policy (%s): %s", arn, err) diff --git a/internal/service/iam/role_policy_attachment.go b/internal/service/iam/role_policy_attachment.go index 70342aa6152..bed0a235675 100644 --- a/internal/service/iam/role_policy_attachment.go +++ b/internal/service/iam/role_policy_attachment.go @@ -138,8 +138,8 @@ func attachPolicyToRole(ctx context.Context, conn *iam.IAM, role, policyARN stri func detachPolicyFromRole(ctx context.Context, conn *iam.IAM, role, policyARN string) error { _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { return conn.DetachRolePolicyWithContext(ctx, &iam.DetachRolePolicyInput{ - RoleName: aws.String(role), PolicyArn: aws.String(policyARN), + RoleName: aws.String(role), }) }, iam.ErrCodeConcurrentModificationException) diff --git a/internal/service/iam/service_package_gen.go b/internal/service/iam/service_package_gen.go index b67b8e30108..4b02e40c658 100644 --- a/internal/service/iam/service_package_gen.go +++ b/internal/service/iam/service_package_gen.go @@ -146,8 +146,9 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka Tags: &types.ServicePackageResourceTags{}, }, { - Factory: ResourcePolicyAttachment, + Factory: resourcePolicyAttachment, TypeName: "aws_iam_policy_attachment", + Name: "Policy Attachment", }, { Factory: ResourceRole, diff --git a/internal/service/iam/user_policy_attachment.go b/internal/service/iam/user_policy_attachment.go index a091c29db9b..ea1b1c3e29f 100644 --- a/internal/service/iam/user_policy_attachment.go +++ b/internal/service/iam/user_policy_attachment.go @@ -123,8 +123,8 @@ func resourceUserPolicyAttachmentImport(ctx context.Context, d *schema.ResourceD func attachPolicyToUser(ctx context.Context, conn *iam.IAM, user, policyARN string) error { _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { return conn.AttachUserPolicyWithContext(ctx, &iam.AttachUserPolicyInput{ - UserName: aws.String(user), PolicyArn: aws.String(policyARN), + UserName: aws.String(user), }) }, iam.ErrCodeConcurrentModificationException) @@ -138,8 +138,8 @@ func attachPolicyToUser(ctx context.Context, conn *iam.IAM, user, policyARN stri func detachPolicyFromUser(ctx context.Context, conn *iam.IAM, user, policyARN string) error { _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, func() (interface{}, error) { return conn.DetachUserPolicyWithContext(ctx, &iam.DetachUserPolicyInput{ - UserName: aws.String(user), PolicyArn: aws.String(policyARN), + UserName: aws.String(user), }) }, iam.ErrCodeConcurrentModificationException) From 12593c95de5ac585899bcbe2718d2b98d1c98dae Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 17:00:49 -0500 Subject: [PATCH 17/20] r/aws_iam_policy_attachment: Tidy up acceptance tests. --- internal/service/iam/exports_test.go | 2 + .../iam/group_policy_attachment_test.go | 2 +- .../service/iam/policy_attachment_test.go | 616 ++++-------------- .../iam/role_policy_attachment_test.go | 2 +- .../iam/user_policy_attachment_test.go | 2 +- 5 files changed, 147 insertions(+), 477 deletions(-) diff --git a/internal/service/iam/exports_test.go b/internal/service/iam/exports_test.go index 6f90daa8667..b9a3db7b48c 100644 --- a/internal/service/iam/exports_test.go +++ b/internal/service/iam/exports_test.go @@ -6,6 +6,7 @@ package iam // Exports for use in tests only. var ( ResourceGroupPolicyAttachment = resourceGroupPolicyAttachment + ResourcePolicyAttachment = resourcePolicyAttachment ResourceRolePolicyAttachment = resourceRolePolicyAttachment ResourceUserPolicyAttachment = resourceUserPolicyAttachment @@ -15,5 +16,6 @@ var ( FindAttachedRolePolicyByTwoPartKey = findAttachedRolePolicyByTwoPartKey FindAttachedUserPolicies = findAttachedUserPolicies FindAttachedUserPolicyByTwoPartKey = findAttachedUserPolicyByTwoPartKey + FindEntitiesForPolicyByARN = findEntitiesForPolicyByARN FindPolicyByARN = findPolicyByARN ) diff --git a/internal/service/iam/group_policy_attachment_test.go b/internal/service/iam/group_policy_attachment_test.go index aa7a309ec7e..62f2617a631 100644 --- a/internal/service/iam/group_policy_attachment_test.go +++ b/internal/service/iam/group_policy_attachment_test.go @@ -153,7 +153,7 @@ func testAccCheckGroupPolicyAttachmentCount(ctx context.Context, groupName strin return fmt.Errorf("GroupPolicyAttachmentCount(%q) = %v, want %v", groupName, got, want) } - return err + return nil } } diff --git a/internal/service/iam/policy_attachment_test.go b/internal/service/iam/policy_attachment_test.go index ff5394a586a..f74f2632253 100644 --- a/internal/service/iam/policy_attachment_test.go +++ b/internal/service/iam/policy_attachment_test.go @@ -8,265 +8,182 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/iam" 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" + tfiam "github.com/hashicorp/terraform-provider-aws/internal/service/iam" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func TestAccIAMPolicyAttachment_basic(t *testing.T) { ctx := acctest.Context(t) - var out iam.ListEntitiesForPolicyOutput - - rString := sdkacctest.RandString(8) - userName := fmt.Sprintf("tf-acc-user-pa-basic-%s", rString) - userName2 := fmt.Sprintf("tf-acc-user-pa-basic-2-%s", rString) - userName3 := fmt.Sprintf("tf-acc-user-pa-basic-3-%s", rString) - roleName := fmt.Sprintf("tf-acc-role-pa-basic-%s", rString) - roleName2 := fmt.Sprintf("tf-acc-role-pa-basic-2-%s", rString) - roleName3 := fmt.Sprintf("tf-acc-role-pa-basic-3-%s", rString) - groupName := fmt.Sprintf("tf-acc-group-pa-basic-%s", rString) - groupName2 := fmt.Sprintf("tf-acc-group-pa-basic-2-%s", rString) - groupName3 := fmt.Sprintf("tf-acc-group-pa-basic-3-%s", rString) - policyName := fmt.Sprintf("tf-acc-policy-pa-basic-%s", rString) - attachmentName := fmt.Sprintf("tf-acc-attachment-pa-basic-%s", rString) + userName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + userName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + roleName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + roleName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + groupName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + groupName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + groupName3 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + attachmentName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_policy_attachment.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckPolicyAttachmentDestroy, + CheckDestroy: testAccCheckPolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccPolicyAttachmentConfig_attach(userName, roleName, groupName, policyName, attachmentName), + Config: testAccPolicyAttachmentConfig_attach(userName1, roleName1, roleName2, policyName, attachmentName), Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, "aws_iam_policy_attachment.test-attach", 3, &out), - testAccCheckPolicyAttachmentAttributes([]string{userName}, []string{roleName}, []string{groupName}, &out), + testAccCheckPolicyAttachmentExists(ctx, resourceName), + testAccCheckPolicyAttachmentCounts(ctx, resourceName, 0, 2, 1), ), }, { - Config: testAccPolicyAttachmentConfig_attachUpdate(userName, userName2, userName3, - roleName, roleName2, roleName3, - groupName, groupName2, groupName3, - policyName, attachmentName), + Config: testAccPolicyAttachmentConfig_attachUpdate(userName1, userName2, roleName1, groupName1, groupName2, groupName3, policyName, attachmentName), Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, "aws_iam_policy_attachment.test-attach", 6, &out), - testAccCheckPolicyAttachmentAttributes([]string{userName2, userName3}, - []string{roleName2, roleName3}, []string{groupName2, groupName3}, &out), + testAccCheckPolicyAttachmentExists(ctx, resourceName), + testAccCheckPolicyAttachmentCounts(ctx, resourceName, 3, 1, 2), ), }, }, }) } -func TestAccIAMPolicyAttachment_paginatedEntities(t *testing.T) { +func TestAccIAMPolicyAttachment_disappears(t *testing.T) { ctx := acctest.Context(t) - var out iam.ListEntitiesForPolicyOutput - - rString := sdkacctest.RandString(8) - userNamePrefix := fmt.Sprintf("tf-acc-user-pa-pe-%s-", rString) - policyName := fmt.Sprintf("tf-acc-policy-pa-pe-%s-", rString) - attachmentName := fmt.Sprintf("tf-acc-attachment-pa-pe-%s-", rString) + userName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + roleName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + roleName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + attachmentName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_iam_policy_attachment.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckPolicyAttachmentDestroy, + CheckDestroy: testAccCheckPolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccPolicyAttachmentConfig_paginatedAttach(userNamePrefix, policyName, attachmentName), + Config: testAccPolicyAttachmentConfig_attach(userName1, roleName1, roleName2, policyName, attachmentName), Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, "aws_iam_policy_attachment.test-paginated-attach", 101, &out), + testAccCheckPolicyAttachmentExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfiam.ResourcePolicyAttachment(), resourceName), ), + ExpectNonEmptyPlan: true, }, }, }) } -func TestAccIAMPolicyAttachment_Groups_renamedGroup(t *testing.T) { +func TestAccIAMPolicyAttachment_paginatedEntities(t *testing.T) { ctx := acctest.Context(t) - var out iam.ListEntitiesForPolicyOutput - - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - groupName1 := fmt.Sprintf("%s-1", rName) - groupName2 := fmt.Sprintf("%s-2", rName) + userNamePrefix := fmt.Sprintf("%s-%s-", acctest.ResourcePrefix, sdkacctest.RandString(3)) + policyName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + attachmentName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_iam_policy_attachment.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckPolicyAttachmentDestroy, + CheckDestroy: testAccCheckPolicyAttachmentDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccPolicyAttachmentConfig_groupsRenamedGroup(rName, groupName1), - Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, resourceName, 1, &out), - testAccCheckPolicyAttachmentAttributes([]string{}, []string{}, []string{groupName1}, &out), - ), - }, - { - Config: testAccPolicyAttachmentConfig_groupsRenamedGroup(rName, groupName2), + Config: testAccPolicyAttachmentConfig_paginatedAttach(userNamePrefix, policyName, attachmentName), Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, resourceName, 1, &out), - testAccCheckPolicyAttachmentAttributes([]string{}, []string{}, []string{groupName2}, &out), + testAccCheckPolicyAttachmentExists(ctx, resourceName), + testAccCheckPolicyAttachmentCounts(ctx, resourceName, 0, 0, 101), ), }, }, }) } -func TestAccIAMPolicyAttachment_Roles_renamedRole(t *testing.T) { - ctx := acctest.Context(t) - var out iam.ListEntitiesForPolicyOutput +func testAccCheckPolicyAttachmentDestroy(ctx context.Context) resource.TestCheckFunc { + return func(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - roleName1 := fmt.Sprintf("%s-1", rName) - roleName2 := fmt.Sprintf("%s-2", rName) - resourceName := "aws_iam_policy_attachment.test" + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_iam_policy_attachment" { + continue + } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckPolicyAttachmentDestroy, - Steps: []resource.TestStep{ - { - Config: testAccPolicyAttachmentConfig_rolesRenamedRole(rName, roleName1), - Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, resourceName, 1, &out), - testAccCheckPolicyAttachmentAttributes([]string{}, []string{roleName1}, []string{}, &out), - ), - }, - { - Config: testAccPolicyAttachmentConfig_rolesRenamedRole(rName, roleName2), - Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, resourceName, 1, &out), - testAccCheckPolicyAttachmentAttributes([]string{}, []string{roleName2}, []string{}, &out), - ), - }, - }, - }) -} + _, _, _, err := tfiam.FindEntitiesForPolicyByARN(ctx, conn, rs.Primary.Attributes["policy_arn"]) -func TestAccIAMPolicyAttachment_Users_renamedUser(t *testing.T) { - ctx := acctest.Context(t) - var out iam.ListEntitiesForPolicyOutput + if tfresource.NotFound(err) { + continue + } - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - userName1 := fmt.Sprintf("%s-1", rName) - userName2 := fmt.Sprintf("%s-2", rName) - resourceName := "aws_iam_policy_attachment.test" + if err != nil { + return err + } - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, iam.EndpointsID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckPolicyAttachmentDestroy, - Steps: []resource.TestStep{ - { - Config: testAccPolicyAttachmentConfig_usersRenamedUser(rName, userName1), - Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, resourceName, 1, &out), - testAccCheckPolicyAttachmentAttributes([]string{userName1}, []string{}, []string{}, &out), - ), - }, - { - Config: testAccPolicyAttachmentConfig_usersRenamedUser(rName, userName2), - Check: resource.ComposeTestCheckFunc( - testAccCheckPolicyAttachmentExists(ctx, resourceName, 1, &out), - testAccCheckPolicyAttachmentAttributes([]string{userName2}, []string{}, []string{}, &out), - ), - }, - }, - }) -} + return fmt.Errorf("IAM Policy Attachment %s still exists", rs.Primary.ID) + } -func testAccCheckPolicyAttachmentDestroy(s *terraform.State) error { - return nil + return nil + } } -func testAccCheckPolicyAttachmentExists(ctx context.Context, n string, c int64, out *iam.ListEntitiesForPolicyOutput) resource.TestCheckFunc { +func testAccCheckPolicyAttachmentExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { return fmt.Errorf("Not found: %s", n) } - if rs.Primary.ID == "" { - return fmt.Errorf("No policy name is set") - } - conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) - arn := rs.Primary.Attributes["policy_arn"] - resp, err := conn.GetPolicyWithContext(ctx, &iam.GetPolicyInput{ - PolicyArn: aws.String(arn), - }) - if err != nil { - return fmt.Errorf("Error: Policy (%s) not found", n) - } - if c != *resp.Policy.AttachmentCount { - return fmt.Errorf("Error: Policy (%s) has wrong number of entities attached on initial creation", n) - } - resp2, err := conn.ListEntitiesForPolicyWithContext(ctx, &iam.ListEntitiesForPolicyInput{ - PolicyArn: aws.String(arn), - }) - if err != nil { - return fmt.Errorf("Error: Failed to get entities for Policy (%s)", arn) - } + _, _, _, err := tfiam.FindEntitiesForPolicyByARN(ctx, conn, rs.Primary.Attributes["policy_arn"]) - *out = *resp2 - return nil + return err } } -func testAccCheckPolicyAttachmentAttributes(users []string, roles []string, groups []string, out *iam.ListEntitiesForPolicyOutput) resource.TestCheckFunc { +func testAccCheckPolicyAttachmentCounts(ctx context.Context, n string, wantGroups, wantRoles, wantUsers int) resource.TestCheckFunc { return func(s *terraform.State) error { - uc := len(users) - rc := len(roles) - gc := len(groups) - - for _, u := range users { - for _, pu := range out.PolicyUsers { - if u == *pu.UserName { - uc-- - } - } + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) } - for _, r := range roles { - for _, pr := range out.PolicyRoles { - if r == *pr.RoleName { - rc-- - } - } + + conn := acctest.Provider.Meta().(*conns.AWSClient).IAMConn(ctx) + + groups, roles, users, err := tfiam.FindEntitiesForPolicyByARN(ctx, conn, rs.Primary.Attributes["policy_arn"]) + + if err != nil { + return err } - for _, g := range groups { - for _, pg := range out.PolicyGroups { - if g == *pg.GroupName { - gc-- - } - } + + if got, want := len(groups), wantGroups; got != want { + return fmt.Errorf("GroupPolicyAttachmentCount = %v, want %v", got, want) } - if uc != 0 || rc != 0 || gc != 0 { - return fmt.Errorf("Error: Number of attached users, roles, or groups was incorrect:\n expected %d users and found %d\nexpected %d roles and found %d\nexpected %d groups and found %d", len(users), len(users)-uc, len(roles), len(roles)-rc, len(groups), len(groups)-gc) + if got, want := len(roles), wantRoles; got != want { + return fmt.Errorf("RolePolicyAttachmentCount = %v, want %v", got, want) } + if got, want := len(users), wantUsers; got != want { + return fmt.Errorf("GroupPolicyAttachmentCount = %v, want %v", got, want) + } + return nil } } -func testAccPolicyAttachmentConfig_attach(userName, roleName, groupName, policyName, attachmentName string) string { +func testAccPolicyAttachmentConfig_attach(userName1, roleName1, roleName2, policyName, attachmentName string) string { return fmt.Sprintf(` -resource "aws_iam_user" "user" { - name = "%s" +resource "aws_iam_user" "test1" { + name = %[1]q } -resource "aws_iam_role" "role" { - name = "%s" +resource "aws_iam_role" "test1" { + name = %[2]q assume_role_policy = < Date: Tue, 14 Nov 2023 17:12:12 -0500 Subject: [PATCH 18/20] Fix acceptance test configuration. --- internal/service/iam/policy_attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/iam/policy_attachment_test.go b/internal/service/iam/policy_attachment_test.go index f74f2632253..6e1979c72e5 100644 --- a/internal/service/iam/policy_attachment_test.go +++ b/internal/service/iam/policy_attachment_test.go @@ -365,7 +365,7 @@ EOF resource "aws_iam_policy_attachment" "test" { name = %[3]q - policy_arn = aws_iam_policy.policy.arn + policy_arn = aws_iam_policy.test.arn users = aws_iam_user.test[*].name } From 1d3c19368ccf857f75d83135ee38c87cd2d41de4 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 17:12:17 -0500 Subject: [PATCH 19/20] Acceptance test output: % make testacc TESTARGS='-run=TestAccIAMPolicyAttachment_' PKG=iam ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run=TestAccIAMPolicyAttachment_ -timeout 360m === RUN TestAccIAMPolicyAttachment_basic === PAUSE TestAccIAMPolicyAttachment_basic === RUN TestAccIAMPolicyAttachment_disappears === PAUSE TestAccIAMPolicyAttachment_disappears === RUN TestAccIAMPolicyAttachment_paginatedEntities === PAUSE TestAccIAMPolicyAttachment_paginatedEntities === CONT TestAccIAMPolicyAttachment_basic === CONT TestAccIAMPolicyAttachment_paginatedEntities === CONT TestAccIAMPolicyAttachment_disappears --- PASS: TestAccIAMPolicyAttachment_disappears (30.25s) --- PASS: TestAccIAMPolicyAttachment_basic (40.91s) --- PASS: TestAccIAMPolicyAttachment_paginatedEntities (78.31s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/iam 84.067s From 313687bdc6feb3c9356c0703e4c57bac4ffff4fb Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 14 Nov 2023 17:13:06 -0500 Subject: [PATCH 20/20] Fix terrafmt error. --- internal/service/iam/policy_attachment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/iam/policy_attachment_test.go b/internal/service/iam/policy_attachment_test.go index 6e1979c72e5..9b78dc607fa 100644 --- a/internal/service/iam/policy_attachment_test.go +++ b/internal/service/iam/policy_attachment_test.go @@ -294,7 +294,7 @@ resource "aws_iam_group" "test3" { } resource "aws_iam_policy" "test" { - name = %[7]q + name = %[7]q description = "A test policy" policy = <