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

2015-11-25 Thread Rob Crittenden
Martin Babinsky wrote:
> 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.

I'm not sure I follow. Are we still talking about tools or did this
shift to requests? It used to be at a single connection was created for
each request and used everywhere internally.

There is probably nothing to prevent you from doing something similar in
the tools but given that restarts are common I hardly see the point.

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

IIRC we kept the old IPAdmin class around in the old days for this very
reason.

rob

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-25 Thread Jan Cholasta

On 25.11.2015 10:40, Fraser Tweedale wrote:

On Wed, Nov 25, 2015 at 09:28:27AM +0100, 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.


This would not handle a connection loss (e.g. due to DS restart) but
would still a useful and relatively non-intrusive measure.
Furthermore it will make it easier for developer to do the Right
Thing and reviewer to verify (i.e. it's easy to see when patch does
LDAP things but is not using the context manager).

Jan, it is well and good to say "developer just has to remember to
open and close the connection" - and I accept that I have been the
main culprit recently - but let us make it as easy as possible to
get right, and hard for a patch that does not do the proper resource
management to slip through.


See the other thread.

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-25 Thread Fraser Tweedale
On Wed, Nov 25, 2015 at 09:28:27AM +0100, 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.
> 
This would not handle a connection loss (e.g. due to DS restart) but
would still a useful and relatively non-intrusive measure.
Furthermore it will make it easier for developer to do the Right
Thing and reviewer to verify (i.e. it's easy to see when patch does
LDAP things but is not using the context manager).

Jan, it is well and good to say "developer just has to remember to
open and close the connection" - and I accept that I have been the
main culprit recently - but let us make it as easy as possible to
get right, and hard for a patch that does not do the proper resource
management to slip through.

Cheers,
Fraser

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-25 Thread 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

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-25 Thread Jan Cholasta

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()
...

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.


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


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-25 Thread Martin Babinsky

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.


--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-24 Thread Jan Cholasta

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




--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-24 Thread Fraser Tweedale
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

Thanks,
Fraser

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-24 Thread Jan Cholasta

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

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-24 Thread Martin Babinsky

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.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-24 Thread Martin Babinsky

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

--
Martin^3 Babinsky
From 0ed648bebd9f09a099b74fab2d6dcd16dfc6cb14 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 24 Nov 2015 16:40:52 +0100
Subject: [PATCH] do not disconnect when using existing connection to check
 default CA ACLs

https://fedorahosted.org/freeipa/ticket/5459
---
 ipaserver/install/cainstance.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 189876f3c0d980e78165d73eed86b2830ac8c5b8..c20bf39c12cff0777d90efad2b0d8d136ee37ec9 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1846,7 +1846,8 @@ def _create_dogtag_profile(profile_id, profile_data):
 
 def ensure_default_caacl():
 """Add the default CA ACL if missing."""
-if not api.Backend.ldap2.isconnected():
+is_already_connected = api.Backend.ldap2.isconnected()
+if not is_already_connected:
 try:
 api.Backend.ldap2.connect(autobind=True)
 except errors.PublicError as e:
@@ -1870,6 +1871,9 @@ def ensure_default_caacl():
 api.Command.caacl_add_profile(u'hosts_services_caIPAserviceCert',
 certprofile=(u'caIPAserviceCert',))
 
+if not is_already_connected:
+api.Backend.ldap2.disconnect()
+
 
 if __name__ == "__main__":
 standard_logging_setup("install.log")
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-11-24 Thread Martin Babinsky

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

--
Martin^3 Babinsky
From 2c87ea0c6b2b91ae344985138dce6c555cfe8578 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 24 Nov 2015 16:40:52 +0100
Subject: [PATCH] do not disconnect when using existing connection to check
 default CA ACLs

https://fedorahosted.org/freeipa/ticket/5459
---
 ipaserver/install/cainstance.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index cca27e9d2a4767e91771a918c12b2c852dc29161..8a8ae2fc5802271f115c79a78c04fde70977f754 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -2004,7 +2004,8 @@ def _create_dogtag_profile(profile_id, profile_data):
 
 def ensure_default_caacl():
 """Add the default CA ACL if missing."""
-if not api.Backend.ldap2.isconnected():
+is_already_connected = api.Backend.ldap2.isconnected()
+if not is_already_connected:
 try:
 api.Backend.ldap2.connect(autobind=True)
 except errors.PublicError as e:
@@ -2028,7 +2029,7 @@ def ensure_default_caacl():
 api.Command.caacl_add_profile(u'hosts_services_caIPAserviceCert',
 certprofile=(u'caIPAserviceCert',))
 
-if api.Backend.ldap2.isconnected():
+if not is_already_connected:
 api.Backend.ldap2.disconnect()
 
 
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

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

2015-11-24 Thread Jan Cholasta

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)


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


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

2015-11-24 Thread Martin Babinsky

On 11/24/2015 04:44 PM, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5459


forgot to attach the actual file *slaps himself*

--
Martin^3 Babinsky
From 3ca5e8348cf1448dd61a069dc4b01e2fdf7ed201 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Tue, 24 Nov 2015 16:40:52 +0100
Subject: [PATCH] do not disconnect when using existing connection to check
 default CA ACLs

https://fedorahosted.org/freeipa/ticket/5459
---
 ipaserver/install/cainstance.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index c72d11d1e0b86c040dc497744cda87aab22caafd..dd3dbd737c0ae89b27756a94b47ea5a9493260ef 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1846,7 +1846,8 @@ def _create_dogtag_profile(profile_id, profile_data):
 
 def ensure_default_caacl():
 """Add the default CA ACL if missing."""
-if not api.Backend.ldap2.isconnected():
+is_already_connected = api.Backed.ldap2.isconnected()
+if not is_already_connected:
 try:
 api.Backend.ldap2.connect(autobind=True)
 except errors.PublicError as e:
@@ -1870,7 +1871,7 @@ def ensure_default_caacl():
 api.Command.caacl_add_profile(u'hosts_services_caIPAserviceCert',
 certprofile=(u'caIPAserviceCert',))
 
-if api.Backend.ldap2.isconnected():
+if not is_already_connected:
 api.Backend.ldap2.disconnect()
 
 
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code