diff --git a/bot_bottle/backend/macos_container/enumerate.py b/bot_bottle/backend/macos_container/enumerate.py index 46dcb72..f8ed062 100644 --- a/bot_bottle/backend/macos_container/enumerate.py +++ b/bot_bottle/backend/macos_container/enumerate.py @@ -8,7 +8,11 @@ from ...bottle_state import read_metadata from .. import ActiveAgent from .infra import INFRA_NAME -_PREFIX = "bot-bottle-" +# The name every agent container carries: `bot-bottle-`. Exported +# because callers that act on a running bottle (gateway-host rewrites, +# registry reconciliation) have to map an enumerated slug back to a +# container name. +CONTAINER_NAME_PREFIX = "bot-bottle-" # The shared per-host infra container carries the same prefix as agent # containers but is infrastructure, not a bottle — one control plane + gateway # serves every agent, so listing it as an agent would invent one per host. @@ -26,9 +30,9 @@ def enumerate_active() -> list[ActiveAgent]: return [] out: list[ActiveAgent] = [] for name in sorted(line.strip() for line in result.stdout.splitlines()): - if not name.startswith(_PREFIX) or name in _INFRA_NAMES: + if not name.startswith(CONTAINER_NAME_PREFIX) or name in _INFRA_NAMES: continue - slug = name[len(_PREFIX):] + slug = name[len(CONTAINER_NAME_PREFIX):] metadata = read_metadata(slug) out.append(ActiveAgent( backend_name="macos-container", diff --git a/bot_bottle/backend/macos_container/gateway_hosts.py b/bot_bottle/backend/macos_container/gateway_hosts.py new file mode 100644 index 0000000..9f5735d --- /dev/null +++ b/bot_bottle/backend/macos_container/gateway_hosts.py @@ -0,0 +1,96 @@ +"""Stable gateway name for macOS agents, via each bottle's `/etc/hosts`. + +The shared gateway's address is assigned by vmnet's DHCP and changes whenever +the infra container is recreated — a source-hash bump, an image upgrade, a +crash. Every agent-facing URL (egress proxy, git-http, supervise) embeds that +address, and the proxy URL reaches the agent as **process environment** at +`container exec` time. A running process's `environ` cannot be rewritten from +outside, so a moved gateway used to strand every running bottle permanently: +not degraded, unreachable, until the bottle was relaunched and its session +thrown away. + +So the agent never learns the address. It is given a stable *name* +(`GATEWAY_HOSTNAME`) in every URL, resolved through its own `/etc/hosts`. +Unlike `environ`, that is a file — it can be rewritten inside a container that +is already running, so a gateway that comes back at a new address is picked up +by live bottles instead of orphaning them. + +Apple Container 1.0 offers no container-name DNS on a user network (the only +nameserver an agent sees is vmnet's, which does not know container names) and +`container run` has no `--add-host`, so the entry is written by exec after the +container starts. + +Writing it needs root, and the agent runs as `node`: the agent therefore +cannot repoint its own gateway name, while the host (which drives `container +exec --user root`) can. That asymmetry is deliberate — keep it. +""" + +from __future__ import annotations + +from ...log import warn +from . import util as container_mod +from .enumerate import CONTAINER_NAME_PREFIX, enumerate_active + +# The name every agent-facing gateway URL uses. Must not collide with a real +# DNS name the agent might resolve; it is bottle-local by construction. +GATEWAY_HOSTNAME = "bot-bottle-gateway" + +# Marker so the rewrite is idempotent and only ever touches our own line — +# the rest of /etc/hosts (localhost, the container's own name) is preserved. +_MARKER = "# bot-bottle gateway" + + +def _rewrite_script(gateway_ip: str) -> str: + """A shell one-liner that replaces our managed line in `/etc/hosts`. + + Rewrites in place via a temp file + `cat` rather than `mv`, so the file + keeps its original inode, ownership, and mode — a bind-mounted or + pre-created `/etc/hosts` must not be replaced by a root-owned 0644 copy + that the runtime then refuses to update. + """ + return ( + "set -e; " + f"grep -v '{_MARKER}' /etc/hosts > /tmp/.bb-hosts || true; " + f"printf '%s %s %s\\n' '{gateway_ip}' '{GATEWAY_HOSTNAME}' " + f"'{_MARKER}' >> /tmp/.bb-hosts; " + "cat /tmp/.bb-hosts > /etc/hosts; " + "rm -f /tmp/.bb-hosts" + ) + + +def set_gateway_host(container_name: str, gateway_ip: str) -> None: + """Point `GATEWAY_HOSTNAME` at `gateway_ip` inside one running container. + + Must run before the agent is exec'd: the agent's proxy URL names the + gateway, so the entry has to exist for its first connection. Idempotent — + re-running with the same address is a no-op in effect. + """ + container_mod.exec_container_as_root( + container_name, ["sh", "-c", _rewrite_script(gateway_ip)], + ) + + +def refresh_gateway_host(gateway_ip: str) -> list[str]: + """Re-point every running bottle at the current gateway address. + + Called once the shared gateway is known to be up, so a bottle stranded by + an earlier gateway restart re-attaches instead of needing a relaunch. + Returns the containers updated. + + Best-effort per bottle: one container that refuses the write (already + exiting, say) must not stop the others from being repaired, and must not + fail the launch that triggered the sweep. + """ + updated: list[str] = [] + for agent in enumerate_active(): + name = f"{CONTAINER_NAME_PREFIX}{agent.slug}" + try: + set_gateway_host(name, gateway_ip) + updated.append(name) + # One bad bottle must not stop the sweep, so this is deliberately broad. + except Exception as e: # noqa: BLE001 # pylint: disable=broad-exception-caught + warn(f"could not re-point {name} at the gateway: {e}") + return updated + + +__all__ = ["GATEWAY_HOSTNAME", "set_gateway_host", "refresh_gateway_host"] diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 056ab78..be8a348 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -59,6 +59,11 @@ from ..docker.egress import EGRESS_PORT from ..util import AGENT_CA_BUNDLE, AGENT_CA_PATH from . import util as container_mod from .bottle import MacosContainerBottle +from .gateway_hosts import ( + GATEWAY_HOSTNAME, + refresh_gateway_host, + set_gateway_host, +) from .bottle_plan import MacosContainerBottlePlan from .consolidated_launch import ( GatewayEndpoint, @@ -100,6 +105,11 @@ def launch( # Step 1: the per-host singletons. Must precede the agent run — its # proxy env needs the gateway's address at `container run` time. endpoint = ensure_gateway() + # The gateway's address may have changed since these bottles launched + # (any infra recreate re-runs DHCP). They name the gateway rather than + # address it, so re-pointing /etc/hosts re-attaches them in place + # instead of leaving them stranded until relaunch. + refresh_gateway_host(endpoint.gateway_ip) # Step 2: mint this bottle's deploy keys, then point it at the SHARED # gateway's CA + git-http/supervise ports. @@ -117,6 +127,9 @@ def launch( # attribution key; `--cap-drop CAP_NET_RAW` at run is what makes it # unforgeable. Poll: `container run --detach` can return before vmnet's # DHCP has assigned the address. + # Resolve the gateway name before anything execs: every agent-facing + # URL uses it, so the entry must exist for the first connection. + set_gateway_host(plan.container_name, endpoint.gateway_ip) source_ip = container_mod.wait_container_ipv4_on_network( plan.container_name, endpoint.network, ) @@ -231,13 +244,20 @@ def _stamp_agent_urls( ) -> MacosContainerBottlePlan: """Point the agent's git-gate insteadOf rewrites + supervise MCP at the shared gateway's ports. Both bypass the egress proxy (NO_PROXY covers the - gateway address).""" + gateway name). + + Addressed by `GATEWAY_HOSTNAME`, never by IP: these URLs are baked into + the agent's gitconfig and MCP config at provision time, so an address here + would strand the bottle the moment the gateway moved. The name is resolved + per connection through `/etc/hosts`, which stays rewritable while the + bottle runs.""" + del endpoint # addressed by name; the address reaches the bottle via /etc/hosts git_gate_url = ( - f"http://{endpoint.gateway_ip}:{_GIT_HTTP_PORT}" + f"http://{GATEWAY_HOSTNAME}:{_GIT_HTTP_PORT}" if plan.git_gate_plan.upstreams else "" ) supervise_url = ( - f"http://{endpoint.gateway_ip}:{SUPERVISE_PORT}/" + f"http://{GATEWAY_HOSTNAME}:{SUPERVISE_PORT}/" if plan.supervise_plan is not None else "" ) return dataclasses.replace( @@ -247,19 +267,25 @@ def _stamp_agent_urls( ) -def _proxy_url(gateway_ip: str, identity_token: str = "") -> str: +def _proxy_url(identity_token: str = "") -> str: """The agent's egress proxy URL. The identity token rides as proxy credentials — the gateway reads Proxy-Authorization, resolves the (source_ip, token) pair against the control plane, and strips it before - upstream. Without a valid pair `/resolve` denies the request (#366).""" + upstream. Without a valid pair `/resolve` denies the request (#366). + + Names the gateway rather than addressing it: this URL reaches the agent as + process environment, which cannot be rewritten once the agent is running, + so an address baked here is unfixable if the gateway moves.""" cred = f"bottle:{identity_token}@" if identity_token else "" - return f"http://{cred}{gateway_ip}:{EGRESS_PORT}" + return f"http://{cred}{GATEWAY_HOSTNAME}:{EGRESS_PORT}" -def _no_proxy(gateway_ip: str) -> str: +def _no_proxy() -> str: # git-http + supervise live on the gateway and must NOT go through the - # egress proxy — the agent reaches them directly by its address. - return f"localhost,127.0.0.1,{gateway_ip}" + # egress proxy — the agent reaches them directly by name. Deliberately + # address-free: NO_PROXY is baked into the run-time env and is therefore + # just as unfixable as the proxy URL if the gateway moves. + return f"localhost,127.0.0.1,{GATEWAY_HOSTNAME}" def _identity_proxy_env( @@ -276,7 +302,8 @@ def _identity_proxy_env( `_agent_env_entries`.""" if not identity_token: return {} - url = _proxy_url(endpoint.gateway_ip, identity_token) + del endpoint # the gateway is named, not addressed + url = _proxy_url(identity_token) return { "HTTPS_PROXY": url, "HTTP_PROXY": url, "https_proxy": url, "http_proxy": url, @@ -338,7 +365,7 @@ def _agent_env_entries( # silently dropping attribution for. Without it a process that egresses # before the exec-time env still fails closed — the agent network is # host-only, so there is no route off it except the gateway. - no_proxy = _no_proxy(endpoint.gateway_ip) + no_proxy = _no_proxy() env = [ f"NO_PROXY={no_proxy}", f"no_proxy={no_proxy}", diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index 98c24b2..9c5f3aa 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -360,6 +360,21 @@ def exec_container(name: str, argv: list[str]) -> None: ) +def exec_container_as_root(name: str, argv: list[str]) -> None: + """`exec_container`, but as uid 0 inside the container. + + For host-driven maintenance the agent itself must not be able to perform — + rewriting `/etc/hosts` to point the gateway name at an address. The agent + runs as `node`, so it cannot repoint its own gateway; the host can. + """ + result = _run_container_op([_CONTAINER, "exec", "--user", "root", name, *argv]) + if result.returncode != 0: + die( + f"container exec (root) in {name} failed: " + f"{(result.stderr or '').strip() or ''}" + ) + + def _run_container_op(cmd: list[str]) -> subprocess.CompletedProcess[str]: result = subprocess.run( cmd, diff --git a/tests/unit/test_macos_container_launch_wiring.py b/tests/unit/test_macos_container_launch_wiring.py index f743ac5..1763e5d 100644 --- a/tests/unit/test_macos_container_launch_wiring.py +++ b/tests/unit/test_macos_container_launch_wiring.py @@ -17,6 +17,7 @@ from unittest.mock import patch from bot_bottle.backend.macos_container.bottle import MacosContainerBottle from bot_bottle.backend.macos_container.bottle_plan import MacosContainerBottlePlan from bot_bottle.backend.macos_container.consolidated_launch import GatewayEndpoint +from bot_bottle.backend.macos_container.gateway_hosts import GATEWAY_HOSTNAME from bot_bottle.backend.macos_container.launch import ( _agent_run_argv, _identity_proxy_env, @@ -128,7 +129,13 @@ class TestAgentRunArgv(unittest.TestCase): """git-http + supervise live on the gateway and must be reached directly, not through its own egress proxy.""" entry = next(a for a in self.argv if a.startswith("NO_PROXY=")) - self.assertIn("192.168.128.3", entry) + self.assertIn(GATEWAY_HOSTNAME, entry) + + def test_no_proxy_names_the_gateway_and_never_addresses_it(self) -> None: + """NO_PROXY is baked into the run-time env, so an address here is as + unfixable as the proxy URL if the gateway moves.""" + entry = next(a for a in self.argv if a.startswith("NO_PROXY=")) + self.assertNotIn("192.168.128.3", entry) def test_forwarded_secrets_stay_off_argv(self) -> None: """Bare name → inherited from the run process env, so the value never @@ -146,7 +153,7 @@ class TestIdentityTokenDelivery(unittest.TestCase): def test_exec_env_carries_the_token_as_proxy_credentials(self) -> None: env = _identity_proxy_env(_endpoint(), "s3cret") self.assertEqual( - "http://bottle:s3cret@192.168.128.3:9099", env["HTTP_PROXY"], + f"http://bottle:s3cret@{GATEWAY_HOSTNAME}:9099", env["HTTP_PROXY"], ) self.assertEqual(env["HTTP_PROXY"], env["https_proxy"]) @@ -214,7 +221,7 @@ class TestPlanIdentityToken(unittest.TestCase): self.assertIn("HTTP_PROXY", argv) self.assertNotIn("s3cret", " ".join(argv)) self.assertEqual( - "http://bottle:s3cret@192.168.128.3:9099", kwargs["env"]["HTTP_PROXY"], + f"http://bottle:s3cret@{GATEWAY_HOSTNAME}:9099", kwargs["env"]["HTTP_PROXY"], ) diff --git a/tests/unit/test_macos_gateway_hosts.py b/tests/unit/test_macos_gateway_hosts.py new file mode 100644 index 0000000..066a8c9 --- /dev/null +++ b/tests/unit/test_macos_gateway_hosts.py @@ -0,0 +1,140 @@ +"""Unit: stable gateway name via each bottle's /etc/hosts (issue #443). + +The gateway's address moves whenever the infra container is recreated. Agents +name it instead of addressing it, and the name resolves through `/etc/hosts` — +a file, so it stays rewritable while the bottle runs, unlike the `environ` the +proxy URL is delivered in. +""" + +from __future__ import annotations + +import unittest +from types import SimpleNamespace +from unittest.mock import patch + +from bot_bottle.backend.macos_container.gateway_hosts import ( + GATEWAY_HOSTNAME, + refresh_gateway_host, + set_gateway_host, +) + +_MOD = "bot_bottle.backend.macos_container.gateway_hosts" + + +class TestSetGatewayHost(unittest.TestCase): + def _script(self, exec_root: object) -> str: + argv = exec_root.call_args.args[1] # type: ignore[attr-defined] + self.assertEqual(["sh", "-c"], argv[:2]) + return argv[2] + + def test_writes_the_address_against_the_stable_name(self) -> None: + with patch(f"{_MOD}.container_mod.exec_container_as_root") as ex: + set_gateway_host("bot-bottle-demo", "192.168.128.19") + self.assertEqual("bot-bottle-demo", ex.call_args.args[0]) + script = self._script(ex) + self.assertIn("192.168.128.19", script) + self.assertIn(GATEWAY_HOSTNAME, script) + + def test_runs_as_root_so_the_agent_cannot_repoint_itself(self) -> None: + """The agent runs as `node`. If it could rewrite /etc/hosts it could + aim its own gateway name elsewhere, so the write must go through the + root-only helper.""" + with patch(f"{_MOD}.container_mod.exec_container_as_root") as ex: + set_gateway_host("bot-bottle-demo", "10.0.0.1") + ex.assert_called_once() + + def test_is_idempotent_by_removing_its_own_line_first(self) -> None: + """Re-pointing must replace the managed entry, not append a second one + — two entries for the same name would resolve by luck of ordering.""" + with patch(f"{_MOD}.container_mod.exec_container_as_root") as ex: + set_gateway_host("bot-bottle-demo", "10.0.0.1") + self.assertIn("grep -v", self._script(ex)) + + def test_preserves_the_rest_of_the_hosts_file(self) -> None: + """localhost and the container's own name must survive the rewrite.""" + with patch(f"{_MOD}.container_mod.exec_container_as_root") as ex: + set_gateway_host("bot-bottle-demo", "10.0.0.1") + script = self._script(ex) + # Filter-and-append, never a truncating write of just our line. + self.assertIn("/etc/hosts >", script) + self.assertIn(">> /tmp/.bb-hosts", script) + + def test_keeps_the_original_inode(self) -> None: + """`cat >` rather than `mv`: a pre-created /etc/hosts must keep its + ownership and mode, not be replaced by a root-owned copy.""" + script = None + with patch(f"{_MOD}.container_mod.exec_container_as_root") as ex: + set_gateway_host("bot-bottle-demo", "10.0.0.1") + script = self._script(ex) + self.assertIn("cat /tmp/.bb-hosts > /etc/hosts", script) + self.assertNotIn("mv ", script) + + +class TestRefreshGatewayHost(unittest.TestCase): + """The re-attach sweep: bottles stranded by an earlier gateway restart get + re-pointed in place instead of needing a relaunch.""" + + def _agents(self, *slugs: str) -> list[SimpleNamespace]: + return [SimpleNamespace(slug=s) for s in slugs] + + def test_repoints_every_running_bottle(self) -> None: + with patch(f"{_MOD}.enumerate_active", return_value=self._agents("a", "b")), \ + patch(f"{_MOD}.set_gateway_host") as setter: + updated = refresh_gateway_host("192.168.128.19") + self.assertEqual(["bot-bottle-a", "bot-bottle-b"], updated) + self.assertEqual( + [("bot-bottle-a", "192.168.128.19"), ("bot-bottle-b", "192.168.128.19")], + [c.args for c in setter.call_args_list], + ) + + def test_one_failing_bottle_does_not_stop_the_sweep(self) -> None: + """A container that is already exiting must not block the repair of + its neighbours, nor fail the launch that triggered the sweep.""" + def _flaky(name: str, _ip: str) -> None: + if name == "bot-bottle-a": + raise RuntimeError("container is exiting") + + with patch(f"{_MOD}.enumerate_active", return_value=self._agents("a", "b")), \ + patch(f"{_MOD}.set_gateway_host", side_effect=_flaky), \ + patch(f"{_MOD}.warn") as warn: + updated = refresh_gateway_host("10.0.0.1") + self.assertEqual(["bot-bottle-b"], updated) + warn.assert_called_once() + + def test_no_running_bottles_is_a_clean_no_op(self) -> None: + with patch(f"{_MOD}.enumerate_active", return_value=[]), \ + patch(f"{_MOD}.set_gateway_host") as setter: + self.assertEqual([], refresh_gateway_host("10.0.0.1")) + setter.assert_not_called() + + + +class TestLaunchWiring(unittest.TestCase): + """Ordering matters: the name must resolve before anything execs, and the + stranded-bottle sweep must run once the gateway is known to be up.""" + + def test_launch_sets_the_host_entry_before_reading_the_source_ip(self) -> None: + """The agent's every URL names the gateway, so the entry has to exist + before the first connection — which means before the agent execs.""" + import inspect + + from bot_bottle.backend.macos_container import launch + + src = inspect.getsource(launch) + set_at = src.index("set_gateway_host(plan.container_name") + exec_at = src.index("wait_container_ipv4_on_network") + self.assertLess(set_at, exec_at) + + def test_launch_refreshes_stranded_bottles_after_ensure_gateway(self) -> None: + import inspect + + from bot_bottle.backend.macos_container import launch + + src = inspect.getsource(launch) + ensure_at = src.index("endpoint = ensure_gateway()") + refresh_at = src.index("refresh_gateway_host(endpoint.gateway_ip)") + self.assertLess(ensure_at, refresh_at) + + +if __name__ == "__main__": + unittest.main()