Fixing s3 bucket domain name in non-commercial regions

[frezbo]

Changes proposed in this pull request:

• Fixing s3 bucket domain name in non-commercial regions

Output from acceptance testing:

terraform-provider-aws (aws:rean-gov-sd)(kc)(git:feature/fix-domain-output)*$ printenv AWS_DEFAULT_REGION
us-gov-west-1
terraform-provider-aws (aws:rean-gov-sd)(kc)(git:feature/fix-domain-output)*$ make testacc TESTARGS='-run=TestAccAWSS3Bucket_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSS3Bucket_basic -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSS3Bucket_basic
--- PASS: TestAccAWSS3Bucket_basic (156.21s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	156.229s
terraform-provider-aws (aws:rean-gov-sd)(kc)(git:feature/fix-domain-output)*$ 

I have added tests to make sure it works in govcloud and china regions. This is my initial stab at golang. Thanks.

[bflad]

Thanks for submitting this @frezbo!

FYI we now have 3 conflicting PRs surrounding S3 and non-Commercial partition handling:

• Refactor s3 acceptance tests to run in multiple partitions. #4466
• Issue #4540 Add new attribute bucket_regional_domain_name in aws_s3_bucket #4556
• Fixing s3 bucket domain name in non-commercial regions #4599

I was working on reviewing #4466 with one last feedback item, so that will likely get merged first and cause merge conflicts in the last two. @frezbo can you look at #4556 and see if the regional bucket name handling will work? I think #4556 is a safer implementation (as it introduces non-breaking changes) at first glance.

[frezbo]

@bflad I really like the #4466 implementation,Looks real nice. !! except I really feel we should be checking for the real-partition names itself rather than the regex there. (My personal view). What do you feel.?

Regarding #4566 does the bucket_domain_name return the same data? It needs to be enhanced with the gov-cloud regions too.

I am ready to close my PR in favor of the others if it solves all issues, or modify as needed as the order in which they get merged in. Thanks

[bflad]

#4466 (testing updates to support GovCloud/China) and #4556 (non-breaking change to add bucket_regional_domain_name attribute based on the SDK endpoints package) have been merged, so looks like this PR is now fully a duplicate. Thanks for submitting this though, sorry that the earlier PRs handled everything already. 😅

[ghost]

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!