[Freeipa-devel] [PATCH] 991/992 fix migration issues

[Martin Kosek]

On Thu, 2012-03-22 at 15:17 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Tue, 2012-03-20 at 22:58 -0400, Rob Crittenden wrote:
> >> Fix a couple of issues found with migration. I made a second patch just
> >> to keep things separate even though its just a one-liner.
> >>
> >> 991 fixes a problem where we have attributes which point to other
> >> entries and these weren't being migrated. This is things like secretary
> >> and manager. This was actually causing things to blow up badly.
> >>
> >> 992 makes the primary key lower-case to match the rest of IPA.
> >>
> >> I've attached an LDIF with a couple of users to demonstrate the fix.
> >>
> >> rob
> >
> >
> > 1) This is not very Pythonic:
> > +            for ind in xrange(len(entry_attrs[attr])):
> > +                value = entry_attrs[attr][ind]
> >
> > This would be better:
> > +            for ind,value in enumerate(entry_attrs[attr]):
> >
> > 2) 992 lowers uid of users group, but you leave updated DNs
> > un-normalized, we may want to lower them as well:
> >
> > # ayaz_kreiger, users, accounts, idm.lab.bos.redhat.com
> > dn: uid=ayaz_kreiger,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=co
> > m
> > uid: ayaz_kreiger
> > ...
> > manager: uid=Mollee_Weisenberg,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=re
> >   dhat,dc=com
> > manager: cn=Doesnot Exist,ou=People,dc=greyoak,dc=com
> > ...
> >
> > 3) We still crash if the DN does not exist (see above) and thus we don't
> > normalize it:
> > # ipa user-show ayaz_kreiger --allipa: ERROR: an internal error has
> > occurred
> >
> > I think we should use the change that came up when I was reviewing
> > Petr3's sudo patch:
> > diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
> > index cf5d8d20eb27a0342f064086e0ee9d85c78c5bae..8f03a09827bd0ee6a08e594617ad8d4dff2b467b 100644
> > --- a/ipalib/plugins/baseldap.py
> > +++ b/ipalib/plugins/baseldap.py
> > @@ -487,7 +487,15 @@ class LDAPObject(Object):
> >               pass
> >           # DN object assures we're returning a decoded (unescaped) value
> >           dn = DN(dn)
> > -        return dn[self.primary_key.name]
> > +        try:
> > +            return dn[self.primary_key.name]
> > +        except KeyError:
> > +            # The primary key is not in the DN.
> > +            # This shouldn't happen, but we don't want a "show" command to
> > +            # crash.
> > +            # Just return the entire DN, it's all we have if the entry
> > +            # doesn't exist
> > +            return unicode(dn)
> >
> > 4) (minor) In function get_dn_syntax:
> > - if obj is None, the function will return None ->  contradicts with the help
> > - I would name the function "is_dn_syntax" since we don't return DN syntax
> >
> > Martin
> >
> 
> For 4 I added a None check. I only want this to return True/False. I 
> don't want to get too clever and try to see if the value is a dn if we 
> don't have schema for it.
> 
> Updated patch atached.
> 
> rob

ACK. Pushed to master, ipa-2-2.

Martin