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

[Rob Crittenden]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-993-3-migration.patch
Type: text/x-patch
Size: 10424 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120329/f6c43087/attachment.bin>