[Freeipa-devel] [PATCH] Add group plugin port to new LDAP backend.

[Jason Gerard DeRose]

On Fri, 2009-05-15 at 14:51 -0400, Rob Crittenden wrote:
> Jason Gerard DeRose wrote:
> > On Fri, 2009-05-15 at 09:40 -0400, Rob Crittenden wrote:
> >> Pavel Zuna wrote:
> >>> Jason Gerard DeRose wrote:
> >>>> On Wed, 2009-05-13 at 16:04 -0400, Rob Crittenden wrote:
> >>>>> Pavel Zuna wrote:
> >>>>>> Rob Crittenden wrote:
> >>>>>>> Pavel Zuna wrote:
> >>>>>>>> Rob Crittenden wrote:
> >>>>>>>>> Pavel Zuna wrote:
> >>>>>>>>>> Rob Crittenden wrote:
> >>>>>>>>>>> Pavel Zuna wrote:
> >>>>>>>>>>>> By the way, there's a little bug I discovered while testing 
> >>>>>>>>>>>> this plugin. It affects the old group plugin as well. When 
> >>>>>>>>>>>> trying to modify a group into a posixGroup, gidNumber doesn't 
> >>>>>>>>>>>> get generated automatically resulting in a object violation 
> >>>>>>>>>>>> LDAP error. Solution is to generate it ourselves, but I didn't 
> >>>>>>>>>>>> know how it works, so I commented that part out for now. 
> >>>>>>>>>>>> (/FIXME in vim)
> >>>>>>>>>>>>
> >>>>>>>>>>> This should be fixed in FDS 1.2. Can you update and give it a try?
> >>>>>>>>>>>
> >>>>>>>>>>> rob
> >>>>>>>>>> Sure, just updated and you're right, it works.  :)
> >>>>>>>>>> Updated patch attached.
> >>>>>>>>>>
> >>>>>>>>>> Pavel
> >>>>>>>>> nack. This won't handle someone using group-mod to set a specific 
> >>>>>>>>> gidnumber. The posixGroup objectclass won't be added.
> >>>>>>>>>
> >>>>>>>>> rob
> >>>>>>>> Fixed patch attached.
> >>>>>>>>
> >>>>>>>> Pavel
> >>>>>>> The basegroup2 part looks ok but nack on group2.
> >>>>>>>
> >>>>>>> I think we should stick with using lower-case attribute names as a 
> >>>>>>> rule of thumb rather than camel case. In any case you test for the 
> >>>>>>> string posixGroup is in the list of objectclasses, this test needs 
> >>>>>>> to be case insensitive.
> >>>>>> When no attributes to retrieve are specified, python-ldap retrieves 
> >>>>>> them all in the original form - camel case. If we specify them, then 
> >>>>>> it returns them in the same form as we requested them. The new LDAP 
> >>>>>> backend doesn't use CIDicts anymore, but only the normal python dict 
> >>>>>> type, so everything is case sensitive. Of course I can make it 
> >>>>>> return attribute names always as lowercase if that's what we want.
> >>>>> I think we need consistent naming otherwise all sorts of odd bugs can 
> >>>>> creep in.
> >>>>>
> >>>>>>> I also wonder if we should be using ldap.get_entry(). Why use this 
> >>>>>>> over group-show?
> >>>>>> It's faster, because we call get_entry directly and because we can 
> >>>>>> request objectClass attribute only. Why invoke an IPA command 
> >>>>>> instead of a making a direct call?
> >>>>> Well, I felt the same way but Jason convinced me that by limiting the 
> >>>>> places we do actual LDAP calls will be beneficial in the long-run. 
> >>>>> The command is run internally, not over XML-RPC, so there isn't a 
> >>>>> whole lot of additional overhead.
> >>>>>
> >>>>> Part of the idea, which we haven't really utilized much yet, is to 
> >>>>> try to make the backend easily replacable.
> >>>> Well, first question, is this Backend.ldap.get_entry(), or a direct call
> >>>> to the python-ldap bindings?  I feel very strongly that no plugins
> >>>> should talk directly to python-ldap (except Backend.ldap).
> >>>>
> >>>> But if this is whether to retrieve the group entry via
> >>>> Command.group_show() or Backend.ldap.get_entry(), I think it depends on
> >>>> what attributes are needed.  If we want the same attributes as
> >>>> Command.group_show() returns, we should call it so we aren't defining
> >>>> the list of (or logic behind) the attributes to choose in multiple
> >>>> places.  If we need all the attributes, calling Backend.ldap.get_entry()
> >>>> is probably best.
> >>> In this context, we need only the 'objectClass' attribute that 
> >>> group_show doesn't return normally unless we pass it the '--all' option 
> >>> and then we get a lot of attributes we don't need as side effect.
> >> The biggest downside is that you'd get the members which really could be 
> >> quite large. I think a better solution is to be able to pass into 
> >> group-show (and really any/all of the plugins) the attributes you want 
> >> to see, with a reasonable default and the --all options available too.
> >>
> >> The problem with getting the entry within another plugin is you may 
> >> require knowledge of the schema to do so. If the schema changes, like it 
> >> did with host recently, you'd have to know to go and change it in other 
> >> places. When I changed the DN of the host entries the only plugin that 
> >> changed was host.
> >>
> >> rob
> > 
> > Hmmm, yeah, good point.  I think I am retracting my earlier +1.
> > 
> > Oh, I've been thinking about how to specify what attributes are pulled
> > by default when an entry is retrieved: I think we should add flag to the
> > Param to indicate whether it should be included.  This keeps things
> > easily plugable.
> > 
> > Although we haven't used them yet, the Attribute plugins are designed to
> > add another Param (in the case of LDAP stuff, an LDAP attribute) to a
> > corresponding Object plugin.  So you could add a new Param to
> > Object.user, which would automatically get picked up by all the user
> > crud methods.  If we have the list of attributes to retrieve hard-coded
> > in Command.user_show(), an Attribute plugin can't be included in the
> > list of attributes to pull without overriding the Command.user_show
> > plugin.
> > 
> > What do you two think?
> 
> But we can never know all possible attributes. We will always need some 
> generic way to pass in a name/value pair and hope for the best, unless 
> we want to slurp in the schema on the framework side and use that as a 
> baseline.
> 
> And speaking of LDAP plugins, a critical missing piece of the new LDAP 
> backend is support for the command-line tools. They all currently use 
> the IPAdmin class.

Do you mean the installer/updater scripts, like ipa-server-install and
whatnot?

> rob