Server Upgrade: Update entries in order specified in file

Dictionary replaced with list. Particular upgrades are
executed in the same order as they are specified in update
a file.

Different updates for the smae cn, are not merged into one upgrade

https://fedorahosted.org/freeipa/ticket/4904

Reviewed-By: David Kupka <dkupka@redhat.com>

9 files changed, 30 insertions, 88 deletions

diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 92b6d56ae..3e4fc3f7a 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -137,22 +137,20 @@ class LDAPUpdate:
             4: 'cn=bob,ou=people,dc=example,dc=com',
         }
 
-        all_updates = {
-        'dn': 'cn=config,dc=example,dc=com':
+        all_updates = [
             {
                 'dn': 'cn=config,dc=example,dc=com',
                 'default': ['attr1':default1'],
                 'updates': ['action:attr1:value1',
                             'action:attr2:value2]
             },
-        'dn': 'cn=bob,ou=people,dc=example,dc=com':
             {
                 'dn': 'cn=bob,ou=people,dc=example,dc=com',
                 'default': ['attr3':default3'],
                 'updates': ['action:attr3:value3',
                             'action:attr4:value4],
             }
-        }
+        ]
 
         The default and update lists are "dispositions"
 
@@ -279,49 +277,6 @@ class LDAPUpdate:
         if fd != sys.stdin: fd.close()
         return text
 
-    def _combine_updates(self, all_updates, update):
-        'Combine a new update with the list of total updates'
-        dn = update.get('dn')
-        assert isinstance(dn, DN)
-
-        if not all_updates.get(dn):
-            all_updates[dn] = update
-            return
-
-        existing_update = all_updates[dn]
-        if 'default' in update:
-            disposition_list = existing_update.setdefault('default', [])
-            disposition_list.extend(update['default'])
-        elif 'updates' in update:
-            disposition_list = existing_update.setdefault('updates', [])
-            disposition_list.extend(update['updates'])
-        else:
-            self.debug("Unknown key in updates %s" % update.keys())
-
-    def merge_updates(self, all_updates, updates):
-        '''
-        Add the new_update dict to the all_updates dict.  If an entry
-        in the new_update already has an entry in all_updates merge
-        the two entries sensibly assuming the new entries take
-        precedence. Otherwise just add the new entry.
-        '''
-
-        for new_update in updates:
-            for new_dn, new_entry in new_update.iteritems():
-                existing_entry = all_updates.get(new_dn)
-                if existing_entry:
-                    # If the existing entry is marked for deletion but the
-                    # new entry is not also a delete then clear the delete
-                    # flag otherwise the newer update will be lost.
-                    if existing_entry.has_key('deleteentry') and not new_entry.has_key('deleteentry'):
-                        self.warning("ldapupdate: entry '%s' previously marked for deletion but" +
-                                     " this subsequent update reestablishes it: %s", new_dn, new_entry)
-                        del existing_entry['deleteentry']
-                    existing_entry.update(new_entry)
-                else:
-                    all_updates[new_dn] = new_entry
-
-
     def parse_update_file(self, data_source_name, source_data, all_updates):
         """Parse the update file into a dictonary of lists and apply the update
            for each DN in the file."""
@@ -380,11 +335,12 @@ class LDAPUpdate:
 
         def emit_update(update):
             '''
-            When processing a dn is completed emit the update by merging it into
-            the set of all updates.
+            When processing a dn is completed emit the update by appending it
+            into list of all updates
             '''
-
-            self._combine_updates(all_updates, update)
+            dn = update.get('dn')
+            assert isinstance(dn, DN)
+            all_updates.append(update)
 
         # Iterate over source input lines
         for source_line in source_data:
@@ -421,7 +377,6 @@ class LDAPUpdate:
                     continue
                 else:
                     emit_item(logical_line)
-                    logical_line = ''
                     logical_line = source_line
 
         if dn is not None:
@@ -784,11 +739,10 @@ class LDAPUpdate:
             raise RuntimeError("Offline updates are not supported.")
 
     def _run_updates(self, all_updates):
-
-        for dn, update in all_updates.iteritems():
+        for update in all_updates:
             self._update_record(update)
 
-        for dn, update in all_updates.iteritems():
+        for update in all_updates:
             self._delete_record(update)
 
     def update(self, files, ordered=False):
@@ -798,16 +752,14 @@ class LDAPUpdate:
         returns True if anything was changed, otherwise False
         """
         self.modified = False
-        all_updates = {}
+        all_updates = []
         try:
             self.create_connection()
             if self.plugins:
                 self.info('PRE_UPDATE')
                 updates = api.Backend.updateclient.update(PRE_UPDATE, self.dm_password, self.ldapi, self.live_run)
-                self.merge_updates(all_updates, updates)
                 # flush out PRE_UPDATE plugin updates before we begin
-                self._run_updates(all_updates)
-                all_updates = {}
+                self._run_updates(updates)
 
             upgrade_files = files
             if ordered:
@@ -823,13 +775,12 @@ class LDAPUpdate:
 
                 self.parse_update_file(f, data, all_updates)
                 self._run_updates(all_updates)
-                all_updates = {}
+                all_updates = []
 
             if self.plugins:
                 self.info('POST_UPDATE')
                 updates = api.Backend.updateclient.update(POST_UPDATE, self.dm_password, self.ldapi, self.live_run)
-                self.merge_updates(all_updates, updates)
-                self._run_updates(all_updates)
+                self._run_updates(updates)
         finally:
             self.close_connection()
 
diff --git a/ipaserver/install/plugins/adtrust.py b/ipaserver/install/plugins/adtrust.py
index 1290278cf..dbec429aa 100644
--- a/ipaserver/install/plugins/adtrust.py
+++ b/ipaserver/install/plugins/adtrust.py
@@ -65,11 +65,10 @@ class update_default_range(PostUpdate):
                        'iparangetype:ipa-local',
                       ]
 
-        updates = {}
         dn = DN(('cn', '%s_id_range' % api.env.realm),
                 api.env.container_ranges, api.env.basedn)
 
-        updates[dn] = {'dn': dn, 'default': range_entry}
+        update = {'dn': dn, 'default': range_entry}
 
         # Default range entry has a hard-coded range size to 200000 which is
         # a default range size in ipa-server-install. This could cause issues
@@ -115,7 +114,7 @@ class update_default_range(PostUpdate):
 
                 root_logger.error("default_range: %s", "\n".join(msg))
 
-        return (False, True, [updates])
+        return (False, True, [update])
 
 
 class update_default_trust_view(PostUpdate):
@@ -156,13 +155,12 @@ class update_default_trust_view(PostUpdate):
         # We have a server with AD trust support without Default Trust View.
         # Create the Default Trust View entry.
 
-        updates = {}
-        updates[default_trust_view_dn] = {
+        update = {
             'dn': default_trust_view_dn,
             'default': default_trust_view_entry
         }
 
-        return (False, True, [updates])
+        return (False, True, [update])
 
 api.register(update_default_range)
 api.register(update_default_trust_view)
diff --git a/ipaserver/install/plugins/ca_renewal_master.py b/ipaserver/install/plugins/ca_renewal_master.py
index e24663910..b0fb527a3 100644
--- a/ipaserver/install/plugins/ca_renewal_master.py
+++ b/ipaserver/install/plugins/ca_renewal_master.py
@@ -98,10 +98,8 @@ class update_ca_renewal_master(PostUpdate):
 
         dn = DN(('cn', 'CA'), ('cn', self.api.env.host), base_dn)
         update = {
-            dn: {
                 'dn': dn,
                 'updates': ['add:ipaConfigString: caRenewalMaster'],
-            },
         }
 
         return (False, True, [update])
diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 5f40be492..f562978bc 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -133,13 +133,11 @@ class update_dns_limits(PostUpdate):
         for limit in self.limit_attributes:
             limit_updates.append('only:%s:%s' % (limit, self.limit_value))
 
-        dnsupdates = {}
-        dnsupdates[dns_service_dn] = {'dn': dns_service_dn,
-                                      'updates': limit_updates}
+        dnsupdate = {'dn': dns_service_dn, 'updates': limit_updates}
         root_logger.debug("DNS: limits for service %s will be updated" % dns_service_dn)
 
 
-        return (False, True, [dnsupdates])
+        return (False, True, [dnsupdate])
 
 api.register(update_dns_limits)
 
diff --git a/ipaserver/install/plugins/rename_managed.py b/ipaserver/install/plugins/rename_managed.py
index 13e6dae5d..adb814c17 100644
--- a/ipaserver/install/plugins/rename_managed.py
+++ b/ipaserver/install/plugins/rename_managed.py
@@ -98,7 +98,8 @@ class GenerateUpdateMixin(object):
                     old_update = {'dn': entry.dn, 'deleteentry': None}
 
                     # Add the delete and replacement updates to the list of all updates
-                    update_list.append({entry.dn: old_update, new_dn: new_update})
+                    update_list.append(old_update)
+                    update_list.append(new_update)
 
             else:
                 # Update the template dn by replacing the old containter with the new container
@@ -125,11 +126,11 @@ class GenerateUpdateMixin(object):
                               'default': entry_to_update(entry)}
 
                 # Add the replacement update to the collection of all updates
-                update_list.append({new_dn: new_update})
+                update_list.append(new_update)
 
         if len(update_list) > 0:
             restart = True
-            update_list.sort(reverse=True)
+            update_list.sort(reverse=True, key=lambda x: x['dn'])
 
         return (restart, update_list)
 
diff --git a/ipaserver/install/plugins/update_passsync.py b/ipaserver/install/plugins/update_passsync.py
index d6595a06f..e0d2fc01c 100644
--- a/ipaserver/install/plugins/update_passsync.py
+++ b/ipaserver/install/plugins/update_passsync.py
@@ -70,9 +70,8 @@ class update_passync_privilege_update(PostUpdate):
 
         update = {'dn': passsync_privilege_dn,
                   'updates': ["add:member:'%s'" % passsync_dn]}
-        updates = {passsync_privilege_dn: update}
 
         sysupgrade.set_upgrade_state('winsync', 'passsync_privilege_updated', True)
-        return (False, True, [updates])
+        return (False, True, [update])
 
 api.register(update_passync_privilege_update)
diff --git a/ipaserver/install/plugins/update_uniqueness.py b/ipaserver/install/plugins/update_uniqueness.py
index 3017d5ac1..e0ee150a7 100644
--- a/ipaserver/install/plugins/update_uniqueness.py
+++ b/ipaserver/install/plugins/update_uniqueness.py
@@ -218,7 +218,7 @@ class update_uniqueness_plugins_to_new_syntax(PreUpdate):
                                   "plugin %s (%s)",
                                   entry.dn, e)
 
-            update_list.append({entry.dn: update})
+            update_list.append(update)
 
         return False, True, update_list
 
diff --git a/ipaserver/install/plugins/updateclient.py b/ipaserver/install/plugins/updateclient.py
index 8f5c5b5fd..85ee3f8a0 100644
--- a/ipaserver/install/plugins/updateclient.py
+++ b/ipaserver/install/plugins/updateclient.py
@@ -128,10 +128,7 @@ class updateclient(backend.Executioner):
                 self.restart(dm_password, live_run)
 
             if apply_now:
-                updates = {}
-                for entry in res:
-                    updates.update(entry)
-                ld.update_from_dict(updates)
+                ld.update_from_dict(res)
             elif res:
                 result.extend(res)
 
diff --git a/ipaserver/install/plugins/upload_cacrt.py b/ipaserver/install/plugins/upload_cacrt.py
index 66270ae76..dcdefee05 100644
--- a/ipaserver/install/plugins/upload_cacrt.py
+++ b/ipaserver/install/plugins/upload_cacrt.py
@@ -45,7 +45,7 @@ class update_upload_cacrt(PostUpdate):
                 if ca_chain:
                     ca_nickname = ca_chain[-1]
 
-        updates = {}
+        updates = []
 
         for nickname, trust_flags in db.list_certs():
             if 'u' in trust_flags:
@@ -64,7 +64,7 @@ class update_upload_cacrt(PostUpdate):
                 if ca_enabled:
                     entry.append('ipaConfigString:ipaCA')
                 entry.append('ipaConfigString:compatCA')
-            updates[dn] = {'dn': dn, 'default': entry}
+            updates.append({'dn': dn, 'default': entry})
 
         if ca_cert:
             dn = DN(('cn', 'CACert'), ('cn', 'ipa'), ('cn','etc'),
@@ -74,9 +74,9 @@ class update_upload_cacrt(PostUpdate):
                      'cn:CAcert',
                      'cACertificate;binary:%s' % ca_cert,
                     ]
-            updates[dn] = {'dn': dn, 'default': entry}
+            updates.append({'dn': dn, 'default': entry})
 
-        return (False, True, [updates])
+        return (False, True, updates)
 
     def _make_entry(self, cert, nickname, trust_flags):
         dn = DN(('cn', nickname), ('cn', 'certificates'), ('cn', 'ipa'),