feat(orchestrator): list_live broker op + reconcile via broker (#468)
Chunk 3 of the host-control-server stack: grow the broker op vocabulary
(PRD gap 3), starting with `list_live`, and invert `reconcile` onto it.
- broker: split the closed op vocabulary into mutation (`launch`/`teardown`,
carry a bottle id + static flags) and query (`list_live`, carries nothing
but its op name) kinds. `verify_request` now enforces a **strict schema**
(open question 1, resolved yes): unknown claim keys are rejected, a mutation
must name its bottle, and a query that smuggles any id/flag is refused.
- broker verb: `LaunchBroker.list_live` / `SubmitBroker.list_live` return the
backend's live source IPs; a backend enumeration failure is converted to the
single `BrokerUnavailableError` "live set unknown" signal. `DockerBroker`
enumerates its labelled containers; `StubBroker` derives from launches (or a
test override).
- host controller: `POST /broker/live` verifies a signed `list_live` token and
returns `{source_ips}`; `BrokerClient.list_live` is its drop-in client.
- reconcile: `OrchestratorCore.reconcile()` drops the `live_source_ips`
parameter and pulls the live set from the broker itself — the tell that the
orchestrator couldn't see the backend goes away. **Fail-safe**: if the broker
can't return an authoritative set the sweep is skipped, never run against an
empty/partial set (which would reap healthy rows). The `/reconcile` HTTP
contract + `OrchestratorClient.reconcile` become a bare trigger.
The macOS launcher's Apple-container enumeration stays for now; it becomes the
host controller's `list_live` when launch itself moves behind the broker (the
pulled-forward chunk 5, next in the stack).
Tests + pyright clean; pylint 10.0 on broker.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -32,7 +32,23 @@ from dataclasses import dataclass
|
||||
from typing import Protocol
|
||||
|
||||
_JWT_HEADER = {"alg": "HS256", "typ": "JWT"}
|
||||
_ALLOWED_OPS = ("launch", "teardown")
|
||||
|
||||
# The closed op vocabulary, split by kind (PRD "ids + static flags" rule):
|
||||
# * mutation ops act on ONE bottle — they carry its id (+ static launch flags);
|
||||
# * query ops enumerate host state — they carry NO ids or flags at all.
|
||||
# Each new host-privileged op is added here deliberately; `verify_request`
|
||||
# enforces the per-kind shape so the privileged surface can't drift.
|
||||
_MUTATION_OPS = ("launch", "teardown")
|
||||
_QUERY_OPS = ("list_live",)
|
||||
_ALLOWED_OPS = _MUTATION_OPS + _QUERY_OPS
|
||||
|
||||
# Every claim key a signed request may carry — the request fields plus the two
|
||||
# per-signature envelope claims. A strict allow-list (open question 1, resolved
|
||||
# "yes"): an unknown key is rejected outright, so the schema can't silently widen
|
||||
# as ops are added.
|
||||
_ALLOWED_CLAIMS = frozenset(
|
||||
{"op", "bottle_id", "source_ip", "image_ref", "slot", "jti", "iat"}
|
||||
)
|
||||
|
||||
|
||||
class BrokerAuthError(Exception):
|
||||
@@ -56,17 +72,28 @@ class BrokerUnavailableError(Exception):
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class LaunchRequest:
|
||||
"""The structured, un-coercible launch/teardown request. Ids + flags
|
||||
only — `image_ref` is a content-addressed id, never a path/argv."""
|
||||
class BrokerRequest:
|
||||
"""A structured, un-coercible broker request. Ids + static flags only —
|
||||
`image_ref` is a content-addressed id, never a path/argv.
|
||||
|
||||
A **mutation** op (`launch`/`teardown`) names the bottle it acts on in
|
||||
`bottle_id` (plus launch's static flags). A **query** op (`list_live`) acts
|
||||
on no single bottle, so it carries nothing but its `op` name — `bottle_id`
|
||||
and every flag stay empty, and `verify_request` rejects a query that smuggles
|
||||
any in. `LaunchRequest` is kept as an alias for the launch/teardown callers."""
|
||||
|
||||
op: str # one of _ALLOWED_OPS
|
||||
bottle_id: str
|
||||
bottle_id: str = ""
|
||||
source_ip: str = ""
|
||||
image_ref: str = ""
|
||||
slot: int | None = None
|
||||
|
||||
|
||||
# Historical name — the request type predates the query ops. Kept so the launch
|
||||
# path (`OrchestratorCore`, `DockerBroker`) reads unchanged.
|
||||
LaunchRequest = BrokerRequest
|
||||
|
||||
|
||||
# --- minimal JWS/JWT (HS256), stdlib only ----------------------------------
|
||||
|
||||
def _b64url(data: bytes) -> str:
|
||||
@@ -118,20 +145,35 @@ def verify_request(token: str, secret: bytes) -> LaunchRequest:
|
||||
raise BrokerAuthError("unexpected header/alg")
|
||||
if not isinstance(claims, dict):
|
||||
raise BrokerAuthError("claims are not an object")
|
||||
# Strict schema (open question 1): a signed request may carry ONLY the known
|
||||
# claim keys, so the privileged surface can't widen by smuggling an extra
|
||||
# claim past the fixed field checks below.
|
||||
unknown = set(claims) - _ALLOWED_CLAIMS
|
||||
if unknown:
|
||||
raise BrokerAuthError(f"unknown claim(s): {', '.join(sorted(unknown))}")
|
||||
|
||||
op = claims.get("op")
|
||||
bottle_id = claims.get("bottle_id")
|
||||
if not isinstance(op, str) or op not in _ALLOWED_OPS:
|
||||
raise BrokerAuthError("request does not match the broker-request schema")
|
||||
bottle_id = claims.get("bottle_id", "")
|
||||
source_ip = claims.get("source_ip", "")
|
||||
image_ref = claims.get("image_ref", "")
|
||||
slot = claims.get("slot")
|
||||
if (
|
||||
not isinstance(op, str) or op not in _ALLOWED_OPS
|
||||
or not isinstance(bottle_id, str) or not bottle_id
|
||||
not isinstance(bottle_id, str)
|
||||
or not isinstance(source_ip, str) or not isinstance(image_ref, str)
|
||||
or not (slot is None or isinstance(slot, int))
|
||||
):
|
||||
raise BrokerAuthError("request does not match the launch-request schema")
|
||||
return LaunchRequest(
|
||||
raise BrokerAuthError("request does not match the broker-request schema")
|
||||
# Per-kind shape: a mutation names exactly one bottle; a query names none and
|
||||
# carries no flags (the "no arguments" rule for `list_live`). Enforcing both
|
||||
# halves keeps `bottle_id`/flags from being a coercible field on a query op.
|
||||
if op in _MUTATION_OPS:
|
||||
if not bottle_id:
|
||||
raise BrokerAuthError(f"{op} requires a bottle_id")
|
||||
elif bottle_id or source_ip or image_ref or slot is not None:
|
||||
raise BrokerAuthError(f"{op} is a query op and takes no ids or flags")
|
||||
return BrokerRequest(
|
||||
op=op, bottle_id=bottle_id, source_ip=source_ip, image_ref=image_ref, slot=slot
|
||||
)
|
||||
|
||||
@@ -139,14 +181,18 @@ def verify_request(token: str, secret: bytes) -> LaunchRequest:
|
||||
# --- the broker itself ------------------------------------------------------
|
||||
|
||||
class SubmitBroker(Protocol):
|
||||
"""The single method `OrchestratorCore` depends on: verify a signed token and
|
||||
perform its op, returning the verified request. Both the in-process
|
||||
`LaunchBroker` and the out-of-process `BrokerClient` (which relays the token
|
||||
to the host control server) satisfy it structurally, so the core is unchanged
|
||||
whether the backend is local or a real host service."""
|
||||
"""The broker surface `OrchestratorCore` depends on. `submit` verifies a
|
||||
signed *mutation* token and performs its op, returning the verified request;
|
||||
`list_live` verifies a signed `list_live` token and returns the source IPs of
|
||||
the bottles the backend currently has running (reconcile's live set). Both the
|
||||
in-process `LaunchBroker` and the out-of-process `BrokerClient` (which relays
|
||||
the token to the host control server) satisfy it structurally, so the core is
|
||||
unchanged whether the backend is local or a real host service."""
|
||||
|
||||
def submit(self, token: str) -> LaunchRequest: ...
|
||||
|
||||
def list_live(self, token: str) -> list[str]: ...
|
||||
|
||||
|
||||
class LaunchBroker(abc.ABC):
|
||||
"""Verifies a signed request came from the orchestrator, then performs
|
||||
@@ -157,15 +203,35 @@ class LaunchBroker(abc.ABC):
|
||||
self._secret = secret
|
||||
|
||||
def submit(self, token: str) -> LaunchRequest:
|
||||
"""Verify `token` and perform its op. Returns the verified request;
|
||||
raises `BrokerAuthError` if provenance/shape fails."""
|
||||
"""Verify `token` and perform its mutation op. Returns the verified
|
||||
request; raises `BrokerAuthError` if provenance/shape fails, or if the
|
||||
token is a non-mutation op (a `list_live` token routed here by mistake)."""
|
||||
req = verify_request(token, self._secret)
|
||||
if req.op == "launch":
|
||||
self._launch(req)
|
||||
else:
|
||||
elif req.op == "teardown":
|
||||
self._teardown(req)
|
||||
else:
|
||||
raise BrokerAuthError(f"{req.op} is not a submit op")
|
||||
return req
|
||||
|
||||
def list_live(self, token: str) -> list[str]:
|
||||
"""Verify a `list_live` `token` and return the source IPs of the bottles
|
||||
the backend currently has running. Raises `BrokerAuthError` on bad
|
||||
provenance/shape or a non-`list_live` op; a backend enumeration failure
|
||||
is converted to `BrokerUnavailableError` — the single "live set could not
|
||||
be determined" signal reconcile catches to skip the sweep rather than
|
||||
reaping healthy rows against a partial set (the out-of-process
|
||||
`BrokerClient` raises the same on an enumeration 502 / no response)."""
|
||||
req = verify_request(token, self._secret)
|
||||
if req.op != "list_live":
|
||||
raise BrokerAuthError(f"{req.op} is not a list_live op")
|
||||
try:
|
||||
return list(self._list_live())
|
||||
except Exception as e: # noqa: BLE001 — any enumeration failure means the
|
||||
# live set is unknown; reconcile must skip, never reap against it.
|
||||
raise BrokerUnavailableError(f"list_live enumeration failed: {e}") from e
|
||||
|
||||
@abc.abstractmethod
|
||||
def _launch(self, req: LaunchRequest) -> None:
|
||||
...
|
||||
@@ -174,15 +240,31 @@ class LaunchBroker(abc.ABC):
|
||||
def _teardown(self, req: LaunchRequest) -> None:
|
||||
...
|
||||
|
||||
@abc.abstractmethod
|
||||
def _list_live(self) -> list[str]:
|
||||
"""Every running bottle's source IP, backend-native. Must raise (not
|
||||
return a partial list) if the live set can't be determined
|
||||
authoritatively, so reconcile skips rather than reaping healthy rows."""
|
||||
...
|
||||
|
||||
|
||||
class StubBroker(LaunchBroker):
|
||||
"""Dev-harness broker: records verified requests without launching
|
||||
anything. Exercises the full sign -> verify -> act contract in-process."""
|
||||
anything. Exercises the full sign -> verify -> act contract in-process.
|
||||
|
||||
Its live set (`_list_live`) is, by default, every bottle it recorded a
|
||||
launch for and no teardown since — the in-process analogue of enumerating
|
||||
the backend. Tests that need to simulate a bottle dying out from under the
|
||||
registry (the case reconcile exists for) set `live_source_ips` explicitly to
|
||||
override that derived set."""
|
||||
|
||||
def __init__(self, secret: bytes) -> None:
|
||||
super().__init__(secret)
|
||||
self.launched: list[LaunchRequest] = []
|
||||
self.torn_down: list[LaunchRequest] = []
|
||||
# When not None, the exact live set `_list_live` reports — lets a test
|
||||
# say "only these IPs are still up" regardless of what was launched.
|
||||
self.live_source_ips: list[str] | None = None
|
||||
|
||||
def _launch(self, req: LaunchRequest) -> None:
|
||||
self.launched.append(req)
|
||||
@@ -190,10 +272,18 @@ class StubBroker(LaunchBroker):
|
||||
def _teardown(self, req: LaunchRequest) -> None:
|
||||
self.torn_down.append(req)
|
||||
|
||||
def _list_live(self) -> list[str]:
|
||||
if self.live_source_ips is not None:
|
||||
return list(self.live_source_ips)
|
||||
torn = {r.bottle_id for r in self.torn_down}
|
||||
return [r.source_ip for r in self.launched
|
||||
if r.bottle_id not in torn and r.source_ip]
|
||||
|
||||
|
||||
__all__ = [
|
||||
"BrokerAuthError",
|
||||
"BrokerUnavailableError",
|
||||
"BrokerRequest",
|
||||
"LaunchRequest",
|
||||
"SubmitBroker",
|
||||
"LaunchBroker",
|
||||
|
||||
Reference in New Issue
Block a user