Add json parsing of ip addr show (SC-723) (#1210)

When obtaining information from "ip addr", default to using
"ip --json addr" rather than using regex to parse "ip addr show"
as json is machine readable as less prone to error.

Deprecate but leave fallback to use "ip addr" for older iproute2
tooling which does not support --json param. 

Fix regex parsing of "ip addr" to support peer addresses and
metrics.

4 files changed, 353 insertions, 77 deletions

diff --git a/tests/data/netinfo/sample-ipaddrshow-json b/tests/data/netinfo/sample-ipaddrshow-json
new file mode 100644
index 00000000..8f6a430c
--- /dev/null
+++ b/tests/data/netinfo/sample-ipaddrshow-json
@@ -0,0 +1,91 @@
+[
+    {
+        "ifindex": 1,
+        "ifname": "lo",
+        "flags": [
+            "LOOPBACK",
+            "UP",
+            "LOWER_UP"
+        ],
+        "mtu": 65536,
+        "qdisc": "noqueue",
+        "operstate": "UNKNOWN",
+        "group": "default",
+        "txqlen": 1000,
+        "link_type": "loopback",
+        "address": "00:00:00:00:00:00",
+        "broadcast": "00:00:00:00:00:00",
+        "addr_info": [
+            {
+                "family": "inet",
+                "local": "127.0.0.1",
+                "prefixlen": 8,
+                "scope": "host",
+                "label": "lo",
+                "valid_life_time": 4294967295,
+                "preferred_life_time": 4294967295
+            },
+            {
+                "family": "inet6",
+                "local": "::1",
+                "prefixlen": 128,
+                "scope": "host",
+                "valid_life_time": 4294967295,
+                "preferred_life_time": 4294967295
+            }
+        ]
+    },
+    {
+        "ifindex": 23,
+        "link_index": 24,
+        "ifname": "enp0s25",
+        "flags": [
+            "BROADCAST",
+            "MULTICAST",
+            "UP",
+            "LOWER_UP"
+        ],
+        "mtu": 1500,
+        "qdisc": "noqueue",
+        "operstate": "UP",
+        "group": "default",
+        "txqlen": 1000,
+        "link_type": "ether",
+        "address": "50:7b:9d:2c:af:91",
+        "broadcast": "ff:ff:ff:ff:ff:ff",
+        "link_netnsid": 0,
+        "addr_info": [
+            {
+                "family": "inet",
+                "local": "192.168.2.18",
+                "prefixlen": 24,
+                "metric": 100,
+                "broadcast": "192.168.2.255",
+                "scope": "global",
+                "dynamic": true,
+                "label": "enp0s25",
+                "valid_life_time": 2339,
+                "preferred_life_time": 2339
+            },
+            {
+                "family": "inet6",
+                "local": "fe80::7777:2222:1111:eeee",
+                "prefixlen": 64,
+                "scope": "global",
+                "dynamic": true,
+                "mngtmpaddr": true,
+                "noprefixroute": true,
+                "valid_life_time": 6823,
+                "preferred_life_time": 3223
+            },
+            {
+                "family": "inet6",
+                "local": "fe80::8107:2b92:867e:f8a6",
+                "prefixlen": 64,
+                "scope": "link",
+                "valid_life_time": 4294967295,
+                "preferred_life_time": 4294967295
+            }
+        ]
+    }
+]
diff --git a/tests/data/netinfo/sample-ipaddrshow-json-down b/tests/data/netinfo/sample-ipaddrshow-json-down
new file mode 100644
index 00000000..7ad5dde0
--- /dev/null
+++ b/tests/data/netinfo/sample-ipaddrshow-json-down
@@ -0,0 +1,57 @@
+[
+    {
+        "ifindex": 1,
+        "ifname": "lo",
+        "flags": [
+            "LOOPBACK",
+            "UP",
+            "LOWER_UP"
+        ],
+        "mtu": 65536,
+        "qdisc": "noqueue",
+        "operstate": "UNKNOWN",
+        "group": "default",
+        "txqlen": 1000,
+        "link_type": "loopback",
+        "address": "00:00:00:00:00:00",
+        "broadcast": "00:00:00:00:00:00",
+        "addr_info": [
+            {
+                "family": "inet",
+                "local": "127.0.0.1",
+                "prefixlen": 8,
+                "scope": "host",
+                "label": "lo",
+                "valid_life_time": 4294967295,
+                "preferred_life_time": 4294967295
+            },
+            {
+                "family": "inet6",
+                "local": "::1",
+                "prefixlen": 128,
+                "scope": "host",
+                "valid_life_time": 4294967295,
+                "preferred_life_time": 4294967295
+            }
+        ]
+    },
+    {
+        "ifindex": 23,
+        "link_index": 24,
+        "ifname": "eth0",
+        "flags": [
+            "BROADCAST",
+            "MULTICAST"
+        ],
+        "mtu": 1500,
+        "qdisc": "noqueue",
+        "operstate": "DOWN",
+        "group": "default",
+        "txqlen": 1000,
+        "link_type": "ether",
+        "address": "00:16:3e:de:51:a6",
+        "broadcast": "ff:ff:ff:ff:ff:ff",
+        "link_netnsid": 0,
+        "addr_info": []
+    }
+]
diff --git a/tests/data/netinfo/sample-ipaddrshow-output b/tests/data/netinfo/sample-ipaddrshow-output
index b2fa2672..2aa3f90c 100644
--- a/tests/data/netinfo/sample-ipaddrshow-output
+++ b/tests/data/netinfo/sample-ipaddrshow-output
@@ -4,10 +4,9 @@
     inet6 ::1/128 scope host \       valid_lft forever preferred_lft forever
 2: enp0s25: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
     link/ether 50:7b:9d:2c:af:91 brd ff:ff:ff:ff:ff:ff
-    inet 192.168.2.18/24 brd 192.168.2.255 scope global dynamic enp0s25
+    inet 192.168.2.18/24 metric 100 brd 192.168.2.255 scope global dynamic enp0s25
        valid_lft 84174sec preferred_lft 84174sec
     inet6 fe80::7777:2222:1111:eeee/64 scope global
        valid_lft forever preferred_lft forever
     inet6 fe80::8107:2b92:867e:f8a6/64 scope link
        valid_lft forever preferred_lft forever
-
diff --git a/tests/unittests/test_netinfo.py b/tests/unittests/test_netinfo.py
index 5ed15729..aecce921 100644
--- a/tests/unittests/test_netinfo.py
+++ b/tests/unittests/test_netinfo.py
@@ -2,16 +2,26 @@
 
 """Tests netinfo module functions and classes."""
 
+import json
 from copy import copy
 
-from cloudinit.netinfo import netdev_info, netdev_pformat, route_pformat
-from tests.unittests.helpers import CiTestCase, mock, readResource
+import pytest
+
+from cloudinit import subp
+from cloudinit.netinfo import (
+    _netdev_info_iproute_json,
+    netdev_info,
+    netdev_pformat,
+    route_pformat,
+)
+from tests.unittests.helpers import mock, readResource
 
 # Example ifconfig and route output
 SAMPLE_OLD_IFCONFIG_OUT = readResource("netinfo/old-ifconfig-output")
 SAMPLE_NEW_IFCONFIG_OUT = readResource("netinfo/new-ifconfig-output")
 SAMPLE_FREEBSD_IFCONFIG_OUT = readResource("netinfo/freebsd-ifconfig-output")
 SAMPLE_IPADDRSHOW_OUT = readResource("netinfo/sample-ipaddrshow-output")
+SAMPLE_IPADDRSHOW_JSON = readResource("netinfo/sample-ipaddrshow-json")
 SAMPLE_ROUTE_OUT_V4 = readResource("netinfo/sample-route-output-v4")
 SAMPLE_ROUTE_OUT_V6 = readResource("netinfo/sample-route-output-v6")
 SAMPLE_IPROUTE_OUT_V4 = readResource("netinfo/sample-iproute-output-v4")
@@ -21,11 +31,7 @@ ROUTE_FORMATTED_OUT = readResource("netinfo/route-formatted-output")
 FREEBSD_NETDEV_OUT = readResource("netinfo/freebsd-netdev-formatted-output")
 
 
-class TestNetInfo(CiTestCase):
-
-    maxDiff = None
-    with_logs = True
-
+class TestNetInfo:
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
     def test_netdev_old_nettools_pformat(self, m_subp, m_which):
@@ -33,7 +39,7 @@ class TestNetInfo(CiTestCase):
         m_subp.return_value = (SAMPLE_OLD_IFCONFIG_OUT, "")
         m_which.side_effect = lambda x: x if x == "ifconfig" else None
         content = netdev_pformat()
-        self.assertEqual(NETDEV_FORMATTED_OUT, content)
+        assert NETDEV_FORMATTED_OUT == content
 
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
@@ -42,7 +48,7 @@ class TestNetInfo(CiTestCase):
         m_subp.return_value = (SAMPLE_NEW_IFCONFIG_OUT, "")
         m_which.side_effect = lambda x: x if x == "ifconfig" else None
         content = netdev_pformat()
-        self.assertEqual(NETDEV_FORMATTED_OUT, content)
+        assert NETDEV_FORMATTED_OUT == content
 
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
@@ -54,13 +60,19 @@ class TestNetInfo(CiTestCase):
         print()
         print(content)
         print()
-        self.assertEqual(FREEBSD_NETDEV_OUT, content)
+        assert FREEBSD_NETDEV_OUT == content
 
+    @pytest.mark.parametrize(
+        "resource,is_json",
+        [(SAMPLE_IPADDRSHOW_OUT, False), (SAMPLE_IPADDRSHOW_JSON, True)],
+    )
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
-    def test_netdev_iproute_pformat(self, m_subp, m_which):
-        """netdev_pformat properly rendering ip route info."""
-        m_subp.return_value = (SAMPLE_IPADDRSHOW_OUT, "")
+    def test_netdev_iproute_pformat(self, m_subp, m_which, resource, is_json):
+        """netdev_pformat properly rendering ip route info (non json)."""
+        m_subp.return_value = (resource, "")
+        if not is_json:
+            m_subp.side_effect = [subp.ProcessExecutionError, (resource, "")]
         m_which.side_effect = lambda x: x if x == "ip" else None
         content = netdev_pformat()
         new_output = copy(NETDEV_FORMATTED_OUT)
@@ -70,19 +82,19 @@ class TestNetInfo(CiTestCase):
         new_output = new_output.replace(
             "255.0.0.0   |   .    |", "255.0.0.0   |  host  |"
         )
-        self.assertEqual(new_output, content)
+        assert new_output == content
 
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
-    def test_netdev_warn_on_missing_commands(self, m_subp, m_which):
+    def test_netdev_warn_on_missing_commands(self, m_subp, m_which, caplog):
         """netdev_pformat warns when missing both ip and 'netstat'."""
         m_which.return_value = None  # Niether ip nor netstat found
         content = netdev_pformat()
-        self.assertEqual("\n", content)
-        self.assertEqual(
-            "WARNING: Could not print networks: missing 'ip' and 'ifconfig'"
-            " commands\n",
-            self.logs.getvalue(),
+        assert "\n" == content
+        log = caplog.records[0]
+        assert log.levelname == "WARNING"
+        assert log.msg == (
+            "Could not print networks: missing 'ip' and 'ifconfig' commands"
         )
         m_subp.assert_not_called()
 
@@ -95,57 +107,62 @@ class TestNetInfo(CiTestCase):
             "",
         )
         m_which.side_effect = lambda x: x if x == "ifconfig" else None
-        self.assertEqual(
-            {
-                "eth0": {
-                    "ipv4": [],
-                    "ipv6": [],
-                    "hwaddr": "00:16:3e:de:51:a6",
-                    "up": False,
-                },
-                "lo": {
-                    "ipv4": [{"ip": "127.0.0.1", "mask": "255.0.0.0"}],
-                    "ipv6": [{"ip": "::1/128", "scope6": "host"}],
-                    "hwaddr": ".",
-                    "up": True,
-                },
+        assert netdev_info(".") == {
+            "eth0": {
+                "ipv4": [],
+                "ipv6": [],
+                "hwaddr": "00:16:3e:de:51:a6",
+                "up": False,
             },
-            netdev_info("."),
-        )
+            "lo": {
+                "ipv4": [{"ip": "127.0.0.1", "mask": "255.0.0.0"}],
+                "ipv6": [{"ip": "::1/128", "scope6": "host"}],
+                "hwaddr": ".",
+                "up": True,
+            },
+        }
 
+    @pytest.mark.parametrize(
+        "resource,is_json",
+        [
+            ("netinfo/sample-ipaddrshow-output-down", False),
+            ("netinfo/sample-ipaddrshow-json-down", True),
+        ],
+    )
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
-    def test_netdev_info_iproute_down(self, m_subp, m_which):
+    def test_netdev_info_iproute_down(
+        self, m_subp, m_which, resource, is_json
+    ):
         """Test netdev_info with ip and down interfaces."""
-        m_subp.return_value = (
-            readResource("netinfo/sample-ipaddrshow-output-down"),
-            "",
-        )
+        m_subp.return_value = (readResource(resource), "")
+        if not is_json:
+            m_subp.side_effect = [
+                subp.ProcessExecutionError,
+                (readResource(resource), ""),
+            ]
         m_which.side_effect = lambda x: x if x == "ip" else None
-        self.assertEqual(
-            {
-                "lo": {
-                    "ipv4": [
-                        {
-                            "ip": "127.0.0.1",
-                            "bcast": ".",
-                            "mask": "255.0.0.0",
-                            "scope": "host",
-                        }
-                    ],
-                    "ipv6": [{"ip": "::1/128", "scope6": "host"}],
-                    "hwaddr": ".",
-                    "up": True,
-                },
-                "eth0": {
-                    "ipv4": [],
-                    "ipv6": [],
-                    "hwaddr": "00:16:3e:de:51:a6",
-                    "up": False,
-                },
+        assert netdev_info(".") == {
+            "lo": {
+                "ipv4": [
+                    {
+                        "ip": "127.0.0.1",
+                        "bcast": ".",
+                        "mask": "255.0.0.0",
+                        "scope": "host",
+                    }
+                ],
+                "ipv6": [{"ip": "::1/128", "scope6": "host"}],
+                "hwaddr": ".",
+                "up": True,
             },
-            netdev_info("."),
-        )
+            "eth0": {
+                "ipv4": [],
+                "ipv6": [],
+                "hwaddr": "00:16:3e:de:51:a6",
+                "up": False,
+            },
+        }
 
     @mock.patch("cloudinit.netinfo.netdev_info")
     def test_netdev_pformat_with_down(self, m_netdev_info):
@@ -166,9 +183,9 @@ class TestNetInfo(CiTestCase):
                 "up": False,
             },
         }
-        self.assertEqual(
-            readResource("netinfo/netdev-formatted-output-down"),
-            netdev_pformat(),
+        assert (
+            readResource("netinfo/netdev-formatted-output-down")
+            == netdev_pformat()
         )
 
     @mock.patch("cloudinit.netinfo.subp.which")
@@ -186,7 +203,7 @@ class TestNetInfo(CiTestCase):
         m_subp.side_effect = subp_netstat_route_selector
         m_which.side_effect = lambda x: x if x == "netstat" else None
         content = route_pformat()
-        self.assertEqual(ROUTE_FORMATTED_OUT, content)
+        assert ROUTE_FORMATTED_OUT == content
 
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
@@ -204,21 +221,133 @@ class TestNetInfo(CiTestCase):
         m_subp.side_effect = subp_iproute_selector
         m_which.side_effect = lambda x: x if x == "ip" else None
         content = route_pformat()
-        self.assertEqual(ROUTE_FORMATTED_OUT, content)
+        assert ROUTE_FORMATTED_OUT == content
 
     @mock.patch("cloudinit.netinfo.subp.which")
     @mock.patch("cloudinit.netinfo.subp.subp")
-    def test_route_warn_on_missing_commands(self, m_subp, m_which):
+    def test_route_warn_on_missing_commands(self, m_subp, m_which, caplog):
         """route_pformat warns when missing both ip and 'netstat'."""
         m_which.return_value = None  # Niether ip nor netstat found
         content = route_pformat()
-        self.assertEqual("\n", content)
-        self.assertEqual(
-            "WARNING: Could not print routes: missing 'ip' and 'netstat'"
-            " commands\n",
-            self.logs.getvalue(),
+        assert "\n" == content
+        log = caplog.records[0]
+        assert log.levelname == "WARNING"
+        assert log.msg == (
+            "Could not print routes: missing 'ip' and 'netstat' commands"
         )
         m_subp.assert_not_called()
 
+    @pytest.mark.parametrize(
+        "input,expected",
+        [
+            # Test hwaddr set when link_type is ether,
+            # Test up True when flags contains UP and LOWER_UP
+            (
+                [
+                    {
+                        "ifname": "eth0",
+                        "link_type": "ether",
+                        "address": "00:00:00:00:00:00",
+                        "flags": ["LOOPBACK", "UP", "LOWER_UP"],
+                    }
+                ],
+                {
+                    "eth0": {
+                        "hwaddr": "00:00:00:00:00:00",
+                        "ipv4": [],
+                        "ipv6": [],
+                        "up": True,
+                    }
+                },
+            ),
+            # Test hwaddr not set when link_type is not ether
+            # Test up False when flags does not contain both UP and LOWER_UP
+            (
+                [
+                    {
+                        "ifname": "eth0",
+                        "link_type": "none",
+                        "address": "00:00:00:00:00:00",
+                        "flags": ["LOOPBACK", "UP"],
+                    }
+                ],
+                {
+                    "eth0": {
+                        "hwaddr": "",
+                        "ipv4": [],
+                        "ipv6": [],
+                        "up": False,
+                    }
+                },
+            ),
+            (
+                [
+                    {
+                        "ifname": "eth0",
+                        "addr_info": [
+                            # Test for ipv4:
+                            #  ip set correctly
+                            #  mask set correctly
+                            #  bcast set correctly
+                            #  scope set correctly
+                            {
+                                "family": "inet",
+                                "local": "10.0.0.1",
+                                "broadcast": "10.0.0.255",
+                                "prefixlen": 24,
+                                "scope": "global",
+                            },
+                            # Test for ipv6:
+                            #  ip set correctly
+                            #  mask set correctly when no 'address' present
+                            #  scope6 set correctly
+                            {
+                                "family": "inet6",
+                                "local": "fd12:3456:7890:1234::5678:9012",
+                                "prefixlen": 64,
+                                "scope": "global",
+                            },
+                            # Test for ipv6:
+                            #  mask not set when 'address' present
+                            {
+                                "family": "inet6",
+                                "local": "fd12:3456:7890:1234::5678:9012",
+                                "address": "fd12:3456:7890:1234::1",
+                                "prefixlen": 64,
+                            },
+                        ],
+                    }
+                ],
+                {
+                    "eth0": {
+                        "hwaddr": "",
+                        "ipv4": [
+                            {
+                                "ip": "10.0.0.1",
+                                "mask": "255.255.255.0",
+                                "bcast": "10.0.0.255",
+                                "scope": "global",
+                            }
+                        ],
+                        "ipv6": [
+                            {
+                                "ip": "fd12:3456:7890:1234::5678:9012/64",
+                                "scope6": "global",
+                            },
+                            {
+                                "ip": "fd12:3456:7890:1234::5678:9012",
+                                "scope6": "",
+                            },
+                        ],
+                        "up": False,
+                    }
+                },
+            ),
+        ],
+    )
+    def test_netdev_info_iproute_json(self, input, expected):
+        out = _netdev_info_iproute_json(json.dumps(input))
+        assert out == expected
+
 
 # vi: ts=4 expandtab