From b57889c84f283fc7cd063b90a507dd912936a1b0 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Tue, 17 Oct 2023 16:38:27 -0400 Subject: [PATCH] Use UUID for setting parent interface in vlans (#4633) * Use UUID for setting parent interface in vlans * Fix vlan test using interface name --- supervisor/dbus/network/setting/generate.py | 18 +++++++-- supervisor/host/network.py | 5 ++- tests/api/test_network.py | 2 +- tests/dbus/network/setting/test_generate.py | 45 ++++++++++++++++++++- tests/dbus/network/setting/test_init.py | 4 ++ 5 files changed, 65 insertions(+), 9 deletions(-) diff --git a/supervisor/dbus/network/setting/generate.py b/supervisor/dbus/network/setting/generate.py index df8217756..e6717437e 100644 --- a/supervisor/dbus/network/setting/generate.py +++ b/supervisor/dbus/network/setting/generate.py @@ -2,7 +2,7 @@ from __future__ import annotations import socket -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING from uuid import uuid4 from dbus_fast import Variant @@ -19,6 +19,7 @@ from . import ( CONF_ATTR_PATH, CONF_ATTR_VLAN, ) +from .. import NetworkManager from ....host.const import InterfaceMethod, InterfaceType if TYPE_CHECKING: @@ -26,8 +27,11 @@ if TYPE_CHECKING: def get_connection_from_interface( - interface: Interface, name: str | None = None, uuid: str | None = None -) -> Any: + interface: Interface, + network_manager: NetworkManager, + name: str | None = None, + uuid: str | None = None, +) -> dict[str, dict[str, Variant]]: """Generate message argument for network interface update.""" # Generate/Update ID/name @@ -121,9 +125,15 @@ def get_connection_from_interface( if interface.type == InterfaceType.ETHERNET: conn[CONF_ATTR_802_ETHERNET] = {ATTR_ASSIGNED_MAC: Variant("s", "preserve")} elif interface.type == "vlan": + parent = interface.vlan.interface + if parent in network_manager and ( + parent_connection := network_manager.get(parent).connection + ): + parent = parent_connection.uuid + conn[CONF_ATTR_VLAN] = { "id": Variant("u", interface.vlan.id), - "parent": Variant("s", interface.vlan.interface), + "parent": Variant("s", parent), } elif interface.type == InterfaceType.WIRELESS: wireless = { diff --git a/supervisor/host/network.py b/supervisor/host/network.py index e9b5e8fb2..981bd9f31 100644 --- a/supervisor/host/network.py +++ b/supervisor/host/network.py @@ -189,6 +189,7 @@ class NetworkManager(CoreSysAttributes): _LOGGER.debug("Updating existing configuration for %s", interface.name) settings = get_connection_from_interface( interface, + self.sys_dbus.network, name=inet.settings.connection.id, uuid=inet.settings.connection.uuid, ) @@ -217,7 +218,7 @@ class NetworkManager(CoreSysAttributes): # Create new configuration and activate interface elif inet and interface.enabled: _LOGGER.debug("Create new configuration for %s", interface.name) - settings = get_connection_from_interface(interface) + settings = get_connection_from_interface(interface, self.sys_dbus.network) try: settings, con = await self.sys_dbus.network.add_and_activate_connection( @@ -244,7 +245,7 @@ class NetworkManager(CoreSysAttributes): # Create new interface (like vlan) elif not inet: - settings = get_connection_from_interface(interface) + settings = get_connection_from_interface(interface, self.sys_dbus.network) try: await self.sys_dbus.network.settings.add_connection(settings) diff --git a/tests/api/test_network.py b/tests/api/test_network.py index 707ad2da8..3c8f7a04a 100644 --- a/tests/api/test_network.py +++ b/tests/api/test_network.py @@ -253,5 +253,5 @@ async def test_api_network_vlan( assert connection["ipv6"] == {"method": Variant("s", "auto")} assert connection["vlan"] == { "id": Variant("u", 1), - "parent": Variant("s", "eth0"), + "parent": Variant("s", "0c23631e-2118-355c-bbb0-8943229cb0d6"), } diff --git a/tests/dbus/network/setting/test_generate.py b/tests/dbus/network/setting/test_generate.py index bf22d0cb9..ac6aa5ed8 100644 --- a/tests/dbus/network/setting/test_generate.py +++ b/tests/dbus/network/setting/test_generate.py @@ -5,6 +5,8 @@ from unittest.mock import PropertyMock, patch from supervisor.dbus.network import NetworkManager from supervisor.dbus.network.interface import NetworkInterface from supervisor.dbus.network.setting.generate import get_connection_from_interface +from supervisor.host.configuration import IpConfig, VlanConfig +from supervisor.host.const import InterfaceMethod, InterfaceType from supervisor.host.network import Interface from tests.const import TEST_INTERFACE @@ -14,7 +16,7 @@ async def test_get_connection_from_interface(network_manager: NetworkManager): """Test network interface.""" dbus_interface = network_manager.get(TEST_INTERFACE) interface = Interface.from_dbus_interface(dbus_interface) - connection_payload = get_connection_from_interface(interface) + connection_payload = get_connection_from_interface(interface, network_manager) assert "connection" in connection_payload @@ -35,9 +37,48 @@ async def test_get_connection_no_path(network_manager: NetworkManager): with patch.object(NetworkInterface, "path", new=PropertyMock(return_value=None)): interface = Interface.from_dbus_interface(dbus_interface) - connection_payload = get_connection_from_interface(interface) + connection_payload = get_connection_from_interface(interface, network_manager) assert "connection" in connection_payload assert "match" not in connection_payload assert connection_payload["connection"]["interface-name"].value == "eth0" + + +async def test_generate_from_vlan(network_manager: NetworkManager): + """Test generate from a vlan interface.""" + vlan_interface = Interface( + name="", + mac="", + path="", + enabled=True, + connected=True, + primary=False, + type=InterfaceType.VLAN, + ipv4=IpConfig(InterfaceMethod.AUTO, [], None, [], None), + ipv6=None, + wifi=None, + vlan=VlanConfig(1, "eth0"), + ) + + connection_payload = get_connection_from_interface(vlan_interface, network_manager) + assert connection_payload["connection"]["id"].value == "Supervisor .1" + assert connection_payload["connection"]["type"].value == "vlan" + assert "uuid" in connection_payload["connection"] + assert "match" not in connection_payload["connection"] + assert "interface-name" not in connection_payload["connection"] + assert connection_payload["ipv4"]["method"].value == "auto" + + assert connection_payload["vlan"]["id"].value == 1 + assert ( + connection_payload["vlan"]["parent"].value + == "0c23631e-2118-355c-bbb0-8943229cb0d6" + ) + + # Ensure value remains if parent interface is already a UUID + vlan_interface.vlan.interface = "0c23631e-2118-355c-bbb0-8943229cb0d6" + connection_payload = get_connection_from_interface(vlan_interface, network_manager) + assert ( + connection_payload["vlan"]["parent"].value + == "0c23631e-2118-355c-bbb0-8943229cb0d6" + ) diff --git a/tests/dbus/network/setting/test_init.py b/tests/dbus/network/setting/test_init.py index f8f66bc60..6018fa52f 100644 --- a/tests/dbus/network/setting/test_init.py +++ b/tests/dbus/network/setting/test_init.py @@ -1,5 +1,7 @@ """Test Network Manager Connection object.""" +from unittest.mock import MagicMock + from dbus_fast.aio.message_bus import MessageBus from dbus_fast.signature import Variant import pytest @@ -42,6 +44,7 @@ async def test_update( interface = Interface.from_dbus_interface(dbus_interface) conn = get_connection_from_interface( interface, + MagicMock(), name=dbus_interface.settings.connection.id, uuid=dbus_interface.settings.connection.uuid, ) @@ -105,6 +108,7 @@ async def test_ipv6_disabled_is_link_local(dbus_interface: NetworkInterface): interface.ipv6.method = InterfaceMethod.DISABLED conn = get_connection_from_interface( interface, + MagicMock(), name=dbus_interface.settings.connection.id, uuid=dbus_interface.settings.connection.uuid, )