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

fix: Add retry for eventual consistency bug in aws_iot_policy #34329

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
784897a
resource/aws_iot_policy: Delete oldest policy version when max number…
jeandek Jul 29, 2019
9852ae0
resource/aws_iot_policy: Add an acceptance test for the resource update
jeandek Jul 30, 2019
5d528f6
resource/aws_iot_policy: Switch old version pruning to use VersionId …
jeandek Jul 30, 2019
4b776b9
fix: Add retry for eventual consistency bug in aws_iot_policy
Octogonapus Nov 9, 2023
1d39372
Revert "resource/aws_iot_policy: Switch old version pruning to use Ve…
ewbankkit Nov 10, 2023
6932cf3
Revert "resource/aws_iot_policy: Add an acceptance test for the resou…
ewbankkit Nov 10, 2023
4c5ac39
Revert "resource/aws_iot_policy: Delete oldest policy version when ma…
ewbankkit Nov 10, 2023
2024a33
Merge commit '4c5ac390611b3e9f1c13cfeb6aef80f8fd5d2dad' into HEAD
ewbankkit Nov 10, 2023
1e13f85
Merge branch 'main' into HEAD
ewbankkit Nov 10, 2023
7a2b6c6
r/aws_iot_policy: Alphabetize attributes.
ewbankkit Nov 10, 2023
b91588d
r/aws_iot_policy: Tidy up Create.
ewbankkit Nov 10, 2023
2b27c8a
r/aws_iot_policy: Tidy up Update.
ewbankkit Nov 10, 2023
9540f23
r/aws_iot_policy: Tidy up Read.
ewbankkit Nov 10, 2023
d2d01b8
r/aws_iot_policy: Tidy up Delete.
ewbankkit Nov 10, 2023
692979c
r/aws_iot_policy: Tidy up acceptance tests.
ewbankkit Nov 10, 2023
e317ee3
Add CHANGELOG entry.
ewbankkit Nov 10, 2023
a7edf47
r/aws_iot_policy: Add configurable timeouts.
ewbankkit Nov 10, 2023
c1560fa
Acceptance test output:
ewbankkit Nov 10, 2023
e4be94a
Add 'TestAccIoTPolicy_update'.
ewbankkit Nov 10, 2023
58f8278
Acceptance test output:
ewbankkit Nov 10, 2023
badbd6f
Add 'TestAccIoTPolicy_prune'.
ewbankkit Nov 10, 2023
c127f06
r/aws_iot_policy: When updating the resource, delete the oldest non-d…
ewbankkit Nov 10, 2023
384459d
r/aws_iot_policy_attachment: Correct order of CRUD handlers.
ewbankkit Nov 10, 2023
0f3043f
r/aws_iot_policy_attachment: Tidy up Create.
ewbankkit Nov 10, 2023
fbb6f52
r/aws_iot_policy_attachment: Tidy up Delete.
ewbankkit Nov 10, 2023
10999fe
Fix providerlint 'AWSAT005'.
ewbankkit Nov 10, 2023
c78f1da
r/aws_iot_policy_attachment: Tidy up Read.
ewbankkit Nov 10, 2023
d41c2da
r/aws_iot_policy_attachment: Tidy up acceptance tests.
ewbankkit Nov 10, 2023
584ce79
Fix providerlint 'AWSAT005'.
ewbankkit Nov 10, 2023
cd4ab9a
Fix golangci-lint 'revive/superfluous-else'.
ewbankkit Nov 11, 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
11 changes: 11 additions & 0 deletions .changelog/34329.txt
Original file line number Diff line number Diff line change
@@ -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)
```
245 changes: 189 additions & 56 deletions internal/service/iot/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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,
Expand All @@ -47,14 +67,6 @@ func ResourcePolicy() *schema.Resource {
return json
},
},
"arn": {
Type: schema.TypeString,
Computed: true,
},
"default_version_id": {
Type: schema.TypeString,
Computed: true,
},
},
}
}
Expand All @@ -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)...)
}
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)...)
Expand All @@ -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
}
Loading
Loading