Add support for dynamic updates (RFC2136) - #31
Conversation
There was a problem hiding this comment.
you really want to use dns.Split or dns.SplitDomainName in these cases.
There was a problem hiding this comment.
Yeah, I only just came across all of the domain label splitting functions earlier for this. Will update them all! Thanks.
In some situations when querying for answers for a given domain, it may not be desireable to resolve the domain to a CNAME if one exists. This is indeed the requirement for dynamic updates, as an rrtype is always explicit.
Conflicts: main.go resolver_test.go
Previously we were creating new records as /net/foo/.A -> value however because there can be multiple records, we should create /net/foo/.A/X where X is a value created by etcd for us.
d18eb08 doesn't function as it was meant to, `Create` is just `Set` with a `prevExist` check
These are actually a badly-reported etcd 'unsupported protocol scheme' error in disguise
...and debug if it fails
when your tests are failing, locks can get left behind and cause you spurious subsequent failures
This unmasks a hidden failure in TestDeleteRecordNoPrerequsites
Multiple records of different types at the same key fails due to the locking approach, despite being a legitimate usecase
Conflicts: resolver_test.go
Conflicts: Makefile resolver.go
Use consistent etcd keys for new records:
Allow unauthenticated update requests
Syncronous update locking
- Clarify that it's remove-name (i.e. all types) - use own etcd namespace and pre-set values - test for deleting something that doesn't exist
This fails because of how we convert RRs to etcd nodes early on: doesn't work for some specialized delete RR forms
- check for existing keys before RR-deletes and inserts - add some extensive testing for insert/update given different etcd layouts - auto-convert old-style keys to directories if needed
Bam! Full basic support and green tests
There was a problem hiding this comment.
I think this might be incorrect – I think i misread the RFC. Having some trouble checking existing implementations (man, bind code is hard to read :-/ ) but I think the "RRSet Exists (Value-Dependent" prereq means that some set of exactly-matching RRs must exist, but permits other records to also exist. I think I might have un-done this in my refactor and @tarnfeld had it right to begin with.
References: Net::DNS, RFC (section 2.4.2)
There was a problem hiding this comment.
The best readable source code confirmation I can find is in Bundy/bind-10.
This seems to imply that, given an RRset exists, then the rdata values already in the DNS need to match exactly and fully the rdata values in the prereq (unless I'm misreading that method), which implies that my implementation here is is correct. (specifically, the check on line 419 indicates that no non-matching rdatas can remain after checking for the prereq ones).
Their tests seem to confirm this (although it's hard to see without reading and mapping out their entire test harness/test data) -- the method should only return true when the prereq contains the exact three rdata values as already existing, not when just one or two are asked for.
I could really do with a second pair of eyes on this :)
There was a problem hiding this comment.
[ Quoting notifications@github.com in "Re: [discodns] Add support for dyna..." ]
minttl, err := strconv.ParseUint(parts[5], 10, 32)if err != nil {return nil, err
+// and type exactly match the given RRs: their data must match, and there must
+// be no more or less RRsThe best readable source code confirmation I can find is in Bundy/bind-10.
This seems to imply that, given an RRset exists, then the rdata values already in the DNS need to match exactly and fully the rdata values in the prereq (unless I'm misreading that method), which implies that my implementation here is is correct. (specifically, the check on line 419 indicates that no non-matching rdatas can remain after checking for the prereq ones).
Their tests seem to confirm this (although it's hard to see without reading and mapping out their entire test harness/test data) -- the method should only return true when the prereq contains the exact three rdata values as already existing, not when just one or two are asked for.
I could really do with a second pair of eyes on this :)
Although it is important to get things right, I think these features of dyn.
updates we not used widely. I.e. most use is to send update to the remote
server, without really caring what's already in the zone.
But of course I'm making this claim without any data what so ever :)
Because SetRcode internally calls SetReply, which always resets the response's opcode to QUERY, we can't then change the opcode to UPDATE, meaning we send an invalid response
4 participants
Implementation of #18. This is still WIP and the commits need cleaning up, but progress is steady.