Method note, up front, because it changes how you should read this. I did not read atenet line by line. I pointed an AI agent at Agent Substrate’s networking code and had it read the whole thing in seven passes, each with a different question, then had a second round of agents try to refute everything the first round produced. Eight findings did not survive that; four of them are quoted at the bottom. Every citation below resolves to a file and line at a named commit, which is the only reason a list produced this way is worth publishing: you can check any of it yourself, and I would rather you did than take it on trust.
Agent Substrate multiplexes many agent “actors” onto a smaller pool of already-warm worker pods, which is the whole trick: it keeps the Kubernetes control plane out of the request path so a request can reach a suspended agent in milliseconds rather than seconds. atenet is the part that gets traffic there. It is an Envoy xDS control plane, an ext_proc server that resolves which worker currently holds a given actor and resumes it if nobody does, and a second gateway on the way back out that authenticates the actor behind every outbound CONNECT.
It is a genuinely good piece of engineering, and the comments in it are unusually honest — several of the items below are things the code already says about itself in a TODO. What follows is the engineering half of what the scan turned up: 52 items about correctness, capacity, operations, observability, and test coverage.
There is a security half too. It is not in this post. The repo ships a SECURITY.md asking for private reports through GitHub advisories and asking reporters to hold details until a fix lands, and it seemed poor form to read that file and then ignore it. Those findings are going through that channel. If they get advisories, I’ll write them up then.
The shape of it
| Read at | agent-substrate/substrate @ 69828945 |
| Surface | cmd/atenet, both gateway manifests, the agentgateway overlays, internal/atunnel, the EgressPolicy chain |
| Method | 7 reading passes, then adversarial verification of every finding |
| Findings | 129 raised, 121 survived verification, deduplicated into 76 items |
| Items here | 52 — 6 P0, 29 P1, 17 P2 |
Already TODO’d in-tree | 6 |
| Held for private disclosure | 24 |
| Tests | go test ./cmd/atenet/... ./internal/atunnel/... passes clean |
Three of the findings are worth walking through, less because they are the most severe than because each one is a small mechanical thing standing in for a larger habit — and because they are the three that were re-checked directly against the code, rather than taken on the agent’s word.
The line that was never backported
Substrate ships two egress manifests. atenet-egress.yaml is the default; atenet-egress-with-sdsmint.yaml is the variant that terminates and re-originates TLS so it can see inside the tunnel. They are siblings, and they drifted.
The sdsmint one sets this on its CONNECT route:
- match: { connect_matcher: {} }
route:
# ...five lines of comment about the MITM listener...
cluster: mitm_internal
# Set request timeout to disabled (0) as CONNECT is a long-running stream.
# There will still be a stream_idle_timeout (5 minutes by default).
timeout: 0s
upgrade_configs:
- upgrade_type: CONNECT
connect_config: {}
The default one does not:
- match: { connect_matcher: {} }
route:
cluster: egress_forward_proxy
upgrade_configs:
- upgrade_type: CONNECT
connect_config: {}
Envoy applies the route timeout to a CONNECT tunnel’s entire lifetime, not just to its headers. Left at the 15-second default, that is not a timeout on a slow request — it is a hard cap on how long any actor’s outbound connection may exist. Server-sent events, gRPC streaming, an SSH session, a slow model call, any download over fifteen seconds: all cut, mid-stream, on the default install path.
The router’s own xDS code gets this right for the ingress CONNECT listener, and says why:
// Envoy applies the route timeout to a CONNECT tunnel's whole
// lifetime, not just its headers; left at Envoy's 15s default
// this would silently kill every tunnel after 15s. 0 disables
// it -- the tunnel's lifetime is bounded by idle timeout,
// drain, and the peers themselves.
Timeout: durationpb.New(0),
So the knowledge is in the tree three times — twice in prose, once in Go — and missing from the one file hack/install-ate.sh actually applies. This is the cheapest fix on the list and the one I would do first. The durable version of it is an end-to-end test that holds a tunnel open past twenty seconds and asserts bytes still flow, so the two manifests cannot drift apart again quietly.
This is also the kind of finding the method is genuinely good at. Nobody reading either file alone would notice; the bug exists only in the diff between two files nobody diffs.
A reconcile that lists, then throws it away
The router keeps Envoy configured through a loop that ticks every five seconds:
func (c *Controller) reconcile(ctx context.Context) error {
_, err := c.atStore.readyTemplates(ctx)
if err != nil { /* ... */ }
if err := c.xdsSrv.UpdateSnapshot(); err != nil { /* ... */ }
return nil
}
Two things are happening here, and neither is doing what it looks like.
readyTemplates is an unpaginated, cluster-wide List of every ActorTemplate, and the result is assigned to _. It is also not served from an informer cache — the client is built with client.New, which is the direct, uncached one — so this is a real API-server round trip. Nothing in the generated xDS resources derives from ActorTemplate at all; the only genuine consumer is the /statusz page, which does its own fetch.
UpdateSnapshot then increments the version counter unconditionally and calls SetSnapshot. go-control-plane pushes on a version change, so Envoy re-ingests the full listener, cluster, route and secret set every five seconds, forever, whether or not a single byte differs. On an idle cluster that is roughly seventeen thousand pushes and seventeen thousand “Deploying updated xDS configuration snapshot” log lines per replica per day.
The obvious reason someone might have written it this way is that the periodic push refreshes the TLS material. It doesn’t: the secret is delivered as a DataSource_Filename with a WatchedDirectory, so Envoy re-reads the file on its own when kubelet rotates it. The push is not load-bearing. That check is the kind of thing the verification round is for — the first pass reported the churn, the second pass went looking for the reason it might be deliberate, and not finding one is what makes the finding worth acting on.
The fix is to hash the built resource set and skip the version bump when nothing changed, and to delete the discarded List. What makes it worth writing down is the test that goes with it, which is one line of intent: two back-to-back reconciles should produce exactly one SetSnapshot.
The benchmark measures a different gateway
The shipped ingress Deployment starts Envoy like this:
- "--component-log-level"
- "upstream:debug,router:debug,ext_proc:debug"
No comment, no time-box, no overlay that removes it. Those are three of Envoy’s hottest components, each emitting several lines per request on the data path, and ext_proc debug output puts request headers — actor hostnames included — into cluster logs by default. The egress Envoy, running the same binary, carries no such flag.
The part worth stopping on is in the load-test harness: before it measures anything, nighthawk_ingress.py strategic-patches the Envoy container’s entire command to remove those flags — and documents why, in the docstring of the function that does it. So the harness is honest about measuring something other than the default. What nobody says is the corollary: the configuration that actually ships, debug logging and all, has never been benchmarked, and the manifest carries no note to that effect.
It sits next to two other manifest gaps that read the same way — as things nobody decided, rather than things somebody chose. The ingress Envoy container has no readiness, liveness, or startup probe of any kind, while its egress twin has two; and neither gateway sets resource requests or limits, which puts both in BestEffort QoS, first to be evicted under node pressure. In each case the sibling manifest already demonstrates the intended shape. That is what makes them cheap: there is nothing to design, only something to copy.
The list
Priorities and effort estimates below are the scan’s, not a maintainer’s. P0 means it would not ship a cluster without it; P1 means it will hurt within a quarter; P2 means it is worth doing when the file is open anyway. Items marked tracked already have a TODO or roadmap entry in-tree — kept in the list because a TODO is a note to self, not a plan, and several of these have been notes to self for a while.
Stop pushing an identical xDS snapshot every 5s, and drop the discarded ActorTemplate LIST
P1Mcontroller.go:49,64-67 · atstore.go:41-44 · router.go:106-108 · xds.go:418,481
reconcile does _, err := c.atStore.readyTemplates(ctx) — an unpaginated, cluster-wide LIST on an uncached client, result discarded — then bumps versionCount++ unconditionally. About 17k pushes and log lines per replica per day on a fully idle cluster. Nothing in the generated resources derives from ActorTemplates, and cert rotation does not depend on the push.
DoHash the built resource set and skip SetSnapshot when unchanged; delete the readyTemplates call or move it behind an informer. Test: two back-to-back reconciles produce exactly one SetSnapshot.
Surface xDS NACKs — add callbacks, a logger, and freshness metrics
P1Mxds.go:207-208 — NewSnapshotCache(true, IDHash{}, nil) and NewServer(ctx, cache, nil)
Nil logger, nil callbacks. A rejected Listener (unreadable cert path, an SDS secret not yet projected, an unknown extension) leaves Envoy on stale or empty config while the router logs the push as a success. Nothing in /statusz distinguishes accepted from rejected, and no atenet.router.xds.* instrument exists.
DoPass a slog logger and serverv3.CallbackFuncs; log and count req.GetErrorDetail() in OnStreamRequest. Export an xDS NACK counter, snapshot-version gauge, and last-accepted-age gauge. Add a --xds-node-id flag — the magic string substrate-envoy-node must match the manifest with no compile-time link.
Add slowloris and connection-lifetime bounds to the ingress listeners
P1Mxds.go:1060-1087, :908-949, :981
Neither HCM sets RequestTimeout, an explicit StreamIdleTimeout, CommonHttpProtocolOptions, or MaxConcurrentStreams. Nothing bounds accept→complete-headers, so a slow-drip client pins a connection and a stream indefinitely and the route timeout never arms. Note: per_connection_buffer_limit_bytes (1 MiB) and stream_idle_timeout (5m) already have safe defaults — the real gaps are the header-phase timer, the stream cap, and connection lifetime.
DoSet RequestTimeout (a few seconds), an explicit StreamIdleTimeout, CommonHttpProtocolOptions{IdleTimeout, MaxConnectionDuration, MaxRequestsPerConnection}, and Http2ProtocolOptions{MaxConcurrentStreams ~100}. Expose as flags beside --route-timeout.
Replace the default Envoy access-log format with a structured one
P1Strackedxds.go:1039 (bare StdoutAccessLog), reused at :916-923, under the TODO at :914-915
The default format carries no %RESPONSE_CODE_DETAILS%, %CONNECTION_TERMINATION_DETAILS%, or %UPSTREAM_TRANSPORT_FAILURE_REASON% — so a parking shed 503, a circuit-breaker UO 503, an ext_proc message timeout, an atunnel mTLS handshake failure, and a route-timeout 504 are all one indistinguishable line. The egress manifest already proves the pattern is acceptable here.
DoAdd a shared SubstitutionFormatString json_format applied in both HCM builders. Closes the existing TODO.
Give the actor ORIGINAL_DST cluster explicit circuit breakers and a cleanup_interval
P1Sxds.go:745-786 (buildOriginalDstCluster) — only ConnectTimeout, LbConfig, TransportSocket, protocol options
The cluster carrying all actor traffic runs on Envoy's implicit 1024 max_connections / max_requests, cluster-wide rather than per actor — the 1025th request gets an opaque UO 503 from a limit no flag names, while the deliberately-sized ext_proc breaker above it is 2048. Separately, cleanup_interval defaults to 5s for ORIGINAL_DST, so every actor idle >5s pays a fresh TCP connect plus a full mTLS handshake — the common case for a many-intermittent-actors substrate.
DoSet CleanupInterval: 60s and explicit CircuitBreakers, both flag-driven, validated against --extproc-max-requests the way --parked-request-max already is.
Decouple the ext_proc message timeout from the parking budget
P1Sdataplane.go:73-77 · xds.go:1005,1013 · ingress/resumer.go:38
SetExtProcMessageTimeout(budget + 5s) runs only when parking is enabled, and lands in both GrpcService.Timeout and MessageTimeout. So with parking on, the millisecond-scale fast path inherits a 10s+ timeout; with --parked-request-max=0 the timeout stays 5s while failFastResumeBudget is 15s — any cold restore over 5s is abandoned by Envoy and, being fail-closed, the client gets a generic 500 instead of the router's mapped 503.
DoKeep MessageTimeout at the default; set MaxMessageTimeout to max(budget, failFastResumeBudget) + margin and emit override_message_timeout from the ingress handler at the moment it parks. Set FailureModeAllow: false explicitly.
Add a retry policy and edge normalization to the actor route
P2Sxds.go:802-814 — no RetryPolicy anywhere in cmd/atenet/internal/router/
A closed idle pooled mTLS connection surfaces as an immediate 503 UC/UF even though the request never reached the actor — common while cleanup_interval stays at 5s, so fix 1.5 first. Separately no NormalizePath, MergeSlashes, or PathWithEscapedSlashesAction: the gateway doesn't route on path, so this isn't a bypass — it just means every actor inherits unnormalized paths for its own authorization decisions.
DoFlag-gated RetryPolicy (connect-failure,refused-stream,reset-before-request-headers-sent), off by default for non-idempotent workloads. Set NormalizePath, MergeSlashes, HeadersWithUnderscoresAction: REJECT_REQUEST.
Validate generated resources, and stop discarding the marshal error
P2Mxds.go:465-481 (only snapshot.Consistent()) · newAny at :1344-1347
Consistent() validates name references, not resource validity; the vendored PGV Validate() is unused and no CI step runs envoy --mode validate. newAny swallows the anypb.New error at 17 call sites, so a nil typed_config would ship, pass Consistent(), log as success, and be NACKed invisibly.
DoAdd a CI check rendering the snapshot into a bootstrap and running envoy --mode validate against the pinned image — that is the valuable half. Call ValidateAll() before SetSnapshot; make newAny return an error or panic at snapshot-build time.
Add HTTP/2 keepalive to the ext_proc and OTLP clusters
P2Sxds.go:560-568, :608-616 — Http2ProtocolOptions{} with no ConnectionKeepalive
Weakest for ext_proc (a single loopback endpoint); genuinely useful for buildOtlpCollectorCluster, which is STRICT_DNS to an off-pod collector where a silently dropped connection through a NAT or LB blackholes spans until TCP notices.
DoSet ConnectionKeepalive{Interval, Timeout} on both.
Don't treat the :authority port as the actor target port on plain ingress
P1Mingress/ingress.go:110-117 · xds.go:876-905, wired at :1146 and :1209
targetPort is derived from the authority unconditionally, and the same dev.ate.authority filter state feeds every listener — nothing distinguishes CONNECT-terminated traffic. The sharp case: grpc-go sets :authority to the dial target including the port, so a client dialing …:443 at the HTTPS ingress gets targetPort=443 and atunnel proxies to 443 on the actor instead of 80 — connection refused, nothing in the logs naming the cause. Separately SplitHostPort("host:abc") succeeds, ParsePort rejects, and the code silently falls back to 80.
DoHonour the authority port only for CONNECT-terminated traffic (a marker in filter state, or key off :method); return a 400 when a port component is present but unparseable. Add test cases for :0, :99999, :abc, and a plain-ingress :8080 — today there is not one authority-with-port case in the table.
Add a per-actor cap to the parking lot
P1Mparking.go:163-176 (enter takes no actor identity) · ingress.go:124 · parking.go:37
The only state is l.active vs l.cfg.Max (default 1024). One saturated actor — or one client hammering one name — can hold every slot for the full budget, after which every other actor is shed with "router at capacity". Singleflight does not help: it coalesces the ateapi calls but every joined request still holds its own slot. The ext_proc breaker above it is a strictly larger global bound, not a per-actor one.
DoChange to enter(ctx, actorRef) with a map[ActorRef]int under the same mutex; reject on either the global Max or a new --parked-request-max-per-actor. Label the rejected counter with which limit fired. Test: saturating actor A still admits actor B.
Bound concurrent detached ResumeActor flights
P1Mresumer.go:183,192,202 · ingress.go:130-131 · ateinterceptors.go:78-83
The flight context is built from context.Background(), cancellation is stripped, and the RPC runs with no deadline; the parking slot is released the moment the caller stops waiting. The only ceiling is ateapi's server-side 10-minute max deadline. So --parked-request-max bounds concurrent callers, not concurrent flights — under churn across distinct cold actors, in-flight RPCs accumulate at admission rate × 10 min.
DoAdd a semaphore held for the whole flight lifetime capping concurrent detached ResumeActor RPCs. Do not add a per-attempt deadline alone — that reintroduces the stranded-worker bug (#675) the code deliberately avoids.
Propagate trace context into ResumeActor
P1Sresumer.go:167-169 starts the span from the caller ctx; :183 roots the flight at context.Background()
The otelgrpc-instrumented client emits every outbound ResumeActor span as a disconnected trace root, and baggage is dropped. The single most valuable trace — "why did this request take 4 seconds?" — is split in two, and it looks correct in a trace UI because the router-local span is parented fine.
DobgCtx := context.WithTimeout(context.WithoutCancel(ctx), r.budget), matching the stated intent in the comment above it, keeping the context.WithoutCancel at :192 — it strips the deadline as well as cancellation, which is what stops the budget from cancelling an in-flight ResumeActor (#675). Add a span link or joiner-count attribute since the leader's span is shared.
Demote the four per-request INFO log lines
P2Singress/ingress.go:87, 129, 145, 159 · egress.go:127
Four slog.InfoContext records per successful request (three via reflective slog.Any), JSON-encoded through one shared stdout writer at the default level, duplicating what the route-duration metric, the trace, the /statusz recorder, and Envoy's own access log already carry. Line :87 logs raw client-controlled md.Host at INFO for scanner traffic.
DoDemote :87/:129/:145 to Debug, keep at most one INFO, replace slog.Any with explicit slog.String. Update TestHandleRequestHeadersDoesNotLogSensitiveData alongside — it pins what these lines may contain.
Give invalid-host rejections their own metric outcome
P2Singress/errors.go:36-39 · ingress.go:100-107
Both the missing-attribute case and the unparseable-DNS-name case return NotFound, which classifyOutcome maps to not_found — the same label a genuine ateapi NotFound produces. The valuable half: dev.ate.authority is set on every ingress listener, so an empty attribute can only mean a broken dataplane config that would silently 404 all traffic — today indistinguishable from scanner noise.
DoTyped sentinels so classifyOutcome emits invalid_host and missing_authority_attribute separately; alert on the latter > 0.
Fix the retryable-code table in docs/request-parking.md
P2Sdocs/request-parking.md:101, :109, :116 vs ingress/resumer.go:143-161
The doc names FailedPrecondition as the worker-pool-saturation code; the real capacity code is ResourceExhausted. An on-call engineer building a dashboard filter from the table sees nothing. Lines :23-24 and :33-34 are correct — only the table and :116 are wrong.
DoAdd a ResourceExhausted (no free workers) row, retitle the FailedPrecondition row, fix :116.
Add timeout: 0s to the default egress CONNECT route
P0Smanifests/ate-install/atenet-egress.yaml:107-112 — no route timeout anywhere in the HCM (78-155)
Envoy's 15s route timeout kills every actor egress tunnel 15 seconds after establishment regardless of activity: long-poll, SSE, gRPC streaming, slow LLM calls, SSH, any download over 15s. This is drift, not a design difference — commit 6fd47c13 added timeout: 0s with the rationale "the route timeout runs until the response is completely processed, which for a stream is never" to the sdsmint manifest only, never backported. hack/install-ate.sh:243-249 confirms atenet-egress.yaml is the default.
DoOne line. Add an e2e holding a tunnel past 20s and asserting bytes still flow, so the two manifests cannot drift again.
Make the actor-identity CA trust bundle reloadable
P1Mtrackedrouter.go:238-249 (one-shot os.ReadFile) · atenet-egress.yaml:70-76 (static trusted_ca) · internal/localca/localca.go:56-64
Nothing re-reads the bundle, and the Envoy side uses a static validation_context while the serving cert ten lines above deliberately goes through filesystem SDS with a watched_directory. localca is built around seamless publish-new-root / age-in / switch rotation — the egress path defeats it: the moment a new CA becomes ActiveForSigning, every fresh actor certificate fails both the handshake and leaf.Verify, and all actor egress in the cluster stops until someone re-runs the installer and restarts both containers.
DoPublish the roots as a ClusterTrustBundle (the TODO at hack/install-ate.sh:382; the pattern exists in egressmitmtrust_controller.go), consume via a projected volume, deliver the validation context over filesystem SDS with a watched_directory, and replace the one-shot read with an atomic.Pointer[x509.CertPool].
Put deadlines on atunnel's certificate mint and gateway dial
P1Sinternal/atunnel/egress.go:119, :145, :265 · credential.go:145-149 · client.go:136-160
The activation ctx has no deadline. A broker that accepts but never answers parks the renewal goroutine forever, the retry/backoff never runs, the cert expires, and every new tunnel is refused with no recovery short of Deactivate/Activate. On the dial side, ReadResponse ignores ctx entirely and sets no read deadline, and the context.AfterFunc that closes both conns is registered only after DialContext returns — so a gateway that completes TLS and never answers CONNECT pins that goroutine and both sockets permanently.
DoBound the mint (e.g. 10s) inside BrokerCertificateSource.Mint, and add a configurable handshake+CONNECT budget inside Client.DialContext, including an explicit read deadline around ReadResponse.
Bound and cache the per-CONNECT GetActor
P1Mtrackedegress/egress.go:167-170 — TODO cites issue #592, directly above an uncached, deadline-free GetActor
There is no admission control on this path at all — the parking lot is built only inside ingress.New. So an actor opening many short-lived connections multiplies ateapi QPS one-for-one, undamped, and because the filter is fail-closed, ateapi slowness becomes egress unavailability with no shedding in between. The "leaked goroutine per CONNECT" claim is wrong — Envoy's message_timeout reset cancels the stream context; the real cost is the serial Process loop holding the stream for the full 5s.
DoWrap GetActor in a sub-5s timeout (2s matches the configured grpc_service timeout); add singleflight coalescing as the safe first step, then a short-TTL positive cache keyed by (atespace, name, uid) — noting a positive cache defers revocation. Add a bounded-concurrency gate mirroring the ingress lot.
Clamp MITM leaf NotAfter to the signing root, and alert on root expiry
P1Strackedinternal/localca/localca.go:179-185 (TODO, signs unmodified) · sdsmint/minter.go:105 · deltastream.go:192-198
The consumer already assumes the clamp exists — deltastream.go says "The floor is for the clamp, where notAfter is the CA's and can be nearer than that." The root is a fixed year with nothing monitoring it, so at the one-year mark every actor's HTTPS egress breaks at once while sdsmint keeps minting happily.
DoThe durable half first: have sdsmint refuse to start (or log Error and export a notAfter gauge) when the active CA has less remaining life than --leaf-cert-ttl, so root expiry is visible weeks ahead. Then implement the clamp.
Serve the last-good CA pool on refresh failure; stop reporting mint failures as unknown SNIs
P1Sinternal/localca/localca.go:107-126 · sdsmint/deltastream.go:161-169 · minter.go:100
On a read error refreshIfNecessary returns without assigning p.pool or advancing p.nextLoad — so a transient FS error fails the call despite a good in-memory pool, and every subsequent call re-reads the file. Downstream, every minter error collapses into "tell Envoy the name does not exist" even though the minter already distinguishes errHostNotAllowed. A CA-pool read failure is reported identically to a syntax rejection, with no error on the SDS stream and no metric (there is no telemetry file in sdsmint at all).
DoKeep serving p.pool on a failed reload (log, back off nextLoad); hard-fail only when there is no pool. Branch handleSubscribe on errors.Is(err, errHostNotAllowed) — only that becomes RemovedResources; any other error returns so Envoy retries, with a distinct counter.
Fix the --leaf-cert-ttl help text
P2Ssdsmint/cmd.go:44 vs deltastream.go:198
The flag says the xDS resource TTL is "half its length, so an actively used name is re-minted about twice per lifetime". The actual derivation is max(time.Until(cert.NotAfter)-time.Minute, time.Second) — lifetime minus one minute, as the manifest correctly documents. An operator sizing sdsmint from --help gets the re-mint rate wrong by ~2×: --help advertises about two re-mints per leaf lifetime where the real TTL (notAfter − 1m) gives about one.
DoRewrite the flag description; add a unit assertion pinning pack()'s TTL to notAfter − 1m.
Set concurrency and message-size limits, and add panic recovery
P2Sextproc/extproc.go:80-85 — the whole gRPC server construction
grpc-go defaults are maxConcurrentStreams: MaxUint32 and a 4 MiB receive limit; each stream is a goroutine plus a map built from every header. grpc-go also does not recover handler panics, and this process hosts the xDS control plane — so one panic takes config service down for the whole ingress fleet. No reachable panic path was found today, and the keepalive claims were refuted — grpc-go already enforces them.
Dogrpc.MaxConcurrentStreams sized from extProcMaxRequests(), grpc.MaxRecvMsgSize ~256 KiB, grpc.MaxHeaderListSize; add a stream interceptor that recovers, logs the stack, counts a panic, and returns codes.Internal so only that stream dies.
Reject unexpected ProcessingRequest types instead of answering them
P2Sextproc/extproc.go:104-115
The default: branch logs at Error once per message and answers with an empty RequestHeaders response regardless of what arrived. Unreachable from a correctly configured Envoy (both dataplanes set body NONE / trailers SKIP), so the real cost is an unrate-limited error-log amplifier from a non-Envoy client.
DoReturn status.Errorf(codes.InvalidArgument, …) to close the stream; log at Warn with a first-per-stream guard.
Truncate and fix the /statusz query ring buffer
P2Sextproc/record.go:100-117 · metadata.go:64-66
Host, redacted path, method, and :authority are stored verbatim with no length bound, retained until overwritten 100 requests later. Separately the "Client" column is a bug: Client is set from m.Headers[":authority"], which is already what Host holds — so the one operator-facing "who sent this" field is always a duplicate, which looks like real data during an incident and is not.
DoTruncate stored strings to ~256 bytes; request the downstream remote address as a CEL attribute alongside the authority attribute and populate Client from it — or drop the column.
Parse the /statusz dashboard template once
P2Sstatus.go:191 — template.New(...).Parse(dashboardHTML) inside handleStatusz
Re-runs lex, parse, and escape analysis against an 18 KB embedded constant on every request. It also means a template syntax error ships as a per-request 500 instead of failing at init.
DoHoist to var dashboardTmpl = template.Must(...).
Give the ingress Envoy container a readiness probe
P0Satenet-router.yaml:269-311 — no readiness, liveness, or startup probe · router.go:188-193 · components/agentgateway/kustomization.yaml:41-75
The pod's only probes are on the Go container, backed by a zero-value Readiness that reports ready until the drain flips it. Verified ordering: the readyz server starts before startDataplane() brings up the xDS listener, so /readyz is 200 while Envoy provably has no LDS/CDS — yet the Service targets Envoy's ports. A failed initial UpdateSnapshot is logged and Serve continues, and a NACK is invisible (1.2), so a config error becomes an outage instead of a stuck rollout. The egress Envoy gets both probes; ingress is the odd one out, and agentgateway ingress gets zero probes of any kind.
DoAdd readiness and startup probes on /ready port admin to the ingress envoy container (and /healthz/ready for agentgateway); wire routerHealth.Report().Dataplane.Healthy plus a last-snapshot-accepted signal into the Readiness, keeping /healthz unconditional so liveness and readiness still diverge during drain.
Set resource requests and limits on both gateways
P0MNo resources: on any container in atenet-router.yaml or atenet-egress.yaml · benchmarking/automation/testtypes/nighthawk_ingress.py:126-149
Both gateways land in BestEffort QoS: no CPU guarantee, first to be evicted under node memory pressure, scheduled as if free — and an Envoy memory blowup takes neighbours down rather than itself. The only counter-example is the sdsmint container. Direct evidence the shipped shape is not the intended one: the benchmark harness has to patch requests=limits in before it can measure (cpu only — memory is absent even there).
DoAdd explicit cpu+memory requests/limits to the atenet-router, envoy, and ext-proc containers for Guaranteed QoS, matching Envoy's --concurrency, sizing memory above the fixed_heap threshold from 5.6.
Run more than one replica of each gateway, with PDBs and spreading
P0Matenet-router.yaml:150 · atenet-egress.yaml:211 · atenet-egress-with-sdsmint.yaml:738 — all replicas: 1
Neither Deployment sets strategy, topologySpreadConstraints, or affinity, and the only PodDisruptionBudget in manifests/ belongs to the API server. So a node drain, cluster upgrade, spot preemption, or single OOM is 100% ingress or 100% egress offline, with nothing telling the eviction API the pod is critical — and the careful drain sequence has nowhere to hand traffic off to. The control plane already got this treatment; the data path did not.
Do≥2 replicas with rollingUpdate: {maxUnavailable: 0, maxSurge: 1}, a maxUnavailable: 1 PDB each, and topology spread on hostname. Confirm first that per-replica in-memory parking state behaves acceptably split across replicas — --parked-request-max becomes per-replica capacity.
Drop the debug component log levels from the shipped router manifest
P0Satenet-router.yaml:274-276 — --component-log-level upstream:debug,router:debug,ext_proc:debug
No comment, no time-box, no overlay that removes it; the egress Envoy carries no such flag. Three of Envoy's hottest components emit multiple lines per request on the data path, and ext_proc debug output puts client-supplied request headers — actor hostnames included — into cluster logs by default. The benchmark harness strategic-patches the entire envoy command to remove these flags before measuring, so any capacity number produced by that harness describes a configuration that does not ship. The harness explains why it patches; the manifest carries no note that its shipped default is unmeasured.
DoRemove the args; move them to a debug-only kustomize overlay or drive them from the router's existing --log-level.
Scrape and trace the egress gateway — it currently emits nothing
P0Satenet-egress.yaml:214-218 (labels only) · :305-357 (no envFrom, no OTEL_RESOURCE_ATTRIBUTES) · one PodMonitoring, selecting app: atenet-router
No prometheus.io/scrape annotation, and the kind Prometheus job keys entirely on it; OTEL_EXPORTER_OTLP_ENDPOINT is unset so the exporter runs into the void. Envoy's egress_connect stats, TLS handshake failures, and every egress authorization decision are unobservable in both environments — the only signal is the stdout access log. The metrics server is already running in egress mode, so this needs no code change.
DoAdd the scrape annotations plus envFrom: [configMapRef: {name: ate-otel-config}] and OTEL_RESOURCE_ATTRIBUTES to the egress ext-proc container, and a PodMonitoring for app: atenet-egress scraping Envoy's /stats/prometheus.
Add an overload_manager to every Envoy bootstrap
P1MNo overload_manager or global_downstream_max_connections anywhere in manifests/ or cmd/ · atenet-egress-with-sdsmint.yaml:817-830, :536-553
With no global downstream connection limit and no memory-pressure shedding — and, per 5.2, no memory limit for fixed_heap to work against — an accepting Envoy keeps taking connections and buffering until the container OOMs, which on the ingress gateway is a full outage rather than graceful degradation. The sdsmint variant is the sharpest case: it states a sizing contract against "Envoy's fixed_heap" and its circuit breakers explicitly defer to it — reasoning from two guards that are not configured.
DoAdd overload_manager with envoy.resource_monitors.fixed_heap driving shrink_heap and stop_accepting_requests, plus global_downstream_max_connections, to all three bootstraps at the thresholds the sdsmint comment assumes. If those numbers are aspirational, delete the comment rather than leave a contract nothing enforces.
Fix the monitoring selectors that break in agentgateway mode
P1Satenet-router-monitoring.yaml:29-35 vs components/agentgateway/kustomization.yaml:52-64, :74-75
The PodMonitoring scrapes named port: admin, but the overlay replaces the container's ports with http/https/connect/connect-tls/readiness/stats — no admin port exists after the patch, so on a cluster switched to agentgateway the leftover PodMonitoring resolves no target — a fresh agentgateway install never gets one, since base/kustomization.yaml does not list it. The patch also never touches the pod-template annotations, so prometheus.io/port: "9090" survives while the container that served 9090 is replaced by one that does not. Both environments silently lose gateway metrics; agentgateway's own stats on 15020 are collected by nothing.
DoPatch the annotation to 15020 with the right path, and either rename the stats port to admin or add an agentgateway-specific PodMonitoring.
Give the ingress router pod the securityContext the egress pod already has
P1Satenet-router.yaml:162-163 — no pod-level securityContext, neither container has one · atenet-egress.yaml:221-236
The pod terminating all untrusted client traffic runs stock envoyproxy/envoy as the image's default user with the full default capability set, no privilege-escalation block, a writable root filesystem, and an unconfined seccomp profile — and unlike egress it needs no sysctl escape, since every listener is above 1024. grep seccompProfile manifests/ hits only the kind Prometheus manifest.
DoCopy the egress securityContext onto both router containers (runAsNonRoot, runAsUser: 65532, allowPrivilegeEscalation: false, drop: [ALL]) and add readOnlyRootFilesystem with an emptyDir for Envoy scratch and add seccompProfile: {type: RuntimeDefault} at pod level on all three gateway manifests.
Set automountServiceAccountToken: false on the egress and agentgateway-ingress pods
P1Sgrep automountServiceAccountToken manifests/ — zero hits repo-wide · atenet-egress.yaml:25-29 · components/agentgateway/kustomization.yaml
For egress the token is provably unused — the manifest comment asserts the invariant ("No RBAC is bound to this ServiceAccount on purpose … It reaches the control plane over the ate API only") and the container's args and mounts confirm it. Worse in agentgateway ingress mode: the overlay removes the only Kubernetes client but never touches serviceAccountName, so the pod keeps an automounted token carrying cluster-wide actortemplates get/watch/list that nothing in it uses — a standing least-privilege violation on the pod that terminates untrusted traffic.
DoAdd automountServiceAccountToken: false to both egress manifests, and patch it (plus ideally a dedicated no-RBAC ServiceAccount) into the agentgateway ingress overlay.
Stop applying the whole ate-install directory on the GKE install path
P1Shack/install-ate.sh:224 (run_ko resolve -f manifests/ate-install) vs :283-289 · manifests/ate-install/base/kustomization.yaml:21-29
Applying the directory applies both mutually exclusive egress manifests in one pass — they define the same ConfigMap, Deployment, and Service. Only afterwards does apply_atenet_egress pick one. The ordering is deterministic (ko walks the directory in sorted order and kubectl applies one stream), but which egress Deployment briefly exists is decided by filename collation rather than --experimental-use-sdsmint, every install pushes two conflicting Deployment specs at the API server, and a non-sdsmint install churns through a pod mounting a secret that does not exist yet.
Dokubectl kustomize manifests/ate-install/base | run_ko resolve -f -, with a GKE-only kustomization adding the monitoring manifest.
Digest-pin the remaining images
P1Scomponents/agentgateway/kustomization.yaml:45, :118 · atenet-dns.yaml:95, :114
Exactly four unpinned refs under manifests/: cr.agentgateway.dev/agentgateway:v1.5.0 twice, busybox:1.36, and coredns/coredns:1.11.1. Every Envoy ref under manifests/ is digest-pinned. A re-pushed tag or a differing node cache silently runs different binaries across replicas with no way to tell from the manifest.
DoPin all four to digests, and add a hack/verify/ check that every non-ko:// image reference carries @sha256: — it would catch four regressions today.
Make the egress readiness probe observe the router's drain flip
P2Satenet-egress.yaml:339-341 (tcpSocket) vs atenet-router.yaml:242-247 (httpGet /readyz)
Port 9090 is neither declared nor probed in the egress Deployment, and the TCP listener stays open for the whole --drain-delay, so the readiness flip is observed by nobody. Overstated in one review: on a normal rollout Kubernetes drops the pod from ready endpoints on deletionTimestamp regardless of probes — what is genuinely lost is the SIGTERM-without-deletion case, the operational drain signal, and consistency between two Deployments running the same binary.
DoAdd the metrics port and switch to httpGet /readyz in both egress manifests; keep the tcpSocket check as a startupProbe.
Add a direction attribute and an egress-denial counter
P1Sextproc.go:141 computes dir and :158 never passes it · metrics.go:63-73, :96-110 · dispatch.go:44-51
recordRouteDuration records only template namespace/name, outcome, and resume — the first two are always empty for egress and resume is meaningless there. classifyOutcome has no Forbidden case, so most egress denials are labelled outcome="resume_error" on a metric named route.duration — an egress cert-rotation outage renders on dashboards as router resume failures. Worst of all, the code documents the exact silent failure this needs to catch: if Envoy cannot parse the CEL attribute it sends an empty map, directionOf returns ingress, and an --mode=egress instance 404s every CONNECT — a total outage that today looks like a spike of not_found with empty labels.
DoThread dir into recordRouteDuration; add a denied bucket for Forbidden/Unauthorized/MethodNotAllowed in classifyOutcome; add atenet.router.egress.denied with a bounded reason label matching the branches (no_ca, no_client_cert, cert_invalid, not_signed, bad_identity, uid_mismatch, not_running, control_plane_unavailable); add a counter for direction-not-served refusals. Register all in the metric registry.
Emit the shutdown-outcome metric the drain code already asks for
P1Stracked ×3drain.go:131-133, :151-154 (the TODOs) · docs/metrics/substrate.yaml:235-237 (recorded blind spot)
Both branches emit only a warn log. A force-stop cancels every in-flight ext_proc stream — including parked requests, whose clients then see a fail-closed 500 — the single most user-visible thing a rollout can do wrong, observable only by grepping logs of already-terminated pods. The egress case is worse: the manifest itself states that long-lived CONNECT tunnels keep downstream_cx_active above zero, so every rollout with active egress hits the incomplete-drain branch — a permanently-firing condition nobody can graph.
DoAdd atenet.router.drain.completed{outcome=clean|dataplane_incomplete|extproc_force_stopped} and a drain-duration histogram (verified: Run blocks on drainDone before the deferred MeterProvider shutdown, so the datapoints flush); clear the blind-spot entry. Then give egress its own shorter drain window so the signal means something.
Add prometheus to the generated Corefile, and stop logging every DNS query
P1Strackedcmd/atenet/internal/dns/corefile.go:35-39 · docs/metrics/substrate.yaml:224-228
The Corefile appends exactly log, errors, health, ready, reload — no prometheus, and log is unconditional with no tie to the dns command's --log-level. Actor DNS is on the request path for every client and emits no telemetry at all: no QPS, no NXDOMAIN rate, no latency, and no signal when a Corefile reload silently fails.
DoAppend prometheus :9153 in buildTemplate and add a PodMonitoring; remove log from the defaults or gate it on debug.
Build data-path dashboards and alerts
P1Mtools/setup-gcp/dashboards/ — three JSONs, no parking reference · grep AlertPolicy tools/setup-gcp/ returns zero hits
The two signals that say the ingress gateway is saturating — parking.active and parking.rejected — are exported and never displayed. There is no alerting at all: no 5xx rate, no ext_proc error rate, no xDS staleness, no certificate expiry, despite the whole system depending on rotating bundles.
DoA gateway dashboard (parking active/rejected, route duration by outcome and direction, downstream rq by code) plus alert policies for 5xx rate, parking.rejected > 0, and ext_proc failure rate. Depends on 1.2 and 3.5 to make the last two alertable at all.
Resolve the OTel Tracer once, and gate span attributes on IsRecording()
P2Singress/ingress.go:94 · resumer.go:167 · vendor otel internal/global/trace.go:78 · sdk/trace/provider.go:155-163
Each calls otel.Tracer(...) per request, which goes through the global delegating provider's exclusive lock and then the SDK provider's exclusive lock plus a map lookup on a freshly built scope — four acquisitions of two process-global mutexes per ingress request, on a server built for concurrent streams, for an object that never changes after startup. ateattr.ActorRefAttributes also allocates a fresh slice per call while the default root sampling ratio discards ~99%.
DoResolve the tracer once after InitTracing and store it on the Server / Handler / Resumer; gate attribute construction on span.IsRecording() or move to SetAttributes after the sampling decision.
Raise the health-check interval and demote its per-tick INFO
P2Scmd.go:59 (--health-interval defaults to 1s) · health.go:102, :212, :232
Each tick runs a real API-server /version call and a real ListActors store query, and logs at Info. Verified nothing gates traffic on the result — /readyz and /healthz come from serverboot's Readiness, and the health report is consumed only by handleStatusz. So it is 1 QPS per replica to two dependencies plus 86,400 INFO lines per pod per day, purely for a dashboard.
DoDefault to 10–15s; demote the per-tick line to Debug and keep failure logs at Error.
Cover Server.Process with a real gRPC stream test
P1Mextproc_test.go:43 exercises processRequestHeaders directly · extproc.go:88-121 untested · coverage 69.1%
The stream loop owns EOF handling, the off-protocol branch, stream.Send failure, client-disconnect propagation, and behaviour under GracefulStop with a parked request in flight — and the drain sequence depends on that last one, so a regression surfaces only as reset client requests during a live rollout.
DoA bufconn test over the real service asserting: clean EOF returns nil; a RequestBody message is rejected rather than answered; client cancellation propagates into the handler; a panicking handler kills only that stream; GracefulStop lets an in-flight parked stream finish.
Wire the drain verification into CI, and add a grace-period arithmetic test
P1Mhack/verify-all.sh:24-28 iterates only hack/verify/*.sh · hack/verify-atenet-drain.sh appears in no workflow
Correction to one reviewer: unit coverage of the drain sequence is not absent — drain_test.go covers ordering, force-stop, dataplane failure, four envoyDrainer cases, and marker lifecycle. What is genuinely missing is CI execution of the live-cluster script and any test comparing the total budget to the grace period: 13s delay + 15s dataplane window + 20s derived ext_proc timeout = 48s against terminationGracePeriodSeconds: 60, so raising the park budget (or setting an explicit --drain-timeout) can silently push shutdown past it — --route-timeout is deliberately excluded from the derivation (config.go:198-208).
DoRun hack/verify-atenet-drain.sh in a serialized kind job (its header explains why it must run alone), add an egress equivalent, and add a unit test asserting drain-delay + dataplane window + drainTimeout < a declared grace-period constant.
Add Go microbenchmarks for the ext_proc hot path
P1Mgrep -rn '^func Benchmark' cmd internal pkg returns nothing · benchmarking/automation/tests.yaml:405-434
The repo has zero Go benchmarks. The only performance signal is four 30-minute nighthawk runs requiring a live GKE cluster and a 50-worker pool, whose end-to-end variance is dominated by the ateapi RPC — so a change adding a per-request allocation or a lock merges green. Real per-request work at hand: a map per request in metadata.go:48, reflective structpb.NewStruct at ingress.go:163, the one process-global per-request mutex at record.go:55, and a full x509 chain Verify per CONNECT at egress.go:229. The fakes already exist.
DoBenchmarkNewRequestMetadata, BenchmarkIngressHandleRequestHeaders, BenchmarkEgressVerifyActorCertificate, BenchmarkQueryRecorderAdd; run with -benchmem in CI and gate on allocs/op.
Cover the ingress port-parsing and parking-fairness edge cases
P1Singress_test.go:136-297 (no authority-with-port case at all) · :343 · actorref_test.go (TestParseActorRef)
TestParseActorRef covers :8080, a trailing dot, and :invalid:port but no out-of-range port; the parking-full test uses one actor against a 1-slot lot. These are exactly the behaviours the port-parsing and parking-fairness items above change, with nothing to lock the fixes in.
DoCases asserting 400 for :0, :99999, :abc; a fairness test (actor B admitted while A holds its per-actor cap). A client-supplied x-ate-target-port case would pass today — that one is a regression guard, not a gap.
Benchmark the egress CONNECT path
P2Lbenchmarking/automation/tests.yaml — 22 entries, all locust or nighthawk-ingress; none drives egress
Egress does strictly more per-request work than ingress — an x509 chain verify plus a synchronous GetActor — and the only acknowledgement of that cost is a code comment. Nobody knows how many CONNECTs/s an egress gateway sustains, or where GetActor starts hurting ateapi.
DoAdd a type: nighthawk-egress runner and tests.yaml entries mirroring the ingress sweep (envoyCpu 2/4/8/16), driving actor-originated CONNECTs with real actor-identity client certs.
Test the QueryRecorder ring buffer
P2Sno record_test.go in cmd/atenet/internal/router/extproc/ · record.go:64-88, :94-98
Untested: the two distinct index paths in Get (while filling vs after wrap), the exactly-full boundary, redactPath's query-string strip, and the one per-request global mutex under contention. A silent ordering regression would have an operator read the last 100 requests in the wrong order during an incident.
DoAdd the file covering partial / exactly-full / post-wrap ordering, redaction, and a concurrent Add/Get loop — make test already runs -race.
Add fuzz targets for the hand-rolled XFCC and authority parsers
P2Sno func Fuzz anywhere · egress.go:287-410 (parseXFCCChain, splitXFCCUnquoted's quote/escape state machine, unquoteXFCC)
A hand-rolled grammar feeding url.PathUnescape and pem.Decode directly in front of the egress authentication decision, whose table tests cover three malformed-header cases (egress_test.go:452-475) but never the quote/escape state machine itself. Honest scoping: no reachable panic was found — splitXFCCUnquoted does no indexing and unquoteXFCC guards its slice — and any surviving leaf still faces the full chain Verify plus IsCA/EKU/purpose checks. This is robustness hardening on pure, total functions, not a live bypass.
DoFuzzParseXFCCChain (no panic; a successful parse round-trips the leaf DER), FuzzSplitXFCCUnquoted, FuzzUnquoteXFCC, FuzzParseActorRef, seeded from existing corpora, with a short -fuzztime CI run.
How this was actually produced
Worth being specific, since the method is doing a lot of the work here.
Seven agents read the same surface in parallel, each given a different question: the generated xDS, the ingress request path, the egress path and actor identity, the ext_proc server, the manifests and operational posture, the EgressPolicy API, and performance plus test coverage. Each returned structured findings with file-and-line citations.
Then a second round, and this is the part that matters. Every finding went to a fresh agent whose only instruction was to knock it down — open the cited file, read around the line, and decide whether the code already handles it, whether the failure can actually occur in the deployed topology, or whether the Envoy or Kubernetes default it complains about is quietly fine. That round killed eight of 129 claims and corrected the severity or the reasoning on several more.
The refutations are the most useful output of the whole exercise, so here are four of them — the ones that don’t touch the findings I’m holding back:
- Unbounded ingress buffers and stream idle. One reviewer claimed
per_connection_buffer_limit_bytesandstream_idle_timeoutare unbounded on the ingress listeners. Envoy already defaults to 1 MiB and 5 minutes, andmax_concurrent_streamsdefaults to 1024 (not 231). Kept only the genuine gaps: the header-phaserequest_timeout, the stream cap, and connection lifetime. - A goroutine leaks per CONNECT while ateapi is wedged. Refuted — Envoy’s
message_timeoutreset cancels the server-side stream context. The deadline in 3.4 is still worth adding, for the serialProcessloop, not for a leak. codes.Canceledis mismapped to a 500. Dropped entirely — the claimed trigger (ateapiStop()during drain) is wrong; grpc-go surfaces that asUnavailable, and nothing in the repo returnsCanceled.- The egress ext-proc container should be a native sidecar. Refuted — the ext-proc container has its own readinessProbe, so the pod is not added to Service endpoints until it is listening. The “burst of failures on every pod start” does not happen.
The pattern in all four is the same, and it is the characteristic failure mode of a fast reader, machine or otherwise: a real fact, cited correctly, attached to a consequence that doesn’t follow. Defaults are the richest source of it. “No max_concurrent_streams is set” is true and sounds alarming; the default is 1024, which is fine. “The Envoy container has no readiness probe” is true, and on a normal rollout the pod still leaves the Service on deletionTimestamp, so the thing you were worried about doesn’t happen and a different, smaller thing does.
What I would and would not trust here
An agent reading a codebase is very good at the class of bug that lives in the gap between two files. The CONNECT timeout above is the clearest example: two sibling manifests, one line different, and no human reviewer diffs manifests against each other. Same for the discarded List, the debug flags the benchmark strips, the probe that exists on one gateway and not its twin. These are all drift findings, and drift is exactly what tireless comparison is for.
It is much weaker at intent. It cannot tell you that a limit is unset because someone measured it and decided the default was right, and it will confidently produce a plausible failure mode for a default that has never failed. That is what the refutation round is patching, imperfectly — eight caught out of 129 is a floor on the error rate, not a ceiling.
So: treat the citations as reliable, the reasoning as a strong hypothesis, and the priorities as one outsider’s opinion with no production context behind it. The three findings I led with are the ones I am asserting rather than merely reporting: each was re-checked directly against the cited files, not against an agent’s summary of them. For the remaining forty-nine, the honest description is that a machine read the code carefully, a second machine argued with it, and I am publishing the transcript because the citations make it checkable.
I would rather publish that with the corrections attached than a cleaner list that quietly dropped them.
Comments