Merge pull request #34378 from flichtenheld/policy_attachment

service/iam: retry attaching policy on ConcurrentModificationException

.changelog/34378.txt

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

internal/service/iam/exports_test.go

@@ -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
+)

internal/service/iam/find.go

@@ -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{}
 

internal/service/iam/group_policy_attachment.go

@@ -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,
 		},
@@ -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,
 			},
 		},
 	}
@@ -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
@@ -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
@@ -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
 }
 
@@ -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
 }