Repository navigation
fix(#3780): a record-driven deploy layers the Key Vault values half, and the chart refuses to invent a database host - #4356
Conversation
…and the chart refuses to invent a database host The operator's hosting-deploy fed `helm upgrade` the record's render alone. That render is secret-free by construction, the chart's memex-portal-secrets renders from secrets.<half>.*, and helm replaces a Secret wholesale — so every record-driven Reconcile of an instance on an external database rewrote ConnectionStrings__orleans with the chart's in-cluster default (Host=memex-postgres-service, a Service no AKS release renders) and every new pod died at silo start on a name that never resolved. Measured on the control instance at helm revision 44 (2026-09-09) and revision 55 (2026-09-14, Ops/Actions/reconcile-memex-20260914-nav-rail); the config repo's lane, which layers `-f vault-values.yaml -f <overlay>`, rolled the same image healthy in between. - hosting-deploy --vault <name>: layers helm-values-<release> FIRST, then the record's render; read by the operator's identity into a 0600 temp file, never echoed, size-only in the log; REQUIRED where the flag is given (a vault with none refuses before helm, naming capture). - templates/_database.tpl: memex.orleansConnectionString fails the render on an external database when neither connection string is supplied, instead of manufacturing the in-cluster host. The mesh string stays a shadowed placeholder by design (CSI classes). - check-chart-invariants.sh gains REFUSALS: a fixture of exactly the #3780 render must fail to template naming the input (negative control measured: the pre-fix template renders it). - operator suite: 11 new cases (464/464; 9 of them red against the pre-fix script). - Doc: DeploymentEnvLayers → "Layer 2 on the operator path"; What's New (Fix). The Plugins half (the plan passes --vault when the record declares vaultValuesKeys) follows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Critical argument-handling and rollout-coordination issues remain unresolved, along with related vault-error and documentation corrections.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR restores the Key Vault values layer for record-driven AKS deployments and rejects unsafe external database defaults.
Changes:
- Adds secure
--vaultretrieval, layering, cleanup, and tests. - Adds Helm refusal and invariant coverage for missing Orleans connection strings.
- Updates deployment documentation, release notes, and CI coverage.
File summaries
| File | Summary |
|---|---|
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-14-a-record-driven-deploy-layers-the-key-vault-values-half.md |
Announces the fix; nit (1 vote): defer or mark the claim pending the paired Plugins change. |
src/MeshWeaver.Documentation/Data/Architecture/DeploymentEnvLayers.md |
Documents deployment layering; moderate (1 vote) rollout-alignment and nit (1 vote) CSI wording updates remain. |
deploy/helm/templates/_database.tpl |
Refuses unsafe external database defaults; critical (1 vote): coordinate with the Plugins rollout or gate the refusal; nit (1 vote): update stale operator documentation. |
deploy/aks/scripts/testdata/values.adonet-external-db-no-connection-string.yaml |
Adds the invalid-render regression fixture. |
deploy/aks/scripts/check-chart-invariants.sh |
Validates the database refusal behavior. |
deploy/aks/operator/test/stubs/deploy/az |
Stubs Key Vault operations for operator tests. |
deploy/aks/operator/test/run-tests.sh |
Covers vault loading, layering, and refusal behavior. |
deploy/aks/operator/bin/hosting-deploy |
Layers vault values securely; critical (2 votes): validate missing or empty --vault values; moderate (1 vote): distinguish missing secrets from access or transport failures. |
.github/workflows/hosting-operator.yml |
Tracks the operator test stub in CI. |
Review details
Suppressed comments (4)
deploy/helm/templates/_database.tpl:83
- The new refusal formalizes the behavior that an external
ConnectionStrings__memexcan be used to derive Orleans when no explicit Orleans string is present, but the repository still documents the old opposite contract indeploy/helm/values.yaml:861-864anddeploy/aks/README.md:1488-1492(both say this shape must fail). Update those operator-facing references so users are not told to supply an unnecessary second credential.
{{- if and (not $orleans) (not .root.Values.postgres.enabled) -}}
{{- fail (printf "memex.orleansConnectionString: '%s' runs AdoNet clustering on an EXTERNAL database (postgres.enabled is false) but names no database for cluster membership: neither secrets.%s.ConnectionStrings__orleans nor secrets.%s.ConnectionStrings__memex is set. The only value left would be the chart's in-cluster default Host=memex-postgres-service, a Service this release does not render — every new pod would then fail at silo start ('MembershipTableManager' failed to start … Name or service not known; MeshWeaver#3780). Supply the connection string in values: on a record-driven deploy that is the Key Vault values half (helm-values-<release>, layered by hosting-deploy --vault when the record declares vaultValuesKeys); on the helm-release lane it is layered as vault-values.yaml. Refusing to render." .half .half .half) -}}
src/MeshWeaver.Documentation/Data/Architecture/DeploymentEnvLayers.md:382
- This sentence says the provisioned instance “reads nothing from any vault,” even though the parenthetical says its connection string comes from a CSI class, which is itself a Key Vault-backed read. Clarify that the operator values-half fetch is skipped; otherwise this reverses the documented Layer 3/CSI behavior and can mislead operators about where the credential comes from.
instance (`vaultValuesKeys: []`, its connection string on a CSI class) reads nothing from any
vault and keeps its one-layer shape.
src/MeshWeaver.Documentation/Data/Architecture/DeploymentEnvLayers.md:376
- The operator-side option is present here, but the paired plan change is explicitly deferred to MeshWeaver.Plugins. On a deployment running only this commit, the record-driven path still supplies no
--vault, so the statement that neither path can produce the bad render is not true yet. Keep this claim aligned with the end-to-end rollout (or label the plugin half as pending).
Three things changed, so that neither path can produce that render again:
1. **`hosting-deploy --vault <name>` layers `helm-values-<release>` FIRST**, then the record's
render — the lane's order, so the reviewed record still wins on every key it declares. The half
is read by the operator's own identity into a `0600` temp file, removed on exit, never echoed;
only its byte count is logged (`::hosting:: vault_values_bytes=`). Where the flag is given the
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-14-a-record-driven-deploy-layers-the-key-vault-values-half.md:41
- This entry is shipped from core, but the PR description explicitly defers the plan change that passes
--vaultto MeshWeaver.Plugins. Until that paired change is deployed, record-driven Provision/Reconcile still invokeshosting-deploywithout this flag and can recreate the #3780 render, so publishing this bullet as an already-changed behavior is misleading. Please defer this claim/What's New entry to the paired change or mark it pending until the plan is live.
- **The plan passes `--vault` when the record declares `vaultValuesKeys`** — the record's own
statement that the chart's Secret carries keys nothing in the record renders. A provisioned
instance with no such declaration keeps its one-layer shape. (This half ships with the Hosting
plugin.)
- Files reviewed: 8/9 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| {{- if and (not $orleans) (not .root.Values.postgres.enabled) -}} | ||
| {{- fail (printf "memex.orleansConnectionString: '%s' runs AdoNet clustering on an EXTERNAL database (postgres.enabled is false) but names no database for cluster membership: neither secrets.%s.ConnectionStrings__orleans nor secrets.%s.ConnectionStrings__memex is set. The only value left would be the chart's in-cluster default Host=memex-postgres-service, a Service this release does not render — every new pod would then fail at silo start ('MembershipTableManager' failed to start … Name or service not known; MeshWeaver#3780). Supply the connection string in values: on a record-driven deploy that is the Key Vault values half (helm-values-<release>, layered by hosting-deploy --vault when the record declares vaultValuesKeys); on the helm-release lane it is layered as vault-values.yaml. Refusing to render." .half .half .half) -}} |
There was a problem hiding this comment.
The order of failures is the other way round, and that is why the refusal is not gated. This branch fires only where postgres.enabled is false AND neither connection string is supplied — i.e. exactly the render that today manufactures Host=memex-postgres-service;…;Database=orleans and crash-loops every new pod at MembershipTableManager (helm revisions 44 and 55 on the control instance). A single-replica record (Localhost clustering, every provisioned instance so far) emits no orleans key and never reaches this branch; the lane always supplies the vault half and never reaches it either. So an operator image carrying this chart but a plan without --vault does not lose a working deploy path: the Reconcile that used to succeed at helm and kill the silo now refuses before helm naming the input. A gate that manufactures a value for a missing input is the shape #3780 exists to remove, and gating the refusal on the plan's capability would put it back. The rollout order is stated in both PR bodies: this half first (the script must understand the flag), then MeshWeaver.Plugins#1892 (the plan emits it), then the record's operator.image bump — and at every intermediate state the worst outcome is a loud refusal, never a wrong render.
Closes nothing yet — half of #3780 (the Plugins half, the plan passing
--vault, follows in MeshWeaver.Plugins).What was measured
Ops/Actions/reconcile-memex-20260914-nav-rail(a record-driven Reconcile, 20:13:57Z) ranhelm upgrade memex /opt/hosting/chart … -f /tmp/hosting-values.yaml— one values file, "rendered from the record" — producing helm revision 55; the surge podmemex-portal-deployment-77476b68-l4gw8then failedMembershipTableManagerat 20:15:23Z and 84 more times in the hour. Same mechanism as revision 44 on 2026-09-09. Every lane-driven deploy in between (helm-release.yml:-f vault-values.yaml -f <overlay>) rolled the same image healthy, and Roll/Restart never touch the Secret.The record's render is secret-free by construction;
memex-portal-secretsrenders fromsecrets.<half>.*, which formemexlive only in Key Vaulthelm-values-memex(the record'svaultValuesKeysnamesConnectionStrings__orleansamong them). With nosecrets:block the chart'sdefaultmanufacturedHost=memex-postgres-service;…;Database=orleans— a Service the release does not render (postgres.enabled: false).ConnectionStrings__memexsurvived only because thememex-kvCSI class shadows it later inenvFrom; nothing shadows the orleans string.What changes
hosting-deploy --vault <name>layershelm-values-<release>FIRST, then the record's render (the lane's order). Read by the operator's own identity into a0600temp file, removed on exit, never echoed — only::hosting:: vault_values_bytes=is logged. Where the flag is given the half is required: a vault that holds none refuses before helm, naminghelm-release capture.memex.orleansConnectionStringfails the render on an external database when neither connection string is supplied.check-chart-invariants.shgains a REFUSALS section with a fixture of exactly the Portal silo fails to start — orleans clustering database host does not resolve (memex) #3780 render, which must fail to template naming the input.DeploymentEnvLayers; What's New (Fix).Verification
deploy/aks/operator/test/run-tests.sh: 464 passed, 0 failed; against the pre-fixhosting-deploy(swapped in fromorigin/main): 455 passed, 9 failed — the new cases are the failures.deploy/aks/scripts/check-chart-invariants.sh: 8 combinations render, 1 refusal holds, exit 0; against the pre-fix_database.tpl:refusal … RENDERED, exit 1.shellcheck -S warningclean on every operator script;dotnet build src/MeshWeaver.Documentation -c Release -warnaserror0/0;DocumentationLinkIntegrityTest+NoConflictMarkersGuardpassed (2/2, trx present).Left deliberately open (stated in the doc)
The mesh string's chart-side copy on a record-driven instance is a shadowed placeholder; with the #4173 probe on the next operator image, an instance with no vault half will hold at
Init:0/1namingmemex-postgres-service— a diagnosable stall, and the reason the record path should eventually feeddatabaseConnectionSecretintosecrets.<half>.ConnectionStrings__memextoo.🤖 Generated with Claude Code