[Freeipa-devel] [PATCHES] 0546-0547 Allow alternate "aci" keyword in ACIs

[Petr Viktorin]

On 04/30/2014 07:25 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> Hello,
>> The first patch adds "==" to ACI object to simplify comparisons.
>> The second patch moves existing "tests" to the test suite.
>>
>> The third patch adds support for an alternate "aci" keyword that DS
>> supports (but I couldn't get any documentaion on it). Dogtag adds ACIs
>> with this keyword to cn=config, so we'll need this fix when parsing ACIs
>> there.
>>
>>
>> Rob, you wrote the parser; does this look OK to you?
>>
>
> ACK.
>
> Only minor quibble is you left a couple of print statements in the tests.

Those are intentional. When a failed test prints something, Nose shows 
the output. It's pretty nice for debugging.

> As you note, I had some "tests" that I ran when I was implementing the
> aci module. Moving these to formal testing is definitely the right thing
> to do.
>
> I do wonder one thing though. In the equality test I had reversed some
> ordering of things to ensure that things were normalized in the same
> way. For the check_aci_parsing() tests is it worth considering doing
> something similar?

I'm not sure I follow; what part are you referring to?

The ordering is problematic for sure; everything relies on dict order.
That's the main reason why in my work I only use this class to parse 
ACIs, and I made a stable routine for generating them.

Until we start running the tests with PYTHONHASHSEED=random, I figured 
we can just use them as they are.

> I noticed that we are apparently not normalizing target filters because
> there is a space in the DN. Something for later.

Or not; I'd like to get rid of the export_to_string part completely.

> There is no ticket. Probably fine since this is mostly just shuffling
> deck chairs.

Yeah, same thinking here. It's part of the general ACI work.

-- 
Petr³