[Freeipa-devel] [PATCH] 817 Add option to wait for values

Tue Jul 19 11:15:31 UTC 2011

On Tue, 2011-07-19 at 00:14 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Sun, 2011-07-17 at 17:42 -0400, Rob Crittenden wrote:
> >> Rob Crittenden wrote:
> >>> Martin Kosek wrote:
> >>>> On Tue, 2011-07-05 at 13:41 -0400, Rob Crittenden wrote:
> >>>>> Rob Crittenden wrote:
> >>>>>> Rob Crittenden wrote:
> >>>>>>> 389-ds postop plugins, such as the managed entry and memberof plugins,
> >>>>>>> add values after the data has been returned to the client. In the case
> >>>>>>> of the managed entry plugin this affects the parent entry as well
> >>>>>>> (adds
> >>>>>>> an objectclass value).
> >>>>>>>
> >>>>>>> This wreaks havoc on our tests as the values don't match what we
> >>>>>>> expect.
> >>>>>>>
> >>>>>>> The solution is to wait for the postop plugins to finish their work,
> >>>>>>> then return. I've added this as an option. The downside is it is going
> >>>>>>> to naturally slow things down, so it is off by default.
> >>>>>>>
> >>>>>>> It is currently only used in the hostgroup plugin.
> >>>>>>>
> >>>>>>> The option is wait_for_attr. Add this to ~/.ipa/default.conf and
> >>>>>>> set it
> >>>>>>> to True and all the current tests will pass (assuming you apply
> >>>>>>> patches
> >>>>>>> 814-816 as well).
> >>>>>>>
> >>>>>>> So now we won't have any excuses for missing test failures in the unit
> >>>>>>> tests...
> >>>>>>>
> >>>>>>> rob
> >>>>>>
> >>>>>> Bah, found a small problem. Self-NACK.
> >>>>>>
> >>>>>> rob
> >>>>>
> >>>>> Updated patch attached.
> >>>>>
> >>>>> Note that I don't think there is a way for us to handle things like
> >>>>> memberof_indirect. We wouldn't know to wait.
> >>>>>
> >>>>> rob
> >>>>
> >>>> Works fine for the hostgroup entry. It's good it can be switched on/off.
> >>>>
> >>>> But what about other managed entries, like user entry? Would it make
> >>>> sense to add a wait here too? Or maybe something systematic to baseldap
> >>>> so that we wouldn't have to implement this wait to every managed entry.
> >>>>
> >>>> Martin
> >>>>
> >>>
> >>> I can certainly add it to users to check for managed groups. Making it
> >>> generic would be difficult because some are conditional (such as users).
> >>>
> >>> rob
> >>
> >> Added support for managed users as well.
> >>
> >> rob
> >
> > Waiting for managed users work too. However, I have just noticed that
> > the entire solution works only partially.
> >
> > It waits for mepOriginEntry objectclass, but it doesn't add the new LDAP
> > attributes "mepmanagedentry" and "memberof" to the<command>-add result:
> >
> > # ipa hostgroup-add hgroup3 --desc=foo --all --raw
> > -------------------------
> > Added hostgroup "hgroup3"
> > -------------------------
> >    dn: cn=hgroup3,cn=hostgroups,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> >    cn: hgroup3
> >    description: foo
> >    ipauniqueid: 20d1b8e4-b114-11e0-ab28-00163e0ed706
> >    objectclass: ipaobject
> >    objectclass: ipahostgroup
> >    objectclass: nestedGroup
> >    objectclass: groupOfNames
> >    objectclass: top
> >    objectclass: mepOriginEntry
> > # ipa hostgroup-show hgroup3 --all --raw
> >    dn: cn=hgroup3,cn=hostgroups,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> >    cn: hgroup3
> >    description: foo
> >    ipauniqueid: 20d1b8e4-b114-11e0-ab28-00163e0ed706
> >    memberof: cn=hgroup3,cn=ng,cn=alt,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com<====
> >    mepmanagedentry: cn=hgroup3,cn=ng,cn=alt,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com<====
> >    objectclass: ipaobject
> >    objectclass: ipahostgroup
> >    objectclass: nestedGroup
> >    objectclass: groupOfNames
> >    objectclass: top
> >    objectclass: mepOriginEntry
> >
> > # ipa user-add --first=Foo --last=Bar fbar2 --all --raw
> > ------------------
> > Added user "fbar2"
> > ------------------
> >    dn: uid=fbar2,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> >    uid: fbar2
> >    givenname: Foo
> >    sn: Bar
> >    cn: Foo Bar
> >    displayname: Foo Bar
> >    initials: FB
> >    homedirectory: /home/fbar2
> >    gecos: Foo Bar
> >    loginshell: /bin/sh
> >    krbprincipalname: fbar2 at IDM.LAB.BOS.REDHAT.COM
> >    uidnumber: 524600004
> >    gidnumber: 524600004
> >    ipauniqueid: b22ab54c-b115-11e0-b354-00163e0ed706
> >    krbpwdpolicyreference: cn=global_policy,cn=IDM.LAB.BOS.REDHAT.COM,cn=kerberos,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> >    objectclass: top
> >    objectclass: person
> >    objectclass: organizationalperson
> >    objectclass: inetorgperson
> >    objectclass: inetuser
> >    objectclass: posixaccount
> >    objectclass: krbprincipalaux
> >    objectclass: krbticketpolicyaux
> >    objectclass: ipaobject
> >    objectclass: mepOriginEntry
> > # ipa user-show fbar2 --all --raw
> >    dn: uid=fbar2,cn=users,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> >    uid: fbar2
> >    givenname: Foo
> >    sn: Bar
> >    cn: Foo Bar
> >    displayname: Foo Bar
> >    initials: FB
> >    homedirectory: /home/fbar2
> >    gecos: Foo Bar
> >    loginshell: /bin/sh
> >    krbprincipalname: fbar2 at IDM.LAB.BOS.REDHAT.COM
> >    uidnumber: 524600004
> >    gidnumber: 524600004
> >    nsaccountlock: False
> >    ipauniqueid: b22ab54c-b115-11e0-b354-00163e0ed706
> >    krbpwdpolicyreference: cn=global_policy,cn=IDM.LAB.BOS.REDHAT.COM,cn=kerberos,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com
> >    memberof: cn=ipausers,cn=groups,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com<====
> >    mepmanagedentry: cn=fbar2,cn=groups,cn=accounts,dc=idm,dc=lab,dc=bos,dc=redhat,dc=com<====
> >    objectclass: top
> >    objectclass: person
> >    objectclass: organizationalperson
> >    objectclass: inetorgperson
> >    objectclass: inetuser
> >    objectclass: posixaccount
> >    objectclass: krbprincipalaux
> >    objectclass: krbticketpolicyaux
> >    objectclass: ipaobject
> >    objectclass: mepOriginEntry
> >
> >
> > I think there attributes should be added in post_callback (and to the
> > tests).
> >
> > Martin
> >
> 
> Updated patch attached. The interesting change here is the 
> entry_from_entry() function.
> 
> Python calls functions passing by value the actual value passed may be 
> an immutable reference. This means we can't simply fetch the new entry 
> and replace what we already have, we have to do it value by value. We 
> also have to wipe out what is already there first because it is possible 
> an attribute has disappeared (I don't think one actually does in 
> practice in these two calls but it is cleaner this way).
> 
> For kicks you can see this in action with this snippet:
> 
> def tryme(x):
>      x = 5
> 
> y = 9
> tryme(y)
> print y
> 
> y is 9. Fun, isn't it?
> 
> rob

You are right, Python references are fun sometimes :-) Fortunately, I
learned this behavior earlier so it doesn't surprise me.

But your patch works fine, I like it. All fixed tests passed.

Pushed to master.

Martin