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.
The main bug is a regression which was introduced by commit 8b24c43b382740106474e26dec59e1419ba77306 as fix for Bug: #9559
Created attachment 10184 [details] Current work in progress on top of master
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)?
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.
Created attachment 10202 [details] Current work in progress on top of master
Created attachment 10242 [details] Patches for v4-1-test
Created attachment 10243 [details] Patches for v4-1-test
Created attachment 10244 [details] Patches for v4-0-test
Karolin, please pick this for 4.1.12 Andrew, can you also ack the patch for 4.0?
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.
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);
(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... ;-)
Created attachment 10251 [details] RPC dnsserver patch to correctly search with dNSTombstoned flag
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.
Created attachment 10252 [details] rpc_server patches for master This includes the patch from Amitay.
Comment on attachment 10252 [details] rpc_server patches for master This looks good to me. Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Patches are okay, except the soa patch.
Created attachment 10253 [details] rpc_server patches for v4-1-test
(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 on attachment 10253 [details] rpc_server patches for v4-1-test This has a bug a new patch will follow
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.
(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
Pushed to autobuild-v4-1-test.
Pushed to v4-1-test (included in 4.1.12). Closing out bug report. Thanks!