Accomodate more interfaces in network status (#1902)

Resolves tiny-pilot/tinypilot-pro#1500

This change introduces a new process for displaying the network status
of different interfaces. Instead of hardcoding the interface names
(`eth0` and `wlan0`), we instead query the device for a list of network
interface names backed by hardware before determining their status.

This change also adjusts the API's JSON response at `/network/status` to
a list of of network interface statuses with a new `name` field.

Additionally, this also changes the user-facing naming of Ethernet
interfaces to LAN. i.e., instead of `Ethernet` a user will see `LAN1`.

<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1902"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

Author: db39

app/api.py

@@ -207,55 +207,59 @@ def network_status():
     """Returns the current status of the available network interfaces.
 
     Returns:
-        On success, a JSON data structure with the following properties:
-        ethernet: object
-        wifi: object
-        The object contains the following fields:
-        isConnected: bool
-        ipAddress: string or null
-        macAddress: string or null
+        On success, a JSON data structure with the following:
+        interfaces: array of objects, where each object represents a network
+            interface with the following properties:
+            name: string
+            isConnected: boolean
+            ipAddress: string or null
+            macAddress: string or null
 
         Example:
         {
-            "ethernet": {
-                "isConnected": true,
-                "ipAddress": "192.168.2.41",
-                "macAddress": "e4-5f-01-98-65-03"
-            },
-            "wifi": {
-                "isConnected": false,
-                "ipAddress": null,
-                "macAddress": null
-            }
+            "interfaces": [
+                {
+                    "name": "eth0",
+                    "isConnected": true,
+                    "ipAddress": "192.168.2.41",
+                    "macAddress": "e4-5f-01-98-65-03"
+                },
+                {
+                    "name": "wlan0",
+                    "isConnected": false,
+                    "ipAddress": null,
+                    "macAddress": null
+                }
+            ]
         }
     """
     # In dev mode, return dummy data because attempting to read the actual
     # settings will fail in most non-Raspberry Pi OS environments.
     if flask.current_app.debug:
         return json_response.success({
-            'ethernet': {
-                'isConnected': True,
-                'ipAddress': '192.168.2.8',
-                'macAddress': '00-b0-d0-63-c2-26',
-            },
-            'wifi': {
-                'isConnected': False,
-                'ipAddress': None,
-                'macAddress': None,
-            },
+            'interfaces': [
+                {
+                    'name': 'eth0',
+                    'isConnected': True,
+                    'ipAddress': '192.168.2.41',
+                    'macAddress': 'e4-5f-01-98-65-03',
+                },
+                {
+                    'name': 'wlan0',
+                    'isConnected': False,
+                    'ipAddress': None,
+                    'macAddress': None,
+                },
+            ],
         })
-    ethernet, wifi = network.determine_network_status()
+    network_interfaces = network.determine_network_status()
     return json_response.success({
-        'ethernet': {
-            'isConnected': ethernet.is_connected,
-            'ipAddress': ethernet.ip_address,
-            'macAddress': ethernet.mac_address,
-        },
-        'wifi': {
-            'isConnected': wifi.is_connected,
-            'ipAddress': wifi.ip_address,
-            'macAddress': wifi.mac_address,
-        },
+        'interfaces': [{
+            'name': interface.name,
+            'isConnected': interface.is_connected,
+            'ipAddress': interface.ip_address,
+            'macAddress': interface.mac_address,
+        } for interface in network_interfaces]
     })
 
 

app/network.py

@@ -3,11 +3,13 @@
 import logging
 import re
 import subprocess
+from pathlib import Path
 
 logger = logging.getLogger(__name__)
 
 _WIFI_COUNTRY_PATTERN = re.compile(r'^\s*country=(.+)$')
 _WIFI_SSID_PATTERN = re.compile(r'^\s*ssid="(.+)"$')
+_INTERFACES_DIR = '/sys/class/net'
 
 
 class Error(Exception):
@@ -20,6 +22,7 @@ class NetworkError(Error):
 
 @dataclasses.dataclass
 class InterfaceStatus:
+    name: str
     is_connected: bool
     ip_address: str  # May be `None` if interface is disabled.
     mac_address: str  # May be `None` if interface is disabled.
@@ -32,13 +35,42 @@ class WifiSettings:
     psk: str  # Optional.
 
 
+def get_network_interfaces():
+    """Get a list of physical network interface names.
+
+    Excludes loopback and virtual interfaces. A device is considered "physical"
+    if /sys/class/net/<ifname>/device exists (i.e., it’s backed by hardware).
+
+    Returns:
+        A list of interface names for all available physical network interfaces.
+    """
+    sys_net_path = Path(_INTERFACES_DIR)
+    if not sys_net_path.is_dir():
+        logger.debug('%s is not available', str(_INTERFACES_DIR))
+        return []
+
+    interface_names = []
+    for iface_path in sys_net_path.iterdir():
+        # We know we don't want the loopback interface.
+        if iface_path.name == 'lo':
+            continue
+        # If /sys/class/net/<ifname>/device exists, the interface appears
+        # to be hardware.
+        if (iface_path / 'device').exists():
+            interface_names.append(iface_path.name)
+
+    return sorted(interface_names)
+
+
 def determine_network_status():
     """Checks the connectivity of the network interfaces.
 
     Returns:
-        A tuple of InterfaceStatus objects for the Ethernet and WiFi interface.
+        A list of InterfaceStatus objects for all available Ethernet and Wi-Fi
+        network interfaces.
     """
-    return inspect_interface('eth0'), inspect_interface('wlan0')
+    interfaces = get_network_interfaces()
+    return [inspect_interface(iface) for iface in interfaces]
 
 
 def inspect_interface(interface_name):
@@ -66,7 +98,7 @@ def inspect_interface(interface_name):
     Returns:
         InterfaceStatus object
     """
-    status = InterfaceStatus(False, None, None)
+    status = InterfaceStatus(interface_name, False, None, None)
 
     try:
         ip_cmd_out_raw = subprocess.check_output([

app/network_test.py

@@ -1,5 +1,7 @@
 import subprocess
+import tempfile
 import unittest
+from pathlib import Path
 from unittest import mock
 
 import network
@@ -13,23 +15,23 @@ class InspectInterfaceTest(unittest.TestCase):
     def test_treats_empty_response_as_inactive_interface(self, mock_cmd):
         mock_cmd.return_value = ''
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
 
     @mock.patch.object(subprocess, 'check_output')
     def test_treats_empty_array_as_inactive_interface(self, mock_cmd):
         mock_cmd.return_value = '[]'
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
 
     @mock.patch.object(subprocess, 'check_output')
     def test_treats_emtpy_object_as_inactive_interface(self, mock_cmd):
         mock_cmd.return_value = '[{}]'
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
 
@@ -38,7 +40,7 @@ def test_disregards_command_failure(self, mock_cmd):
         mock_cmd.side_effect = mock.Mock(
             side_effect=subprocess.CalledProcessError(returncode=1, cmd='ip'))
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
 
@@ -48,7 +50,7 @@ def test_parses_operstate_down_as_not_connected(self, mock_cmd):
             [{"operstate":"DOWN"}]
         """
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
 
@@ -58,7 +60,7 @@ def test_parses_operstate_up_as_connected(self, mock_cmd):
             [{"operstate":"UP"}]
         """
         self.assertEqual(
-            network.InterfaceStatus(True, None, None),
+            network.InterfaceStatus('eth0', True, None, None),
             network.inspect_interface('eth0'),
         )
 
@@ -68,7 +70,7 @@ def test_parses_mac_address(self, mock_cmd):
             [{"address":"00-b0-d0-63-c2-26"}]
         """
         self.assertEqual(
-            network.InterfaceStatus(False, None, '00-b0-d0-63-c2-26'),
+            network.InterfaceStatus('eth0', False, None, '00-b0-d0-63-c2-26'),
             network.inspect_interface('eth0'),
         )
 
@@ -78,7 +80,7 @@ def test_normalizes_mac_address_to_use_dashes(self, mock_cmd):
             [{"address":"00:b0:d0:63:c2:26"}]
         """
         self.assertEqual(
-            network.InterfaceStatus(False, None, '00-b0-d0-63-c2-26'),
+            network.InterfaceStatus('eth0', False, None, '00-b0-d0-63-c2-26'),
             network.inspect_interface('eth0'),
         )
 
@@ -88,7 +90,7 @@ def test_parses_ip_address(self, mock_cmd):
             [{"addr_info":[{"family":"inet","local":"192.168.2.5"}]}]
         """
         self.assertEqual(
-            network.InterfaceStatus(False, '192.168.2.5', None),
+            network.InterfaceStatus('eth0', False, '192.168.2.5', None),
             network.inspect_interface('eth0'),
         )
 
@@ -98,7 +100,7 @@ def test_disregards_other_families_such_as_ipv6(self, mock_cmd):
             [{"addr_info":[{"family":"inet6","local":"::ffff:c0a8:205"}]}]
         """
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
 
@@ -112,14 +114,58 @@ def test_parses_all_data(self, mock_cmd):
             }]
         """
         self.assertEqual(
-            network.InterfaceStatus(True, '192.168.2.5', '00-b0-d0-63-c2-26'),
+            network.InterfaceStatus('eth0', True, '192.168.2.5',
+                                    '00-b0-d0-63-c2-26'),
             network.inspect_interface('eth0'),
         )
 
     @mock.patch.object(subprocess, 'check_output')
     def test_disregards_invalid_json(self, mock_cmd):
         mock_cmd.return_value = '[{"address'
         self.assertEqual(
-            network.InterfaceStatus(False, None, None),
+            network.InterfaceStatus('eth0', False, None, None),
             network.inspect_interface('eth0'),
         )
+
+
+class GetNetworkInterfacesTest(unittest.TestCase):
+
+    def test_returns_empty_list_when_path_is_not_a_directory(self):
+        with tempfile.NamedTemporaryFile() as mock_file:
+            with mock.patch.object(network, '_INTERFACES_DIR', mock_file.name):
+                self.assertEqual([], network.get_network_interfaces())
+
+    def test_returns_empty_list_when_path_does_not_exist(self):
+        with tempfile.TemporaryDirectory() as mock_dir:
+            with mock.patch.object(network, '_INTERFACES_DIR',
+                                   f'{mock_dir}/path/does/not/exist'):
+                self.assertEqual([], network.get_network_interfaces())
+
+    def test_returns_empty_list_when_directory_has_no_interfaces(self):
+        with tempfile.TemporaryDirectory() as mock_dir:
+            with mock.patch.object(network, '_INTERFACES_DIR', mock_dir):
+                self.assertEqual([], network.get_network_interfaces())
+
+    def test_excludes_loopback_and_virtual_interfaces(self):
+        with tempfile.TemporaryDirectory() as mock_dir:
+            mock_net_interfaces_dir = Path(mock_dir)
+            # Physical interfaces (with 'device').
+            (mock_net_interfaces_dir / 'eth0' / 'device').mkdir(parents=True)
+            (mock_net_interfaces_dir / 'wlan0' / 'device').mkdir(parents=True)
+            # Loopback (no 'device' in the path).
+            (mock_net_interfaces_dir / 'lo').mkdir()
+            # Some virtual interface (no 'device' in the path).
+            (mock_net_interfaces_dir / 'veth0').mkdir()
+            with mock.patch.object(network, '_INTERFACES_DIR', mock_dir):
+                self.assertEqual(['eth0', 'wlan0'],
+                                 network.get_network_interfaces())
+
+    def test_returns_sorted_interface_names(self):
+        with tempfile.TemporaryDirectory() as mock_dir:
+            mock_net_interfaces_dir = Path(mock_dir)
+            # Create in unsorted order.
+            (mock_net_interfaces_dir / 'wlan0' / 'device').mkdir(parents=True)
+            (mock_net_interfaces_dir / 'eth0' / 'device').mkdir(parents=True)
+            with mock.patch.object(network, '_INTERFACES_DIR', mock_dir):
+                self.assertEqual(['eth0', 'wlan0'],
+                                 network.get_network_interfaces())

app/static/js/controllers.js

@@ -203,21 +203,11 @@ export async function getNetworkStatus() {
   })
     .then(processJsonResponse)
     .then((response) => {
-      ["ethernet", "wifi"].forEach((field) => {
-        // eslint-disable-next-line no-prototype-builtins
-        if (!response.hasOwnProperty(field)) {
-          throw new ControllerError(`Missing expected ${field} field`);
-        }
-        ["isConnected", "ipAddress", "macAddress"].forEach((property) => {
-          // eslint-disable-next-line no-prototype-builtins
-          if (!response[field].hasOwnProperty(property)) {
-            throw new ControllerError(
-              `Missing expected ${field}.${property} field`
-            );
-          }
-        });
-      });
-      return response;
+      // eslint-disable-next-line no-prototype-builtins
+      if (!response.hasOwnProperty("interfaces")) {
+        throw new ControllerError("Missing expected interfaces field");
+      }
+      return response.interfaces;
     });
 }