diff --git a/.changelog/23844.txt b/.changelog/23844.txt new file mode 100644 index 00000000000..b4c96c31a8e --- /dev/null +++ b/.changelog/23844.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_s3_bucket: Update `request_payer` parameter to be configurable. Please refer to the documentation for details on drift detection and potential conflicts when configuring this parameter with the standalone `aws_s3_bucket_request_payment_configuration` resource. +``` \ No newline at end of file diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 6e5c251ee82..6a7d8eafd33 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -399,9 +399,11 @@ func ResourceBucket() *schema.Resource { }, "request_payer": { - Type: schema.TypeString, - Computed: true, - Deprecated: "Use the aws_s3_bucket_request_payment_configuration resource instead", + Type: schema.TypeString, + Optional: true, + Computed: true, + Deprecated: "Use the aws_s3_bucket_request_payment_configuration resource instead", + ValidateFunc: validation.StringInSlice(s3.Payer_Values(), false), }, "replication_configuration": { @@ -850,6 +852,12 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error { } } + if d.HasChange("request_payer") { + if err := resourceBucketInternalRequestPayerUpdate(conn, d); err != nil { + return fmt.Errorf("error updating S3 Bucket (%s) Request Payer: %w", d.Id(), err) + } + } + if d.HasChange("replication_configuration") { if err := resourceBucketInternalReplicationConfigurationUpdate(conn, d); err != nil { return fmt.Errorf("error updating S3 Bucket (%s) Replication configuration: %w", d.Id(), err) @@ -1978,6 +1986,23 @@ func resourceBucketInternalReplicationConfigurationUpdate(conn *s3.S3, d *schema return err } +func resourceBucketInternalRequestPayerUpdate(conn *s3.S3, d *schema.ResourceData) error { + payer := d.Get("request_payer").(string) + + input := &s3.PutBucketRequestPaymentInput{ + Bucket: aws.String(d.Id()), + RequestPaymentConfiguration: &s3.RequestPaymentConfiguration{ + Payer: aws.String(payer), + }, + } + + _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { + return conn.PutBucketRequestPayment(input) + }) + + return err +} + func resourceBucketInternalServerSideEncryptionConfigurationUpdate(conn *s3.S3, d *schema.ResourceData) error { serverSideEncryptionConfiguration := d.Get("server_side_encryption_configuration").([]interface{}) diff --git a/internal/service/s3/bucket_request_payment_configuration_test.go b/internal/service/s3/bucket_request_payment_configuration_test.go index ee96edb9322..ef4c2472696 100644 --- a/internal/service/s3/bucket_request_payment_configuration_test.go +++ b/internal/service/s3/bucket_request_payment_configuration_test.go @@ -106,6 +106,64 @@ func TestAccS3BucketRequestPaymentConfiguration_update(t *testing.T) { }) } +func TestAccS3BucketRequestPaymentConfiguration_migrate_noChange(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + bucketResourceName := "aws_s3_bucket.test" + resourceName := "aws_s3_bucket_request_payment_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketRequestPaymentConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketConfig_withRequestPayer(rName, s3.PayerRequester), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExists(bucketResourceName), + resource.TestCheckResourceAttr(bucketResourceName, "request_payer", s3.PayerRequester), + ), + }, + { + Config: testAccBucketRequestPaymentConfigurationBasicConfig(rName, s3.PayerRequester), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketRequestPaymentConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "payer", s3.PayerRequester), + ), + }, + }, + }) +} + +func TestAccS3BucketRequestPaymentConfiguration_migrate_withChange(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + bucketResourceName := "aws_s3_bucket.test" + resourceName := "aws_s3_bucket_request_payment_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckBucketRequestPaymentConfigurationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketConfig_withRequestPayer(rName, s3.PayerRequester), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExists(bucketResourceName), + resource.TestCheckResourceAttr(bucketResourceName, "request_payer", s3.PayerRequester), + ), + }, + { + Config: testAccBucketRequestPaymentConfigurationBasicConfig(rName, s3.PayerBucketOwner), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketRequestPaymentConfigurationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "payer", s3.PayerBucketOwner), + ), + }, + }, + }) +} + func testAccCheckBucketRequestPaymentConfigurationDestroy(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index ece924edfa3..540eee11436 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -298,6 +298,40 @@ func TestAccS3Bucket_Basic_keyEnabled(t *testing.T) { }) } +func TestAccS3Bucket_Basic_requestPayer(t *testing.T) { + bucketName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_bucket.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckBucketDestroy, + Steps: []resource.TestStep{ + { + Config: testAccBucketConfig_withRequestPayer(bucketName, s3.PayerBucketOwner), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "request_payer", s3.PayerBucketOwner), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, + }, + { + Config: testAccBucketConfig_withRequestPayer(bucketName, s3.PayerRequester), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "request_payer", s3.PayerRequester), + ), + }, + }, + }) +} + // Test TestAccS3Bucket_disappears is designed to fail with a "plan // not empty" error in Terraform, to check against regressions. // See https://github.com/hashicorp/terraform/pull/2925 @@ -4170,6 +4204,15 @@ resource "aws_s3_bucket" "source" { `, bucketName)) } +func testAccBucketConfig_withRequestPayer(bucketName, requestPayer string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + request_payer = %[2]q +} +`, bucketName, requestPayer) +} + func testAccBucketConfig_withVersioning(bucketName string, enabled bool) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 1e9afcec8ff..0ce46c35bff 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -44,6 +44,10 @@ Configuring with both will cause inconsistencies and may overwrite configuration or with the deprecated parameter `replication_configuration` in the resource `aws_s3_bucket`. Configuring with both will cause inconsistencies and may overwrite configuration. +~> **NOTE on S3 Bucket Request Payment Configuration:** S3 Bucket Request Payment can be configured in either the standalone resource [`aws_s3_bucket_request_payment_configuration`](s3_bucket_request_payment_configuration.html) +or with the deprecated parameter `request_payer` in the resource `aws_s3_bucket`. +Configuring with both will cause inconsistencies and may overwrite configuration. + ~> **NOTE on S3 Bucket Server Side Encryption Configuration:** S3 Bucket Server Side Encryption can be configured in either the standalone resource [`aws_s3_bucket_server_side_encryption_configuration`](s3_bucket_server_side_encryption_configuration.html) or with the deprecated parameter `server_side_encryption_configuration` in the resource `aws_s3_bucket`. Configuring with both will cause inconsistencies and may overwrite configuration. @@ -447,6 +451,11 @@ The following arguments are supported: Use the resource [`aws_s3_bucket_policy`](s3_bucket_policy.html) instead. * `replication_configuration` - (Optional, **Deprecated**) A configuration of [replication configuration](http://docs.aws.amazon.com/AmazonS3/latest/dev/crr.html). See [Replication Configuration](#replication-configuration) below for details. Terraform will only perform drift detection if a configuration value is provided. Use the resource [`aws_s3_bucket_replication_configuration`](s3_bucket_replication_configuration.html) instead. +* `request_payer` - (Optional, **Deprecated**) Specifies who should bear the cost of Amazon S3 data transfer. + Can be either `BucketOwner` or `Requester`. By default, the owner of the S3 bucket would incur the costs of any data transfer. + See [Requester Pays Buckets](http://docs.aws.amazon.com/AmazonS3/latest/dev/RequesterPaysBuckets.html) developer guide for more information. + Terraform will only perform drift detection if a configuration value is provided. + Use the resource [`aws_s3_bucket_request_payment_configuration`](s3_bucket_request_payment_configuration.html) instead. * `server_side_encryption_configuration` - (Optional, **Deprecated**) A configuration of [server-side encryption configuration](http://docs.aws.amazon.com/AmazonS3/latest/dev/bucket-encryption.html). See [Server Side Encryption Configuration](#server-side-encryption-configuration) below for details. Terraform will only perform drift detection if a configuration value is provided. Use the resource [`aws_s3_bucket_server_side_encryption_configuration`](s3_bucket_server_side_encryption_configuration.html) instead. @@ -676,7 +685,6 @@ In addition to all arguments above, the following attributes are exported: * `days` - The number of days specified for the default retention period. * `years` - The number of years specified for the default retention period. * `region` - The AWS region this bucket resides in. -* `request_payer` - Either `BucketOwner` or `Requester` that pays for the download and request fees. * `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). * `website_endpoint` - The website endpoint, if the bucket is configured with a website. If not, this will be an empty string. * `website_domain` - The domain of the website endpoint, if the bucket is configured with a website. If not, this will be an empty string. This is used to create Route 53 alias records.