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

Günter Kukkukk linux at kukkukk.com
Tue Mar 4 20:49:09 MST 2014


Hello Kai,

comments inline

Am 24.02.2014 07:33, schrieb Kai Blin:
> 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?

both patches are in _no_ way related to usual TTL dns lifetime!
So no _DNS_ test _can_ verify the Microsoft specific internal dwTimeStamp ADS storage.
(dnsp_DnssrvRpcRecord)

Sure, after doing some DNS call one could then use LDB queries to verify ADS "dwTimeStamp",
unfortunately i atm feel not able to add those LDB queries to the dns test module... :-(

Anyway - my local stuff is working - and I'm hunting the much more complex scavenging/aging/tombstone
problem.

Cheers, Günter

> 
> ...
> 
>> 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
> 


-- 



More information about the samba-technical mailing list