refactor(egress): provisioned-wins merge + _route_to_yaml_fields (PRD 0031)
Replace _merge_provider_route's five-case nested conditional with a flat provisioned-wins merge: provider routes claim their hosts outright, manifest routes for unclaimed hosts append unchanged. Token slot assignment moves to a single _assign_token_slots pass over the merged list. Add _route_to_yaml_fields as the single authoritative EgressRoute→YAML mapping, eliminating the risk of EgressRoute and egress_addon_core.Route silently drifting apart when new fields are added. egress_manifest_routes is now a pure lifter with no slot assignment. _merge_provider_route and _find_or_alloc_token_env are removed. Tests updated: conflict-die case removed, upgrade-bare replaced with provider-wins semantics, slot-assignment tests moved to TestSlotAssignment.
This commit is contained in:
+68
-133
@@ -24,6 +24,7 @@ flow (PRD 0014) at egress and renames the MCP tool.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import dataclasses
|
||||
from abc import ABC, abstractmethod
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
@@ -141,43 +142,20 @@ class EgressPlan:
|
||||
def egress_manifest_routes(
|
||||
bottle: Bottle,
|
||||
) -> tuple[EgressRoute, ...]:
|
||||
"""Lift each `bottle.egress.routes[]` manifest entry into a
|
||||
resolved EgressRoute. Order is preserved so route lookup at
|
||||
the proxy is stable.
|
||||
|
||||
Token-env slots are assigned per distinct `token_ref`: the first
|
||||
authenticated route with `token_ref` "GH_PAT" gets
|
||||
`EGRESS_TOKEN_0`; a second route with the same `token_ref`
|
||||
shares slot 0. Unauthenticated routes (`auth` omitted) contribute
|
||||
no slot.
|
||||
|
||||
This is the effective set the addon enforces. Provider runtime
|
||||
routes are intentionally not injected implicitly; every allowed
|
||||
host must come from the home-owned bottle manifest."""
|
||||
"""Lift each `bottle.egress.routes[]` manifest entry into an EgressRoute.
|
||||
Order is preserved. Token slots are not assigned here — slot assignment
|
||||
is a final step in `egress_routes_for_bottle` after provider and manifest
|
||||
routes are merged."""
|
||||
out: list[EgressRoute] = []
|
||||
slot_for_token: dict[str, str] = {}
|
||||
for r in bottle.egress.routes:
|
||||
if r.AuthScheme and r.TokenRef:
|
||||
token_env = slot_for_token.get(r.TokenRef)
|
||||
if token_env is None:
|
||||
token_env = f"EGRESS_TOKEN_{len(slot_for_token)}"
|
||||
slot_for_token[r.TokenRef] = token_env
|
||||
out.append(EgressRoute(
|
||||
host=r.Host,
|
||||
path_allowlist=r.PathAllowlist,
|
||||
auth_scheme=r.AuthScheme,
|
||||
token_env=token_env,
|
||||
token_ref=r.TokenRef,
|
||||
roles=r.Role,
|
||||
tls_passthrough=r.Pipelock.TlsPassthrough,
|
||||
))
|
||||
else:
|
||||
out.append(EgressRoute(
|
||||
host=r.Host,
|
||||
path_allowlist=r.PathAllowlist,
|
||||
roles=r.Role,
|
||||
tls_passthrough=r.Pipelock.TlsPassthrough,
|
||||
))
|
||||
out.append(EgressRoute(
|
||||
host=r.Host,
|
||||
path_allowlist=r.PathAllowlist,
|
||||
auth_scheme=r.AuthScheme,
|
||||
token_ref=r.TokenRef,
|
||||
roles=r.Role,
|
||||
tls_passthrough=r.Pipelock.TlsPassthrough,
|
||||
))
|
||||
return tuple(out)
|
||||
|
||||
|
||||
@@ -185,90 +163,39 @@ def egress_routes_for_bottle(
|
||||
bottle: Bottle,
|
||||
provider_routes: tuple[EgressRoute, ...] = (),
|
||||
) -> tuple[EgressRoute, ...]:
|
||||
"""Effective egress routes for the agent. This is what gets rendered
|
||||
into routes.yaml and what the addon enforces.
|
||||
"""Effective egress routes for the agent.
|
||||
|
||||
Merges manifest-declared routes with provider-owned routes. The
|
||||
manifest is the primary surface; `provider_routes` are synthesised
|
||||
by `agent_provision_plan` and may add or upgrade manifest entries.
|
||||
Provider routes that conflict with an existing authenticated manifest
|
||||
route (different auth scheme or token ref) raise a hard error."""
|
||||
routes = list(egress_manifest_routes(bottle))
|
||||
for pr in provider_routes:
|
||||
routes = _merge_provider_route(routes, pr)
|
||||
return tuple(routes)
|
||||
Provider routes own their hosts outright; manifest routes for hosts
|
||||
not claimed by any provider are appended. Token slots are assigned
|
||||
in a final pass over the merged list in order, so provisioned routes
|
||||
get the lower slot numbers."""
|
||||
manifest = egress_manifest_routes(bottle)
|
||||
provisioned_hosts = {pr.host.lower() for pr in provider_routes}
|
||||
merged = list(provider_routes) + [
|
||||
r for r in manifest if r.host.lower() not in provisioned_hosts
|
||||
]
|
||||
return _assign_token_slots(merged)
|
||||
|
||||
|
||||
def _find_or_alloc_token_env(routes: list[EgressRoute], token_ref: str) -> str:
|
||||
"""Return the existing token_env slot for `token_ref`, or allocate the next one."""
|
||||
if not token_ref:
|
||||
return ""
|
||||
for route in routes:
|
||||
if route.token_ref == token_ref and route.token_env:
|
||||
return route.token_env
|
||||
return f"EGRESS_TOKEN_{len({r.token_env for r in routes if r.token_env})}"
|
||||
def _assign_token_slots(
|
||||
routes: list[EgressRoute],
|
||||
) -> tuple[EgressRoute, ...]:
|
||||
"""Assign EGRESS_TOKEN_N slots to authenticated routes in order.
|
||||
|
||||
|
||||
def _merge_provider_route(
|
||||
routes: list[EgressRoute], pr: EgressRoute,
|
||||
) -> list[EgressRoute]:
|
||||
"""Merge one provider-declared route into the manifest route list.
|
||||
|
||||
Upgrade a bare-pass manifest route to authenticated if the provider
|
||||
declares auth for that host, or append if the host isn't in the manifest.
|
||||
Identical auth (same scheme + token_ref) on an existing route is a
|
||||
no-op, with a tls_passthrough upgrade if the provider route sets it.
|
||||
Conflicting auth (different scheme or token_ref) dies."""
|
||||
for idx, route in enumerate(routes):
|
||||
if route.host.lower() != pr.host.lower():
|
||||
continue
|
||||
if route.auth_scheme or route.token_ref:
|
||||
if route.auth_scheme == pr.auth_scheme and route.token_ref == pr.token_ref:
|
||||
if pr.tls_passthrough and not route.tls_passthrough:
|
||||
routes[idx] = EgressRoute(
|
||||
host=route.host,
|
||||
path_allowlist=route.path_allowlist,
|
||||
auth_scheme=route.auth_scheme,
|
||||
token_env=route.token_env,
|
||||
token_ref=route.token_ref,
|
||||
roles=route.roles,
|
||||
tls_passthrough=True,
|
||||
)
|
||||
return routes
|
||||
die(
|
||||
f"provider egress route for {pr.host!r} conflicts with an "
|
||||
f"authenticated manifest route (different auth scheme or token "
|
||||
f"ref). Remove the manifest route's auth block or disable the "
|
||||
f"feature that adds this provider route."
|
||||
)
|
||||
token_env = (
|
||||
_find_or_alloc_token_env(routes, pr.token_ref)
|
||||
if pr.auth_scheme and pr.token_ref
|
||||
else ""
|
||||
)
|
||||
routes[idx] = EgressRoute(
|
||||
host=route.host,
|
||||
path_allowlist=route.path_allowlist,
|
||||
auth_scheme=pr.auth_scheme,
|
||||
token_env=token_env,
|
||||
token_ref=pr.token_ref,
|
||||
roles=route.roles,
|
||||
tls_passthrough=pr.tls_passthrough,
|
||||
)
|
||||
return routes
|
||||
token_env = (
|
||||
_find_or_alloc_token_env(routes, pr.token_ref)
|
||||
if pr.auth_scheme and pr.token_ref
|
||||
else ""
|
||||
)
|
||||
routes.append(EgressRoute(
|
||||
host=pr.host,
|
||||
auth_scheme=pr.auth_scheme,
|
||||
token_env=token_env,
|
||||
token_ref=pr.token_ref,
|
||||
tls_passthrough=pr.tls_passthrough,
|
||||
))
|
||||
return routes
|
||||
Routes sharing a token_ref share a slot. Unauthenticated routes
|
||||
(no auth_scheme / token_ref) keep token_env empty."""
|
||||
slot_for_ref: dict[str, str] = {}
|
||||
out: list[EgressRoute] = []
|
||||
for r in routes:
|
||||
if r.auth_scheme and r.token_ref:
|
||||
slot = slot_for_ref.get(r.token_ref)
|
||||
if slot is None:
|
||||
slot = f"EGRESS_TOKEN_{len(slot_for_ref)}"
|
||||
slot_for_ref[r.token_ref] = slot
|
||||
out.append(dataclasses.replace(r, token_env=slot))
|
||||
else:
|
||||
out.append(r)
|
||||
return tuple(out)
|
||||
|
||||
|
||||
def egress_token_env_map(
|
||||
@@ -296,35 +223,43 @@ def egress_token_env_map(
|
||||
return out
|
||||
|
||||
|
||||
def _route_to_yaml_fields(r: EgressRoute) -> dict:
|
||||
"""Return the addon-visible fields for one route.
|
||||
|
||||
Single authoritative mapping between EgressRoute (host-side) and
|
||||
egress_addon_core.Route (sidecar-side). When a field is added to
|
||||
the addon's Route that must appear in the YAML, add it here and
|
||||
in egress_addon_core._parse_one together."""
|
||||
fields: dict = {"host": r.host}
|
||||
if r.auth_scheme and r.token_env:
|
||||
fields["auth_scheme"] = r.auth_scheme
|
||||
fields["token_env"] = r.token_env
|
||||
if r.path_allowlist:
|
||||
fields["path_allowlist"] = list(r.path_allowlist)
|
||||
return fields
|
||||
|
||||
|
||||
def egress_render_routes(
|
||||
routes: tuple[EgressRoute, ...],
|
||||
) -> str:
|
||||
"""Serialize the route table for the addon to read.
|
||||
|
||||
YAML content — no token values, no host env-var names. The only
|
||||
thing the addon needs at runtime is the host → path_allowlist
|
||||
+ auth_scheme + in-container env-var mapping. The actual token
|
||||
values arrive via the container's environ.
|
||||
|
||||
Authenticated routes carry `auth_scheme` + `token_env`;
|
||||
unauthenticated routes omit both keys (the addon's parser
|
||||
enforces both-or-neither). Hand-rolled YAML in the style of
|
||||
`pipelock_render_yaml` so the addon's parser
|
||||
(`yaml_subset.parse_yaml_subset`) round-trips it cleanly."""
|
||||
YAML content — no token values, no host env-var names. Fields are
|
||||
determined by `_route_to_yaml_fields`, which is the single point of
|
||||
truth for the EgressRoute → egress_addon_core.Route mapping."""
|
||||
lines: list[str] = ["routes:"]
|
||||
if not routes:
|
||||
# `routes:` with an empty list on the same line — the parser
|
||||
# needs SOMETHING here. Empty inline list is the cleanest.
|
||||
lines[0] = "routes: []"
|
||||
return "\n".join(lines) + "\n"
|
||||
for r in routes:
|
||||
lines.append(f' - host: "{r.host}"')
|
||||
if r.auth_scheme and r.token_env:
|
||||
lines.append(f' auth_scheme: "{r.auth_scheme}"')
|
||||
lines.append(f' token_env: "{r.token_env}"')
|
||||
if r.path_allowlist:
|
||||
f = _route_to_yaml_fields(r)
|
||||
lines.append(f' - host: "{f["host"]}"')
|
||||
if "auth_scheme" in f:
|
||||
lines.append(f' auth_scheme: "{f["auth_scheme"]}"')
|
||||
lines.append(f' token_env: "{f["token_env"]}"')
|
||||
if "path_allowlist" in f:
|
||||
lines.append(" path_allowlist:")
|
||||
for p in r.path_allowlist:
|
||||
for p in f["path_allowlist"]:
|
||||
lines.append(f' - "{p}"')
|
||||
return "\n".join(lines) + "\n"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user