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

[Rob Crittenden]

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)
>
> 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