[Freeipa-devel] [PATCH] 0050 Fail on unknown Command options

[Petr Vobornik]

On 05/16/2012 02:11 PM, Martin Kosek wrote:
> On Wed, 2012-05-16 at 10:37 +0200, Petr Viktorin wrote:
>> On 05/16/2012 09:58 AM, Martin Kosek wrote:
>>> On Tue, 2012-05-15 at 13:35 +0200, Petr Viktorin wrote:
>>>> On 05/15/2012 09:55 AM, Martin Kosek wrote:
>>>>> On Mon, 2012-05-14 at 14:47 +0200, Petr Viktorin wrote:
>>>>>> The final part of rejecting unknown Command arguments: enable the
>>>>>> validation, add tests.
>>>>>> Also fix up things that were changed since the previous patches.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/2509
>>>>>>
>>>>>
>>>>> The patch looks OK so far. I just found an error in permission/aci
>>>>> plugin - --subtree does not work when it matches a result:
>>>>>
>>>>> # ipa permission-find --subtree=foo
>>>>> ---------------------
>>>>> 0 permissions matched
>>>>> ---------------------
>>>>> ----------------------------
>>>>> Number of entries returned 0
>>>>> ----------------------------
>>>>>
>>>>>     ipa permission-find
>>>>> --subtree='ldap:///ipauniqueid=*,cn=hbac,dc=idm,dc=lab,dc=bos,dc=redhat,dc=Com'
>>>>> ipa: ERROR: Unknown option: subtree
>>>>
>>>> Attaching fixed patch.
>>>>
>>>>> We should not pass **options to aci_show, it is too risky. There may be
>>>>> other places where we don't use an option-safe approach that we want to
>>>>> have fixed.
>>>>
>>>> We shouldn't really pass **options to any command; listing everything
>>>> explicitly would be much safer. Unfortunately, in a lot of cases where
>>>> commands call other commands, it's currently done this way.
>>>>
>>>
>>>
>>> Martin
>>>
>>
>> Attaching a rebased patch.
>>
>
> Yup, this one is fine. Now, I did not find issues in the patch itself,
> tests are clean.
>
> However, thanks to this new check I found issues in Web UI (automember,
> selfservice, delegation screen) which use illegal options and which
> should be fixed before we push your patch:
>
> https://fedorahosted.org/freeipa/ticket/2760
>
> Martin
>

I found an issue in automountmap_add_indirect. It complains that 'key' 
is unknown option.
-- 
Petr Vobornik