[Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type

[Martin Kosek]

On 06/24/2013 04:22 PM, Petr Viktorin wrote:
> On 06/20/2013 12:56 PM, Tomas Babej wrote:
>> On 06/17/2013 02:34 PM, Ana Krivokapic wrote:
>>> On 06/06/2013 11:10 AM, Tomas Babej wrote:
>>>> Hi,
>>>>
>>>> Adds --use-posix option to ipa trust-add command. It takes two
>>>> allowed values:
>>>> 'yes' : the 'ipa-ad-trust-posix' range type is enforced
>>>> 'no' : the 'ipa-ad-trust' range type is enforced
>>>>
>>>> When --use-posix option is not specified, the range type should be
>>>> determined by ID range discovery.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/3650
>>>>
>>>> Tomas
>>>>
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>
>>> The patch works nicely, but I have a few comments:
>>>
>>> 1) You added a new option to the API, but you forgot to bump the
>>> IPA_API_VERSION_MINOR in the VERSION file.
>>>
>>> 2) Typo in commit message: "shold" instead of "should".
>>>
>>> 3) This construct:
>>>
>>> +            if range_type is not None:
>>> +                if range_type != old_range_type:
>>>
>>> can be replaced with a more readable variant which also avoids nested ifs:
>>>
>>> +            if range_type and range_type != old_range_type:
>>>
>>
>> All fixed.
>>
>>> 4) Some unit tests to cover the behavior of the newly added option
>>> would be nice.
>>
>> This is not doable at the moment, we have no unit test framework to test
>> the trust-add command.
>>
>>> --
>>> Regards,
>>>
>>> Ana Krivokapic
>>> Associate Software Engineer
>>> FreeIPA team
>>> Red Hat Inc.
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> Tomas
>
> I don't know much about AD trusts, but a command-line/API option that takes a
> 'yes' or 'no' string raised a tiny warning flag for me.
>
> It looks like it's possible that there can be other range types in the future
> than posix and algorithmic? If that's the case, there should be a --range-type
> option instead. (If not, I'd still go for --range-type but that would just be
> bikeshedding.)
>
> In any case I think an explicit 'auto' option would be nice.
>
> But that's just an outsider's view, maybe --use-posix makes more sense.
>
>
> AFAIK, for CLI changes there should be a a design page; is this covered anywhere?

It should be covered in the parent RFE ticket:
https://fedorahosted.org/freeipa/ticket/2904

I see it is not there. This is still a task for Tomas or Alexander.

Martin