Skip to content

Add ProxyAdminService with member-aware responses - #250

Closed
liam-lowe wants to merge 5 commits into
mainfrom
liam-lowe/proxyadmin
Closed

Add ProxyAdminService with member-aware responses#250
liam-lowe wants to merge 5 commits into
mainfrom
liam-lowe/proxyadmin

Conversation

@liam-lowe

@liam-lowe liam-lowe commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

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.Serve answers 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:

  • Serve applies the narrowing View last. It refuses to answer a counterparty that has none.
  • The peer listener refuses to forward. That is what keeps a group call to a single round of fan-out.
  • A method outside the admin service passes through untouched. One yamux session serves one grpc.Server. On a mux this interceptor is therefore installed server-wide and also sees every replication call.

Merge takes 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 loopback operator listener is trusted local access. Any scope, forwarding allowed, nothing withheld.
  • The peer listener serves sibling pods and refuses to forward.
  • The mux listener is reached by another organization. It serves only listed methods. Its answer is built field by field rather than by deleting fields. A field added to the proto later is therefore not served across an organizational boundary until someone adds it there.

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 sets RequireAnyClientCert and replaces VerifyPeerCertificate with 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.

NewProxy gains 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 CONNECTED only 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_target carries 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.

ProxyAdmin is 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, whose IsAllowed returns 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.listenAddress must be loopback. That listener has no TLS and no authorization.
  • proxyAdmin.peer.tls.caServerName is 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.skipCAVerification must be false. GetClientTLSConfig assigns it to InsecureSkipVerify.

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_active is set to 1 on every observer tick until the session's lifetime ends. num_muxes_active is 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 reads mux.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.ServerMetrics under 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 in grpc_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_NAME comes 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/test at 58s.
  • make test pins -timeout=5m. proxy/test exceeds 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-lint and make proto-breaking: pass.
  • 9 test files and 47 test functions added.
  • proxy/proxyadmin_tls_test.go builds a real CA and asserts that a leaf from a different CA is refused by the peer listener. That is the claim peerServerTLSConfig exists to make.

@liam-lowe
liam-lowe requested a review from a team as a code owner July 30, 2026 07:10
@liam-lowe liam-lowe closed this Jul 30, 2026
@liam-lowe liam-lowe reopened this Jul 30, 2026
@liam-lowe
liam-lowe marked this pull request as draft July 30, 2026 16:57
@liam-lowe liam-lowe changed the title Add ProxyAdminService, the proxy's own control-plane API Add ProxyAdminService Jul 30, 2026
@liam-lowe
liam-lowe force-pushed the liam-lowe/proxyadmin branch 8 times, most recently from 7ae10e4 to 27537ff Compare August 10, 2026 15:59
Comment thread .github/workflows/proto.yml Outdated
name: lint, breaking and generation drift - pull request
steps:
- name: Checkout
uses: actions/checkout@v5

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Opengrepsecurity.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

Comment thread .github/workflows/proto.yml Outdated
fetch-depth: 0

- name: Install buf
uses: bufbuild/buf-setup-action@v1

@github-actions github-actions Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Opengrepsecurity.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

@liam-lowe
liam-lowe force-pushed the liam-lowe/proxyadmin branch 2 times, most recently from 3b226c3 to c50f001 Compare August 11, 2026 23:11
@liam-lowe liam-lowe changed the title Add ProxyAdminService Add ProxyAdminService with member-aware responses Aug 11, 2026
@liam-lowe
liam-lowe force-pushed the liam-lowe/proxyadmin branch 2 times, most recently from 8c28d08 to 79ec5ce Compare August 12, 2026 19:44
@liam-lowe
liam-lowe force-pushed the liam-lowe/proxyadmin branch from 79ec5ce to 366ed09 Compare September 2, 2026 00:30
@liam-lowe liam-lowe closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant