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

[Petr Viktorin]

On 05/25/2012 09:26 AM, Martin Kosek wrote:
> On Fri, 2012-05-25 at 09:20 +0200, Petr Vobornik wrote:
>> 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.
>
> I assume this is a cause and would need to be fixed in Petr3's patch:
>
>   847                 # Add a submount key
>   848                 self.api.Command['automountkey_add'](
>   849                     location, parentmap, automountkey=key, key=key,
>   850                     automountinformation='-fstype=autofs ldap:%s' %
> map)
>
> Martin
>

Thanks!
Fixed and rebased. I'll test this code path in a separate patch.

-- 
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0050-04-Fail-on-unknown-Command-options.patch
Type: text/x-patch
Size: 16076 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120525/1a160846/attachment.bin>