Feat: Select active proxies via listener.roles - #709
Conversation
proxy-sidecar mode always started BOTH the reverse proxy (:8080, inbound) and the forward proxy (:8081, outbound), and validation unconditionally required reverse_proxy_backend. That forces standalone/laptop egress-only deployments to set a dummy backend and move the reverse proxy off its colliding :8080 default. Add listener.roles ([reverse] and/or [forward]); an empty list defaults to BOTH, so pods and every existing config are unchanged. A subset runs one shape: roles: [forward] # egress-only TLS-bridge (laptop demo) roles: [reverse] # inbound JWT validation only - config: ListenerConfig.Roles, RoleReverse/RoleForward, ActiveRoles() (empty => both). - presets: fill an addr default only for an active role (so forward-only never binds :8080 and reverse-only never binds :8081/:8082). - validate: reject unknown roles; require reverse_proxy_backend only when the reverse role is active; reject tls_bridge enabled without the forward role (it only affects outbound). - main: start each proxy (and, with forward, the transparent listener) only when its role is active — mirroring the existing empty-addr-skip pattern for the transparent proxy and session API. The forward-only demo config drops both prior workaround lines (reverse_proxy_backend, reverse_proxy_addr): just listener.roles: [forward]. Tests: ActiveRoles default/subset; ApplyPreset fills only active-role addrs; validation for missing backend (reverse), unknown role, and tls_bridge-without-forward. Verified at runtime — forward-only binds :8081 (+:8082,:9094) and never :8080; reverse-only without a backend fails validation. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
📝 WalkthroughWalkthroughProxy-sidecar listeners now support explicit reverse and forward roles. Presets, validation, and ChangesProxy-sidecar roles
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ListenerConfig
participant AuthbridgeProxy
participant ReverseProxy
participant ForwardProxy
participant TransparentListener
ListenerConfig->>AuthbridgeProxy: ActiveRoles()
AuthbridgeProxy->>ReverseProxy: Start when reverse role is active
AuthbridgeProxy->>ForwardProxy: Start when forward role is active
AuthbridgeProxy->>TransparentListener: Create when forward role is active
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mrsabath
left a comment
There was a problem hiding this comment.
LGTM. Clean, well-scoped, and backward-compatible — listener.roles is threaded through a single source of truth (ActiveRoles(), empty ⇒ both) that presets, validate, and main all consume identically, so there's no divergent role logic across the three files and existing configs / the operator path are unchanged.
Runtime safety checks out: transparentLn is nil-guarded on shutdown (main.go:466-467), and bridge is only consumed inside the forward branch — with validate.go now rejecting tls_bridge enabled without the forward role, there's no dead-config path. Test coverage is thorough: role defaulting, per-role preset addr-filling, and all four validation cases (reverse-without-backend, unknown role, tls_bridge ± forward).
One non-blocking doc/scope nit inline about the demo-config claim in the description.
Areas reviewed: Go (config, validation, main wiring), Tests, feature-gate parity
Commit: 1, signed-off (DCO pass) · CI: all green
Verdict: APPROVE
| // Valid values: "reverse", "forward". The preset fills an addr default | ||
| // only for an active role, and reverse_proxy_backend is required only when | ||
| // the reverse role is active. Ignored outside proxy-sidecar mode. | ||
| Roles []string `yaml:"roles" json:"roles"` |
There was a problem hiding this comment.
suggestion (non-blocking) — the PR description's "Change" section says "The forward-only demo config now drops both prior workaround lines — just listener: {roles: [forward]}", but no demo/YAML config is in this diff (only the 5 Go files). A reader following the description will go looking for that config edit. Either fold the demo-config change into this PR or reword the body to note it's a follow-up — otherwise the forward-only demo still carries the reverse_proxy_backend: http://127.0.0.1:9 + reverse_proxy_addr workaround this feature is meant to retire. The code itself is complete and correct as-is.
There was a problem hiding this comment.
Dobra uwaga, dzięki Mariusz! 🙂 You're right — the body was writing a check the diff doesn't cash. There's no in-repo demo config for this local forward-proxy setup (it's an ad-hoc local.yaml), so rather than fold a YAML edit in here I reworded the description: this PR is code-only and just enables the slimmer config, and the full one-liner demo (roles: [forward] + generate_ca) composes with #707. Went with the reword per your either/or.
Summary
proxy-sidecar mode always started BOTH the reverse proxy (
:8080, inbound) and the forward proxy (:8081, outbound), andvalidate.gounconditionally requiredreverse_proxy_backend. Real deployments come in three shapes — pods want both, a laptop/demo wants forward-only (egress TLS-bridge), and some cases want reverse-only — but only "both" was expressible, forcing egress-only users into a dummyreverse_proxy_backend: http://127.0.0.1:9plus areverse_proxy_addroverride (the default:8080collides with the kind cluster's port-forward).Change
Add
listener.roles— a list ofreverseand/orforward. Empty (the default) = both, so pods and every existing config (and the operator path) are unchanged. A subset runs a single shape:roles)listener: {roles: [forward]}:8081(+ transparent:8082, session API:9094)listener: {roles: [reverse], reverse_proxy_backend: ...}:8080(+ session API):8080; reverse-only never binds:8081/:8082.reverse_proxy_backendonly when thereverserole is active; rejectstls_bridgeenabled without theforwardrole (it only affects outbound).forward) only when its role is active, mirroring the existing empty-addr-skip pattern for the transparent proxy and session API.This PR is code-only (the 5 Go files below); there's no in-repo demo config to edit — the local forward-proxy setup is an ad-hoc
local.yaml. What this feature enables: that config can drop both prior workaround lines (reverse_proxy_backend,reverse_proxy_addr) down tolistener: {roles: [forward]}(see Composition).Testing Instructions
Unit tests:
ActiveRolesdefault/subset;ApplyPresetfills only active-role addrs; validation for reverse-without-backend, unknown role, and tls_bridge-without-forward. Verified at runtime: forward-only binds:8081/:8082/:9094and never:8080; reverse-only without a backend fails validation with a clear message.Composition
Independent of #707 (
generate_ca); together the laptop demo islistener: {roles: [forward]}+tls_bridge: {mode: enabled, ca_dir, generate_ca: true}.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com