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

[Martin Kosek]

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