Add ProxyAdminService with member-aware responses - #250
Conversation
7ae10e4 to
27537ff
Compare
| name: lint, breaking and generation drift - pull request | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v5 |
There was a problem hiding this comment.
Opengrep — security.gha.unpinned-action (WARNING)
Unpinned action reference actions/checkout@v5: this uses: resolves a mutable ref (tag or branch), so the code that runs in CI can change without this line changing. A compromised upstream can repoint the tag and execute arbitrary code with access to this repository's secrets and GITHUB_TOKEN (tj-actions/changed-files, March 2025). Pin to the full 40-character commit SHA with the resolved version in a trailing comment, e.g. uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2. Prefer deputy pin --ecosystems github-actions, which resolves the ref, writes a version comment that reflects the most specific ref actually pointing at that commit, and verifies the SHA is reachable from a real branch upstream. That last check matters: pinning alone does not detect imposter or dangling commits, and this rule only sees the shape of the ref, never its provenance. Reusable workflow calls (owner/repo/.github/workflows/x.yml@ref) run with the same trust as actions and are pinned the same way. Not reported, by campaign policy: temporalio/* refs (first-party, pinned by internal process), local ./ actions, self-repository $/ refs (resolve to the running commit, so they are already pin-equivalent), and docker:// images (pinned by digest as a separate ecosystem).
Fixed in 3b226c3
Fixed in c50f001
Fixed in 8c28d08
Fixed in 79ec5ce
Fixed in 366ed09
| fetch-depth: 0 | ||
|
|
||
| - name: Install buf | ||
| uses: bufbuild/buf-setup-action@v1 |
There was a problem hiding this comment.
Opengrep — security.gha.unpinned-action (WARNING)
Unpinned action reference bufbuild/buf-setup-action@v1: this uses: resolves a mutable ref (tag or branch), so the code that runs in CI can change without this line changing. A compromised upstream can repoint the tag and execute arbitrary code with access to this repository's secrets and GITHUB_TOKEN (tj-actions/changed-files, March 2025). Pin to the full 40-character commit SHA with the resolved version in a trailing comment, e.g. uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2. Prefer deputy pin --ecosystems github-actions, which resolves the ref, writes a version comment that reflects the most specific ref actually pointing at that commit, and verifies the SHA is reachable from a real branch upstream. That last check matters: pinning alone does not detect imposter or dangling commits, and this rule only sees the shape of the ref, never its provenance. Reusable workflow calls (owner/repo/.github/workflows/x.yml@ref) run with the same trust as actions and are pinned the same way. Not reported, by campaign policy: temporalio/* refs (first-party, pinned by internal process), local ./ actions, self-repository $/ refs (resolve to the running commit, so they are already pin-equivalent), and docker:// images (pinned by digest as a separate ecosystem).
Fixed in 3b226c3
Fixed in c50f001
Fixed in 8c28d08
Fixed in 79ec5ce
Fixed in 366ed09
3b226c3 to
c50f001
Compare
8c28d08 to
79ec5ce
Compare
79ec5ce to
366ed09
Compare
Adds ProxyAdminService, a control-plane gRPC API that describes a proxy's cluster connections. One call describes the whole proxy deployment rather than the pod that happened to receive it.
The admin plane
adminplane.Serveanswers one RPC at whatever scope the caller asked for and the listener allows: this process, this deployment, or once across a mux to another organization. An endpoint supplies four functions. It inherits deadline budgeting, discovery, concurrency capping, dial lifecycle and the unreachable roster.Three properties are structural rather than conventions each handler has to remember:
Serveapplies the narrowingViewlast. It refuses to answer a counterparty that has none.grpc.Server. On a mux this interceptor is therefore installed server-wide and also sees every replication call.Mergetakes members as a slice rather than a pre-folded value. An endpoint whose aggregate is not a sum can then still be expressed. Shard ownership is disjoint across pods. A configuration check wants to know whether the members agree rather than what they add up to.Self is recognized after the call, not before it. A DNS record carries no identity. Every discovered address is dialed, this pod's own included. A reply bearing this pod's own id is then discarded.
Three listeners, three roles
The mux listener withholds this deployment's shape. That shape is the per-pod rows carrying pod ids, addresses and versions. It also withholds every cluster connection belonging to a different migration.
Peer TLS does not reuse
encryption.GetServerTLSConfig. That function setsRequireAnyClientCertand replacesVerifyPeerCertificatewith one that logs the subject and returns nil. It never checks the chain. On a listener bound to the pod network the certificate is the only thing distinguishing a sibling from anything else that can reach it.NewProxygains an identity and an error return. The identity is supplied rather than read from config. A value in the shared config file would be identical on every replica. Deduplication by id would then collapse the whole deployment into a single member. The error return replaces a Fatal-then-continue that silently dropped a cluster connection when the log component was disabled.What the state means
A connection reports
CONNECTEDonly when it holds as many connected sessions as it is configured to hold. A session that never established was never added to the manager. Counting only the sessions held would report a connection running at a third of its configured capacity as fully healthy.mux_sessions_targetcarries the configured count. The same comparison is available to whoever reads the response.Configuration
Each discovery provider gets its own typed block, selected by name. Every layered configuration tool in this stack deep-merges and cannot delete keys. Switching provider through a Helm override leaves the previous provider's block behind. It has to be inert. A single flat options map could not be switched at all under strict decoding.
ProxyAdminis a value rather than a pointer because nil and an empty listen address already mean the same thing.The counterparty method list is not resolved through
auth.AccessControl, whoseIsAllowedreturns true for an empty list. That fail-open default is a compatibility promise the replication ACL depends on. Repeating it here would make the natural spelling of "off" the widest possible setting.Validation composes through
S2SProxyConfig.Validate(). A bad value names its own field path. Three checks cover a listener that would start up looking configured while being weaker than it reads:proxyAdmin.listenAddressmust be loopback. That listener has no TLS and no authorization.proxyAdmin.peer.tls.caServerNameis required whenever peer TLS is enabled. Siblings are dialed by IP. Without it the dial has no name to verify the sibling certificate against.proxyAdmin.peer.tls.skipCAVerificationmust be false.GetClientTLSConfigassigns it toInsecureSkipVerify.Metrics
The admin API answers an operator who asks. Alerting needs the same facts without one. The chart leaves that API's listener on loopback.
The session state breakdown is what nothing exported before.
mux_connection_activeis set to 1 on every observer tick until the session's lifetime ends.num_muxes_activeis the size of the session map. A session failing its ping but still in the map therefore reads as healthy in both. Their label sets do not join in PromQL either. The sampler readsmux.CountSessions, the same function the admin API reads. The metric and the endpoint cannot disagree.The admin listeners reuse the registered server metrics rather than declaring a second collector. A second
grpcprom.ServerMetricsunder the same namespace and subsystem panics at process start. The registry rejects a duplicate fully-qualified name. Metrics wrap the admin interceptor so its rejections land ingrpc_code.Helm
The operator listener ships enabled on loopback. A query then needs only an exec or a port-forward rather than a config change.
The peer listener stays commented out. It is what lets one pod answer for the whole deployment. It binds the pod network. It should be a decision rather than a default.
When dns discovery is selected the name defaults to this release's headless Service. That Service already publishes one A record per endpoint. Nothing about it changes. Without the default, every install would hand-write a cluster-internal DNS name.
POD_NAMEcomes through the downward API so each pod names itself in an aggregated response. Two members reporting the same id are indistinguishable from one member answering twice.Testing
make lint: 0 issues.make bins: passes.go test -race -tags test_dep ./...: every package passes,proxy/testat 58s.make testpins-timeout=5m.proxy/testexceeds that on a loaded machine while passing well inside it when the tree is not running in parallel.helm unittest s2s-proxy/: 9 tests pass. The configmap tests cover the case the config shape was designed around. Switching discovery provider through an override leaves the previous provider's block behind. The binary must still accept it under strict decoding.make helm-example: no drift.make proto-lintandmake proto-breaking: pass.proxy/proxyadmin_tls_test.gobuilds a real CA and asserts that a leaf from a different CA is refused by the peer listener. That is the claimpeerServerTLSConfigexists to make.