lxd: add preference for LXD cloud-init.* config keys over user keys (#1108)

LXD now adds cloud-init scoped configuration keys network-config,
user-data and vendor-data. The existing user.user-data,
user.vendor-data, user.network-config and meta-data will be
deprecated in newer LXD.

cloud-init will prefer LXD config keys cloud-init.* keys above
user.* keys even if both are present. Warnings will be emitted
for ignored user.* keys if cloud-init.* overrides are present.

Expectation is that the configuration user.network-config,
user.meta-data, user.user-data and user.vendor-data* keys should
not be present at the same time as the comparable cloud-init.* keys.

2 files changed, 277 insertions, 50 deletions

diff --git a/cloudinit/sources/DataSourceLXD.py b/cloudinit/sources/DataSourceLXD.py
index 732b32ff..55ae52a2 100644
--- a/cloudinit/sources/DataSourceLXD.py
+++ b/cloudinit/sources/DataSourceLXD.py
@@ -10,6 +10,7 @@ Notes:
  * TODO( Hotplug support using websockets API 1.0/events )
 """
 
+from json.decoder import JSONDecodeError
 import os
 
 import requests
@@ -41,14 +42,16 @@ LXD_SOCKET_API_VERSION = "1.0"
 
 # Config key mappings to alias as top-level instance data keys
 CONFIG_KEY_ALIASES = {
+    "cloud-init.user-data": "user-data",
+    "cloud-init.network-config": "network-config",
+    "cloud-init.vendor-data": "vendor-data",
     "user.user-data": "user-data",
     "user.network-config": "network-config",
-    "user.network_mode": "network_mode",
     "user.vendor-data": "vendor-data"
 }
 
 
-def generate_fallback_network_config(network_mode: str = "") -> dict:
+def generate_fallback_network_config() -> dict:
     """Return network config V1 dict representing instance network config."""
     network_v1 = {
         "version": 1,
@@ -76,12 +79,6 @@ def generate_fallback_network_config(network_mode: str = "") -> dict:
                 network_v1["config"][0]["name"] = "enc9"
             else:
                 network_v1["config"][0]["name"] = "enp5s0"
-    if network_mode == "link-local":
-        network_v1["config"][0]["subnets"][0]["control"] = "manual"
-    elif network_mode not in ("", "dhcp"):
-        LOG.warning(
-            "Ignoring unexpected value user.network_mode: %s", network_mode
-        )
     return network_v1
 
 
@@ -244,10 +241,7 @@ class DataSourceLXD(sources.DataSource):
                     "network-config"
                 )
             else:
-                network_mode = self._crawled_metadata.get("network_mode", "")
-                self._network_config = generate_fallback_network_config(
-                    network_mode
-                )
+                self._network_config = generate_fallback_network_config()
         return self._network_config
 
 
@@ -294,7 +288,7 @@ def read_metadata(
     with requests.Session() as session:
         session.mount(version_url, LXDSocketAdapter())
         # Raw meta-data as text
-        md_route = "{route}/meta-data".format(route=version_url)
+        md_route = "{route}meta-data".format(route=version_url)
         response = session.get(md_route)
         LOG.debug("[GET] [HTTP:%d] %s", response.status_code, md_route)
         if not response.ok:
@@ -302,7 +296,7 @@ def read_metadata(
                 "Invalid HTTP response [{code}] from {route}: {resp}".format(
                     code=response.status_code,
                     route=md_route,
-                    resp=response.txt
+                    resp=response.text
                 )
             )
 
@@ -310,19 +304,42 @@ def read_metadata(
         if metadata_only:
             return md  # Skip network-data, vendor-data, user-data
 
-        config_url = version_url + "config"
-        # Represent all advertized/available config routes under
-        # the dict path {LXD_SOCKET_API_VERSION: {config: {...}}.
-        LOG.debug("[GET] %s", config_url)
-        config_routes = session.get(config_url).json()
         md[LXD_SOCKET_API_VERSION] = {
             "config": {},
             "meta-data": md["meta-data"]
         }
-        for config_route in config_routes:
+
+        config_url = version_url + "config"
+        # Represent all advertized/available config routes under
+        # the dict path {LXD_SOCKET_API_VERSION: {config: {...}}.
+        response = session.get(config_url)
+        LOG.debug("[GET] [HTTP:%d] %s", response.status_code, config_url)
+        if not response.ok:
+            raise sources.InvalidMetaDataException(
+                "Invalid HTTP response [{code}] from {route}: {resp}".format(
+                    code=response.status_code,
+                    route=config_url,
+                    resp=response.text
+                )
+            )
+        try:
+            config_routes = response.json()
+        except JSONDecodeError as exc:
+            raise sources.InvalidMetaDataException(
+                "Unable to determine cloud-init config from {route}."
+                " Expected JSON but found: {resp}".format(
+                    route=config_url,
+                    resp=response.text
+                )
+            ) from exc
+
+        # Sorting keys to ensure we always process in alphabetical order.
+        # cloud-init.* keys will sort before user.* keys which is preferred
+        # precedence.
+        for config_route in sorted(config_routes):
             url = "http://lxd{route}".format(route=config_route)
-            LOG.debug("[GET] %s", url)
             response = session.get(url)
+            LOG.debug("[GET] [HTTP:%d] %s", response.status_code, url)
             if response.ok:
                 cfg_key = config_route.rpartition("/")[-1]
                 # Leave raw data values/format unchanged to represent it in
@@ -331,9 +348,21 @@ def read_metadata(
                 md[LXD_SOCKET_API_VERSION]["config"][cfg_key] = response.text
                 # Promote common CONFIG_KEY_ALIASES to top-level keys.
                 if cfg_key in CONFIG_KEY_ALIASES:
-                    md[CONFIG_KEY_ALIASES[cfg_key]] = response.text
+                    # Due to sort of config_routes, promote cloud-init.*
+                    # aliases before user.*. This allows user.* keys to act as
+                    # fallback config on old LXD, with new cloud-init images.
+                    if CONFIG_KEY_ALIASES[cfg_key] not in md:
+                        md[CONFIG_KEY_ALIASES[cfg_key]] = response.text
+                    else:
+                        LOG.warning(
+                            "Ignoring LXD config %s in favor of %s value.",
+                            cfg_key, cfg_key.replace("user", "cloud-init", 1)
+                        )
             else:
-                LOG.debug("Skipping %s on invalid response", url)
+                LOG.debug(
+                    "Skipping %s on [HTTP:%d]:%s",
+                    url, response.status_code, response.text
+                )
     return md
 
 
diff --git a/cloudinit/sources/tests/test_lxd.py b/cloudinit/sources/tests/test_lxd.py
index c2027616..fc2a41df 100644
--- a/cloudinit/sources/tests/test_lxd.py
+++ b/cloudinit/sources/tests/test_lxd.py
@@ -2,13 +2,17 @@
 
 from collections import namedtuple
 from copy import deepcopy
+import json
+import re
 import stat
 from unittest import mock
 import yaml
 
 import pytest
 
-from cloudinit.sources import DataSourceLXD as lxd, UNSET
+from cloudinit.sources import (
+    DataSourceLXD as lxd, InvalidMetaDataException, UNSET
+)
 DS_PATH = "cloudinit.sources.DataSourceLXD."
 
 
@@ -24,8 +28,6 @@ NETWORK_V1 = {
         }
     ]
 }
-NETWORK_V1_MANUAL = deepcopy(NETWORK_V1)
-NETWORK_V1_MANUAL["config"][0]["subnets"][0]["control"] = "manual"
 
 
 def _add_network_v1_device(devname) -> dict:
@@ -79,18 +81,15 @@ def lxd_ds(request, paths, lxd_metadata):
 class TestGenerateFallbackNetworkConfig:
 
     @pytest.mark.parametrize(
-        "uname_machine,systemd_detect_virt,network_mode,expected", (
+        "uname_machine,systemd_detect_virt,expected", (
             # None for systemd_detect_virt returns None from which
-            ({}, None, "", NETWORK_V1),
-            ({}, None, "dhcp", NETWORK_V1),
-            # invalid network_mode logs warning
-            ({}, None, "bogus", NETWORK_V1),
-            ({}, None, "link-local", NETWORK_V1_MANUAL),
-            ("anything", "lxc\n", "", NETWORK_V1),
+            ({}, None, NETWORK_V1),
+            ({}, None, NETWORK_V1),
+            ("anything", "lxc\n", NETWORK_V1),
             # `uname -m` on kvm determines devname
-            ("x86_64", "kvm\n", "", _add_network_v1_device("enp5s0")),
-            ("ppc64le", "kvm\n", "", _add_network_v1_device("enp0s5")),
-            ("s390x", "kvm\n", "", _add_network_v1_device("enc9"))
+            ("x86_64", "kvm\n", _add_network_v1_device("enp5s0")),
+            ("ppc64le", "kvm\n", _add_network_v1_device("enp0s5")),
+            ("s390x", "kvm\n", _add_network_v1_device("enc9"))
         )
     )
     @mock.patch(DS_PATH + "util.system_info")
@@ -103,22 +102,14 @@ class TestGenerateFallbackNetworkConfig:
         m_system_info,
         uname_machine,
         systemd_detect_virt,
-        network_mode,
         expected,
-        caplog
     ):
-        """Return network config v2 based on uname -m, systemd-detect-virt.
-
-        LXC config network_mode of "link-local" will determine whether to set
-        "activation-mode: manual", leaving the interface down.
-        """
+        """Return network config v2 based on uname -m, systemd-detect-virt."""
         if systemd_detect_virt is None:
             m_which.return_value = None
         m_system_info.return_value = {"uname": ["", "", "", "", uname_machine]}
         m_subp.return_value = (systemd_detect_virt, "")
-        assert expected == lxd.generate_fallback_network_config(
-            network_mode=network_mode
-        )
+        assert expected == lxd.generate_fallback_network_config()
         if systemd_detect_virt is None:
             assert 0 == m_subp.call_count
             assert 0 == m_system_info.call_count
@@ -130,10 +121,6 @@ class TestGenerateFallbackNetworkConfig:
                 assert 0 == m_system_info.call_count
             else:
                 assert 1 == m_system_info.call_count
-        if network_mode not in ("dhcp", "", "link-local"):
-            assert "Ignoring unexpected value user.network_mode: {}".format(
-                network_mode
-            ) in caplog.text
 
 
 class TestDataSourceLXD:
@@ -182,4 +169,215 @@ class TestIsPlatformViable:
         else:
             assert 0 == m_lstat.call_count
 
+
+class TestReadMetadata:
+    @pytest.mark.parametrize(
+        "url_responses,expected,logs", (
+            (  # Assert non-JSON format from config route
+                {
+                    "http://lxd/1.0/meta-data": "local-hostname: md\n",
+                    "http://lxd/1.0/config": "[NOT_JSON",
+                },
+                InvalidMetaDataException(
+                    "Unable to determine cloud-init config from"
+                    " http://lxd/1.0/config. Expected JSON but found:"
+                    " [NOT_JSON"),
+                ["[GET] [HTTP:200] http://lxd/1.0/meta-data",
+                 "[GET] [HTTP:200] http://lxd/1.0/config"],
+            ),
+            (   # Assert success on just meta-data
+                {
+                    "http://lxd/1.0/meta-data": "local-hostname: md\n",
+                    "http://lxd/1.0/config": "[]",
+                },
+                {"1.0": {"config": {}, "meta-data": "local-hostname: md\n"},
+                 "meta-data": "local-hostname: md\n"},
+                ["[GET] [HTTP:200] http://lxd/1.0/meta-data",
+                 "[GET] [HTTP:200] http://lxd/1.0/config"],
+            ),
+            (   # Assert 404s for config routes log skipping
+                {
+                    "http://lxd/1.0/meta-data": "local-hostname: md\n",
+                    "http://lxd/1.0/config":
+                        '["/1.0/config/user.custom1",'
+                        ' "/1.0/config/user.meta-data",'
+                        ' "/1.0/config/user.network-config",'
+                        ' "/1.0/config/user.user-data",'
+                        ' "/1.0/config/user.vendor-data"]',
+                    "http://lxd/1.0/config/user.custom1": "custom1",
+                    "http://lxd/1.0/config/user.meta-data": "",  # 404
+                    "http://lxd/1.0/config/user.network-config": "net-config",
+                    "http://lxd/1.0/config/user.user-data": "",  # 404
+                    "http://lxd/1.0/config/user.vendor-data": "",  # 404
+                },
+                {
+                    "1.0": {
+                        "config": {
+                            "user.custom1": "custom1",   # Not promoted
+                            "user.network-config": "net-config",
+                        },
+                        "meta-data": "local-hostname: md\n",
+                    },
+                    "meta-data": "local-hostname: md\n",
+                    "network-config": "net-config",
+                },
+                [
+                    "Skipping http://lxd/1.0/config/user.vendor-data on"
+                    " [HTTP:404]",
+                    "Skipping http://lxd/1.0/config/user.meta-data on"
+                    " [HTTP:404]",
+                    "Skipping http://lxd/1.0/config/user.user-data on"
+                    " [HTTP:404]",
+                    "[GET] [HTTP:200] http://lxd/1.0/config",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.custom1",
+                    "[GET] [HTTP:200]"
+                    " http://lxd/1.0/config/user.network-config",
+                ],
+            ),
+            (   # Assert all CONFIG_KEY_ALIASES promoted to top-level keys
+                {
+                    "http://lxd/1.0/meta-data": "local-hostname: md\n",
+                    "http://lxd/1.0/config":
+                        '["/1.0/config/user.custom1",'
+                        ' "/1.0/config/user.meta-data",'
+                        ' "/1.0/config/user.network-config",'
+                        ' "/1.0/config/user.user-data",'
+                        ' "/1.0/config/user.vendor-data"]',
+                    "http://lxd/1.0/config/user.custom1": "custom1",
+                    "http://lxd/1.0/config/user.meta-data": "meta-data",
+                    "http://lxd/1.0/config/user.network-config": "net-config",
+                    "http://lxd/1.0/config/user.user-data": "user-data",
+                    "http://lxd/1.0/config/user.vendor-data": "vendor-data",
+                },
+                {
+                    "1.0": {
+                        "config": {
+                            "user.custom1": "custom1",   # Not promoted
+                            "user.meta-data": "meta-data",
+                            "user.network-config": "net-config",
+                            "user.user-data": "user-data",
+                            "user.vendor-data": "vendor-data",
+                        },
+                        "meta-data": "local-hostname: md\n",
+                    },
+                    "meta-data": "local-hostname: md\n",
+                    "network-config": "net-config",
+                    "user-data": "user-data",
+                    "vendor-data": "vendor-data",
+                },
+                [
+                    "[GET] [HTTP:200] http://lxd/1.0/meta-data",
+                    "[GET] [HTTP:200] http://lxd/1.0/config",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.custom1",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.meta-data",
+                    "[GET] [HTTP:200]"
+                    " http://lxd/1.0/config/user.network-config",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.user-data",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.vendor-data",
+                ],
+            ),
+            (   # Assert cloud-init.* config key values prefered over user.*
+                {
+                    "http://lxd/1.0/meta-data": "local-hostname: md\n",
+                    "http://lxd/1.0/config":
+                        '["/1.0/config/user.meta-data",'
+                        ' "/1.0/config/user.network-config",'
+                        ' "/1.0/config/user.user-data",'
+                        ' "/1.0/config/user.vendor-data",'
+                        ' "/1.0/config/cloud-init.network-config",'
+                        ' "/1.0/config/cloud-init.user-data",'
+                        ' "/1.0/config/cloud-init.vendor-data"]',
+                    "http://lxd/1.0/config/user.meta-data": "user.meta-data",
+                    "http://lxd/1.0/config/user.network-config":
+                        "user.network-config",
+                    "http://lxd/1.0/config/user.user-data": "user.user-data",
+                    "http://lxd/1.0/config/user.vendor-data":
+                        "user.vendor-data",
+                    "http://lxd/1.0/config/cloud-init.meta-data":
+                        "cloud-init.meta-data",
+                    "http://lxd/1.0/config/cloud-init.network-config":
+                        "cloud-init.network-config",
+                    "http://lxd/1.0/config/cloud-init.user-data":
+                        "cloud-init.user-data",
+                    "http://lxd/1.0/config/cloud-init.vendor-data":
+                        "cloud-init.vendor-data",
+                },
+                {
+                    "1.0": {
+                        "config": {
+                            "user.meta-data": "user.meta-data",
+                            "user.network-config": "user.network-config",
+                            "user.user-data": "user.user-data",
+                            "user.vendor-data": "user.vendor-data",
+                            "cloud-init.network-config":
+                                "cloud-init.network-config",
+                            "cloud-init.user-data": "cloud-init.user-data",
+                            "cloud-init.vendor-data":
+                                "cloud-init.vendor-data",
+                        },
+                        "meta-data": "local-hostname: md\n",
+                    },
+                    "meta-data": "local-hostname: md\n",
+                    "network-config": "cloud-init.network-config",
+                    "user-data": "cloud-init.user-data",
+                    "vendor-data": "cloud-init.vendor-data",
+                },
+                [
+                    "[GET] [HTTP:200] http://lxd/1.0/meta-data",
+                    "[GET] [HTTP:200] http://lxd/1.0/config",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.meta-data",
+                    "[GET] [HTTP:200]"
+                    " http://lxd/1.0/config/user.network-config",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.user-data",
+                    "[GET] [HTTP:200] http://lxd/1.0/config/user.vendor-data",
+                    "[GET] [HTTP:200]"
+                    " http://lxd/1.0/config/cloud-init.network-config",
+                    "[GET] [HTTP:200]"
+                    " http://lxd/1.0/config/cloud-init.user-data",
+                    "[GET] [HTTP:200]"
+                    " http://lxd/1.0/config/cloud-init.vendor-data",
+                    "Ignoring LXD config user.user-data in favor of"
+                    " cloud-init.user-data value.",
+                    "Ignoring LXD config user.network-config in favor of"
+                    " cloud-init.network-config value.",
+                    "Ignoring LXD config user.vendor-data in favor of"
+                    " cloud-init.vendor-data value.",
+                ],
+            ),
+        )
+    )
+    @mock.patch.object(lxd.requests.Session, 'get')
+    def test_read_metadata_handles_unexpected_content_or_http_status(
+        self, session_get, url_responses, expected, logs, caplog
+    ):
+        """read_metadata handles valid and invalid content and status codes."""
+
+        def fake_get(url):
+            """Mock Response json, ok, status_code, text from url_responses."""
+            m_resp = mock.MagicMock()
+            content = url_responses.get(url, '')
+            m_resp.json.side_effect = lambda: json.loads(content)
+            if content:
+                mock_ok = mock.PropertyMock(return_value=True)
+                mock_status_code = mock.PropertyMock(return_value=200)
+            else:
+                mock_ok = mock.PropertyMock(return_value=False)
+                mock_status_code = mock.PropertyMock(return_value=404)
+            type(m_resp).ok = mock_ok
+            type(m_resp).status_code = mock_status_code
+            mock_text = mock.PropertyMock(return_value=content)
+            type(m_resp).text = mock_text
+            return m_resp
+
+        session_get.side_effect = fake_get
+
+        if isinstance(expected, Exception):
+            with pytest.raises(type(expected), match=re.escape(str(expected))):
+                lxd.read_metadata()
+        else:
+            assert expected == lxd.read_metadata()
+        caplogs = caplog.text
+        for log in logs:
+            assert log in caplogs
+
 # vi: ts=4 expandtab