From 7182bd1e59025cab71f85e23f3420de8a155c441 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 9 Jul 2020 19:27:52 -0400 Subject: [PATCH] resource/aws_s3_bucket: Convert region to read-only attribute Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/592 Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/1656 Output from acceptance testing (NOTE: CUR data source and resource need to be tested in standalone account due to Organization permissions and appear to be failing due to new validation in the API that's not handled in the resource yet): ``` --- PASS: TestAccAWSS3Bucket_acceleration (65.86s) --- PASS: TestAccAWSS3Bucket_AclToGrant (67.94s) --- PASS: TestAccAWSS3Bucket_basic (37.25s) --- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (35.95s) --- PASS: TestAccAWSS3Bucket_Cors_Delete (31.78s) --- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (37.29s) --- PASS: TestAccAWSS3Bucket_Cors_Update (65.22s) --- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (62.31s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (37.28s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (43.14s) --- PASS: TestAccAWSS3Bucket_forceDestroy (31.61s) --- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (31.54s) --- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (37.95s) --- PASS: TestAccAWSS3Bucket_generatedName (35.53s) --- PASS: TestAccAWSS3Bucket_GrantToAcl (57.50s) --- PASS: TestAccAWSS3Bucket_LifecycleBasic (86.93s) --- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (62.03s) --- PASS: TestAccAWSS3Bucket_LifecycleRule_Expiration_EmptyConfigurationBlock (31.01s) --- PASS: TestAccAWSS3Bucket_Logging (55.35s) --- PASS: TestAccAWSS3Bucket_namePrefix (35.81s) --- PASS: TestAccAWSS3Bucket_objectLock (60.93s) --- PASS: TestAccAWSS3Bucket_Policy (88.67s) --- PASS: TestAccAWSS3Bucket_Replication (147.39s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (86.62s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AddAccessControlTranslation (84.62s) --- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (28.14s) --- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (152.22s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (52.74s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (51.40s) --- PASS: TestAccAWSS3Bucket_RequestPayer (63.26s) --- PASS: TestAccAWSS3Bucket_shouldFailNotFound (15.41s) --- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (118.49s) --- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (163.94s) --- PASS: TestAccAWSS3Bucket_UpdateAcl (58.70s) --- PASS: TestAccAWSS3Bucket_UpdateGrant (91.75s) --- PASS: TestAccAWSS3Bucket_Versioning (90.14s) --- PASS: TestAccAWSS3Bucket_Website_Simple (89.22s) --- PASS: TestAccAWSS3Bucket_WebsiteRedirect (86.48s) --- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (63.97s) --- PASS: TestAccAWSSsmResourceDataSync_basic (15.77s) --- PASS: TestAccAWSSsmResourceDataSync_update (28.49s) TestAccAwsCurReportDefinition_basic: testing.go:684: Step 0 error: errors during apply: Error: Error creating AWS Cost And Usage Report Definition: ValidationException: Failed to verify customer bucket permission. accountId= --OMITTED--, bucket name: tf-test-bucket-3532084976228094739, bucket region: us-east-1 TestAccDataSourceAwsCurReportDefinition_basic: testing.go:684: Step 0 error: errors during apply: Error: Error creating AWS Cost And Usage Report Definition: ValidationException: Failed to verify customer bucket permission. accountId= --OMITTED--, bucket name: tf-test-bucket-9147728765044904331, bucket region: us-east-1 ``` --- ...a_source_aws_cur_report_definition_test.go | 8 ++--- ...resource_aws_cur_report_definition_test.go | 12 +++---- aws/resource_aws_s3_bucket.go | 8 +---- aws/resource_aws_s3_bucket_test.go | 31 ------------------- ...esource_aws_ssm_resource_data_sync_test.go | 2 -- website/docs/guides/version-3-upgrade.html.md | 22 +++++++++++++ website/docs/r/s3_bucket.html.markdown | 3 -- .../r/ssm_resource_data_sync.html.markdown | 1 - 8 files changed, 31 insertions(+), 56 deletions(-) diff --git a/aws/data_source_aws_cur_report_definition_test.go b/aws/data_source_aws_cur_report_definition_test.go index a266f3b13982..69b90fe92c1c 100644 --- a/aws/data_source_aws_cur_report_definition_test.go +++ b/aws/data_source_aws_cur_report_definition_test.go @@ -20,7 +20,6 @@ func TestAccDataSourceAwsCurReportDefinition_basic(t *testing.T) { reportName := acctest.RandomWithPrefix("tf_acc_test") bucketName := fmt.Sprintf("tf-test-bucket-%d", acctest.RandInt()) - bucketRegion := "us-east-1" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSCur(t) }, @@ -28,7 +27,7 @@ func TestAccDataSourceAwsCurReportDefinition_basic(t *testing.T) { CheckDestroy: testAccCheckAwsCurReportDefinitionDestroy, Steps: []resource.TestStep{ { - Config: testAccDataSourceAwsCurReportDefinitionConfig_basic(reportName, bucketName, bucketRegion), + Config: testAccDataSourceAwsCurReportDefinitionConfig_basic(reportName, bucketName), Check: resource.ComposeTestCheckFunc( testAccDataSourceAwsCurReportDefinitionCheckExists(datasourceName, resourceName), resource.TestCheckResourceAttrPair(datasourceName, "report_name", resourceName, "report_name"), @@ -60,7 +59,7 @@ func testAccDataSourceAwsCurReportDefinitionCheckExists(datasourceName, resource } // note: cur report definitions are currently only supported in us-east-1 -func testAccDataSourceAwsCurReportDefinitionConfig_basic(reportName string, bucketName string, bucketRegion string) string { +func testAccDataSourceAwsCurReportDefinitionConfig_basic(reportName string, bucketName string) string { return fmt.Sprintf(` provider "aws" { region = "us-east-1" @@ -72,7 +71,6 @@ resource "aws_s3_bucket" "test" { bucket = "%[2]s" acl = "private" force_destroy = true - region = "%[3]s" } resource "aws_s3_bucket_policy" "test" { @@ -124,5 +122,5 @@ resource "aws_cur_report_definition" "test" { data "aws_cur_report_definition" "test" { report_name = "${aws_cur_report_definition.test.report_name}" } -`, reportName, bucketName, bucketRegion) +`, reportName, bucketName) } diff --git a/aws/resource_aws_cur_report_definition_test.go b/aws/resource_aws_cur_report_definition_test.go index 4446b148db2e..1060dd897f88 100644 --- a/aws/resource_aws_cur_report_definition_test.go +++ b/aws/resource_aws_cur_report_definition_test.go @@ -18,10 +18,9 @@ func TestAccAwsCurReportDefinition_basic(t *testing.T) { defer os.Setenv("AWS_DEFAULT_REGION", oldvar) resourceName := "aws_cur_report_definition.test" - + s3BucketResourceName := "aws_s3_bucket.test" reportName := acctest.RandomWithPrefix("tf_acc_test") bucketName := fmt.Sprintf("tf-test-bucket-%d", acctest.RandInt()) - bucketRegion := "us-east-1" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSCur(t) }, @@ -29,7 +28,7 @@ func TestAccAwsCurReportDefinition_basic(t *testing.T) { CheckDestroy: testAccCheckAwsCurReportDefinitionDestroy, Steps: []resource.TestStep{ { - Config: testAccAwsCurReportDefinitionConfig_basic(reportName, bucketName, bucketRegion), + Config: testAccAwsCurReportDefinitionConfig_basic(reportName, bucketName), Check: resource.ComposeTestCheckFunc( testAccCheckAwsCurReportDefinitionExists(resourceName), resource.TestCheckResourceAttr(resourceName, "report_name", reportName), @@ -38,7 +37,7 @@ func TestAccAwsCurReportDefinition_basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "additional_schema_elements.#", "1"), resource.TestCheckResourceAttr(resourceName, "s3_bucket", bucketName), resource.TestCheckResourceAttr(resourceName, "s3_prefix", ""), - resource.TestCheckResourceAttr(resourceName, "s3_region", bucketRegion), + resource.TestCheckResourceAttrPair(resourceName, "s3_region", s3BucketResourceName, "region"), resource.TestCheckResourceAttr(resourceName, "additional_artifacts.#", "2"), ), }, @@ -105,7 +104,7 @@ func testAccPreCheckAWSCur(t *testing.T) { } // note: cur report definitions are currently only supported in us-east-1 -func testAccAwsCurReportDefinitionConfig_basic(reportName string, bucketName string, bucketRegion string) string { +func testAccAwsCurReportDefinitionConfig_basic(reportName string, bucketName string) string { return fmt.Sprintf(` provider "aws" { region = "us-east-1" @@ -115,7 +114,6 @@ resource "aws_s3_bucket" "test" { bucket = "%[2]s" acl = "private" force_destroy = true - region = "%[3]s" } resource "aws_s3_bucket_policy" "test" { @@ -163,5 +161,5 @@ resource "aws_cur_report_definition" "test" { s3_region = "${aws_s3_bucket.test.region}" additional_artifacts = ["REDSHIFT", "QUICKSIGHT"] } -`, reportName, bucketName, bucketRegion) +`, reportName, bucketName) } diff --git a/aws/resource_aws_s3_bucket.go b/aws/resource_aws_s3_bucket.go index 472825a664b5..f61eb84b1168 100644 --- a/aws/resource_aws_s3_bucket.go +++ b/aws/resource_aws_s3_bucket.go @@ -205,7 +205,6 @@ func resourceAwsS3Bucket() *schema.Resource { "region": { Type: schema.TypeString, - Optional: true, Computed: true, }, "website_endpoint": { @@ -666,12 +665,7 @@ func resourceAwsS3BucketCreate(d *schema.ResourceData, meta interface{}) error { log.Printf("[DEBUG] S3 bucket %s has canned ACL %s", bucket, acl) } - var awsRegion string - if region, ok := d.GetOk("region"); ok { - awsRegion = region.(string) - } else { - awsRegion = meta.(*AWSClient).region - } + awsRegion := meta.(*AWSClient).region log.Printf("[DEBUG] S3 bucket create: %s, using region: %s", bucket, awsRegion) // Special case us-east-1 region and do not set the LocationConstraint. diff --git a/aws/resource_aws_s3_bucket_test.go b/aws/resource_aws_s3_bucket_test.go index 824ebe096df8..922785824ca5 100644 --- a/aws/resource_aws_s3_bucket_test.go +++ b/aws/resource_aws_s3_bucket_test.go @@ -435,26 +435,6 @@ func TestAccAWSS3Bucket_generatedName(t *testing.T) { }) } -func TestAccAWSS3Bucket_region(t *testing.T) { - resourceName := "aws_s3_bucket.test" - rName := acctest.RandomWithPrefix("tf-acc-test") - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSS3BucketDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAWSS3BucketConfigWithRegion(rName), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSS3BucketExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "region", testAccGetRegion()), - ), - }, - }, - }) -} - func TestAccAWSS3Bucket_acceleration(t *testing.T) { bucketName := acctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.bucket" @@ -3071,17 +3051,6 @@ resource "aws_s3_bucket" "bucket6" { `, randInt) } -func testAccAWSS3BucketConfigWithRegion(bucketName string) string { - return fmt.Sprintf(` -data "aws_region" "current" {} - -resource "aws_s3_bucket" "test" { - bucket = %[1]q - region = data.aws_region.current.name -} -`, bucketName) -} - func testAccAWSS3BucketWebsiteConfig(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "bucket" { diff --git a/aws/resource_aws_ssm_resource_data_sync_test.go b/aws/resource_aws_ssm_resource_data_sync_test.go index 834fb2890f78..2e831e72a97e 100644 --- a/aws/resource_aws_ssm_resource_data_sync_test.go +++ b/aws/resource_aws_ssm_resource_data_sync_test.go @@ -96,7 +96,6 @@ func testAccSsmResourceDataSyncConfig(rInt int, rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "hoge" { bucket = "tf-test-bucket-%d" - region = "us-west-2" force_destroy = true } @@ -150,7 +149,6 @@ func testAccSsmResourceDataSyncConfigUpdate(rInt int, rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "hoge" { bucket = "tf-test-bucket-%d" - region = "us-west-2" force_destroy = true } diff --git a/website/docs/guides/version-3-upgrade.html.md b/website/docs/guides/version-3-upgrade.html.md index 13cc975647d9..99c3bb87349d 100644 --- a/website/docs/guides/version-3-upgrade.html.md +++ b/website/docs/guides/version-3-upgrade.html.md @@ -433,6 +433,28 @@ resource "aws_msk_cluster" "example" { Previously when importing the `aws_s3_bucket` resource with the [`terraform import` command](/docs/commands/import.html), the Terraform AWS Provider would automatically attempt to import an associated `aws_s3_bucket_policy` resource as well. This automatic resource import has been removed. Use the [`aws_s3_bucket_policy` resource import](/docs/providers/aws/r/s3_bucket_policy.html#import) to import that resource separately. +### region Attribute Is Now Read-Only + +The `region` attribute is no longer configurable, but it remains as a read-only attribute. The region of the `aws_s3_bucket` resource is determined by the region of the Terraform AWS Provider, similar to all other resources. + +For example, given this previous configuration: + +```hcl +resource "aws_s3_bucket" "example" { + # ... other configuration ... + + region = "us-west-2" +} +``` + +An updated configuration: + +```hcl +resource "aws_s3_bucket" "example" { + # ... other configuration ... +} +``` + ## Resource: aws_sns_platform_application ### platform_credential and platform_principal Arguments No Longer Stored as SHA256 Hash diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index f00019acbebf..71d2fd1fadf7 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -253,7 +253,6 @@ resource "aws_iam_role_policy_attachment" "replication" { resource "aws_s3_bucket" "destination" { bucket = "tf-test-bucket-destination-12345" - region = "eu-west-1" versioning { enabled = true @@ -264,7 +263,6 @@ resource "aws_s3_bucket" "bucket" { provider = "aws.central" bucket = "tf-test-bucket-12345" acl = "private" - region = "eu-central-1" versioning { enabled = true @@ -349,7 +347,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`. -* `region` - (Optional) If specified, the AWS region this bucket should reside in. Otherwise, the region used by the callee. * `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) diff --git a/website/docs/r/ssm_resource_data_sync.html.markdown b/website/docs/r/ssm_resource_data_sync.html.markdown index 1d0dba8da65f..a52defed228c 100644 --- a/website/docs/r/ssm_resource_data_sync.html.markdown +++ b/website/docs/r/ssm_resource_data_sync.html.markdown @@ -15,7 +15,6 @@ Provides a SSM resource data sync. ```hcl resource "aws_s3_bucket" "hoge" { bucket = "tf-test-bucket-1234" - region = "us-east-1" } resource "aws_s3_bucket_policy" "hoge" {