[PATCH] s4-drs: replmd_delete implementation

[Eduardo Lima]

Hi Tridge,

Thanks for the comments. I've almost conclude this part of the work but I
have a few question.

>What you'll need to do is first find the Deleted Objects base DN. You
> should probably add a utility function in dsdb/common/util.c to do
>this, perhaps called dsdb_get_deleted_obects_dn(). Have a look at
>samdb_ntds_settings_dn() for an example. This function would get the
> Deleted Objects DN either by searching for it, or by adding
>"CN=Deleted Objects" to the defaultNamingContext DN from the rootDSE.

I created this utility function that gets the defaultNamingContext and put
"CN=Deleted Objects" in front of the DN. Will this work for every type of
object? (every object in the domain - even if it's in the deepest depth -
should be moved directly to CN=Deleted Objects,DC=w2k8... when deleted?)

>Another aspect of your code that could be fixed is that you currently
>do the ldb_rename() using the top level ldb context. This means that
>all modules above this one will be getting the rename. That may not be
>the right thing to do (depending on whether the modules above need to
>do any rename processing).

>We should probably have a ldb_next_rename() function that uses
>ldb_build_rename_req() to make a rename call only to the lower modules
>in the module stack. It would also be nice to have a ldb_next_modify()
>call that does all the messy work for a modify in a similar
>fashion. That would make the end of your replmd_delete() function
>where you do the modify much simpler.

This is the part that I'm having more difficulty. I still can't understand
well what the modules are and the way that they work in the Samba source.  I
saw that ldb_module structure has *prev and *next fields and modules might
be linked together but why would all the modules be renamed if I use the
ldb_rename with the top level ldb context? When I do a bin/ldbdel operation,
what is the execution's flow that the modules follow?

Is there any document or presentation that I could read to better understand
it? Or could you point any part of the code that I could debug to see the
modules' flow in a easier way?

Thanks a lot.

--
Eduardo Lima
Sent from Campinas, SP, Brazil

On Sat, Nov 14, 2009 at 00:08, <tridge at samba.org> wrote:

> Hi Eduardo,
>
>  > This is the replmd_delete implementation patch.
>
> thanks for the patch!
>
>  > The DN of the deleted user is modified to the windows DN's format, the
> user
>  > is moved to Deleted Objects, isDeleted=TRUE and LastKnownParent fields
> were
>  > created.
>
> Currently your patch doesn't always move the object into the "Deleted
> Objects" container. It takes the current DN, finds its parent, then
> finds the parent of that, then adds "CN=Deleted Objects". That works
> for users in the CN=Users container, but won't work for any objects
> that are at a different depth.
>
> What you'll need to do is first find the Deleted Objects base DN. You
> should probably add a utility function in dsdb/common/util.c to do
> this, perhaps called dsdb_get_deleted_obects_dn(). Have a look at
> samdb_ntds_settings_dn() for an example. This function would get the
> Deleted Objects DN either by searching for it, or by adding
> "CN=Deleted Objects" to the defaultNamingContext DN from the rootDSE.
>
> Your code should also have a few more error messages when something
> goes wrong. For example, when you call ldb_rename() you should print a
> error messaging saying that something went wrong if it did. For
> example:
>
> if (ret != LDB_SUCCESS) {
>   DEBUG(0,__location__ ": Failed to rename object from '%s' to '%s'\n",
>                         ldb_dn_get_linearized(old_dn),
>                         ldb_dn_get_linearized(new_dn));
>   goto done;
> }
>
> Please also try to follow the existing coding style. For example, you
> put a { at the end of the replmd_delete() declaration, whereas other
> samba code puts it on the next line.
>
>  > Doing a diff between Windows and Samba deleted objects it shows that
> Samba
>  > still has some fields that does not appear in Windows. I tried to remove
>  > them using ldb_msg_remove_attr but they weren't removed. I need to fix
> this
>  > later.
>
> your code is doing a ldb modify, so in the modify you would need to
> list them as LDB_FLAG_MOD_DELETE. So you actually should use a
> ldb_msg_add_*() call, but sets the element flags to mark it as an
> element to delete.
>
>  > Tridge said that each deleted object needs to be marked with the time
> that
>  > deletion happened but I could not find this field in the Windows'
> object.
>  > Does anyone know how I can see this field?
>
> It probably is just the whenChanged field, but also have a look at the
> replPropertyMeta record and compare with what Windows does (the
> --show-binary flag to ldbsearch will be useful for that).
>
> Another aspect of your code that could be fixed is that you currently
> do the ldb_rename() using the top level ldb context. This means that
> all modules above this one will be getting the rename. That may not be
> the right thing to do (depending on whether the modules above need to
> do any rename processing).
>
> We should probably have a ldb_next_rename() function that uses
> ldb_build_rename_req() to make a rename call only to the lower modules
> in the module stack. It would also be nice to have a ldb_next_modify()
> call that does all the messy work for a modify in a similar
> fashion. That would make the end of your replmd_delete() function
> where you do the modify much simpler.
>
> If you want to work on that, then a separate patch that adds these
> helper functions would be best.
>
> Cheers, Tridge
>