Fix config order of precedence, putting kernel command line over system.

The correct order of precedence when reading the base config:
  builtin config
  system config
  kernel command line provided config.

This reverts commit 63501f44, which actually broke the behavior it
reported to fix.  It also adds some unit tests to ensure this behavior
is not broken again.

LP: #1582323

4 files changed, 98 insertions, 17 deletions

diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index 86a13785..ca3f02fe 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -845,23 +845,15 @@ class Modules(object):
 
 
 def fetch_base_config():
-    base_cfgs = []
-    default_cfg = util.get_builtin_cfg()
-
-    # Anything in your conf.d location??
-    # or the 'default' cloud.cfg location???
-    base_cfgs.append(util.read_conf_with_confd(CLOUD_CONFIG))
-
-    # Kernel/cmdline parameters override system config
-    kern_contents = util.read_cc_from_cmdline()
-    if kern_contents:
-        base_cfgs.append(util.load_yaml(kern_contents, default={}))
-
-    # And finally the default gets to play
-    if default_cfg:
-        base_cfgs.append(default_cfg)
-
-    return util.mergemanydict(base_cfgs)
+    return util.mergemanydict(
+        [
+            # builtin config
+            util.get_builtin_cfg(),
+            # Anything in your conf.d or 'default' cloud.cfg location.
+            util.read_conf_with_confd(CLOUD_CONFIG),
+            # Kernel/cmdline parameters override system config
+            util.read_conf_from_cmdline(),
+        ], reverse=True)
 
 
 def _pkl_store(obj, fname):
diff --git a/cloudinit/util.py b/cloudinit/util.py
index cc084719..01c396b4 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -993,6 +993,11 @@ def read_conf_with_confd(cfgfile):
     return mergemanydict([confd_cfg, cfg])
 
 
+def read_conf_from_cmdline(cmdline=None):
+    # return a dictionary or config on the cmdline or None
+    return load_yaml(read_cc_from_cmdline(cmdline=cmdline))
+
+
 def read_cc_from_cmdline(cmdline=None):
     # this should support reading cloud-config information from
     # the kernel command line.  It is intended to support content of the
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index a2355a79..a73e3f61 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -300,6 +300,39 @@ def dir2dict(startdir, prefix=None):
     return flist
 
 
+def wrap_and_call(prefix, mocks, func, *args, **kwargs):
+    """
+    call func(args, **kwargs) with mocks applied, then unapplies mocks
+    nicer to read than repeating dectorators on each function
+
+    prefix: prefix for mock names (e.g. 'cloudinit.stages.util') or None
+    mocks: dictionary of names (under 'prefix') to mock and either
+        a return value or a dictionary to pass to the mock.patch call
+    func: function to call with mocks applied
+    *args,**kwargs: arguments for 'func'
+
+    return_value: return from 'func'
+    """
+    delim = '.'
+    if prefix is None:
+        prefix = ''
+    prefix = prefix.rstrip(delim)
+    unwraps = []
+    for fname, kw in mocks.items():
+        if prefix:
+            fname = delim.join((prefix, fname))
+        if not isinstance(kw, dict):
+            kw = {'return_value': kw}
+        p = mock.patch(fname, **kw)
+        p.start()
+        unwraps.append(p)
+    try:
+        return func(*args, **kwargs)
+    finally:
+        for p in unwraps:
+            p.stop()
+
+
 try:
     skipIf = unittest.skipIf
 except AttributeError:
diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py
index 55d9b93f..581b581a 100644
--- a/tests/unittests/test_data.py
+++ b/tests/unittests/test_data.py
@@ -559,3 +559,54 @@ class TestConvertString(helpers.TestCase):
         text = "hi mom"
         msg = ud.convert_string(text)
         self.assertEqual(text, msg.get_payload(decode=False))
+
+
+class TestFetchBaseConfig(helpers.TestCase):
+
+    def test_only_builtin_gets_builtin2(self):
+        ret = helpers.wrap_and_call(
+            'cloudinit.stages.util',
+            {'read_conf_with_confd': None,
+             'read_conf_from_cmdline': None},
+            stages.fetch_base_config)
+        self.assertEqual(util.get_builtin_cfg(), ret)
+
+    def test_conf_d_overrides_defaults(self):
+        builtin = util.get_builtin_cfg()
+        test_key = sorted(builtin)[0]
+        test_value = 'test'
+        ret = helpers.wrap_and_call(
+            'cloudinit.stages.util',
+            {'read_conf_with_confd': {'return_value': {test_key: test_value}},
+             'read_conf_from_cmdline': None},
+            stages.fetch_base_config)
+        self.assertEqual(ret.get(test_key), test_value)
+        builtin[test_key] = test_value
+        self.assertEqual(ret, builtin)
+
+    def test_cmdline_overrides_defaults(self):
+        builtin = util.get_builtin_cfg()
+        test_key = sorted(builtin)[0]
+        test_value = 'test'
+        cmdline = {test_key: test_value}
+        ret = helpers.wrap_and_call(
+            'cloudinit.stages.util',
+            {'read_conf_from_cmdline': {'return_value': cmdline},
+             'read_conf_with_confd': None},
+            stages.fetch_base_config)
+        self.assertEqual(ret.get(test_key), test_value)
+        builtin[test_key] = test_value
+        self.assertEqual(ret, builtin)
+
+    def test_cmdline_overrides_conf_d_and_defaults(self):
+        builtin = {'key1': 'value0', 'key3': 'other2'}
+        conf_d = {'key1': 'value1', 'key2': 'other1'}
+        cmdline = {'key3': 'other3', 'key2': 'other2'}
+        ret = helpers.wrap_and_call(
+            'cloudinit.stages.util',
+            {'read_conf_with_confd': {'return_value': conf_d},
+             'get_builtin_cfg': {'return_value': builtin},
+             'read_conf_from_cmdline': {'return_value': cmdline}},
+            stages.fetch_base_config)
+        self.assertEqual(ret, {'key1': 'value1', 'key2': 'other2',
+                               'key3': 'other3'})