Skip to content

Fix bug in Route 53 ALIAS record#336

Merged
captncraig merged 1 commit intoStackExchange:masterfrom
koenrh:fix-route53-alias-record-type
Jun 25, 2018
Merged

Fix bug in Route 53 ALIAS record#336
captncraig merged 1 commit intoStackExchange:masterfrom
koenrh:fix-route53-alias-record-type

Conversation

@koenrh
Copy link
Copy Markdown
Contributor

@koenrh koenrh commented Mar 17, 2018

While test-driving the Route 53 ALIAS record type, I noticed a little bug that prevents one from using multiple ALIAS records for the same record set name. For example, two ALIAS records for the record set name www with record set types A and AAAA, respectively.

D("foo.bar", REGISTRAR, DnsProvider("ROUTE53"),
  R53_ALIAS("www", "AAAA", "foo.cloudfront.net.", R53_ZONE("BAZ")),
  R53_ALIAS("www", "A", "foo.cloudfront.net.", R53_ZONE("BAZ"))
);

If one would deploy this configuration, then only an ALIAS record set for www with record set type A would have been created. The first one (www with record set type AAAA) would have been skipped.

This is caused by the way the updates map in the example below is populated. The key (a struct) contains both a 'Name' (e.g. www.foo.bar) and a Type (e.g. R53_ALIAS). Which means that two ALIAS records, with different record set types, end up with the same key (www.foo.bar, R53_ALIAS), which means that only either of the ALIAS records (the latter) is actually deployed.

for k := range namesToUpdate {
updates[k] = nil
for _, rc := range dc.Records {
if getKey(rc) == k {
updates[k] = append(updates[k], rc)
}
}
}

I have added a quick fix that appends the resource record type to the actual record type so that different ALIAS records will actually be considered as such. I'm curious to know what you think, and/or whether this should be addresses in a different way.

@tlimoncelli
Copy link
Copy Markdown
Contributor

Good catch!

Can you add a test for this? (either a unit test for getKey() or an integration test in integrationTest/integration_test.go ?)

@StackExchange StackExchange deleted a comment from tlimoncelli Apr 25, 2018
@captncraig captncraig merged commit 921fa98 into StackExchange:master Jun 25, 2018
@koenrh koenrh deleted the fix-route53-alias-record-type branch October 8, 2018 21:37
rblenkinsopp pushed a commit to rblenkinsopp/dnscontrol that referenced this pull request Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants