[Freeipa-devel] [PATCH] 114 Fix sudo help and summaries

[Rob Crittenden]

Martin Kosek wrote:
> On Fri, 2011-08-26 at 09:31 +0200, Martin Kosek wrote:
>> On Thu, 2011-08-25 at 09:28 -0400, Rob Crittenden wrote:
>>> Martin Kosek wrote:
>>>> 1) Add sudorule docstring headline
>>>>
>>>> 2) Fix naming inconsistency in Sudo plugins help and summaries,
>>>>      especially capitalization of Sudo objects - Sudo Rule, Sudo
>>>>      Command and Sudo Command Group
>>>>
>>>> 3) Add missing summaries for sudorule-add-option and
>>>>      sudorule-remove-option
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/1595
>>>> https://fedorahosted.org/freeipa/ticket/1596
>>>
>>> This breaks compatibility with old clients:
>>>
>>> $ ipa sudorule-add-option test2
>>> Sudo Option: foo
>>> ipa: ERROR: non-public: ValueError:
>>> sudorule_add_option.validate_output(): unexpected keys ['summary'] in
>>> {'result': {'ipasudoopt': (u'foo',), 'cn': (u'test2',),
>>> 'ipaenabledflag': (u'TRUE',)}, 'summary': u'Added option "foo" to Sudo
>>> Rule "test2"'}
>>> Traceback (most recent call last):
>>>     File "/usr/lib/python2.7/site-packages/ipalib/backend.py", line 125,
>>> in execute
>>>       result = self.Command[_name](*args, **options)
>>>     File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 443,
>>> in __call__
>>>       self.validate_output(ret)
>>>     File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 895,
>>> in validate_output
>>>       nice, extra, output)
>>> ValueError: sudorule_add_option.validate_output(): unexpected keys
>>> ['summary'] in {'result': {'ipasudoopt': (u'foo',), 'cn': (u'test2',),
>>> 'ipaenabledflag': (u'TRUE',)}, 'summary': u'Added option "foo" to Sudo
>>> Rule "test2"'}
>>> ipa: ERROR: an internal error has occurred
>>>
>>
>> Thanks for catching this. I wonder if we should let output param
>> validation skip unexpected keys in order to be able to do the change in
>> Output + keep backwards compatibility in cases like this one.
>>
>> I reworked the patch so that the summaries are printed via
>> output_for_cli() - this solves this problem.
>>
>> Martin
>
> I had to rework the whole patch because of latest i18n patches. Here it
> is, it can be applied to current master.
>
> Martin

ACK

Fixed a couple of whitespace issues and pushed to master and ipa-2-1

rob