Merge pull request #34329 from Octogonapus/b-aws_iot_policy-delete_co…

…nsistency_fix

fix: Add retry for eventual consistency bug in aws_iot_policy

.changelog/34329.txt

@@ -0,0 +1,11 @@
+```release-note:bug
+resource/aws_iot_policy: Retry `DeleteConflictException` errors on delete
+```
+
+```release-note:enhancement
+resource/aws_iot_policy: Add configurable timeouts
+```
+
+```release-note:enhancement
+resource/aws_iot_policy: When updating the resource, delete the oldest non-default version of the policy if creating a new version would exceed the maximum number of versions (5)
+```

internal/service/iot/policy.go

@@ -5,18 +5,24 @@ package iot
 
 import (
 	"context"
+	"fmt"
 	"log"
+	"strconv"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/iot"
 	"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/structure"
 	"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/tfresource"
 	"github.com/hashicorp/terraform-provider-aws/internal/verify"
+	"golang.org/x/exp/slices"
 )
 
 // @SDKResource("aws_iot_policy")
@@ -26,11 +32,25 @@ func ResourcePolicy() *schema.Resource {
 		ReadWithoutTimeout:   resourcePolicyRead,
 		UpdateWithoutTimeout: resourcePolicyUpdate,
 		DeleteWithoutTimeout: resourcePolicyDelete,
+
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
 
+		Timeouts: &schema.ResourceTimeout{
+			Update: schema.DefaultTimeout(1 * time.Minute),
+			Delete: schema.DefaultTimeout(5 * time.Minute),
+		},
+
 		Schema: map[string]*schema.Schema{
+			"arn": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
+			"default_version_id": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
 			"name": {
 				Type:     schema.TypeString,
 				Required: true,
@@ -47,14 +67,6 @@ func ResourcePolicy() *schema.Resource {
 					return json
 				},
 			},
-			"arn": {
-				Type:     schema.TypeString,
-				Computed: true,
-			},
-			"default_version_id": {
-				Type:     schema.TypeString,
-				Computed: true,
-			},
 		},
 	}
 }
@@ -68,16 +80,19 @@ func resourcePolicyCreate(ctx context.Context, d *schema.ResourceData, meta inte
 		return sdkdiag.AppendErrorf(diags, "policy (%s) is invalid JSON: %s", policy, err)
 	}
 
-	out, err := conn.CreatePolicyWithContext(ctx, &iot.CreatePolicyInput{
-		PolicyName:     aws.String(d.Get("name").(string)),
+	name := d.Get("name").(string)
+	input := &iot.CreatePolicyInput{
 		PolicyDocument: aws.String(policy),
-	})
+		PolicyName:     aws.String(name),
+	}
+
+	output, err := conn.CreatePolicyWithContext(ctx, input)
 
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "creating IoT Policy: %s", err)
+		return sdkdiag.AppendErrorf(diags, "creating IoT Policy (%s): %s", name, err)
 	}
 
-	d.SetId(aws.StringValue(out.PolicyName))
+	d.SetId(aws.StringValue(output.PolicyName))
 
 	return append(diags, resourcePolicyRead(ctx, d, meta)...)
 }
@@ -86,11 +101,9 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).IoTConn(ctx)
 
-	out, err := conn.GetPolicyWithContext(ctx, &iot.GetPolicyInput{
-		PolicyName: aws.String(d.Id()),
-	})
+	output, err := FindPolicyByName(ctx, conn, d.Id())
 
-	if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
+	if !d.IsNewResource() && tfresource.NotFound(err) {
 		log.Printf("[WARN] IoT Policy (%s) not found, removing from state", d.Id())
 		d.SetId("")
 		return diags
@@ -100,13 +113,13 @@ func resourcePolicyRead(ctx context.Context, d *schema.ResourceData, meta interf
 		return sdkdiag.AppendErrorf(diags, "reading IoT Policy (%s): %s", d.Id(), err)
 	}
 
-	d.Set("arn", out.PolicyArn)
-	d.Set("default_version_id", out.DefaultVersionId)
-	d.Set("name", out.PolicyName)
+	d.Set("arn", output.PolicyArn)
+	d.Set("default_version_id", output.DefaultVersionId)
+	d.Set("name", output.PolicyName)
 
-	policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(out.PolicyDocument))
+	policyToSet, err := verify.PolicyToSet(d.Get("policy").(string), aws.StringValue(output.PolicyDocument))
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "reading IoT Policy (%s): %s", d.Id(), err)
+		return sdkdiag.AppendFromErr(diags, err)
 	}
 
 	d.Set("policy", policyToSet)
@@ -118,21 +131,63 @@ func resourcePolicyUpdate(ctx context.Context, d *schema.ResourceData, meta inte
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).IoTConn(ctx)
 
-	if d.HasChange("policy") {
-		policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
+	policy, err := structure.NormalizeJsonString(d.Get("policy").(string))
+	if err != nil {
+		return sdkdiag.AppendErrorf(diags, "policy (%s) is invalid JSON: %s", policy, err)
+	}
+
+	input := &iot.CreatePolicyVersionInput{
+		PolicyDocument: aws.String(policy),
+		PolicyName:     aws.String(d.Id()),
+		SetAsDefault:   aws.Bool(true),
+	}
+
+	_, errCreate := conn.CreatePolicyVersionWithContext(ctx, input)
+
+	// "VersionsLimitExceededException: The policy ... already has the maximum number of versions (5)"
+	if tfawserr.ErrCodeEquals(errCreate, iot.ErrCodeVersionsLimitExceededException) {
+		// Prune the lowest version and retry.
+		policyVersions, err := FindPolicyVersionsByName(ctx, conn, d.Id())
+
 		if err != nil {
-			return sdkdiag.AppendErrorf(diags, "policy (%s) is invalid JSON: %s", policy, err)
+			return sdkdiag.AppendErrorf(diags, "reading IoT Policy (%s) versions: %s", d.Id(), err)
 		}
 
-		_, err = conn.CreatePolicyVersionWithContext(ctx, &iot.CreatePolicyVersionInput{
-			PolicyName:     aws.String(d.Id()),
-			PolicyDocument: aws.String(policy),
-			SetAsDefault:   aws.Bool(true),
-		})
+		var versionIDs []int
 
-		if err != nil {
-			return sdkdiag.AppendErrorf(diags, "updating IoT Policy (%s): %s", d.Id(), err)
+		for _, v := range policyVersions {
+			if aws.BoolValue(v.IsDefaultVersion) {
+				continue
+			}
+
+			v, err := strconv.Atoi(aws.StringValue(v.VersionId))
+
+			if err != nil {
+				continue
+			}
+
+			versionIDs = append(versionIDs, v)
 		}
+
+		if len(versionIDs) > 0 {
+			// Sort ascending.
+			slices.Sort(versionIDs)
+			versionID := strconv.Itoa(versionIDs[0])
+
+			if err := deletePolicyVersion(ctx, conn, d.Id(), versionID, d.Timeout(schema.TimeoutUpdate)); err != nil {
+				return sdkdiag.AppendFromErr(diags, err)
+			}
+
+			if err != nil {
+				return sdkdiag.AppendErrorf(diags, "waiting for IoT Policy (%s) version (%s) delete: %s", d.Id(), versionID, err)
+			}
+
+			_, errCreate = conn.CreatePolicyVersionWithContext(ctx, input)
+		}
+	}
+
+	if errCreate != nil {
+		return sdkdiag.AppendErrorf(diags, "updating IoT Policy (%s): %s", d.Id(), errCreate)
 	}
 
 	return append(diags, resourcePolicyRead(ctx, d, meta)...)
@@ -142,44 +197,122 @@ func resourcePolicyDelete(ctx context.Context, d *schema.ResourceData, meta inte
 	var diags diag.Diagnostics
 	conn := meta.(*conns.AWSClient).IoTConn(ctx)
 
-	out, err := conn.ListPolicyVersionsWithContext(ctx, &iot.ListPolicyVersionsInput{
-		PolicyName: aws.String(d.Id()),
-	})
+	policyVersions, err := FindPolicyVersionsByName(ctx, conn, d.Id())
+
+	if tfresource.NotFound(err) {
+		return diags
+	}
 
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "listing IoT Policy (%s) versions: %s", d.Id(), err)
+		return sdkdiag.AppendErrorf(diags, "reading IoT Policy (%s) versions: %s", d.Id(), err)
 	}
 
-	// Delete all non-default versions of the policy
-	for _, ver := range out.PolicyVersions {
-		if !aws.BoolValue(ver.IsDefaultVersion) {
-			_, err = conn.DeletePolicyVersionWithContext(ctx, &iot.DeletePolicyVersionInput{
-				PolicyName:      aws.String(d.Id()),
-				PolicyVersionId: ver.VersionId,
-			})
+	// Delete all non-default versions of the policy.
+	for _, v := range policyVersions {
+		if aws.BoolValue(v.IsDefaultVersion) {
+			continue
+		}
 
-			if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
-				continue
-			}
+		if err := deletePolicyVersion(ctx, conn, d.Id(), aws.StringValue(v.VersionId), d.Timeout(schema.TimeoutDelete)); err != nil {
+			return sdkdiag.AppendFromErr(diags, err)
+		}
+	}
 
-			if err != nil {
-				return sdkdiag.AppendErrorf(diags, "deleting IoT Policy (%s) version (%s): %s", d.Id(), aws.StringValue(ver.VersionId), err)
-			}
+	// Delete default policy version.
+	if err := deletePolicy(ctx, conn, d.Id(), d.Timeout(schema.TimeoutDelete)); err != nil {
+		return sdkdiag.AppendFromErr(diags, err)
+	}
+
+	return diags
+}
+
+func FindPolicyByName(ctx context.Context, conn *iot.IoT, name string) (*iot.GetPolicyOutput, error) {
+	input := &iot.GetPolicyInput{
+		PolicyName: aws.String(name),
+	}
+
+	output, err := conn.GetPolicyWithContext(ctx, input)
+
+	if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
+		return nil, &retry.NotFoundError{
+			LastError:   err,
+			LastRequest: input,
 		}
 	}
 
-	//Delete default policy version
-	_, err = conn.DeletePolicyWithContext(ctx, &iot.DeletePolicyInput{
-		PolicyName: aws.String(d.Id()),
-	})
+	if err != nil {
+		return nil, err
+	}
+
+	if output == nil {
+		return nil, tfresource.NewEmptyResultError(input)
+	}
+
+	return output, nil
+}
+
+func FindPolicyVersionsByName(ctx context.Context, conn *iot.IoT, name string) ([]*iot.PolicyVersion, error) {
+	input := &iot.ListPolicyVersionsInput{
+		PolicyName: aws.String(name),
+	}
+
+	output, err := conn.ListPolicyVersionsWithContext(ctx, input)
 
 	if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
-		return diags
+		return nil, &retry.NotFoundError{
+			LastError:   err,
+			LastRequest: input,
+		}
 	}
 
 	if err != nil {
-		return sdkdiag.AppendErrorf(diags, "deleting IoT Policy (%s): %s", d.Id(), err)
+		return nil, err
 	}
 
-	return diags
+	if output == nil || len(output.PolicyVersions) == 0 {
+		return nil, tfresource.NewEmptyResultError(input)
+	}
+
+	return output.PolicyVersions, nil
+}
+
+func deletePolicy(ctx context.Context, conn *iot.IoT, name string, timeout time.Duration) error {
+	input := &iot.DeletePolicyInput{
+		PolicyName: aws.String(name),
+	}
+
+	_, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, timeout, func() (interface{}, error) {
+		return conn.DeletePolicyWithContext(ctx, input)
+	}, iot.ErrCodeDeleteConflictException)
+
+	if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
+		return nil
+	}
+
+	if err != nil {
+		return fmt.Errorf("deleting IoT Policy (%s): %w", name, err)
+	}
+
+	return nil
+}
+
+func deletePolicyVersion(ctx context.Context, conn *iot.IoT, name, versionID string, timeout time.Duration) error {
+	input := &iot.DeletePolicyVersionInput{
+		PolicyName:      aws.String(name),
+		PolicyVersionId: aws.String(versionID),
+	}
+
+	_, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, timeout, func() (interface{}, error) {
+		return conn.DeletePolicyVersionWithContext(ctx, input)
+	}, iot.ErrCodeDeleteConflictException)
+
+	if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
+		return nil
+	}
+
+	if err != nil {
+		return fmt.Errorf("deleting IoT Policy (%s) version (%s): %w", name, versionID, err)
+	}
+
+	return nil
 }