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

[Martin Kosek]

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


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.

Martin