Skip to content

Commit

Permalink
service/s3: Add d.IsNewResource checks and standardize retry logic (#…
Browse files Browse the repository at this point in the history 
…18537)

* service/s3: Add d.IsNewResource checks and standardize retry logic

Reference: #10587
Reference: #16796

Output from acceptance testing:

```
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_basic (38.97s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_removed (56.16s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_updateBasic (87.68s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithFilter_Empty (12.16s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithFilter_MultipleTags (60.16s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithFilter_Prefix (60.97s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithFilter_PrefixAndTags (60.97s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithFilter_Remove (61.00s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithFilter_SingleTag (60.97s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithStorageClassAnalysis_Default (39.98s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithStorageClassAnalysis_Empty (12.14s)
--- PASS: TestAccAWSS3BucketAnalyticsConfiguration_WithStorageClassAnalysis_Full (39.60s)

--- PASS: TestAccAWSS3BucketInventory_basic (26.27s)
--- PASS: TestAccAWSS3BucketInventory_encryptWithSSES3 (26.34s)
--- PASS: TestAccAWSS3BucketInventory_encryptWithSSEKMS (26.45s)

--- PASS: TestAccAWSS3BucketMetric_basic (26.21s)
--- PASS: TestAccAWSS3BucketMetric_WithEmptyFilter (2.25s)
--- PASS: TestAccAWSS3BucketMetric_WithFilterMultipleTags (46.71s)
--- PASS: TestAccAWSS3BucketMetric_WithFilterPrefix (45.51s)
--- PASS: TestAccAWSS3BucketMetric_WithFilterPrefixAndMultipleTags (46.55s)
--- PASS: TestAccAWSS3BucketMetric_WithFilterPrefixAndSingleTag (45.88s)
--- PASS: TestAccAWSS3BucketMetric_WithFilterSingleTag (46.49s)

--- PASS: TestAccAWSS3BucketNotification_LambdaFunction (45.26s)
--- PASS: TestAccAWSS3BucketNotification_LambdaFunction_LambdaFunctionArn_Alias (40.24s)
--- PASS: TestAccAWSS3BucketNotification_Queue (27.96s)
--- PASS: TestAccAWSS3BucketNotification_Topic (28.53s)
--- PASS: TestAccAWSS3BucketNotification_Topic_Multiple (29.23s)
--- PASS: TestAccAWSS3BucketNotification_update (49.26s)

--- PASS: TestAccAWSS3BucketPublicAccessBlock_basic (29.52s)
--- PASS: TestAccAWSS3BucketPublicAccessBlock_BlockPublicAcls (68.84s)
--- PASS: TestAccAWSS3BucketPublicAccessBlock_BlockPublicPolicy (68.21s)
--- PASS: TestAccAWSS3BucketPublicAccessBlock_disappears (26.48s)
--- PASS: TestAccAWSS3BucketPublicAccessBlock_disappears_Bucket (18.16s)
--- PASS: TestAccAWSS3BucketPublicAccessBlock_IgnorePublicAcls (68.18s)
--- PASS: TestAccAWSS3BucketPublicAccessBlock_RestrictPublicBuckets (68.83s)
```

* resource/aws_s3_bucket_metric: Standardize on S3 Bucket Metrics Configuration friendly name in logging and errors
  • Loading branch information
@bflad
bflad committed Apr 2, 2021
1 parent 8e86e12 commit 90a4677
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 104 deletions.
1 change: 1 addition & 0 deletions aws/internal/service/s3/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package s3
// https://docs.aws.amazon.com/sdk-for-go/api/service/s3/#pkg-constants

const (
ErrCodeNoSuchConfiguration = "NoSuchConfiguration"
ErrCodeNoSuchPublicAccessBlockConfiguration = "NoSuchPublicAccessBlockConfiguration"
ErrCodeNoSuchTagSet = "NoSuchTagSet"
)
10 changes: 10 additions & 0 deletions aws/internal/service/s3/waiter/waiter.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package waiter

import (
"time"
)

const (
// Maximum amount of time to wait for S3 changes to propagate
PropagationTimeout = 1 * time.Minute
)
39 changes: 28 additions & 11 deletions aws/resource_aws_s3_bucket_analytics_configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
tfs3 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/s3"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

Expand Down Expand Up @@ -147,19 +148,23 @@ func resourceAwsS3BucketAnalyticsConfigurationPut(d *schema.ResourceData, meta i

err := resource.Retry(1*time.Minute, func() *resource.RetryError {
_, err := s3conn.PutBucketAnalyticsConfiguration(input)

if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) {
return resource.RetryableError(err)
}

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}
return nil
})
if isResourceTimeoutError(err) {

if tfresource.TimedOut(err) {
_, err = s3conn.PutBucketAnalyticsConfiguration(input)
}

if err != nil {
return fmt.Errorf("Error adding S3 analytics configuration: %w", err)
return fmt.Errorf("error adding S3 Bucket Analytics Configuration: %w", err)
}

d.SetId(fmt.Sprintf("%s:%s", bucket, name))
Expand All @@ -185,13 +190,25 @@ func resourceAwsS3BucketAnalyticsConfigurationRead(d *schema.ResourceData, meta

log.Printf("[DEBUG] Reading S3 bucket analytics configuration: %s", input)
output, err := conn.GetBucketAnalyticsConfiguration(input)

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

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

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NoSuchConfiguration", "The specified configuration does not exist.") {
log.Printf("[WARN] %s S3 bucket analytics configuration not found, removing from state.", d.Id())
d.SetId("")
return nil
}
return fmt.Errorf("error getting S3 Bucket Analytics Configuration %q: %w", d.Id(), err)
return fmt.Errorf("error getting S3 Bucket Analytics Configuration (%s): %w", d.Id(), err)
}

if output == nil {
return fmt.Errorf("error getting S3 Bucket Analytics Configuration (%s): empty response", d.Id())
}

if err := d.Set("filter", flattenS3AnalyticsFilter(output.AnalyticsConfiguration.Filter)); err != nil {
Expand Down
81 changes: 53 additions & 28 deletions aws/resource_aws_s3_bucket_inventory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import (
"fmt"
"log"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"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"
tfs3 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/s3"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/s3/waiter"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

func resourceAwsS3BucketInventory() *schema.Resource {
Expand Down Expand Up @@ -232,21 +235,26 @@ func resourceAwsS3BucketInventoryPut(d *schema.ResourceData, meta interface{}) e
}

log.Printf("[DEBUG] Putting S3 bucket inventory configuration: %s", input)
err := resource.Retry(1*time.Minute, func() *resource.RetryError {
err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
_, err := conn.PutBucketInventoryConfiguration(input)

if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) {
return resource.RetryableError(err)
}

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}

return nil
})
if isResourceTimeoutError(err) {

if tfresource.TimedOut(err) {
_, err = conn.PutBucketInventoryConfiguration(input)
}

if err != nil {
return fmt.Errorf("Error putting S3 bucket inventory configuration: %s", err)
return fmt.Errorf("error putting S3 Bucket Inventory Configuration: %w", err)
}

d.SetId(fmt.Sprintf("%s:%s", bucket, name))
Expand All @@ -269,11 +277,17 @@ func resourceAwsS3BucketInventoryDelete(d *schema.ResourceData, meta interface{}

log.Printf("[DEBUG] Deleting S3 bucket inventory configuration: %s", input)
_, err = conn.DeleteBucketInventoryConfiguration(input)

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

if tfawserr.ErrCodeEquals(err, tfs3.ErrCodeNoSuchConfiguration) {
return nil
}

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NoSuchConfiguration", "The specified configuration does not exist.") {
return nil
}
return fmt.Errorf("Error deleting S3 bucket inventory configuration: %s", err)
return fmt.Errorf("error deleting S3 Bucket Inventory Configuration (%s): %w", d.Id(), err)
}

return nil
Expand All @@ -297,38 +311,49 @@ func resourceAwsS3BucketInventoryRead(d *schema.ResourceData, meta interface{})

log.Printf("[DEBUG] Reading S3 bucket inventory configuration: %s", input)
var output *s3.GetBucketInventoryConfigurationOutput
err = resource.Retry(1*time.Minute, func() *resource.RetryError {
err = resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
var err error
output, err = conn.GetBucketInventoryConfiguration(input)

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

if d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfs3.ErrCodeNoSuchConfiguration) {
return resource.RetryableError(err)
}

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NoSuchConfiguration", "The specified configuration does not exist.") {
if d.IsNewResource() {
return resource.RetryableError(err)
}
return nil
}
return resource.NonRetryableError(err)
}

return nil
})
if isResourceTimeoutError(err) {

if tfresource.TimedOut(err) {
output, err = conn.GetBucketInventoryConfiguration(input)
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NoSuchConfiguration", "The specified configuration does not exist.") {
if !d.IsNewResource() {
return nil
}
}
}
if err != nil {
return fmt.Errorf("error getting S3 Bucket Inventory (%s): %s", d.Id(), err)

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

if output == nil || output.InventoryConfiguration == nil {
log.Printf("[WARN] %s S3 bucket inventory configuration not found, removing from state.", d.Id())
if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, tfs3.ErrCodeNoSuchConfiguration) {
log.Printf("[WARN] S3 Bucket Inventory Configuration (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}

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

if output == nil || output.InventoryConfiguration == nil {
return fmt.Errorf("error getting S3 Bucket Inventory Configuration (%s): empty response", d.Id())
}

d.Set("enabled", output.InventoryConfiguration.IsEnabled)
d.Set("included_object_versions", output.InventoryConfiguration.IncludedObjectVersions)

Expand Down
66 changes: 46 additions & 20 deletions aws/resource_aws_s3_bucket_metric.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import (
"fmt"
"log"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/hashicorp/aws-sdk-go-base/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
tfs3 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/s3"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/s3/waiter"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

func resourceAwsS3BucketMetric() *schema.Resource {
Expand Down Expand Up @@ -80,22 +83,27 @@ func resourceAwsS3BucketMetricPut(d *schema.ResourceData, meta interface{}) erro
MetricsConfiguration: metricsConfiguration,
}

log.Printf("[DEBUG] Putting metric configuration: %s", input)
err := resource.Retry(1*time.Minute, func() *resource.RetryError {
log.Printf("[DEBUG] Putting S3 Bucket Metrics Configuration: %s", input)
err := resource.Retry(waiter.PropagationTimeout, func() *resource.RetryError {
_, err := conn.PutBucketMetricsConfiguration(input)

if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) {
return resource.RetryableError(err)
}

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}

return nil
})
if isResourceTimeoutError(err) {

if tfresource.TimedOut(err) {
_, err = conn.PutBucketMetricsConfiguration(input)
}

if err != nil {
return fmt.Errorf("Error putting S3 metric configuration: %s", err)
return fmt.Errorf("error putting S3 Bucket Metrics Configuration: %w", err)
}

d.SetId(fmt.Sprintf("%s:%s", bucket, name))
Expand All @@ -116,13 +124,19 @@ func resourceAwsS3BucketMetricDelete(d *schema.ResourceData, meta interface{}) e
Id: aws.String(name),
}

log.Printf("[DEBUG] Deleting S3 bucket metric configuration: %s", input)
log.Printf("[DEBUG] Deleting S3 Bucket Metrics Configuration: %s", input)
_, err = conn.DeleteBucketMetricsConfiguration(input)

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

if tfawserr.ErrCodeEquals(err, tfs3.ErrCodeNoSuchConfiguration) {
return nil
}

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NoSuchConfiguration", "The specified configuration does not exist.") {
return nil
}
return fmt.Errorf("Error deleting S3 metric configuration: %w", err)
return fmt.Errorf("error deleting S3 Bucket Metrics Configuration (%s): %w", d.Id(), err)
}

return nil
Expand All @@ -144,15 +158,27 @@ func resourceAwsS3BucketMetricRead(d *schema.ResourceData, meta interface{}) err
Id: aws.String(name),
}

log.Printf("[DEBUG] Reading S3 bucket metrics configuration: %s", input)
log.Printf("[DEBUG] Reading S3 Bucket Metrics Configuration: %s", input)
output, err := conn.GetBucketMetricsConfiguration(input)

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

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

if err != nil {
if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NoSuchConfiguration", "The specified configuration does not exist.") {
log.Printf("[WARN] %s S3 bucket metrics configuration not found, removing from state.", d.Id())
d.SetId("")
return nil
}
return err
return fmt.Errorf("error reading S3 Bucket Metrics Configuration (%s): %w", d.Id(), err)
}

if output == nil || output.MetricsConfiguration == nil {
return fmt.Errorf("error reading S3 Bucket Metrics Configuration (%s): empty response", d.Id())
}

if output.MetricsConfiguration.Filter != nil {
Expand Down
Loading

0 comments on commit 90a4677

Please sign in to comment.