On 02/25/2013 02:16 PM, Tomas Babej wrote: > On Fri 22 Feb 2013 04:34:55 PM CET, Martin Kosek wrote: >> 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 > > Yep. Ok, the command should now handle this use case as well. > > Tomas
Thanks, it is working fine now. ACK. Pushed to master, ipa-3-1. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel