Joseph Doherty
2ee84af1c0
Phase 0 of the ClusterClient→gRPC migration
(docs/plans/2026-07-22-clusterclient-to-grpc-plan.md). Standalone hardening: it
closes a gap that exists today and is a precondition for moving command/control
onto gRPC in later phases.
T0.1 — delete the ManagementActor ClusterClientReceptionist registration.
It was built for an out-of-cluster CLI that was never written: the shipped CLI
speaks HTTP Basic to /management, which asks the actor in-process through
ManagementActorHolder. Nothing in the repo ever sent to /user/management. The
actor still runs there; only the cross-boundary advertisement is gone. Six
documents claimed the CLI used ClusterClient — including the CLI's own README
"Architecture Notes" — and are corrected here rather than left to rot.
T0.2 — record, do not port, the dead integration-routing path.
IntegrationCallRequest is unwired at BOTH ends: RouteIntegrationCallAsync has
zero callers anywhere, and RegisterLocalHandler(Integration, …) appears only in
a test, so production always answers "Integration handler not available". It is
excluded from the gRPC contract (28 of 29 commands migrate) rather than
enshrined on an additive-only wire format, and deleting it during a
transport migration would mix a behavioural change into a change whose whole
value is that behaviour is identical. See
docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md.
T0.3 — preshared-key authentication on SiteStreamService.
The service shipped with no auth at all: plaintext h2c, no interceptor, so
anything that could reach a site node's :8083 could open a live data stream or
read audit rows back via PullAuditEvents/PullSiteCalls. ControlPlaneAuthInterceptor
now gates /sitestream.SiteStreamService/ — modeled on LocalDbSyncAuthInterceptor
(constant-time compare, fail-closed, PermissionDenied) but gating a SET of
service prefixes so phases 1A/1B add services rather than interceptors. LocalDb
sync keeps its own separate key: it authenticates the pair partner, not central,
and collapsing the two would make a site's central-facing key also admit writes
into its database.
Keys are per site (SB-GRPC-PSK-<siteId>), never fleet-wide, so a compromised
site yields only its own. Central attaches them through ControlPlaneCredentials,
which binds CallCredentials to the channel — covering unary and streaming
uniformly, and letting the key resolve asynchronously, which a client
interceptor could not do without blocking. All three central→site channel
creation sites go through it (SiteStreamGrpcClient and both audit pull invokers);
the pull invokers' channel caches are re-keyed by (site, endpoint) because
credentials are per-site and bound to the channel.
Two decisions beyond the plan:
* StartupValidator now requires GrpcPsk on Site nodes. The plan specified only
the runtime gate, but fail-closed with no boot check produces a node that
joins, answers heartbeats and reports healthy while refusing every stream,
audit pull and telemetry ingest — silent and total. Same reasoning as the
existing inbound API-key pepper rule.
* Added Communication:SitePsks as a central-side key map. The plan assumed
central would read the store, seeded via a dev KEK; the docker rig
deliberately boots with no master key, so store-only resolution would leave
it unable to dial its own sites. The store stays primary — it is the only
source that can serve a site added at runtime — with the map covering
key-less hosts and one-off pins. Neither source falling back to
"unauthenticated" is the invariant.
T0.4 — dev keys on both rigs and tests.
34 tests. The seven that matter most exercise a real in-process gRPC stack over
TestServer: the unit tests on either side of the wire would both stay green if
the halves disagreed, and gRPC refuses call credentials on a plaintext channel
by default — the UnsafeUseInsecureChannelCallCredentials opt-in is only provable
by making a real call. They confirm correct key passes on unary AND streaming,
wrong key and no-credentials both get PermissionDenied, and an unresolvable key
fails the call with nothing reaching the service.
OPERATIONAL: a site node upgraded to this build without a key will not boot.
That includes the gitignored deploy/wonder-app-vd03/ overlay.
60 lines
3.3 KiB
Markdown
60 lines
3.3 KiB
Markdown
# Integration call routing (`IntegrationCallRequest`) is dead on both ends
|
||
|
||
**Date:** 2026-07-22 · **Status:** OPEN (decision needed: wire or delete) · **Severity:** Low (no
|
||
runtime impact — the path cannot be reached) · **Area:** Central–Site Communication
|
||
|
||
## What
|
||
|
||
"Pattern 4: Integration Routing" — `CommunicationService.RouteIntegrationCallAsync` →
|
||
`SiteEnvelope(IntegrationCallRequest)` → `SiteCommunicationActor` → an integration handler — is
|
||
plumbed end to end but connected at neither end.
|
||
|
||
- **No producer.** `RouteIntegrationCallAsync` (`CommunicationService.cs`, "Pattern 4") has **zero
|
||
callers** in `src/` or `tests/`. It is the only one of `CommunicationService`'s command methods
|
||
with none.
|
||
- **No handler.** `SiteCommunicationActor` forwards to `_integrationHandler` when one is
|
||
registered, but `RegisterLocalHandler(LocalHandlerType.Integration, …)` appears **only** in
|
||
`SiteCommunicationActorTests.cs`. `AkkaHostedService` registers the other three handler types
|
||
(`Artifacts`, `EventLog`, `ParkedMessages`) and never this one.
|
||
|
||
So if anything ever did call it, the site would answer
|
||
`IntegrationCallResponse(Success: false, Error: "Integration handler not available")`
|
||
(`SiteCommunicationActor.cs`, Pattern 4) — and the two tests that exercise the path both register
|
||
the handler themselves first, which is why the suite has never noticed.
|
||
|
||
Do not confuse this with the **Inbound API**'s routed-site-script path, which is live, tested, and
|
||
uses different messages entirely. This is a separate, unused routing pattern that predates it.
|
||
|
||
## Why it is recorded rather than fixed
|
||
|
||
Found during the recon for the ClusterClient→gRPC transport migration
|
||
([`docs/plans/2026-07-22-clusterclient-to-grpc-plan.md`](../plans/2026-07-22-clusterclient-to-grpc-plan.md),
|
||
T0.2), which had to enumerate every command crossing the site↔central boundary. Of the **29**
|
||
command types, this is the one that is excluded: **28 migrate to the gRPC contract.**
|
||
|
||
Porting it would mean designing a proto contract, a `oneof` slot and round-trip mapper tests for a
|
||
verb no caller can invoke and no site can service — and enshrining it on a wire format whose
|
||
evolution rules are additive-only, so an unused RPC slot is permanent. Deleting it during a
|
||
transport migration would mix an unrelated behavioural change into a change whose whole value is
|
||
that behaviour is identical. Hence: excluded from the contract, behaviour untouched, decision
|
||
deferred to its own change.
|
||
|
||
## Decision needed
|
||
|
||
Either:
|
||
|
||
1. **Delete** — remove `RouteIntegrationCallAsync`, the `IntegrationCallRequest`/`Response`
|
||
messages, the `SiteCommunicationActor` receive block, `LocalHandlerType.Integration`, and the
|
||
three tests that cover them. This is the default if no consumer is planned.
|
||
2. **Wire** — register a real integration handler on site nodes and give the method a caller. This
|
||
only makes sense if there is a requirement it serves; none is recorded in
|
||
`docs/requirements/`.
|
||
|
||
Whichever is chosen, do it **before Phase 4** of the migration, since Phase 4 deletes the Akka
|
||
transport underneath this path. If it is still dead at that point, option 1 is forced.
|
||
|
||
## Filing
|
||
|
||
To be filed as a Gitea issue on `dohertj2/scadabridge` by the repo owner — this note is the
|
||
in-repo record of the finding and of the migration exclusion it justifies.
|