[Freeipa-devel] [PATCH 0057] Do not allow removal of ID range of an active trust

[Alexander Bokovoy]

On Tue, 28 May 2013, Tomas Babej wrote:
>On 05/28/2013 02:35 PM, Alexander Bokovoy wrote:
>>On Mon, 27 May 2013, Tomas Babej wrote:
>>>>>>We got rid of openldap utilities now. While using 
>>>>>>python.ldap module, I also made the tests much more robust 
>>>>>>and added a new test case.
>>>>>In general patches look fine, there is one small nitpick.
>>>>>I'll run tests on Monday and then will provide final ACK.
>>>>>
>>>>>>--- a/tests/test_xmlrpc/test_range_plugin.py
>>>>>>+++ b/tests/test_xmlrpc/test_range_plugin.py
>>>>>>@@ -22,66 +22,171 @@ Test the `ipalib/plugins/idrange.py` 
>>>>>>module, and XML-RPC in general.
>>>>>>"""
>>>>>>
>>>>>>from ipalib import api, errors, _
>>>>>>+from ipapython.ipautil import run
>>>>>This import is unused, can be removed.
>>>>>
>>>>Fixed, thanks for catching that.
>>>>
>>>>Updated patch attached.
>>So I tried to run this test on a machine where there is already trust
>>established and I think there should be done some changes.
>
>I perused the log. Seems that the failures you're experiencing are 
>not relevant to the patch itself,
>since the newly added tests passed.
>
>This is problem with test_range_plugin.py tests that has been there 
>for quite a while, the parameters
>of the ranges such as size, and base ID/RID/secondary RID are 
>hardcoded in the test case.
Yep.


>>Probably it would be wise to add pre-start procedure to pull existing
>>ranges and define constants for the ranges so that they don't overlap
>>with existing ones. Perhaps selecting something from a top of the range
>>space...
>>
>>Attached is the log
>
>I agree. This has not been relevant until now, since we did not do 
>much testing on IPA instances
>with trusts set up, and even then there's random factor in having the 
>overlap with the already created
>trust range.
>
>I'd propose fixing this in a separate effort as a part of continouous 
>integration improvements. I see it
>as a separate issue of its own.
>
>What do you think?
Please file a separate ticket then.

ACK for this one.

-- 
/ Alexander Bokovoy