resource/aws_route53_record: Switch allow_overwrite default from true to false #7734

bflad · 2019-02-26T19:55:20Z

Closes #3895
Reference: #2926

Previously, the aws_route53_record resource did not follow standard Terraform conventions of requiring existing infrastructure to be imported into Terraform's state for management, which meant operators could unexpectedly affect that existing infrastructure. In version 1.10.0, we introduced the allow_overwrite argument so operators could opt into the upcoming import requirement and force the Terraform resource during resource creation to error if it attempted to create a Route53 record that previously existed.

Here we make the breaking change to switch the default resource behavior to error on creation for existing records. Operators can opt out of the new behavior by setting allow_overwrite = true, but it is marked as deprecated for removal in the next major version and will display the deprecation warning when used to signal that workflows should be adjusted if necessary.

Output from acceptance testing:

--- PASS: TestAccAWSRoute53Record_Alias_Elb (319.83s)
--- PASS: TestAccAWSRoute53Record_Alias_S3 (123.47s)
--- PASS: TestAccAWSRoute53Record_Alias_Uppercase (184.67s)
--- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (450.17s)
--- PASS: TestAccAWSRoute53Record_AliasChange (157.29s)
--- PASS: TestAccAWSRoute53Record_allowOverwrite (365.48s)
--- PASS: TestAccAWSRoute53Record_basic (130.53s)
--- PASS: TestAccAWSRoute53Record_basic_fqdn (146.30s)
--- PASS: TestAccAWSRoute53Record_caaSupport (177.87s)
--- PASS: TestAccAWSRoute53Record_disappears (107.58s)
--- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (247.77s)
--- PASS: TestAccAWSRoute53Record_empty (115.48s)
--- PASS: TestAccAWSRoute53Record_failover (204.75s)
--- PASS: TestAccAWSRoute53Record_generatesSuffix (180.48s)
--- PASS: TestAccAWSRoute53Record_geolocation_basic (196.58s)
--- PASS: TestAccAWSRoute53Record_importBasic (174.28s)
--- PASS: TestAccAWSRoute53Record_importUnderscored (114.40s)
--- PASS: TestAccAWSRoute53Record_latency_basic (173.99s)
--- PASS: TestAccAWSRoute53Record_longTXTrecord (114.97s)
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (197.09s)
--- PASS: TestAccAWSRoute53Record_SetIdentifierChange (206.47s)
--- PASS: TestAccAWSRoute53Record_spfSupport (152.57s)
--- PASS: TestAccAWSRoute53Record_txtSupport (170.00s)
--- PASS: TestAccAWSRoute53Record_TypeChange (220.81s)
--- PASS: TestAccAWSRoute53Record_weighted_alias (278.61s)
--- PASS: TestAccAWSRoute53Record_weighted_basic (112.68s)
--- PASS: TestAccAWSRoute53Record_wildcard (216.04s)

… to false References: * #3895 * #2926 Previously, the `aws_route53_record` resource did not follow standard Terraform conventions of requiring existing infrastructure to be imported into Terraform's state for management, which meant operators could unexpectedly affect that existing infrastructure. In version 1.10.0, we introduced the `allow_overwrite` argument so operators could opt into the upcoming import requirement and force the Terraform resource during resource creation to error if it attempted to create a Route53 record that previously existed. Here we make the breaking change to switch the default resource behavior to error on creation for existing records. Operators can opt out of the new behavior by enabling the flag, but it is marked as deprecated for removal in the next major version and will display the deprecation warning when used to signal that workflows should be adjusted if necessary. Output from acceptance testing: ``` --- PASS: TestAccAWSRoute53Record_Alias_Elb (319.83s) --- PASS: TestAccAWSRoute53Record_Alias_S3 (123.47s) --- PASS: TestAccAWSRoute53Record_Alias_Uppercase (184.67s) --- PASS: TestAccAWSRoute53Record_Alias_VpcEndpoint (450.17s) --- PASS: TestAccAWSRoute53Record_AliasChange (157.29s) --- PASS: TestAccAWSRoute53Record_allowOverwrite (365.48s) --- PASS: TestAccAWSRoute53Record_basic (130.53s) --- PASS: TestAccAWSRoute53Record_basic_fqdn (146.30s) --- PASS: TestAccAWSRoute53Record_caaSupport (177.87s) --- PASS: TestAccAWSRoute53Record_disappears (107.58s) --- PASS: TestAccAWSRoute53Record_disappears_MultipleRecords (247.77s) --- PASS: TestAccAWSRoute53Record_empty (115.48s) --- PASS: TestAccAWSRoute53Record_failover (204.75s) --- PASS: TestAccAWSRoute53Record_generatesSuffix (180.48s) --- PASS: TestAccAWSRoute53Record_geolocation_basic (196.58s) --- PASS: TestAccAWSRoute53Record_importBasic (174.28s) --- PASS: TestAccAWSRoute53Record_importUnderscored (114.40s) --- PASS: TestAccAWSRoute53Record_latency_basic (173.99s) --- PASS: TestAccAWSRoute53Record_longTXTrecord (114.97s) --- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (197.09s) --- PASS: TestAccAWSRoute53Record_SetIdentifierChange (206.47s) --- PASS: TestAccAWSRoute53Record_spfSupport (152.57s) --- PASS: TestAccAWSRoute53Record_txtSupport (170.00s) --- PASS: TestAccAWSRoute53Record_TypeChange (220.81s) --- PASS: TestAccAWSRoute53Record_weighted_alias (278.61s) --- PASS: TestAccAWSRoute53Record_weighted_basic (112.68s) --- PASS: TestAccAWSRoute53Record_wildcard (216.04s) ```

paddycarver · 2019-02-26T20:01:05Z

website/docs/guides/version-2-upgrade.html.md

+
+### allow_overwrite Default Value Change
+
+The resource now requires existing Route 53 Records to be imported into the Terraform state for management unless the `allow_overwrite` argument is enabled. The `allow_overwrite` flag is considered deprecated for removal in the next major version of the Terraform AWS Provider (version 3.0.0).


Suggestion: don't name a specific version, just say "a future version". Then if you forget for 3.0.0, you didn't break a promise.

👍 Great suggestion

…nt as future major version, not necessarily next

This picks up a bunch of upstream improvements [1] including [2] to address [3]. Generated with a manual Gopkg.toml edit followed by: $ cd terraform/exec/plugins $ dep ensure with: $ dep version dep: version : v0.5.0-31-g73b3afe build date : 2019-02-08 git hash : 73b3afe go version : go1.10.3 go compiler : gc platform : linux/amd64 features : ImportDuringSolve=false [1]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/CHANGELOG.md [2]: hashicorp/terraform-provider-aws#7734 (v2.0.0) [3]: https://bugzilla.redhat.com/show_bug.cgi?id=1659970

This picks up a bunch of upstream improvements [1] including [2] to address [3]. Generated with a manual Gopkg.toml edit followed by: $ cd terraform/exec/plugins $ dep ensure with: $ dep version dep: version : v0.5.1 build date : 2019-03-20 git hash : faa61893 go version : go1.10.3 go compiler : gc platform : linux/amd64 features : ImportDuringSolve=false The aws-iam-authenticator pin gets us closer to [4], although (1) I don't know why dep isn't figuring that out for itself and (2) they got rid of their 0.3.1 tag [5]? Anyway, this avoids: $ hack/build.sh ... # github.com/openshift/installer/pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws/data_source_aws_eks_cluster_auth.go:35:38: too many arguments in call to token.NewGenerator have (bool) want () ... The AWS SDK pin gets us closer to [6] and avoids: $ hack/build.sh ... # github.com/openshift/installer/pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws/structure.go:4801:7: spec.ServiceNames undefined (type *appmesh.VirtualRouterSpec has no field or method ServiceNames) ... The = prefixes in Gopkg.toml settle things down, because [7]: Note: When you specify a version without an operator, dep automatically uses the ^ operator by default. dep ensure will interpret the given version as the min-boundary of a range... [1]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/CHANGELOG.md [2]: hashicorp/terraform-provider-aws#7734 (v2.0.0) [3]: https://bugzilla.redhat.com/show_bug.cgi?id=1659970 [4]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/go.mod#L27 [5]: https://github.com/kubernetes-sigs/aws-iam-authenticator/tags [6]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/go.mod#L7 [7]: https://golang.github.io/dep/docs/Gopkg.toml.html#version-rules

osterman · 2019-04-09T06:44:47Z

@bflad just making sure #7846 is on your radar =) Hoping that allow_overwrite will not actually get deprecated as this is a very valid use-case for setting the SOA record that necessitates allow_overwrite.

ghost · 2020-03-30T17:48:03Z

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!

bflad added breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. service/route53 Issues and PRs that pertain to the route53 service. technical-debt Addresses areas of the codebase that need refactoring or redesign. labels Feb 26, 2019

bflad added this to the v2.0.0 milestone Feb 26, 2019

bflad requested a review from a team February 26, 2019 19:55

ghost added size/S Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. labels Feb 26, 2019

bflad force-pushed the td-aws_route53_record-allow_overwrite-default branch from e60b809 to a8c6bdf Compare February 26, 2019 20:00

paddycarver reviewed Feb 26, 2019

View reviewed changes

paddycarver approved these changes Feb 26, 2019

View reviewed changes

guides/version-2-upgrade: Clarify aws_route53_record import requireme…

133194c

…nt as future major version, not necessarily next

bflad merged commit 509ddc9 into master Feb 26, 2019

bflad deleted the td-aws_route53_record-allow_overwrite-default branch February 26, 2019 22:30

bflad added a commit that referenced this pull request Feb 26, 2019

Update CHANGELOG for #7734

7d8d894

bflad mentioned this pull request Mar 13, 2019

Terraform modified root domain that wasn't managed by terraform instead of throwing an error #7920

Closed

wking mentioned this pull request Mar 20, 2019

Bug 1659970: terraform/exec/plugins/vendor: Bump terraform-provider-aws to v2.2.0 openshift/installer#1442

Merged

osterman mentioned this pull request Apr 9, 2019

aws_route53_record allow_overwrite deprecation will break ability to update SOA #7846

Closed

bflad mentioned this pull request Apr 10, 2019

resource/aws_route53_record: Remove deprecation from allow_overwrite argument #8274

Merged

bflad mentioned this pull request May 10, 2019

NS error when creating new zone #8598

Closed

pamiescmp mentioned this pull request May 28, 2019

Wildcard certificate stopped working jpamies/terraform-aws-certificate#1

Closed

jpamies mentioned this pull request May 28, 2019

Fix allow_overwrite, default value changed jpamies/terraform-aws-certificate#2

Merged

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_route53_record: Switch allow_overwrite default from true to false #7734

resource/aws_route53_record: Switch allow_overwrite default from true to false #7734

bflad commented Feb 26, 2019

paddycarver Feb 26, 2019

bflad Feb 26, 2019

osterman commented Apr 9, 2019 •

edited

Loading

ghost commented Mar 30, 2020


		### allow_overwrite Default Value Change

		The resource now requires existing Route 53 Records to be imported into the Terraform state for management unless the `allow_overwrite` argument is enabled. The `allow_overwrite` flag is considered deprecated for removal in the next major version of the Terraform AWS Provider (version 3.0.0).

resource/aws_route53_record: Switch allow_overwrite default from true to false #7734

resource/aws_route53_record: Switch allow_overwrite default from true to false #7734

Conversation

bflad commented Feb 26, 2019

paddycarver Feb 26, 2019

Choose a reason for hiding this comment

bflad Feb 26, 2019

Choose a reason for hiding this comment

osterman commented Apr 9, 2019 • edited Loading

ghost commented Mar 30, 2020

osterman commented Apr 9, 2019 •

edited

Loading