Split payload so we can set auto without IP configuration (#1982)

* Guard for no interfaces

* Host reload on update

* Extract payload

* Check eth and wifi interfaces with a valid ip4 config

* Add tests

* Fix tests

* Move to enum

setup.py

@@ -36,6 +36,8 @@ setup(
         "supervisor.addons",
         "supervisor.api",
         "supervisor.dbus",
+        "supervisor.dbus.payloads",
+        "supervisor.dbus.network",
         "supervisor.discovery",
         "supervisor.discovery.services",
         "supervisor.services",

supervisor/api/network.py

@@ -93,6 +93,6 @@ class APINetwork(CoreSysAttributes):
             self.sys_host.network.interfaces[req_interface].update_settings(**args)
         )
 
-        await asyncio.shield(self.sys_host.network.update())
+        await asyncio.shield(self.sys_host.reload())
 
         return await asyncio.shield(self.interface_info(request))

supervisor/dbus/const.py

@@ -72,3 +72,10 @@ class InterfaceMethodSimple(str, Enum):
 
     DHCP = "dhcp"
     STATIC = "static"
+
+
+class ConnectionType(str, Enum):
+    """Connection type."""
+
+    ETHERNET = "802-3-ethernet"
+    WIRELESS = "802-11-wireless"

supervisor/dbus/network/__init__.py

@@ -2,13 +2,14 @@
 import logging
 from typing import Dict, Optional
 
-from ...exceptions import DBusError, DBusInterfaceError
+from ...exceptions import DBusError, DBusFatalError, DBusInterfaceError
 from ...utils.gdbus import DBus
 from ..const import (
     DBUS_ATTR_ACTIVE_CONNECTIONS,
     DBUS_ATTR_PRIMARY_CONNECTION,
     DBUS_NAME_NM,
     DBUS_OBJECT_NM,
+    ConnectionType,
 )
 from ..interface import DBusInterface
 from ..utils import dbus_connected
@@ -65,11 +66,17 @@ class NetworkManager(DBusInterface):
 
             await interface.connect(self.dbus, connection)
 
-            if not interface.connection.default:
+            if interface.connection.type not in [
+                ConnectionType.ETHERNET,
+                ConnectionType.WIRELESS,
+            ]:
                 continue
             try:
                 await interface.connection.update_information()
-            except IndexError:
+            except (IndexError, DBusFatalError):
+                continue
+
+            if not interface.connection.ip4_config:
                 continue
 
             if interface.connection.object_path == data.get(

supervisor/dbus/network/connection.py

@@ -22,6 +22,7 @@ from ..const import (
     DBUS_NAME_DEVICE,
     DBUS_NAME_IP4CONFIG,
     DBUS_NAME_NM,
+    DBUS_OBJECT_BASE,
 )
 from .configuration import (
     AddressData,
@@ -91,6 +92,9 @@ class NetworkConnection(NetworkAttributes):
 
     async def update_information(self):
         """Update the information for childs ."""
+        if self._properties[DBUS_ATTR_IP4CONFIG] == DBUS_OBJECT_BASE:
+            return
+
         settings = await DBus.connect(
             DBUS_NAME_NM, self._properties[DBUS_ATTR_CONNECTION]
         )

supervisor/dbus/network/interface.py

@@ -1,5 +1,4 @@
 """NetworkInterface object for Network Manager."""
-from ...const import ATTR_ADDRESS, ATTR_DNS, ATTR_GATEWAY, ATTR_METHOD, ATTR_PREFIX
 from ...utils.gdbus import DBus
 from ..const import (
     DBUS_NAME_CONNECTION_ACTIVE,
@@ -7,8 +6,8 @@ from ..const import (
     DBUS_OBJECT_BASE,
     InterfaceMethod,
 )
+from ..payloads.interface_update import interface_update_payload
 from .connection import NetworkConnection
-from .utils import ip2int
 
 
 class NetworkInterface:
@@ -85,42 +84,9 @@ class NetworkInterface:
 
     async def update_settings(self, **kwargs) -> None:
         """Update IP configuration used for this interface."""
-        if kwargs.get(ATTR_DNS):
+        payload = interface_update_payload(self, **kwargs)
-            kwargs[ATTR_DNS] = [ip2int(x.strip()) for x in kwargs[ATTR_DNS]]
 
-        if kwargs.get(ATTR_METHOD):
+        await self.connection.settings.dbus.Settings.Connection.Update(payload)
-            kwargs[ATTR_METHOD] = (
-                InterfaceMethod.MANUAL
-                if kwargs[ATTR_METHOD] == "static"
-                else InterfaceMethod.AUTO
-            )
-
-        if kwargs.get(ATTR_ADDRESS):
-            if "/" in kwargs[ATTR_ADDRESS]:
-                kwargs[ATTR_PREFIX] = kwargs[ATTR_ADDRESS].split("/")[-1]
-                kwargs[ATTR_ADDRESS] = kwargs[ATTR_ADDRESS].split("/")[0]
-            kwargs[ATTR_METHOD] = InterfaceMethod.MANUAL
-
-        await self.connection.settings.dbus.Settings.Connection.Update(
-            f"""{{
-                    'connection':
-                        {{
-                            'id': <'{self.id}'>,
-                            'type': <'{self.type}'>
-                        }},
-                    'ipv4':
-                        {{
-                            'method': <'{kwargs.get(ATTR_METHOD, self.method)}'>,
-                            'dns': <[{",".join([f"uint32 {x}" for x in kwargs.get(ATTR_DNS, self.nameservers)])}]>,
-                            'address-data': <[
-                                {{
-                                    'address': <'{kwargs.get(ATTR_ADDRESS, self.ip_address)}'>,
-                                    'prefix': <uint32 {kwargs.get(ATTR_PREFIX, self.prefix)}>
-                                }}]>,
-                            'gateway': <'{kwargs.get(ATTR_GATEWAY, self.gateway)}'>
-                                }}
-                }}"""
-        )
 
         await self.nm_dbus.ActivateConnection(
             self.connection.settings.dbus.object_path,

supervisor/dbus/payloads/__init__.py

@@ -0,0 +1 @@
+"""Init file for DBUS payloads."""

supervisor/dbus/payloads/interface_update.py

@@ -0,0 +1,55 @@
+"""Payload generators for DBUS communication."""
+from ...const import ATTR_ADDRESS, ATTR_DNS, ATTR_GATEWAY, ATTR_METHOD, ATTR_PREFIX
+from ..const import InterfaceMethod
+from ..network.utils import ip2int
+
+
+def interface_update_payload(interface, **kwargs) -> str:
+    """Generate a payload for network interface update."""
+    if kwargs.get(ATTR_DNS):
+        kwargs[ATTR_DNS] = [ip2int(x.strip()) for x in kwargs[ATTR_DNS]]
+
+    if kwargs.get(ATTR_METHOD):
+        kwargs[ATTR_METHOD] = (
+            InterfaceMethod.MANUAL
+            if kwargs[ATTR_METHOD] == "static"
+            else InterfaceMethod.AUTO
+        )
+
+    if kwargs.get(ATTR_ADDRESS):
+        if "/" in kwargs[ATTR_ADDRESS]:
+            kwargs[ATTR_PREFIX] = kwargs[ATTR_ADDRESS].split("/")[-1]
+            kwargs[ATTR_ADDRESS] = kwargs[ATTR_ADDRESS].split("/")[0]
+        kwargs[ATTR_METHOD] = InterfaceMethod.MANUAL
+
+    if kwargs.get(ATTR_METHOD) == "auto":
+        return f"""{{
+                    'connection':
+                        {{
+                            'id': <'{interface.id}'>,
+                            'type': <'{interface.type}'>
+                        }},
+                    'ipv4':
+                        {{
+                            'method': <'{InterfaceMethod.AUTO}'>
+                        }}
+                }}"""
+
+    return f"""{{
+                    'connection':
+                        {{
+                            'id': <'{interface.id}'>,
+                            'type': <'{interface.type}'>
+                        }},
+                    'ipv4':
+                        {{
+                            'method': <'{InterfaceMethod.MANUAL}'>,
+                            'dns': <[{",".join([f"uint32 {x}" for x in kwargs.get(ATTR_DNS, interface.nameservers)])}]>,
+                            'address-data': <[
+                                {{
+                                    'address': <'{kwargs.get(ATTR_ADDRESS, interface.ip_address)}'>,
+                                    'prefix': <uint32 {kwargs.get(ATTR_PREFIX, interface.prefix)}>
+                                }}]>,
+                            'gateway': <'{kwargs.get(ATTR_GATEWAY, interface.gateway)}'>
+                                }}
+                }}"""

supervisor/host/__init__.py

@@ -69,7 +69,7 @@ class HostManager(CoreSysAttributes):
                 [HostFeature.REBOOT, HostFeature.SHUTDOWN, HostFeature.SERVICES]
             )
 
-        if self.sys_dbus.network.is_connected:
+        if self.sys_dbus.network.is_connected and self.sys_dbus.network.interfaces:
             features.append(HostFeature.NETWORK)
 
         if self.sys_dbus.hostname.is_connected:

tests/conftest.py

@@ -11,6 +11,7 @@ from supervisor.bootstrap import initialize_coresys
 from supervisor.coresys import CoreSys
 from supervisor.dbus.const import DBUS_NAME_NM, DBUS_OBJECT_BASE
 from supervisor.dbus.network import NetworkManager
+from supervisor.dbus.network.interface import NetworkInterface
 from supervisor.docker import DockerAPI
 from supervisor.utils.gdbus import DBus
 
@@ -124,3 +125,12 @@ async def api_client(aiohttp_client, coresys):
     api.webapp = web.Application()
     await api.load()
     yield await aiohttp_client(api.webapp)
+
+
+@pytest.fixture
+async def network_interface(dbus):
+    """Fixture for a network interface."""
+    interface = NetworkInterface()
+    await interface.connect(dbus, "/org/freedesktop/NetworkManager/ActiveConnection/1")
+    await interface.connection.update_information()
+    yield interface

tests/dbus/payloads/test_interface_update_payload.py

@@ -0,0 +1,49 @@
+"""Test interface update payload."""
+import pytest
+
+from supervisor.dbus.payloads.interface_update import interface_update_payload
+from supervisor.utils.gdbus import DBus
+
+
+@pytest.mark.asyncio
+async def test_interface_update_payload(network_interface):
+    """Test interface update payload."""
+    assert (
+        interface_update_payload(network_interface, **{"method": "auto"})
+        == """{
+                    'connection':
+                        {
+                            'id': <'Wired connection 1'>,
+                            'type': <'802-3-ethernet'>
+                        },
+                    'ipv4':
+                        {
+                            'method': <'auto'>
+                        }
+                }"""
+    )
+
+    assert (
+        interface_update_payload(network_interface, **{})
+        == """{
+                    'connection':
+                        {
+                            'id': <'Wired connection 1'>,
+                            'type': <'802-3-ethernet'>
+                        },
+                    'ipv4':
+                        {
+                            'method': <'manual'>,
+                            'dns': <[uint32 16951488]>,
+                            'address-data': <[
+                                {
+                                    'address': <'192.168.2.148'>,
+                                    'prefix': <uint32 24>
+                                }]>,
+                            'gateway': <'192.168.2.1'>
+                                }
+                }"""
+    )
+
+    data = interface_update_payload(network_interface, **{"address": "1.1.1.1"})
+    assert DBus.parse_gvariant(data)["ipv4"]["address-data"][0]["address"] == "1.1.1.1"