← Back to team overview

cloud-init-dev team mailing list archive

[Merge] ~raharper/cloud-init:fix/ephemeral-dhcp-static-routes into cloud-init:master

 

Ryan Harper has proposed merging ~raharper/cloud-init:fix/ephemeral-dhcp-static-routes into cloud-init:master.

Commit message:
net: add rfc3442 (classless static routes) to EphemeralDHCP
    
The EphemeralDHCP context manager did not parse or handle
rfc3442 classless static routes which prevented reading
datasource metadata in some clouds.  This branch adds support
for extracting the field from the leases output, parsing the
format and then adding the required iproute2 ip commands to
apply (and teardown) the static routes.
    
LP: #1821102

Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1821102 in cloud-init: "Cloud-init should not setup ephemeral ipv4 if apply_network_config is False for OpenStack"
  https://bugs.launchpad.net/cloud-init/+bug/1821102

For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/368553
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:fix/ephemeral-dhcp-static-routes into cloud-init:master.
diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index 3642fb1..73ac819 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -677,7 +677,7 @@ class EphemeralIPv4Network(object):
     """
 
     def __init__(self, interface, ip, prefix_or_mask, broadcast, router=None,
-                 connectivity_url=None):
+                 connectivity_url=None, static_routes=None):
         """Setup context manager and validate call signature.
 
         @param interface: Name of the network interface to bring up.
@@ -688,6 +688,7 @@ class EphemeralIPv4Network(object):
         @param router: Optionally the default gateway IP.
         @param connectivity_url: Optionally, a URL to verify if a usable
            connection already exists.
+        @param static_routes: Optionally a list of static routes from DHCP
         """
         if not all([interface, ip, prefix_or_mask, broadcast]):
             raise ValueError(
@@ -704,6 +705,7 @@ class EphemeralIPv4Network(object):
         self.ip = ip
         self.broadcast = broadcast
         self.router = router
+        self.static_routes = static_routes
         self.cleanup_cmds = []  # List of commands to run to cleanup state.
 
     def __enter__(self):
@@ -716,7 +718,21 @@ class EphemeralIPv4Network(object):
                 return
 
         self._bringup_device()
-        if self.router:
+
+        # rfc3442 requires us to ignore the router config *if* classless static
+        # routes are provided.
+        #
+        # https://tools.ietf.org/html/rfc3442
+        #
+        # If the DHCP server returns both a Classless Static Routes option and
+        # a Router option, the DHCP client MUST ignore the Router option.
+        #
+        # Similarly, if the DHCP server returns both a Classless Static Routes
+        # option and a Static Routes option, the DHCP client MUST ignore the
+        # Static Routes option.
+        if self.static_routes:
+            self._bringup_static_routes()
+        elif self.router:
             self._bringup_router()
 
     def __exit__(self, excp_type, excp_value, excp_traceback):
@@ -760,6 +776,21 @@ class EphemeralIPv4Network(object):
                 ['ip', '-family', 'inet', 'addr', 'del', cidr, 'dev',
                  self.interface])
 
+    def _bringup_static_routes(self):
+        # static_routes = ["169.254.169.254/32,130.56.248.255",
+        #                  "0.0.0.0/0,130.56.240.1"]
+        for sroute in self.static_routes:
+            net_address, gateway = sroute.split(",")
+            via_arg = []
+            if gateway != "0.0.0.0/0":
+                via_arg = ['via', gateway]
+            util.subp(
+                ['ip', '-4', 'route', 'add', net_address] + via_arg +
+                ['dev', self.interface], capture=True)
+            self.cleanup_cmds.insert(
+                0, ['ip', '-4', 'route', 'del', net_address] + via_arg +
+                   ['dev', self.interface])
+
     def _bringup_router(self):
         """Perform the ip commands to fully setup the router if needed."""
         # Check if a default route exists and exit if it does
diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py
index c98a97c..92d2076 100644
--- a/cloudinit/net/dhcp.py
+++ b/cloudinit/net/dhcp.py
@@ -92,10 +92,14 @@ class EphemeralDHCPv4(object):
         nmap = {'interface': 'interface', 'ip': 'fixed-address',
                 'prefix_or_mask': 'subnet-mask',
                 'broadcast': 'broadcast-address',
+                'static_routes': 'rfc3442-classless-static-routes',
                 'router': 'routers'}
         kwargs = dict([(k, self.lease.get(v)) for k, v in nmap.items()])
         if not kwargs['broadcast']:
             kwargs['broadcast'] = bcip(kwargs['prefix_or_mask'], kwargs['ip'])
+        if kwargs['static_routes']:
+            kwargs['static_routes'] = (
+                parse_static_routes(kwargs['static_routes']))
         if self.connectivity_url:
             kwargs['connectivity_url'] = self.connectivity_url
         ephipv4 = EphemeralIPv4Network(**kwargs)
@@ -272,4 +276,55 @@ def networkd_get_option_from_leases(keyname, leases_d=None):
             return data[keyname]
     return None
 
+
+def parse_static_routes(rfc3442):
+    # 32,169,254,169,254,130,56,248,255,0,130,56,240,1 ->
+    # [
+    #  "169.254.169.254/32,130.56.248.255",
+    #  "0.0.0.0/0",130.56.248.255"
+    # ]
+
+    rfc3442 = rfc3442.rstrip(";")
+    tokens = rfc3442.split(',')
+    static_routes = []
+
+    current_idx = 0
+    for idx, tok in enumerate(tokens):
+        if idx < current_idx:
+            continue
+        net_length = int(tok)
+        if net_length in range(25, 33):
+            if len(tokens[idx:]) < 9:
+                return None
+            net_address = ".".join(tokens[idx+1:idx+5])
+            gateway = ".".join(tokens[idx+5:idx+9])
+            current_idx = idx + 9
+        elif net_length in range(17, 25):
+            if len(tokens[idx:]) < 8:
+                return None
+            net_address = ".".join(tokens[idx+1:idx+4] + ["0"])
+            gateway = ".".join(tokens[idx+4:idx+8])
+            current_idx = idx + 8
+        elif net_length in range(9, 17):
+            if len(tokens[idx:]) < 7:
+                return None
+            net_address = ".".join(tokens[idx+1:idx+3] + ["0", "0"])
+            gateway = ".".join(tokens[idx+3:idx+7])
+            current_idx = idx + 7
+        elif net_length in range(1, 9):
+            if len(tokens[idx:]) < 6:
+                return None
+            net_address = ".".join(tokens[idx+1:idx+2] + ["0", "0", "0"])
+            gateway = ".".join(tokens[idx+2:idx+6])
+            current_idx = idx + 6
+        elif net_length == 0:
+            if len(tokens[idx:]) < 5:
+                return None
+            net_address = "0.0.0.0/0"
+            gateway = ".".join(tokens[idx+1:idx+5])
+            current_idx = idx + 5
+
+        static_routes.append("%s,%s" % (net_address, gateway))
+    return static_routes
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py
index 5139024..d7fd727 100644
--- a/cloudinit/net/tests/test_dhcp.py
+++ b/cloudinit/net/tests/test_dhcp.py
@@ -64,6 +64,54 @@ class TestParseDHCPLeasesFile(CiTestCase):
         self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file))
 
 
+class TestDHCPRFC3442(CiTestCase):
+
+    def test_parse_lease_finds_rfc3442_classless_static_routes(self):
+        """parse_dhcp_lease_file returns rfc3442-classless-static-routes."""
+        lease_file = self.tmp_path('leases')
+        content = dedent("""
+            lease {
+              interface "wlp3s0";
+              fixed-address 192.168.2.74;
+              option subnet-mask 255.255.255.0;
+              option routers 192.168.2.1;
+              option rfc3442-classless-static-routes 0,130,56,240,1;
+              renew 4 2017/07/27 18:02:30;
+              expire 5 2017/07/28 07:08:15;
+            }
+        """)
+        expected = [
+            {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74',
+             'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1',
+             'rfc3442-classless-static-routes': '0,130,56,240,1',
+             'renew': '4 2017/07/27 18:02:30',
+             'expire': '5 2017/07/28 07:08:15'}]
+        write_file(lease_file, content)
+        self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file))
+
+    @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
+    @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
+    def test_obtain_lease_parses_static_routes(self, m_maybe, m_ipv4):
+        """EphemeralDHPCv4 parses rfc3442 routes for EphemeralIPv4Network"""
+        lease = [
+            {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74',
+             'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1',
+             'rfc3442-classless-static-routes': '0,130,56,240,1',
+             'renew': '4 2017/07/27 18:02:30',
+             'expire': '5 2017/07/28 07:08:15'}]
+        m_maybe.return_value = lease
+        eph = net.dhcp.EphemeralDHCPv4()
+        eph.obtain_lease()
+        expected_kwargs = {
+            'interface': 'wlp3s0',
+            'ip': '192.168.2.74',
+            'prefix_or_mask': '255.255.255.0',
+            'broadcast': '192.168.2.255',
+            'static_routes': ['0.0.0.0/0,130.56.240.1'],
+            'router': '192.168.2.1'}
+        m_ipv4.assert_called_with(**expected_kwargs)
+
+
 class TestDHCPDiscoveryClean(CiTestCase):
     with_logs = True
 
diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py
index 6d2affe..b5686a9 100644
--- a/cloudinit/net/tests/test_init.py
+++ b/cloudinit/net/tests/test_init.py
@@ -549,6 +549,45 @@ class TestEphemeralIPV4Network(CiTestCase):
             self.assertEqual(expected_setup_calls, m_subp.call_args_list)
         m_subp.assert_has_calls(expected_teardown_calls)
 
+    def test_ephemeral_ipv4_network_with_rfc3442_static_routes(self, m_subp):
+        params = {
+            'interface': 'eth0', 'ip': '192.168.2.2',
+            'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255',
+            'static_routes': ['169.254.169.254/32,192.168.2.1',
+                              '0.0.0.0/0,192.168.2.1'],
+            'router': '192.168.2.1'}
+        expected_setup_calls = [
+            mock.call(
+                ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/24',
+                 'broadcast', '192.168.2.255', 'dev', 'eth0'],
+                capture=True, update_env={'LANG': 'C'}),
+            mock.call(
+                ['ip', '-family', 'inet', 'link', 'set', 'dev', 'eth0', 'up'],
+                capture=True),
+            mock.call(
+                ['ip', '-4', 'route', 'add', '169.254.169.254/32',
+                 'via', '192.168.2.1', 'dev', 'eth0'], capture=True),
+            mock.call(
+                ['ip', '-4', 'route', 'add', '0.0.0.0/0',
+                 'via', '192.168.2.1', 'dev', 'eth0'], capture=True)]
+        expected_teardown_calls = [
+            mock.call(
+                ['ip', '-4', 'route', 'del', '0.0.0.0/0',
+                 'via', '192.168.2.1', 'dev', 'eth0'], capture=True),
+            mock.call(
+                ['ip', '-4', 'route', 'del', '169.254.169.254/32',
+                 'via', '192.168.2.1', 'dev', 'eth0'], capture=True),
+            mock.call(
+                ['ip', '-family', 'inet', 'link', 'set', 'dev',
+                 'eth0', 'down'], capture=True),
+            mock.call(
+                ['ip', '-family', 'inet', 'addr', 'del',
+                 '192.168.2.2/24', 'dev', 'eth0'], capture=True)
+        ]
+        with net.EphemeralIPv4Network(**params):
+            self.assertEqual(expected_setup_calls, m_subp.call_args_list)
+        m_subp.assert_has_calls(expected_setup_calls + expected_teardown_calls)
+
 
 class TestApplyNetworkCfgNames(CiTestCase):
     V1_CONFIG = textwrap.dedent("""\
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index afb614e..478e4f1 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -1774,7 +1774,8 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         self.assertEqual(m_dhcp.call_count, 2)
         m_net.assert_any_call(
             broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9',
-            prefix_or_mask='255.255.255.0', router='192.168.2.1')
+            prefix_or_mask='255.255.255.0', router='192.168.2.1',
+            static_routes=None)
         self.assertEqual(m_net.call_count, 2)
 
     def test__reprovision_calls__poll_imds(self, fake_resp,
@@ -1812,7 +1813,8 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         self.assertEqual(m_dhcp.call_count, 2)
         m_net.assert_any_call(
             broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9',
-            prefix_or_mask='255.255.255.0', router='192.168.2.1')
+            prefix_or_mask='255.255.255.0', router='192.168.2.1',
+            static_routes=None)
         self.assertEqual(m_net.call_count, 2)
 
 
diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
index 20d59bf..1ec8e00 100644
--- a/tests/unittests/test_datasource/test_ec2.py
+++ b/tests/unittests/test_datasource/test_ec2.py
@@ -538,7 +538,8 @@ class TestEc2(test_helpers.HttprettyTestCase):
         m_dhcp.assert_called_once_with('eth9')
         m_net.assert_called_once_with(
             broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9',
-            prefix_or_mask='255.255.255.0', router='192.168.2.1')
+            prefix_or_mask='255.255.255.0', router='192.168.2.1',
+            static_routes=None)
         self.assertIn('Crawl of metadata service took', self.logs.getvalue())