Joseph Doherty
3fa955556d
Phase 0's gate doc now carries the full suite picture, not just the rig checks.
Playwright: 170 pass / 2 fail / 1 skip of 173. Both failures were run down to
root cause and both are pre-existing on main, unrelated to this branch (which
touches no EF, CentralUI, Transport or ManagementService file):
- TransportImportTests is a REAL production bug: BundleImporter.cs:1298 opens a
user-initiated transaction while the central context has EnableRetryOnFailure,
so SqlServerRetryingExecutionStrategy refuses the split query inside it and
bundle import fails against real MS SQL. The unit/integration suite cannot see
it -- the in-memory EF provider has no retrying strategy and BeginTransaction
is a no-op there.
- SmsNotificationE2ETests is a stale fixture: SID 'ACtest123' (2026-06-19) vs the
^AC[0-9a-fA-F]{32}$ guard added 2026-07-10 (40088a21). Failing since then, which
has also silenced everything after the toast assertion -- including the
secret-non-leak check on the Auth Token.
Also records that the earlier 44-failure run is void: a concurrent deploy.sh was
recreating the cluster underneath it.
Neither is fixed here; both are out of scope for a PSK-auth branch.
144 lines
7.3 KiB
Markdown
144 lines
7.3 KiB
Markdown
# ClusterClient → gRPC migration — live gate results
|
||
|
||
Rig: `docker/` (2 central + 3×2 site + traefik), rebuilt from the branch under test via
|
||
`bash docker/deploy.sh`. Recorded check-by-check in the family's live-gate format. Phase 5's
|
||
full eight-check gate is recorded further down as those phases land; this file starts with
|
||
Phase 0, whose DoD has its own smaller gate.
|
||
|
||
---
|
||
|
||
## Phase 0 — PSK auth + dead-code removal — **PASS** (2026-07-22)
|
||
|
||
Branch `feat/grpc-phase0-psk` @ `228ff8b4`. Image rebuilt, all 9 containers recreated.
|
||
|
||
### Baseline (pre-change build, same rig)
|
||
|
||
An unauthenticated call from the host to a site's audit-pull RPC was **accepted**:
|
||
|
||
```
|
||
$ grpcurl -plaintext -d '{"batch_size":1}' localhost:9023 sitestream.SiteStreamService/PullAuditEvents
|
||
{}
|
||
```
|
||
|
||
That is the gap Phase 0 closes, reproduced rather than assumed.
|
||
|
||
### Checks
|
||
|
||
| # | Check | Result |
|
||
|---|---|---|
|
||
| 1 | All 8 nodes boot with keys configured (new `StartupValidator` rule) | **PASS** — all recreated and reached ready |
|
||
| 2 | Unauthenticated `PullAuditEvents` ⇒ `PermissionDenied`, all 3 sites | **PASS** |
|
||
| 3 | Wrong key (site-b's key presented to site-a) ⇒ `PermissionDenied` | **PASS** — per-site scoping is real, not decorative |
|
||
| 4 | Correct key ⇒ success, all 3 sites | **PASS** |
|
||
| 5 | Central's own authenticated paths still work | **PASS** — 14 successful `PullAuditEvents` from central to site-a; **0** auth failures in either central's log |
|
||
| 6 | LocalDb sync unaffected by the new interceptor | **PASS** — **0** control-plane rejections and **0** sync auth failures on the passive peer; session connected after one boot-order retry |
|
||
| 7 | No interceptor-activation errors | **PASS** — 0 (see the defect below) |
|
||
|
||
Evidence for 2–4:
|
||
|
||
```
|
||
=== NO CREDENTIALS ===
|
||
:9023 -> ERROR: Code: PermissionDenied Message: Control plane authentication failed.
|
||
:9033 -> ERROR: Code: PermissionDenied Message: Control plane authentication failed.
|
||
:9043 -> ERROR: Code: PermissionDenied Message: Control plane authentication failed.
|
||
|
||
=== WRONG KEY (site-b's key against site-a) ===
|
||
:9023 -> ERROR: Code: PermissionDenied Message: Control plane authentication failed.
|
||
|
||
=== CORRECT KEY ===
|
||
site-a :9023 -> {}
|
||
site-b :9033 -> {}
|
||
site-c :9043 -> {}
|
||
```
|
||
|
||
Site-a rejected exactly **2** calls — the two deliberate probes above — and nothing else.
|
||
|
||
### Defect the gate caught that the test suite did not
|
||
|
||
**First run of this gate FAILED**, and is worth recording because the failure mode is
|
||
deceptive.
|
||
|
||
`Grpc.AspNetCore` activates a type-registered interceptor through
|
||
`InterceptorRegistration.GetFactory()`, which throws when more than one public constructor is
|
||
applicable. `ControlPlaneAuthInterceptor` shipped with two — the DI one and a prefix-set
|
||
overload intended for later phases.
|
||
|
||
The throw happens **inside the pipeline, per call**, so:
|
||
|
||
- nothing failed at startup; the node booted, joined its pair and reported healthy;
|
||
- every gated call died with `Unknown / "Exception was thrown by handler"`, which reads as a
|
||
handler bug rather than an auth bug;
|
||
- **correct key, wrong key and no key produced identical errors** — the tell. A gate that
|
||
cannot distinguish those is not authenticating anything.
|
||
|
||
Site-a's log at the time: three `PullAuditEvents` calls, three identical
|
||
`System.InvalidOperationException: Multiple constructors accepting all given argument types
|
||
have been found in type 'ControlPlaneAuthInterceptor'`.
|
||
|
||
The full suite was green when this shipped — **29 suites, 6,872 tests, 0 failures**. The
|
||
in-process end-to-end test missed it because it registered the interceptor with
|
||
`AddSingleton` alongside `AddGrpc`, so DI returned the instance and gRPC's activation path
|
||
never ran.
|
||
|
||
Fixed in `228ff8b4`: the prefix-set constructor is `internal`; the end-to-end harness now
|
||
registers exactly as `Program.cs` does (by type, not in DI); and a reflection assertion pins
|
||
"exactly one public constructor", since that is the actual invariant.
|
||
|
||
**Lesson for phases 1A/1B, which both add services to this interceptor:** extend
|
||
`DefaultGatedPrefixes`; do not add a second public constructor. And any in-process harness for
|
||
a DI-activated component must mirror the production registration shape or it proves less than
|
||
it appears to.
|
||
|
||
### Test suite alongside the gate
|
||
|
||
Non-Playwright: **29 suites, 6,872 tests, 0 failures**.
|
||
|
||
Playwright (against this rig): **170 passed, 2 failed, 1 skipped** of 173. Both failures were
|
||
run down to root cause and **both are pre-existing on `main`, unrelated to Phase 0** — this
|
||
branch touches no EF, CentralUI, Transport or ManagementService file (`git diff --stat
|
||
main...HEAD -- src/` is 15 files, all Communication/Host/AuditLog gRPC plumbing).
|
||
|
||
An earlier run of this suite reported 44 failures. That run is **void**: a `docker/deploy.sh`
|
||
was recreating the cluster underneath it, so the fast `LoginTests`/`NavigationTests` failures
|
||
were "app unreachable", not defects.
|
||
|
||
**1. `TransportImportTests.ImportSyntheticBundle_AppliesAndShowsAuditDrillIn` — a real
|
||
production bug, not a test defect.** Central's log during the failure:
|
||
|
||
```
|
||
[ERR] An exception occurred while iterating over the results of a query ...
|
||
System.InvalidOperationException: The configured execution strategy
|
||
'SqlServerRetryingExecutionStrategy' does not support user-initiated transactions.
|
||
at Microsoft.EntityFrameworkCore.Query.Internal.SplitQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
|
||
```
|
||
|
||
`BundleImporter.cs:1298` opens a user-initiated transaction; the central context is configured
|
||
with `EnableRetryOnFailure` (`ConfigurationDatabase/ServiceCollectionExtensions.cs:33`). SQL
|
||
Server's retrying strategy refuses to run a split query inside a caller's transaction, so
|
||
**bundle import fails against real MS SQL**. The fix is the one the exception names: wrap the
|
||
transaction in `Database.CreateExecutionStrategy().ExecuteAsync(...)`.
|
||
|
||
Why the whole unit/integration suite is green on it: those tests use the in-memory EF provider,
|
||
which has no retrying execution strategy — and `BeginTransactionAsync` is a no-op there. The
|
||
comment directly above line 1298 documents that divergence without drawing the conclusion. Only
|
||
a rig-backed test can see this.
|
||
|
||
**2. `SmsNotificationE2ETests.SmsConfigPage_CreateOrRender_NeverLeaksAuthToken` — a stale test
|
||
fixture.** No server-side error at all: the page renders 200, and no `INSERT INTO
|
||
SmsConfigurations` is ever issued. The test's fixture SID is `ACtest123` (`d6ead8ae`,
|
||
2026-06-19). `SmsConfiguration.razor:231` rejects anything not matching `^AC[0-9a-fA-F]{32}$`,
|
||
added by `40088a21` (2026-07-10) to close an un-escaped URI-interpolation hole. `Save()` sets
|
||
`_formError` and returns — no toast, exactly as observed. The fixture was never updated.
|
||
|
||
This has been failing since 2026-07-10, and it matters more than a red line: everything after
|
||
the toast assertion — including **the secret-non-leak assertion that the Auth Token value never
|
||
reaches the page HTML** — has not executed since. Fix is a valid 32-hex SID in the fixture.
|
||
|
||
### Not covered by this gate
|
||
|
||
- Streaming subscriptions were exercised in-process (TestServer), not over the rig. The
|
||
interceptor is path-scoped, not method-scoped, so the rig's `PullAuditEvents` evidence covers
|
||
the same code path — but a live `SubscribeInstance` under load is untested here.
|
||
- Key **rotation** on a live pair.
|
||
- `docker-env2` was updated with its own key but not redeployed or gated.
|