[PATCH] internal DNS server: add missing timestamps on dyn. records and fix adding new dnsRecords
Andrew Bartlett
abartlet at samba.org
Mon Feb 24 14:27:05 MST 2014
On Mon, 2014-02-24 at 07:33 +0100, Kai Blin wrote:
> On 2014-02-22 07:00, Günter Kukkukk wrote:
>
> Hiu Günter,
>
> >>> the missing timestamp on dyn. DNS records should be fixed ASAP - otherwise
> >>> those records are treated as STATIC DNS entries, which never get scavenged/aged!
> >>
> >> You spent some time digging there. Can you add a test that'll pass against the MS AD DNS server for this, so we can make sure not to break it by
> >> accident?
> >>
> >
> > i think, this time stamp patch is in _no_ way invasive at all - it shouldn't do any harm.
>
> I wasn't afraid that this patch breaks anything, but that's not the
> point of a test necessarily. As far as I understand, test-driven
> development at this stage looks like:
>
> 1. find a bug
> 2. write a test triggering the bug, the test fails
> 3. fix the bug
> 4. now the test passes
>
> The main idea is to never run into the same bug again, not to prove that
> the code written in 3 doesn't break anything.
>
> > All dns tools i used so far put a time stamp on (normal) dyn. dns update records.
> > There's MS docu around that only "drastic" tools also allow aging of _static_ dns records.
> >
> > AFAIK - the internal dns server is the only one not using time stamps:
> > http://picpaste.com/pics/samba-dns-Skec94fz.1393042765.png
> >
> > The samba bind DLZ module does the same for good reasons - but yes - we atm have
> > no tests around. Which would be a really MAJOR test case to simulate what Microsoft
> > is doing here.... with scavenging/aging/tombstone handling.
> > This patch alone can't be simulated at all.
>
> Right, but the "add a dynamic DNS entry, verify it has a >0 TTL" part
> should be testable, right?
>
> ...
>
> > THIS patch just "reverts the complete dnsNode deletion" - BUT in addition fixes the bug
> > that new dnsRecords could not be added to an empty dnsNode.
> >
> > At least in this area, the internal dns server now behaves the same as the DLZ module.
>
> Right, but is this the valid behaviour?
>
> > make test TESTS=samba.tests.dns
> >
> > just ONLY confirms that this patch does not break "what we have tested til today".
>
> Right, and if we keep adding new code without tests, we'll lose the
> confidence of being able to say this.
>
> > Anyway - i leave it up to you whether you push these 2 patches...
>
> As much as I hate to block on this one, I don't really feel comfortable
> to accept these patches without tests. It has less to do with these
> specific patches and much more to do with a decision on the principle.
Kai,
Thanks for expressing this so well.
Andrew Bartlett
--
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list