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

[Martin Kosek]

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