docs: site secrets ride the artifact deploy; gRPC hub retired in ScadaBridge

This commit is contained in:
Joseph Doherty
2026-09-28 20:22:45 -04:00
parent bc29dc412b
commit 0ccf68951d
8 changed files with 259 additions and 6 deletions
@@ -120,6 +120,7 @@ All 7 fix-now items landed via PLAN-04/05/06/07/08 (verified in review 08 round
| 124 | **No template-graph-watermark short-circuit on the pending-updates fleet flatten** *(Low)*. `PendingUpdatesEvaluator.EvaluateAsync` re-flattens every deployed instance through `DeploymentService.GetInstanceStalenessAsync` on every cache-TTL expiry (`PendingUpdatesCacheTtl`, default 60 s) while any Deployer/Admin circuit is open, even when nothing in the template/instance/artifact graph has changed since the last evaluation. | `docs/plans/2026-09-14-pending-updates-design.md` §3.2 ("Not in scope") | The single-flight TTL cache already bounds the cost to one fleet flatten per TTL fleet-wide (not per viewer); a watermark short-circuit is a further optimization, not a correctness need, and needs a cheap "has anything changed" signal that does not exist yet. | The fleet flatten's cost (instance count x template depth) becomes measurable against `PendingUpdatesCacheTtl` on a large deployment. |
| 125 | **Pending-updates counts have no Health-dashboard tile or CLI surface** *(Low)*. The aggregate (`PendingUpdatesSnapshot.StaleInstanceCount` / `StaleArtifactSiteCount`) is consumed only by the Central UI Sites page and rail indicator; there is no Health dashboard KPI tile and no `deploy`/`site` CLI command surfacing it — `site list` shows only the raw `artifactRevisionHash`/`artifactsDeployedAt` fields on each site, not whether they are current. | `docs/plans/2026-09-14-pending-updates-design.md` §3.2 ("Not in scope") | Out of scope for the UI-focused ask; the CLI/Health surfaces are a separate consumer of the same `IPendingUpdatesCache`. | An operator wants the pending-updates counts without opening the Central UI (headless monitoring, CLI scripting). |
| 126 | ~~**TODO — roll the two outstanding migrations to the QA pair vq03/vq04**~~ **DONE 2026-09-28** — QA vq03/vq04 upgraded `2b9a7c3c` → `b3021e7c`, 55 → 57, artifacts deployed (record `docs/deployment/deployment-records/2026-09-28-wonder-app-vq03-vq04-b3021e7c.md`). *(deployment, closed)*. **DEV vd03 DONE 2026-09-28** (`fe89363b`, 55 → 57, record `docs/deployment/deployment-records/2026-09-28-wonder-app-vd03-fe89363b.md` — the `sb-mig-55to57.sql` there is the same script QA needs). QA still runs `2b9a7c3c` with the DB at migration 55 (`20260909203046_EnforceGlobalInstanceNamesAndProtectProtocolConfiguration`); `main` (`ac833f5a`, pushed 2026-09-14) carries two more: **56** `20260911135501_AddAlarmMesReportable` (computed-alarm `IsMesReportable` flag) and **57** `20260914173147_AddSiteArtifactRevision` (`Sites.ArtifactRevisionHash`/`ArtifactsDeployedAt`, both nullable). Both are additive `AddColumn`s — no data upgrade, no coordinated step like the 2026-09-10 protocol-config encryption. Recipe (same as the 55 rollout): generate ONE from→to delta script `dotnet ef migrations script EnforceGlobalInstanceNamesAndProtectProtocolConfiguration AddSiteArtifactRevision --project src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase --startup-project src/ZB.MOM.WW.ScadaBridge.Host -o sb-mig-55to57.sql` (NOT idempotent, identical for vd03 and the QA pair), take the pre-upgrade DB backup, stop the pair TOGETHER, apply via `sqlcmd -C -b -I` on the SQL host, publish/swap the `ac833f5a` binaries, start the pair, confirm the Central boot log shows 0 pending migrations and no "Applying migration" (the script already applied them). After upgrade every site reads *Artifacts never deployed* until one *Deploy Artifacts to All Sites* — run it as the last step. Write the deployment record `docs/deployment/deployment-records/<date>-wonder-app-{vd03,vq03-vq04}-ac833f5a.md`. | Pending-updates finish (2026-09-14) | ~~Blocked by the 2026-09-11 remote-access hold~~ — **unblocked 2026-09-28**: `winrm` is re-enabled for deployment activities (CLAUDE.md). vd03 done; QA done 2026-09-28. | Closed 2026-09-28. |
| 127 | **TODO — roll out site secrets via artifact deploy to vd03 + QA (vq03/vq04)**. The gRPC secrets hub is retired in ScadaBridge as of `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md` (implemented on branch `feat/site-secrets-artifact-deploy`, rig-only so far); vd03 and QA still run the pre-feature build with `Secrets:Replication:Enabled=false`, so upgrading them to a build carrying this change will **not** itself fail fast (the retirement's `StartupValidator` rejection only fires on `Enabled=true`, which neither environment sets) — but a site still needs the four prerequisites below before *Deploy Artifacts* will actually deliver its secrets. Prerequisites, in order: (1) provision the `zbsecrets` schema/store in the environment's configuration SQL Server (the same server `ConfigurationDb` already points at); (2) set `Secrets:Store:Provider=SqlServer` on **both** central nodes of the environment; (3) verify the KEK (`ZB_SECRETS_MASTER_KEY` / master-key file) is identical across **every** node — both centrals and every site node — since a mismatch is caught only at deploy time by `DeployedSecretsMirror`'s per-secret verify, not at boot; (4) recreate, at the new central shared store, any secret an operator previously hand-seeded directly into a site's local `secrets.db` (those never round-tripped through central and would otherwise be silently absent from the referenced-secrets scan). Then run *Deploy Artifacts* against every affected site as the rollout's last step. | `docs/plans/2026-09-28-site-secrets-artifact-deploy-design.md` §4 *Rollout*; `docs/deployment/topology-guide.md` → *Secrets: shared store, per-site deploy, and the KEK* | Deployment record, not a code change — vd03/QA config is user-held and their rollout schedule is independent of the branch merging to main. | User asks for the vd03/QA secrets window. |
## Resolved (verified against the code 2026-07-10)
Rows removed from the Deferred table above once confirmed shipped. Kept here for traceability.
@@ -152,3 +152,76 @@ site secrets recreated at central; then *Deploy Artifacts*. Tracked as a deferre
Per-site explicit secret lists; secrets for external systems / database connections (they ship
literal values today); plaintext transport; moving bootstrap secrets through the deploy.
## 6. Implementation notes (as built)
Execution (tasks 1–11, ledger `.superpowers/sdd/2026-09-28-site-secrets-artifact-deploy/progress.md`)
matched this design in shape but landed several deviations, called out here so the doc stays
accurate as the spec:
- **Central collection.** The scanner and per-site resolve described in §3.1/§3.3 are implemented
as `SiteSecretsCollector` (`ZB.MOM.WW.ScadaBridge.DeploymentManager`), not inline in
`ArtifactDeploymentService`. Fail-closed (§3.3) is per site, not per batch: a site with an
unresolved reference is skipped (`_communicationService` is never called for it) while every
other site in the same sweep proceeds normally.
- **Audit (§3.7).** The `DeploySiteSecrets` / `SiteSecrets` audit row is written on **both**
`DeployCoreAsync` (the sweep) and `RetryForSiteAsync` (the per-site retry), including the
fail-closed path on each — the design's §3.7 wording ("one row per site deploy that carried
secrets") did not call out the retry path explicitly; it is in scope and covered.
- **Stamp gating (§3.6).** Implemented as designed, plus one addition: a **partial apply**
(`SecretsApplied` less than the shipped count, not just the mixed-version `null` case) is also
treated as a failure and is not stamped, with its own message (`Site applied {n} of {m}
secrets`) distinct from the "predates secret deployment" wording.
- **Hash (§3.4).** Implemented as designed. Confirmed explicitly (not just implied by
"null-when-empty"): an **empty** `Secrets`/`MissingSecrets` list and a **null** one hash
identically, and the pre-existing golden hash for a site with no secret references is
unchanged by this feature (pinned by a dedicated regression test).
- **Site apply (§3.5).** `HandleDeployArtifacts` writes the full `deployed_secrets` set and then
runs `DeployedSecretsMirror.ReconcileAsync` **synchronously** before replying, exactly as
designed. `SecretsApplied` counts only the names *this deploy shipped* that applied and
verified without error — a mirror failure for a name this deploy did not ship (e.g. another,
already-broken deployed secret, or an unrelated removal) is logged at Warning, excluded from
the count, and does not fail the deploy. Removal failures are likewise Warnings, never deploy
failures.
- **`DeployedSecretsMirror` — apply is confirm-then-ledger, not the blind `ApplyReplicatedAsync`
§3.5 originally described.** The real library store is strict last-writer-wins on
`(UpdatedUtc, Revision)` (a tie is a no-op) and tombstones rather than hard-deletes, which the
naive "apply then ledger" sequence gets wrong in two ways: a routine remove-then-re-add can
no-op against a still-live tombstone while the ledger is written anyway (silently broken), and
an out-of-band local secret with a genuinely newer stamp can be ledgered and later deleted by a
central tombstone (destroying the operator's own secret). The mirror now always reads the row
back after applying and only ledgers when the local ciphertext matches what was shipped; a
tombstoned or ledger-owned row is safely re-stamped (`UpdatedUtc`/`Revision` bumped past the
local value, including across clock skew) and re-applied, while a live row with different
ciphertext that the ledger does **not** already own is left completely untouched and reported
as a distinct out-of-band-conflict failure. Ownership is decided from `mirrored_secrets`
membership at any revision, not from tombstone state alone — so a mirror-owned name that drifts
locally (an operator's edit over an already-deployed name) is re-stamped back to the deployed
value on the next pass: **central is authoritative for every name it deploys.** Every apply/
delete also invalidates the shared `ISecretResolver` cache before the verify step, and one
poisoned row never blocks the rest of the pass.
- **Hub retirement (§3.8) — scope widened to ALL secrets replication, not just the gRPC hub.**
ScadaBridge also retires the historical SQL-Server *replication* mode
(`Secrets:Replication:Mode=SqlServer`, distinct from the still-supported
`Secrets:Store:Provider=SqlServer` shared-store feature) for the same reason the gRPC hub goes:
a site's `ISecretStore` must be the plain, undecorated local SQLite store, because
`DeployedSecretsMirror` calls `DeleteAsync` directly and a replication decorator would publish
that delete fleet-wide. `Secrets:Replication:Enabled` set to anything other than
absent/empty/`false` (a deliberately strict, non-boolean-coerced check — `"0"`/`"1"`/`"yes"`
all count as "set") now fails `StartupValidator.Validate` on **either** role. A leftover
`Secrets:Replication:Mode` or any `Secrets:GrpcHub:*` key logs a startup Warning naming both
sections. The pre-host `${secret:}` expander was fixed in the same task set to read the shared
SQL store on a `Store:Provider=SqlServer` central (it previously only did this for the retired
Grpc mode) — without that fix a central's boot-time `${secret:}` tokens and its running
`ISecretResolver` could resolve a token differently.
- **`ZB.MOM.WW.Secrets` 0.6.3 (hub sweep logging, §3.7's third bullet — `EventId`s 4/5/6,
`ISecretsHubSyncStatus`) was implemented and tested on a `scadaproj` branch but is NOT
published to the package feed, and ScadaBridge does not consume it.** Publishing is an outward
side effect that needs explicit user consent, and it would be a dead pin here regardless —
hub retirement (above) removes ScadaBridge's only dependency on the hub client entirely. It
remains available for the library's other hub consumers once published.
- **Rig posture.** `docker/docker-compose.yml`'s central nodes now carry
`Secrets__Store__Provider=SqlServer` plus a shared KEK; site-a shares that KEK (it is the
secrets-via-artifact-deploy site); site-b/site-c deliberately carry a **mismatched** KEK so the
`DeployedSecretsMirror` verify-failure path has a real, provable negative on the rig rather than
an assumed one.