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

Sugestion: aws_route53_record allow_overwrite should have the default value to false #3895

Closed
marcosdiez opened this issue Mar 23, 2018 · 4 comments · Fixed by #7734
Closed
Labels
question A question about existing functionality; most questions are re-routed to discuss.hashicorp.com. service/route53 Issues and PRs that pertain to the route53 service.
Milestone

Comments

@marcosdiez
Copy link
Contributor

Terraform Version

Terraform v0.11.5
+ provider.aws v1.11.0

Affected Resource(s)

  • aws_route53_record

Terraform Configuration Files

provider "aws" {
  version = "1.11"
  region  = "us-east-1"
}

resource "aws_route53_record" "www_d2" {
  zone_id = "ZZZZZZZZZZZZZZ"
  name    = "marcos_test.AAAAAAAAA.com"
  type    = "A"
  ttl     = "300"

  #   allow_overwrite = "false"
  records = ["10.10.10.20"]
}

Sugestion

One of the nice things about terraform is that it only fiddles with resources it created (unless one explicitly uses terraform import)

That means one can be certain that he or she won't step on nobody else's toes while using terraform.

Weirdly, that does not happen with aws_route53_record.
It simply overwrites existing records as if they did not matter.
@Erouan50 was nice enough to create a parameter called allow_overwrite exactly to prevent that behavior.

Nevertheless the default value is true. I deeply believe it should be false. The former is counter intuitive and when one notices, it may be too late. Although this is a behavior change, it's the same as adding a sit belt to the existing process. More or less like terraform --apply is not automatic now.

Please consider changing the default value of allow_overwrite to false

References

Are there any other GitHub issues (open or closed) or Pull Requests that should be linked here? For example:

@bflad
Copy link
Contributor

bflad commented Mar 23, 2018

Hi @marcosdiez 👋 When the original resource was written (quite a long time ago), it was set to always use the UPSERT action on resource creation, as you said, contrary to how almost all Terraform resources since then operate on creation. In order to introduce non-breaking behavior to the existing resource, the allow_overwrite flag was introduced to allow you to opt into the CREATE action on resource creation so it properly errors. The allow_overwrite flag is a workaround and opportunity to allow the community to ensure their environments and configuration are ready for when the potentially breaking change occurs to flip the resource creation behavior. We do not intend to introduce breaking compatibility without a major version increase.

The eventual plan is to switch the behavior to always use the CREATE action on resource creation and potentially remove the allow_overwrite flag in the next major version (2.0.0) of the AWS provider. We do not have a specific release date for this yet.

@bflad bflad added question A question about existing functionality; most questions are re-routed to discuss.hashicorp.com. service/route53 Issues and PRs that pertain to the route53 service. labels Mar 23, 2018
@bflad bflad added this to the v2.0.0 milestone Mar 23, 2018
@marcosdiez
Copy link
Contributor Author

That's a very sane decision.
Congratulations for such a wise choice!

Marcos

bflad added a commit that referenced this issue Feb 26, 2019
… 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)
```
@bflad
Copy link
Contributor

bflad commented Feb 26, 2019

The behavior of requiring imports for existing infrastructure is now the default in version 2.0.0, releasing later this week. The allow_overwrite option will still be available as a deprecated flag to allow operators time to fix their workflows before another future major release which removes the flag.

@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
question A question about existing functionality; most questions are re-routed to discuss.hashicorp.com. service/route53 Issues and PRs that pertain to the route53 service.
Projects
None yet
2 participants