refactor(gateway): fail closed without an orchestrator URL; drop stale single-tenant comments
Follow-ups from the #402 review of the single-tenant data-plane teardown. - egress_entrypoint.sh: refuse to launch mitmdump when BOT_BOTTLE_ORCHESTRATOR_URL is unset, so the fail-closed guarantee no longer rests solely on mitmproxy's errorcheck addon exiting on the addon's load-time raise. A misconfigured gateway can never come up as a bare TLS-bumping open proxy with no policy. - orchestrator/gateway.py: ensure_running() raises GatewayError on an empty orchestrator URL — a URL-less launch would only crash-loop the now-resolver-only daemons (egress raises, git-http exits 1, supervise exits 2). The env-injection branch is now unconditional. - Drop stale "single-tenant" / "reads routes.yaml" comments in gateway.py and egress_entrypoint.sh, and the /etc/egress/routes.yaml layout line in Dockerfile.gateway. - Tests: gateway fixtures supply an orchestrator URL; add a refuse-without-URL test and assert the URL env is injected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit is contained in:
@@ -20,7 +20,6 @@
|
||||
# /app/egress-entrypoint.sh mitmdump launcher
|
||||
# /app/supervise_server.py + .py supervise MCP server
|
||||
# /app/gateway_init.py PID 1 supervisor
|
||||
# /etc/egress/routes.yaml bind-mounted at run time
|
||||
# /etc/git-gate/pre-receive docker-cp'd at start time
|
||||
# /git-gate-entrypoint.sh docker-cp'd at start time
|
||||
# /git-gate/creds/* docker-cp'd at start time
|
||||
|
||||
@@ -15,11 +15,23 @@
|
||||
# mitmproxy at it. The option REPLACES mitmproxy's default
|
||||
# trust store, so passing the upstream CA alone would break
|
||||
# non-chained hosts.
|
||||
# * `-s /app/egress_addon.py` loads the addon that reads
|
||||
# /etc/egress/routes.yaml.
|
||||
# * `-s /app/egress_addon.py` loads the addon that resolves each
|
||||
# request's policy from the orchestrator control plane by source
|
||||
# IP (PRD 0070). There is no static routes file.
|
||||
|
||||
set -e
|
||||
|
||||
# Fail closed on a missing policy source. The addon itself raises at
|
||||
# load when BOT_BOTTLE_ORCHESTRATOR_URL is unset (so mitmdump exits via
|
||||
# its errorcheck addon), but that leaves the fail-closed guarantee at the
|
||||
# mercy of a mitmproxy version keeping that behavior. Refuse here too, so
|
||||
# a misconfigured gateway can never come up as a bare TLS-bumping open
|
||||
# proxy with no policy — independent of mitmproxy's startup-error handling.
|
||||
if [ -z "$BOT_BOTTLE_ORCHESTRATOR_URL" ]; then
|
||||
echo "egress: BOT_BOTTLE_ORCHESTRATOR_URL is required (no static routes fallback)" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Pin mitmproxy's config dir to the bind-mount location of its CA
|
||||
# regardless of which user mitmdump runs as. In the legacy
|
||||
# four-daemon setup (Dockerfile.egress, USER mitmproxy) this
|
||||
|
||||
@@ -126,7 +126,10 @@ class DockerGateway(Gateway):
|
||||
self.network = network
|
||||
# The control-plane URL the gateway's data plane resolves per bottle
|
||||
# against — reached by container name over docker DNS on the shared
|
||||
# network (container↔container, no host firewall). Empty → single-tenant.
|
||||
# network (container↔container, no host firewall). Mandatory to *run*
|
||||
# the gateway (see `ensure_running`); empty is tolerated only for the
|
||||
# construct-then-read-CA path (`ca_cert_pem` on an already-running
|
||||
# container), which never launches a container.
|
||||
self._orchestrator_url = orchestrator_url
|
||||
self._build_context = build_context or _REPO_ROOT
|
||||
self._dockerfile = dockerfile
|
||||
@@ -193,6 +196,16 @@ class DockerGateway(Gateway):
|
||||
)
|
||||
|
||||
def ensure_running(self) -> None:
|
||||
# Fail closed on a missing policy source. The data-plane daemons are
|
||||
# resolver-only now (PRD 0070) — without an orchestrator URL egress
|
||||
# raises, git-http exits 1, and supervise exits 2 — so launching a
|
||||
# gateway without one would only crash-loop its daemons. Refuse here so
|
||||
# the misconfiguration surfaces as a clear error, not a broken container.
|
||||
if not self._orchestrator_url:
|
||||
raise GatewayError(
|
||||
"gateway requires an orchestrator URL to run "
|
||||
"(resolver-only data plane; no single-tenant fallback)"
|
||||
)
|
||||
# Recreate when the running container's image is stale (a rebuild),
|
||||
# so source changes to the gateway's flat daemons take effect — not
|
||||
# just when the container is absent.
|
||||
@@ -220,16 +233,15 @@ class DockerGateway(Gateway):
|
||||
for port in self._host_port_bindings:
|
||||
argv += ["--publish", f"0.0.0.0:{port}:{port}"]
|
||||
run_env = dict(os.environ)
|
||||
if self._orchestrator_url:
|
||||
# Makes the gateway's egress / git / supervise daemons multi-tenant:
|
||||
# each request resolves source-IP -> policy against the control plane.
|
||||
argv += ["--env", f"BOT_BOTTLE_ORCHESTRATOR_URL={self._orchestrator_url}"]
|
||||
# ...and presents the control-plane secret on those /resolve calls
|
||||
# (the control plane requires it). Bare `--env NAME` keeps the value
|
||||
# off argv / `docker inspect`; only the gateway (not the agent) is
|
||||
# given it. Only needed in multi-tenant mode, where /resolve is used.
|
||||
argv += ["--env", CONTROL_PLANE_TOKEN_ENV]
|
||||
run_env[CONTROL_PLANE_TOKEN_ENV] = host_control_plane_token()
|
||||
# The gateway's egress / git / supervise daemons resolve source-IP ->
|
||||
# policy against the control plane per request (guaranteed non-empty by
|
||||
# the check above).
|
||||
argv += ["--env", f"BOT_BOTTLE_ORCHESTRATOR_URL={self._orchestrator_url}"]
|
||||
# ...and present the control-plane secret on those /resolve calls (the
|
||||
# control plane requires it). Bare `--env NAME` keeps the value off argv
|
||||
# / `docker inspect`; only the gateway (not the agent) is given it.
|
||||
argv += ["--env", CONTROL_PLANE_TOKEN_ENV]
|
||||
run_env[CONTROL_PLANE_TOKEN_ENV] = host_control_plane_token()
|
||||
argv.append(self.image_ref)
|
||||
proc = run_docker(argv, env=run_env)
|
||||
if proc.returncode != 0:
|
||||
|
||||
@@ -20,7 +20,11 @@ IMAGE = "busybox"
|
||||
class TestDockerGatewayIntegration(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.name = "bot-bottle-orch-gateway-itest-" + secrets.token_hex(4)
|
||||
self.sc = DockerGateway(IMAGE, name=self.name)
|
||||
# Resolver-only data plane (PRD 0070) requires an orchestrator URL to
|
||||
# run; busybox never dials it, so a placeholder is enough here.
|
||||
self.sc = DockerGateway(
|
||||
IMAGE, name=self.name, orchestrator_url="http://orchestrator:9000",
|
||||
)
|
||||
self.addCleanup(self.sc.stop)
|
||||
|
||||
def _count(self) -> int:
|
||||
|
||||
@@ -22,13 +22,27 @@ def _proc(returncode: int = 0, stdout: str = "", stderr: str = "") -> Mock:
|
||||
return Mock(returncode=returncode, stdout=stdout, stderr=stderr)
|
||||
|
||||
|
||||
_ORCH_URL = "http://orchestrator:9000"
|
||||
|
||||
|
||||
class TestDockerGateway(unittest.TestCase):
|
||||
def setUp(self) -> None:
|
||||
self.sc = DockerGateway("bot-bottle-gateway:latest")
|
||||
# Resolver-only data plane (PRD 0070): running the gateway requires an
|
||||
# orchestrator URL, so the fixture supplies one.
|
||||
self.sc = DockerGateway("bot-bottle-gateway:latest", orchestrator_url=_ORCH_URL)
|
||||
|
||||
def test_default_name(self) -> None:
|
||||
self.assertEqual(GATEWAY_NAME, self.sc.name)
|
||||
|
||||
def test_ensure_running_refuses_without_orchestrator_url(self) -> None:
|
||||
# No policy source → the data-plane daemons would only crash-loop, so
|
||||
# the launch must fail closed with a clear error rather than start one.
|
||||
sc = DockerGateway("bot-bottle-gateway:latest")
|
||||
with patch(_RUN_DOCKER) as m:
|
||||
with self.assertRaises(GatewayError):
|
||||
sc.ensure_running()
|
||||
m.assert_not_called()
|
||||
|
||||
def test_is_running_reads_docker_ps(self) -> None:
|
||||
with patch(_RUN_DOCKER, return_value=_proc(stdout=self.sc.name + "\n")):
|
||||
self.assertTrue(self.sc.is_running())
|
||||
@@ -98,6 +112,8 @@ class TestDockerGateway(unittest.TestCase):
|
||||
for a in runs[0]))
|
||||
self.assertTrue(any(
|
||||
a.endswith(":/run/supervise") for a in runs[0]))
|
||||
# Data plane resolves policy against the orchestrator control plane.
|
||||
self.assertIn(f"BOT_BOTTLE_ORCHESTRATOR_URL={_ORCH_URL}", runs[0])
|
||||
|
||||
def test_ensure_running_creates_network_when_missing(self) -> None:
|
||||
calls: list[list[str]] = []
|
||||
|
||||
Reference in New Issue
Block a user