From a926db3db5b9a8288ef3d0ee7a7004009580d919 Mon Sep 17 00:00:00 2001 From: akutz Date: Tue, 15 Mar 2022 13:56:56 -0500 Subject: [PATCH] Fix sysconfig render when set-name is missing This patch addresses https://bugs.launchpad.net/cloud-init/+bug/1855945, where the absence of set-name in a network configuration potentially results in an unintended network configuration. Please note a function from cloudinit.net.networkstate was relocated under cloudinit.net due to a circular dependency introduced by the former calling get_interfaces_by_mac in the latter. A stub function has been created to preserve backward compatibility. --- cloudinit/net/network_state.py | 29 +++++++++++- tests/unittests/test_net.py | 81 +++++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 880ca462b78..e3e73335a92 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -10,6 +10,7 @@ from cloudinit import safeyaml, util from cloudinit.net import ( + get_interfaces_by_mac, ipv4_mask_to_net_prefix, ipv6_mask_to_net_prefix, is_ipv6_addr, @@ -665,10 +666,18 @@ def handle_ethernets(self, command): ] } """ + + # Get the interfaces by MAC address to update an interface's + # device name to the name of the device that matches a provided + # MAC address when the set-name directive is not present. + # + # Please see https://bugs.launchpad.net/cloud-init/+bug/1855945 + # for more information. + ifaces_by_mac = get_interfaces_by_mac() + for eth, cfg in command.items(): phy_cmd = { "type": "physical", - "name": cfg.get("set-name", eth), } match = cfg.get("match", {}) mac_address = match.get("macaddress", None) @@ -680,6 +689,24 @@ def handle_ethernets(self, command): str(cfg), ) phy_cmd["mac_address"] = mac_address + + # Determine the name of the interface by using one of the + # following in the order they are listed: + # * set-name + # * interface name looked up by mac + # * value of "eth" key from this loop + name = eth + set_name = cfg.get("set-name", None) + if set_name: + name = set_name + elif mac_address and ifaces_by_mac: + lcase_mac_address = mac_address.lower() + for iface_mac, iface_name in ifaces_by_mac.items(): + if lcase_mac_address == iface_mac.lower(): + name = iface_name + break + phy_cmd["name"] = name + driver = match.get("driver", None) if driver: phy_cmd["params"] = {"driver": driver} diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 9f73c5c61b9..fa8b99a08e9 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -3871,6 +3871,29 @@ ), }, }, + "v2-dev-name-via-mac-lookup": { + "expected_sysconfig_rhel": { + "ifcfg-eth0": textwrap.dedent( + """\ + BOOTPROTO=none + DEVICE=eth0 + HWADDR=cf:d6:af:48:e8:80 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no""" + ), + }, + "yaml": textwrap.dedent( + """\ + version: 2 + ethernets: + nic0: + match: + macaddress: 'cf:d6:af:48:e8:80' + """ + ), + }, } @@ -3954,6 +3977,9 @@ "device/driver": None, "device/device": None, "name_assign_type": "4", + "addr_assign_type": "0", + "uevent": "", + "type": "32", } } @@ -4080,16 +4106,22 @@ def test_device_driver( "device/driver": "hv_netsvc", "device/device": "0x3", "name_assign_type": "4", + "addr_assign_type": "0", + "uevent": "", + "type": "32", }, "eth1": { "bridge": False, "carrier": False, "dormant": False, "operstate": "down", - "address": "00:11:22:33:44:55", + "address": "00:11:22:33:44:56", "device/driver": "mlx4_core", "device/device": "0x7", "name_assign_type": "4", + "addr_assign_type": "0", + "uevent": "", + "type": "32", }, } @@ -4158,16 +4190,22 @@ def test_device_driver_blacklist( "device/driver": "hv_netsvc", "device/device": "0x3", "name_assign_type": "4", + "addr_assign_type": "0", + "uevent": "", + "type": "32", }, "eth0": { "bridge": False, "carrier": False, "dormant": False, "operstate": "down", - "address": "00:11:22:33:44:55", + "address": "00:11:22:33:44:56", "device/driver": "mlx4_core", "device/device": "0x7", "name_assign_type": "4", + "addr_assign_type": "0", + "uevent": "", + "type": "32", }, } @@ -5099,6 +5137,45 @@ def test_from_v2_route_metric(self): expected, self._render_and_read(network_config=v2data) ) + @mock.patch("cloudinit.net.sys_dev_path") + @mock.patch("cloudinit.net.read_sys_net") + @mock.patch("cloudinit.net.get_devicelist") + def test_iface_name_from_device_with_matching_mac_address( + self, + mock_get_devicelist, + mock_read_sys_net, + mock_sys_dev_path, + ): + devices = { + "eth0": { + "bridge": False, + "carrier": False, + "dormant": False, + "operstate": "down", + "address": "CF:D6:AF:48:E8:80", + "device/driver": "hv_netsvc", + "device/device": "0x3", + "name_assign_type": "4", + "addr_assign_type": "0", + "uevent": "", + "type": "32", + }, + } + + tmp_dir = self.tmp_dir() + _setup_test( + tmp_dir, + mock_get_devicelist, + mock_read_sys_net, + mock_sys_dev_path, + dev_attrs=devices, + ) + + entry = NETWORK_CONFIGS["v2-dev-name-via-mac-lookup"] + found = self._render_and_read(network_config=yaml.load(entry["yaml"])) + self._compare_files_to_expected(entry[self.expected_name], found) + self._assert_headers(found) + @mock.patch( "cloudinit.net.is_openvswitch_internal_interface",