On 02/22/2013 03:01 PM, Tomas Babej wrote: > 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
I think I am being a nitpicker now (sorry), but this condition also fails if the old_attrs has a setting, but I am removing it in a current -mod command: # 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 # ipa idrange-mod local_range --dom-sid S-1-2 --secondary-rid-base= ipa: ERROR: invalid 'ID Range setup': Options dom-sid and secondary-rid-base cannot be used together Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel