[Freeipa-devel] [PATCHES] Improve framework parameter validation

[Jan Cholasta]

On 27.3.2012 16:00, Martin Kosek wrote:
> On Thu, 2012-03-15 at 14:57 +0100, Jan Cholasta wrote:
>> On 15.3.2012 14:20, Petr Viktorin wrote:
>>> On 03/15/2012 12:05 PM, Jan Cholasta wrote:
>>>> On 15.3.2012 11:36, Jan Cholasta wrote:
>>>>> (this is a continuation of
>>>>> <http://www.redhat.com/archives/freeipa-devel/2011-September/msg00327.html>)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> the attached patches fix<https://fedorahosted.org/freeipa/ticket/1847>
>>>>> and<https://fedorahosted.org/freeipa/ticket/2245>:
>>>>>
>>>>> [PATCH] Fix the procedure for getting default values of command
>>>>> parameters.
>>>>>
>>>>> The parameters used in default_from of other parameters are now properly
>>>>> validated before the default_from is called.
>>>>>
>>>>> [PATCH] Change parameters to use only default_from for dynamic default
>>>>> values.
>>>>>
>>>>> Replace all occurences of create_default with equivalent default_from
>>>>> and remove create_default from the framework. This is needed for proper
>>>>> parameter validation, as there is no way to tell which parameters to
>>>>> validate prior to calling create_default, because create_default does
>>>>> not provide information about which parameters are used for generating
>>>>> the default value.
>>>>>
>>>>> Honza
>>>>>
>>>>
>>>> Forgot to remove one FIXME bit in dns.py. Update patch attached.
>>>>
>>>> Honza
>>>
>>>
>>>   >  diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
>>>   >  index a10960a..61c645d 100644
>>>   >  --- a/ipalib/plugins/dns.py
>>>   >  +++ b/ipalib/plugins/dns.py
>>>   >  @@ -1528,7 +1528,7 @@ class dnszone(LDAPObject):
>>>   >  label=_('SOA serial'),
>>>   >  doc=_('SOA record serial number'),
>>>   >  minvalue=1,
>>>   >  - create_default=_create_zone_serial,
>>>   >  + default_from=_create_zone_serial,
>>>   >  autofill=True,
>>>   >  ),
>>>   >  Int('idnssoarefresh',
>>>   >  diff --git a/ipalib/plugins/passwd.py b/ipalib/plugins/passwd.py
>>>   >  index b26f7e9..9bee314 100644
>>>   >  --- a/ipalib/plugins/passwd.py
>>>   >  +++ b/ipalib/plugins/passwd.py
>>>   >  @@ -69,7 +69,7 @@ class passwd(Command):
>>>   >  label=_('User name'),
>>>   >  primary_key=True,
>>>   >  autofill=True,
>>>   >  - create_default=lambda **kw: util.get_current_principal(),
>>>   >  + default_from=lambda: util.get_current_principal(),
>>>   >  normalizer=lambda value: normalize_principal(value),
>>>   >  ),
>>>   >  Password('password',
>>>
>>>
>>> This is just a minor nitpick, but I'd like to know if there's a reason
>>> behind it: why are you sometimes using lambda and sometimes not?
>>
>> I use lambda as a protective measure against accidents caused by adding
>> optional arguments to the functions used. _create_zone_serial is an
>> exception to that rule, because it is private to the dns plugin.
>>
>>>
>>> The patch works well here, but I think I'm not the one to ack it.
>>>
>>
>> Honza
>>
>
> The patch looks OK, I found just minor issues.
>
> 1) We may want to add some check for wildcards (**kw) in default_from, I
> guess it would mess with your dependency solver. Some nice error would
> warn developers that they are doing something bad.

Added the check.

>
> 2) Patch 47.4 needs minor rebasing

Done.

>
> Martin
>

Updated patches attached.

Honza

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-44.5-parameter-validation.patch
Type: text/x-patch
Size: 10028 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120327/065303f3/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-47.5-parameter-remove-create-default.patch
Type: text/x-patch
Size: 13562 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120327/065303f3/attachment-0001.bin>