Added option to avoid DNS record overwrite

[Erouan50]

The current behavior of the aws_route53_record resource is to overwrite the record if one already exists. For example, we suppose that the notexample.com CNAME to foo.bar is added by hand via the CLI. If the new Terraform resource with the same name but with a different CNAME value is added, the record is going to be overridden. This might be a big problem: a record might be changed by mistake.

So this PR proposes to add an option named allow_overwrite on the resource to protect overwrite. When the option is true the CREATE mode is used instead of UPSERT in the change batch. This means that if an added record already exists on route53, the application will fail. By default, the overwrite is permitted to avoid compatibility issue.

[bflad]

@Erouan50 thanks for this contribution! I have definitely gotten bit with this behavior in the past. Do you have time to implement an acceptance test for this as well? e.g.

func TestAccAWSRoute53Record_allowOverwrite(t *testing.T) {
  resource.Test(t, resource.TestCase{
    PreCheck:      func() { testAccPreCheck(t) },
    IDRefreshName: "aws_route53_record.default",
    Providers:     testAccProviders,
    CheckDestroy:  testAccCheckRoute53RecordDestroy,
    Steps: []resource.TestStep{
      {
        PreConfig: func() {
        // Create new Route53 record ahead of time with AWS SDK and wait until its ready
        },
        Config: testAccRoute53RecordConfig_allowOverride(false),
        ExpectError: regexp.MustCompile("... expected AWS error here ..."),
      },
      {
        Config: testAccRoute53RecordConfig_allowOverride(true),
        Check: resource.ComposeTestCheckFunc(
          testAccCheckRoute53RecordExists("aws_route53_record.default"),
	),
      },
    },
  })
}

func testAccRoute53RecordConfig_allowOverride(allowOverride bool) string {
  return fmt.Sprintf(`
resource "aws_route53_zone" "main" {
  name = "notexample.com"
}

resource "aws_route53_record" "default" {
  allow_override = %v
  zone_id = "${aws_route53_zone.main.zone_id}"
  name = "www.notexample.com"
  type = "A"
  ttl = "30"
  records = ["127.0.0.1"]
}
`, allowOverride)
}

[Erouan50]

@bflad I just added it:

TF_ACC=1 go test -v github.com/terraform-providers/terraform-provider-aws/aws -run AccAWSRoute53Record_allowOverwrite
=== RUN   TestAccAWSRoute53Record_allowOverwrite
--- PASS: TestAccAWSRoute53Record_allowOverwrite (111.62s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	111.641s

[bflad]

Thanks for this contribution and creating the extra acceptance test!

Tests passed: 21
=== RUN   TestAccAWSRoute53Record_txtSupport
--- PASS: TestAccAWSRoute53Record_txtSupport (198.78s)
=== RUN   TestAccAWSRoute53Record_multivalue_answer_basic
--- PASS: TestAccAWSRoute53Record_multivalue_answer_basic (199.46s)
=== RUN   TestAccAWSRoute53Record_latency_basic
--- PASS: TestAccAWSRoute53Record_latency_basic (204.06s)
=== RUN   TestAccAWSRoute53Record_generatesSuffix
--- PASS: TestAccAWSRoute53Record_generatesSuffix (212.36s)
=== RUN   TestAccAWSRoute53Record_s3_alias
--- PASS: TestAccAWSRoute53Record_s3_alias (250.28s)
=== RUN   TestAccAWSRoute53Record_failover
--- PASS: TestAccAWSRoute53Record_failover (251.02s)
=== RUN   TestAccAWSRoute53Record_spfSupport
--- PASS: TestAccAWSRoute53Record_spfSupport (254.37s)
=== RUN   TestAccAWSRoute53Record_caaSupport
--- PASS: TestAccAWSRoute53Record_caaSupport (254.52s)
=== RUN   TestAccAWSRoute53Record_geolocation_basic
--- PASS: TestAccAWSRoute53Record_geolocation_basic (268.55s)
=== RUN   TestAccAWSRoute53Record_basic
--- PASS: TestAccAWSRoute53Record_basic (271.87s)
=== RUN   TestAccAWSRoute53Record_basic_fqdn
--- PASS: TestAccAWSRoute53Record_basic_fqdn (274.43s)
=== RUN   TestAccAWSRoute53Record_weighted_basic
--- PASS: TestAccAWSRoute53Record_weighted_basic (276.17s)
=== RUN   TestAccAWSRoute53Record_empty
--- PASS: TestAccAWSRoute53Record_empty (278.52s)
=== RUN   TestAccAWSRoute53Record_wildcard
--- PASS: TestAccAWSRoute53Record_wildcard (299.59s)
=== RUN   TestAccAWSRoute53Record_alias
--- PASS: TestAccAWSRoute53Record_alias (299.83s)
=== RUN   TestAccAWSRoute53Record_AliasChange
--- PASS: TestAccAWSRoute53Record_AliasChange (302.04s)
=== RUN   TestAccAWSRoute53Record_SetIdentiferChange
--- PASS: TestAccAWSRoute53Record_SetIdentiferChange (319.42s)
=== RUN   TestAccAWSRoute53Record_longTXTrecord
--- PASS: TestAccAWSRoute53Record_longTXTrecord (324.94s)
=== RUN   TestAccAWSRoute53Record_TypeChange
--- PASS: TestAccAWSRoute53Record_TypeChange (327.58s)
=== RUN   TestAccAWSRoute53Record_weighted_alias
--- PASS: TestAccAWSRoute53Record_weighted_alias (409.37s)
=== RUN   TestAccAWSRoute53Record_allowOverwrite
--- PASS: TestAccAWSRoute53Record_allowOverwrite (226.51s)

FYI, I pushed one extra commit in there because the acceptance test (while it was written fantastically!) wouldn't work in our parallelized acceptance testing environment that we run regularly. This is due to colliding notexample.com zone names for the aws_route53_zone data source. This is a problem we wouldn't have expected you to know or care about so I went ahead and fixed it to fast track merging this. I hope you don't mind. We'll eventually fix the hardcoded testing. 😢

[Erouan50]

No worries! Thank you for accepting my contribution!

[bflad]

This has been released in version 1.10.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!