[Freeipa-devel] [PATCHES 0024-0025] Improvements to idrange.py

[Tomas Babej]

On 02/21/2013 02:22 PM, Martin Kosek wrote:
> On 02/20/2013 03:19 PM, Tomas Babej wrote:
>> On Wed 20 Feb 2013 02:24:03 PM CET, Alexander Bokovoy wrote:
>>> On Wed, 20 Feb 2013, Tomas Babej wrote:
>>>> On 12/21/2012 12:15 PM, Tomas Babej wrote:
>>>>> Hi,
>>>>>
>>>>> Sending updated and rebased versions of patches 0024 and 0025.
>>>>>
>>>>> Tomas
>>>>>
>>>>>
>>>> Sending rebased version, these got quite rotten.
>>> Thanks for updating them.
>>>
>>>> @@ -504,25 +515,37 @@ class idrange_mod(LDAPUpdate):
>>>>                              'not be found. Please specify the SID
>>>> directly '
>>>>                              'using dom-sid option.'))
>>>>
>>>> -        try:
>>>> -            (old_dn, old_attrs) = ldap.get_entry(dn,
>>>> -                                                ['ipabaseid',
>>>> -                                                'ipaidrangesize',
>>>> -                                                'ipabaserid',
>>>> -                                                'ipasecondarybaserid'])
>>>> -        except errors.NotFound:
>>>> -            self.obj.handle_not_found(*keys)
>>>> +        if in_updated_attrs('ipanttrusteddomainsid'):
>>>> +            if in_updated_attrs('ipasecondarybaserid'):
>>>> +                raise errors.ValidationError(name='ID Range setup',
>>>> +                    error=_('Options dom_sid and secondary_rid_base
>>>> cannot '
>>>> +                            'be used together'))
>>> Since we agreed to refer to options by their CLI name (--dom-sid and
>>> --secondary-rid-base) in the other patch, it makes sense to use it
>>> here too.
>>>
>>>
>>>> -        if is_set('ipanttrusteddomainsid'):
>>>> -            # Validate SID as the one of trusted domains
>>>> -
>>>> self.obj.validate_trusted_domain_sid(entry_attrs['ipanttrusteddomainsid'])
>>>>
>>>> +            if not in_updated_attrs('ipabaserid'):
>>>> +                raise errors.ValidationError(name='ID Range setup',
>>>> +                    error=_('Options dom_sid and rid_base must '
>>>> +                            'be used together'))
>>> Same here.
>>>
>>>> +            # secondary base rid must be set if and only if base rid
>>>> is set
>>>> +            if in_updated_attrs('ipasecondarybaserid') !=\
>>>> +                in_updated_attrs('ipabaserid'):
>>>> +                raise errors.ValidationError(name='ID Range setup',
>>>> +                    error=_('Options secondary_rid_base and rid_base
>>>> must '
>>>> +                            'be used together'))
>>> Same here.
>>>
>>>> +        dict(
>>>> +            desc='Try to modify ID range %r so it has only primary
>>>> rid range set' % (testrange8),
>>>> +            command=('idrange_mod', [testrange8],
>>>> +                      dict(ipabaserid=testrange8_base_rid)),
>>>> +            expected=errors.ValidationError(
>>>> +                name='ID Range setup', error='Options
>>>> secondary_rid_base and rid_base must be used together'),
>>>> +        ),
>>> And synchronize error message here too.
>>>
>> Thanks!
>>
>> Sending the updated patch 0024.
>>
>> Tomas
>>
> In patch 0024 your intention is OK, but the checking functions are not:
>
>           is_set = lambda x: (x in entry_attrs) and (x is not None)
> +        in_updated_attrs = lambda x: any((x in attrs and x is not None)
> +                                         for attrs in (entry_attrs, old_attrs))
>
> They return True even when the attribute is None because they check if *x* is
> None and not if *attrs[x]* is None. Example:
>
> # ipa idrange-add --base-id=1200000 --range-size=200000 --rid-base=1000 --secondary-rid-base=1000000 local_range
> ----------------------------
> Added ID range "local_range"
> ----------------------------
>    Range name: local_range
>    First Posix ID of the range: 1200000
>    Number of IDs in the range: 200000
>    First RID of the corresponding RID range: 1000
>    First RID of the secondary RID range: 1000000
>    Range type: local domain range
>
> This command should be NOOP, but is not:
>
> # ipa idrange-mod local_range --dom-sid=
> ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base
> cannot be used together
>
> Martin
Thanks for the catch, all checking functions fixed.

Sending the complete patchset, up to date.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0025-6-Add-trusted-domain-range-objectclass-when-using-idra.patch
Type: text/x-patch
Size: 1325 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130222/a320a283/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0024-6-Make-options-checks-in-idrange-add-mod-consistent.patch
Type: text/x-patch
Size: 9505 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130222/a320a283/attachment-0001.bin>