provider/aws: Fix aws_route53_record alias perpetual diff

[stack72]

Fixes #9628
Fixes #9298

When a route53_record alias is updated in the console, AWS prepends
dualstack. to the name. This is there incase IPV6 is wanted. It is
exactly the same without it as it is with it

In order to stop perpetual diffs, I introduced a normalizeFunc that will
that tke alias name and strip known issues:

• dualstack
• trailing dot

This normalize fun will continue to grow I'm sure

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRoute53Record_'                                         ✹
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/29 00:28:12 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRoute53Record_ -timeout 120m
=== RUN   TestAccAWSRoute53Record_basic
--- PASS: TestAccAWSRoute53Record_basic (124.64s)
=== RUN   TestAccAWSRoute53Record_basic_fqdn
--- PASS: TestAccAWSRoute53Record_basic_fqdn (132.07s)
=== RUN   TestAccAWSRoute53Record_txtSupport
--- PASS: TestAccAWSRoute53Record_txtSupport (134.07s)
=== RUN   TestAccAWSRoute53Record_spfSupport
--- PASS: TestAccAWSRoute53Record_spfSupport (113.36s)
=== RUN   TestAccAWSRoute53Record_generatesSuffix
--- PASS: TestAccAWSRoute53Record_generatesSuffix (112.62s)
=== RUN   TestAccAWSRoute53Record_wildcard
--- PASS: TestAccAWSRoute53Record_wildcard (162.84s)
=== RUN   TestAccAWSRoute53Record_failover
--- PASS: TestAccAWSRoute53Record_failover (126.18s)
=== RUN   TestAccAWSRoute53Record_weighted_basic
--- PASS: TestAccAWSRoute53Record_weighted_basic (121.10s)
=== RUN   TestAccAWSRoute53Record_alias
--- PASS: TestAccAWSRoute53Record_alias (118.14s)
=== RUN   TestAccAWSRoute53Record_s3_alias
--- PASS: TestAccAWSRoute53Record_s3_alias (155.07s)
=== RUN   TestAccAWSRoute53Record_weighted_alias
--- PASS: TestAccAWSRoute53Record_weighted_alias (235.41s)
=== RUN   TestAccAWSRoute53Record_geolocation_basic
^[[C--- PASS: TestAccAWSRoute53Record_geolocation_basic (125.32s)
=== RUN   TestAccAWSRoute53Record_latency_basic
--- PASS: TestAccAWSRoute53Record_latency_basic (122.23s)
=== RUN   TestAccAWSRoute53Record_TypeChange
--- PASS: TestAccAWSRoute53Record_TypeChange (231.98s)
=== RUN   TestAccAWSRoute53Record_empty
--- PASS: TestAccAWSRoute53Record_empty (116.48s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    2131.526s

Before this fix, I was getting the following by recreating the code in

~ aws_route53_record.alias
    alias.1563903989.evaluate_target_health: "true" => "false"
    alias.1563903989.name:                   "9828-recreation-106795730.us-west-2.elb.amazonaws.com." => ""
    alias.1563903989.zone_id:                "Z1H1FL5HABSF5" => ""
    alias.318754017.evaluate_target_health:  "" => "true"
    alias.318754017.name:                    "" => "9828-recreation-106795730.us-west-2.elb.amazonaws.com"
    alias.318754017.zone_id:                 "" => "Z1H1FL5HABSF5"

Plan: 0 to add, 1 to change, 0 to destroy.

After this fix:

No changes. Infrastructure is up-to-date. This means that Terraform
could not detect any differences between your configuration and
the real physical resources that exist. As a result, Terraform
doesn't need to do anything.

[catsby]

LGTM – are there any scenaiors that exist where an existing alias is not trigging a diff (no "dual stack"), but they may have a . suffix, in which upgrading after this will cause a diff? Long way of asking if we need a migration for this:

+   if strings.HasSuffix(input, ".") {
+       return input[0 : len(input)-1]
+   }

[stack72]

The . is something that was already in place for the read:

name := strings.TrimSuffix(*alias.DNSName, ".")

I just moved it into this function as to avoid multiple normalization operations

[catsby]

Question on migration but otherwise 👍

[catsby]

could either of these operations be replaced with strings.TrimLeft ? https://golang.org/pkg/strings/#TrimLeft ?

[catsby]

I think I meant TrimRight :)

[catsby]

will alias here ever not be a string? All the invocations thus far are a string that I can see

[stack72]

@catsby updated to TrimRight as tested it here :) https://play.golang.org/p/MX_RHqsbfb

[apparentlymart]

Sorry for the late comment here @stack72... but when I've seen this class of error so far I've seen zone_id change too, as if Route53 uses a different zone for the dualstack. form of the hostname than for the regular form. You can see this e.g. in the example in the summary of #9628.

Do you think we need to do something additional here to catch that case, or is this fine because the non-prefixed hostname happens to resolve in the other zone too?

[stack72]

TBH I haven't come across a zone change yet - that is a legit change IMO - I have only come across the issue where f we chose / edit an alias in the console, it adds dualstack. so we are guarding here against that.

I have no doubt that this normalize func will expand in the future though :)

[CpuID]

@stack72 still seeing this in 0.7.9 currently.

~ aws_route53_record.api-lb-e01
    alias.1991554031.evaluate_target_health: "" => "true"
    alias.1991554031.name:                   "" => "api-lb-e01-1049240735.us-east-1.elb.amazonaws.com"
    alias.1991554031.zone_id:                "" => "Z3DZXE0Q79N41H"
    alias.880662862.evaluate_target_health:  "true" => "false"
    alias.880662862.name:                    "api-lb-e01-1049240735.us-east-1.elb.amazonaws.com." => ""
    alias.880662862.zone_id:                 "Z35SXDOTRQ7X7K" => ""

$ terraform version
Terraform v0.7.9

When I try an apply, it passes and claims it applied but in the Route53 console looks like a no-op (still has the old Hosted Zone ID attached). I can confirm that the Route53 console shows a record prefixed with dualstack. currently.

Also note there are valid use cases for setting the dualstack. prefix, which do not seem to be handled by the variable ${aws_elb.api-lb-e01.dns_name} currently. Would it be worth adding an extra attribute to the aws_elb resource of something like .dualstack_dns_name?

[CpuID]

#10004 is a duplicate of this issue, future correspondence there.

[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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.