From d8a010dd9cb1713288254b39b13ee5493d09428a Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 4 Feb 2022 14:56:27 -0500 Subject: [PATCH 01/11] r/s3_bucket: deprecate 'acl' and 'grant' arguments --- internal/service/s3/bucket.go | 197 ++++++------------------- internal/service/s3/bucket_test.go | 34 ++++- website/docs/r/s3_bucket.html.markdown | 36 +---- 3 files changed, 85 insertions(+), 182 deletions(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 13e95c0dad8..d1f70cb1708 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -75,46 +75,38 @@ func ResourceBucket() *schema.Resource { }, "acl": { - Type: schema.TypeString, - Default: "private", - Optional: true, - ConflictsWith: []string{"grant"}, - ValidateFunc: validation.StringInSlice(BucketCannedACL_Values(), false), + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "grant": { - Type: schema.TypeSet, - Optional: true, - Set: grantHash, - ConflictsWith: []string{"acl"}, + Type: schema.TypeSet, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "id": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "type": { - Type: schema.TypeString, - Required: true, - // TypeAmazonCustomerByEmail is not currently supported - ValidateFunc: validation.StringInSlice([]string{ - s3.TypeCanonicalUser, - s3.TypeGroup, - }, false), + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "uri": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", }, "permissions": { - Type: schema.TypeSet, - Required: true, - Set: schema.HashString, - Elem: &schema.Schema{ - Type: schema.TypeString, - ValidateFunc: validation.StringInSlice(s3.Permission_Values(), false), - }, + Type: schema.TypeSet, + Computed: true, + Deprecated: "Use the aws_s3_bucket_acl resource instead", + Elem: &schema.Schema{Type: schema.TypeString}, }, }, }, @@ -168,31 +160,31 @@ func ResourceBucket() *schema.Resource { "website": { Type: schema.TypeList, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "index_document": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "error_document": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "redirect_all_requests_to": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "routing_rules": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, }, }, @@ -211,12 +203,12 @@ func ResourceBucket() *schema.Resource { "website_endpoint": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "website_domain": { Type: schema.TypeString, Computed: true, - Deprecated: "Use the aws_s3_bucket_website_configuration resource instead when available in a future minor version", + Deprecated: "Use the aws_s3_bucket_website_configuration resource", }, "versioning": { @@ -681,12 +673,6 @@ func resourceBucketCreate(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(bucket), } - if acl, ok := d.GetOk("acl"); ok { - acl := acl.(string) - req.ACL = aws.String(acl) - log.Printf("[DEBUG] S3 bucket %s has canned ACL %s", bucket, acl) - } - awsRegion := meta.(*conns.AWSClient).Region log.Printf("[DEBUG] S3 bucket create: %s, using region: %s", bucket, awsRegion) @@ -750,18 +736,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("acl") && !d.IsNewResource() { - if err := resourceBucketACLUpdate(conn, d); err != nil { - return err - } - } - - if d.HasChange("grant") { - if err := resourceBucketGrantsUpdate(conn, d); err != nil { - return err - } - } - if d.HasChange("object_lock_configuration") { if err := resourceBucketInternalObjectLockConfigurationUpdate(conn, d); err != nil { return err @@ -842,24 +816,34 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { d.Set("policy", nil) } - //Read the Grant ACL. Reset if `acl` (canned ACL) is set. - if acl, ok := d.GetOk("acl"); ok && acl.(string) != "private" { + // Read the Grant ACL if configured outside this resource; + // In the event grants are not configured on the bucket, the API returns an empty array + + // Reset `grant` if `acl` (canned ACL) is set. + if acl, ok := d.GetOk("acl"); ok && acl.(string) != s3.BucketCannedACLPrivate { if err := d.Set("grant", nil); err != nil { - return fmt.Errorf("error resetting grant %s", err) + return fmt.Errorf("error resetting grant %w", err) } } else { - apResponse, err := verify.RetryOnAWSCode("NoSuchBucket", func() (interface{}, error) { + // Set the ACL to its default i.e. "private" (to mimic pre-v4.0 schema) + d.Set("acl", s3.BucketCannedACLPrivate) + + apResponse, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { return conn.GetBucketAcl(&s3.GetBucketAclInput{ Bucket: aws.String(d.Id()), }) }) + if err != nil { - return fmt.Errorf("error getting S3 Bucket (%s) ACL: %s", d.Id(), err) + return fmt.Errorf("error getting S3 Bucket (%s) ACL: %w", d.Id(), err) } - log.Printf("[DEBUG] S3 bucket: %s, read ACL grants policy: %+v", d.Id(), apResponse) - grants := flattenGrants(apResponse.(*s3.GetBucketAclOutput)) - if err := d.Set("grant", schema.NewSet(grantHash, grants)); err != nil { - return fmt.Errorf("error setting grant %s", err) + + if aclOutput, ok := apResponse.(*s3.GetBucketAclOutput); ok { + if err := d.Set("grant", flattenGrants(aclOutput)); err != nil { + return fmt.Errorf("error setting grant %s", err) + } + } else { + d.Set("grant", nil) } } @@ -1195,74 +1179,6 @@ func resourceBucketDelete(d *schema.ResourceData, meta interface{}) error { return nil } -func resourceBucketGrantsUpdate(conn *s3.S3, d *schema.ResourceData) error { - bucket := d.Get("bucket").(string) - rawGrants := d.Get("grant").(*schema.Set).List() - - if len(rawGrants) == 0 { - log.Printf("[DEBUG] S3 bucket: %s, Grants fallback to canned ACL", bucket) - if err := resourceBucketACLUpdate(conn, d); err != nil { - return fmt.Errorf("Error fallback to canned ACL, %s", err) - } - } else { - apResponse, err := verify.RetryOnAWSCode("NoSuchBucket", func() (interface{}, error) { - return conn.GetBucketAcl(&s3.GetBucketAclInput{ - Bucket: aws.String(d.Id()), - }) - }) - - if err != nil { - return fmt.Errorf("error getting S3 Bucket (%s) ACL: %s", d.Id(), err) - } - - ap := apResponse.(*s3.GetBucketAclOutput) - log.Printf("[DEBUG] S3 bucket: %s, read ACL grants policy: %+v", d.Id(), ap) - - grants := make([]*s3.Grant, 0, len(rawGrants)) - for _, rawGrant := range rawGrants { - log.Printf("[DEBUG] S3 bucket: %s, put grant: %#v", bucket, rawGrant) - grantMap := rawGrant.(map[string]interface{}) - for _, rawPermission := range grantMap["permissions"].(*schema.Set).List() { - ge := &s3.Grantee{} - if i, ok := grantMap["id"].(string); ok && i != "" { - ge.SetID(i) - } - if t, ok := grantMap["type"].(string); ok && t != "" { - ge.SetType(t) - } - if u, ok := grantMap["uri"].(string); ok && u != "" { - ge.SetURI(u) - } - - g := &s3.Grant{ - Grantee: ge, - Permission: aws.String(rawPermission.(string)), - } - grants = append(grants, g) - } - } - - grantsInput := &s3.PutBucketAclInput{ - Bucket: aws.String(bucket), - AccessControlPolicy: &s3.AccessControlPolicy{ - Grants: grants, - Owner: ap.Owner, - }, - } - - log.Printf("[DEBUG] S3 bucket: %s, put Grants: %#v", bucket, grantsInput) - - _, err = verify.RetryOnAWSCode("NoSuchBucket", func() (interface{}, error) { - return conn.PutBucketAcl(grantsInput) - }) - - if err != nil { - return fmt.Errorf("Error putting S3 Grants: %s", err) - } - } - return nil -} - func websiteEndpoint(client *conns.AWSClient, d *schema.ResourceData) (*S3Website, error) { // If the bucket doesn't have a website configuration, return an empty // endpoint @@ -1344,26 +1260,6 @@ func isOldRegion(region string) bool { return false } -func resourceBucketACLUpdate(conn *s3.S3, d *schema.ResourceData) error { - acl := d.Get("acl").(string) - bucket := d.Get("bucket").(string) - - i := &s3.PutBucketAclInput{ - Bucket: aws.String(bucket), - ACL: aws.String(acl), - } - log.Printf("[DEBUG] S3 put bucket ACL: %#v", i) - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return conn.PutBucketAcl(i) - }) - if err != nil { - return fmt.Errorf("Error putting S3 ACL: %s", err) - } - - return nil -} - func resourceBucketInternalObjectLockConfigurationUpdate(conn *s3.S3, d *schema.ResourceData) error { // S3 Object Lock configuration cannot be deleted, only updated. req := &s3.PutObjectLockConfigurationInput{ @@ -2212,6 +2108,9 @@ func flattenS3ObjectLockConfiguration(conf *s3.ObjectLockConfiguration) []interf } func flattenGrants(ap *s3.GetBucketAclOutput) []interface{} { + if len(ap.Grants) == 0 { + return []interface{}{} + } //if ACL grants contains bucket owner FULL_CONTROL only - it is default "private" acl if len(ap.Grants) == 1 && aws.StringValue(ap.Grants[0].Grantee.ID) == aws.StringValue(ap.Owner.ID) && aws.StringValue(ap.Grants[0].Permission) == s3.PermissionFullControl { diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 81d18d1414a..2810c8e909f 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -70,7 +70,7 @@ func TestAccS3Bucket_Basic_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -98,13 +98,16 @@ func TestAccS3Bucket_Basic_emptyString(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) } func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -164,6 +167,9 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { } func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -249,6 +255,9 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { } func TestAccS3Bucket_Tags_ignoreTags(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -294,6 +303,9 @@ func TestAccS3Bucket_Tags_ignoreTags(t *testing.T) { } func TestAccS3Bucket_Tags_basic(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + rInt := sdkacctest.RandInt() resourceName := "aws_s3_bucket.bucket1" @@ -336,7 +348,7 @@ func TestAccS3Bucket_Basic_namePrefix(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "bucket_prefix"}, + ImportStateVerifyIgnore: []string{"force_destroy", "bucket_prefix"}, }, }, }) @@ -361,7 +373,7 @@ func TestAccS3Bucket_Basic_generatedName(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "bucket_prefix"}, + ImportStateVerifyIgnore: []string{"force_destroy", "bucket_prefix"}, }, }, }) @@ -371,6 +383,9 @@ func TestAccS3Bucket_Basic_generatedName(t *testing.T) { // not empty" error in Terraform, to check against regressions. // See https://github.com/hashicorp/terraform/pull/2925 func TestAccS3Bucket_Basic_shouldFailNotFound(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.bucket" @@ -415,7 +430,7 @@ func TestAccS3Bucket_Manage_objectLock(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) @@ -450,6 +465,9 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning(t *testing.T) { } func TestAccS3Bucket_Basic_forceDestroy(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -476,6 +494,9 @@ func TestAccS3Bucket_Basic_forceDestroy(t *testing.T) { // to not contain these extra slashes, out-of-band handling and other AWS // services may create keys with extra slashes (empty "directory" prefixes). func TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -497,6 +518,9 @@ func TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes(t *testing.T) { } func TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled(t *testing.T) { + // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider + t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") + resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 1806a8c672b..79d7af1504c 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -69,25 +69,8 @@ See the [`aws_s3_bucket_server_side_encryption_configuration` resource](s3_bucke ### Using ACL policy grants -```terraform -data "aws_canonical_user_id" "current_user" {} - -resource "aws_s3_bucket" "bucket" { - bucket = "mybucket" - - grant { - id = data.aws_canonical_user_id.current_user.id - type = "CanonicalUser" - permissions = ["FULL_CONTROL"] - } - - grant { - type = "Group" - permissions = ["READ_ACP", "WRITE"] - uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" - } -} -``` +The `acl` and `grant` arguments are read-only as of version 4.0. +See the [`aws_s3_bucket_acl` resource](s3_bucket_acl.html.markdown) for configuration details. ## Argument Reference @@ -95,19 +78,10 @@ The following arguments are supported: * `bucket` - (Optional, Forces new resource) The name of the bucket. If omitted, Terraform will assign a random, unique name. Must be lowercase and less than or equal to 63 characters in length. A full list of bucket naming rules [may be found here](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html). * `bucket_prefix` - (Optional, Forces new resource) Creates a unique bucket name beginning with the specified prefix. Conflicts with `bucket`. Must be lowercase and less than or equal to 37 characters in length. A full list of bucket naming rules [may be found here](https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html). -* `acl` - (Optional) The [canned ACL](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl) to apply. Valid values are `private`, `public-read`, `public-read-write`, `aws-exec-read`, `authenticated-read`, and `log-delivery-write`. Defaults to `private`. Conflicts with `grant`. -* `grant` - (Optional) An [ACL policy grant](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#sample-acl) (documented below). Conflicts with `acl`. * `tags` - (Optional) A map of tags to assign to the bucket. If configured with a provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. * `force_destroy` - (Optional, Default:`false`) A boolean that indicates all objects (including any [locked objects](https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock-overview.html)) should be deleted from the bucket so that the bucket can be destroyed without error. These objects are *not* recoverable. * `object_lock_configuration` - (Optional) A configuration of [S3 object locking](https://docs.aws.amazon.com/AmazonS3/latest/dev/object-lock.html) (documented below) -The `grant` object supports the following: - -* `id` - (optional) Canonical user id to grant for. Used only when `type` is `CanonicalUser`. -* `type` - (required) - Type of grantee to apply for. Valid values are `CanonicalUser` and `Group`. `AmazonCustomerByEmail` is not supported. -* `permissions` - (required) List of permissions to apply for grantee. Valid values are `READ`, `WRITE`, `READ_ACP`, `WRITE_ACP`, `FULL_CONTROL`. -* `uri` - (optional) Uri address to grant for. Used only when `type` is `Group`. - The `object_lock_configuration` object supports the following: * `object_lock_enabled` - (Required) Indicates whether this bucket has an Object Lock configuration enabled. Valid value is `Enabled`. @@ -122,6 +96,7 @@ In addition to all arguments above, the following attributes are exported: * `id` - The name of the bucket. * `acceleration_status` - (Optional) The accelerate configuration status of the bucket. Not available in `cn-north-1` or `us-gov-west-1`. +* `acl` - The [canned ACL](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#canned-acl) applied to the bucket. * `arn` - The ARN of the bucket. Will be of format `arn:aws:s3:::bucketname`. * `bucket_domain_name` - The bucket domain name. Will be of format `bucketname.s3.amazonaws.com`. * `bucket_regional_domain_name` - The bucket region-specific domain name. The bucket domain name including the region name, please refer [here](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region) for format. Note: The AWS CloudFront allows specifying S3 region-specific endpoint when creating S3 origin, it will prevent [redirect issues](https://forums.aws.amazon.com/thread.jspa?threadID=216814) from CloudFront to S3 Origin URL. @@ -131,6 +106,11 @@ In addition to all arguments above, the following attributes are exported: * `allowed_origins` - Set of origins customers are able to access the bucket from. * `expose_headers` - Set of headers in the response that customers are able to access from their applications. * `max_age_seconds` The time in seconds that browser can cache the response for a preflight request. +* `grant` - The set of [ACL policy grants](https://docs.aws.amazon.com/AmazonS3/latest/dev/acl-overview.html#sample-acl). + * `id` - Canonical user id of the grantee. + * `type` - Type of grantee. + * `permissions` - List of permissions given to the grantee. + * `uri` - URI of the grantee group. * `hosted_zone_id` - The [Route 53 Hosted Zone ID](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_website_region_endpoints) for this bucket's region. * `lifecycle_rule` - A configuration of [object lifecycle management](http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html). * `id` - Unique identifier for the rule. From 48a6b95bc7bff446eba0481b2f1c56084f80e084 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 4 Feb 2022 14:56:39 -0500 Subject: [PATCH 02/11] Update CHANGELOG for #22537 --- .changelog/22537.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22537.txt diff --git a/.changelog/22537.txt b/.changelog/22537.txt new file mode 100644 index 00000000000..a246fd4f951 --- /dev/null +++ b/.changelog/22537.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +resource/aws_s3_bucket: The `acl` and `grant` arguments have been deprecated and are now read-only. Use the `aws_s3_bucket_acl` resource instead. +``` \ No newline at end of file From 75c9586211b804694fae3703679f42046cc1ebd0 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 4 Feb 2022 15:00:08 -0500 Subject: [PATCH 03/11] update s3 bucket tests --- internal/service/s3/bucket_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 2810c8e909f..e14bfbaec22 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -131,7 +131,7 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, { Config: testAccBucketConfig_withUpdatedTags(bucketName), @@ -217,7 +217,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, { Config: testAccBucketConfig_withTags(bucketName), @@ -322,7 +322,7 @@ func TestAccS3Bucket_Tags_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) From 8a04b2696f7cb55faf8c4c04cd0884052b6028dc Mon Sep 17 00:00:00 2001 From: angie pinilla Date: Fri, 4 Feb 2022 21:26:00 -0500 Subject: [PATCH 04/11] Update website/docs/r/s3_bucket.html.markdown --- website/docs/r/s3_bucket.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 79d7af1504c..edd8fea4d64 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -69,7 +69,7 @@ See the [`aws_s3_bucket_server_side_encryption_configuration` resource](s3_bucke ### Using ACL policy grants -The `acl` and `grant` arguments are read-only as of version 4.0. +The `acl` and `grant` arguments are read-only as of version 4.0 of the Terraform AWS Provider. See the [`aws_s3_bucket_acl` resource](s3_bucket_acl.html.markdown) for configuration details. ## Argument Reference From 61cbb7f127ce5192ce09d75a745951155ff64a8e Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 4 Feb 2022 21:30:45 -0500 Subject: [PATCH 05/11] remove TODO's --- internal/service/s3/bucket_test.go | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index e14bfbaec22..1bfa6ccdd81 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -105,9 +105,6 @@ func TestAccS3Bucket_Basic_emptyString(t *testing.T) { } func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -167,9 +164,6 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { } func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -255,9 +249,6 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { } func TestAccS3Bucket_Tags_ignoreTags(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -303,9 +294,6 @@ func TestAccS3Bucket_Tags_ignoreTags(t *testing.T) { } func TestAccS3Bucket_Tags_basic(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - rInt := sdkacctest.RandInt() resourceName := "aws_s3_bucket.bucket1" @@ -348,7 +336,7 @@ func TestAccS3Bucket_Basic_namePrefix(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "bucket_prefix"}, + ImportStateVerifyIgnore: []string{"force_destroy", "acl", "bucket_prefix"}, }, }, }) @@ -383,9 +371,6 @@ func TestAccS3Bucket_Basic_generatedName(t *testing.T) { // not empty" error in Terraform, to check against regressions. // See https://github.com/hashicorp/terraform/pull/2925 func TestAccS3Bucket_Basic_shouldFailNotFound(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.bucket" @@ -465,9 +450,6 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning(t *testing.T) { } func TestAccS3Bucket_Basic_forceDestroy(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -494,9 +476,6 @@ func TestAccS3Bucket_Basic_forceDestroy(t *testing.T) { // to not contain these extra slashes, out-of-band handling and other AWS // services may create keys with extra slashes (empty "directory" prefixes). func TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") @@ -518,9 +497,6 @@ func TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes(t *testing.T) { } func TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled(t *testing.T) { - // TODO: remove skip once aws_s3_bucket_acl resource is available in the provider - t.Skipf("skipping acceptance testing: aws_s3_bucket 'acl' and 'grant' are read-only, migrate configuration to aws_s3_bucket_acl") - resourceName := "aws_s3_bucket.bucket" bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") From 07139cc762068140198b7c0cec08f1fe99aca09e Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 4 Feb 2022 21:35:26 -0500 Subject: [PATCH 06/11] remove lifecycle.ignore_changes --- internal/service/s3/bucket_test.go | 74 +----------------------------- 1 file changed, 1 insertion(+), 73 deletions(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 1bfa6ccdd81..eaaceb77191 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -336,7 +336,7 @@ func TestAccS3Bucket_Basic_namePrefix(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl", "bucket_prefix"}, + ImportStateVerifyIgnore: []string{"force_destroy", "bucket_prefix"}, }, }, }) @@ -1060,12 +1060,6 @@ func testAccBucketConfig_withNoTags(bucketName string) string { resource "aws_s3_bucket" "bucket" { bucket = %[1]q force_destroy = false - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1086,12 +1080,6 @@ resource "aws_s3_bucket" "bucket" { Key2 = "BBB" Key3 = "CCC" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1113,12 +1101,6 @@ resource "aws_s3_bucket" "bucket" { Key4 = "DDD" Key5 = "EEE" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1138,12 +1120,6 @@ resource "aws_s3_bucket" "bucket1" { Name = "tf-test-bucket-1-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test1" { @@ -1159,12 +1135,6 @@ resource "aws_s3_bucket" "bucket2" { Name = "tf-test-bucket-2-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test2" { @@ -1180,12 +1150,6 @@ resource "aws_s3_bucket" "bucket3" { Name = "tf-test-bucket-3-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test3" { @@ -1201,12 +1165,6 @@ resource "aws_s3_bucket" "bucket4" { Name = "tf-test-bucket-4-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test4" { @@ -1222,12 +1180,6 @@ resource "aws_s3_bucket" "bucket5" { Name = "tf-test-bucket-5-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test5" { @@ -1243,12 +1195,6 @@ resource "aws_s3_bucket" "bucket6" { Name = "tf-test-bucket-6-%[1]d" Environment = "%[1]d" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test6" { @@ -1262,12 +1208,6 @@ func testAccBucketDestroyedConfig(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "bucket" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1315,12 +1255,6 @@ func testAccBucketConfig_forceDestroy(bucketName string) string { resource "aws_s3_bucket" "bucket" { bucket = "%s" force_destroy = true - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -1339,12 +1273,6 @@ resource "aws_s3_bucket" "bucket" { object_lock_configuration { object_lock_enabled = "Enabled" } - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { From e3ef95de4d2daf166b228892302748d72331e625 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Fri, 4 Feb 2022 21:38:05 -0500 Subject: [PATCH 07/11] remove lifecycle.ignore_changes from s3_bucket_acl tests --- internal/service/s3/bucket_acl_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/internal/service/s3/bucket_acl_test.go b/internal/service/s3/bucket_acl_test.go index 7a706180815..24c8de07562 100644 --- a/internal/service/s3/bucket_acl_test.go +++ b/internal/service/s3/bucket_acl_test.go @@ -435,12 +435,6 @@ func testAccBucketAclBasicConfig(rName, acl string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -456,12 +450,6 @@ data "aws_canonical_user_id" "current" {} resource "aws_s3_bucket" "test" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { @@ -499,12 +487,6 @@ data "aws_partition" "current" {} resource "aws_s3_bucket" "test" { bucket = %[1]q - - lifecycle { - ignore_changes = [ - grant, - ] - } } resource "aws_s3_bucket_acl" "test" { From 965d307f65a378d5384fbe169da6c64803be6894 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Mon, 7 Feb 2022 02:53:05 -0500 Subject: [PATCH 08/11] add instructions for breaking change introduced in #22537 --- website/docs/guides/version-4-upgrade.html.md | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/website/docs/guides/version-4-upgrade.html.md b/website/docs/guides/version-4-upgrade.html.md index a4c0fe02744..e7bdba9cb89 100644 --- a/website/docs/guides/version-4-upgrade.html.md +++ b/website/docs/guides/version-4-upgrade.html.md @@ -743,6 +743,60 @@ The resources that were imported are shown above. These resources are now in your Terraform state and will henceforth be managed by Terraform. ``` +### `acl` Argument deprecation + +Switch your Terraform configuration to the `aws_s3_bucket_acl` resource instead. + +For example, given this previous configuration: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... + acl = "private" +} +``` + +It will receive the following error after upgrading: + +``` +│ Error: Value for unconfigurable attribute +│ +│ with aws_s3_bucket.example, +│ on main.tf line 1, in resource "aws_s3_bucket" "example": +│ 1: resource "aws_s3_bucket" "example" { +│ +│ Can't configure a value for "acl": its value will be decided automatically based on the result of applying this configuration. +``` + +Since the `acl` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_acl` +resource and remove any reference to `acl` in the `aws_s3_bucket` resource: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} + +resource "aws_s3_bucket_acl" "example" { + bucket = aws_s3_bucket.example.id + acl = "private" +} +``` + +It is then recommended running `terraform import` on each new resource to prevent data loss, e.g. + +```shell +$ terraform import aws_s3_bucket_acl.example example,private +aws_s3_bucket_acl.example: Importing from ID "example,private"... +aws_s3_bucket_acl.example: Import prepared! + Prepared aws_s3_bucket_acl for import +aws_s3_bucket_acl.example: Refreshing state... [id=example,private] + +Import successful! + +The resources that were imported are shown above. These resources are now in +your Terraform state and will henceforth be managed by Terraform. +``` + ### `cors_rule` Argument deprecation Switch your Terraform configuration to the `aws_s3_bucket_cors_configuration` resource instead. @@ -810,6 +864,98 @@ The resources that were imported are shown above. These resources are now in your Terraform state and will henceforth be managed by Terraform. ``` +### `grant` Argument deprecation + +Switch your Terraform configuration to the `aws_s3_bucket_acl` resource instead. + +For example, given this previous configuration: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... + grant { + id = data.aws_canonical_user_id.current_user.id + type = "CanonicalUser" + permissions = ["FULL_CONTROL"] + } + grant { + type = "Group" + permissions = ["READ_ACP", "WRITE"] + uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" + } +} +``` + +It will receive the following error after upgrading: + +``` +│ Error: Value for unconfigurable attribute +│ +│ with aws_s3_bucket.example, +│ on main.tf line 1, in resource "aws_s3_bucket" "example": +│ 1: resource "aws_s3_bucket" "example" { +│ +│ Can't configure a value for "grant": its value will be decided automatically based on the result of applying this configuration. +``` + +Since the `grant` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_acl` +resource and remove any reference to `grant` in the `aws_s3_bucket` resource: + +```terraform +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} + +resource "aws_s3_bucket_acl" "example" { + bucket = aws_s3_bucket.example.id + + access_control_policy { + grant { + grantee { + id = data.aws_canonical_user_id.current_user.id + type = "CanonicalUser" + } + permission = "FULL_CONTROL" + } + + grant { + grantee { + type = "Group" + uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" + } + permission = "READ_ACP" + } + + grant { + grantee { + type = "Group" + uri = "http://acs.amazonaws.com/groups/s3/LogDelivery" + } + permission = "WRITE" + } + + owner { + id = data.aws_canonical_user_id.current_user.id + } + } +} +``` + +It is then recommended running `terraform import` on each new resource to prevent data loss, e.g. + +```shell +$ terraform import aws_s3_bucket_acl.example example +aws_s3_bucket_acl.example: Importing from ID "example"... +aws_s3_bucket_acl.example: Import prepared! + Prepared aws_s3_bucket_acl for import +aws_s3_bucket_acl.example: Refreshing state... [id=example] + +Import successful! + +The resources that were imported are shown above. These resources are now in +your Terraform state and will henceforth be managed by Terraform. +``` + ### `lifecycle_rule` Argument deprecation Switch your Terraform configuration to the `aws_s3_bucket_lifecycle_configuration` resource instead. From 7ff2f3d3fdc05ce36f29e9c720f2f41f37559540 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Mon, 7 Feb 2022 03:01:25 -0500 Subject: [PATCH 09/11] remove one more 'acl' from import step in test file --- internal/service/s3/bucket_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index eaaceb77191..33898082e0e 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -443,7 +443,7 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + ImportStateVerifyIgnore: []string{"force_destroy"}, }, }, }) From 30f7e807d203b263a21d9abb05aa2438cabbaf74 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 7 Feb 2022 09:13:02 -0500 Subject: [PATCH 10/11] Fix 'testAccBucketConfig_objectLockEnabledWithVersioning' to use 'aws_s3_bucket_acl'. --- internal/service/s3/bucket_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 33898082e0e..85e8d550eb1 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -1232,8 +1232,7 @@ resource "aws_s3_bucket" "test" { func testAccBucketConfig_objectLockEnabledWithVersioning(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { - bucket = "%s" - acl = "private" + bucket = %[1]q force_destroy = true object_lock_configuration { @@ -1241,6 +1240,11 @@ resource "aws_s3_bucket" "test" { } } +resource "aws_s3_bucket_acl" "test" { + bucket = aws_s3_bucket.test.id + acl = "private" +} + resource "aws_s3_bucket_versioning" "test" { bucket = aws_s3_bucket.test.id versioning_configuration { From 07a57b6bd6a3107b72869108beebdabed166495a Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Mon, 7 Feb 2022 17:02:46 -0500 Subject: [PATCH 11/11] add not-found handling after Get calls during read of non-new buckets --- internal/service/s3/bucket.go | 141 ++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index d1f70cb1708..ce489102cd9 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -806,6 +806,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The call to HeadBucket above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketPolicy, the error should be caught for non-new buckets as follows. + 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 && !tfawserr.ErrCodeEquals(err, ErrCodeNoSuchBucketPolicy) { return fmt.Errorf("error getting S3 bucket (%s) policy: %w", d.Id(), err) } @@ -834,6 +843,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketAcl, the error should be caught for non-new buckets as follows. + 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 getting S3 Bucket (%s) ACL: %w", d.Id(), err) } @@ -853,6 +871,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketCors, the error should be caught for non-new buckets as follows. + 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 && !tfawserr.ErrCodeEquals(err, ErrCodeNoSuchCORSConfiguration) { return fmt.Errorf("error getting S3 Bucket CORS configuration: %s", err) } @@ -871,6 +899,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketWebsite, the error should be caught for non-new buckets as follows. + 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 && !tfawserr.ErrCodeEquals(err, ErrCodeNotImplemented, ErrCodeNoSuchWebsiteConfiguration) { return fmt.Errorf("error getting S3 Bucket website configuration: %w", err) } @@ -895,6 +933,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketVersioning, the error should be caught for non-new buckets as follows. + 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 getting S3 Bucket versioning (%s): %w", d.Id(), err) } @@ -913,6 +960,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketAccelerateConfiguration, the error should be caught for non-new buckets as follows. + 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 + } + // Amazon S3 Transfer Acceleration might not be supported in the region if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeMethodNotAllowed, ErrCodeUnsupportedArgument) { return fmt.Errorf("error getting S3 Bucket acceleration configuration: %w", err) @@ -930,6 +986,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketRequestPayment, the error should be caught for non-new buckets as follows. + 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 getting S3 Bucket request payment: %s", err) } @@ -945,6 +1010,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLogging, the error should be caught for non-new buckets as follows. + 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 getting S3 Bucket logging: %s", err) } @@ -964,6 +1038,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLifecycleConfiguration, the error should be caught for non-new buckets as follows. + 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 && !tfawserr.ErrCodeEquals(err, ErrCodeNoSuchLifecycleConfiguration) { return fmt.Errorf("error getting S3 Bucket (%s) Lifecycle Configuration: %w", d.Id(), err) } @@ -984,6 +1068,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketReplication, the error should be caught for non-new buckets as follows. + 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 && !tfawserr.ErrCodeEquals(err, ErrCodeReplicationConfigurationNotFound) { return fmt.Errorf("error getting S3 Bucket replication: %w", err) } @@ -1004,6 +1097,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { Bucket: aws.String(d.Id()), }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketEncryption, the error should be caught for non-new buckets as follows. + 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 && !tfawserr.ErrMessageContains(err, ErrCodeServerSideEncryptionConfigurationNotFound, "encryption configuration was not found") { return fmt.Errorf("error getting S3 Bucket encryption: %w", err) } @@ -1023,6 +1126,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { }) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetObjectLockConfiguration, the error should be caught for non-new buckets as follows. + 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 + } + // Object lock not supported in all partitions (extra guard, also guards in read func) if err != nil && !tfawserr.ErrCodeEquals(err, ErrCodeMethodNotAllowed, ErrCodeObjectLockConfigurationNotFound) { if meta.(*conns.AWSClient).Partition == endpoints.AwsPartitionID || meta.(*conns.AWSClient).Partition == endpoints.AwsUsGovPartitionID { @@ -1058,6 +1170,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { r.Config.Credentials = conn.Config.Credentials }) }) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as s3manager.GetBucketRegionWithClient, the error should be caught for non-new buckets as follows. + 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 getting S3 Bucket location: %s", err) } @@ -1084,6 +1206,16 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { // Add website_endpoint as an attribute websiteEndpoint, err := websiteEndpoint(meta.(*conns.AWSClient), d) + + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketLocation, the error should be caught for non-new buckets as follows. + 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 err } @@ -1101,6 +1233,15 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { return BucketListTags(conn, d.Id()) }) + // The S3 API method calls above can occasionally return no error (i.e. NoSuchBucket) + // after a bucket has been deleted (eventual consistency woes :/), thus, when making extra S3 API calls + // such as GetBucketTagging, the error should be caught for non-new buckets as follows. + 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 listing tags for S3 Bucket (%s): %s", d.Id(), err) }