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

[Pavel Zuna]

On 03/02/2011 08:50 PM, Jakub Hrozek wrote:
> 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
>

Tested a few corner cases and it seems to be cool. ACK from me too.

Pavel