Bug 10749 - Support for DNS_TYPE_TOMBSTONE records
Summary: Support for DNS_TYPE_TOMBSTONE records
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: DNS server (internal) (show other bugs)
Version: 4.1.4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on: 9559 9831 10751
Blocks: 10077 10466 10812
  Show dependency treegraph
 
Reported: 2014-07-29 11:22 UTC by Stefan Metzmacher
Modified: 2014-09-11 02:52 UTC (History)
5 users (show)

See Also:


Attachments
Current work in progress on top of master (82.34 KB, patch)
2014-08-07 13:52 UTC, Stefan Metzmacher
no flags Details
Current work in progress on top of master (83.00 KB, patch)
2014-08-18 20:59 UTC, Stefan Metzmacher
no flags Details
Patches for v4-1-test (106.96 KB, patch)
2014-09-01 09:29 UTC, Stefan Metzmacher
no flags Details
Patches for v4-1-test (110.45 KB, patch)
2014-09-01 09:32 UTC, Stefan Metzmacher
abartlet: review+
Details
Patches for v4-0-test (112.72 KB, patch)
2014-09-01 10:28 UTC, Stefan Metzmacher
no flags Details
Patches for v4-1-test (110.46 KB, patch)
2014-09-02 13:16 UTC, Stefan Metzmacher
metze: review+
abartlet: review+
Details
RPC dnsserver patch to correctly search with dNSTombstoned flag (2.75 KB, patch)
2014-09-04 02:26 UTC, Amitay Isaacs
no flags Details
rpc_server patches for master (7.36 KB, patch)
2014-09-04 05:20 UTC, Stefan Metzmacher
abartlet: review+
Details
rpc_server patches for v4-1-test (5.24 KB, patch)
2014-09-04 06:12 UTC, Stefan Metzmacher
amitay: review+
Details
Patches for v4-1-test (depends on bug 9831 and bug 10751) (117.12 KB, patch)
2014-09-04 09:18 UTC, Stefan Metzmacher
amitay: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Metzmacher 2014-07-29 11:22:21 UTC
Instead of deleting the whole LDAP object of type dnsNode,
if there're no more dnsRecord attribute values remaining,
we should add a dnsRecord with DNS_TYPE_TOMBSTONE
and set dnsTombstoned=TRUE.

If the record gets updated the DNS_TYPE_TOMBSTONE gets deleted
again and dnsTombstoned=FALSE is stored.

This will reduce the amount of deleted LDAP objects.
Comment 1 Stefan Metzmacher 2014-08-07 13:09:49 UTC
The main bug is a regression which was introduced by commit 8b24c43b382740106474e26dec59e1419ba77306 as fix for Bug: #9559
Comment 2 Stefan Metzmacher 2014-08-07 13:52:17 UTC
Created attachment 10184 [details]
Current work in progress on top of master
Comment 3 Andrew Bartlett 2014-08-08 04:21:50 UTC
Just a quick note to say this is an amazing set of patches, and makes our DNS code a much better place.  

Should we have some unit tests for the new common lookup code (given how few unit tests we have of DNS, and the difficultly testing the DLZ module)?
Comment 4 Kai Blin 2014-08-08 08:20:45 UTC
I'd argue we don't have any DNS unit tests, just a bunch of integration tests. Unit tests for this API would be highly desirable, though.
Comment 5 Stefan Metzmacher 2014-08-18 20:59:19 UTC
Created attachment 10202 [details]
Current work in progress on top of master
Comment 6 Stefan Metzmacher 2014-09-01 09:29:27 UTC
Created attachment 10242 [details]
Patches for v4-1-test
Comment 7 Stefan Metzmacher 2014-09-01 09:32:21 UTC
Created attachment 10243 [details]
Patches for v4-1-test
Comment 8 Stefan Metzmacher 2014-09-01 10:28:28 UTC
Created attachment 10244 [details]
Patches for v4-0-test
Comment 9 Stefan Metzmacher 2014-09-02 06:11:57 UTC
Karolin, please pick this for 4.1.12

Andrew, can you also ack the patch for 4.0?
Comment 10 Amitay Isaacs 2014-09-02 06:22:02 UTC
The changes related to tombstone attribute to dns server need to reflected in the RPC dnsserver code.  Otherwise, we will have inconsistency between RPC server and DNS server.

I have started making the changes, however they are not yet complete.
Comment 11 Stefan Metzmacher 2014-09-02 13:16:26 UTC
Created attachment 10248 [details]
Patches for v4-1-test

Updated patch that compiles.

This is the only difference:

-+      status = gensec_update(gensec_client_context, tctx, server_to_client, &client_to_server);
++      status = gensec_update(gensec_client_context, tctx, tctx->ev, server_to_client, &client_to_server);
Comment 12 Karolin Seeger 2014-09-03 18:10:38 UTC
(In reply to comment #11)
> Created attachment 10248 [details]
> Patches for v4-1-test
> 
> Updated patch that compiles.
> 
> This is the only difference:
> 
> -+      status = gensec_update(gensec_client_context, tctx, server_to_client,
> &client_to_server);
> ++      status = gensec_update(gensec_client_context, tctx, tctx->ev,
> server_to_client, &client_to_server);

Metze told me that this is an important bugfix that should be included in the next releases. Could someone please ack the current patchsets? Thanks!
4.1 is already frozen, but if they will be acked until tomorrow... ;-)
Comment 13 Amitay Isaacs 2014-09-04 02:26:42 UTC
Created attachment 10251 [details]
RPC dnsserver patch to correctly search with dNSTombstoned flag
Comment 14 Amitay Isaacs 2014-09-04 02:26:58 UTC
As requested by Andrew, here are some more comments on inconsistencies between DNS server (internal, bind) and RPC dnsserver after these patches are introduced.

- RPC dnsserver does not check dNSTombstoned flag yet, so the search results would also return deleted entries.  (Patch for this is attached)

- Any operations to add an entry which is tombstoned, will add dns record without resetting the dNSTombonsted flag.  This will be just wrong and introduce database inconsistencies. (I am working on this patch)

- Delete operation is not affected, since DNS server considers entries without any dns records as tombstoned.
Comment 15 Stefan Metzmacher 2014-09-04 05:20:11 UTC
Created attachment 10252 [details]
rpc_server patches for master

This includes the patch from Amitay.
Comment 16 Andrew Bartlett 2014-09-04 05:27:40 UTC
Comment on attachment 10252 [details]
rpc_server patches for master

This looks good to me.
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Comment 17 Amitay Isaacs 2014-09-04 06:03:58 UTC
Patches are okay, except the soa patch.
Comment 18 Stefan Metzmacher 2014-09-04 06:12:41 UTC
Created attachment 10253 [details]
rpc_server patches for v4-1-test
Comment 19 Stefan Metzmacher 2014-09-04 06:30:20 UTC
(In reply to comment #18)
> Created attachment 10253 [details]
> rpc_server patches for v4-1-test

Karolin, can you please cherry-pick -x the master patches
once they're through autobuild. I'll post the commit
hashes here...
Comment 20 Stefan Metzmacher 2014-09-04 07:16:07 UTC
Comment on attachment 10253 [details]
rpc_server patches for v4-1-test

This has a bug a new patch will follow
Comment 21 Stefan Metzmacher 2014-09-04 09:18:31 UTC
Created attachment 10255 [details]
Patches for v4-1-test (depends on bug 9831 and bug 10751)

Karolin, you need to apply the patches for #9831 and #10751 before.
Comment 22 Stefan Metzmacher 2014-09-04 15:15:31 UTC
(In reply to comment #21)
> Created attachment 10255 [details]
> Patches for v4-1-test (depends on bug 9831 and bug 10751)
> 
> Karolin, you need to apply the patches for #9831 and #10751 before.


Karolin, please add cherry-pick information for

2c342e488dcd3cef465a1b376bb22bf495f6832b s4-rpc: dnsserver: handle updates of tombstoned dnsNode objects

6f2862e76608862bb4142a86f36c8506114bf6c7 s4-rpc: dnsserver: Do not search for deleted DNS entries
Comment 23 Karolin Seeger 2014-09-05 18:32:25 UTC
Pushed to autobuild-v4-1-test.
Comment 24 Karolin Seeger 2014-09-08 18:55:24 UTC
Pushed to v4-1-test (included in 4.1.12).
Closing out bug report.

Thanks!