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

[Tomas Babej]

On 05/29/2013 12:25 PM, Martin Kosek wrote:
> On 05/28/2013 03:48 PM, Alexander Bokovoy wrote:
>> 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.
>>
> May-be-NACK.
>
> Would it make sense to replace the error with DependentEntry error? We use in
> cases like this elsewhere and I think it makes more sense in this case too.
>
> Martin

Sure, I changed the error class in idrange.py and tests accordingly.

I ran the unit tests again to verify the changes.

Here is the updated patch.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0057-6-Do-not-allow-removal-of-ID-range-of-an-active-trust.patch
Type: text/x-patch
Size: 8697 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130530/cc7ba5d1/attachment.bin>