Repository navigation
Configure verified TLS for the PostgreSQL demo - #2700
nellshamrell wants to merge 3 commits into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Nell Shamrell-Harrington <nells@microsoft.com>
526fef3 to
ebbb759
Compare
Approver reviewReviewed head: 1. OrientationThis PR makes the PostgreSQL demo connect securely to the TLS-enforcing Kubernetes recipes. The implementation is small and consistent with its stated purpose.
2. What the code changesThe diff contains 46 added lines across two files, with no application-code or dependency changes:
The existing image, database name, username, password selection, and modeled connections remain unchanged. This PR configures the client; server-side TLS enforcement belongs to radius-project/resource-types-contrib#419. 3. Data and control flowThe deployment references the operator-created The application then:
I checked the tagged 4. Key implementation decisionsCan deployment configuration replace a client-code change?Code's answer: Set Why it matters: The existing driver already supports the required behavior. A code change would introduce another configuration path and require a new image without adding necessary functionality. Alternative: Construct an explicit Assessment: The configuration-only choice is sound for this sample. Driver-source inspection and the recorded runtime tests support it. Does the client receive only public trust material?Code's answer: Mount a separate Why it matters: The client needs to authenticate the server, not possess its private key. The combined integration script also checked that the CA Secret contained only Assessment: The separation is correct. The recipe mounts the whole named Secret, so keeping that Secret public-only remains an operator responsibility. Should optional server policy make the demo use plaintext?Code's answer: No. The demo always requests verified TLS ( Why it matters: "Optional" permits other clients to use plaintext; it does not require this client to weaken its connection. Alternative: Branch on the projected server policy. That adds complexity and could turn an intentional server allowance into an unnecessary client downgrade. Assessment: Always using verified TLS is the simpler and safer choice. 5. Failure handling and operationsTrust failures remain failures, rather than triggering a fallback. PostgreSQL connection errors propagate from repository creation; they do not cause There are two operational distinctions:
The existing application creates a pool per repository request. Its request-handler cleanup begins after Database restarts deserve particular care: the recipes do not provide durable storage. The explicit backup warning in 6. Security and compatibilityThe implementation verifies both the certificate chain and the hostname. The required DNS subject alternative name, the certificate field identifying valid server names, matches the recipe's returned Service hostname (
The compatibility break is intentional: this template no longer works with plaintext-only PostgreSQL recipes ( No new permissions, packages, authentication rules, or credential outputs are introduced. 7. Test evidence and gapsThe evidence exercises the changed behavior, not just compilation. I inspected the preserved combined-run report, probe scripts, demo logs, and build log for this exact samples commit. The combined-run summary is also recorded in radius-project/resource-types-contrib#419.
The Bicep cases used the docs-embedded template; the report records that it was byte-identical to the samples template. The principal gap is durable automation. Standard demo CI deploys The recorded run used local PR recipe artifacts. It does not prove that subsequently published public artifacts contain the same implementation. No new cluster deployment was run during this review. 8. Risk register
9. FindingsBlocking findings: None identified in the changed code. Non-blocking concern: Preserve the PostgreSQL verification matrix as repeatable CI coverage. The contract introduced at Strengths: The PR uses existing driver behavior, keeps private keys off the client, preserves credential selection, and documents the release and storage hazards. Residual risks: Public artifact publication remains unverified, 10. Approval recommendationApprove with follow-up. The configuration is correct for the inspected driver and is supported by substantive integration evidence against the exact samples head. Require coordinated recipe publication before rollout, and track a durable PostgreSQL TLS regression test; a failed smoke test against the published artifacts would change this recommendation. 11. Coverage and confidenceBoth changed files were reviewed completely, along with the database factory, request handlers, image configuration, relevant workflow behavior, external volume/hostname contracts, and tagged driver implementation. I ran merge-base diff validation and refreshed CI/review status; I inspected rather than reran the recorded integration tests. Architecture, trust, failure propagation, lifecycle, compatibility, rollout, and rollback were covered. Schema migrations, new concurrency mechanisms, and performance benchmarking were not applicable to this configuration-only change. The outstanding release question is whether the public recipe artifacts match the tested local artifacts. Confidence: high in the implementation; medium in release readiness until published artifacts are checked. This assessment is a comment, not a formal GitHub approval. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Nell Shamrell-Harrington <nells@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Nell Shamrell-Harrington <nells@microsoft.com>
Summary
Make the PostgreSQL demo work with TLS-enforcing recipes by configuring verified TLS and mounting only the public CA certificate. The existing image and client are unchanged: no TypeScript, dependency, or password-selection changes.
PGSSLMODE=verify-fullenables certificate and hostname verification;NODE_EXTRA_CA_CERTSloads the mounted CA. The server's private key is never mounted in the demo.Reason for change
Companion to the recipe fix, radius-project/resource-types-contrib#419, and tutorial update, radius-project/docs#2054.
This targets
edge. Coordinate publication of the recipes, sample, and docs: older plaintext-only recipes are incompatible with this configuration. No new demo image is needed.How to test
From the repository root:
Follow the README's certificate setup in an isolated TLS-capable environment. Deploy
samples/demo/app-postgresql.bicep, wait for the database and demo rollouts, then create, list, and delete a todo. Repeat with both recipes underrequiredandoptional: the demo should use TLS in both modes. Wrong-CA and hostname-mismatch cases must fail without plaintext fallback.Validation passed: the local build and actual demo database, HTTP, trust-rejection, and password-precedence checks with both recipes under required/optional policies.
File change summary
samples/demo/app-postgresql.bicepsamples/demo/README.md