Skip to content

Commit

Permalink
resource/aws_s3_bucket: Convert region to read-only attribute (#14127)
Browse files Browse the repository at this point in the history
Reference: #592
Reference: #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
```
  • Loading branch information
bflad authored Jul 15, 2020
1 parent 76adb80 commit 24d0629
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 56 deletions.
8 changes: 3 additions & 5 deletions aws/data_source_aws_cur_report_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@ 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) },
Providers: testAccProviders,
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"),
Expand Down Expand Up @@ -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"
Expand All @@ -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" {
Expand Down Expand Up @@ -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)
}
12 changes: 5 additions & 7 deletions aws/resource_aws_cur_report_definition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,17 @@ 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) },
Providers: testAccProviders,
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),
Expand All @@ -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"),
),
},
Expand Down Expand Up @@ -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"
Expand All @@ -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" {
Expand Down Expand Up @@ -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)
}
8 changes: 1 addition & 7 deletions aws/resource_aws_s3_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ func resourceAwsS3Bucket() *schema.Resource {

"region": {
Type: schema.TypeString,
Optional: true,
Computed: true,
},
"website_endpoint": {
Expand Down Expand Up @@ -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.
Expand Down
31 changes: 0 additions & 31 deletions aws/resource_aws_s3_bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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" {
Expand Down
2 changes: 0 additions & 2 deletions aws/resource_aws_ssm_resource_data_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
22 changes: 22 additions & 0 deletions website/docs/guides/version-3-upgrade.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions website/docs/r/s3_bucket.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion website/docs/r/ssm_resource_data_sync.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down

0 comments on commit 24d0629

Please sign in to comment.