r/s3_bucket: make request_payer configurable

.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.
+```

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{})
 

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
 

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" {

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.