r/s3_bucket: read-only request_payer argument

.changelog/22613.txt

@@ -0,0 +1,3 @@
+```release-note:breaking-change
+resource/aws_s3_bucket: The `request_payer` argument has been deprecated and is now read-only. Use the `aws_s3_bucket_request_payment_configuration` resource instead.
+```

internal/service/s3/bucket.go

@@ -382,10 +382,9 @@ func ResourceBucket() *schema.Resource {
 			},
 
 			"request_payer": {
-				Type:         schema.TypeString,
-				Optional:     true,
-				Computed:     true,
-				ValidateFunc: validation.StringInSlice(s3.Payer_Values(), false),
+				Type:       schema.TypeString,
+				Computed:   true,
+				Deprecated: "Use the aws_s3_bucket_request_payment_configuration resource instead",
 			},
 
 			"replication_configuration": {
@@ -811,12 +810,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
-	if d.HasChange("request_payer") {
-		if err := resourceBucketRequestPayerUpdate(conn, d); err != nil {
-			return err
-		}
-	}
-
 	if d.HasChange("replication_configuration") {
 		if err := resourceBucketInternalReplicationConfigurationUpdate(conn, d); err != nil {
 			return err
@@ -1758,28 +1751,6 @@ func resourceBucketAccelerationUpdate(conn *s3.S3, d *schema.ResourceData) error
 	return nil
 }
 
-func resourceBucketRequestPayerUpdate(conn *s3.S3, d *schema.ResourceData) error {
-	bucket := d.Get("bucket").(string)
-	payer := d.Get("request_payer").(string)
-
-	i := &s3.PutBucketRequestPaymentInput{
-		Bucket: aws.String(bucket),
-		RequestPaymentConfiguration: &s3.RequestPaymentConfiguration{
-			Payer: aws.String(payer),
-		},
-	}
-	log.Printf("[DEBUG] S3 put bucket request payer: %#v", i)
-
-	_, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) {
-		return conn.PutBucketRequestPayment(i)
-	})
-	if err != nil {
-		return fmt.Errorf("Error putting S3 request payer: %s", err)
-	}
-
-	return nil
-}
-
 func resourceBucketInternalServerSideEncryptionConfigurationUpdate(conn *s3.S3, d *schema.ResourceData) error {
 	bucket := d.Get("bucket").(string)
 	serverSideEncryptionConfiguration := d.Get("server_side_encryption_configuration").([]interface{})

internal/service/s3/bucket_request_payment_configuration_test.go

@@ -189,12 +189,6 @@ func testAccBucketRequestPaymentConfigurationBasicConfig(rName, payer string) st
 	return fmt.Sprintf(`
 resource "aws_s3_bucket" "test" {
   bucket = %[1]q
-
-  lifecycle {
-    ignore_changes = [
-      request_payer
-    ]
-  }
 }
 
 resource "aws_s3_bucket_request_payment_configuration" "test" {

internal/service/s3/bucket_test.go

@@ -409,42 +409,6 @@ func TestAccS3Bucket_Basic_acceleration(t *testing.T) {
 	})
 }
 
-func TestAccS3Bucket_Basic_requestPayer(t *testing.T) {
-	bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket")
-	resourceName := "aws_s3_bucket.bucket"
-
-	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: testAccBucketRequestPayerBucketOwnerConfig(bucketName),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckBucketExists(resourceName),
-					resource.TestCheckResourceAttr(resourceName, "request_payer", "BucketOwner"),
-					testAccCheckRequestPayer(resourceName, "BucketOwner"),
-				),
-			},
-			{
-				ResourceName:            resourceName,
-				ImportState:             true,
-				ImportStateVerify:       true,
-				ImportStateVerifyIgnore: []string{"force_destroy", "acl"},
-			},
-			{
-				Config: testAccBucketRequestPayerRequesterConfig(bucketName),
-				Check: resource.ComposeTestCheckFunc(
-					testAccCheckBucketExists(resourceName),
-					resource.TestCheckResourceAttr(resourceName, "request_payer", "Requester"),
-					testAccCheckRequestPayer(resourceName, "Requester"),
-				),
-			},
-		},
-	})
-}
-
 func TestAccS3Bucket_Security_policy(t *testing.T) {
 	bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket")
 	partition := acctest.Partition()
@@ -2887,28 +2851,6 @@ func testAccCheckBucketCors(n string, corsRules []*s3.CORSRule) resource.TestChe
 	}
 }
 
-func testAccCheckRequestPayer(n, expectedPayer string) resource.TestCheckFunc {
-	return func(s *terraform.State) error {
-		rs := s.RootModule().Resources[n]
-		conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn
-
-		out, err := conn.GetBucketRequestPayment(&s3.GetBucketRequestPaymentInput{
-			Bucket: aws.String(rs.Primary.ID),
-		})
-
-		if err != nil {
-			return fmt.Errorf("GetBucketRequestPayment error: %v", err)
-		}
-
-		if *out.Payer != expectedPayer {
-			return fmt.Errorf("bad error request payer type, expected: %v, got %v",
-				expectedPayer, out.Payer)
-		}
-
-		return nil
-	}
-}
-
 func testAccCheckBucketLogging(n, b, p string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs := s.RootModule().Resources[n]
@@ -3305,24 +3247,6 @@ resource "aws_s3_bucket" "bucket" {
 `, bucketName)
 }
 
-func testAccBucketRequestPayerBucketOwnerConfig(bucketName string) string {
-	return fmt.Sprintf(`
-resource "aws_s3_bucket" "bucket" {
-  bucket        = %[1]q
-  request_payer = "BucketOwner"
-}
-`, bucketName)
-}
-
-func testAccBucketRequestPayerRequesterConfig(bucketName string) string {
-	return fmt.Sprintf(`
-resource "aws_s3_bucket" "bucket" {
-  bucket        = %[1]q
-  request_payer = "Requester"
-}
-`, bucketName)
-}
-
 func testAccBucketWithPolicyConfig(bucketName, partition string) string {
 	return fmt.Sprintf(`
 resource "aws_s3_bucket" "bucket" {

website/docs/guides/version-4-upgrade.html.md

@@ -689,6 +689,60 @@ To help distribute the management of S3 bucket settings via independent resource
 have become read-only. Configurations dependent on these arguments should be updated to use the corresponding `aws_s3_bucket_*` resource.
 Once updated, new `aws_s3_bucket_*` resources should be imported into Terraform state.
 
+### `request_payer` Argument deprecation
+
+Switch your Terraform configuration to the `aws_s3_bucket_request_payment_configuration` resource instead.
+
+For example, given this previous configuration:
+
+```terraform
+resource "aws_s3_bucket" "example" {
+  # ... other configuration ...
+  request_payer = "Requester"
+}
+```
+
+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 "request_payer": its value will be decided automatically based on the result of applying this configuration.
+```
+
+Since the `request_payer` argument changed to read-only, the recommendation is to update the configuration to use the `aws_s3_bucket_request_payment_configuration`
+resource and remove any reference to `request_payer` in the `aws_s3_bucket` resource:
+
+```terraform
+resource "aws_s3_bucket" "example" {
+  # ... other configuration ...
+}
+
+resource "aws_s3_bucket_request_payment_configuration" "example" {
+  bucket = aws_s3_bucket.example.id
+  payer  = "Requester"
+}
+```
+
+It is then recommended running `terraform import` on each new resource to prevent data loss, e.g.
+
+```shell
+$ terraform import aws_s3_bucket_request_payment_configuration.example example
+aws_s3_bucket_request_payment_configuration.example: Importing from ID "example"...
+aws_s3_bucket_request_payment_configuration.example: Import prepared!
+  Prepared aws_s3_bucket_request_payment_configuration for import
+aws_s3_bucket_request_payment_configuration.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.
+```
+
 ### `website`, `website_domain`, and `website_endpoint` Argument deprecation
 
 Switch your Terraform configuration to the `aws_s3_bucket_website_configuration` resource instead.

website/docs/r/s3_bucket.html.markdown

@@ -376,7 +376,6 @@ The following arguments are supported:
 * `logging` - (Optional) A settings of [bucket logging](https://docs.aws.amazon.com/AmazonS3/latest/UG/ManagingBucketLogging.html) (documented below).
 * `lifecycle_rule` - (Optional) A configuration of [object lifecycle management](http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html) (documented below).
 * `acceleration_status` - (Optional) Sets the accelerate configuration of an existing bucket. Can be `Enabled` or `Suspended`.
-* `request_payer` - (Optional) 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.
@@ -563,6 +562,7 @@ In addition to all arguments above, the following attributes are exported:
 * `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.
 * `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.
 * `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` - The website configuration, if configured.
     * `error_document` - The name of the error document for the website.