[Freeipa-devel] [PATCH 0104] do not disconnect when using existing connection to check default CA ACLs

[Martin Babinsky]

On 11/25/2015 09:56 AM, Jan Cholasta wrote:
> On 25.11.2015 09:28, Martin Babinsky wrote:
>> On 11/25/2015 07:21 AM, Jan Cholasta wrote:
>>> On 25.11.2015 05:56, Fraser Tweedale wrote:
>>>> On Tue, Nov 24, 2015 at 05:38:45PM +0100, Jan Cholasta wrote:
>>>>> On 24.11.2015 17:17, Martin Babinsky wrote:
>>>>>> On 11/24/2015 05:10 PM, Martin Babinsky wrote:
>>>>>>> On 11/24/2015 05:01 PM, Martin Babinsky wrote:
>>>>>>>> On 11/24/2015 04:58 PM, Jan Cholasta wrote:
>>>>>>>>> On 24.11.2015 16:48, Martin Babinsky wrote:
>>>>>>>>>> On 11/24/2015 04:44 PM, Martin Babinsky wrote:
>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5459
>>>>>>>>>>>
>>>>>>>>>> forgot to attach the actual file *slaps himself*
>>>>>>>>>
>>>>>>>>> ipaserver/install/cainstance.py:1849: [E1101(no-member),
>>>>>>>>> ensure_default_caacl] Instance of 'API' has no 'Backed' member)
>>>>>>>>>
>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> Also attaching rebased patch for ipa-4-2
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Ignore the rebased patch, the original 104.1 applies on 4-2 just
>>>>>> fine.
>>>>>
>>>>> Thanks, ACK.
>>>>>
>>>>> Pushed to:
>>>>> master: ed830af693c596b286b30959eb3166b59cc030c6
>>>>> ipa-4-2: c5faaede276f3052517ddf86e64cb228e95dca2a
>>>>>
>>>> Thanks for anaysing and fixing these connection issues which were
>>>> presumably introduced by my patches.  I did not hit these in my
>>>> testing.  Admittedly I was focused on the ipa-4-2 branch and my
>>>> testing was mainly done there.
>>>>
>>>> Brittle LDAP connection logic in install and upgrade is an ongoing
>>>> problem.  What shall we do to improve the situation?  Push
>>>> connection details into the Backend and let it connect if/when
>>>> needed, rather than managing connection state from the outside?
>>>>
>>>> I filed a ticket: https://fedorahosted.org/freeipa/ticket/5491
>>>
>>> I don't think we need to be smart about it, everyone just needs to make
>>> sure that when an ad-hoc connection is opened somewhere, it is also
>>> closed in the same place. The same applies for any other resource.
>>>
>>>>
>>>> Thanks,
>>>> Fraser
>>>>
>>>
>>>
>>
>> Maybe it would be a good idea to implement some decorator/context
>> manager to connect/disconnect from specified connection and wrap the
>> CRUD code. Example usage:
>>
>> """
>> decorator:
>> @connected(api.Backend.ldap2, ldapi=True)
>> def do_magic(api, *args, **kwargs):
>>     # do stuff
>>
>> context manager:
>>
>> with connected(api.Backend.ldap2, ccache=example_ccache):
>>      do_some_other_magic()
>> """
>>
>> I remember Jan having some objections against this so it would be nice
>> to hear why this is not a good idea.
>
> a) For ad-hoc connection objects, LDAPClient should be used instead of
> ldap2. It already supports the context manager protocol:
>
>      with LDAPClient(uri) as conn:
>          conn.gssapi_bind()
>          ...
>
I didn't know that you can and should use LDAPClient this way. We should 
document it somewhere and encourage other contributors to use this approach.

> b) The api.Backend.ldap2 object should be connected and disconnected
> exactly once per api object per command. This is not done ATM and the
> usual workaround is to connect it manually, which has lead to the mess
> we have now.
>
Then we should find time to implement this behavior so that we can stop 
shooting ourselves in the foot when messing around with API commands.

> Also, only GSSAPI authentication with valid IPA user credentials should
> be used with ldap2, even for ldapi connections.
>


-- 
Martin^3 Babinsky