[Freeipa-devel] [PATCH] 065 Replace only if old and new have nothing in common

[Jakub Hrozek]

On Wed, Feb 23, 2011 at 12:36:06PM -0500, Rob Crittenden wrote:
> Jakub Hrozek wrote:
> >-----BEGIN PGP SIGNED MESSAGE-----
> >Hash: SHA1
> >
> >On 02/23/2011 04:47 PM, Rob Crittenden wrote:
> >>Jakub Hrozek wrote:
> >>>Replace only if old and new have nothing in common
> >>>
> >>
> >>This has problems when removing the last member. There is no adds, rems
> >>has a single value (the member being removed). The intersection is 0 so
> >>force_replace gets set to True and nothing ends up getting done.
> >>
> >>I added a len(v)>  0 to this conditional and it seems to work. I also
> >>added a small test case based on Endi's initial report. I'm getting a
> >>100% test pass rate.
> >>
> >>rob
> >
> >I hit one more problem with the patch, although I'm not entirely sure
> >how is that possible - when a user is renamed, his memberof becomes
> >indirect memberof:
> >
> ># ipa user-mod --rename test2 test
> >- --------------------
> >Modified user "test"
> >- --------------------
> >   User login: test2
> >   First name: Test
> >   Last name: User
> >   Home directory: /home/test
> >   Login shell: /bin/sh
> >   Account disabled: False
> >   Indirect Member of group: ipausers
> 
> I think this is another timing issue with 389-ds postop plugins,
> this time the referential integrity plugin. I don't think this is
> related to this change.
> 
> We start with:
> 
> dn: uid=test, ...
> uid: test
> memberOf: ipausers
> 
> dn: cn=ipausers, ...
> cn: ipausers
> member: uid=test,...
> 
> When we we do the rename we immediately end up with:
> 
> dn: uid=test2, ..
> uid: test2
> memberOf: ipausers
> 
> dn: cn=ipausers, ...
> cn: ipausers
> member: uid=test, ...
> 
> We determine indirect membership by comparing the user's memberOf
> with the results of a query for member=uid=test2
> 
> If the refint plugin hasn't updated the ipausers group by the time
> we do the query the user will appear to be an indirect member.
> 
> rob

OK, you're probably right, I can't reproduce the issue anymore.

This patch has an ACK from me. Since this is a very low-level change
at a late stage, I have asked Martin to take a second look.

    Jakub