linked attributes, DRS and abusing the ldb modules API

[simo]

On Wed, 2009-08-26 at 17:52 +1000, tridge at samba.org wrote:
> Hi Metze,
> 
> I've been looking at supporting linked attributes in the
> repl_meta_data module. The test I've been using is to add/remove a
> couple of users from a global group on a W2K3 system, with DRS pull
> replication happening to a S4 DC.
> 
> As it stands in the tree now, the DRS pull does pull across the modify
> request for the 'member' attribute of the group, and this gets applies
> correctly, but the memberOf linked attributes don't get updated. The
> reason they don't get updated is that the linked_attributes module
> comes before the repl_meta_data module in the module list (as the
> repl_meta_data module gets pulled in as part of the partition module).
> 
> So to make this work, we have a few choices:
> 
>  1) we could reproduce the linked_attributes logic in the
>     repl_meta_data module. I'm guessing that is what your 'todo' in
>     dsdb_extended_replicated_objects_commit() impled?
> 
>  2) we could call into the linked_attributes module from the
>     repl_meta_data module, letting the linked_attributes module do its
>     work as usual when it gets a modify request on an attribute that
>     is linked
> 
>  3) we could change the module ordering

4) instead of doing a ldb_next_request() call you do a normal
ldb_request() call that goes through the whole stack to perform the
modify operation you received.

If we need to skip modules in this special case we could use a control
to signal other modules should ignore this request. IMO this approach
would be a lot less problematic.

> I've written a quick patch that implements (2). What do you think
> about this approach? The patch is below.
> 
> Note in particular the line:
> 
> 		if (ldb_load_modules_list(ldb, module_list, module->next, &ac->module->next) != LDB_SUCCESS) {
> 
> which is an approach I don't think we've used before, but does offer a
> way to re-use modules in a fairly clean fashion. For now it assumes
> the modules don't need initialisation, but that could be fixed.
> 
> The patch works in the sense that the member/memberOf attributes get
> correctly updated by a DRS pull, although we do generate a warning as
> we get some spurious updates to the memberOf linked attribute target,
> which are then discarded (which turns out to do the right thing).
> 
> Cheers, Tridge
> 
> 
> diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> index 5d4c486..cf41b3c 100644
> --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
> @@ -46,6 +46,7 @@
>  #include "librpc/gen_ndr/ndr_drsuapi.h"
>  #include "librpc/gen_ndr/ndr_drsblobs.h"
>  #include "param/param.h"
> +#include "ldb_private.h"

Including ldb_private.h is a violation of the interface, we've gone
through great pains to remove any access to ldb_private.h from all code
except internal ldb code.

If/when we will build samba4 against a standalone ldb we will not have
access to this header.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>