Add aws_s3_access_point resource #11276

ewbankkit · 2019-12-12T23:24:41Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

**New Resource:** `aws_s3_access_point`

Output from acceptance testing:

aws_s3_access_point resource

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSS3AccessPoint_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSS3AccessPoint_ -timeout 120m
=== RUN   TestAccAWSS3AccessPoint_basic
=== PAUSE TestAccAWSS3AccessPoint_basic
=== RUN   TestAccAWSS3AccessPoint_disappears
=== PAUSE TestAccAWSS3AccessPoint_disappears
=== RUN   TestAccAWSS3AccessPoint_bucketDisappears
=== PAUSE TestAccAWSS3AccessPoint_bucketDisappears
=== RUN   TestAccAWSS3AccessPoint_Policy
=== PAUSE TestAccAWSS3AccessPoint_Policy
=== RUN   TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration
=== PAUSE TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration
=== RUN   TestAccAWSS3AccessPoint_VpcConfiguration
=== PAUSE TestAccAWSS3AccessPoint_VpcConfiguration
=== CONT  TestAccAWSS3AccessPoint_basic
=== CONT  TestAccAWSS3AccessPoint_Policy
=== CONT  TestAccAWSS3AccessPoint_disappears
=== CONT  TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration
=== CONT  TestAccAWSS3AccessPoint_VpcConfiguration
=== CONT  TestAccAWSS3AccessPoint_bucketDisappears
--- PASS: TestAccAWSS3AccessPoint_bucketDisappears (29.83s)
--- PASS: TestAccAWSS3AccessPoint_disappears (36.52s)
--- PASS: TestAccAWSS3AccessPoint_basic (42.12s)
--- PASS: TestAccAWSS3AccessPoint_VpcConfiguration (42.25s)
--- PASS: TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration (42.33s)
--- PASS: TestAccAWSS3AccessPoint_Policy (100.66s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	100.688s

aws_s3_bucket_object resource

$ $ make testacc TEST=./aws TESTARGS='-run=TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint -timeout 120m
=== RUN   TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint
=== PAUSE TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint
=== CONT  TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint (72.39s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	72.440s

aws_s3_bucket_object data source

$ make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint -timeout 120m
=== RUN   TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint
=== PAUSE TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint
=== CONT  TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint
--- PASS: TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint (44.76s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	44.792s

aws_s3_bucket_objects data source

$ make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint -timeout 120m
=== RUN   TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint
=== PAUSE TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint
=== CONT  TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint
--- PASS: TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint (63.86s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	63.888s

aws_s3_bucket sweeper

$ TEST=./aws SWEEP=us-west-2 SWEEPARGS=-sweep-run=aws_s3_bucket make sweep
WARNING: This will destroy infrastructure. Use only in development accounts.
go test ./aws -v -sweep=us-west-2 -sweep-run=aws_s3_bucket
2019/12/14 18:15:59 [DEBUG] Running Sweepers for region (us-west-2):
2019/12/14 18:15:59 [DEBUG] Running Sweeper (aws_s3_bucket_object) in region (us-west-2)
2019/12/14 18:15:59 [INFO] Building AWS auth structure
2019/12/14 18:15:59 [INFO] Setting AWS metadata API timeout to 100ms
2019/12/14 18:16:00 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id
2019/12/14 18:16:00 [INFO] AWS Auth provider used: "EnvProvider"
2019/12/14 18:16:00 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/12/14 18:16:00 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/12/14 18:16:01 [INFO] Skipping S3 Bucket: 123456789012-awsmacietrail-dataevent
2019/12/14 18:16:03 [DEBUG] Sweeper (aws_s3_bucket) has dependency (aws_s3_bucket_object), running..
2019/12/14 18:16:03 [DEBUG] Sweeper (aws_s3_bucket_object) already ran in region (us-west-2)
2019/12/14 18:16:03 [DEBUG] Running Sweeper (aws_s3_bucket) in region (us-west-2)
2019/12/14 18:16:03 [INFO] Building AWS auth structure
2019/12/14 18:16:03 [INFO] Setting AWS metadata API timeout to 100ms
2019/12/14 18:16:04 [INFO] Ignoring AWS metadata API endpoint at default location as it doesn't return any instance-id
2019/12/14 18:16:04 [INFO] AWS Auth provider used: "EnvProvider"
2019/12/14 18:16:04 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/12/14 18:16:04 [DEBUG] Trying to get account information via sts:GetCallerIdentity
2019/12/14 18:16:05 [INFO] Skipping S3 Bucket: 123456789012-awsmacietrail-dataevent
2019/12/14 18:16:06 [INFO] Deleting (tf-acc-ewbankkit-001) S3 Access Point: test1
2019/12/14 18:16:06 [INFO] Deleting (tf-acc-ewbankkit-001) S3 Access Point: test2
2019/12/14 18:16:07 [INFO] Deleting S3 Bucket: tf-acc-ewbankkit-001
2019/12/14 18:16:07 [DEBUG] Waiting for state to become: [success]
2019/12/14 18:16:07 Sweeper Tests ran successfully:
	- aws_s3_bucket_object
	- aws_s3_bucket
ok  	github.com/terraform-providers/terraform-provider-aws/aws	8.623s

ewbankkit · 2019-12-14T23:25:11Z

Removed WIP.
Ready for review.

For aws_s3_bucket.force_destroy I am not deleting all the associated S3 access points as this requires additional IAM permissions (ListAccessPoints, DeleteAccessPoint).

Test 'aws_s3_bucket_object' with bucket set to access point ARN. Add access point domain name. Delete all access points associated with a bucket in test sweeper.

iancward · 2020-01-28T16:10:27Z

Any chance this will be reviewed/merged/released soon?

bflad

This looks great, thanks so much, @ewbankkit 🚀 Will handle the minor nits on merge.

Output from acceptance testing:

--- PASS: TestAccAWSS3AccessPoint_basic (25.18s)
--- PASS: TestAccAWSS3AccessPoint_bucketDisappears (16.15s)
--- PASS: TestAccAWSS3AccessPoint_disappears (21.58s)
--- PASS: TestAccAWSS3AccessPoint_Policy (56.65s)
--- PASS: TestAccAWSS3AccessPoint_PublicAccessBlockConfiguration (25.37s)
--- PASS: TestAccAWSS3AccessPoint_VpcConfiguration (23.90s)

--- PASS: TestAccAWSS3BucketObject_updatesWithVersioningViaAccessPoint (35.50s)

--- PASS: TestAccDataSourceAWSS3BucketObject_basicViaAccessPoint (27.07s)

--- PASS: TestAccDataSourceAWSS3BucketObjects_basicViaAccessPoint (40.23s)

bflad · 2020-02-23T20:18:48Z

aws/resource_aws_s3_bucket_test.go

@@ -87,6 +88,45 @@ func testSweepS3Buckets(region string) error {
 			continue
 		}

+		// "Before you can delete this bucket, you must first delete all access points associated with this bucket."


Nit: This sweeper code can be self-contained with the aws_s3_access_point testing code and the aws_s3_bucket sweeper can depend on aws_s3_access_point. 👍 We only limit the aws_s3_bucket sweeper due to some legacy S3 things remaining in the HashiCorp testing account.

bflad · 2020-02-23T20:19:17Z

website/docs/r/s3_access_point.html.markdown

+}
+
+resource "aws_s3_access_point" "example" {
+  account_id = data.aws_caller_identity.current.account_id


Nit: For basic usage, most folks should not need the data source. 👍

bflad · 2020-02-23T20:21:21Z

aws/resource_aws_s3_access_point_test.go

+	}
+}
+
+func testAccCheckAWSS3AccessPointDomainName(n string, key string) resource.TestCheckFunc {


Nit: This custom check function can be replaced with testAccMatchResourceAttrRegionalHostname() (or a new non-regex version of that function)

bflad · 2020-02-23T20:24:24Z

aws/resource_aws_s3_access_point.go

+	d.Set("account_id", accountId)
+	d.Set("arn", arn.String())
+	d.Set("bucket", output.Bucket)
+	d.Set("domain_name", fmt.Sprintf("%s-%s.s3-accesspoint.%s.amazonaws.com", name, accountId, meta.(*AWSClient).region))


Nit: Instead of hardcoding the partition DNS suffix, we should likely try to future-proof this with the DNS suffix in the provider configuration.

Suggested change

d.Set("domain_name", fmt.Sprintf("%s-%s.s3-accesspoint.%s.amazonaws.com", name, accountId, meta.(*AWSClient).region))

d.Set("domain_name", fmt.Sprintf("%s-%s.s3-accesspoint.%s.%s", name, accountId, meta.(*AWSClient).region), meta.(*AWSClient).dnsSuffix)

Potentially worth creating a receiver method on AWSClient itself for this and other usage:

func (client *AWSClient) RegionalHostname(prefix string) string { return fmt.Sprintf("%s.%s.%s", prefix, client.region, client.dnsSuffix) }

I'll create a tracking issue for this as I'd like to ensure there is a covering code linter for this going forward.

Reference: #11276 (review) Output from sweeper in AWS Commercial: ``` 2020/02/23 16:19:05 Sweeper Tests ran successfully: - aws_s3_access_point ``` Output from sweeper in AWS GovCloud (US): ``` 2020/02/23 16:19:23 Sweeper Tests ran successfully: - aws_s3_access_point ``` Output from acceptance testing: ``` --- PASS: TestAccAWSS3AccessPoint_basic (36.70s) ```

ghost · 2020-02-28T14:38:09Z

This has been released in version 2.51.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2020-03-27T17:10:48Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

ewbankkit requested a review from a team December 12, 2019 23:24

ewbankkit force-pushed the issue-11123 branch 3 times, most recently from 3d8e6f2 to 1d9aa45 Compare December 13, 2019 22:52

ghost added service/s3 Issues and PRs that pertain to the s3 service. size/XXL Managed by automation to categorize the size of a PR. and removed size/XL Managed by automation to categorize the size of a PR. labels Dec 13, 2019

ewbankkit force-pushed the issue-11123 branch 3 times, most recently from 775e8c2 to d8ff62f Compare December 14, 2019 23:22

ewbankkit changed the title ~~[WIP] Add aws_s3_access_point resource~~ Add aws_s3_access_point resource Dec 14, 2019

ewbankkit changed the title ~~Add aws_s3_access_point resource~~ [WIP] Add aws_s3_access_point resource Dec 15, 2019

Add aws_s3_access_point resource.

5b57db7

Test 'aws_s3_bucket_object' with bucket set to access point ARN. Add access point domain name. Delete all access points associated with a bucket in test sweeper.

ewbankkit force-pushed the issue-11123 branch from d8ff62f to 5b57db7 Compare December 15, 2019 22:15

ewbankkit changed the title ~~[WIP] Add aws_s3_access_point resource~~ Add aws_s3_access_point resource Dec 15, 2019

bflad added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 11, 2020

bflad self-assigned this Feb 11, 2020

bflad added this to the v2.51.0 milestone Feb 23, 2020

bflad approved these changes Feb 23, 2020

View reviewed changes

bflad merged commit 8b443fe into hashicorp:master Feb 23, 2020

bflad added a commit that referenced this pull request Feb 23, 2020

Update CHANGELOG for #11276

87fcb50

bflad mentioned this pull request Feb 23, 2020

Introduce AWSClient Receiver Methods for Partition/Regional Hostnames with Linter #12140

Closed

ewbankkit deleted the issue-11123 branch February 23, 2020 23:02

ghost locked and limited conversation to collaborators Mar 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add aws_s3_access_point resource #11276

Add aws_s3_access_point resource #11276

ewbankkit commented Dec 12, 2019 •

edited

Loading

ewbankkit commented Dec 14, 2019

iancward commented Jan 28, 2020

bflad left a comment

bflad Feb 23, 2020

bflad Feb 23, 2020

bflad Feb 23, 2020

bflad Feb 23, 2020

ghost commented Feb 28, 2020

ghost commented Mar 27, 2020

	d.Set("domain_name", fmt.Sprintf("%s-%s.s3-accesspoint.%s.amazonaws.com", name, accountId, meta.(*AWSClient).region))
	d.Set("domain_name", fmt.Sprintf("%s-%s.s3-accesspoint.%s.%s", name, accountId, meta.(AWSClient).region), meta.(AWSClient).dnsSuffix)

Add aws_s3_access_point resource #11276

Add aws_s3_access_point resource #11276

Conversation

ewbankkit commented Dec 12, 2019 • edited Loading

Community Note

aws_s3_access_point resource

aws_s3_bucket_object resource

aws_s3_bucket_object data source

aws_s3_bucket_objects data source

aws_s3_bucket sweeper

ewbankkit commented Dec 14, 2019

iancward commented Jan 28, 2020

bflad left a comment

Choose a reason for hiding this comment

bflad Feb 23, 2020

Choose a reason for hiding this comment

bflad Feb 23, 2020

Choose a reason for hiding this comment

bflad Feb 23, 2020

Choose a reason for hiding this comment

bflad Feb 23, 2020

Choose a reason for hiding this comment

ghost commented Feb 28, 2020

ghost commented Mar 27, 2020

ewbankkit commented Dec 12, 2019 •

edited

Loading