From 9fabfff5aac6e1b398e18429e6dfad54f59e7f75 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Thu, 14 Apr 2022 09:31:17 -0500 Subject: [PATCH] Fix KeyError when rendering sysconfig IPv6 routes (#1380) Reference:https://github.com/canonical/cloud-init/commit/9fabfff5aac6e1b398e18429e6dfad54f59e7f75 Conflict:(1)do not delete ipv6_mask_to_net_prefix. (2)do not add "NM_CONTROLLED=no" in test because of cloud-init-20.4-nm-controlled.patch. (3)format diffs. Route rendering code was expecting a netmask rather than using the prefix. A prefix is provided to the renderer, but was being hidden from the route rendering code. This commit exposes the prefix and prefers it for IPv6, given how uncommon netmasks are for IPv6. LP: #1958506 --- cloudinit/net/network_state.py | 4 +- cloudinit/net/renderer.py | 4 +- cloudinit/net/sysconfig.py | 41 +++++++------- tests/unittests/test_net.py | 100 +++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 25 deletions(-) diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 0a4e0c6..0d81291 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -885,12 +885,12 @@ def _normalize_net_keys(network, address_keys=()): else: # this supports input of 255.255.255.0 prefix = ipv4_mask_to_net_prefix(maybe_prefix) + elif "prefix" in net: + prefix = int(net["prefix"]) elif netmask and not ipv6: prefix = ipv4_mask_to_net_prefix(netmask) elif netmask and ipv6: prefix = ipv6_mask_to_net_prefix(netmask) - elif 'prefix' in net: - prefix = int(net['prefix']) else: prefix = 64 if ipv6 else 24 diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py index 54a83b5..e387173 100644 --- a/cloudinit/net/renderer.py +++ b/cloudinit/net/renderer.py @@ -8,7 +8,7 @@ import abc import io -from cloudinit.net.network_state import parse_net_config_data +from cloudinit.net.network_state import NetworkState, parse_net_config_data from cloudinit.net.udev import generate_udev_rule @@ -32,7 +32,7 @@ class Renderer(object): pass @staticmethod - def _render_persistent_net(network_state): + def _render_persistent_net(network_state: NetworkState): """Given state, emit udev rules to map mac to ifname.""" # TODO(harlowja): this seems shared between eni renderer and # this, so move it to a shared location. diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 4c6caef..6d17d69 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -4,6 +4,7 @@ import copy import io import os import re +from typing import Mapping from configobj import ConfigObj @@ -17,6 +18,7 @@ from cloudinit.net import network_state from . import renderer from .network_state import ( is_ipv6_addr, net_prefix_to_ipv4_mask, subnet_is_ipv6, IPV6_DYNAMIC_TYPES) +from cloudinit.net.network_state import NetworkState LOG = logging.getLogger(__name__) KNOWN_DISTROS = ['almalinux', 'centos', 'cloudlinux', 'eurolinux', 'fedora', @@ -178,7 +180,6 @@ class Route(ConfigMap): index = key.replace("ADDRESS", "") address_value = str(self._conf[key]) - netmask_value = str(self._conf["NETMASK" + index]) gateway_value = str(self._conf["GATEWAY" + index]) # only accept combinations: @@ -188,6 +189,7 @@ class Route(ConfigMap): # do not add ipv4 routes if proto is ipv6 # (this array will contain a mix of ipv4 and ipv6) if proto == "ipv4" and not self.is_ipv6_route(address_value): + netmask_value = str(self._conf["NETMASK" + index]) # increase IPv4 index reindex = reindex + 1 buf.write( @@ -209,9 +211,7 @@ class Route(ConfigMap): % ("METRIC" + str(reindex), _quote_value(metric_value)) ) elif proto == "ipv6" and self.is_ipv6_route(address_value): - prefix_value = network_state.ipv6_mask_to_net_prefix( - netmask_value - ) + prefix_value = str(self._conf[f"PREFIX{index}"]) metric_value = ( "metric " + str(self._conf["METRIC" + index]) if "METRIC" + index in self._conf @@ -604,12 +604,9 @@ class Renderer(renderer.Renderer): raise ValueError("Duplicate declaration of default " "route found for interface '%s'" % (iface_cfg.name)) - # NOTE(harlowja): ipv6 and ipv4 default gateways - gw_key = 'GATEWAY0' - nm_key = 'NETMASK0' - addr_key = 'ADDRESS0' - # The owning interface provides the default route. - # + # NOTE that instead of defining the route0 settings, + # the owning interface provides the default route. + # TODO(harlowja): add validation that no other iface has # also provided the default route? iface_cfg['DEFROUTE'] = True @@ -626,19 +623,19 @@ class Renderer(renderer.Renderer): iface_cfg['METRIC'] = route['metric'] else: - gw_key = 'GATEWAY%s' % route_cfg.last_idx - nm_key = 'NETMASK%s' % route_cfg.last_idx - addr_key = 'ADDRESS%s' % route_cfg.last_idx - metric_key = 'METRIC%s' % route_cfg.last_idx - route_cfg.last_idx += 1 # add default routes only to ifcfg files, not # to route-* or route6-* - for (old_key, new_key) in [('gateway', gw_key), - ('metric', metric_key), - ('netmask', nm_key), - ('network', addr_key)]: + for old_key, new_name in [ + ("gateway", "GATEWAY"), + ("metric", "METRIC"), + ("prefix", "PREFIX"), + ("netmask", "NETMASK"), + ("network", "ADDRESS"), + ]: if old_key in route: + new_key = f"{new_name}{route_cfg.last_idx}" route_cfg[new_key] = route[old_key] + route_cfg.last_idx += 1 @classmethod def _render_bonding_opts(cls, iface_cfg, iface, flavor): @@ -890,7 +887,7 @@ class Renderer(renderer.Renderer): '''Given state, return /etc/sysconfig files + contents''' if not templates: templates = cls.templates - iface_contents = {} + iface_contents: Mapping[str, NetInterface] = {} for iface in network_state.iter_interfaces(): if iface['type'] == "loopback": continue @@ -922,7 +919,9 @@ class Renderer(renderer.Renderer): contents[cpath] = iface_cfg.routes.to_string(proto) return contents - def render_network_state(self, network_state, templates=None, target=None): + def render_network_state( + self, network_state: NetworkState, templates=None, target=None + ): if not templates: templates = self.templates file_mode = 0o644 diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 8737a76..a698c65 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -3871,6 +3871,106 @@ USERCTL=no self._compare_files_to_expected( expected, self._render_and_read(network_config=v2data)) + def test_from_v2_routes(self): + """verify routes (including IPv6) get rendered using v2 config. + LP: #1958506 + """ + v2_data = { + "version": 2, + "ethernets": { + "eth0": { + "addresses": [ + "10.54.2.19/21", + "2a00:1730:fff9:100::52/128", + ], + "gateway4": "10.54.0.1", + "gateway6": "2a00:1730:fff9:100::1", + "match": {"macaddress": "52:54:00:3f:fc:f7"}, + "mtu": 1400, + "nameservers": { + "addresses": [ + "10.52.1.1", + "10.52.1.71", + "2001:4860:4860::8888", + "2001:4860:4860::8844", + ] + }, + "routes": [ + { + "scope": "link", + "to": "10.54.0.1/32", + "via": "0.0.0.0", + }, + { + "scope": "link", + "to": "0.0.0.0/0", + "via": "10.54.0.1", + }, + { + "scope": "link", + "to": "2a00:1730:fff9:100::1/128", + "via": "::0", + }, + { + "scope": "link", + "to": "::0/0", + "via": "2a00:1730:fff9:100::1", + }, + ], + "set-name": "eth0", + } + }, + } + + expected = { + "ifcfg-eth0": textwrap.dedent( + """\ + # Created by cloud-init on instance boot automatically, do not edit. + # + BOOTPROTO=none + DEFROUTE=yes + DEVICE=eth0 + DNS1=10.52.1.1 + DNS2=10.52.1.71 + DNS3=2001:4860:4860::8888 + GATEWAY=10.54.0.1 + HWADDR=52:54:00:3f:fc:f7 + IPADDR=10.54.2.19 + IPV6ADDR=2a00:1730:fff9:100::52/128 + IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_DEFAULTGW=2a00:1730:fff9:100::1 + IPV6_FORCE_ACCEPT_RA=no + MTU=1400 + NETMASK=255.255.248.0 + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """ # noqa: E501 + ), + "route-eth0": textwrap.dedent( + """\ + # Created by cloud-init on instance boot automatically, do not edit. + # + ADDRESS0=10.54.0.1 + GATEWAY0=0.0.0.0 + NETMASK0=255.255.255.255 + """ # noqa: E501 + ), + "route6-eth0": textwrap.dedent( + """\ + # Created by cloud-init on instance boot automatically, do not edit. + # + 2a00:1730:fff9:100::1/128 via ::0 dev eth0 + ::0/64 via 2a00:1730:fff9:100::1 dev eth0 + """ # noqa: E501 + ), + } + + found = self._render_and_read(network_config=v2_data) + self._compare_files_to_expected(expected, found) + self._assert_headers(found) + @mock.patch( "cloudinit.net.is_openvswitch_internal_interface", -- 2.33.0