Don't destroy records when changing types between A/CNAME/NS #119
Description
Is your request related to a new offering from AWS?
No.
Is your request related to a problem? Please describe.
This is revisiting #95, and I would like the opportunity to mount an argument:
Suppose you have foo.example.com which is currently a CNAME, and you want to change it to an A record alias. When using this module to perform this change, the old record will be destroyed and the new one created, because of course the aws_route53_record resource key contains the record type.
The problem is that this causes an outage. It's not even a minor outage: if a user queries foo.example.com in between deletion and creation, they will cache the NXDOMAIN for the refresh value defined in the SOA, which by default is 15 minutes in Route53.
The AWS provider uses the ChangeResourceRecordSets API and supports atomic delete/recreate of records. So Terraform and the provider can handle seamless type changes -- the outage only occurs because of the module behavior.
Describe the solution you'd like.
When calculating aws_route53_record keys, use the same key for A, NS, and CNAME records, in order to allow seamlessly transitioning between these record types. (A and CNAME are obvious based on the above example, but NS is also important when transitioning to/from a delegation.)
This is a breaking change requiring manual state renames, but it could be tucked behind a feature flag to mitigate that pain.
Describe alternatives you've considered.
Internally we've forked the records module to accomplish this.
In locals, we're calculating recordsets as:
recordsets = {
for rs in local.records : try(
rs.key,
join(" ", compact([
coalesce(rs.name, "__apex__"),
contains(["A", "CNAME", "NS"], rs.type) ? "A_CNAME_NS" : rs.type,
try(rs.set_identifier, ""),
]))
) => rs
}The __apex__ stuff is unrelated to this issue -- we do it because we define a lot of apex records and the key is calculated as e.g. " A" and the space prefix is weird. That's harmless though, just cosmetic, so I would happily live with that if this issue were resolved so as not to need to maintain a fork of the records submodule.
Alternatively, key could be explicitly set on the record. But hardly anyone would know to do this, so I see this FR as avoiding a footgun.