fix(gateway): migrate --multiplex resumes from live state, compensates the whole destructive phase, and refuses an unknown default principal

Three P1 findings from the review of #111062 (fixes #110850 remainder):

- interrupted-detection keyed off `plan.default.has_gateway`, which is true for an
  installed-but-dead unit; `systemd_install` writes the unit before the start that
  can still be killed, so an apply interrupted at default/start (or mid-restart of
  a stopped default) was reported as "already multiplexed" and never recovered.
  `interrupted` is now derived from the manifest vs the LIVE default's recorded
  served_profiles: flag on + manifest + not every migrated profile served = resume.

- the compensation `try` began after `_remove_secondary_gateways` and the flag
  write, so a later secondary's stop, a unit's daemon-reload or the config write
  failing left the first secondary removed with no rollback. The flag write,
  removals and default bring-up now all sit inside one boundary that rolls back
  through the manifest; `_preflight_apply` refuses failures knowable from the plan
  (root system unit without a recorded User=, unresolvable recorded user, an
  unwritable config.yaml) before any working gateway is stopped.

- `_guard_unix_user` blocked an unknown SECONDARY principal but accepted
  `default_uid is None`; a default system unit whose User= this host cannot
  resolve is the same boundary from the other side and now blocks the update
  hook (same known uid still folds, different known uid still refuses).
This commit is contained in:
teknium1
2026-09-15 23:07:49 -07:00
committed by Teknium
parent 9085ef967c
commit 7dde7a2424
4 changed files with 202 additions and 26 deletions
+70 -17
View File
@@ -88,8 +88,10 @@ class MigrationPlan:
profiles: list[ProfileGateway]
multiplex_flag_on: bool
live_served: Optional[list[str]] # served_profiles the live default gateway recorded, if any
# A manifest with the flag on and no live default gateway: an earlier apply died between flipping
# the flag and bringing the multiplexer up (#110850). Not "already multiplexed" — resumable.
# A manifest with the flag on and no LIVE default gateway: an earlier apply died between flipping
# the flag and the multiplexer confirming it is up (#110850). An installed unit is not proof of
# anything — `systemd_install` writes the unit before the start that can still fail or be killed.
# Not "already multiplexed" — resumable.
interrupted: bool = False
blockers: list[str] = field(default_factory=list)
notices: list[str] = field(default_factory=list)
@@ -449,7 +451,7 @@ def build_migration_plan() -> MigrationPlan:
multiplex_flag_on=_read_multiplex_flag(default_home),
live_served=recorded_served_profiles(default_home),
)
plan.interrupted = plan.multiplex_flag_on and not plan.default.has_gateway and _read_manifest(default_home) is not None
plan.interrupted = plan.multiplex_flag_on and _manifest_not_yet_served(_read_manifest(default_home), plan.live_served)
if len(plan.profiles) < 2:
plan.notices.append("Only one profile exists: nothing to multiplex.")
return plan
@@ -487,7 +489,7 @@ def format_plan(plan: MigrationPlan, *, dry_run: bool) -> list[str]:
return lines
if plan.interrupted:
lines.append(f" ↻ An earlier migration was interrupted before the default gateway came up "
f"(flag on, no gateway; manifest {plan.default_home / MANIFEST_NAME}); this run resumes it.")
f"(flag on, no live multiplexer; manifest {plan.default_home / MANIFEST_NAME}); this run resumes it.")
steps = []
for p in plan.standalone_secondaries:
what = " + ".join(x for x in (f"stop pid {p.pid}" if p.pid else "", f"uninstall {p.service_label()}" if p.services else "") if x)
@@ -615,6 +617,19 @@ def _read_manifest(default_home: Path) -> Optional[dict]:
return data if isinstance(data, dict) else None
def _manifest_not_yet_served(manifest: Optional[dict], live_served: Optional[list[str]]) -> bool:
"""The postcondition ``apply_migration`` waits for, re-derived from live state: a LIVE default that
recorded serving every profile the manifest migrated. Anything less — no live gateway, an
installed-but-dead unit (``systemd_install`` writes the unit before the start that can still fail),
a standalone default never restarted — is a half-applied migration, not "already multiplexed".
Profiles created after the migration are not in the manifest, so they cannot flag it as interrupted."""
if manifest is None:
return False
recs = [r for r in (manifest.get("default"), *(_manifest_secondaries(manifest) or [])) if isinstance(r, dict)]
migrated = {str(r.get("profile") or "default") for r in recs} | {"default"}
return not migrated <= set(live_served or [])
def _write_manifest(default_home: Path, data: dict) -> None:
from utils import atomic_json_write
@@ -691,14 +706,48 @@ def _remove_secondary_gateways(plan: MigrationPlan) -> None:
print(f" ✓ {p.name}: stopped standalone gateway (pid {p.pid})")
def _preflight_apply(plan: MigrationPlan, target: Optional[tuple[str, bool]], run_as_user: Optional[str]) -> Optional[str]:
"""A failure of the destructive phase that is knowable from the plan alone, refused BEFORE any
working per-profile gateway is stopped: rollback is the fallback for surprises, not the plan.
Mirrors the checks ``systemd_install``/``_service_call`` make on a system unit (root, resolvable
``User=``) and the config write's read-guard."""
from hermes_cli import gateway as gw
from hermes_cli.config import require_readable_config_before_write
try:
require_readable_config_before_write(plan.default_home / "config.yaml")
except Exception as exc:
return f"default: config.yaml cannot be updated ({exc})"
touches_system_unit = target == ("systemd", True) or any(p.has_system_unit for p in plan.standalone_secondaries)
if touches_system_unit:
try:
gw._require_root_for_system_service("migration")
except Exception as exc:
return str(exc)
if plan.default.service is None and target == ("systemd", True):
if run_as_user is None:
try:
gw._system_service_identity() # the #110850 refusal (implicit root), before anything is removed
except ValueError as exc:
return f"default: {exc}"
else:
import pwd
try:
pwd.getpwnam(run_as_user)
except KeyError:
return f"default: the recorded service user '{run_as_user}' does not exist on this host"
return None
def apply_migration(plan: MigrationPlan, *, served_wait: float = _SERVED_WAIT_SECONDS) -> bool:
"""Stop/uninstall every secondary gateway, flip the flag, bring up the multiplexer, verify.
"""Flip the flag, stop/uninstall every secondary gateway, bring up the multiplexer, verify.
Returns True when the multiplexer verifiably serves every profile.
Bringing the default up is the one step that can fail after the destructive ones (a system unit
that needs ``--run-as-user``, an unreachable user bus). It runs inside a rollback: on failure the
manifest written before the first destructive step restores the flag and every recorded per-profile
gateway (#110850), so the fleet never ends with the flag on and no gateway at all."""
Every step after the manifest write is fallible (a config write, a secondary's stop or its
unit's daemon-reload, a system unit that needs ``--run-as-user``, an unreachable user bus) and
runs inside ONE compensating boundary: on failure the manifest written before the first
destructive step restores the flag and every recorded per-profile gateway (#110850), so the
fleet never ends half-migrated. The flag goes on first so an apply killed anywhere after it is
resumable from the manifest (flag on + manifest + no live multiplexer = interrupted)."""
if plan.blocked:
_print(["✗ Migration refused:", *[f" • {b}" for b in plan.blockers]])
return False
@@ -707,18 +756,22 @@ def apply_migration(plan: MigrationPlan, *, served_wait: float = _SERVED_WAIT_SE
return True
target, run_as_user = plan.target_service_kind(), plan.target_run_as_user()
if plan.interrupted:
# An earlier apply removed the secondaries and flipped the flag but never brought the default
# up; the manifest is the only record of the units that existed. Finish from it, don't rewrite it.
# An earlier apply flipped the flag (and removed some or all secondaries) but the default never
# came up; the manifest is the only record of the units that existed. Finish from it, don't rewrite it.
manifest = _read_manifest(plan.default_home) or {}
target, run_as_user = _target_from_manifest(manifest)
print(f" ↻ resuming an interrupted migration recorded in {_manifest_path(plan.default_home)}")
elif _read_manifest(plan.default_home) is not None:
# Flag off + manifest present = a rollback that did not finish. Overwriting the manifest would
# discard the only record of the units that rollback still has to restore.
# Flag off + manifest present = a rollback (or an apply killed before its flag write) that did
# not finish. Overwriting the manifest would discard the only record of the units to restore.
_print([f"✗ A previous migration's manifest is still at {_manifest_path(plan.default_home)} (its rollback did not finish).",
" Finish it with: hermes gateway migrate --standalone (or delete the manifest to start over)"])
return False
else:
blocker = _preflight_apply(plan, target, run_as_user)
if blocker is not None:
_print(["✗ Migration refused before changing anything:", f" • {blocker}"])
return False
manifest = {
"version": 1, "migrated_at": time.strftime("%Y-%m-%dT%H:%M:%S%z"),
"flag_was": plan.multiplex_flag_on,
@@ -728,13 +781,13 @@ def apply_migration(plan: MigrationPlan, *, served_wait: float = _SERVED_WAIT_SE
# Recovery metadata must exist before the first destructive operation; the manifest never
# changes afterwards, so this is the only write it needs.
_write_manifest(plan.default_home, manifest)
_remove_secondary_gateways(plan)
_write_multiplex_flag(plan.default_home, True)
print(f" ✓ default: gateway.multiplex_profiles: true ({plan.default_home / 'config.yaml'})")
try:
_write_multiplex_flag(plan.default_home, True)
print(f" ✓ default: gateway.multiplex_profiles: true ({plan.default_home / 'config.yaml'})")
_remove_secondary_gateways(plan) # on resume: whatever an apply killed mid-removal left installed
print(f" ✓ {_restart_default(plan.default, target, plan.default_home, run_as_user=run_as_user)}")
except Exception as exc:
_print([f" ✗ default: could not bring up the multiplexed gateway ({exc})",
_print([f" ✗ migration failed ({exc})",
" ↩ Rolling back to per-profile gateways so no profile is left without one..."])
rolled_back = rollback_migration(plan.default_home)
if not rolled_back:
+7 -1
View File
@@ -105,6 +105,11 @@ def _guard_service_domain(plan: MigrationPlan, profile: ProfileGateway) -> Optio
def _guard_unix_user(plan: MigrationPlan, profile: ProfileGateway) -> Optional[str]:
default_uid = plan.default.uid
if default_uid is None and plan.default.has_system_unit:
# Consolidating INTO a principal this host cannot identify is the same unknown boundary
# from the other side: the default's system unit names an account NSS does not resolve.
return ("The default gateway runs a system unit whose User= cannot be resolved on this host: "
"an unknown service principal is not folded into automatically.")
if profile.uid is None and profile.has_system_unit:
# Unknown principal is not "same user": the unit names an account this host cannot resolve.
return (f"Profile '{profile.name}' runs a system unit whose User= cannot be resolved on this host: "
@@ -134,12 +139,13 @@ _AUTO_MIGRATION_GUARDS: tuple[Callable[[MigrationPlan, ProfileGateway], Optional
def auto_migration_blockers(plan: MigrationPlan) -> list[str]:
"""Every boundary a standalone secondary sits behind; empty when the fleet is one user, one service
domain, one profiles/ tree — the only shape ``hermes update`` may fold on its own."""
return [
findings = [
finding
for profile in plan.standalone_secondaries
for guard in _AUTO_MIGRATION_GUARDS
if (finding := guard(plan, profile)) is not None
]
return list(dict.fromkeys(findings)) # a default-side finding repeats per secondary
# --------------------------------------------------------------------------- opt-out
@@ -78,6 +78,8 @@ def fleet(tmp_path, monkeypatch):
monkeypatch.setattr(gm, "_service_op", _service_op)
monkeypatch.setattr(gm, "_stop_gateway_process", lambda home: state.pids.pop(_name(home), None))
monkeypatch.setattr(gm, "_host_supports_migration", lambda: None)
# Part of the faked service layer: the real check asks systemd's questions (root, NSS user).
monkeypatch.setattr(gm, "_preflight_apply", lambda plan, target, run_as_user: None)
state.root = root
return state
@@ -92,6 +94,9 @@ def _units(recorded) -> list:
return list(recorded) if isinstance(recorded, list) else [recorded]
_real_preflight = gm._preflight_apply
def _config_flag(root: Path):
import yaml
raw = yaml.safe_load((root / "config.yaml").read_text(encoding="utf-8")) or {}
@@ -564,3 +569,111 @@ def test_opt_out_reads_effective_config_managed_false_wins_and_string_false_is_f
assert auto_migration_opted_out(fleet.root) is True
gm.maybe_auto_migrate_after_update()
assert fleet.ops == [] and _config_flag(fleet.root) is None
@pytest.mark.parametrize("default_unit_preinstalled", [False, True])
def test_interruption_after_the_default_unit_exists_is_still_interrupted_not_already_multiplexed(
fleet, monkeypatch, capsys, default_unit_preinstalled,
):
"""``systemd_install`` writes the unit before the start that can be killed; an existing stopped
default unit can be interrupted mid-restart. Either way the flag is on and a default unit exists
but nothing serves anyone: that is an interrupted migration to resume, not a completed one. Only
a LIVE multiplexer that recorded ``served_profiles`` counts as already multiplexed."""
if default_unit_preinstalled:
fleet.services["default"] = ("systemd", False)
real_op = gm._service_op
def _killed_at_start(kind, system, verb, home, *, run_as_user=None):
if verb in ("start", "restart") and _name(home) == "default":
raise KeyboardInterrupt()
real_op(kind, system, verb, home, run_as_user=run_as_user)
with pytest.MonkeyPatch.context() as dying:
dying.setattr(gm, "_service_op", _killed_at_start)
with pytest.raises(KeyboardInterrupt):
gm.apply_migration(gm.build_migration_plan(), served_wait=0.1)
assert _config_flag(fleet.root) is True and fleet.services == {"default": ("systemd", False)}
assert (fleet.root / gm.MANIFEST_NAME).exists()
plan = gm.build_migration_plan()
assert plan.interrupted and not plan.already_multiplexed
fleet.ops.clear()
assert gm.apply_migration(plan, served_wait=5.0) is True
assert fleet.ops[-1] == ("default", "restart") and "serves 3 profiles" in capsys.readouterr().out
# Postcondition met: the next plan sees the live multiplexer and stops.
assert gm.build_migration_plan().already_multiplexed
@pytest.mark.parametrize("failing_op", [("ops", "stop"), ("coder", "uninstall"), ("default", "flag")])
def test_failure_anywhere_in_the_destructive_phase_restores_the_removed_secondaries(fleet, monkeypatch, capsys, failing_op):
"""The compensation boundary covers the whole destructive phase, not only the default bring-up:
a later secondary's stop, a unit unlink/daemon-reload, or the flag write failing after an earlier
secondary was removed must put that secondary back (the manifest alone is not a restored fleet)."""
name, verb = failing_op
real_op = gm._service_op
def _failing(kind, system, verb_, home, *, run_as_user=None):
if (_name(home), verb_) == (name, verb):
raise RuntimeError(f"{name} {verb} failed")
real_op(kind, system, verb_, home, run_as_user=run_as_user)
monkeypatch.setattr(gm, "_service_op", _failing)
if verb == "flag":
real_flag = gm._write_multiplex_flag
monkeypatch.setattr(gm, "_write_multiplex_flag",
lambda home, value: (_ for _ in ()).throw(OSError("read-only config")) if value else real_flag(home, value))
assert gm.apply_migration(gm.build_migration_plan(), served_wait=0.1) is False
out = capsys.readouterr().out
assert "Rolling back" in out and "Rolled back" in out
assert _config_flag(fleet.root) is not True
assert fleet.services == {"coder": ("systemd", False), "ops": ("systemd", False)}
assert not (fleet.root / gm.MANIFEST_NAME).exists()
assert gm.build_migration_plan().eligible_for_migration()
def test_known_bringup_refusal_is_rejected_before_any_secondary_is_touched(fleet, monkeypatch, capsys):
"""A system-unit fleet with no recorded User= run by root is the #110850 refusal: known from the plan,
so it is refused before a working gateway is stopped rather than discovered and rolled back."""
from hermes_cli import gateway as gw
fleet.services.update({"coder": ("systemd", True), "ops": ("systemd", True)})
monkeypatch.setattr(gm, "_systemd_service_user", lambda home, services: None)
monkeypatch.setattr(gm, "_preflight_apply", _real_preflight)
monkeypatch.setattr(gw, "_require_root_for_system_service", lambda action: None) # we are "root"
for var in ("SUDO_USER", "USER", "LOGNAME"):
monkeypatch.setenv(var, "root")
assert gm.apply_migration(gm.build_migration_plan(), served_wait=0.1) is False
out = capsys.readouterr().out
assert "before changing anything" in out and "--run-as-user root" in out
assert fleet.ops == [] and _config_flag(fleet.root) is None and not (fleet.root / gm.MANIFEST_NAME).exists()
def test_unknown_default_system_principal_blocks_the_update_hook(fleet, tmp_path, monkeypatch, capsys):
"""Mirror of the unknown-secondary case: the default's system unit names a User= this host cannot
resolve while both secondaries are known root system units. Folding INTO an unidentifiable
principal is the same boundary; known-same uid still folds, known-different still refuses."""
from hermes_cli import gateway as gw
from hermes_cli.gateway_migrate_guards import auto_migration_blockers, gateway_identity
unit_dir = tmp_path / "system"; unit_dir.mkdir()
monkeypatch.setattr(gw, "_SYSTEM_UNIT_DIR", unit_dir)
with gm._home_env(fleet.root):
gw.get_systemd_unit_path(system=True).write_text("[Service]\nUser=no-such-pr111062-user\n", encoding="utf-8")
for name in ("coder", "ops"):
with gm._home_env(fleet.root / "profiles" / name):
gw.get_systemd_unit_path(system=True).write_text("[Service]\nUser=root\n", encoding="utf-8")
fleet.services.update({"default": ("systemd", True), "coder": ("systemd", True), "ops": ("systemd", True)})
fleet.pids.clear() # stopped units everywhere: identity comes from User=, resolved for real
plan = gm.build_migration_plan()
assert plan.default.uid is None and {p.uid for p in plan.standalone_secondaries} == {0}
assert gateway_identity(fleet.root, None, [("systemd", True)])[0] is None
blockers = auto_migration_blockers(plan)
assert len(blockers) == 1 and "default gateway" in blockers[0] and "cannot be resolved" in blockers[0]
gm.maybe_auto_migrate_after_update()
out = capsys.readouterr().out
assert "cannot be resolved" in out and gm.MIGRATE_COMMAND in out
assert fleet.ops == [] and _config_flag(fleet.root) is None
# Controls: same known uid folds; a different known uid refuses.
monkeypatch.setattr(gm, "_gateway_identity", lambda home, pid, services: (0, home))
assert auto_migration_blockers(gm.build_migration_plan()) == []
monkeypatch.setattr(gm, "_gateway_identity", lambda home, pid, services: (0 if _name(home) == "default" else 1000, home))
assert any("UNIX privilege boundary" in b for b in auto_migration_blockers(gm.build_migration_plan()))
@@ -854,7 +854,7 @@ A standalone secondary behind any of these boundaries stops the automatic path:
|---|---|
| different service manager or scope | default on user systemd, a secondary on **system** systemd (or launchd), or the default detached with a service-managed secondary |
| more than one installed unit on a profile | a user **and** a system unit for the same profile (the explicit command removes both) |
| different UNIX user | a system unit with its own `User=`, or a live gateway owned by another uid; a system unit whose `User=` this host cannot resolve counts as unknown, never as "same user" |
| different UNIX user | a system unit with its own `User=`, or a live gateway owned by another uid; a system unit whose `User=` this host cannot resolve — on the secondary **or** on the default — counts as unknown, never as "same user" |
| `HERMES_HOME` outside `<default home>/profiles/` | a unit pinning `HERMES_HOME=/opt/hermes/profiles/emma` |
In that case `hermes update` prints the boundary it found plus
@@ -957,13 +957,17 @@ previous value, restarts the default gateway, and reinstalls/starts every
recorded per-profile service (a system unit comes back with the `User=` it had).
The manifest is removed once everything is back.
The forward migration is transactional in the same way: if bringing the default
gateway up fails after the per-profile gateways were removed (for example a
system unit that has to run as root), `--multiplex` rolls back through the
manifest on the spot so no profile is left without a gateway. Should the
process die between flipping the flag and starting the default, the next
`hermes gateway migrate --multiplex` sees the manifest with no live gateway and
resumes from it instead of reporting "already multiplexed".
The forward migration is transactional in the same way. Failures it can see
coming from the plan (a system unit that would have to run as root without a
recorded `User=`, a config file it cannot rewrite) are refused before any
per-profile gateway is stopped. Anything that fails after the manifest is
written — the flag write, a later secondary's stop or unit removal, the
default's install or start — rolls back through the manifest on the spot, so no
profile is left without a gateway. Should the process die anywhere in that
window, the next `hermes gateway migrate --multiplex` sees the flag on, the
manifest, and no live multiplexer serving the migrated profiles (an installed
but stopped default unit does not count) and resumes from the manifest instead
of reporting "already multiplexed".
If no manifest exists (you enabled multiplexing by hand), leave multiplex mode
with `hermes config set gateway.multiplex_profiles false && hermes gateway restart`
and reinstall the per-profile services you want.