On Wed, Dec 05, 2012 at 12:39:33AM -0500, Simo Sorce wrote:
On Tue, 2012-12-04 at 21:39 +0100, Jakub Hrozek wrote:
Hi,
this thread obsoletes the previous memberof related threads into a single one to make it easier for the reviewers to apply the patches without having to guess the dependencies. The patches should apply cleanly on origin/master. In addition to bringing all the patches together, this set also addresses some of Simo's concerns, in particular the ghost users are better explained in the big comment blocks above each major memberof operation.
The patches implement the missing operations of the memberof plugin related to ghost users - delete and modify. The mod operation is given some special care because not only we need to propagate the modification to parent groups, but we also need to retain the inherited ghost attributes.
The intent is to fix issues such as #1652. Most patches also include unit tests to make sure we don't break existing memberof plugin functionality.
More information is in the patches themselves or in the commit messages.
The first 5 patches get a tentative ack from me. They look good and they compile fine and a basic smoke test seem to show no issue whatsoever, also make check works as expected.
On the last patch I have a question. I see that you do a number of base searches for each hild of a group in order to source ghost users. But that seem a bit wasteful.
Why don't you simply do a subtree search on the whole tree with (&(objectclass=group)(memberof=<dn of the group you are handling>)) (you might also add a &(ghost=*) ) ?
This would give you all groups that are 'children' of the group you are actually handling with a single search and avoids searching user entries and then immediately discarding them as they have no ghost members.
Simo.
Mostly because memberof is transitive so I thought I might get too many "ghost" duplicates in case the nesting went too deep which would need to be filtered later (using the muop operations).
On the other hand, in the typical case, there would probably *many* more user entries to discard when traversing direct members rather than memberofs, so you might be right..