Re: [Freeipa-devel] [PATCH] 0748 Handle encoding for ipautil.run

2015-11-18 Thread Petr Viktorin
On 11/03/2015 02:39 PM, Petr Viktorin wrote:
> Hello,
> Python 3's strings are Unicode, so data coming to or leaving a Python
> program needs to be decoded/encoded if it's to be handled as a string.
> One of the boundaries where encoding is necessary is external programs,
> specifically, ipautil.run.
> Unfortunately there's no one set of options that would work for all
> run() invocations, so I went through them all and specified the
> stdin/stdout/stderr encoding where necessary. I've also updated the call
> sites to make it clearer if the return values are used or not.
> If an encoding is not set, run() will accept/return bytes. (This is a
> fail-safe setting, since it can't raise errors, and using bytes where
> strings are expected generally fails loudly in py3.)
> 
> Note that the changes are not effective under Python 2.

ping,
Could someone look at this patch?


-- 
Petr Viktorin

-- 
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] Caching ldap limits for whole connection (performance)

2015-11-18 Thread Martin Basti



On 18.11.2015 18:01, Martin Basti wrote:



On 18.11.2015 17:58, Rob Crittenden wrote:

Martin Basti wrote:


On 18.11.2015 17:34, Martin Basti wrote:


On 18.11.2015 14:25, Petr Vobornik wrote:

On 11/17/2015 10:37 AM, Martin Basti wrote:


On 16.11.2015 20:18, Rob Crittenden wrote:

Martin Basti wrote:


On 16.11.2015 18:57, Martin Basti wrote:

How does this code work (IMO it doesn't), ldap2.py

 def find_entries(self, filter=None, attrs_list=None,
base_dn=None,
  scope=_ldap.SCOPE_SUBTREE, time_limit=None,
  size_limit=None, search_refs=False,
paged_search=False):

 def _get_limits():
 """Get configured global limits, caching them for 
more

calls"""
 if not _lims:
 config = self.get_ipa_config()
 _lims['time'] = 
int(config.get('ipasearchtimelimit',

[None])[0])
 _lims['size'] =
int(config.get('ipasearchrecordslimit', [None])[0])
 return _lims
 _lims = {}

 if time_limit is None:
 time_limit = _get_limits()['time']
 if size_limit is None:
 size_limit = _get_limits()['size']

Code above is supposed to do caching, but it doesn't do it. This
might
work if _lims were self._lims.
I tried similar code to test this behavior:

class test:
 def __init__(self):
pass

 def cached_call(self):
"""configured global limits"""
_lims = {}
def _get_limits():
if not _lims:
_lims['t']='oujeee'
print 'getting limits'
return _lims

print "Limits:", _get_limits()['t']

t = test()
t.cached_call()
t.cached_call()
t.cached_call()
t.cached_call()

Output:
$ python testcaching.py
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee

So it does not do caching, or am I wrong?
Martin^2


That code works, the whole caching is just for the second call of
_get_limits()

Can we instead just caching limits for second use, do caching for
whole
connection?
This may be effective for methods/commands that calls search 
and show

several times.

Is there something that prevents us to do that?


It already is cached. See get_ipa_config().

rob

I missed that part there, thank you.
Martin


I tried user_add and according to access log(
http://fpaste.org/291835/44785307/ ) it alone does 13 searches for
ipa config:

SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0
filter="(objectClass=*)" attrs=ALL

So I think it is not working correctly.
I did testing, I put following debug prints into get_ipa_config 
method:


+   print("call: get_ipa_config")
 try:
 config_entry = getattr(context, 'config_entry')
 if config_entry.conn is self.conn:
 return config_entry
+print("get_ipa_config: different connection!")
 except AttributeError:
 # Not in our context yet
 pass

then I restarted apache, executed "ipa user-add test2" and result is
here: http://fpaste.org/291926/64204144/

what means that the check in get_ipa_config is wrong, or ipa framework
always creates a new connection.
Except the first call that does not have cached value in context,
every other call asked directly LDAP for the value.


The comparation of connections in get_ipa_config does not work.
http://fpaste.org/291937/64863144/

I added repr(connection) of cached and current connection, and 
self.conn

is still the same but *is* operator returned failed.


Nice catch. I guess the next step is to see when this was broken. I had
a 4.1.4 system lying around and that works so this is a fairly recent 
issue.


I'd suggest expanding your debugging to show the two connections, maybe
that will provide some clues.

IIRC there are tests to ensure that context.conn is being set but
perhaps something changed.

rob


Yes, thank you Petr to catch this.
https://fedorahosted.org/freeipa/ticket/5463


Connection objects has different reference since beginning id(self.conn) 
!= id(config_entry.conn), so the object in config_entry.conn is not the 
same as object self.conn. Otherwise object added and received to/from 
context is the same object.



--
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 0344] Use absolute domain name in detection of A/AAAA records

2015-11-18 Thread Petr Spacek
On 12.11.2015 13:58, Martin Basti wrote:
> 
> 
> On 09.11.2015 08:47, Petr Spacek wrote:
>> On 4.11.2015 16:16, Martin Basti wrote:
>>> Patch attached.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5421
>> I'm not entirely sure how this patch will interact with magic included in
>> ipalib/plugins/dns.py:class dns_resolve(Command).
>>
>> I would like to delete the 'normalization' from at least one of these places.
>>
>> Also, as you know, DNS names are not strings and should be manipulated using
>> python-dns so all crazy things in DNS names do not break in weird corner 
>> cases.
>>
> Updated patch attached.

Hmm, you bravely ignored my comment about class dns_resolve(Command) above,
sooo: NACK.

As far as I can tell ipalib/plugins/dns.py:class dns_resolve(Command) behaves
in the same brain-dead way as original is_host_resolvable() function. Please
fix both, not just one.

-- 
Petr^2 Spacek

-- 
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] Caching ldap limits for whole connection (performance)

2015-11-18 Thread Martin Basti



On 18.11.2015 17:58, Rob Crittenden wrote:

Martin Basti wrote:


On 18.11.2015 17:34, Martin Basti wrote:


On 18.11.2015 14:25, Petr Vobornik wrote:

On 11/17/2015 10:37 AM, Martin Basti wrote:


On 16.11.2015 20:18, Rob Crittenden wrote:

Martin Basti wrote:


On 16.11.2015 18:57, Martin Basti wrote:

How does this code work (IMO it doesn't), ldap2.py

 def find_entries(self, filter=None, attrs_list=None,
base_dn=None,
  scope=_ldap.SCOPE_SUBTREE, time_limit=None,
  size_limit=None, search_refs=False,
paged_search=False):

 def _get_limits():
 """Get configured global limits, caching them for more
calls"""
 if not _lims:
 config = self.get_ipa_config()
 _lims['time'] = int(config.get('ipasearchtimelimit',
[None])[0])
 _lims['size'] =
int(config.get('ipasearchrecordslimit', [None])[0])
 return _lims
 _lims = {}

 if time_limit is None:
 time_limit = _get_limits()['time']
 if size_limit is None:
 size_limit = _get_limits()['size']

Code above is supposed to do caching, but it doesn't do it. This
might
work if _lims were self._lims.
I tried similar code to test this behavior:

class test:
 def __init__(self):
pass

 def cached_call(self):
"""configured global limits"""
_lims = {}
def _get_limits():
if not _lims:
_lims['t']='oujeee'
print 'getting limits'
return _lims

print "Limits:", _get_limits()['t']

t = test()
t.cached_call()
t.cached_call()
t.cached_call()
t.cached_call()

Output:
$ python testcaching.py
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee

So it does not do caching, or am I wrong?
Martin^2


That code works, the whole caching is just for the second call of
_get_limits()

Can we instead just caching limits for second use, do caching for
whole
connection?
This may be effective for methods/commands that calls search and show
several times.

Is there something that prevents us to do that?


It already is cached. See get_ipa_config().

rob

I missed that part there, thank you.
Martin


I tried user_add and according to access log(
http://fpaste.org/291835/44785307/ ) it alone does 13 searches for
ipa config:

SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0
filter="(objectClass=*)" attrs=ALL

So I think it is not working correctly.

I did testing, I put following debug prints into get_ipa_config method:

+   print("call: get_ipa_config")
 try:
 config_entry = getattr(context, 'config_entry')
 if config_entry.conn is self.conn:
 return config_entry
+print("get_ipa_config: different connection!")
 except AttributeError:
 # Not in our context yet
 pass

then I restarted apache, executed "ipa user-add test2" and result is
here: http://fpaste.org/291926/64204144/

what means that the check in get_ipa_config is wrong, or ipa framework
always creates a new connection.
Except the first call that does not have cached value in context,
every other call asked directly LDAP for the value.


The comparation of connections in get_ipa_config does not work.
http://fpaste.org/291937/64863144/

I added repr(connection) of cached and current connection, and self.conn
is still the same but *is* operator returned failed.


Nice catch. I guess the next step is to see when this was broken. I had
a 4.1.4 system lying around and that works so this is a fairly recent issue.

I'd suggest expanding your debugging to show the two connections, maybe
that will provide some clues.

IIRC there are tests to ensure that context.conn is being set but
perhaps something changed.

rob


Yes, thank you Petr to catch this.
https://fedorahosted.org/freeipa/ticket/5463


--
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] Caching ldap limits for whole connection (performance)

2015-11-18 Thread Rob Crittenden
Martin Basti wrote:
> 
> 
> On 18.11.2015 17:34, Martin Basti wrote:
>>
>>
>> On 18.11.2015 14:25, Petr Vobornik wrote:
>>> On 11/17/2015 10:37 AM, Martin Basti wrote:


 On 16.11.2015 20:18, Rob Crittenden wrote:
> Martin Basti wrote:
>>
>>
>> On 16.11.2015 18:57, Martin Basti wrote:
>>> How does this code work (IMO it doesn't), ldap2.py
>>>
>>> def find_entries(self, filter=None, attrs_list=None,
>>> base_dn=None,
>>>  scope=_ldap.SCOPE_SUBTREE, time_limit=None,
>>>  size_limit=None, search_refs=False,
>>> paged_search=False):
>>>
>>> def _get_limits():
>>> """Get configured global limits, caching them for more
>>> calls"""
>>> if not _lims:
>>> config = self.get_ipa_config()
>>> _lims['time'] = int(config.get('ipasearchtimelimit',
>>> [None])[0])
>>> _lims['size'] =
>>> int(config.get('ipasearchrecordslimit', [None])[0])
>>> return _lims
>>> _lims = {}
>>>
>>> if time_limit is None:
>>> time_limit = _get_limits()['time']
>>> if size_limit is None:
>>> size_limit = _get_limits()['size']
>>>
>>> Code above is supposed to do caching, but it doesn't do it. This
>>> might
>>> work if _lims were self._lims.
>>> I tried similar code to test this behavior:
>>>
>>> class test:
>>> def __init__(self):
>>>pass
>>>
>>> def cached_call(self):
>>>"""configured global limits"""
>>>_lims = {}
>>>def _get_limits():
>>>if not _lims:
>>>_lims['t']='oujeee'
>>>print 'getting limits'
>>>return _lims
>>>
>>>print "Limits:", _get_limits()['t']
>>>
>>> t = test()
>>> t.cached_call()
>>> t.cached_call()
>>> t.cached_call()
>>> t.cached_call()
>>>
>>> Output:
>>> $ python testcaching.py
>>> Limits: getting limits
>>> oujeee
>>> Limits: getting limits
>>> oujeee
>>> Limits: getting limits
>>> oujeee
>>> Limits: getting limits
>>> oujeee
>>>
>>> So it does not do caching, or am I wrong?
>>> Martin^2
>>>
>> That code works, the whole caching is just for the second call of
>> _get_limits()
>>
>> Can we instead just caching limits for second use, do caching for
>> whole
>> connection?
>> This may be effective for methods/commands that calls search and show
>> several times.
>>
>> Is there something that prevents us to do that?
>>
>
> It already is cached. See get_ipa_config().
>
> rob
 I missed that part there, thank you.
 Martin

>>>
>>> I tried user_add and according to access log(
>>> http://fpaste.org/291835/44785307/ ) it alone does 13 searches for
>>> ipa config:
>>>
>>> SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0
>>> filter="(objectClass=*)" attrs=ALL
>>>
>>> So I think it is not working correctly.
>>
>> I did testing, I put following debug prints into get_ipa_config method:
>>
>> +   print("call: get_ipa_config")
>> try:
>> config_entry = getattr(context, 'config_entry')
>> if config_entry.conn is self.conn:
>> return config_entry
>> +print("get_ipa_config: different connection!")
>> except AttributeError:
>> # Not in our context yet
>> pass
>>
>> then I restarted apache, executed "ipa user-add test2" and result is
>> here: http://fpaste.org/291926/64204144/
>>
>> what means that the check in get_ipa_config is wrong, or ipa framework
>> always creates a new connection.
>> Except the first call that does not have cached value in context,
>> every other call asked directly LDAP for the value.
>>
> The comparation of connections in get_ipa_config does not work.
> http://fpaste.org/291937/64863144/
> 
> I added repr(connection) of cached and current connection, and self.conn
> is still the same but *is* operator returned failed.
> 

Nice catch. I guess the next step is to see when this was broken. I had
a 4.1.4 system lying around and that works so this is a fairly recent issue.

I'd suggest expanding your debugging to show the two connections, maybe
that will provide some clues.

IIRC there are tests to ensure that context.conn is being set but
perhaps something changed.

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] Caching ldap limits for whole connection (performance)

2015-11-18 Thread Martin Basti



On 18.11.2015 17:34, Martin Basti wrote:



On 18.11.2015 14:25, Petr Vobornik wrote:

On 11/17/2015 10:37 AM, Martin Basti wrote:



On 16.11.2015 20:18, Rob Crittenden wrote:

Martin Basti wrote:



On 16.11.2015 18:57, Martin Basti wrote:

How does this code work (IMO it doesn't), ldap2.py

def find_entries(self, filter=None, attrs_list=None, 
base_dn=None,

 scope=_ldap.SCOPE_SUBTREE, time_limit=None,
 size_limit=None, search_refs=False,
paged_search=False):

def _get_limits():
"""Get configured global limits, caching them for more
calls"""
if not _lims:
config = self.get_ipa_config()
_lims['time'] = int(config.get('ipasearchtimelimit',
[None])[0])
_lims['size'] =
int(config.get('ipasearchrecordslimit', [None])[0])
return _lims
_lims = {}

if time_limit is None:
time_limit = _get_limits()['time']
if size_limit is None:
size_limit = _get_limits()['size']

Code above is supposed to do caching, but it doesn't do it. This 
might

work if _lims were self._lims.
I tried similar code to test this behavior:

class test:
def __init__(self):
   pass

def cached_call(self):
   """configured global limits"""
   _lims = {}
   def _get_limits():
   if not _lims:
   _lims['t']='oujeee'
   print 'getting limits'
   return _lims

   print "Limits:", _get_limits()['t']

t = test()
t.cached_call()
t.cached_call()
t.cached_call()
t.cached_call()

Output:
$ python testcaching.py
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee

So it does not do caching, or am I wrong?
Martin^2


That code works, the whole caching is just for the second call of
_get_limits()

Can we instead just caching limits for second use, do caching for 
whole

connection?
This may be effective for methods/commands that calls search and show
several times.

Is there something that prevents us to do that?



It already is cached. See get_ipa_config().

rob

I missed that part there, thank you.
Martin



I tried user_add and according to access log( 
http://fpaste.org/291835/44785307/ ) it alone does 13 searches for 
ipa config:


SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0 
filter="(objectClass=*)" attrs=ALL


So I think it is not working correctly.


I did testing, I put following debug prints into get_ipa_config method:

+   print("call: get_ipa_config")
try:
config_entry = getattr(context, 'config_entry')
if config_entry.conn is self.conn:
return config_entry
+print("get_ipa_config: different connection!")
except AttributeError:
# Not in our context yet
pass

then I restarted apache, executed "ipa user-add test2" and result is 
here: http://fpaste.org/291926/64204144/


what means that the check in get_ipa_config is wrong, or ipa framework 
always creates a new connection.
Except the first call that does not have cached value in context, 
every other call asked directly LDAP for the value.



The comparation of connections in get_ipa_config does not work.
http://fpaste.org/291937/64863144/

I added repr(connection) of cached and current connection, and self.conn 
is still the same but *is* operator returned failed.


--
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] Caching ldap limits for whole connection (performance)

2015-11-18 Thread Martin Basti



On 18.11.2015 14:25, Petr Vobornik wrote:

On 11/17/2015 10:37 AM, Martin Basti wrote:



On 16.11.2015 20:18, Rob Crittenden wrote:

Martin Basti wrote:



On 16.11.2015 18:57, Martin Basti wrote:

How does this code work (IMO it doesn't), ldap2.py

def find_entries(self, filter=None, attrs_list=None, 
base_dn=None,

 scope=_ldap.SCOPE_SUBTREE, time_limit=None,
 size_limit=None, search_refs=False,
paged_search=False):

def _get_limits():
"""Get configured global limits, caching them for more
calls"""
if not _lims:
config = self.get_ipa_config()
_lims['time'] = int(config.get('ipasearchtimelimit',
[None])[0])
_lims['size'] =
int(config.get('ipasearchrecordslimit', [None])[0])
return _lims
_lims = {}

if time_limit is None:
time_limit = _get_limits()['time']
if size_limit is None:
size_limit = _get_limits()['size']

Code above is supposed to do caching, but it doesn't do it. This 
might

work if _lims were self._lims.
I tried similar code to test this behavior:

class test:
def __init__(self):
   pass

def cached_call(self):
   """configured global limits"""
   _lims = {}
   def _get_limits():
   if not _lims:
   _lims['t']='oujeee'
   print 'getting limits'
   return _lims

   print "Limits:", _get_limits()['t']

t = test()
t.cached_call()
t.cached_call()
t.cached_call()
t.cached_call()

Output:
$ python testcaching.py
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee

So it does not do caching, or am I wrong?
Martin^2


That code works, the whole caching is just for the second call of
_get_limits()

Can we instead just caching limits for second use, do caching for 
whole

connection?
This may be effective for methods/commands that calls search and show
several times.

Is there something that prevents us to do that?



It already is cached. See get_ipa_config().

rob

I missed that part there, thank you.
Martin



I tried user_add and according to access log( 
http://fpaste.org/291835/44785307/ ) it alone does 13 searches for ipa 
config:


SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0 
filter="(objectClass=*)" attrs=ALL


So I think it is not working correctly.


I did testing, I put following debug prints into get_ipa_config method:

+   print("call: get_ipa_config")
try:
config_entry = getattr(context, 'config_entry')
if config_entry.conn is self.conn:
return config_entry
+print("get_ipa_config: different connection!")
except AttributeError:
# Not in our context yet
pass

then I restarted apache, executed "ipa user-add test2" and result is 
here: http://fpaste.org/291926/64204144/


what means that the check in get_ipa_config is wrong, or ipa framework 
always creates a new connection.
Except the first call that does not have cached value in context, every 
other call asked directly LDAP for the value.


--
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] First part of the replica promotion tests + testplan

2015-11-18 Thread Martin Basti



On 09.11.2015 15:09, Oleg Fayans wrote:

Hi guys,

Here are first two automated testcases from this (so far incomplete) 
testplan: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan


Testplan review is highly appreciated





PATCH 16: NACK

1)
What is the reason to add an unused parameter to 'domain_level' to 
install_topo()?

Also it is good practise to add new option as the last parameter.

2)
cab you in both tests specify a domain level with constant instead of 
number literal?


3)
both test call install_topo with custom domain level, but it cannot work 
because 1)  (did you run the test?)


4)
How the test "TestLevel1" is supposed to work?
Respectively why there is call of install_topo() that installs replica. 
As this test just tests that ipa-replica-prepare is not working anymore, 
is it worth to spend 20 minutes with installing replica and then just no 
tot use it? IMO to install master in install step is enough.


Martin^2

-- 
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] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

2015-11-18 Thread Martin Babinsky

On 11/18/2015 02:16 PM, Lenka Doudova wrote:

Hi,

here's a patch that adds a few comments to stageuser tests in order to
allow easier determining of a problem when tests fail.

Lenka




Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the comments 
in 'options_ok' do not correctly map to the test names anyway.


I am also not sure if this patch is worth reviewing and pushing as it 
IMHO doesn't help in the identification of failed tests at all.


This should be solved at more fundamental level.

--
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 0331, 0337] User plugin: allow multiple managers per user - CLI part

2015-11-18 Thread Martin Basti



On 12.11.2015 12:39, Martin Basti wrote:



On 27.10.2015 14:59, Martin Basti wrote:



On 20.10.2015 18:46, Martin Basti wrote:



On 20.10.2015 16:07, Martin Basti wrote:



On 20.10.2015 15:57, Martin Basti wrote:

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

Patch attached.

Test are failing, a fix in UserTracker has to be done (partially 
in my patch 329)




SelfNACK, I forgot to add stageuser tests



Updated patch attached.

I extracted tests to the separate patch, tests do not work, I had 
issues with user and stageuser trackers.





Patch to fix issues with --addattr and managers added and attached.




The new one patch 0331 attached, patch 0337 is not needed anymore.

This patch also fixes https://fedorahosted.org/freeipa/ticket/5387



updated patch attached.
From c76b225561f2c8e9efba3d950eba9ce65c1ce7c4 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 5 Nov 2015 17:11:23 +0100
Subject: [PATCH] Allow multiple managers per user - CLI part

Added commands:
* user-add-manager
* user-remove-manager
* stageuser-add-manager
* stageuser-remove-manager

Commit contains override of convert_attribute_members method in baseuser
class that ensures the managers will be returned in 'manager' attribute
due to backward compatibility instead of 'manager_user' as would be
expected.

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

This patch also fixes: https://fedorahosted.org/freeipa/ticket/5387
---
 API.txt | 44 ++
 VERSION |  4 ++--
 ipalib/plugins/baseuser.py  | 58 -
 ipalib/plugins/stageuser.py | 22 -
 ipalib/plugins/user.py  | 24 +--
 5 files changed, 120 insertions(+), 32 deletions(-)

diff --git a/API.txt b/API.txt
index 873c6d54221a0c1657b5457bd9dceedb4adf06b3..0976c97213775d79da43ee382a0badbe029b7960 100644
--- a/API.txt
+++ b/API.txt
@@ -4248,6 +4248,17 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: stageuser_add_manager
+args: 1,5,3
+arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('no_members', autofill=True, default=False, exclude='webui')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('user*', alwaysask=True, cli_name='users', csv=True)
+option: Str('version?', exclude='webui')
+output: Output('completed', , None)
+output: Output('failed', , None)
+output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: stageuser_del
 args: 1,2,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=True, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
@@ -4367,6 +4378,17 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: stageuser_remove_manager
+args: 1,5,3
+arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('no_members', autofill=True, default=False, exclude='webui')
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
+option: Str('user*', alwaysask=True, cli_name='users', csv=True)
+option: Str('version?', exclude='webui')
+output: Output('completed', , None)
+output: Output('failed', , None)
+output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: stageuser_show
 args: 1,5,3
 arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
@@ -5208,6 +5230,17 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: user_add_manager
+args: 1,5,3
+arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', primary_key=True, query=True, required=True)
+option: Flag('all', autofill=True, cli_name='all', default=False, exclude

Re: [Freeipa-devel] [PATCH 0350] raise time limit for ldapsearch in upgrade

2015-11-18 Thread Martin Basti



On 18.11.2015 14:24, Martin Kosek wrote:

On 11/18/2015 02:18 PM, Martin Basti wrote:


On 18.11.2015 13:55, Martin Kosek wrote:

On 11/18/2015 01:30 PM, Martin Basti wrote:

+DEFAULT_TIME_LIMIT = -1.0
+DEFAULT_SIZE_LIMIT = 0

...

   if time_limit is None or time_limit == 0:
-time_limit = -1.0
+if self.time_limit is not None and self.time_limit != 0:
+time_limit = self.time_limit
+else:
+time_limit = self.DEFAULT_TIME_LIMIT
+

I wonder why is the -1 default time limit a float number, I would expect that
some trouble may emerge out of it. Maybe Rob knows?


Python LDAP allows to have smaller granularity than seconds and it internally
convert time limit values to float.

Hm, ok. I was just curious since the value we expose in API is Int and the
default does not use the smaller granularity, so I was thinking "Why bother?".
If it is fixed in your patch, good. If not, good also, no need to bikeshed here
I suppose.
I just keep the original values there, I do not want to touch it with 
this patch


--
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 0350] raise time limit for ldapsearch in upgrade

2015-11-18 Thread Martin Kosek
On 11/18/2015 02:18 PM, Martin Basti wrote:
> 
> 
> On 18.11.2015 13:55, Martin Kosek wrote:
>> On 11/18/2015 01:30 PM, Martin Basti wrote:
>>> +DEFAULT_TIME_LIMIT = -1.0
>>> +DEFAULT_SIZE_LIMIT = 0
>> ...
>>>   if time_limit is None or time_limit == 0:
>>> -time_limit = -1.0
>>> +if self.time_limit is not None and self.time_limit != 0:
>>> +time_limit = self.time_limit
>>> +else:
>>> +time_limit = self.DEFAULT_TIME_LIMIT
>>> +
>> I wonder why is the -1 default time limit a float number, I would expect that
>> some trouble may emerge out of it. Maybe Rob knows?
>>
> Python LDAP allows to have smaller granularity than seconds and it internally
> convert time limit values to float.

Hm, ok. I was just curious since the value we expose in API is Int and the
default does not use the smaller granularity, so I was thinking "Why bother?".
If it is fixed in your patch, good. If not, good also, no need to bikeshed here
I suppose.

-- 
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] Caching ldap limits for whole connection (performance)

2015-11-18 Thread Petr Vobornik

On 11/17/2015 10:37 AM, Martin Basti wrote:



On 16.11.2015 20:18, Rob Crittenden wrote:

Martin Basti wrote:



On 16.11.2015 18:57, Martin Basti wrote:

How does this code work (IMO it doesn't), ldap2.py

def find_entries(self, filter=None, attrs_list=None, base_dn=None,
 scope=_ldap.SCOPE_SUBTREE, time_limit=None,
 size_limit=None, search_refs=False,
paged_search=False):

def _get_limits():
"""Get configured global limits, caching them for more
calls"""
if not _lims:
config = self.get_ipa_config()
_lims['time'] = int(config.get('ipasearchtimelimit',
[None])[0])
_lims['size'] =
int(config.get('ipasearchrecordslimit', [None])[0])
return _lims
_lims = {}

if time_limit is None:
time_limit = _get_limits()['time']
if size_limit is None:
size_limit = _get_limits()['size']

Code above is supposed to do caching, but it doesn't do it. This might
work if _lims were self._lims.
I tried similar code to test this behavior:

class test:
def __init__(self):
   pass

def cached_call(self):
   """configured global limits"""
   _lims = {}
   def _get_limits():
   if not _lims:
   _lims['t']='oujeee'
   print 'getting limits'
   return _lims

   print "Limits:", _get_limits()['t']

t = test()
t.cached_call()
t.cached_call()
t.cached_call()
t.cached_call()

Output:
$ python testcaching.py
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee
Limits: getting limits
oujeee

So it does not do caching, or am I wrong?
Martin^2


That code works, the whole caching is just for the second call of
_get_limits()

Can we instead just caching limits for second use, do caching for whole
connection?
This may be effective for methods/commands that calls search and show
several times.

Is there something that prevents us to do that?



It already is cached. See get_ipa_config().

rob

I missed that part there, thank you.
Martin



I tried user_add and according to access log( 
http://fpaste.org/291835/44785307/ ) it alone does 13 searches for ipa 
config:


SRCH base="cn=ipaconfig,cn=etc,dc=example.com" scope=0 
filter="(objectClass=*)" attrs=ALL


So I think it is not working correctly.
--
Petr Vobornik

--
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 0350] raise time limit for ldapsearch in upgrade

2015-11-18 Thread Martin Basti



On 18.11.2015 13:55, Martin Kosek wrote:

On 11/18/2015 01:30 PM, Martin Basti wrote:

+DEFAULT_TIME_LIMIT = -1.0
+DEFAULT_SIZE_LIMIT = 0

...

  if time_limit is None or time_limit == 0:
-time_limit = -1.0
+if self.time_limit is not None and self.time_limit != 0:
+time_limit = self.time_limit
+else:
+time_limit = self.DEFAULT_TIME_LIMIT
+

I wonder why is the -1 default time limit a float number, I would expect that
some trouble may emerge out of it. Maybe Rob knows?

Python LDAP allows to have smaller granularity than seconds and it 
internally convert time limit values to float.


--
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


[Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

2015-11-18 Thread Lenka Doudova

Hi,

here's a patch that adds a few comments to stageuser tests in order to 
allow easier determining of a problem when tests fail.


Lenka
From a1ec552f27eeb05f5f9c41d59e076aa6ef7601db Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 18 Nov 2015 09:11:29 +0100
Subject: [PATCH] Adding comments in stageuser plugin tests.

Adding comments that should allow better orientation in the test_create_attr method. Original description was insufficient for identification of problem in case of failed tests.

Adding comments in stageuser plugin tests

Adding comments that should allow better orientation in the test_create_attr method. Original description was insufficient for identification of problem in case of failed tests.
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py | 62 +++
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 43c59b7c7ec2902064fc363c66e98cc98e8b9e17..78244f30b57986a9d45a272f15458d438e5cb6b0 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -54,33 +54,33 @@ sshpubkeyfp = (u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B '
'public key test (ssh-rsa)')
 
 options_ok = [
-{u'cn': u'name'},
-{u'initials': u'in'},
-{u'displayname': u'display'},
-{u'homedirectory': u'/home/homedir'},
-{u'gecos': u'gecos'},
-{u'loginshell': u'/bin/shell'},
-{u'mail': u'email@email.email'},
-{u'title': u'newbie'},
-{u'krbprincipalname': u'kerberos@%s' % api.env.realm},
-{u'krbprincipalname': u'KERBEROS@%s' % api.env.realm},
-{u'street': u'first street'},
-{u'l': u'prague'},
-{u'st': u'czech'},
-{u'postalcode': u'12345'},
-{u'telephonenumber': u'123456789'},
-{u'facsimiletelephonenumber': u'123456789'},
-{u'mobile': u'123456789'},
-{u'pager': u'123456789'},
-{u'ou': u'engineering'},
-{u'carlicense': u'abc1234'},
-{u'ipasshpubkey': sshpubkey},
-{u'manager': u'auser1'},
-{u'uidnumber': uid},
-{u'gidnumber': gid},
-{u'uidnumber': uid, u'gidnumber': gid},
-{u'userpassword': u'Secret123'},
-{u'random': True},
+{u'cn': u'name'},  # 1
+{u'initials': u'in'},  # 2
+{u'displayname': u'display'},  # 3
+{u'homedirectory': u'/home/homedir'},  # 4
+{u'gecos': u'gecos'},  # 5
+{u'loginshell': u'/bin/shell'},  # 6
+{u'mail': u'email@email.email'},  # 7
+{u'title': u'newbie'},  # 8
+{u'krbprincipalname': u'kerberos@%s' % api.env.realm},  # 9
+{u'krbprincipalname': u'KERBEROS@%s' % api.env.realm},  # 10
+{u'street': u'first street'},  # 11
+{u'l': u'prague'},  # 12
+{u'st': u'czech'},  # 13
+{u'postalcode': u'12345'},  # 14
+{u'telephonenumber': u'123456789'},  # 15
+{u'facsimiletelephonenumber': u'123456789'},  # 16
+{u'mobile': u'123456789'},  # 17
+{u'pager': u'123456789'},  # 18
+{u'ou': u'engineering'},  # 19
+{u'carlicense': u'abc1234'},  # 20
+{u'ipasshpubkey': sshpubkey},  # 21
+{u'manager': u'auser1'},  # 22
+{u'uidnumber': uid},  # 23
+{u'gidnumber': gid},  # 24
+{u'uidnumber': uid, u'gidnumber': gid},  # 25
+{u'userpassword': u'Secret123'},  # 26
+{u'random': True},  # 27
 ]
 
 
@@ -461,7 +461,13 @@ class TestStagedUser(XMLRPC_test):
 
 def test_create_attr(self, stageduser2, user, user6):
 """ Tests creating a user with various valid attributes listed
-in 'options_ok' list"""
+in 'options_ok' list
+Should this test fail, a message like the following appears:
+...::test_create_attr[stageduser2XX] FAILED
+where the 'XX' specifies option with which the 'ipa stageuser-add'
+command was performed. The options are listed and numbered
+accordingly in 'options_ok' list.
+"""
 # create staged user with specified parameters
 user.ensure_exists()  # necessary for manager test
 stageduser2.ensure_missing()
-- 
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 506] cert renewal: make renewal of ipaCert atomic

2015-11-18 Thread Jan Cholasta

On 10.11.2015 19:19, Rob Crittenden wrote:

Jan Cholasta wrote:

On 9.11.2015 16:51, Rob Crittenden wrote:

Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza





There be a note in renew_ra_cert that the lock is obtained in advance by
renew_ra_cert_pre.


Added comment.



It looks like it will silently fail if the lock cannot be acquired. Is
that desired?


All unhandled exceptions are logged to syslog in both renew_ra_cert_pre
and renew_ra_cert:

 try:
 main()
 except Exception:
 syslog.syslog(syslog.LOG_ERR, traceback.format_exc())

Updated patch attached.



My confusion was with the auto-expiration. I guess this is ok. When
debugging this sort of thing via logs the more the merrier, so I guess
I'd have added a syslog to say "obtaining lock" or "locked" and then
something when the renewal actually starts, so one can try to piece
together what happened after the fact if something goes wrong.

I guess certmonger already logs when a pre/post command is executed so
that may already be available.


Yes. The ticket is not related to logging anyway.

Is the last patch OK, then?

--
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 0350] raise time limit for ldapsearch in upgrade

2015-11-18 Thread Martin Kosek
On 11/18/2015 01:30 PM, Martin Basti wrote:
> +DEFAULT_TIME_LIMIT = -1.0
> +DEFAULT_SIZE_LIMIT = 0
...
>  if time_limit is None or time_limit == 0:
> -time_limit = -1.0
> +if self.time_limit is not None and self.time_limit != 0:
> +time_limit = self.time_limit
> +else:
> +time_limit = self.DEFAULT_TIME_LIMIT
> +

I wonder why is the -1 default time limit a float number, I would expect that
some trouble may emerge out of it. Maybe Rob knows?

-- 
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


[Freeipa-devel] [PATCH 0350] raise time limit for ldapsearch in upgrade

2015-11-18 Thread Martin Basti

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

Patch attached.
From 02de683d938d4d81466af862f17d7412511eb1c2 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 18 Nov 2015 10:31:05 +0100
Subject: [PATCH] Upgrade: increase time limit for upgrades

Default ldap search limit is now 30s by default during upgrade.

This value is configurable via default.conf file, option
'upgrade_ldapsearch_limit'.

Limits must be changed for the whole ldap2 connection, because this
connection is used inside update plugins and commands called from
upgrade.

Together with increasing the time limit, also size limit should be
unlimited during upgrade. With sizelimit=None we may get the
TimeExceeded exception from getting default value of the sizelimit from LDAP.

https://fedorahosted.org/freeipa/ticket/5267
---
 ipalib/constants.py |  3 +++
 ipapython/ipaldap.py| 35 ++-
 ipaserver/install/ldapupdate.py |  7 +++
 ipaserver/plugins/ldap2.py  | 35 ++-
 4 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/ipalib/constants.py b/ipalib/constants.py
index fc0560ba4fe746f11e8ff3175508ace2e50c3187..15be3ff865cdb0e6a01570144c9fdcc4c6909dac 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -189,6 +189,9 @@ DEFAULT_CONFIG = (
 # behavior when newer clients talk to older servers. Use with caution.
 ('skip_version_check', False),
 
+# Upgrade
+('upgrade_ldapsearch_limit', 30),  # limit in seconds
+
 # 
 #  The remaining keys are never set from the values here!
 # 
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 7e5bc04fea1dc89cc141051025d00a57d74b2b41..7b70894b27b5c32ed05251cf3bc6d1824c88010e 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -689,8 +689,12 @@ class LDAPClient(object):
 'nsslapd-minssf-exclude-rootdse': True,
 })
 
+DEFAULT_TIME_LIMIT = -1.0
+DEFAULT_SIZE_LIMIT = 0
+
 def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
- no_schema=False, decode_attrs=True):
+ no_schema=False, decode_attrs=True, time_limit=None,
+ size_limit=None):
 """Create LDAPClient object.
 
 :param ldap_uri: The LDAP URI to connect to
@@ -706,12 +710,18 @@ class LDAPClient(object):
 :param decode_attrs:
 If true, attributes are decoded to Python types according to their
 syntax.
+:param time_limit: time limit for ldap search that is used for whole
+connection
+:param size_limit: size limit for ldap seach that is used for whole
+connection
 """
 self.ldap_uri = ldap_uri
 self._start_tls = start_tls
 self._force_schema_updates = force_schema_updates
 self._no_schema = no_schema
 self._decode_attrs = decode_attrs
+self.time_limit = time_limit
+self.size_limit = size_limit
 
 self.log = log_mgr.get_logger(self)
 self._has_schema = False
@@ -1283,6 +1293,11 @@ class LDAPClient(object):
 (default skips these entries)
 paged_search -- search using paged results control
 
+size_limit and time_limit priority:
+1. use locally specified limits (if specified)
+2. use limits specified in __init__ (if specified)
+3. use default limits
+
 :raises: errors.NotFound if result set is empty
  or base_dn doesn't exist
 """
@@ -1295,9 +1310,17 @@ class LDAPClient(object):
 truncated = False
 
 if time_limit is None or time_limit == 0:
-time_limit = -1.0
+if self.time_limit is not None and self.time_limit != 0:
+time_limit = self.time_limit
+else:
+time_limit = self.DEFAULT_TIME_LIMIT
+
 if size_limit is None:
-size_limit = 0
+if self.size_limit is not None:
+size_limit = self.size_limit
+else:
+size_limit = self.DEFAULT_SIZE_LIMIT
+
 if not isinstance(size_limit, int):
 size_limit = int(size_limit)
 if not isinstance(time_limit, float):
@@ -1544,7 +1567,8 @@ class IPAdmin(LDAPClient):
 def __init__(self, host='', port=389, cacert=None, debug=None, ldapi=False,
  realm=None, protocol=None, force_schema_updates=True,
  start_tls=False, ldap_uri=None, no_schema=False,
- decode_attrs=True, sasl_nocanon=False, demand_cert=False):
+ decode_attrs=True, sasl_nocanon=False, demand_cert=False,
+ time_limit=None, size_limit=None):
 self._conn = None
 log_mgr.get_logger(self, True)
 if debug and debug.lower() == "on":
@@ -1564,7 +1588,8 @@ class

[Freeipa-devel] [PATCH 0096] check whether replica exists before executing the domain level 1 deletion code

2015-11-18 Thread Martin Babinsky

Additional fix for https://fedorahosted.org/freeipa/ticket/5424

In current implementation the topology suffices are checked first and 
after that the error about non-existent host is raised. This does not 
make much sense to me, we should check for host existence before any 
work is done.


--
Martin^3 Babinsky
From 93d9f706d31a1f57438fecb7dd10b97b52b0b240 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 18 Nov 2015 13:12:50 +0100
Subject: [PATCH] check whether replica exists before executing the domain
 level 1 deletion code

Move this check before the parts that check topology suffix connectivity, wait
for removed segments etc. If the hostname does not exist, it should really be
one of the first errors user encounters during ipa-replica-manage del.

https://fedorahosted.org/freeipa/ticket/5424
---
 install/tools/ipa-replica-manage | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 469b1d43a3aeb2f959f24defe5a73fae756bbc21..942ddc042b271ff1304be23469adb58d2cea7581 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -759,6 +759,15 @@ def del_master_managed(realm, hostname, options):
 print("Can't remove itself: %s" % (options.host))
 sys.exit(1)
 
+try:
+api.Command.host_show(hostname_u)
+except errors.NotFound:
+if not options.cleanup:
+print("{hostname} does not exist.".format(hostname=hostname))
+print("Please specify an actual server or add the --cleanup "
+  "option to force clean up.")
+sys.exit(1)
+
 # 1. Connect to the local server
 try:
 thisrepl = replication.ReplicationManager(realm, options.host,
@@ -791,13 +800,7 @@ def del_master_managed(realm, hostname, options):
 try:
 api.Command.server_del(hostname_u)
 except errors.NotFound:
-if not options.cleanup:
-print("{hostname} does not exist.".format(hostname=hostname))
-print("Please specify an actual server or add the --cleanup "
-  "option to force clean up.")
-sys.exit(1)
-else:
-print("Server entry already deleted: %s" % (hostname))
+print("Server entry already deleted: %s" % (hostname))
 
 # 6. Cleanup
 try:
-- 
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] [PATCHES 151-153] ipasam: fix wrong usage of talloc_new()

2015-11-18 Thread Alexander Bokovoy

On Wed, 18 Nov 2015, Sumit Bose wrote:

Hi,

please find attached 3 small patches for ipasam. The first fixes
https://fedorahosted.org/freeipa/ticket/5457 . The second is related
because if the compat tree is enabled the lookup will still fails
because the group is found twice.

The third patch fixes an issue valgrind found while I was checking the
other issue.

ACK for all three, they are obvious fixes (now ;).

We should also commit them to 4.1 and 4.2.

--
/ Alexander Bokovoy

--
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 0348] CI: fix KRA installation with domain level>0

2015-11-18 Thread Martin Basti



On 16.11.2015 17:33, Aleš Mareček wrote:

ACK.

Pushed to master: e56a1535b041c1ca1f398b54eda6bf04f1c1e33b


- Original Message -

From: "Martin Basti" 
To: "freeipa-devel" 
Sent: Monday, November 16, 2015 4:06:51 PM
Subject: [Freeipa-devel] [PATCH 0348] CI: fix KRA installation with domain  
level>0

Patch attached.
Part of https://fedorahosted.org/freeipa/ticket/5379

Martin^2


--
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


--
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 0347] Fix CI tests domain_level ENV config

2015-11-18 Thread Martin Basti



On 16.11.2015 17:32, Aleš Mareček wrote:

ACK.

Pushed to master: 79f7c71e61698457ae63b4f67bf296859e3f12d1


- Original Message -

From: "Martin Basti" 
To: "freeipa-devel" 
Sent: Friday, November 13, 2015 7:39:00 PM
Subject: [Freeipa-devel] [PATCH 0347] Fix CI tests domain_level ENV config

Patch attached.

Following test should pass:
ipa-run-tests test_integration/test_testconfig.py --verbose



--
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


--
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] ipa_kdb_tests: Fix test with default krb5.conf

2015-11-18 Thread Martin Basti



On 13.11.2015 09:29, Lukas Slebodnik wrote:

ehlo,

ipa_kdb_tests test failed for me on minimal f23.

LS



ACK
Pushed to master: 73058af717b1ab470858be9273d38ff27d35002a

-- 
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] cmocka_tests: Do not use deprecated cmocka interface

2015-11-18 Thread Martin Basti



On 13.11.2015 09:33, Lukas Slebodnik wrote:

ehlo,

The cmocka-1.0 introduced new interface for tests
which is not compatible with the old one.
And the old interface is deprecated which caused compiled warnings.

LS



ACK
Pushed to master: 75c26f9ec8d69af88bbf1d07b2c7b38d08e8d67d

-- 
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] BUILD: provide check target in custom Makefiles

2015-11-18 Thread Martin Basti



On 13.11.2015 09:36, Lukas Slebodnik wrote:

ehlo,

The automake generated makefiles have already a target check.
We need to provide this target also to non-generated
Makefiles so we can recursively call make check from
top level Makefile.

LS



ACK
Pushed to master: 2d39acf626358c01b5f18567f991b195ca842641

-- 
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] SPEC: Run cmocka based uni test in %check phase

2015-11-18 Thread Martin Basti



On 13.11.2015 12:04, Lukas Slebodnik wrote:

On (13/11/15 09:37), Lukas Slebodnik wrote:

ehlo,

this patch depends on
freeipa-lslebodn-0007-BUILD-provide-check-target-in-custom-Makefile.patch

LS
>From 507b57b4a166b0490d4217a9603a67577bb36036 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Fri, 13 Nov 2015 07:11:38 +
Subject: [PATCH 8/8] SPEC: Run cmocka based uni test in %check phase

---
freeipa.spec.in | 3 +++
1 file changed, 3 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
c3ca3413ffc3850b849a69adbbae8476355f3c76..fec2a83fcc83c02423601d88500e789c83f7a7d0
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -405,6 +405,9 @@ make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} all
make IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} client
%endif # ONLY_CLIENT

+%check
+make %{?_smp_mflags} check VERBOSE=yes
+

Tests were not executed in mock because there were missing build
dependencies. Updated patch is attached.

LS



ACK
Pushed to master: 559420562839b7cfe8e9af45bf22a3e9755963a7

-- 
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

[Freeipa-devel] [PATCHES 151-153] ipasam: fix wrong usage of talloc_new()

2015-11-18 Thread Sumit Bose
Hi,

please find attached 3 small patches for ipasam. The first fixes
https://fedorahosted.org/freeipa/ticket/5457 . The second is related
because if the compat tree is enabled the lookup will still fails
because the group is found twice.

The third patch fixes an issue valgrind found while I was checking the
other issue.

bye,
Sumit
From 8b4025136271f158ae50172cdbc6dca4fbe2ee65 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Wed, 18 Nov 2015 12:29:43 +0100
Subject: [PATCH 151/153] ipasam: fix wrong usage of talloc_new()

Fixes https://fedorahosted.org/freeipa/ticket/5457
---
 daemons/ipa-sam/ipa_sam.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
96452302f31e7a102b0ae2c17f343c75014b987b..37b5cf9140b9507e189363bff4c56cb33a82a89b
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -3029,7 +3029,7 @@ static int ipasam_get_sid_by_gid(struct ldapsam_privates 
*ldap_state,
enum idmap_error_code err;
struct unixid id;
 
-   tmp_ctx = talloc_new("ipasam_get_sid_by_gid");
+   tmp_ctx = talloc_init("ipasam_get_sid_by_gid");
if (tmp_ctx == NULL) {
return ENOMEM;
}
-- 
2.4.3

From deea949d98bff62493dd9048f0de3b5986103534 Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Wed, 18 Nov 2015 12:31:26 +0100
Subject: [PATCH 152/153] ipasam: use more restrictive search filter for group
 lookup

Since we are interested in looking up the SID of a group it makes sense
to include the objectclass which contains the SID attribute in the
search filter. This makes sure the group is not accidentally found a
second time in the compat tree.

Related to https://fedorahosted.org/freeipa/ticket/5457
---
 daemons/ipa-sam/ipa_sam.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
37b5cf9140b9507e189363bff4c56cb33a82a89b..60e73374df6c3e1cda8287069bc062101439fc64
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -3034,9 +3034,11 @@ static int ipasam_get_sid_by_gid(struct ldapsam_privates 
*ldap_state,
return ENOMEM;
}
 
-   filter = talloc_asprintf(tmp_ctx, "(&(%s=%s)(%s=%lu))",
+   filter = talloc_asprintf(tmp_ctx, "(&(%s=%s)(%s=%s)(%s=%lu))",
  LDAP_ATTRIBUTE_OBJECTCLASS,
  LDAP_OBJ_POSIXGROUP,
+ LDAP_ATTRIBUTE_OBJECTCLASS,
+ LDAP_OBJ_GROUPMAP,
  LDAP_ATTRIBUTE_GIDNUMBER,
  (unsigned long) gid);
if (filter == NULL) {
-- 
2.4.3

From 08195df7a456cacd6438b8e75be8dc215c6c780c Mon Sep 17 00:00:00 2001
From: Sumit Bose 
Date: Wed, 18 Nov 2015 12:34:49 +0100
Subject: [PATCH 153/153] ipasam: fix a use-after-free issue

Since endptr points to a location inside of dummy, dummy should be freed
only after dereferencing endptr.
---
 daemons/ipa-sam/ipa_sam.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index 
60e73374df6c3e1cda8287069bc062101439fc64..c51316757a80994a3992ab29f52e21b5a2c5673c
 100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -2105,11 +2105,12 @@ static bool get_uint32_t_from_ldap_msg(struct 
ldapsam_privates *ldap_state,
}
 
l = strtoul(dummy, &endptr, 10);
-   TALLOC_FREE(dummy);
 
if (l < 0 || l > UINT32_MAX || *endptr != '\0') {
+   TALLOC_FREE(dummy);
return false;
}
+   TALLOC_FREE(dummy);
 
*val = l;
 
-- 
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