On 23/07/14 15:30, Rob Crittenden wrote:
Martin Basti wrote:
This patch fixes ordering problem of schema updates

Martin should it be in IPA 4.0.x ? It requires rebased ldap_python (will
be in Fedora 21)

Patch attached
It looks like the modlist is only generated during a live run which
would diminish the utility of the --test mode.

Is it safe to assume this was done on purpose because schema changes are
being done incrementally vs done all at once, so that parent classes may
not exist in test mode?

rob
My bad, I fixed it. Now modlist will show  updated data  in --test mode too.

Data are not updated to LDAP in test mode, so there is no restrictions to have a proper order of classes, and could be written all at once.

Updated patch attached.

--
Martin Basti

From cfc259534ea5d8aaff2c57f66cf41a87864e1fcd Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 23 Jul 2014 14:42:33 +0200
Subject: [PATCH] FIX: ldap_updater needs correct orderng of the updates

Required bugfix in python-ldap 2.4.15

Updates must respect SUP objectclasses/attributes and update
dependencies first
---
 freeipa.spec.in                   |   2 +-
 ipaserver/install/schemaupdate.py | 116 ++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 38 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 447b532b66a0329a5715aca98222ab0ef1aebee4..202df6a6b85d62b17a711014261f69f86c9763df 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -98,7 +98,7 @@ Requires: httpd >= 2.4.6-6
 Requires: mod_wsgi
 Requires: mod_auth_kerb >= 5.4-16
 Requires: mod_nss >= 1.0.8-26
-Requires: python-ldap
+Requires: python-ldap >= 2.4.15
 Requires: python-krbV
 Requires: acl
 Requires: python-pyasn1
diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py
index bb2f0f161499c0358452d97f27f5c39b9b64a5ad..3dbcf7882e410af5a08fe23d23677ad5d0ea01c4 100644
--- a/ipaserver/install/schemaupdate.py
+++ b/ipaserver/install/schemaupdate.py
@@ -29,17 +29,58 @@ from ipaserver.install.ldapupdate import connect
 from ipaserver.install import installutils
 
 
-SCHEMA_ELEMENT_CLASSES = {
+SCHEMA_ELEMENT_CLASSES = (
     # All schema model classes this tool can modify
-    'objectclasses': ldap.schema.models.ObjectClass,
-    'attributetypes': ldap.schema.models.AttributeType,
-}
+    # Depends on order, attributes first, then objectclasses
+    ('attributetypes', ldap.schema.models.AttributeType),
+    ('objectclasses', ldap.schema.models.ObjectClass),
+)
+
+SCHEMA_ELEMENT_CLASSES_KEYS = (x[0] for x in SCHEMA_ELEMENT_CLASSES)
 
 ORIGIN = 'IPA v%s' % ipapython.version.VERSION
 
 log = log_mgr.get_logger(__name__)
 
 
+def _get_oid_dependency_order(schema, cls):
+        """
+        Returns a ordered list of OIDs sets, in order which respects inheritance in LDAP
+        OIDs in second set, depend on first set, etc.
+
+        :return [set(1st-tree-level), set(2nd-tree-level), ...]
+        """
+        top_node = '_'
+        ordered_oid_groups = []
+
+        tree = schema.tree(cls)  # tree structure of schema
+
+        # remove top_node from tree, it breaks ordering
+        # we don't need this, tree from file is not consistent
+        del tree[top_node]
+        unordered_oids = tree.keys()
+
+        # split into two groups, parents and child nodes, and iterate until
+        # child nodes are not empty
+        while unordered_oids:
+            parent_nodes = set()
+            child_nodes = set()
+
+            for node in unordered_oids:
+                if node not in child_nodes:
+                    # if node was child once, must remain as child
+                    parent_nodes.add(node)
+                for child_oid in tree[node]:
+                    # if any node is child, must be removed from parents
+                    parent_nodes.discard(child_oid)
+                    child_nodes.add(child_oid)
+
+            ordered_oid_groups.append(parent_nodes)
+            unordered_oids = child_nodes
+
+        return ordered_oid_groups
+
+
 def update_schema(schema_files, ldapi=False, dm_password=None, live_run=True):
     """Update schema to match the given ldif files
 
@@ -69,57 +110,58 @@ def update_schema(schema_files, ldapi=False, dm_password=None, live_run=True):
     old_schema = conn.schema
 
     schema_entry = conn.get_entry(DN(('cn', 'schema')),
-                                  SCHEMA_ELEMENT_CLASSES.keys())
+                                  SCHEMA_ELEMENT_CLASSES_KEYS)
 
     modified = False
 
     # The exact representation the DS gives us for each OID
     # (for debug logging)
     old_entries_by_oid = {cls(str(attr)).oid: str(attr)
-                          for attrname, cls in SCHEMA_ELEMENT_CLASSES.items()
+                          for (attrname, cls) in SCHEMA_ELEMENT_CLASSES
                           for attr in schema_entry[attrname]}
 
     for filename in schema_files:
         log.info('Processing schema LDIF file %s', filename)
         dn, new_schema = ldap.schema.subentry.urlfetch(filename)
 
-        for attrname, cls in SCHEMA_ELEMENT_CLASSES.items():
+        for attrname, cls in SCHEMA_ELEMENT_CLASSES:
+            for oids_set in _get_oid_dependency_order(new_schema, cls):
+                # Set of all elements of this class, as strings given by the DS
+                new_elements = []
+                for oid in oids_set:
+                    new_obj = new_schema.get_obj(cls, oid)
+                    old_obj = old_schema.get_obj(cls, oid)
+                    # Compare python-ldap's sanitized string representations
+                    # to see if the value is different
+                    # This can give false positives, e.g. with case differences
+                    # in case-insensitive names.
+                    # But, false positives are harmless (and infrequent)
+                    if not old_obj or str(new_obj) != str(old_obj):
+                        # Note: An add will automatically replace any existing
+                        # schema with the same OID. So, we only add.
+                        value = add_x_origin(new_obj)
+                        new_elements.append(value)
 
-            # Set of all elements of this class, as strings given by the DS
-            new_elements = []
+                        if old_obj:
+                            old_attr = old_entries_by_oid.get(oid)
+                            log.info('Replace: %s', old_attr)
+                            log.info('   with: %s', value)
+                        else:
+                            log.info('Add: %s', value)
 
-            for oid in new_schema.listall(cls):
-                new_obj = new_schema.get_obj(cls, oid)
-                old_obj = old_schema.get_obj(cls, oid)
-                # Compare python-ldap's sanitized string representations
-                # to see if the value is different
-                # This can give false positives, e.g. with case differences
-                # in case-insensitive names.
-                # But, false positives are harmless (and infrequent)
-                if not old_obj or str(new_obj) != str(old_obj):
-                    # Note: An add will automatically replace any existing
-                    # schema with the same OID. So, we only add.
-                    value = add_x_origin(new_obj)
-                    new_elements.append(value)
-
-                    if old_obj:
-                        old_attr = old_entries_by_oid.get(oid)
-                        log.info('Replace: %s', old_attr)
-                        log.info('   with: %s', value)
-                    else:
-                        log.info('Add: %s', value)
-
-            modified = modified or new_elements
-            schema_entry[attrname].extend(new_elements)
+                modified = modified or new_elements
+                schema_entry[attrname].extend(new_elements)
+                # we need to iterate schema updates, due to dependencies (SUP)
+                # schema_entry doesn't respect order of objectclasses/attributes
+                # so updates must be executed with groups of independent OIDs
+                if live_run:
+                    conn.update_entry(schema_entry)
 
     # FIXME: We should have a better way to display the modlist,
     # for now display raw output of our internal routine
     modlist = schema_entry.generate_modlist()
-    log.debug("Complete schema modlist:\n%s", pprint.pformat(modlist))
-
-    if modified and live_run:
-        conn.update_entry(schema_entry)
-    else:
+    log.debug("Schema modlist:\n%s", pprint.pformat(modlist))
+    if not (modified and live_run):
         log.info('Not updating schema')
 
     return modified
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to