fix(gateway): order the system unit after the target user's manager
run_gateway() adopts the user bus once at boot; the generated system unit had no ordering against user@<uid>.service, so after a reboot the two race and the adoption can miss until the next gateway restart. Emit After=/Wants= user@<uid>.service for the unit's User= (uid now returned by _system_service_identity, which already resolved the account). Existing system units are flagged outdated once and refreshed on the next install/restart. Refs #104893.
This commit is contained in:
@@ -2343,7 +2343,7 @@ def _require_root_for_system_service(action: str) -> None:
|
||||
raise SystemScopeRequiresRootError(f"System gateway {action} requires root. Re-run with sudo.", action)
|
||||
|
||||
|
||||
def _system_service_identity(run_as_user: str | None = None) -> tuple[str, str, str]:
|
||||
def _system_service_identity(run_as_user: str | None = None) -> tuple[str, str, str, int]:
|
||||
import getpass
|
||||
import grp
|
||||
import pwd
|
||||
@@ -2364,7 +2364,7 @@ def _system_service_identity(run_as_user: str | None = None) -> tuple[str, str,
|
||||
user_info = pwd.getpwnam(username)
|
||||
except KeyError as e:
|
||||
raise ValueError(f"Unknown user: {username}") from e
|
||||
return username, grp.getgrgid(user_info.pw_gid).gr_name, user_info.pw_dir
|
||||
return username, grp.getgrgid(user_info.pw_gid).gr_name, user_info.pw_dir, user_info.pw_uid
|
||||
|
||||
|
||||
def _read_systemd_user_from_unit(unit_path: Path) -> str | None:
|
||||
@@ -2765,7 +2765,7 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
|
||||
restart_timeout = resolve_systemd_timeout_stop_sec(_get_restart_drain_timeout(), _get_cron_drain_timeout())
|
||||
|
||||
if system:
|
||||
username, group_name, home_dir = _system_service_identity(run_as_user)
|
||||
username, group_name, home_dir, uid = _system_service_identity(run_as_user)
|
||||
hermes_home = _hermes_home_for_target_user(home_dir)
|
||||
# Profile arg relative to the TARGET user's ~/.hermes when hermes_home lives under it.
|
||||
target_root = Path(home_dir) / ".hermes"
|
||||
@@ -2785,6 +2785,10 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
|
||||
path_entries = [e for e in _target_node_entries if e not in path_entries] + path_entries
|
||||
user_home = Path(home_dir)
|
||||
identity_lines = f"User={username}\nGroup={group_name}\n"
|
||||
# Restart-safe cron/Kanban workers cross `systemd-run --user`, which needs this user's manager;
|
||||
# without the ordering the gateway and user@<uid>.service race at boot and the one-shot bus
|
||||
# adoption in run_gateway() can miss (#104893).
|
||||
ordering_lines = f"After=user@{uid}.service\nWants=user@{uid}.service\n"
|
||||
env_lines = (
|
||||
f'Environment="HOME={home_dir}"\n'
|
||||
f'Environment="USER={username}"\n'
|
||||
@@ -2795,7 +2799,7 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
|
||||
hermes_home = str(get_hermes_home().resolve())
|
||||
profile_arg = _profile_arg(hermes_home)
|
||||
user_home = Path.home()
|
||||
identity_lines = env_lines = ""
|
||||
identity_lines = env_lines = ordering_lines = ""
|
||||
wanted_by = "default.target"
|
||||
|
||||
watchdog_seconds = _systemd_watchdog_seconds(hermes_home)
|
||||
@@ -2810,7 +2814,7 @@ def generate_systemd_unit(system: bool = False, run_as_user: str | None = None)
|
||||
Description={SERVICE_DESCRIPTION}
|
||||
After=network-online.target
|
||||
Wants=network-online.target
|
||||
StartLimitIntervalSec=0
|
||||
{ordering_lines}StartLimitIntervalSec=0
|
||||
|
||||
[Service]
|
||||
Type={systemd_type}
|
||||
|
||||
@@ -1259,7 +1259,7 @@ class TestSystemUnitHermesHome:
|
||||
monkeypatch.setattr(
|
||||
gateway_cli,
|
||||
"_system_service_identity",
|
||||
lambda run_as_user=None: ("alice", "alice", str(target_home)),
|
||||
lambda run_as_user=None: ("alice", "alice", str(target_home), 1001),
|
||||
)
|
||||
monkeypatch.setattr(gateway_cli, "get_hermes_home", lambda: root_hermes)
|
||||
monkeypatch.setattr(gateway_cli, "_build_service_path_dirs", lambda: [])
|
||||
@@ -1294,13 +1294,31 @@ class TestSystemUnitHermesHome:
|
||||
|
||||
assert entries == ["/opt/external-node/bin"]
|
||||
|
||||
def test_system_unit_orders_after_target_user_manager(self, monkeypatch, tmp_path):
|
||||
"""#104893: restart-safe workers need user@<uid>.service; the system unit must not race it at boot."""
|
||||
monkeypatch.setattr(Path, "home", staticmethod(lambda: Path("/root")))
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
monkeypatch.setattr(
|
||||
gateway_cli, "_system_service_identity",
|
||||
lambda run_as_user=None: ("alice", "alice", str(tmp_path), 1001),
|
||||
)
|
||||
monkeypatch.setattr(gateway_cli, "_build_service_path_dirs", lambda: [])
|
||||
|
||||
system_unit = gateway_cli.generate_systemd_unit(system=True, run_as_user="alice")
|
||||
user_unit = gateway_cli.generate_systemd_unit(system=False)
|
||||
|
||||
unit_section = system_unit.split("[Service]")[0]
|
||||
assert "After=user@1001.service" in unit_section
|
||||
assert "Wants=user@1001.service" in unit_section
|
||||
assert "user@" not in user_unit
|
||||
|
||||
def test_system_unit_uses_target_user_home_not_calling_user(self, monkeypatch):
|
||||
# Simulate sudo: Path.home() returns /root, target user is alice
|
||||
monkeypatch.setattr(Path, "home", staticmethod(lambda: Path("/root")))
|
||||
monkeypatch.delenv("HERMES_HOME", raising=False)
|
||||
monkeypatch.setattr(
|
||||
gateway_cli, "_system_service_identity",
|
||||
lambda run_as_user=None: ("alice", "alice", "/home/alice"),
|
||||
lambda run_as_user=None: ("alice", "alice", "/home/alice", 1001),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
gateway_cli, "_build_user_local_paths",
|
||||
@@ -1343,7 +1361,7 @@ class TestSystemUnitRefreshSyncsHermesHome:
|
||||
monkeypatch.setattr(
|
||||
gateway_cli,
|
||||
"_system_service_identity",
|
||||
lambda run_as_user=None: ("alice", "alice", str(alice_home)),
|
||||
lambda run_as_user=None: ("alice", "alice", str(alice_home), 1001),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
gateway_cli, "_build_user_local_paths", lambda home, existing: []
|
||||
@@ -1516,7 +1534,7 @@ class TestSystemServiceIdentityRootHandling:
|
||||
root_info = pwd.getpwnam("root")
|
||||
root_group = grp.getgrgid(root_info.pw_gid).gr_name
|
||||
|
||||
username, group, home = gateway_cli._system_service_identity(run_as_user="root")
|
||||
username, group, home, _uid = gateway_cli._system_service_identity(run_as_user="root")
|
||||
assert username == "root"
|
||||
assert home == root_info.pw_dir
|
||||
|
||||
@@ -1528,7 +1546,7 @@ class TestSystemServiceIdentityRootHandling:
|
||||
monkeypatch.setenv("LOGNAME", "nobody")
|
||||
|
||||
try:
|
||||
username, group, home = gateway_cli._system_service_identity(run_as_user=None)
|
||||
username, group, home, _uid = gateway_cli._system_service_identity(run_as_user=None)
|
||||
assert username == "nobody"
|
||||
except ValueError as e:
|
||||
# "nobody" might not exist on all systems
|
||||
@@ -1663,7 +1681,7 @@ class TestProfileArg:
|
||||
monkeypatch.setattr(
|
||||
gateway_cli,
|
||||
"_system_service_identity",
|
||||
lambda run_as_user=None: ("alice", "alice", str(target_home)),
|
||||
lambda run_as_user=None: ("alice", "alice", str(target_home), 1001),
|
||||
)
|
||||
|
||||
unit = gateway_cli.generate_systemd_unit(system=True, run_as_user="alice")
|
||||
@@ -1753,7 +1771,7 @@ class TestSystemUnitPathRemapping:
|
||||
monkeypatch.setattr(gateway_cli, "get_python_path", lambda: str(venv_bin / "python"))
|
||||
monkeypatch.setattr(
|
||||
gateway_cli, "_system_service_identity",
|
||||
lambda run_as_user=None: ("alice", "alice", target_home),
|
||||
lambda run_as_user=None: ("alice", "alice", target_home, 1001),
|
||||
)
|
||||
|
||||
unit = gateway_cli.generate_systemd_unit(system=True)
|
||||
|
||||
@@ -44,7 +44,7 @@ def test_system_unit_reads_watchdog_from_target_home(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
gateway_cli,
|
||||
"_system_service_identity",
|
||||
lambda _user: ("service", "service", str(tmp_path / "account")),
|
||||
lambda _user: ("service", "service", str(tmp_path / "account"), 1001),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
gateway_cli,
|
||||
|
||||
Reference in New Issue
Block a user