refactor(egress): write routes.yaml as actual YAML, not JSON-in-yml
`egress_render_routes` now emits hand-rolled YAML in the same style
as `pipelock_render_yaml`. The egress addon parses it via
`yaml_subset.parse_yaml_subset` — the same parser the manifest
loader + pipelock_apply use.
Why bother: routes.yaml is bind-mounted into the egress sidecar
AND surfaced to operators through `routes edit` (PRD 0019). JSON-
in-yml renders ugly in $EDITOR and signals "this is data" rather
than "this is config you can read at a glance". Real YAML reads
cleanly.
Mechanics:
- `yaml_subset.py` drops its `claude_bottle.log` dependency.
Errors now raise `YamlSubsetError` (a `ValueError`); the
manifest loader + pipelock_apply catch it at the boundary
and forward to `die` / `PipelockApplyError` so callers see
the same behavior they did before.
- `Dockerfile.egress` adds one COPY line for `yaml_subset.py`
so it sits flat in `/app/` next to the addon. The addon
uses an absolute-import-with-fallback shim so the same file
works inside the container AND from the host's unit tests.
- `egress_apply._merge_single_route` round-trips current
routes.yaml through `parse_yaml_subset` + a new
`_render_routes_payload` helper instead of `json.loads` +
`json.dumps`.
End-to-end: rebuilt the egress image, ran `./cli.py start` to a
full bring-up, confirmed the addon's boot log shows `egress:
loaded 9 route(s)` — i.e., the YAML parses inside the container.
453 unit + 3 integration tests pass.
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
"""Unit: Egress route lift + routes.yaml render + token
|
||||
resolution (PRD 0017)."""
|
||||
|
||||
import json
|
||||
import unittest
|
||||
|
||||
from claude_bottle.egress import (
|
||||
@@ -14,6 +13,7 @@ from claude_bottle.egress import (
|
||||
)
|
||||
from claude_bottle.log import Die
|
||||
from claude_bottle.manifest import Manifest
|
||||
from claude_bottle.yaml_subset import parse_yaml_subset
|
||||
|
||||
|
||||
def _bottle(routes):
|
||||
@@ -134,6 +134,15 @@ class TestTokenEnvMap(unittest.TestCase):
|
||||
|
||||
|
||||
class TestRenderRoutes(unittest.TestCase):
|
||||
"""Render is YAML now (PRD 0017 follow-up). Tests parse the
|
||||
output via `yaml_subset` — the same parser the addon uses —
|
||||
so the assertions check the actual semantic shape the addon
|
||||
will see, not the textual layout."""
|
||||
|
||||
@staticmethod
|
||||
def _parsed(routes) -> list[dict]:
|
||||
return parse_yaml_subset(egress_render_routes(routes))["routes"]
|
||||
|
||||
def test_authenticated_route_serialised_with_auth_fields(self):
|
||||
b = _bottle([{
|
||||
"host": "api.github.com",
|
||||
@@ -141,7 +150,7 @@ class TestRenderRoutes(unittest.TestCase):
|
||||
"path_allowlist": ["/repos/x/"],
|
||||
}])
|
||||
routes = egress_manifest_routes(b)
|
||||
payload = json.loads(egress_render_routes(routes))
|
||||
parsed = self._parsed(routes)
|
||||
self.assertEqual(
|
||||
[{
|
||||
"host": "api.github.com",
|
||||
@@ -149,7 +158,7 @@ class TestRenderRoutes(unittest.TestCase):
|
||||
"auth_scheme": "Bearer",
|
||||
"token_env": "EGRESS_TOKEN_0",
|
||||
}],
|
||||
payload["routes"],
|
||||
parsed,
|
||||
)
|
||||
|
||||
def test_unauthenticated_route_omits_auth_fields(self):
|
||||
@@ -159,8 +168,7 @@ class TestRenderRoutes(unittest.TestCase):
|
||||
# round-trip as a partial pair and crash.
|
||||
b = _bottle([{"host": "github.com", "path_allowlist": ["/x/"]}])
|
||||
routes = egress_manifest_routes(b)
|
||||
payload = json.loads(egress_render_routes(routes))
|
||||
entry = payload["routes"][0]
|
||||
entry = self._parsed(routes)[0]
|
||||
self.assertNotIn("auth_scheme", entry)
|
||||
self.assertNotIn("token_env", entry)
|
||||
|
||||
@@ -170,8 +178,12 @@ class TestRenderRoutes(unittest.TestCase):
|
||||
"auth": {"scheme": "Bearer", "token_ref": "CL"},
|
||||
}])
|
||||
routes = egress_manifest_routes(b)
|
||||
payload = json.loads(egress_render_routes(routes))
|
||||
self.assertNotIn("path_allowlist", payload["routes"][0])
|
||||
self.assertNotIn("path_allowlist", self._parsed(routes)[0])
|
||||
|
||||
def test_empty_routes_round_trips(self):
|
||||
rendered = egress_render_routes(())
|
||||
# Inline-empty-list form is what the parser accepts.
|
||||
self.assertEqual([], parse_yaml_subset(rendered)["routes"])
|
||||
|
||||
def test_round_trip_through_addon_core(self):
|
||||
# Render here → parse in the addon must succeed for every
|
||||
|
||||
@@ -105,16 +105,39 @@ class TestParseRoutes(unittest.TestCase):
|
||||
|
||||
|
||||
class TestLoadRoutes(unittest.TestCase):
|
||||
def test_json_text_round_trip(self):
|
||||
routes = load_routes('{"routes":[{"host":"api.example"}]}')
|
||||
def test_yaml_text_round_trip(self):
|
||||
routes = load_routes(
|
||||
'routes:\n'
|
||||
' - host: "api.example"\n'
|
||||
)
|
||||
self.assertEqual(1, len(routes))
|
||||
self.assertEqual("api.example", routes[0].host)
|
||||
|
||||
def test_invalid_json_raises_value_error(self):
|
||||
# Both decode and schema errors land as ValueError so callers
|
||||
# have a single except clause.
|
||||
def test_full_route_shape_parses(self):
|
||||
routes = load_routes(
|
||||
'routes:\n'
|
||||
' - host: "api.example"\n'
|
||||
' auth_scheme: "Bearer"\n'
|
||||
' token_env: "EGRESS_TOKEN_0"\n'
|
||||
' path_allowlist:\n'
|
||||
' - "/v1/"\n'
|
||||
' - "/messages"\n'
|
||||
)
|
||||
self.assertEqual(1, len(routes))
|
||||
r = routes[0]
|
||||
self.assertEqual("api.example", r.host)
|
||||
self.assertEqual("Bearer", r.auth_scheme)
|
||||
self.assertEqual("EGRESS_TOKEN_0", r.token_env)
|
||||
self.assertEqual(("/v1/", "/messages"), r.path_allowlist)
|
||||
|
||||
def test_empty_routes_list(self):
|
||||
routes = load_routes("routes: []\n")
|
||||
self.assertEqual((), routes)
|
||||
|
||||
def test_invalid_yaml_raises_value_error(self):
|
||||
# Tab indent is a YamlSubsetError; ValueError is its base.
|
||||
with self.assertRaises(ValueError):
|
||||
load_routes("not json at all")
|
||||
load_routes("routes:\n\t- host: x\n")
|
||||
|
||||
|
||||
# --- match_route ---------------------------------------------------------
|
||||
|
||||
@@ -4,8 +4,6 @@ integration test."""
|
||||
|
||||
import unittest
|
||||
|
||||
import json
|
||||
|
||||
from claude_bottle.backend.docker.egress_apply import (
|
||||
EgressApplyError,
|
||||
_hosts_in_routes,
|
||||
@@ -13,56 +11,68 @@ from claude_bottle.backend.docker.egress_apply import (
|
||||
_pipelock_safe_hosts,
|
||||
validate_routes_content,
|
||||
)
|
||||
from claude_bottle.yaml_subset import parse_yaml_subset
|
||||
|
||||
|
||||
# YAML fixtures matching the hand-rolled `_render_routes_payload`
|
||||
# shape. Per-test custom shapes are spelled inline; these are the
|
||||
# common ones.
|
||||
_ROUTES_EMPTY = "routes: []\n"
|
||||
_ROUTES_ONE = 'routes:\n - host: "api.anthropic.com"\n'
|
||||
|
||||
|
||||
def _routes(parsed: str) -> list[dict]:
|
||||
"""Parse a YAML routes string and pull out the routes list, so
|
||||
tests can assert on shape directly."""
|
||||
return parse_yaml_subset(parsed)["routes"]
|
||||
|
||||
|
||||
class TestValidateRoutesContent(unittest.TestCase):
|
||||
def test_accepts_minimal_route_table(self):
|
||||
validate_routes_content('{"routes": []}')
|
||||
validate_routes_content(
|
||||
'{"routes": [{"host": "api.github.com"}]}'
|
||||
)
|
||||
validate_routes_content(_ROUTES_EMPTY)
|
||||
validate_routes_content(_ROUTES_ONE)
|
||||
|
||||
def test_accepts_full_route(self):
|
||||
validate_routes_content(
|
||||
'{"routes": [{"host": "api.github.com",'
|
||||
' "path_allowlist": ["/repos/x/"],'
|
||||
' "auth_scheme": "Bearer",'
|
||||
' "token_env": "EGRESS_TOKEN_0"}]}'
|
||||
'routes:\n'
|
||||
' - host: "api.github.com"\n'
|
||||
' auth_scheme: "Bearer"\n'
|
||||
' token_env: "EGRESS_TOKEN_0"\n'
|
||||
' path_allowlist:\n'
|
||||
' - "/repos/x/"\n'
|
||||
)
|
||||
|
||||
def test_rejects_bad_json(self):
|
||||
def test_rejects_bad_yaml(self):
|
||||
with self.assertRaises(EgressApplyError) as cm:
|
||||
validate_routes_content("{not json")
|
||||
validate_routes_content("routes:\n\t- host: x\n")
|
||||
self.assertIn("not valid", str(cm.exception))
|
||||
|
||||
def test_rejects_non_object_top_level(self):
|
||||
with self.assertRaises(EgressApplyError):
|
||||
validate_routes_content("[]")
|
||||
|
||||
def test_rejects_missing_routes_key(self):
|
||||
with self.assertRaises(EgressApplyError):
|
||||
validate_routes_content('{"other": []}')
|
||||
validate_routes_content("other: []\n")
|
||||
|
||||
def test_rejects_non_list_routes(self):
|
||||
with self.assertRaises(EgressApplyError):
|
||||
validate_routes_content('{"routes": "not a list"}')
|
||||
validate_routes_content('routes: "not a list"\n')
|
||||
|
||||
def test_rejects_partial_auth_pair(self):
|
||||
# The addon-core parser enforces both-or-neither — the apply
|
||||
# path picks this up before SIGHUP'ing the sidecar.
|
||||
with self.assertRaises(EgressApplyError):
|
||||
validate_routes_content(
|
||||
'{"routes": [{"host": "x.example",'
|
||||
' "auth_scheme": "Bearer"}]}'
|
||||
'routes:\n'
|
||||
' - host: "x.example"\n'
|
||||
' auth_scheme: "Bearer"\n'
|
||||
)
|
||||
|
||||
|
||||
class TestHostsInRoutes(unittest.TestCase):
|
||||
def test_extracts_each_unique_host(self):
|
||||
hosts = _hosts_in_routes(
|
||||
'{"routes": [{"host": "api.github.com"},'
|
||||
' {"host": "github.com"},'
|
||||
' {"host": "api.anthropic.com"}]}'
|
||||
'routes:\n'
|
||||
' - host: "api.github.com"\n'
|
||||
' - host: "github.com"\n'
|
||||
' - host: "api.anthropic.com"\n'
|
||||
)
|
||||
# Sorted+deduped.
|
||||
self.assertEqual(
|
||||
@@ -72,28 +82,34 @@ class TestHostsInRoutes(unittest.TestCase):
|
||||
|
||||
def test_dedupes_same_host(self):
|
||||
hosts = _hosts_in_routes(
|
||||
'{"routes": [{"host": "x.example", "path_allowlist": ["/a/"]},'
|
||||
' {"host": "x.example", "path_allowlist": ["/b/"]}]}'
|
||||
'routes:\n'
|
||||
' - host: "x.example"\n'
|
||||
' path_allowlist:\n'
|
||||
' - "/a/"\n'
|
||||
' - host: "x.example"\n'
|
||||
' path_allowlist:\n'
|
||||
' - "/b/"\n'
|
||||
)
|
||||
self.assertEqual(["x.example"], hosts)
|
||||
|
||||
def test_empty_routes_returns_empty(self):
|
||||
self.assertEqual([], _hosts_in_routes('{"routes": []}'))
|
||||
self.assertEqual([], _hosts_in_routes(_ROUTES_EMPTY))
|
||||
|
||||
def test_invalid_routes_raises(self):
|
||||
# The mirror helper relies on parsing succeeding; bad input
|
||||
# should error before pipelock is touched.
|
||||
with self.assertRaises(EgressApplyError):
|
||||
_hosts_in_routes('{"routes": [{"path": "/no-host/"}]}')
|
||||
_hosts_in_routes(
|
||||
'routes:\n - path_allowlist:\n - "/no-host/"\n'
|
||||
)
|
||||
|
||||
|
||||
class TestMergeSingleRoute(unittest.TestCase):
|
||||
BASE = '{"routes": [{"host": "api.anthropic.com"}]}'
|
||||
BASE = _ROUTES_ONE
|
||||
|
||||
def test_appends_route_when_host_absent(self):
|
||||
merged = _merge_single_route(self.BASE, {"host": "github.com"})
|
||||
routes = json.loads(merged)["routes"]
|
||||
hosts = [r["host"] for r in routes]
|
||||
hosts = [r["host"] for r in _routes(merged)]
|
||||
self.assertEqual(["api.anthropic.com", "github.com"], hosts)
|
||||
|
||||
def test_appends_path_allowlist(self):
|
||||
@@ -101,7 +117,7 @@ class TestMergeSingleRoute(unittest.TestCase):
|
||||
self.BASE,
|
||||
{"host": "github.com", "path_allowlist": ["/repos/x/"]},
|
||||
)
|
||||
new_route = json.loads(merged)["routes"][-1]
|
||||
new_route = _routes(merged)[-1]
|
||||
self.assertEqual(["/repos/x/"], new_route["path_allowlist"])
|
||||
|
||||
def test_appends_auth_with_token_env_slot(self):
|
||||
@@ -112,72 +128,80 @@ class TestMergeSingleRoute(unittest.TestCase):
|
||||
"auth": {"scheme": "Bearer", "token_ref": "GH"},
|
||||
},
|
||||
)
|
||||
new_route = json.loads(merged)["routes"][-1]
|
||||
new_route = _routes(merged)[-1]
|
||||
self.assertEqual("Bearer", new_route["auth_scheme"])
|
||||
# First auth slot when no prior auth routes exist.
|
||||
self.assertEqual("EGRESS_TOKEN_0", new_route["token_env"])
|
||||
|
||||
def test_auth_slot_increments_past_existing(self):
|
||||
base = json.dumps({"routes": [
|
||||
{"host": "api.anthropic.com",
|
||||
"auth_scheme": "Bearer",
|
||||
"token_env": "EGRESS_TOKEN_0"},
|
||||
]})
|
||||
base = (
|
||||
'routes:\n'
|
||||
' - host: "api.anthropic.com"\n'
|
||||
' auth_scheme: "Bearer"\n'
|
||||
' token_env: "EGRESS_TOKEN_0"\n'
|
||||
)
|
||||
merged = _merge_single_route(base, {
|
||||
"host": "api.github.com",
|
||||
"auth": {"scheme": "Bearer", "token_ref": "GH"},
|
||||
})
|
||||
new_route = json.loads(merged)["routes"][-1]
|
||||
new_route = _routes(merged)[-1]
|
||||
self.assertEqual("EGRESS_TOKEN_1", new_route["token_env"])
|
||||
|
||||
def test_existing_host_merges_path_allowlist_as_union(self):
|
||||
base = json.dumps({"routes": [
|
||||
{"host": "github.com", "path_allowlist": ["/a/"]},
|
||||
]})
|
||||
base = (
|
||||
'routes:\n'
|
||||
' - host: "github.com"\n'
|
||||
' path_allowlist:\n'
|
||||
' - "/a/"\n'
|
||||
)
|
||||
merged = _merge_single_route(base, {
|
||||
"host": "github.com",
|
||||
"path_allowlist": ["/b/"],
|
||||
})
|
||||
routes = json.loads(merged)["routes"]
|
||||
routes = _routes(merged)
|
||||
self.assertEqual(1, len(routes)) # not duplicated
|
||||
self.assertEqual(["/a/", "/b/"], routes[0]["path_allowlist"])
|
||||
|
||||
def test_existing_host_dedup_path_allowlist(self):
|
||||
base = json.dumps({"routes": [
|
||||
{"host": "github.com", "path_allowlist": ["/a/"]},
|
||||
]})
|
||||
base = (
|
||||
'routes:\n'
|
||||
' - host: "github.com"\n'
|
||||
' path_allowlist:\n'
|
||||
' - "/a/"\n'
|
||||
)
|
||||
merged = _merge_single_route(base, {
|
||||
"host": "github.com",
|
||||
"path_allowlist": ["/a/", "/b/"],
|
||||
})
|
||||
self.assertEqual(
|
||||
["/a/", "/b/"],
|
||||
json.loads(merged)["routes"][0]["path_allowlist"],
|
||||
_routes(merged)[0]["path_allowlist"],
|
||||
)
|
||||
|
||||
def test_existing_host_preserves_existing_auth_ignores_proposed(self):
|
||||
# Tool docs: auth on an existing host is operator-controlled,
|
||||
# not agent-controlled. The merge must not overwrite.
|
||||
base = json.dumps({"routes": [
|
||||
{"host": "api.github.com",
|
||||
"auth_scheme": "Bearer",
|
||||
"token_env": "EGRESS_TOKEN_0"},
|
||||
]})
|
||||
base = (
|
||||
'routes:\n'
|
||||
' - host: "api.github.com"\n'
|
||||
' auth_scheme: "Bearer"\n'
|
||||
' token_env: "EGRESS_TOKEN_0"\n'
|
||||
)
|
||||
merged = _merge_single_route(base, {
|
||||
"host": "api.github.com",
|
||||
"auth": {"scheme": "token", "token_ref": "DIFFERENT"},
|
||||
})
|
||||
route = json.loads(merged)["routes"][0]
|
||||
route = _routes(merged)[0]
|
||||
self.assertEqual("Bearer", route["auth_scheme"])
|
||||
self.assertEqual("EGRESS_TOKEN_0", route["token_env"])
|
||||
|
||||
def test_host_match_is_case_insensitive(self):
|
||||
base = json.dumps({"routes": [{"host": "GitHub.com"}]})
|
||||
base = 'routes:\n - host: "GitHub.com"\n'
|
||||
merged = _merge_single_route(base, {
|
||||
"host": "github.com",
|
||||
"path_allowlist": ["/x/"],
|
||||
})
|
||||
routes = json.loads(merged)["routes"]
|
||||
routes = _routes(merged)
|
||||
self.assertEqual(1, len(routes))
|
||||
self.assertEqual(["/x/"], routes[0]["path_allowlist"])
|
||||
|
||||
@@ -187,7 +211,7 @@ class TestMergeSingleRoute(unittest.TestCase):
|
||||
|
||||
def test_invalid_current_yaml_raises(self):
|
||||
with self.assertRaises(EgressApplyError):
|
||||
_merge_single_route("{not json", {"host": "x.example"})
|
||||
_merge_single_route("routes:\n\tbad", {"host": "x.example"})
|
||||
|
||||
|
||||
class TestPipelockSafeHosts(unittest.TestCase):
|
||||
|
||||
@@ -5,7 +5,7 @@ actually use, and every rejection case the PRD enumerates."""
|
||||
import textwrap
|
||||
import unittest
|
||||
|
||||
from claude_bottle.log import Die
|
||||
from claude_bottle.yaml_subset import YamlSubsetError
|
||||
from claude_bottle.yaml_subset import parse_frontmatter, parse_yaml_subset
|
||||
|
||||
|
||||
@@ -60,7 +60,7 @@ class TestForbiddenBoolLikes(unittest.TestCase):
|
||||
"""Ambiguous bool-ish tokens have to be quoted explicitly."""
|
||||
|
||||
def _expect_die(self, src: str):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y(src)
|
||||
|
||||
def test_yes_dies(self):
|
||||
@@ -81,7 +81,7 @@ class TestForbiddenBoolLikes(unittest.TestCase):
|
||||
|
||||
class TestForbiddenScalarShapes(unittest.TestCase):
|
||||
def _expect_die(self, src: str):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y(src)
|
||||
|
||||
def test_bare_date_dies(self):
|
||||
@@ -120,14 +120,14 @@ class TestMapping(unittest.TestCase):
|
||||
self.assertEqual({"outer": {"inner": "hello", "other": 5}}, out)
|
||||
|
||||
def test_duplicate_key_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y("""
|
||||
a: 1
|
||||
a: 2
|
||||
""")
|
||||
|
||||
def test_key_must_be_bare_identifier(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y('"weird key": 1\n')
|
||||
|
||||
|
||||
@@ -202,20 +202,20 @@ class TestInline(unittest.TestCase):
|
||||
)
|
||||
|
||||
def test_nested_flow_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y("l: [[1, 2], [3, 4]]\n")
|
||||
|
||||
|
||||
class TestForbiddenConstructs(unittest.TestCase):
|
||||
def test_anchor_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y("""
|
||||
a: &anchor 1
|
||||
b: *anchor
|
||||
""")
|
||||
|
||||
def test_multiline_block_scalar_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y("""
|
||||
k: |
|
||||
line 1
|
||||
@@ -223,11 +223,11 @@ class TestForbiddenConstructs(unittest.TestCase):
|
||||
""")
|
||||
|
||||
def test_tag_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y("k: !!str hello\n")
|
||||
|
||||
def test_tab_in_indent_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
_y("a:\n\tb: 1\n")
|
||||
|
||||
|
||||
@@ -306,7 +306,7 @@ class TestFrontmatter(unittest.TestCase):
|
||||
self.assertEqual(text, body)
|
||||
|
||||
def test_unclosed_frontmatter_dies(self):
|
||||
with self.assertRaises(Die):
|
||||
with self.assertRaises(YamlSubsetError):
|
||||
parse_frontmatter("---\nbottle: dev\nno closing")
|
||||
|
||||
def test_body_preserves_blank_lines(self):
|
||||
|
||||
Reference in New Issue
Block a user