Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provider: Remove hardcoded AWS China handling #7654

Merged
merged 1 commit into from
Feb 25, 2019
Merged

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Feb 22, 2019

The usage of IsChinaCloud() (and previously IsGovCloud()) represented the first iteration of trying to support the 3 public AWS partitions when they offered differing capabilities due to running different service API versions. While this approach of checking the AWS partition provides an immediate fix for these situations, it does not automatically handle if/when those new capabilities are enabled.

In newer iterations of this handling, we attempt to simply ignore read errors based on specific error messages, like UnsupportedOperation and passthrough any create/update errors. See https://github.com/terraform-providers/terraform-provider-aws/pull/3794/files for an example of this.

Here we introduce the potentially breaking change of removing the AWS China protections in the aws_instance and aws_s3_bucket_object resources to further discourage the usage of this approach. Should any issues arise in AWS China from these removals, we will implement the error-based approach going forward. It is entirely possible that both EC2 Instance tagging on creation and S3 Object tagging is supported now in that partition though given both have been supported in AWS GovCloud for an extended period of time now.

The usage of `IsChinaCloud()` (and previously `IsGovCloud()`) represented the first iteration of trying to support the 3 public AWS partitions when they offered differing capabilities due to running different service API versions. While this approach of checking the AWS partition provides an immediate fix for these situations, it does not automatically handle if/when those new capabilities are enabled.

In newer iterations of this handling, we attempt to simply ignore read errors based on specific error messages, like `UnsupportedOperation` and passthrough any create/update errors. See https://github.com/terraform-providers/terraform-provider-aws/pull/3794/files for an example of this.

Here we introduce the potentially breaking change of removing the AWS China protections in the aws_instance and aws_s3_bucket_object resources to further discourage the usage of this approach. Should any issues arise in AWS China from these removals, we will implement the error-based approach going forward. It is entirely possible that both EC2 Instance tagging on creation and S3 Object tagging is supported now in that partition though given both have been supported in AWS GovCloud for an extended period of time now.
@bflad bflad added technical-debt Addresses areas of the codebase that need refactoring or redesign. provider Pertains to the provider itself, rather than any interaction with AWS. partition/aws-cn Pertains to the aws-cn partition. labels Feb 22, 2019
@bflad bflad added this to the v2.0.0 milestone Feb 22, 2019
@bflad bflad requested a review from a team February 22, 2019 20:15
@ghost ghost added size/M Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. service/s3 Issues and PRs that pertain to the s3 service. labels Feb 22, 2019
@bflad bflad added the breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. label Feb 23, 2019
@bflad bflad merged commit f2129c3 into master Feb 25, 2019
@bflad bflad deleted the td-IsChinaCloud-removal branch February 25, 2019 19:12
bflad added a commit that referenced this pull request Feb 25, 2019
@ghost
Copy link

ghost commented Mar 31, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. partition/aws-cn Pertains to the aws-cn partition. provider Pertains to the provider itself, rather than any interaction with AWS. service/ec2 Issues and PRs that pertain to the ec2 service. service/s3 Issues and PRs that pertain to the s3 service. size/M Managed by automation to categorize the size of a PR. technical-debt Addresses areas of the codebase that need refactoring or redesign.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants