feat(pipeline): Add Scheme to Context and plumb through listeners#399
Conversation
Plugins that compose target URLs from a request (token-broker's serverURL construction is the current case) have to either hardcode "http://" or carry their own scheme config. The transport already knows the scheme - it's on the :scheme pseudo-header in ext_proc / ext_authz and on r.URL.Scheme in the Go HTTP listeners - but the framework never surfaced it to plugins. Add a Scheme string field to pipeline.Context alongside Host/Path. Free-form string (not enum) so ws/wss/gRPC-Web and future transports pass through without a framework PR. Empty when the listener can't determine - plugins that need a concrete scheme pick a default themselves rather than assume, so missing listener plumbing doesn't hide behind a framework-supplied default. This commit adds the field and documents it. Listeners populate it in the next commit; a plugin consumer (token-broker) follows after. Resolves rossoctl#397 (part 1 of 3). Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Each listener reads scheme from its transport-native source and sets
the new Context.Scheme field at pctx construction:
ext_proc :scheme pseudo-header on the headers block
(inbound + outbound, headers-only + body paths:
4 call sites)
ext_authz httpReq.GetScheme() on the CheckRequest's HTTP
attribute (shared by inbound + outbound pctx)
forwardproxy r.URL.Scheme on the incoming proxy request; Go
populates it from the absolute-URL request line
that HTTP forward proxies receive
reverseproxy r.TLS presence-based: TLS set -> https, absent
-> http. r.URL.Scheme is empty on Go server-side
requests so it's not usable; r.TLS is the
reliable signal. X-Forwarded-Proto support is
deferred pending a trusted-proxy policy.
Populated unconditionally on both inbound and outbound pctx where
applicable (ext_proc, ext_authz). Empty-string passes through when
the listener can't determine - plugins default explicitly per the
field's godoc.
Tests:
ext_proc 5 cases (inbound+outbound x http+https, plus
missing-pseudo-header-is-empty)
ext_authz 3 cases (http, https, empty passes through);
asserts both inbound and outbound pctx see the
same scheme
forwardproxy 1 case wiring a live httptest backend through
the proxy, assertion on pctx.Scheme == "http"
reverseproxy 2 cases using httptest.NewServer (plaintext)
and NewTLSServer (tls) to confirm r.TLS-based
derivation
All new tests use a small schemeCapturePlugin (duplicated per
package rather than promoted to authlib/plugintesting: the plugin
shape is only useful in listener tests and not worth exporting).
Resolves rossoctl#397 (part 2 of 3).
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Closes the D2 follow-up from PR rossoctl#391's review. token-broker's serverURL construction was hardcoded to "http://" + host, which is fine for in-cluster cleartext but silently breaks any cross-cluster or internet-facing broker target. With pctx.Scheme (issue rossoctl#397, part 2) surfaced by every listener, the plugin now reads the scheme from the framework instead of inventing it: serverURL := pctx.Scheme + "://" + host Defaults to "http" when pctx.Scheme is empty — matches the previous hardcoded behavior, so existing callers upgrade without surprise. The X-Server-Url header sent to the broker now reflects the actual scheme the agent used to call the target. Test: 3 cases (https, http-explicit, empty-defaults-http) asserting the X-Server-Url the broker received matches pctx.Scheme. Resolves rossoctl#397 (part 3 of 3). Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Five small fixes from review: - reverseproxy/server.go requestScheme: document the intentional divergence from Context.Scheme's "empty when undetermined" convention. reverseproxy always returns "http" or "https" and is confidently wrong behind a TLS-terminating upstream (r.TLS is nil on the inner hop even though the caller's scheme was https). Named in the helper's godoc so a reader comparing listeners sees the contract difference acknowledged. - extproc scheme test: use outboundRequest(...) for outbound cases instead of the shape-identical inboundRequest(...). Reads cleaner and guards against the helpers drifting later. - forwardproxy + reverseproxy scheme tests: switch from pipeline.New to plugintesting.BuildPipeline (a thin wrapper over pipeline.New) so the four listener scheme tests share one construction style. Existing non-scheme tests in those files stay on pipeline.New — aligning only the scheme tests keeps blast radius narrow. - extproc's schemeCapturePlugin godoc now explicitly calls out the byte-for-byte duplication across the four listener test packages and why it's intentional (not worth exporting through authlib/ plugintesting). A reader spotting the duplication on grep finds the explanation in the extproc copy. No behavior change. All listener tests still pass. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
|
Addressing review in #1 reverseproxy policy divergence — Added a "Contract note" paragraph to #2 PR title — Shortened to "feat(pipeline): Add Scheme to Context and plumb through listeners" (66 chars). Dropped "+ token-broker" from the title; the PR body keeps the full scope documented. #3 outbound tests using #4 mixed #5 duplicated All tests pass, gofmt clean. |
esnible
left a comment
There was a problem hiding this comment.
Solid, well-tested PR. The design choices (free-form string, empty-when-unknown, per-direction population) are consistent across all four listeners, and the tokenbroker migration preserves the previous hardcoded behavior on the empty path. The reverseproxy requestScheme helper documents its TLS-heuristic limitation clearly and defers X-Forwarded-Proto until a trust policy exists — correct scope discipline. All 11 new test cases are assertive and each listener's population mechanism is covered.
Areas reviewed: Go (pipeline/Context, ext_proc, ext_authz, forwardproxy, reverseproxy listeners, tokenbroker plugin), Go tests, docs
Commits: 4 commits, all signed off (DCO passes)
CI status: All required checks pass
Closes #397.
Adds a
Scheme stringfield topipeline.Contextso plugins can read the request's URL scheme (http/https/ future transports) without reinventing transport-specific detection. Each listener populates the field from its native source; the token-broker plugin switches from its hardcoded"http://"topctx.Scheme + "://".Three commits
1.
feat(pipeline): Add Scheme field to ContextFree-form string (not enum) so
ws/wss/ gRPC-specific schemes pass through without a framework PR. Empty when the listener can't determine — plugins default explicitly per the godoc so missing listener plumbing doesn't hide behind a framework-supplied default.2.
feat(listeners): Populate pctx.Scheme in all 4 listenersext_proc:schemepseudo-header on the headers block (both inbound + outbound, both headers-only + body paths)ext_authzhttpReq.GetScheme()on the CheckRequest's HTTP attribute, propagated to both inbound + outbound pctxforwardproxyr.URL.Scheme— Go populates this reliably for HTTP forward proxies from the absolute-URL request linereverseproxyr.TLS-presence heuristic: TLS set → https, absent → http.r.URL.Schemeis empty on Go server-side requests and isn't usable. X-Forwarded-Proto is deferred pending a trusted-proxy policyPer-listener tests using a small
schemeCapturePluginthat records the value it sees on OnRequest.3.
feat(tokenbroker): Use pctx.Scheme for serverURL compositionCloses the D2 follow-up from my earlier review of PR #391. Defaults to
"http"whenpctx.Schemeis empty so existing callers upgrade without surprise.Design points
Schemestays on pctx only. NoSessionEvent.Schemefield in this PR; if operators ever need it in/v1/sessions, that's a separate decision.Schemeis basic request metadata (like Method / Host / Path), not an extension slot requiringReads: []string{"scheme"}declaration.Test plan
go build ./... && go test -race ./...pass acrossauthbridge/authlib,authbridge/cmd/authbridge, andauthbridge/cmd/abctlunderGOWORK=offgofmt -lclean on all touched.gofilespctx.Schemepopulates through both ext_proc and forwardproxy paths when traffic flowsScope
Narrow to what #397 asked for. Deliberately out of scope:
SessionEvent.Scheme) — no consumer asking for it todayserver_schemefallback config on token-broker — the framework field obviates the needAssisted-By: Claude (Anthropic AI) noreply@anthropic.com