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

s3_bucket_lifecycle_configuration: Fix import, diffs #23144

Merged
merged 8 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/23144.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_s3_bucket_lifecycle_configuration: Fix extraneous diffs especially after import
```
70 changes: 48 additions & 22 deletions internal/service/s3/bucket_lifecycle_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@ import (
"context"
"fmt"
"log"
"reflect"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/s3"
"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/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/experimental/nullable"
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
)

Expand Down Expand Up @@ -114,14 +119,12 @@ func ResourceBucketLifecycleConfiguration() *schema.Resource {
},
},
"object_size_greater_than": {
Type: schema.TypeInt,
Type: nullable.TypeNullableInt,
Optional: true,
Default: 0, // API returns 0
},
"object_size_less_than": {
Type: schema.TypeInt,
Type: nullable.TypeNullableInt,
Optional: true,
Default: 0, // API returns 0
},
"prefix": {
Type: schema.TypeString,
Expand Down Expand Up @@ -249,7 +252,7 @@ func resourceBucketLifecycleConfigurationCreate(ctx context.Context, d *schema.R

rules, err := ExpandLifecycleRules(d.Get("rule").([]interface{}))
if err != nil {
return diag.FromErr(fmt.Errorf("error creating S3 Lifecycle Configuration for bucket (%s): %w", bucket, err))
return diag.Errorf("error creating S3 Lifecycle Configuration for bucket (%s): %s", bucket, err)
}

input := &s3.PutBucketLifecycleConfigurationInput{
Expand All @@ -268,11 +271,15 @@ func resourceBucketLifecycleConfigurationCreate(ctx context.Context, d *schema.R
})

if err != nil {
return diag.FromErr(fmt.Errorf("error creating S3 Lifecycle Configuration for bucket (%s): %w", bucket, err))
return diag.Errorf("error creating S3 Lifecycle Configuration for bucket (%s): %s", bucket, err)
}

d.SetId(CreateResourceID(bucket, expectedBucketOwner))

if _, err = waitForLifecycleConfigurationRulesStatus(ctx, conn, bucket, expectedBucketOwner, rules); err != nil {
return diag.Errorf("error waiting for S3 Lifecycle Configuration for bucket (%s) to reach expected rules status after update: %s", d.Id(), err)
}

return resourceBucketLifecycleConfigurationRead(ctx, d, meta)
}

Expand All @@ -292,30 +299,49 @@ func resourceBucketLifecycleConfigurationRead(ctx context.Context, d *schema.Res
input.ExpectedBucketOwner = aws.String(expectedBucketOwner)
}

output, err := verify.RetryOnAWSCode(ErrCodeNoSuchLifecycleConfiguration, func() (interface{}, error) {
return conn.GetBucketLifecycleConfigurationWithContext(ctx, input)
var lastOutput, output *s3.GetBucketLifecycleConfigurationOutput

err = resource.RetryContext(ctx, lifecycleConfigurationRulesSteadyTimeout, func() *resource.RetryError {
var err error

time.Sleep(lifecycleConfigurationRetryDelay)

output, err = conn.GetBucketLifecycleConfigurationWithContext(ctx, input)

if d.IsNewResource() && tfawserr.ErrCodeEquals(err, ErrCodeNoSuchLifecycleConfiguration, s3.ErrCodeNoSuchBucket) {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

if lastOutput == nil || !reflect.DeepEqual(*lastOutput, *output) {
lastOutput = output
return resource.RetryableError(fmt.Errorf("bucket lifecycle configuration has not stablized; trying again"))
}

return nil
})

if tfresource.TimedOut(err) {
output, err = conn.GetBucketLifecycleConfigurationWithContext(ctx, input)
}

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, ErrCodeNoSuchLifecycleConfiguration, s3.ErrCodeNoSuchBucket) {
log.Printf("[WARN] S3 Bucket Lifecycle Configuration (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}

if err != nil {
return diag.FromErr(fmt.Errorf("error getting S3 Bucket Lifecycle Configuration (%s): %w", d.Id(), err))
}

lifecycleConfig, ok := output.(*s3.GetBucketLifecycleConfigurationOutput)

if !ok || lifecycleConfig == nil {
return diag.FromErr(fmt.Errorf("error reading S3 Bucket Lifecycle Configuration (%s): empty output", d.Id()))
return diag.Errorf("error getting S3 Bucket Lifecycle Configuration (%s): %s", d.Id(), err)
}

d.Set("bucket", bucket)
d.Set("expected_bucket_owner", expectedBucketOwner)
if err := d.Set("rule", FlattenLifecycleRules(lifecycleConfig.Rules)); err != nil {
return diag.FromErr(fmt.Errorf("error setting rule: %w", err))
if err := d.Set("rule", FlattenLifecycleRules(output.Rules)); err != nil {
return diag.Errorf("error setting rule: %s", err)
}

return nil
Expand All @@ -331,7 +357,7 @@ func resourceBucketLifecycleConfigurationUpdate(ctx context.Context, d *schema.R

rules, err := ExpandLifecycleRules(d.Get("rule").([]interface{}))
if err != nil {
return diag.FromErr(fmt.Errorf("error updating S3 Bucket Lifecycle Configuration rule: %w", err))
return diag.Errorf("error updating S3 Bucket Lifecycle Configuration rule: %s", err)
}

input := &s3.PutBucketLifecycleConfigurationInput{
Expand All @@ -350,11 +376,11 @@ func resourceBucketLifecycleConfigurationUpdate(ctx context.Context, d *schema.R
})

if err != nil {
return diag.FromErr(fmt.Errorf("error updating S3 Bucket Lifecycle Configuration (%s): %w", d.Id(), err))
return diag.Errorf("error updating S3 Bucket Lifecycle Configuration (%s): %s", d.Id(), err)
}

if err := waitForLifecycleConfigurationRulesStatus(ctx, conn, bucket, expectedBucketOwner, rules); err != nil {
return diag.FromErr(fmt.Errorf("error waiting for S3 Lifecycle Configuration for bucket (%s) to reach expected rules status after update: %w", d.Id(), err))
if _, err := waitForLifecycleConfigurationRulesStatus(ctx, conn, bucket, expectedBucketOwner, rules); err != nil {
return diag.Errorf("error waiting for S3 Lifecycle Configuration for bucket (%s) to reach expected rules status after update: %s", d.Id(), err)
}

return resourceBucketLifecycleConfigurationRead(ctx, d, meta)
Expand Down Expand Up @@ -383,7 +409,7 @@ func resourceBucketLifecycleConfigurationDelete(ctx context.Context, d *schema.R
}

if err != nil {
return diag.FromErr(fmt.Errorf("error deleting S3 Bucket Lifecycle Configuration (%s): %w", d.Id(), err))
return diag.Errorf("error deleting S3 Bucket Lifecycle Configuration (%s): %s", d.Id(), err)
}

return nil
Expand Down
17 changes: 1 addition & 16 deletions internal/service/s3/bucket_lifecycle_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestAccS3BucketLifecycleConfiguration_basic(t *testing.T) {
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "rule.*", map[string]string{
"expiration.#": "1",
"expiration.0.days": "365",
"filter.#": "1",
"filter.#": "0",
"id": rName,
"status": tfs3.LifecycleRuleStatusEnabled,
}),
Expand Down Expand Up @@ -706,9 +706,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
expiration {
days = 365
}

# One of prefix or filter required to ensure XML is well-formed
filter {}
}
}
`, rName)
Expand All @@ -734,9 +731,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
expiration {
days = 365
}

# One of prefix or filter required to ensure XML is well-formed
filter {}
}
}
`, rName, status)
Expand Down Expand Up @@ -821,9 +815,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
expiration {
expired_object_delete_marker = %[2]t
}

# One of prefix or filter required to ensure XML is well-formed
filter {}
}
}
`, rName, expired)
Expand All @@ -848,9 +839,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {
status = "Enabled"

expiration {}

# One of prefix or filter required to ensure XML is well-formed
filter {}
}
}
`, rName)
Expand All @@ -877,9 +865,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "test" {

id = %[1]q
status = "Enabled"

# One of prefix or filter required to ensure XML is well-formed
filter {}
}
}
`, rName, days)
Expand Down
14 changes: 11 additions & 3 deletions internal/service/s3/flex.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,15 @@ func FlattenLifecycleRuleExpiration(expiration *s3.LifecycleExpiration) []interf

func FlattenLifecycleRuleFilter(filter *s3.LifecycleRuleFilter) []interface{} {
if filter == nil {
return []interface{}{}
return nil
}

if filter.And == nil &&
filter.ObjectSizeGreaterThan == nil &&
filter.ObjectSizeLessThan == nil &&
(filter.Prefix == nil || aws.StringValue(filter.Prefix) == "") &&
filter.Tag == nil {
return nil
}

m := make(map[string]interface{})
Expand All @@ -900,7 +908,7 @@ func FlattenLifecycleRuleFilter(filter *s3.LifecycleRuleFilter) []interface{} {
m["object_size_less_than"] = int(aws.Int64Value(filter.ObjectSizeLessThan))
}

if filter.Prefix != nil {
if filter.Prefix != nil && aws.StringValue(filter.Prefix) != "" {
m["prefix"] = aws.StringValue(filter.Prefix)
}

Expand Down Expand Up @@ -939,7 +947,7 @@ func FlattenLifecycleRuleFilterAndOperator(andOp *s3.LifecycleRuleAndOperator) [

func FlattenLifecycleRuleFilterTag(tag *s3.Tag) []interface{} {
if tag == nil {
return []interface{}{}
return nil
}

t := KeyValueTags([]*s3.Tag{tag}).IgnoreAWS().Map()
Expand Down
2 changes: 1 addition & 1 deletion internal/service/s3/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func lifecycleConfigurationRulesStatus(ctx context.Context, conn *s3.S3, bucket,

output, err := conn.GetBucketLifecycleConfigurationWithContext(ctx, input)

if tfawserr.ErrCodeEquals(err, ErrCodeNoSuchLifecycleConfiguration) {
if tfawserr.ErrCodeEquals(err, ErrCodeNoSuchLifecycleConfiguration, s3.ErrCodeNoSuchBucket) {
return nil, "", nil
}

Expand Down
23 changes: 16 additions & 7 deletions internal/service/s3/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ const (
bucketCreatedTimeout = 2 * time.Minute
propagationTimeout = 1 * time.Minute
lifecycleConfigurationRulesPropagationTimeout = 2 * time.Minute
lifecycleConfigurationRulesSteadyTimeout = 2 * time.Minute
bucketVersioningStableTimeout = 1 * time.Minute
lifecycleConfigurationRetryDelay = 15 * time.Second

// LifecycleConfigurationRulesStatusReady occurs when all configured rules reach their desired state (Enabled or Disabled)
LifecycleConfigurationRulesStatusReady = "READY"
Expand All @@ -25,17 +27,24 @@ func retryWhenBucketNotFound(f func() (interface{}, error)) (interface{}, error)
return tfresource.RetryWhenAWSErrCodeEquals(propagationTimeout, f, s3.ErrCodeNoSuchBucket)
}

func waitForLifecycleConfigurationRulesStatus(ctx context.Context, conn *s3.S3, bucket, expectedBucketOwner string, rules []*s3.LifecycleRule) error {
func waitForLifecycleConfigurationRulesStatus(ctx context.Context, conn *s3.S3, bucket, expectedBucketOwner string, rules []*s3.LifecycleRule) (*s3.GetBucketLifecycleConfigurationOutput, error) {
stateConf := &resource.StateChangeConf{
Pending: []string{"", LifecycleConfigurationRulesStatusNotReady},
Target: []string{LifecycleConfigurationRulesStatusReady},
Refresh: lifecycleConfigurationRulesStatus(ctx, conn, bucket, expectedBucketOwner, rules),
Timeout: lifecycleConfigurationRulesPropagationTimeout,
Pending: []string{"", LifecycleConfigurationRulesStatusNotReady},
Target: []string{LifecycleConfigurationRulesStatusReady},
Refresh: lifecycleConfigurationRulesStatus(ctx, conn, bucket, expectedBucketOwner, rules),
Timeout: lifecycleConfigurationRulesPropagationTimeout,
MinTimeout: 5 * time.Second,
ContinuousTargetOccurence: 3,
NotFoundChecks: 20,
}

_, err := stateConf.WaitForState()
outputRaw, err := stateConf.WaitForState()

if output, ok := outputRaw.(*s3.GetBucketLifecycleConfigurationOutput); ok {
return output, err
}

return err
return nil, err
}

func waitForBucketVersioningStatus(ctx context.Context, conn *s3.S3, bucket, expectedBucketOwner string) (*s3.GetBucketVersioningOutput, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ The `rule` configuration block supports the following arguments:

* `abort_incomplete_multipart_upload` - (Optional) Configuration block that specifies the days since the initiation of an incomplete multipart upload that Amazon S3 will wait before permanently removing all parts of the upload [documented below](#abort_incomplete_multipart_upload).
* `expiration` - (Optional) Configuration block that specifies the expiration for the lifecycle of the object in the form of date, days and, whether the object has a delete marker [documented below](#expiration).
* `filter` - (Optional, Required if `prefix` not specified) Configuration block used to identify objects that a Lifecycle Rule applies to [documented below](#filter).
* `filter` - (Optional) Configuration block used to identify objects that a Lifecycle Rule applies to [documented below](#filter).
* `id` - (Required) Unique identifier for the rule. The value cannot be longer than 255 characters.
* `noncurrent_version_expiration` - (Optional) Configuration block that specifies when noncurrent object versions expire [documented below](#noncurrent_version_expiration).
* `noncurrent_version_transition` - (Optional) Set of configuration blocks that specify the transition rule for the lifecycle rule that describes when noncurrent objects transition to a specific storage class [documented below](#noncurrent_version_transition).
* `prefix` - (Optional, Required if `filter` not specified) Prefix identifying one or more objects to which the rule applies. This has been deprecated by Amazon S3 and `filter` should be used instead.
* `prefix` - (Optional) **DEPRECATED** Use `filter` instead. This has been deprecated by Amazon S3. Prefix identifying one or more objects to which the rule applies.
* `status` - (Required) Whether the rule is currently being applied. Valid values: `Enabled` or `Disabled`.
* `transition` - (Optional) Set of configuration blocks that specify when an Amazon S3 object transitions to a specified storage class [documented below](#transition).

Expand Down