[Freeipa-devel] [PATCH] 377 Using LDAPI to setup CA and KRA agents.

[Jan Cholasta]

On 7.9.2015 07:32, Jan Cholasta wrote:
> On 4.9.2015 16:53, Petr Vobornik wrote:
>> On 09/04/2015 04:03 PM, Endi Sukma Dewata wrote:
>>> On 9/4/2015 6:35 AM, Martin Basti wrote:
>>>>
>>>>
>>>> On 09/02/2015 06:42 AM, Endi Sukma Dewata wrote:
>>>>> On 9/1/2015 1:52 AM, Martin Basti wrote:
>>>>>>>>>> The CA and KRA installation code has been modified to use LDAPI
>>>>>>>>>> to create the CA and KRA agents directly in the CA and KRA
>>>>>>>>>> database. This way it's no longer necessary to use the Directory
>>>>>>>>>> Manager password or CA and KRA admin certificate.
>>>>>>>>>>
>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5257
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thank you.
>>>>>>>>>
>>>>>>>>> 1) Can you use following code instead of direct call of
>>>>>>>>> ldap2.ldap2()?
>>>>>>>>>
>>>>>>>>> if not api.Backend.ldap2.is_connected():
>>>>>>>>>      api.Backend.ldap2.connect(autobind=True)
>>>>>>>>>
>>>>>>>>> conn = api.Backend.ldap2
>>>>>>>
>>>>>>> Why would you want to do that? The original code is fine, except the
>>>>>>> connection check is not necessary (it is a new instance of ldap2, so
>>>>>>> .isconnected() will always return False).
>>>>>>>
>>>>>>>>
>>>>>>>> It's actually isconnected() instead of is_connected(), but even so,
>>>>>>>> the
>>>>>>>> proposed code doesn't work:
>>>>>>>>
>>>>>>>> ipa.ipapython.install.cli.install_tool(Server): DEBUG    The
>>>>>>>> ipa-server-install command failed, exception: TypeError: 'ldap2'
>>>>>>>> object
>>>>>>>> is not callable
>>>>>>>> ipa.ipapython.install.cli.install_tool(Server): ERROR 'ldap2'
>>>>>>>> object
>>>>>>>> is not callable
>>>>>>>>
>>>>>>>>> 2) Patch needs rebase to master branch.
>>>>>>>>
>>>>>>>> The original patch does apply cleanly to master. Did you see a
>>>>>>>> conflict?
>>>>>> Sorry my bad.
>>>>>>
>>>>>> Martin^2
>>>>>>>>
>>>>>>>>> 3)
>>>>>>>>> +        user_dn = DN(('uid', "ipara"), ('ou', 'People'),
>>>>>>>>> self.basedn)
>>>>>>>>> +        conn.create(
>>>>>>>>> +            dn=user_dn,
>>>>>>>>>
>>>>>>>>> can you use add entry() instead of create()? We don't use native
>>>>>>>>> python-ldap, but rather ipaldap methods
>>>>>>>>
>>>>>>>> It's actually calling the ldap2.create() defined in
>>>>>>>> ipaserver/plugins/ldap2.py, which calls add_entry().
>>>>>>>
>>>>>>> NACK. We don't use ldap2.create(). Use add_entry().
>>>>>>>
>>>>>>>>
>>>>>>>> So my original patch still stands.
>>>>>
>>>>> New patch attached.
>>>>>
>>>> ACK, but IMO that comments is not necessary and I would like to push
>>>> the
>>>> patch without it.
>>>>
>>>> Martin^2
>>>
>>> It is necessary if we don't want people to use it. Otherwise someone
>>> could make the same mistake. Or better yet, just remove the method.
>>>
>>
>> +
>> +        NOTE: Do not use this method.
>>
>> I agree that the comment should not be in this patch - it is not
>> relevant to vaults.
>>
>> The comment or a removal of the method(if it is really useless) should
>> be in a different patch. If comment is the way than please also add why
>> it should not be used.
>
> The method was intended to be used with frontend objects, but they never
> happened in IPA, so it was left unused (instead we have no clean
> interface between frontend and backend and call backend-specific methods
> ad-hoc, what a great design /sarcasm). I personally would like to revive
> the concept, so I would not remove the methods. I don't think a comment
> is necessary either, because up until now, nobody tried to use the method.
>

Pushed to:
master: 72cfcfa0bd1e867537fcc788512e5fca20708b83
ipa-4-2: 3973da56d334040d9fee88d52c38265066debd56

-- 
Jan Cholasta