[PATCH] internal DNS server: add missing timestamps on dyn. records and fix adding new dnsRecords

Kai Blin kai at samba.org
Sun Feb 23 23:33:51 MST 2014


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.

Cheers,
Kai

-- 
Kai Blin
Worldforge developer http://www.worldforge.org/
Wine developer http://wiki.winehq.org/KaiBlin
Samba team member http://www.samba.org/samba/team/


More information about the samba-technical mailing list