provider/aws: Cleanup Route 53 subdomain name handling

[catsby]

This PR tidies up the handling of Route 53 record name handling. It's a stab at fixing/closing some issues named in #1126, #1264, and #1122, while maintaining the convenience of using shorthand names (www instead of www.domain.com) in the configuration (#969).

Note that I'm changing what we're setting here for name, so instead of setting the fully qualified domain name, I'm only setting what's returned from expandRecordName. As a result, existing names would be recreated.

Ex:

name	expanded	saved
www	www.domain.com	www
domain.com	domain.com	domain.com
www.domain.com	www.domain.com	www.domain.com

Alternatively, we can revert #969 and require people to use fully qualified domain names for the record.name property.

[phinze]

Do we need any nil checks here to avoid pointer deref panics?

[catsby]

I don't believe so; name is required to create the Zone, so, I'm trusting AWS to have that. Assuming so, then the err check immediately before this should bail us out if the Zone isn't found.

If you feel strongly enough I can add them though; generally I agree with your caution here.

[phinze]

Nah I don't feel too strongly about nil checking when logic says there's no way for nil to show up. Just wanted to double check since I didn't have the context. Due diligence etc etc. LGTM

[phinze]

👍 LGTM 🚢

[catsby]

@radeksimko and @wazoo would you mind taking a look and giving any feedback you may have?

[radeksimko]

👍 It fixes the bug introduced in #969

Nonetheless #1122 remains valid and there's actually one extra reason to have proper update which this PR is bringing:

resource "aws_route53_record" "www" {
   zone_id = "${aws_route53_zone.primary.zone_id}"
   name = "subdomain"
   type = "A"
   ttl = "300"
   records = ["127.0.0.1"]
}

changed to

resource "aws_route53_record" "www" {
   zone_id = "${aws_route53_zone.primary.zone_id}"
   name = "subdomain.domain.com"
   type = "A"
   ttl = "300"
   records = ["127.0.0.1"]
}

results in:

aws_route53_record.www: Destroying...
aws_route53_record.www: Destruction complete
aws_route53_record.www: Creating...

[wazoo]

This looks good to me, to make sure I understand it you decided to only do the delete/create if its a FQDN and just the short name does nothing? The only thing I wondered was if you could compare the value (from tfstate) and if current = configured then do nothing.

[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.