[PATCH] s4-drs: replmd_delete implementation

[tridge at samba.org]

Hi Eduardo,

 > Sorry for the late answer. The two last weeks were very difficult here. Now
 > I'm working hard on this task to delivery it faster.

no problem! I've also been working in a closely related area, which
resulted in Andrew Bartlett and I merging most of your code into my
drs-linked-attribs tree today.

The basic framework of the replmd_delete() code you had was
reasonable, but it turned out that your work closely intersected with
the work that Andrew and I have been concentrating on. We've been
working on fixing repl_meta_data.c to cope with the proper storage of
linked attribute meta data. We need this in order to be able to
properly replicate with AD domains that are in functional level of
W2003 and above. In these higher functional levels, linked attributes
are stored and replicated with a per-value granularity, which is quite
different from the granularity of replication for other attributes.

This intersected with your work, as we needed to have special handling
for linked attributes during delete operations.

What I suggest you now do is checkout my drs-linked-attribs tree, and
have a look at the changes that Andrew and I have been making. You can
grab that tree like this:

 git remote add tridge git://git.samba.org/tridge/samba.git
 git fetch tridge
 git checkout -b drs-linked-attribs tridge/drs-linked-attribs -t

If you then do a "git log" you'll see that most of your patches are
now in my tree, although they had to be modified quite a lot to
integrate them with the work that Andrew and I had been doing.

Specific changes include:

 - we imported your dsdb_find_nc_root() function, but changed it to
   use the existing ldb_dn_compare() function, which meant you don't
   need the function that counts the number of commas in a DN

 - we added a function dsdb_wellknown_dn() which works out a wellknown
   DN (such as the Deleted Objects DN) from its wellknown GUID. This
   function uses a trick of doing a base search with a specially
   formatted WKGUID DN, as a way to efficiently find the wellknown
   DN. This trick is the equivalent to a command line ldbsearch like
   this:

     bin/ldbsearch -s base -b '<WKGUID=18E2EA80684F11D2B9AA00C04F79F805,DC=bludom,DC=tridgell,DC=net>' dn --show-deleted

 - we added dsdb_find_nc_root() based on your code, but changed to use
   the dsdb_wellknown_dn() function

 - we added the dsdb_module_modify() and dsdb_module_rename()
   functions, as helper functions to do a rename or modify below the
   current position in the ldb module stack

We are still working to make the drs-linked-attribs tree pass all
tests. We need to do more work in the linked_attributes module, and in
the replmd_rename call in repl_meta_data. We also need to teach the
DRS getncchanges code (both client and server) to honor the extended
DN components we are using to store the meta data associated with
linked attributes. Until we get all that done we won't be pushing the
repl_meta_data changes to master tree, although I will push the new
utility functions.

 > >From what I understood, now I need to find de Deleted Objects container for
 > de root of NC using the wellKnownObjects, right? (Currently I'm just
 > inserting CN=Deleted Objects to the root of NC). But I can't find a good
 > example to understand how to use a wellKnownObject.

hopefully the dsdb_wellknown_dn() function we've added will show you
how to handle these wellknown DNs. It is quite tricky, but also
efficient.

 > Right before when I return the deleted_dn (in function
 > dsdb_get_deleted_objects_dn) and base_dn (in function dsdb_find_nc_root) I
 > do a talloc_free(tmp_ctx). Is this right? Do I need to pass a tmp_ctx
 > created in the replmd_delete to the other functions (I think I'll lose the
 > returned DN if I don't do this)

Please have a look at how Andrew and I have handled memory contexts in
the new functions we have added based on your code.

 > I also changed function samldb_modify() in samldb.c to test if there is a
 > relax control. This is currently not working because I could not make
 > ldb_modify_ctrl work:
 > 
 > ctrl = talloc(tmp_ctx, struct ldb_control);
 > ctrl->oid = LDB_CONTROL_RELAX_OID;
 > ctrl->critical = 1;
 > 
 > ret = ldb_modify_ctrl(ldb, msg, &ctrl);

Andrew and I decided that we really needed helper functions to make
this easier to code. So Andrew added a set of helpers in
dsdb/samdb/ldb_modules/util.c which allow you to do ldb operations
with controls by just passing a set of flags. So you can now do things
like this:

	ret = dsdb_module_search_dn(module, msg, &res, msg->dn, NULL,
				    DSDB_SEARCH_SHOW_DELETED |
				    DSDB_SEARCH_REVEAL_INTERNALS |
				    DSDB_SEARCH_SHOW_DN_IN_STORAGE_FORMAT);

and you'll end up with 3 controls corresponding to the 3 flags in the
above. This makes dealing with controls like the relax control much
easier in our modules.

It turned out that we actually didn't need the relax control in the
replmd_delete() any more, as we changed from using ldb_modify() to
using dsdb_module_modify(). This means we are now only invoking the
modules below the repl_meta_data module in the module stack when doing
the modify, and the module that was causing you problems with the
samAccountType attribute is above repl_meta_data in the module
stack. So we ended up not needing the relax control.

 > You can also modify the functions I've created so far.

That's what we ended up doing!

Sorry this all took so long. Andrew and I have been working hard on
this for the much of the last week, using pair programming. Most of
the complexity wasn't in your replmd_delete() code, but was instead on
the new linked attributes code, it was just that the two sets of
changes overlapped quite a lot in repl_meta_data.c

Cheers, Tridge