Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route53 Zone doesn't connect and load records properly #1012

Closed
slandry90 opened this issue Oct 14, 2020 · 16 comments · Fixed by #1015
Closed

Route53 Zone doesn't connect and load records properly #1012

slandry90 opened this issue Oct 14, 2020 · 16 comments · Fixed by #1015

Comments

@slandry90
Copy link
Contributor

Describe the bug
When trying to connect a route53 zone, we encounter an error "Missing apex NS record". This is happening because name of the map on the records does not match the name of the maps for the recordSets and is therefore just returning None.

We did some digging in the code and were able to isolate the issue down to Route53Conversions toVinylRecordSet function.. Fqdn.merge(r53RecordSet.getName, zoneName).firstLabel should be r53RecordSet.getName.toString so the name of the map is correct consistently.

Once we fixed the above and allowed the zone to be added we ran into additional issues where no records show up in the zone on vinyl (even though they exist in r53, no errors) and records cannot be added or removed (error out with "Failed validating update to DNS for change 784b4a3d-b3a8-4b82-8596-0c684fa34ef3:jenkins: Incompatible record already exists in DNS." for example)

Records appear to be missing because the zoneid is "unknown" in Route53Conversions, we updated toVinylRecordSets in the loadZone function which added the zoneid to the recordSets and they now appear in the zone in vinyl but we are still unable to CRUD records through vinyl for the zone

sidebar, SOA records are also broken but we'll open a new issue for that and we'll have an MR for that as well.

VinylDNS Version
0.9.7

To Reproduce
Steps to reproduce the behavior:

  1. Add route53 zone to vinyldns through portal returns "Missing apex NS record"
@pauljamescleary
Copy link
Contributor

Thanks @slandry90 , I am going to add an integration test to recreate your issue.

The "Failed validating update to DNS" happens when a change is sent to the backend (like create a record set), and the record set already exists there and isn't a match for the change being applied. This got through likely because of what you mentioned, that you saw no records in vinyldns.

@pauljamescleary
Copy link
Contributor

pauljamescleary commented Oct 14, 2020

@slandry90 for the first issue, missing Apex NS, the issue is that ListRecordSetsRequest does not by default return the NS record (I don't think it returns the SOA record either which is unfortunate). Is this what you saw?

For that one, in the Route53Backend we need to make an additional call in def loadZone to do an additional ListReusableDelegationSets and transform the name servers into NS records from there.

Update - actually strike that, the GetHostedZone request returns the name servers

@pauljamescleary
Copy link
Contributor

@slandry90 found the other issue, yes we are not setting the zone id when we do the zone sync process, so it is being saved as "unknown" in the database.

I have an integration test that will fix both of these issues.

@pauljamescleary
Copy link
Contributor

For public hosted zones, we should get an SOA record back. For private hosted zones, it looks like there is no SOA record returned.

Right now, I am unsure if we need the SOA data at all for Route 53 backend. We use it in the DnsBackend to check if the zone exists, but that is a different call in Route53

@pauljamescleary
Copy link
Contributor

Thanks @slandry90 for the great write up. #1015 is up to address these issues.

@slandry90
Copy link
Contributor Author

slandry90 commented Oct 15, 2020

Thanks @pauljamescleary this appears to solve the CRUD issues with the zone that was already connected, however I was still unable to connect to a new zone. Same error for Apex NS, this is a public hosted zone and does not have a delegation set. Here's a log of the zone object at load time before the Apex NS Error.

2020-10-15 02:36:11,160 [test] INFO | (Boot:48) | ZoneView(Zone: [id="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="example2.info."; account="system"; adminGroupId="8b651df9-7b70-4d1a-aaaf-5e71c1634bc2"; status="Active"; shared="false"; connection="None"; transferConnection="None"; reverse="false"; isTest="false"; created="2020-10-15T02:36:10.605Z"; ],Map((example2,UNKNOWN) -> RecordSet: [id="6b7772af-67e3-4fd0-8f00-dbeff794ad6a"; zoneId="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="example2"; type="UNKNOWN"; ttl="900"; created="2020-10-15T02:36:11.147Z"; updated="Some(2020-10-15T02:36:11.147Z)"; account="system"; status="Active"; records="List()"; ownerGroupId="None"fqdn="Some(example2.info.)"], (blah-blah,A) -> RecordSet: [id="d73bd465-a97d-4cd5-ba7d-d1e36fae0566"; zoneId="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="blah-blah"; type="A"; ttl="300"; created="2020-10-15T02:36:11.147Z"; updated="Some(2020-10-15T02:36:11.147Z)"; account="system"; status="Active"; records="List(0.0.0.0)"; ownerGroupId="None"fqdn="Some(blah-blah.example2.info.)"], (test,CNAME) -> RecordSet: [id="268e6a65-ab08-4a6b-89f6-325db6af5e3b"; zoneId="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="test"; type="CNAME"; ttl="360"; created="2020-10-15T02:36:11.148Z"; updated="Some(2020-10-15T02:36:11.148Z)"; account="system"; status="Active"; records="List(0.0.0.0.)"; ownerGroupId="None"fqdn="Some(test.example2.info.)"], (example2,NS) -> RecordSet: [id="a38cd3bf-3ed6-44ef-a539-e4052e79be40"; zoneId="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="example2"; type="NS"; ttl="172800"; created="2020-10-15T02:36:11.143Z"; updated="Some(2020-10-15T02:36:11.143Z)"; account="system"; status="Active"; records="List(ns-47.awsdns-05.com., ns-1419.awsdns-49.org., ns-699.awsdns-23.net., ns-1610.awsdns-09.co.uk.)"; ownerGroupId="None"fqdn="Some(example2.info.)"], (test2,CNAME) -> RecordSet: [id="dd102334-f7bd-4a65-af95-816b4f3d2334"; zoneId="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="test2"; type="CNAME"; ttl="7200"; created="2020-10-15T02:36:11.149Z"; updated="Some(2020-10-15T02:36:11.149Z)"; account="system"; status="Active"; records="List(0.0.0.0.)"; ownerGroupId="None"fqdn="Some(test2.example2.info.)"]))

@pauljamescleary
Copy link
Contributor

@slandry90 what error are you seeing. Are you seeing "Missing apex NS record"?

It appears as though the zone name is example2.info based on this line Zone: [id="95081fca-2932-47a7-a485-0aa0a6aed0fb"; name="example2.info."; ...

but the NS record is coming back as example2.

This line assumes the NS record name matches the zone name.

def hasApexNS(zoneView: ZoneView): Result[Unit] = {
    val apexRecord = zoneView.recordSetsMap.get(zoneView.zone.name, RecordType.NS)

Here is the bug...

  def toVinylRecordSet(zoneName: String, r53RecordSet: ResourceRecordSet): RecordSet = {
    val typ = toVinylRecordType(RRType.fromValue(r53RecordSet.getType))
    RecordSet(
      "unknown",
      Fqdn.merge(r53RecordSet.getName, zoneName).firstLabel,

We pull the first label; however, we should pull the whole FQDN if it matches the zone name (that is what we do with DNS). I will update the PR.

@pauljamescleary
Copy link
Contributor

@slandry90 updated the PR, added tests

pauljamescleary added a commit that referenced this issue Oct 15, 2020
Fixes #1012 

Changes in this pull request:
- When available, ensure that the `zoneId` is passed when loading vinyldns record sets
- Use the `DelegationSet` on the hosted zone to _simulate_ NS records.  NS records do not exist by default on private hosted zones
@pauljamescleary
Copy link
Contributor

@slandry90 #1015, was merged now to master, will be online later to see if you have any issues. Thanks for opening the issue and testing!

@slandry90
Copy link
Contributor Author

Still receiving the same "Missing apex NS record" error with the latest change set, will readd logging and sent that along shortly

@slandry90
Copy link
Contributor Author

slandry90 commented Oct 15, 2020

@pauljamescleary Same issue as before, the NS record name is still showing up as the short name instead of the full zone name. I also tested with a private zone and am getting the same response back.

2020-10-15 16:53:22,199 [test] INFO | (hasApexNS:47) | ZoneView(Zone: [id="ce1ebac9-bde6-45b8-9dce-d0fe3c4691ae"; name="example2.info."; account="system"; adminGroupId="8b651df9-7b70-4d1a-aaaf-5e71c1634bc2"; status="Active"; shared="false"; connection="None"; transferConnection="None"; reverse="false"; isTest="false"; created="2020-10-15T16:53:21.532Z"; ],Map((example2,UNKNOWN) -> RecordSet: [id="2b0a697b-47bc-47d6-b2be-56a994bfd857"; zoneId="ce1ebac9-bde6-45b8-9dce-d0fe3c4691ae"; name="example2"; type="UNKNOWN"; ttl="900"; created="2020-10-15T16:53:22.188Z"; updated="Some(2020-10-15T16:53:22.188Z)"; account="system"; status="Active"; records="List()"; ownerGroupId="None"fqdn="Some(example2.info.)"], (blah-blah,A) -> RecordSet: [id="5c52ad51-655b-4ee7-b7b6-0d1edd34b7fd"; zoneId="ce1ebac9-bde6-45b8-9dce-d0fe3c4691ae"; name="blah-blah"; type="A"; ttl="300"; created="2020-10-15T16:53:22.188Z"; updated="Some(2020-10-15T16:53:22.188Z)"; account="system"; status="Active"; records="List(0.0.0.0)"; ownerGroupId="None"fqdn="Some(blah-blah.example2.info.)"], (test,CNAME) -> RecordSet: [id="5c135cef-4fc4-43b9-83ae-13eef8427fbb"; zoneId="ce1ebac9-bde6-45b8-9dce-d0fe3c4691ae"; name="test"; type="CNAME"; ttl="360"; created="2020-10-15T16:53:22.189Z"; updated="Some(2020-10-15T16:53:22.189Z)"; account="system"; status="Active"; records="List(0.0.0.0.)"; ownerGroupId="None"fqdn="Some(test.example2.info.)"], (example2,NS) -> RecordSet: [id="db8868bd-408d-4120-8208-dc653f028e4a"; zoneId="ce1ebac9-bde6-45b8-9dce-d0fe3c4691ae"; name="example2"; type="NS"; ttl="172800"; created="2020-10-15T16:53:22.185Z"; updated="Some(2020-10-15T16:53:22.185Z)"; account="system"; status="Active"; records="List(ns-47.awsdns-05.com., ns-1419.awsdns-49.org., ns-699.awsdns-23.net., ns-1610.awsdns-09.co.uk.)"; ownerGroupId="None"fqdn="Some(example2.info.)"], (test2,CNAME) -> RecordSet: [id="3da0b978-216a-49db-98e0-70f0b2eb8abb"; zoneId="ce1ebac9-bde6-45b8-9dce-d0fe3c4691ae"; name="test2"; type="CNAME"; ttl="7200"; created="2020-10-15T16:53:22.190Z"; updated="Some(2020-10-15T16:53:22.190Z)"; account="system"; status="Active"; records="List(0.0.0.0.)"; ownerGroupId="None"fqdn="Some(test2.example2.info.)"]))

@pauljamescleary
Copy link
Contributor

Thanks for the update. I know this is verified in the new Route53ApiIntegrationSpec. I’ll check again tomorrow to see what is missing.

@pauljamescleary
Copy link
Contributor

@slandry90 I was finally able to get access to a public hosted zone and found the issue. Will have a PR up for this today, it is a tiny bug

@pauljamescleary
Copy link
Contributor

Opened #1017, I was able to test this against a "live" public hosted zone in Route 53 for connect, load zone, create record, delete record.

pauljamescleary added a commit that referenced this issue Oct 16, 2020
Addresses #1012

Was able to get a "real" public hosted zone connected.  The `Fqdn.merge` was not working properly when merging record name and zone names when they matched.

Added a bunch of unit tests along with the fix.

This was tested against a "real" public hosted zone.  Verified connect, load zone, add record, delete record.
@slandry90
Copy link
Contributor Author

Pulled down the latest and validated. Everything works as expected now! Thanks for the help and quick fixes!

@pauljamescleary
Copy link
Contributor

@slandry90 great, thanks for testing! Took a little while to get a test aws account setup on my end, then it was easy to verify everything. Evidently aws localstack is not quite 1-1 with the real thing

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 a pull request may close this issue.

2 participants