Skip to content

Commit

Permalink
resource/aws_s3_bucket: Add d.IsNewResource() checks before removing …
Browse files Browse the repository at this point in the history 
…from state (#18488)

Reference: #11064
Reference: #11894
Reference: #16796

The read-after-create retry logic was previously introduced, however to fully satisfy upcoming linting, this ensures  `d.IsNewResource()` is always checked before potential state removal.

Output from acceptance testing in AWS Commercial:

```
--- PASS: TestAccAWSS3Bucket_acceleration (162.34s)
--- PASS: TestAccAWSS3Bucket_AclToGrant (120.99s)
--- PASS: TestAccAWSS3Bucket_basic (53.62s)
--- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (68.41s)
--- PASS: TestAccAWSS3Bucket_Cors_Delete (43.05s)
--- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (32.87s)
--- PASS: TestAccAWSS3Bucket_Cors_Update (144.98s)
--- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (120.85s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (53.62s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (63.60s)
--- PASS: TestAccAWSS3Bucket_forceDestroy (43.05s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (44.07s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (52.76s)
--- PASS: TestAccAWSS3Bucket_generatedName (44.56s)
--- PASS: TestAccAWSS3Bucket_GrantToAcl (131.72s)
--- PASS: TestAccAWSS3Bucket_ignoreTags (48.65s)
--- PASS: TestAccAWSS3Bucket_LifecycleBasic (185.77s)
--- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (133.55s)
--- PASS: TestAccAWSS3Bucket_LifecycleRule_AbortIncompleteMultipartUploadDays_NoExpiration (91.92s)
--- PASS: TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock (68.06s)
--- PASS: TestAccAWSS3Bucket_Logging (80.94s)
--- PASS: TestAccAWSS3Bucket_namePrefix (86.62s)
--- PASS: TestAccAWSS3Bucket_objectLock (120.99s)
--- PASS: TestAccAWSS3Bucket_Policy (204.40s)
--- PASS: TestAccAWSS3Bucket_Replication (170.13s)
--- PASS: TestAccAWSS3Bucket_Replication_MultipleDestinations_EmptyFilter (96.34s)
--- PASS: TestAccAWSS3Bucket_Replication_MultipleDestinations_NonEmptyFilter (90.92s)
--- PASS: TestAccAWSS3Bucket_Replication_MultipleDestinations_TwoDestination (81.48s)
--- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (127.52s)
--- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AddAccessControlTranslation (152.48s)
--- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (19.04s)
--- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (127.40s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (67.60s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (68.88s)
--- PASS: TestAccAWSS3Bucket_RequestPayer (55.86s)
--- PASS: TestAccAWSS3Bucket_SameRegionReplicationSchemaV2 (86.01s)
--- PASS: TestAccAWSS3Bucket_shouldFailNotFound (27.15s)
--- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (184.89s)
--- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (249.63s)
--- PASS: TestAccAWSS3Bucket_UpdateAcl (165.05s)
--- PASS: TestAccAWSS3Bucket_UpdateGrant (202.57s)
--- PASS: TestAccAWSS3Bucket_Versioning (185.93s)
--- PASS: TestAccAWSS3Bucket_Website_Simple (194.76s)
--- PASS: TestAccAWSS3Bucket_WebsiteRedirect (205.52s)
--- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (145.48s)
```

Output from acceptance testing in AWS GovCloud (US):

```
--- PASS: TestAccAWSS3Bucket_AclToGrant (160.61s)
--- PASS: TestAccAWSS3Bucket_basic (102.32s)
--- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (47.12s)
--- PASS: TestAccAWSS3Bucket_Cors_Delete (73.58s)
--- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (93.14s)
--- PASS: TestAccAWSS3Bucket_Cors_Update (124.65s)
--- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (177.31s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (102.41s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (104.21s)
--- PASS: TestAccAWSS3Bucket_forceDestroy (30.55s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (77.52s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (82.55s)
--- PASS: TestAccAWSS3Bucket_generatedName (56.14s)
--- PASS: TestAccAWSS3Bucket_GrantToAcl (148.69s)
--- PASS: TestAccAWSS3Bucket_ignoreTags (77.72s)
--- PASS: TestAccAWSS3Bucket_LifecycleBasic (106.81s)
--- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (57.95s)
--- PASS: TestAccAWSS3Bucket_LifecycleRule_AbortIncompleteMultipartUploadDays_NoExpiration (94.53s)
--- PASS: TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock (93.10s)
--- PASS: TestAccAWSS3Bucket_Logging (96.14s)
--- PASS: TestAccAWSS3Bucket_namePrefix (86.46s)
--- PASS: TestAccAWSS3Bucket_objectLock (177.29s)
--- PASS: TestAccAWSS3Bucket_Policy (139.09s)
--- PASS: TestAccAWSS3Bucket_Replication (257.20s)
--- PASS: TestAccAWSS3Bucket_Replication_MultipleDestinations_TwoDestination (83.12s)
--- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (141.90s)
--- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AddAccessControlTranslation (199.30s)
--- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (54.11s)
--- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (127.71s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (118.37s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (40.94s)
--- PASS: TestAccAWSS3Bucket_RequestPayer (60.94s)
--- PASS: TestAccAWSS3Bucket_SameRegionReplicationSchemaV2 (117.02s)
--- PASS: TestAccAWSS3Bucket_shouldFailNotFound (55.59s)
--- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (132.60s)
--- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (170.72s)
--- PASS: TestAccAWSS3Bucket_UpdateAcl (179.16s)
--- PASS: TestAccAWSS3Bucket_UpdateGrant (168.05s)
--- PASS: TestAccAWSS3Bucket_Versioning (152.26s)
--- PASS: TestAccAWSS3Bucket_Website_Simple (190.02s)
--- PASS: TestAccAWSS3Bucket_WebsiteRedirect (191.16s)
--- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (60.83s)
--- SKIP: TestAccAWSS3Bucket_acceleration (0.00s)
--- SKIP: TestAccAWSS3Bucket_Replication_MultipleDestinations_EmptyFilter (46.15s)
--- SKIP: TestAccAWSS3Bucket_Replication_MultipleDestinations_NonEmptyFilter (46.08s)
```
  • Loading branch information
@bflad
bflad authored Mar 31, 2021
1 parent 50d933f commit c49c6c2
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions aws/resource_aws_s3_bucket.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ import (
"github.com/aws/aws-sdk-go/aws/request"
"github.com/aws/aws-sdk-go/service/s3"
"github.com/aws/aws-sdk-go/service/s3/s3manager"
"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/structure"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
)

const s3BucketCreationTimeout = 2 * time.Minute
Expand Down Expand Up @@ -810,7 +812,7 @@ func resourceAwsS3BucketRead(d *schema.ResourceData, meta interface{}) error {
return resource.RetryableError(err)
}

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

Expand All @@ -821,18 +823,24 @@ func resourceAwsS3BucketRead(d *schema.ResourceData, meta interface{}) error {
return nil
})

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

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

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

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

// In the import case, we won't have this
Expand Down

0 comments on commit c49c6c2

Please sign in to comment.