Fixed merging capabilities.

Instead of previously having merging which was not backwards compatible
with the 0.7.1 and prior methods this patch works to ensure said backwards
compatible while at the same time making the new merging functionality
work in a more customizable manner.

6 files changed, 191 insertions, 93 deletions

diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py
index d30d6338..c97ca3e8 100644
--- a/cloudinit/handlers/cloud_config.py
+++ b/cloudinit/handlers/cloud_config.py
@@ -30,7 +30,25 @@ from cloudinit.settings import (PER_ALWAYS)
 LOG = logging.getLogger(__name__)
 
 MERGE_HEADER = 'Merge-Type'
-DEF_MERGERS = mergers.default_mergers()
+
+# Due to the way the loading of yaml configuration was done previously,
+# where previously each cloud config part was appended to a larger yaml
+# file and then finally that file was loaded as one big yaml file we need
+# to mimic that behavior by altering the default strategy to be replacing
+# keys of prior merges.
+#
+#
+# For example
+# #file 1
+# a: 3
+# #file 2
+# a: 22
+# #combined file (comments not included)
+# a: 3
+# a: 22
+#
+# This gets loaded into yaml with final result {'a': 22}
+DEF_MERGERS = mergers.string_extract_mergers('dict(replace)+list()+str()')
 
 
 class CloudConfigPartHandler(handlers.Handler):
@@ -39,7 +57,6 @@ class CloudConfigPartHandler(handlers.Handler):
         self.cloud_buf = None
         self.cloud_fn = paths.get_ipath("cloud_config")
         self.file_names = []
-        self.mergers = [DEF_MERGERS]
 
     def list_types(self):
         return [
@@ -54,6 +71,8 @@ class CloudConfigPartHandler(handlers.Handler):
         if self.file_names:
             file_lines.append("# from %s files" % (len(self.file_names)))
             for fn in self.file_names:
+                if not fn:
+                    fn = '?'
                 file_lines.append("# %s" % (fn))
             file_lines.append("")
         if self.cloud_buf is not None:
@@ -86,26 +105,20 @@ class CloudConfigPartHandler(handlers.Handler):
         all_mergers.extend(mergers_header)
         if not all_mergers:
             all_mergers = DEF_MERGERS
-        return all_mergers
+        return (payload_yaml, all_mergers)
 
     def _merge_part(self, payload, headers):
-        next_mergers = self._extract_mergers(payload, headers)
-        # Use the merger list from the last call, since it is the one
-        # that will be defining how to merge with the next payload.
-        curr_mergers = list(self.mergers[-1])
-        LOG.debug("Merging by applying %s", curr_mergers)
-        self.mergers.append(next_mergers)
-        merger = mergers.construct(curr_mergers)
+        (payload_yaml, my_mergers) = self._extract_mergers(payload, headers)
+        LOG.debug("Merging by applying %s", my_mergers)
+        merger = mergers.construct(my_mergers)
         if self.cloud_buf is None:
             # First time through, merge with an empty dict...
             self.cloud_buf = {}
-        self.cloud_buf = merger.merge(self.cloud_buf,
-                                      util.load_yaml(payload))
+        self.cloud_buf = merger.merge(self.cloud_buf, payload_yaml)
 
     def _reset(self):
         self.file_names = []
         self.cloud_buf = None
-        self.mergers = [DEF_MERGERS]
 
     def handle_part(self, _data, ctype, filename,  # pylint: disable=W0221
                     payload, _frequency, headers):  # pylint: disable=W0613
@@ -118,7 +131,10 @@ class CloudConfigPartHandler(handlers.Handler):
             return
         try:
             self._merge_part(payload, headers)
-            self.file_names.append(filename)
+            # Ensure filename is ok to store
+            for i in ("\n", "\r", "\t"):
+                filename = filename.replace(i, " ")
+            self.file_names.append(filename.strip())
         except:
             util.logexc(LOG, "Failed at merging in cloud config part from %s",
                         filename)
diff --git a/cloudinit/mergers/__init__.py b/cloudinit/mergers/__init__.py
index f504e15f..0978b2c6 100644
--- a/cloudinit/mergers/__init__.py
+++ b/cloudinit/mergers/__init__.py
@@ -55,6 +55,9 @@ class UnknownMerger(object):
         if not meth:
             meth = self._handle_unknown
             args.insert(0, method_name)
+        LOG.debug("Merging '%s' into '%s' using method '%s' of '%s'",
+                  type_name, type_utils.obj_name(merge_with),
+                  meth.__name__, self)
         return meth(*args)
 
 
@@ -66,6 +69,9 @@ class LookupMerger(UnknownMerger):
         else:
             self._lookups = lookups
 
+    def __str__(self):
+        return 'LookupMerger: (%s)' % (len(self._lookups))
+
     # For items which can not be merged by the parent this object
     # will lookup in a internally maintained set of objects and
     # find which one of those objects can perform the merge. If
@@ -78,6 +84,8 @@ class LookupMerger(UnknownMerger):
                 # First one that has that method/attr gets to be
                 # the one that will be called
                 meth = getattr(merger, meth_wanted)
+                LOG.debug(("Merging using located merger '%s'"
+                            " since it had method '%s'"), merger, meth_wanted)
                 break
         if not meth:
             return UnknownMerger._handle_unknown(self, meth_wanted,
@@ -87,9 +95,9 @@ class LookupMerger(UnknownMerger):
 
 def dict_extract_mergers(config):
     parsed_mergers = []
-    raw_mergers = config.get('merge_how')
+    raw_mergers = config.pop('merge_how', None)
     if raw_mergers is None:
-        raw_mergers = config.get('merge_type')
+        raw_mergers = config.pop('merge_type', None)
     if raw_mergers is None:
         return parsed_mergers
     if isinstance(raw_mergers, (str, basestring)):
diff --git a/cloudinit/mergers/m_dict.py b/cloudinit/mergers/m_dict.py
index 45a7d3a5..a16141fa 100644
--- a/cloudinit/mergers/m_dict.py
+++ b/cloudinit/mergers/m_dict.py
@@ -16,33 +16,71 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+DEF_MERGE_TYPE = 'no_replace'
+MERGE_TYPES = ('replace', DEF_MERGE_TYPE,)
+
+
+def _has_any(what, *keys):
+    for k in keys:
+        if k in what:
+            return True
+    return False
+
 
 class Merger(object):
     def __init__(self, merger, opts):
         self._merger = merger
-        self._overwrite = 'overwrite' in opts
-
-    # This merging algorithm will attempt to merge with
-    # another dictionary, on encountering any other type of object
-    # it will not merge with said object, but will instead return
-    # the original value
-    #
-    # On encountering a dictionary, it will create a new dictionary
-    # composed of the original and the one to merge with, if 'overwrite'
-    # is enabled then keys that exist in the original will be overwritten
-    # by keys in the one to merge with (and associated values). Otherwise
-    # if not in overwrite mode the 2 conflicting keys themselves will
-    # be merged.
-    def _on_dict(self, value, merge_with):
-        if not isinstance(merge_with, (dict)):
-            return value
-        merged = dict(value)
+        # Affects merging behavior...
+        self._method = DEF_MERGE_TYPE
+        for m in MERGE_TYPES:
+            if m in opts:
+                self._method = m
+                break
+        # Affect how recursive merging is done on other primitives.
+        self._recurse_str = 'recurse_str' in opts
+        self._recurse_array = _has_any(opts, 'recurse_array', 'recurse_list')
+        self._allow_delete = 'allow_delete' in opts
+        # Backwards compat require this to be on.
+        self._recurse_dict = True
+
+    def __str__(self):
+        s = ('DictMerger: (method=%s,recurse_str=%s,'
+             'recurse_dict=%s,recurse_array=%s,allow_delete=%s)')
+        s = s % (self._method, self._recurse_str,
+                 self._recurse_dict, self._recurse_array, self._allow_delete)
+        return s
+
+    def _do_dict_replace(self, value, merge_with, do_replace):
+
+        def merge_same_key(old_v, new_v):
+            if do_replace:
+                return new_v
+            if isinstance(new_v, (list, tuple)) and self._recurse_array:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (basestring)) and self._recurse_str:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (dict)) and self._recurse_dict:
+                return self._merger.merge(old_v, new_v)
+            # Otherwise leave it be...
+            return old_v
+
         for (k, v) in merge_with.items():
-            if k in merged:
-                if not self._overwrite:
-                    merged[k] = self._merger.merge(merged[k], v)
+            if k in value:
+                if v is None and self._allow_delete:
+                    value.pop(k)
                 else:
-                    merged[k] = v
+                    value[k] = merge_same_key(value[k], v)
             else:
-                merged[k] = v
+                value[k] = v
+        return value
+
+    def _on_dict(self, value, merge_with):
+        if not isinstance(merge_with, (dict)):
+            return value
+        if self._method == 'replace':
+            merged = self._do_dict_replace(dict(value), merge_with, True)
+        elif self._method == 'no_replace':
+            merged = self._do_dict_replace(dict(value), merge_with, False)
+        else:
+            raise NotImplementedError("Unknown merge type %s" % (self._method))
         return merged
diff --git a/cloudinit/mergers/m_list.py b/cloudinit/mergers/m_list.py
index a56ff007..76591bea 100644
--- a/cloudinit/mergers/m_list.py
+++ b/cloudinit/mergers/m_list.py
@@ -16,35 +16,71 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+DEF_MERGE_TYPE = 'replace'
+MERGE_TYPES = ('append', 'prepend', DEF_MERGE_TYPE, 'no_replace')
+
+def _has_any(what, *keys):
+    for k in keys:
+        if k in what:
+            return True
+    return False
+
 
 class Merger(object):
     def __init__(self, merger, opts):
         self._merger = merger
-        self._discard_non = 'discard_non_list' in opts
-        self._extend = 'extend' in opts
+        # Affects merging behavior...
+        self._method = DEF_MERGE_TYPE
+        for m in MERGE_TYPES:
+            if m in opts:
+                self._method = m
+                break
+        # Affect how recursive merging is done on other primitives
+        self._recurse_str = _has_any(opts, 'recurse_str')
+        self._recurse_dict = _has_any(opts, 'recurse_dict')
+        self._recurse_array = _has_any(opts, 'recurse_array', 'recurse_list')
+
+    def __str__(self):
+        return ('ListMerger: (method=%s,recurse_str=%s,'
+                'recurse_dict=%s,recurse_array=%s)') % (self._method,
+                                                        self._recurse_str,
+                                                        self._recurse_dict,
+                                                        self._recurse_array)
 
     def _on_tuple(self, value, merge_with):
-        return self._on_list(list(value), merge_with)
-
-    # On encountering a list or tuple type this action will be applied
-    # a new list will be returned, if the value to merge with is itself
-    # a list and we have been told to 'extend', then the value here will
-    # be extended with the other list. If in 'extend' mode then we will
-    # attempt to merge instead, which means that values from the list
-    # to merge with will replace values in te original list (they will
-    # also be merged recursively).
-    #
-    # If the value to merge with is not a list, and we are set to discared
-    # then no modifications will take place, otherwise we will just append
-    # the value to merge with onto the end of our own list.
+        return tuple(self._on_list(list(value), merge_with))
+
     def _on_list(self, value, merge_with):
-        new_value = list(value)
-        if isinstance(merge_with, (tuple, list)):
-            if self._extend:
-                new_value.extend(merge_with)
-            else:
-                return new_value
-        else:
-            if not self._discard_non:
-                new_value.append(merge_with)
-        return new_value
+        if (self._method == 'replace' and
+            not isinstance(merge_with, (tuple, list))):
+            return merge_with
+
+        # Ok we now know that what we are merging with is a list or tuple.
+        merged_list = []
+        if self._method == 'prepend':
+            merged_list.extend(merge_with)
+            merged_list.extend(value)
+            return merged_list
+        elif self._method == 'append':
+            merged_list.extend(value)
+            merged_list.extend(merge_with)
+            return merged_list
+
+        def merge_same_index(old_v, new_v):
+            if self._method == 'no_replace':
+                # Leave it be...
+                return old_v
+            if isinstance(new_v, (list, tuple)) and self._recurse_array:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (str, basestring)) and self._recurse_str:
+                return self._merger.merge(old_v, new_v)
+            if isinstance(new_v, (dict)) and self._recurse_dict:
+                return self._merger.merge(old_v, new_v)
+            return new_v
+
+        # Ok now we are replacing same indexes
+        merged_list.extend(value)
+        common_len = min(len(merged_list), len(merge_with))
+        for i in xrange(0, common_len):
+            merged_list[i] = merge_same_index(merged_list[i], merge_with[i])
+        return merged_list
diff --git a/cloudinit/mergers/m_str.py b/cloudinit/mergers/m_str.py
index 291c91c2..e22ce28a 100644
--- a/cloudinit/mergers/m_str.py
+++ b/cloudinit/mergers/m_str.py
@@ -1,26 +1,30 @@
+# -*- coding: utf-8 -*-
 # vi: ts=4 expandtab
 #
-#    Copyright (C) 2012 Yahoo! Inc.
+# Copyright (C) 2012 Yahoo! Inc.
 #
-#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
+# Author: Joshua Harlow <harlowja@yahoo-inc.com>
 #
-#    This program is free software: you can redistribute it and/or modify
-#    it under the terms of the GNU General Public License version 3, as
-#    published by the Free Software Foundation.
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3, as
+# published by the Free Software Foundation.
 #
-#    This program is distributed in the hope that it will be useful,
-#    but WITHOUT ANY WARRANTY; without even the implied warranty of
-#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-#    GNU General Public License for more details.
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
 #
-#    You should have received a copy of the GNU General Public License
-#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
 
 
 class Merger(object):
     def __init__(self, _merger, opts):
         self._append = 'append' in opts
 
+    def __str__(self):
+        return 'StringMerger: (append=%s)' % (self._append)
+
     # On encountering a unicode object to merge value with
     # we will for now just proxy into the string method to let it handle it.
     def _on_unicode(self, value, merge_with):
@@ -30,10 +34,11 @@ class Merger(object):
     # perform the following action, if appending we will
     # merge them together, otherwise we will just return value.
     def _on_str(self, value, merge_with):
+        if not isinstance(value, (basestring)):
+            return merge_with
         if not self._append:
-            return value
+            return merge_with
+        if isinstance(value, unicode):
+            return value + unicode(merge_with)
         else:
-            if isinstance(value, (unicode)):
-                return value + unicode(merge_with)
-            else:
-                return value + str(merge_with)
+            return value + str(merge_with)
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 053fa95d..b27b3567 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -543,21 +543,16 @@ def make_url(scheme, host, port=None,
 def mergemanydict(srcs, reverse=False):
     if reverse:
         srcs = reversed(srcs)
-    m_cfg = {}
-    merge_how = [mergers.default_mergers()]
-    for a_cfg in srcs:
-        if a_cfg:
-            # Take the last merger as the one that
-            # will define how to merge next...
-            mergers_to_apply = list(merge_how[-1])
+    merged_cfg = {}
+    for cfg in srcs:
+        if cfg:
+            # Figure out which mergers to apply...
+            mergers_to_apply = mergers.dict_extract_mergers(cfg)
+            if not mergers_to_apply:
+                mergers_to_apply = mergers.default_mergers()
             merger = mergers.construct(mergers_to_apply)
-            m_cfg = merger.merge(m_cfg, a_cfg)
-            # If the config has now has new merger set,
-            # extract them to be used next time...
-            new_mergers = mergers.dict_extract_mergers(m_cfg)
-            if new_mergers:
-                merge_how.append(new_mergers)
-    return m_cfg
+            merged_cfg = merger.merge(merged_cfg, cfg)
+    return merged_cfg
 
 
 @contextlib.contextmanager