Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service/iam: retry attaching policy on ConcurrentModificationException #34378

Merged
merged 20 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
54ffafc
service/iam: retry attaching policy on ConcurrentModificationException
flichtenheld Nov 13, 2023
0ce4f41
Add CHANGELOG entries.
ewbankkit Nov 14, 2023
72bf557
r/aws_iam_group_policy_attachment: Add plan-time validation of `polic…
ewbankkit Nov 14, 2023
3027899
r/aws_iam_policy_attachment: Add plan-time validation of `policy_arn`.
ewbankkit Nov 14, 2023
fab3687
r/aws_iam_role_policy_attachment: Add plan-time validation of `policy…
ewbankkit Nov 14, 2023
7e40676
r/aws_iam_user_policy_attachment: Add plan-time validation of `policy…
ewbankkit Nov 14, 2023
42e0b26
r/aws_iam_group_policy_attachment: Tidy up.
ewbankkit Nov 14, 2023
b6fb2c5
r/aws_iam_group_policy_attachment: Tidy up acceptance tests.
ewbankkit Nov 14, 2023
09b41a8
Acceptance test output:
ewbankkit Nov 14, 2023
46b8b5e
r/aws_iam_user_policy_attachment: Tidy up.
ewbankkit Nov 14, 2023
2af0899
r/aws_iam_user_policy_attachment: Tidy up acceptance tests.
ewbankkit Nov 14, 2023
985ec6a
Acceptance test output:
ewbankkit Nov 14, 2023
f02e517
r/aws_iam_role_policy_attachment: Tidy up.
ewbankkit Nov 14, 2023
6c2809d
r/aws_iam_role_policy_attachment: Tidy up acceptance tests.
ewbankkit Nov 14, 2023
d5950c9
Acceptance test output:
ewbankkit Nov 14, 2023
55b6ee9
r/aws_iam_policy_attachment: Tidy up.
ewbankkit Nov 14, 2023
12593c9
r/aws_iam_policy_attachment: Tidy up acceptance tests.
ewbankkit Nov 14, 2023
503a57d
Fix acceptance test configuration.
ewbankkit Nov 14, 2023
1d3c193
Acceptance test output:
ewbankkit Nov 14, 2023
313687b
Fix terrafmt error.
ewbankkit Nov 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .changelog/34378.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
```release-note:bug
resource/aws_iam_group_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete
```

```release-note:bug
resource/aws_iam_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete
```

```release-note:bug
resource/aws_iam_role_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete
```

```release-note:bug
resource/aws_iam_user_policy_attachment: Retry `ConcurrentModificationException` errors on create and delete
```

```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`
```

```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`
```
21 changes: 21 additions & 0 deletions internal/service/iam/exports_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0

package iam

// Exports for use in tests only.
var (
ResourceGroupPolicyAttachment = resourceGroupPolicyAttachment
ResourcePolicyAttachment = resourcePolicyAttachment
ResourceRolePolicyAttachment = resourceRolePolicyAttachment
ResourceUserPolicyAttachment = resourceUserPolicyAttachment

FindAttachedGroupPolicies = findAttachedGroupPolicies
FindAttachedGroupPolicyByTwoPartKey = findAttachedGroupPolicyByTwoPartKey
FindAttachedRolePolicies = findAttachedRolePolicies
FindAttachedRolePolicyByTwoPartKey = findAttachedRolePolicyByTwoPartKey
FindAttachedUserPolicies = findAttachedUserPolicies
FindAttachedUserPolicyByTwoPartKey = findAttachedUserPolicyByTwoPartKey
FindEntitiesForPolicyByARN = findEntitiesForPolicyByARN
FindPolicyByARN = findPolicyByARN
)
68 changes: 0 additions & 68 deletions internal/service/iam/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,74 +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{
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{}

Expand Down
174 changes: 107 additions & 67 deletions internal/service/iam/group_policy_attachment.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ 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")
func ResourceGroupPolicyAttachment() *schema.Resource {
// @SDKResource("aws_iam_group_policy_attachment", name="Group Policy Attachment")
func resourceGroupPolicyAttachment() *schema.Resource {
return &schema.Resource{
CreateWithoutTimeout: resourceGroupPolicyAttachmentCreate,
ReadWithoutTimeout: resourceGroupPolicyAttachmentRead,
DeleteWithoutTimeout: resourceGroupPolicyAttachmentDelete,

Importer: &schema.ResourceImporter{
StateContext: resourceGroupPolicyAttachmentImport,
},
Expand All @@ -38,9 +41,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,
},
},
}
Expand All @@ -51,11 +55,10 @@ 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)
if err != nil {
return sdkdiag.AppendErrorf(diags, "attaching policy %s to IAM group %s: %v", arn, group, err)
if err := attachPolicyToGroup(ctx, conn, group, policyARN); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}

//lintignore:R016 // Allow legacy unstable ID usage in managed resource
Expand All @@ -67,57 +70,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)
// 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)

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 Group Managed Policy Attachment (%s) not found", id),
})
}

return nil
})
group := d.Get("group").(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, policyARN)

if tfresource.TimedOut(err) {
attachedPolicy, err = FindGroupAttachedPolicy(ctx, conn, group, arn)
}
_, err := tfresource.RetryWhenNewResourceNotFound(ctx, propagationTimeout, func() (interface{}, error) {
return findAttachedGroupPolicyByTwoPartKey(ctx, conn, group, 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 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
Expand All @@ -126,13 +96,11 @@ 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)
if err != nil {
return sdkdiag.AppendErrorf(diags, "removing policy %s from IAM Group %s: %v", arn, group, err)
if err := detachPolicyFromGroup(ctx, conn, d.Get("group").(string), d.Get("policy_arn").(string)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}

return diags
}

Expand All @@ -141,26 +109,98 @@ 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 <group-name>/<policy_arn>", 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
}

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),
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(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)

if tfawserr.ErrCodeEquals(err, iam.ErrCodeNoSuchEntityException) {
return nil
}

if err != nil {
return fmt.Errorf("detaching IAM Policy (%s) from IAM Group (%s): %w", policyARN, group, err)
}

return nil
}

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 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 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
})
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
}
Loading
Loading