From 312922ce9b46d2196ed4914010b6d3d14df46ffa Mon Sep 17 00:00:00 2001 From: giles knap Date: Mon, 29 Dec 2025 21:38:33 +0000 Subject: [PATCH] Improve defaults and environment management update config.py to supply all the defaults and environment variable overrides update the Unit description for the client service so that it overrides the defaults for client socket and configuration file location. --- docs/how-to/client_service.md | 18 ++++++++++- src/usb_remote/__main__.py | 19 +++-------- src/usb_remote/client_api.py | 29 ++++++----------- src/usb_remote/config.py | 61 ++++++++++++++++++++++++----------- src/usb_remote/server.py | 6 ++-- src/usb_remote/service.py | 7 ++-- src/usb_remote/utility.py | 9 +++--- tests/conftest_system.py | 3 -- tests/test_config.py | 47 +++++++++++---------------- tests/test_system_cli.py | 2 +- 10 files changed, 106 insertions(+), 95 deletions(-) diff --git a/docs/how-to/client_service.md b/docs/how-to/client_service.md index 45c34c6..3a3ca1c 100644 --- a/docs/how-to/client_service.md +++ b/docs/how-to/client_service.md @@ -45,10 +45,26 @@ For speed and ease of use, you can install the client service using the `uv` too ```bash sudo -s # uv (installed by root) requires the root profile so use sudo -s uvx usb-remote install-service --service-type client - systemctl enable --now usb-remote-client.service exit ``` +3. Configure the client service to let it know which usb-remote servers to connect to. Create the configuration directory and file: + + ```bash + sudo mkdir -p /etc/usb-remote-client + sudo vim /etc/usb-remote-client/usb-remote.config + ``` + + Add your configuration settings as needed (see [Client Configuration File](../reference/config.md) for details). + +4. Enable and start the service: + + ```bash + sudo systemctl enable --now usb-remote-client.service + ``` + +```bash + Check service status: ```bash diff --git a/src/usb_remote/__main__.py b/src/usb_remote/__main__.py index 0dbd4dd..d093e3c 100644 --- a/src/usb_remote/__main__.py +++ b/src/usb_remote/__main__.py @@ -13,7 +13,7 @@ from .client import attach_device, detach_device, find_device, list_devices from .client_service import ClientService from .config import ( - DEFAULT_CONFIG_PATH, + Defaults, discover_config_path, get_config, save_servers, @@ -327,22 +327,13 @@ def config_show() -> None: if config_path is None: typer.echo("No configuration file found.") - typer.echo(f"Default location: {DEFAULT_CONFIG_PATH}") + typer.echo(f"Default location: {Defaults.CONFIG_PATH}") typer.echo("\nDefault configuration:") else: typer.echo(f"Configuration file: {config_path}") typer.echo() - config = get_config() - - typer.echo(f"Servers ({len(config.servers)}):") - if config.servers: - for server in config.servers: - typer.echo(f" - {server}") - else: - typer.echo(" (none)") - - typer.echo(f"\nTimeout: {config.timeout}s") + typer.echo(get_config()) @config_app.command(name="add-server") @@ -359,7 +350,7 @@ def config_add_server( config.servers.append(server) save_servers(config.servers) - config_path = discover_config_path() or DEFAULT_CONFIG_PATH + config_path = discover_config_path() or Defaults.CONFIG_PATH typer.echo(f"Added server '{server}' to {config_path}") @@ -398,7 +389,7 @@ def config_set_timeout( config.timeout = timeout config.to_file() - config_path = discover_config_path() or DEFAULT_CONFIG_PATH + config_path = discover_config_path() or Defaults.CONFIG_PATH typer.echo(f"Set timeout to {timeout}s in {config_path}") diff --git a/src/usb_remote/client_api.py b/src/usb_remote/client_api.py index e4e95f0..6a7e440 100644 --- a/src/usb_remote/client_api.py +++ b/src/usb_remote/client_api.py @@ -1,37 +1,26 @@ """Pydantic models for client service socket communication.""" import os -from pathlib import Path from typing import Literal from pydantic import BaseModel, ConfigDict -from .usbdevice import UsbDevice +from usb_remote.config import Defaults, Environment -CLIENT_SOCKET_PATH = "/tmp/usb-remote-client.sock" -CLIENT_SOCKET_PATH_SYSTEMD = "/run/usb-remote-client/usb-remote-client.sock" +from .usbdevice import UsbDevice def get_client_socket_path() -> str: - """Get the appropriate socket path based on execution context. + """Get the appropriate socket path based on environment variable or default. Returns: - /run/usb-remote-client/usb-remote-client.sock when running as systemd service, - /tmp/usb-remote-client.sock otherwise. + Path to the client socket file. """ - # Detect systemd service by checking for INVOCATION_ID environment variable - # and verifying the runtime directory exists (created by systemd's RuntimeDirectory) - if os.environ.get("INVOCATION_ID"): - socket_dir = Path(CLIENT_SOCKET_PATH_SYSTEMD).parent - # Check if the directory exists and is writable (systemd creates it) - if socket_dir.exists() and os.access(socket_dir, os.W_OK): - return CLIENT_SOCKET_PATH_SYSTEMD - else: - raise RuntimeError( - f"Expected systemd runtime directory {socket_dir}" - f" does not exist or is not writable." - ) - return CLIENT_SOCKET_PATH + # prefer the environment variable if set but fall back to default socket path + socket_path = os.environ.get( + Environment.USB_REMOTE_CLIENT_SOCKET, Defaults.CLIENT_SOCKET + ) + return socket_path class StrictBaseModel(BaseModel): diff --git a/src/usb_remote/config.py b/src/usb_remote/config.py index 0f69103..d131bac 100644 --- a/src/usb_remote/config.py +++ b/src/usb_remote/config.py @@ -2,6 +2,7 @@ import logging import os +from enum import StrEnum from pathlib import Path import yaml @@ -9,11 +10,22 @@ logger = logging.getLogger(__name__) -DEFAULT_CONFIG_PATH = Path.home() / ".config" / "usb-remote" / "usb-remote.config" -SYSTEMD_CONFIG_PATH = Path("/etc/usb-remote-client/usb-remote.config") -DEFAULT_TIMEOUT = 5.0 -SERVER_PORT = 5055 +class Defaults: + """Default configuration values.""" + + CLIENT_SOCKET = "/tmp/usb-remote-client.sock" + CONFIG_PATH = Path.home() / ".config" / "usb-remote" / "usb-remote.config" + SERVER_PORT = 5055 + TIMEOUT = 2.0 + + +class Environment(StrEnum): + """Environment Variables that may override Defaults above.""" + + USB_REMOTE_CLIENT_SOCKET = "USB_REMOTE_CLIENT_SOCKET" + USB_REMOTE_CONFIG_PATH = "USB_REMOTE_CONFIG_PATH" + USB_REMOTE_SERVER_PORT = "USB_REMOTE_SERVER_PORT" class UsbRemoteConfig(BaseModel): @@ -21,10 +33,24 @@ class UsbRemoteConfig(BaseModel): servers: list[str] = Field(default_factory=list) server_ranges: list[str] = Field(default_factory=list) - timeout: float = Field(default=DEFAULT_TIMEOUT, gt=0) - server_port: int = Field(default=SERVER_PORT) + timeout: float = Field(default=Defaults.TIMEOUT, gt=0) + server_port: int = Field(default=Defaults.SERVER_PORT) model_config = ConfigDict(extra="forbid") + def __str__(self) -> str: + def do_list_format(s: list[str]) -> str: + return " none" if not s else "\n".join(f" - {s}" for s in s) + + return ( + f"UsbRemoteConfig:\n" + f" servers:\n" + f"{do_list_format(self.servers)}\n" + f" server_ranges:\n" + f"{do_list_format(self.server_ranges)}\n" + f" timeout={self.timeout}\n" + f" server_port={self.server_port}" + ) + @classmethod def from_file(cls, config_path: Path) -> "UsbRemoteConfig": """ @@ -61,7 +87,7 @@ def to_file(self) -> None: Args: config_path: Path to the config file. """ - config_path = discover_config_path() or DEFAULT_CONFIG_PATH + config_path = discover_config_path() or Defaults.CONFIG_PATH # Create directory if it doesn't exist config_path.parent.mkdir(parents=True, exist_ok=True) @@ -90,30 +116,27 @@ def discover_config_path() -> Path | None: Path to config file if found, None otherwise. """ # 1. Check environment variable - env_config = os.environ.get("USB_REMOTE_CONFIG") + env_config = os.environ.get(Environment.USB_REMOTE_CONFIG_PATH) if env_config: env_path = Path(env_config).expanduser() if env_path.exists(): - logger.debug(f"Using config from USB_REMOTE_CONFIG: {env_path}") + logger.debug(f"Using config from USB_REMOTE_CONFIG_PATH: {env_path}") return env_path else: - logger.warning(f"USB_REMOTE_CONFIG points to non-existent file: {env_path}") - - # 2. Check systemd config (when running as systemd service) - if os.environ.get("INVOCATION_ID") and SYSTEMD_CONFIG_PATH.exists(): - logger.debug(f"Using systemd config: {SYSTEMD_CONFIG_PATH}") - return SYSTEMD_CONFIG_PATH + logger.warning( + f"USB_REMOTE_CONFIG_PATH points to non-existent file: {env_path}" + ) - # 3. Check local directory + # 2. Check local directory local_config = Path.cwd() / ".usb-remote.config" if local_config.exists(): logger.debug(f"Using local config: {local_config}") return local_config # 3. Check default location - if DEFAULT_CONFIG_PATH.exists(): - logger.debug(f"Using default config: {DEFAULT_CONFIG_PATH}") - return DEFAULT_CONFIG_PATH + if Defaults.CONFIG_PATH.exists(): + logger.debug(f"Using default config: {Defaults.CONFIG_PATH}") + return Defaults.CONFIG_PATH logger.debug("No config file found") return None diff --git a/src/usb_remote/server.py b/src/usb_remote/server.py index f991a9b..e0c348a 100644 --- a/src/usb_remote/server.py +++ b/src/usb_remote/server.py @@ -16,7 +16,7 @@ multiple_matches_response, not_found_response, ) -from .config import SERVER_PORT +from .config import Defaults, Environment from .usbdevice import ( DeviceNotFoundError, MultipleDevicesError, @@ -34,7 +34,9 @@ def __init__(self, host: str = "0.0.0.0", port: int | None = None): self.host = host # Allow server port to be overridden via environment variable if port is None: - port = int(os.environ.get("USB_REMOTE_SERVER_PORT", SERVER_PORT)) + port = int( + os.environ.get(Environment.USB_REMOTE_SERVER_PORT, Defaults.SERVER_PORT) + ) self.port = port self.server_socket = None self.running = False diff --git a/src/usb_remote/service.py b/src/usb_remote/service.py index a64870b..54715b9 100644 --- a/src/usb_remote/service.py +++ b/src/usb_remote/service.py @@ -37,16 +37,19 @@ [Service] Type=simple User={user} +# TODO : Change to an appropriate group if we need access from non-root users +# Group=root WorkingDirectory={working_dir} ExecStart={executable} -m usb_remote client-service Restart=on-failure RestartSec=5s RuntimeDirectory=usb-remote-client RuntimeDirectoryMode=0755 -# TODO : Change to an appropriate group if we need access from non-root users -RuntimeDirectoryGroup=root ConfigurationDirectory=usb-remote-client ConfigurationDirectoryMode=0755 +Environment="USB_REMOTE_CONFIG_PATH=/etc/usb-remote-client/usb-remote.config" +Environment="USB_REMOTE_CLIENT_SOCKET=/run/usb-remote-client/usb-remote-client.sock" + # Security hardening NoNewPrivileges=true diff --git a/src/usb_remote/utility.py b/src/usb_remote/utility.py index 8b2a2a1..6780d78 100644 --- a/src/usb_remote/utility.py +++ b/src/usb_remote/utility.py @@ -6,7 +6,7 @@ import socket import subprocess -from usb_remote.config import SERVER_PORT, get_server_ranges, get_servers +from usb_remote.config import get_server_port, get_server_ranges, get_servers logger = logging.getLogger(__name__) @@ -72,11 +72,12 @@ def _scan_ip_range(range_spec: str) -> list[str]: for current_int in range(int(start_ip), int(end_ip) + 1): current_ip = ipaddress.ip_address(current_int) ip_str = str(current_ip) - if _is_port_open(ip_str, SERVER_PORT): - logger.info(f"Found server at {ip_str}:{SERVER_PORT}") + port = get_server_port() + if _is_port_open(ip_str, port): + logger.info(f"Found server at {ip_str}:{port}") responsive_servers.append(ip_str) else: - logger.debug(f"No response from {ip_str}:{SERVER_PORT}") + logger.debug(f"No response from {ip_str}:{port}") current_int += 1 except ValueError as e: diff --git a/tests/conftest_system.py b/tests/conftest_system.py index ca192d1..9ce5fb8 100644 --- a/tests/conftest_system.py +++ b/tests/conftest_system.py @@ -16,9 +16,6 @@ import pytest -# Unset INVOCATION_ID at module level to prevent systemd socket detection -_original_invocation_id = os.environ.pop("INVOCATION_ID", None) - @pytest.fixture(scope="session") def mock_subprocess_run(): diff --git a/tests/test_config.py b/tests/test_config.py index 36e555e..0c44bb4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -7,7 +7,7 @@ import pytest from usb_remote.config import ( - DEFAULT_TIMEOUT, + Defaults, UsbRemoteConfig, discover_config_path, get_config, @@ -42,7 +42,7 @@ def test_default_values(self): """Test that default values are set correctly.""" config = UsbRemoteConfig() assert config.servers == [] - assert config.timeout == DEFAULT_TIMEOUT + assert config.timeout == Defaults.TIMEOUT def test_custom_values(self): """Test setting custom values.""" @@ -76,7 +76,7 @@ def test_from_file_empty(self, temp_config_file): config = UsbRemoteConfig.from_file(temp_config_file) assert config.servers == [] - assert config.timeout == DEFAULT_TIMEOUT + assert config.timeout == Defaults.TIMEOUT def test_from_file_not_found(self, tmp_path): """Test loading from a non-existent file.""" @@ -84,7 +84,7 @@ def test_from_file_not_found(self, tmp_path): config = UsbRemoteConfig.from_file(nonexistent) assert config.servers == [] - assert config.timeout == DEFAULT_TIMEOUT + assert config.timeout == Defaults.TIMEOUT def test_from_file_invalid_yaml(self, temp_config_file): """Test loading from a file with invalid YAML.""" @@ -93,7 +93,7 @@ def test_from_file_invalid_yaml(self, temp_config_file): # Should return defaults on error assert config.servers == [] - assert config.timeout == DEFAULT_TIMEOUT + assert config.timeout == Defaults.TIMEOUT def test_to_file(self, temp_config_file): """Test saving config to file.""" @@ -133,25 +133,25 @@ class TestDiscoverConfigPath: """Test config file discovery logic.""" def test_environment_variable_priority(self, temp_config_file): - """Test that USB_REMOTE_CONFIG env var takes priority.""" + """Test that USB_REMOTE_CONFIG_PATH env var takes priority.""" temp_config_file.write_text("servers: []") - with patch.dict(os.environ, {"USB_REMOTE_CONFIG": str(temp_config_file)}): + with patch.dict(os.environ, {"USB_REMOTE_CONFIG_PATH": str(temp_config_file)}): result = discover_config_path() assert result == temp_config_file def test_environment_variable_nonexistent(self, tmp_path): - """Test that nonexistent USB_REMOTE_CONFIG is handled.""" + """Test that nonexistent USB_REMOTE_CONFIG_PATH is handled.""" nonexistent = tmp_path / "nonexistent.config" - with patch.dict(os.environ, {"USB_REMOTE_CONFIG": str(nonexistent)}): + with patch.dict(os.environ, {"USB_REMOTE_CONFIG_PATH": str(nonexistent)}): with patch("usb_remote.config.Path.cwd", return_value=tmp_path): with patch( - "usb_remote.config.DEFAULT_CONFIG_PATH", tmp_path / "default" + "usb_remote.config.Defaults.CONFIG_PATH", tmp_path / "default" ): with patch( - "usb_remote.config.SYSTEMD_CONFIG_PATH", + "usb_remote.config.Defaults.CONFIG_PATH", tmp_path / "systemd", ): result = discover_config_path() @@ -166,10 +166,7 @@ def test_local_directory_priority(self, tmp_path): with patch.dict(os.environ, {}, clear=True): with patch.object(Path, "cwd", return_value=tmp_path): - with patch( - "usb_remote.config.SYSTEMD_CONFIG_PATH", tmp_path / "systemd" - ): - result = discover_config_path() + result = discover_config_path() assert result == local_config @@ -181,12 +178,8 @@ def test_default_location(self, tmp_path): with patch.dict(os.environ, {}, clear=True): with patch.object(Path, "cwd", return_value=tmp_path): - with patch("usb_remote.config.DEFAULT_CONFIG_PATH", default_config): - with patch( - "usb_remote.config.SYSTEMD_CONFIG_PATH", - tmp_path / "systemd", - ): - result = discover_config_path() + with patch("usb_remote.config.Defaults.CONFIG_PATH", default_config): + result = discover_config_path() assert result == default_config @@ -195,13 +188,9 @@ def test_no_config_found(self, tmp_path): with patch.dict(os.environ, {}, clear=True): with patch.object(Path, "cwd", return_value=tmp_path): with patch( - "usb_remote.config.DEFAULT_CONFIG_PATH", tmp_path / "nonexistent" + "usb_remote.config.Defaults.CONFIG_PATH", tmp_path / "nonexistent" ): - with patch( - "usb_remote.config.SYSTEMD_CONFIG_PATH", - tmp_path / "systemd", - ): - result = discover_config_path() + result = discover_config_path() assert result is None @@ -216,7 +205,7 @@ def test_get_config_with_defaults(self): assert isinstance(config, UsbRemoteConfig) assert config.servers == [] - assert config.timeout == DEFAULT_TIMEOUT + assert config.timeout == Defaults.TIMEOUT def test_get_config_from_file(self, temp_config_file, sample_config_content): """Test getting config from a file.""" @@ -259,7 +248,7 @@ def test_get_timeout_default(self): with patch("usb_remote.config.get_config", return_value=UsbRemoteConfig()): timeout = get_timeout() - assert timeout == DEFAULT_TIMEOUT + assert timeout == Defaults.TIMEOUT def test_get_timeout_custom(self): """Test getting custom timeout.""" diff --git a/tests/test_system_cli.py b/tests/test_system_cli.py index 450c2dd..824964d 100644 --- a/tests/test_system_cli.py +++ b/tests/test_system_cli.py @@ -179,7 +179,7 @@ def test_config_show_command(self, mock_subprocess_run, tmp_path): assert result.exit_code == 0, f"Command failed: {result.stdout}" assert "192.168.1.100" in result.stdout assert "server2.local" in result.stdout - assert "5.0s" in result.stdout + assert "5.0" in result.stdout def test_config_show_no_config(self, mock_subprocess_run): """Test config show command with no config file."""