[Freeipa-devel] [PATCH] 298 Add safe updates for objectClasses

[Martin Kosek]

On 09/05/2012 09:22 AM, Martin Kosek wrote:
> On 09/05/2012 03:47 AM, Rob Crittenden wrote:
>> Rob Crittenden wrote:
>>> Martin Kosek wrote:
>>>> On 08/30/2012 02:53 PM, Rob Crittenden wrote:
>>>>> Martin Kosek wrote:
>>>>>> Current objectclass updates in a form of "replace" update instruction
>>>>>> dependent on exact match of the old object class specification in the
>>>>>> update instruction and the real value in LDAP. However, this
>>>>>> approach is
>>>>>> very error prone as object class definition can easily differ as for
>>>>>> example because of unexpected X-ORIGIN value. Such objectclass update
>>>>>> failures may lead to serious malfunctions later.
>>>>>>
>>>>>> Add new update instruction type "replaceoc" with the following format:
>>>>>> replaceoc:OID:new
>>>>>> This update instruction will always replace an objectclass with
>>>>>> specified OID with the new definition.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/2440
>>>>>
>>>>> This works ok. Martin and I had a conversation in IRC about it.
>>>>>
>>>>> This moves from replacing a specific bit of schema with a new one, in
>>>>> all
>>>>> cases. I wonder if we should be more conservative and know what we're
>>>>> replacing
>>>>> in advance.
>>>>>
>>>>> rob
>>>>>
>>>>
>>>> You are right, I was too harsh when replacing the objectclasses. This
>>>> would
>>>> cause issues when LDAP update would be run on a replica with lower
>>>> version and
>>>> older objectclass definitions.
>>>>
>>>> I came up with an alternative solution and instead of always replacing
>>>> the
>>>> objectclass I rather reverted to old-OC:new-OC style which should be
>>>> safer.
>>>> Now, the LDAP updater always normalizes an objectclass before
>>>> comparing it
>>>> using python-ldap objectclass model. With this approach, objectclasses
>>>> differing only in X-ORIGIN or white spaces should match and be updated.
>>>>
>>>> Martin
>>>>
>>>
>>> NACK
>>>
>>> I think this:
>>>
>>> +                            for value in replaced_values:
>>> +                                entry_values.remove(old)
>>>
>>> Should be:
>>>
>>> +                                entry_values.remove(value)
> 
> Right. Thanks for the fix. It only worked in my testing because I had no
> objectclass update which would differ in X-ORIGIN or whitespaces or case. I
> tried to mangle an update file and with this fix it did the update even if
> X-ORIGIN and whitespaces differed.
> 
>>>
>>> I'm still doing other testing but this is what I've found so far.
>>>
>>> rob
>>
>> I did some more testing and it looks like this will do the trick.
>>
>> I also found a place where the schema was left as unicode and causing it to
>> blow up inside python-ldap. Here is the diff on my working instance:
>>
>> diff -u  ipaserver/install/ldapupdate.py
>> /usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py
>> --- ipaserver/install/ldapupdate.py     2012-09-04 16:59:33.210688723 -0400
>> +++ /usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py 2012-09-04
>> 21:47:01.583574375 -0400
>> @@ -643,7 +643,7 @@
>>                                  self.debug('replace: no match for replaced
>> ObjectClass "%s"', old)
>>                                  continue
>>                              for value in replaced_values:
>> -                                entry_values.remove(old)
>> +                                entry_values.remove(value)
>>                          else:
>>                              entry_values.remove(old)
>>                          entry_values.append(new)
>> @@ -772,7 +772,11 @@
>>                  updated = False
>>                  changes = self.conn.generateModList(entry.origDataDict(),
>> entry.toDict())
>>                  if (entry.dn == DN(('cn', 'schema'))):
>> -                    updated = self.is_schema_updated(entry.toDict())
>> +                    d = dict()
>> +                    e = entry.toDict()
>> +                    for k,v in e.items():
>> +                        d[k] = [str(x) for x in v]
>> +                    updated = self.is_schema_updated(d)
>>                  else:
>>                      if len(changes) >= 1:
>>                          updated = True
>>
>> rob
>>
> 
> I did not hit this error during testing, but at least it won't harm. Sending an
> updated patch.
> 

Sending a slightly updated patch which now makes sure that we always pass
objectclass of str (and not unicode) type to ObjectClass model. This should
make it more bullet-proof.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-298-4-add-safe-updates-for-objectclasses.patch
Type: text/x-patch
Size: 6768 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120905/a9cb27a7/attachment.bin>