[Freeipa-devel] [PATCH] 993 disable UPG for migration

Rob Crittenden rcritten at redhat.com
Mon Apr 2 19:18:36 UTC 2012 

Martin Kosek wrote:
> On Fri, 2012-03-30 at 09:05 -0400, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Thu, 2012-03-29 at 11:27 -0400, Rob Crittenden wrote:
>>>> Martin Kosek wrote:
>>>>> On Wed, 2012-03-28 at 17:28 -0400, Rob Crittenden wrote:
>>>>>> Martin Kosek wrote:
>>>>>>> On Thu, 2012-03-22 at 15:21 -0400, Rob Crittenden wrote:
>>>>>>>> We don't want to create private groups automatically for migrated users,
>>>>>>>> there could be namespace overlap (and race conditions prevent us from
>>>>>>>> trying to check in advance).
>>>>>>>>
>>>>>>>> Check the sanity of groups in general, warn if the group for the
>>>>>>>> gidnumber doesn't exist at least on the remote server.
>>>>>>>>
>>>>>>>> Fill in the user's shell as well.
>>>>>>>>
>>>>>>>> This will migrate users that are non-POSIX on the remote server.
>>>>>>>>
>>>>>>>> rob
>>>>>>>
>>>>>>> This patch fixes the problem of creating UPGs for migrated users, but
>>>>>>> there are several parts which confused me.
>>>>>>>
>>>>>>> 1) Confusing defaults
>>>>>>>
>>>>>>> +    if 'def_group_gid' in ctx:
>>>>>>> +        entry_attrs.setdefault('gidnumber', ctx['def_group_gid'])
>>>>>>>
>>>>>>> This statement seems redundant, because the account either is posix and
>>>>>>> has both uidnumber and gidnumber or it is non-posix and does not have
>>>>>>> any.
>>>>>>>
>>>>>>> Now, we don't touch gidNumber for posix user, and non-posix users are
>>>>>>> made posix with this statement:
>>>>>>>
>>>>>>> +    # migrated user is not already POSIX, make it so
>>>>>>> +    if 'uidnumber' not in entry_attrs:
>>>>>>> +        entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999]
>>>>>>>
>>>>>>>
>>>>>>> 2) Missing UPG
>>>>>>> When UPG is disabled, the following statement will result in a user with
>>>>>>> a GID pointing to non-existent group.
>>>>>>>
>>>>>>> +    # migrated user is not already POSIX, make it so
>>>>>>> +    if 'uidnumber' not in entry_attrs:
>>>>>>> +        entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999]
>>>>>>>
>>>>>>> We may want to run ldap.has_upg() and report a add this user to "failed
>>>>>>> users" with appropriate error.
>>>>>>>
>>>>>>> 3) Check for GID
>>>>>>> The patch implements a check if a group with the gidNumber exists on a
>>>>>>> remote LDAP server and the result is a warning:
>>>>>>>
>>>>>>> -            (g_dn, g_attrs) = ldap.get_entry(ctx['def_group_dn'], ['gidnumber'])
>>>>>>> +            (remote_dn, remote_entry) = ds_ldap.find_entry_by_attr(
>>>>>>> +                'gidnumber', entry_attrs['gidnumber'][0], 'posixgroup', [''],
>>>>>>> +                 search_bases['group']
>>>>>>> +            )
>>>>>>>
>>>>>>> I have few (minor-ish) questions there:
>>>>>>> a) Is the warning in a apache log enough? Maybe it should be included in
>>>>>>> migrate-ds output.
>>>>>>> b) This will be a one more remote LDAP call for every user, we may want
>>>>>>> to optimize it with something like that:
>>>>>>>
>>>>>>> valid_gids = []
>>>>>>> if user.gidnumber not in valid_gids:
>>>>>>>        run the check in remote LDAP
>>>>>>>        valid_gids.append(user.gidnumber)
>>>>>>>
>>>>>>> That would save us 999 LDAP queries for 1000 migrated with the same
>>>>>>> primary group.
>>>>>>>
>>>>>>> 4) Extraneous Warning:
>>>>>>> When non-posix user is migrated and thus we make it a posix user, we
>>>>>>> still produce a warning for non-existent group:
>>>>>>>
>>>>>>> [Fri Mar 23 04:21:36 2012] [error] ipa: WARNING: Migrated user's GID
>>>>>>> number 999 does not point to a known group.
>>>>>>>
>>>>>>> 5) Extraneous LDAP call
>>>>>>>
>>>>>>> Isn't the following call to LDAP to get a description redundant? We
>>>>>>> already have the description in entry_attrs.
>>>>>>>
>>>>>>> +    (dn, desc_attr) = ldap.get_entry(dn, ['description'])
>>>>>>> +    entry_attrs.update(desc_attr)
>>>>>>> +    if 'description' in entry_attrs and NO_UPG_MAGIC in
>>>>>>> entry_attrs['description']:
>>>>>>>
>>>>>>>
>>>>>>> Martin
>>>>>>>
>>>>>>
>>>>>> I think this covers your concerns.
>>>>>>
>>>>>> I can't do anything but log warnings at this point in order to maintain
>>>>>> backwards compatibility. I looked into returning a warning entry and it
>>>>>> blew up in validate_output() on older clients.
>>>>>>
>>>>>> rob
>>>>>>
>>>>>
>>>>> This patch is much better and covers my previous concerns. I just find
>>>>> an issue with UPG. It is not created for non-posix users when UPGs are
>>>>> enabled:
>>>>>
>>>>> # echo "Secret123" | ipa migrate-ds ldap://ldap.example.com
>>>>> --with-compat --base-dn="dc=greyoak,dc=com"
>>>>> -----------
>>>>> migrate-ds:
>>>>> -----------
>>>>> Migrated:
>>>>>      user: darcee_leeson, ayaz_kreiger, mnonposix, mollee_weisenberg
>>>>>      group: ipagroup
>>>>> Failed user:
>>>>> Failed group:
>>>>> ----------
>>>>> Passwords have been migrated in pre-hashed format.
>>>>> IPA is unable to generate Kerberos keys unless provided
>>>>> with clear text passwords. All migrated users need to
>>>>> login at https://your.domain/ipa/migration/ before they
>>>>> can use their Kerberos accounts.
>>>>>
>>>>> # ipa user-show mnonposix
>>>>>      User login: mnonposix
>>>>>      First name: Mister
>>>>>      Last name: Nonposix
>>>>>      Home directory: /home/mnonposix
>>>>>      Login shell: /bin/sh
>>>>>      UID: 328000195
>>>>>      GID: 328000195
>>>>>      Org. Unit: Product Testing
>>>>>      Job Title: Test User
>>>>>      Account disabled: False
>>>>>      Password: True
>>>>>      Member of groups: ipausers
>>>>>      Kerberos keys available: False
>>>>>
>>>>> # ipa group-show mnonposix
>>>>> ipa: ERROR: mnonposix: group not found
>>>>
>>>> Yes, I was always disabling UPG. I now allow it when migrating a
>>>> non-POSIX user.
>>>>
>>>>> I am also thinking if we need to ask if UPG is enabled for every
>>>>> migrated user - every ldap.has_upg() call means one query to host LDAP.
>>>>> Maybe we should ask just in the beginning and store the setting in
>>>>> "ctx['upg_enabled']. I don't think that anyone needs to switch UPG
>>>>> status during migration process.
>>>>
>>>> I agree, nice optimization.
>>>>
>>>> rob
>>>
>>> This patch is OK, everything worked as expected. We just now need to
>>> specify if we want a special option/flag to enable non-posix ->   posix
>>> user conversion. If not, then ACK.
>>>
>>> Martin
>>>
>>
>> Simo and I had a brief discussion about this in IRC and I'm going to
>> punt on non-POSIX user conversion for now. There could be specific
>> reasons for this, like having a shared user (nss_ldap) or some sort of
>> system user that they don't to be a full POSIX user.
>>
>> Adding a new option this late in 2.2 doesn't seem like a good idea so
>> I'll patch it to simply fail non-POSIX users for now. I've opened a
>> ticket to optionally allow them to be converted.
>>
>> rob
>
> Ok. Your patch 993 is still useful and fixes the creation of UPGs, we
> just need to remove nonposix->posix conversion and make sure non-posix
> users are migrated successfully as non-posix users.
>
> Martin
>

I decided to not migrate non-POSIX users at all. They are a-typical in 
IPA and won't show at all using our tools so I don't see the point in 
migrating them. non-POSIX users in IPA are typically stored in 
cn=sysconfig rather than cn=users.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-993-4-migration.patch
Type: text/x-patch
Size: 10043 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120402/05885893/attachment.bin>

More information about the Freeipa-devel mailing list