[Freeipa-devel] [PATCH] Add group plugin port to new LDAP backend.
Pavel Zuna
pzuna at redhat.com
Fri May 15 10:22:53 UTC 2009
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.
Generally speaking, I think the decision of what call is made should go
like this:
We need to get attribute A,B,C,... (even if they happen to be the same
as group_show returns)? Call get_entry.
We need to get the same attributes group_show returns (whatever those
may be)? Call group_show.
>>>> I'm not sure if the logic around setting gidnumber is right. If you
>>>> set the gidnumber but aren't using the --posix flag it looks like it
>>>> will always append posixgroup to the list of objectclasses. I'm pretty
>>>> sure the LDAP server is going to reject the update. I suppose making a
>>>> list(set(objectclasses)) would work for de-duping.
>>> You're right, it's broken. I'll fix it.
>>>
>>> Pavel
>> ok
>>
>> rob
Pavel
More information about the Freeipa-devel
mailing list