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 22 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)
```
243 changes: 187 additions & 56 deletions internal/service/iot/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,24 @@

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 @@
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 @@
return json
},
},
"arn": {
Type: schema.TypeString,
Computed: true,
},
"default_version_id": {
Type: schema.TypeString,
Computed: true,
},
},
}
}
Expand All @@ -68,16 +80,19 @@
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 @@
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 @@
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,61 @@
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
}

if v, err := strconv.Atoi(aws.StringValue(v.VersionId)); err != nil {
continue
} else {

Check warning on line 165 in internal/service/iot/policy.go

View workflow job for this annotation

GitHub Actions / 2 of 2

superfluous-else: if block ends with a continue statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
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 +195,122 @@
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, "reading IoT Policy (%s) versions: %s", d.Id(), err)
}

// Delete all non-default versions of the policy.
for _, v := range policyVersions {
if aws.BoolValue(v.IsDefaultVersion) {
continue
}

if err := deletePolicyVersion(ctx, conn, d.Id(), aws.StringValue(v.VersionId), d.Timeout(schema.TimeoutDelete)); err != nil {
return sdkdiag.AppendFromErr(diags, 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,
}
}

if err != nil {
return sdkdiag.AppendErrorf(diags, "listing IoT Policy (%s) versions: %s", d.Id(), err)
return nil, 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,
})
if output == nil {
return nil, tfresource.NewEmptyResultError(input)
}

if tfawserr.ErrCodeEquals(err, iot.ErrCodeResourceNotFoundException) {
continue
}
return output, nil
}

if err != nil {
return sdkdiag.AppendErrorf(diags, "deleting IoT Policy (%s) version (%s): %s", d.Id(), aws.StringValue(ver.VersionId), err)
}
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 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 || 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 diags
return nil
}

if err != nil {
return sdkdiag.AppendErrorf(diags, "deleting IoT Policy (%s): %s", d.Id(), err)
return fmt.Errorf("deleting IoT Policy (%s): %w", name, err)
}

return diags
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