Skip to content

router authn/z part 3: Convert all e2e tests so that they pass for static-mtls ingress auth mode - #2390

Merged
Taahir Ahmed (ahmedtd) merged 5 commits into
agent-substrate:mainfrom
ahmedtd:router-authnz-3
Oct 20, 2026
Merged

Taahir Ahmed (ahmedtd) merged 5 commits into
agent-substrate:mainfrom
ahmedtd:router-authnz-3

Conversation

@ahmedtd

@ahmedtd Taahir Ahmed (ahmedtd) commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

This PR can be broken into the following parts:

  • Defining a way for e2e tests running in Github Actions to get a client certificate the router understands. (The test code creates a PodCertificateRequest that doesn't correspond to any real pod).
  • Switching the router deployed for e2e tests into static-mtls mode.
  • Adding a new e2e test to verify static-mtls mode
  • Converting all other e2e tests that speak to the router to present a client certificate.

@zlammerts-svg

Copy link
Copy Markdown
Collaborator

Taahir Ahmed (@ahmedtd) Can you please make sure the PR is ready for review today or on Monday? Thank you!

Comment on lines +63 to +65
if e2e.IngressMTLS() {
t.Skipf("This subtest only checks deprecated-insecure functionality")
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is in ext_proc so it applies on TLS too, and I think this means the presubmit (envoy-only one) skips it so never runs. Maybe we just send it with router.TLSConfig()?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've adapted this so it goes through the same router as the other test cases. Let's see if it passes.

}

address := fmt.Sprintf("127.0.0.1:%d", localPort)
transport := http.DefaultTransport.(*http.Transport).Clone()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultTransport here means you move the suit to negotiate h1 and h2 when its TLS.

Was this intentional? if yes, why?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't intentional, but I'm not sure what that affects. Does the router only speak H1?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just that now it would only go to h2 i think

// serviceDNSTrustPool returns the live trust bundle of the servicedns signer,
// which issues the serving certificates of substrate's in-cluster Services
// (ateapi and atenet-router among them).
func serviceDNSTrustPool(ctx context.Context, clientset kubernetes.Interface) (*x509.CertPool, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: isnt this the exact same as serverTLSConfig? should we put it somewhere shared?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be shared. Most clients of the Substrate API and router should be getting these roots from a clusterTrustBundle projected volume, not directly interacting with the ClusterTrustBundle k8s API.

Comment thread internal/e2e/ingressauth.go Outdated
Comment on lines +79 to +83
// The router client's identity. The router's base manifest
// (manifests/ate-install/atenet-router.yaml) allowlists
// RouterClientServiceAccount's SPIFFE ID, so the two must change together. The
// namespace is fixed rather than per-test because the allowlist is fixed at
// install time.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The atenet-router manifest that was merged in #2387 shouldn't carry an e2e-only identity, otherwise with static-mtls install, we trust whoever can create this namespace and a PCR in it. Can we ship the manifest with no IDs and the e2e install add this one? I think that means letting the router start with an empty allowlist in deprecated-insecure mode (it changes nothing there, clients without certs already are allowed), and we probably should keep the non-empty check for static-mtls.

I'd rather fix this here than in a follow-up, since this is the PR that makes CI depend on the ID

ref: https://github.com/agent-substrate/substrate/pull/2387/changes#diff-4b797bcd7d88f3f0c6cf0f84c51e38803b7a8fd7959b7107431e1e6fb5cfab76R243

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #2903 to make it configurable on cmd/ate-setup. It's a little big, but I can merge it into this one if you like.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm with the understanding that there is a follow up to remove deprecated-insecure mode

@ahmedtd
Taahir Ahmed (ahmedtd) added this pull request to the merge queue Oct 20, 2026
Merged via the queue into agent-substrate:main with commit 70358cf Oct 20, 2026
15 checks passed
Dmitry Berkovich (dberkov) pushed a commit to dberkov/substrate that referenced this pull request Oct 20, 2026
…ithout the router's alert (agent-substrate#2961)

`TestIngressAuthentication` fails about one e2e job in three since
static-mtls became the e2e default (5 of the 15 pr-workflow runs since
agent-substrate#2390 merged): the refused client's request ends with `http2: client
conn could not be established` instead of the router's TLS alert, and
the test calls the refusal unproven. The router closes the connection
right after the alert, and the port-forward (`portforward.go:549 ...
write: broken pipe`) or Go's HTTP/2 client reports that close first.

Recent instances, all on the `https` listener:
-
https://github.com/agent-substrate/substrate/actions/runs/39402878520/job/118561649470
(untrusted CA)
-
https://github.com/agent-substrate/substrate/actions/runs/39408627974/job/118580615105
(unlisted SPIFFE ID)
-
https://github.com/agent-substrate/substrate/actions/runs/39410976322/job/118588372580
(no client certificate)
-
https://github.com/agent-substrate/substrate/actions/runs/39411585756/job/118590529170
(no client certificate)
-
https://github.com/agent-substrate/substrate/actions/runs/39413905393/job/118598129904
(unlisted SPIFFE ID)

**Change.** A refusal that fails without a TLS alert is retried on a
fresh port-forward, three times at most. An alert is still required, and
a client the router lets through still fails the test.

**Tests.** `go vet` and the suite compile; the retry only shows under
the flake, which CI's e2e jobs exercise.

- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR (none
needed)
Yufan Su (yufan-su) pushed a commit to yufan-su/substrate that referenced this pull request Oct 21, 2026
… data plane (agent-substrate#2686)

Adds what it takes to measure ate-api-server's and Postgres's cold
resume capacity on the fake data plane from the benchmark automation,
plus the first tests that use it.

- **boomer: record server-side latency.** `resumepaused` records
`locust_server_duration_milliseconds` from the `x-server-elapsed-us`
trailer that `ateinterceptors.ServerUnaryInterceptor` sets, next to the
client latency. The gap between the two is network and client time.
- **Scrape boomer in automation runner Jobs.** The runner container
declares port 8001 and `monitoring.yaml`'s `boomer-worker` job keeps
`substrate-benchmark-runner` pods, so automated runs get the same
latency histograms as manual ones.
- **Shape the control plane for capacity runs.** A locust test can carry
a `controlPlane:` block (`benchmarking/automation/controlplane.py`):
- `apiServer:` sets ate-api-server's replicas, CPU and memory (requests
= limits) and its Postgres pool size.
- `postgres:` loads `pg_stat_statements` with a `postgres_exporter`
sidecar that Prometheus scrapes, and passes extra Postgres settings.

Placement is left to the install: capacity tests install with
`--cordon-control-plane`, which keeps ate-api-server and Postgres on
their own tainted pools, away from the load. The block patches the
installed manifests after the deploy; a TODO in `controlplane.py`
records the drift guards that shim still lacks.
- **Summarize ladder runs per step, on the runner.** For a ladder run,
`benchmarking/locust/ladder_steps.py` reads each step's boundaries from
the runner's log, queries the in-cluster Prometheus over the step's
steady window, and appends a `ladder_step_NNN` row per step to
`stats.jsonl`: throughput, client and server latency, CPU and memory of
ate-api-server, Postgres and fake-atelet, and Postgres connections,
commits and waits. It is additive, like the server telemetry harvest it
follows: a failure is logged and the run's other results are kept.
- **Cold resume tests.** `tests.yaml` adds two:
- `ateapi_cold_resume_pause_fake_2cpu`: a 100–1500/s cold resume ladder
against one 2 CPU replica.
- `ateapi_cold_resume_pause_fake_smoke`: the same setup at 50 actors and
20 → 50/s, a quick end-to-end check.

Each actor books 10m CPU, since a fake Worker reports its node's
allocatable as its capacity and placement must never bind. The
benchmarking README explains the per-step rows.

**Running them.** The target cluster needs the `ate-control-plane` and
`ate-postgres` node pools `--cordon-control-plane` expects, and a node
with room for the runner. On current `main` the target cluster's env
also needs `SNAPSHOT_LOCATION` (agent-substrate#2544) and
`ATE_INGRESS_ALLOWED_SPIFFE_IDS` (agent-substrate#2390), which every install now
requires.

Addresses agent-substrate#2261

- [x] Tests pass (`go test -race ./internal/benchmarking/...`, the
`benchmarking/automation` and `benchmarking/locust` Python tests,
`hack/verify-all.sh`)
- [x] Appropriate changes to documentation are included in the PR
Yufan Su (yufan-su) pushed a commit to yufan-su/substrate that referenced this pull request Oct 21, 2026
…gent-substrate#2976)

PR agent-substrate#2390 removed the hardcoded `--ingress-allowed-spiffe-ids` flag from
`manifests/ate-install/atenet-router.yaml` and made
`ingressAllowedSPIFFEIDs` (`ATE_INGRESS_ALLOWED_SPIFFE_IDS` /
`--ingress-allowed-spiffe-ids`) required in `ate-setup deploy
ate-system` when `--atenet-dataplane=envoy`.

`deploy_substrate()` in `benchmarking/automation/orchestrator.py` does
not set `ATE_INGRESS_ALLOWED_SPIFFE_IDS`, so automated benchmark runs
fail at `deploy_ate_system`. Set `ATE_INGRESS_ALLOWED_SPIFFE_IDS` to
`BENCHMARK_RUNNER_PRINCIPAL` alongside `ATE_API_AUTHZ_BOOTSTRAP_OWNERS`,
and update the benchmark install docs.

Fixes agent-substrate#2975 

- [x] Tests pass
- [x] Docs updated
Sneha-at (Sneha-at) pushed a commit to Sneha-at/substrate that referenced this pull request Oct 22, 2026
TestActorArbitraryPortAccess has been flaking in e2e since the router
moved to static-mtls in agent-substrate#2390, e.g.
https://github.com/agent-substrate/substrate/actions/runs/39549120410/job/119054007125

client-go closes the whole port-forward when any one connection on it
errors:

https://github.com/kubernetes/client-go/blob/v0.37.0/tools/portforward/portforward.go#L549-L550

RouterClient shared one CONNECT port-forward for the life of the client.
So one reset from the router failed the tunnel in flight and every
Connect after it, which then got "connection refused" from the dead
local port. agent-substrate#2961 fixed the same thing in ingressauth by forwarding per
attempt.


<!-- The PR title becomes the release note. Write it for users. -->

Why is this needed? (Or link the issue. Open one first for features,
API, install or CI changes.)

Fixes a trivial test flake bug, test only change. Spotted while
reviewing PRs like
agent-substrate#2502

<!-- e.g. "AI-assisted, I reviewed and tested it." No AI co-author
trailers. -->
AI use: AI-assisted, I reviewed it.

<!-- Breaking change? Describe what users must do on upgrade and ask for
the `breaking-change` label. -->

- [x] Tests pass
- [x] Docs updated
Steven Shriver (stshrive) added a commit to stshrive/substrate-gke that referenced this pull request Oct 26, 2026
… next steps

From Substrate v0.6.0 the default Envoy ingress router admits only clients
that present an allowlisted pod certificate, and ate-setup refuses to
deploy it without ATE_INGRESS_ALLOWED_SPIFFE_IDS
(agent-substrate/substrate#2390). A v0.6.0 install stopped at "Turn on
Substrate" with "at least one of ATE_INGRESS_ALLOWED_SPIFFE_IDS ... must
be set".

The control-plane deploy now passes the counter demos' driver identities
(agent-substrate/substrate#3178), spiffe://cluster.local/ns/ate-demo-counter/sa/default
and spiffe://cluster.local/ns/ate-demo-counter-microvm/sa/default, with
any IDs the caller exported in ATE_INGRESS_ALLOWED_SPIFFE_IDS appended
and duplicates dropped. ate-setup reads the variable as one value, so the
installer joins the lists. Both demo IDs are listed because the sandbox
runtime and demo are chosen after the control plane is deployed. The
pod identity signer fixes the trust domain, so the IDs are known in
advance. The control-plane screen shows the allowlist, and the upgrade
hand-over exports it. Older trees and the agentgateway router ignore it.

The Envoy router also serves only TLS, so the next steps' port-forward
to port 80 and plain curl no longer reach an actor. Next steps now follow
the router:

- agentgateway: unchanged (port-forward, kubectl ate, curl).
- Envoy, tree with demos/counter/driver: kubectl logs of the driver,
  which calls the actor through the router.
- Envoy, no driver (v0.6.0): install kubectl ate and create the actor,
  then a note that callers need an allowlisted pod certificate, linking
  upstream's router README at the install's commit.

The README has a section on the allowlist: the ID format, which
workloads need an entry, adding your own clients before or after the
install, and the agentgateway alternative.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Steven Shriver (stshrive) added a commit to stshrive/substrate-gke that referenced this pull request Oct 26, 2026
… next steps

From Substrate v0.6.0 the default Envoy ingress router admits only clients
that present an allowlisted pod certificate, and ate-setup refuses to
deploy it without ATE_INGRESS_ALLOWED_SPIFFE_IDS
(agent-substrate/substrate#2390). A v0.6.0 install stopped at "Turn on
Substrate" with "at least one of ATE_INGRESS_ALLOWED_SPIFFE_IDS ... must
be set".

The control-plane deploy now passes the counter demos' driver identities
(agent-substrate/substrate#3178), spiffe://cluster.local/ns/ate-demo-counter/sa/default
and spiffe://cluster.local/ns/ate-demo-counter-microvm/sa/default, with
any IDs the caller exported in ATE_INGRESS_ALLOWED_SPIFFE_IDS appended
and duplicates dropped. ate-setup reads the variable as one value, so the
installer joins the lists. Both demo IDs are listed because the sandbox
runtime and demo are chosen after the control plane is deployed. The
pod identity signer fixes the trust domain, so the IDs are known in
advance. The control-plane screen shows the allowlist, and the upgrade
hand-over exports it. Older trees and the agentgateway router ignore it.

The Envoy router also serves only TLS, so the next steps' port-forward
to port 80 and plain curl no longer reach an actor. Next steps now follow
the router:

- agentgateway: unchanged (port-forward, kubectl ate, curl).
- Envoy, tree with demos/counter/driver: kubectl logs of the driver,
  which calls the actor through the router.
- Envoy, no driver (v0.6.0): install kubectl ate and create the actor,
  then a note that callers need an allowlisted pod certificate, linking
  upstream's router README at the install's commit.

The README has a section on the allowlist: the ID format, which
workloads need an entry, adding your own clients before or after the
install, and the agentgateway alternative.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/network area/security Security related issue/pr area/tests Enhancing / fixing test coverage. kind/feature An enhancement / feature request or implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants