Add option to force Gov/China Cloud

[sodre]

Sometimes it is necessary to use the China/GovCloud logic when using custom endpoints.

This PR allows the user to configure those options.

[bflad]

@sodre we don't necessarily want to keep utilizing the isGovCloud() and isChinaCloud() logic. It is not implemented in very many places and could cause trouble if/when APIs in those AWS partitions are upgraded. Especially for non-AWS implementations, we may want to introduce separate logic.

Can you explain where you're running into trouble and maybe we can instead improve the resources themselves?

[sodre]

@bflad For testing purposes, we run something similar to localstack that has the same behavior as the actual GovCloud/China partition. In order to connect to those endpoints I need to configure custom endpoints and a custom region.

The custom region is not detected as being part of GovCloud/China partition and Terraform fails exactly where there are checks for "isGovCloud/isChina". The purpose of the patch is to let us inform terraform that those custom endpoints/custom region has the same "quirks" as GovCloud/ChinaCloud, e.g. #3317.

e.g. #3317.

[bflad]

@sodre I asked in that PR that the isGovCloud() check be removed and instead to infer the correct behavior for an "out of date" API endpoint with a code comment (in that case: default the VPC endpoint type to Gateway if it wasn't returned from the API). Basically, this is to automatically future-proof the resource for if/when AWS updates the US Gov partition.

I think we would personally prefer to fix other resources similarly to handle these situations rather than introduce and depend on any specific non-standard checks that will later require manual intervention. I worry that we can run into scenarios where now an update to remove a check like isGovCloud() (because it really is no longer needed) would now break configurations depending on the behavior of this new force_gov_cloud until the "localstack", etc. APIs were updated. At worst, we could create a separate check/config for each non-standard API.

Do you have other examples?

[sodre]

The one I ran into is related to SQS resources:

• https://github.com/terraform-providers/terraform-provider-aws/blob/a5f9efcc2381d870a6a69cbb07abe70da6bb55d1/aws/resource_aws_sqs_queue.go#L316-L324

Other places it will likely come up:

• https://github.com/terraform-providers/terraform-provider-aws/blob/6a3d5c60e186a08d1f59fbd3b915ddfd4749d0d7/aws/resource_aws_cloudwatch_log_group.go#L124-L130
• https://github.com/terraform-providers/terraform-provider-aws/blob/d64a5684a43e2179585e7e1444ba2bea97cd0cc1/aws/resource_aws_instance.go#L476-L505

[sodre]

Realistically, this issue will come up everywhere where IsGovCloud or IsChinaCloud is used in your code.

• ChinaCloud: 4 cases
• GovCloud: 4 cases

[bflad]

Thanks. Are you able to set region = "us-gov-west-1" to workaround the behavior for now?

[sodre]

not really :(

[sodre]

@bflad,
The reason I proposed the PR as configuration option was because it is very close semantically to SkipRegionValidation. In the PR we are saying "Region is Gov/ChinaCloud"

Unfortunately, the choice of the region name is outside the developers hand. That is why I think changing the region name would not fix it. Isn't that also used to create validate the ARNs?

[sodre]

Hi @bflad,

So I can inform the folks on my side, is there a plan to merge this PR?

[bflad]

I talked with a few people and we believe this will cause maintenance or confusion issues in the future. We would happily accept changes to the resources to allow "backwards" compatibility though (e.g. removing the IsGovCloud() checks).

For example, the SQS resource can be adjusted to skip returning an error on the InvalidAction instead of relying on the IsGovCloud() check. I can submit a PR for this tomorrow.

[sodre]

No problem. Thanks for responding quickly. We will wait for your PR.

[bflad]

As promised, PR submitted: #3794

If you could locally build and test the provider from that PR in your environment that would be fantastic. 👍

As for other resources where you have or will run into this, please submit new separate issues for each resource and they can pretty easily be addressed. Most importantly, we will be looking for the exact error message as each service implements these errors differently.

[sodre]

@bflad I'll get back to you on Monday! Thank you for getting to it so quickly.

[bflad]

The SQS ListQueueTags fix has been released in version 1.12.0 of the AWS provider. 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!