provider: Automatically validate new regions from AWS SDK

[bflad]

Reference: https://docs.aws.amazon.com/sdk-for-go/api/aws/endpoints/

This version keeps the stricter == matching for ValidateRegion(), but we could loosen it to the below, which would also turn on regexp matching.

if _, isValidRegion := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), c.Region); isValidRegion {
  return nil
}

[radeksimko]

I like the idea, although manual maintenance kind of forced us to support the region fully (i.e. lookup all the account IDs etc.) instead of semi-automatically providing basic support.

Overall I'm happy for you to merge this, but not before addressing this https://github.com/terraform-providers/terraform-provider-aws/blob/c7e748cff0995061b1ceacc444d9e92ece430d77/aws/hosted_zones.go#L28

Otherwise there's chance we'll expose people to crash once Amazon introduces new region and we bump SDK and forget to lookup the ID and add it to the map.

[bflad]

It turns out that code is actually "okay" (at least from crash standpoint): https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/hosted_zones_test.go#L15

The type is map[string]string and string literals have a zero value of "". We just see crashes other places due to usage of map[string]interface{} which has nil zero value

[radeksimko]

Right, so it won't crash 👌

You just made me think that maybe for some lists, like this one in particular we should actually silently fail to allow folks use the S3 bucket resource, but I still think such failure should be logged as WARN or something.

https://github.com/terraform-providers/terraform-provider-aws/blob/e221786c28ba9dfb0b7794df4031a1feaa03309b/aws/resource_aws_s3_bucket.go#L1035-L1039

https://github.com/terraform-providers/terraform-provider-aws/blob/5f3605b9807c6ba4e57a4cdd6faa9bd787a6dac4/aws/data_source_aws_s3_bucket.go#L94-L97

[bflad]

I think there are some nuances to consider. I think cn-northwest-1 for example supports S3 buckets but not S3 website hosting. As long as we "best effort" support some functionality that AWS is inconsistently offering across regions then I'll apply any changes you'd like

[bflad]

Changed the S3 website hosted zone ID function signature to HostedZoneIDForRegion(region string) (string, error) and it'll now log WARN messages when missing instead of setting it to "". Please let me know if you'd like anything else adjusted. 😄

[radeksimko]

👍

[bflad]

This has been released in terraform-provider-aws version 1.9.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[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!