feat: add deployment pipeline, Helm charts, and Terraform infrastructure#414
feat: add deployment pipeline, Helm charts, and Terraform infrastructure#414linusnorton wants to merge 68 commits intomasterfrom
Conversation
Adds full CI/CD pipeline adapted from the expressjs-monorepo-template, targeting SDS clusters (dev/stg/prod) with the cath namespace. - Fix Dockerfiles for web, api, and crons apps (multi-stage builds) - Add postgres Dockerfile with migration runner and Prisma Studio - Add postgres Helm chart with pip-ss-kv Key Vault secrets - Add umbrella Helm chart (cath-service) aggregating all subcharts - Add Helm values templates for preview, staging, and production - Add 3-tier GitHub Actions pipeline (orchestrator/stage/job workflows) - Add Terraform infrastructure (PostgreSQL, Redis, App Insights, Key Vault) - Fix crons Helm values (correct image, identity, Key Vault references) - Add team annotations to all Chart.yaml files - Remove old CI-only workflows (test.yml, e2e.yml, osv-scanner.yml)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved legacy E2E/OSV workflows and replaced them with a modular, reusable GitHub Actions CI/CD pipeline: change detection, staged build/deploy/promote workflows, Helm deployments, Terraform infra, helper scripts, umbrella Helm chart, Dockerfile/runtime adjustments and a new Postgres service with health proxy. (34 words) Changes
Sequence DiagramsequenceDiagram
actor Dev
participant GitHub
participant Detect as Detect Changes
participant Build as Build Stage
participant Infra as Infra Stage
participant Deploy as Helm Deploy
participant Smoke as Smoke Test
participant E2E as E2E Test
participant Promote as Promote Images
Dev->>GitHub: push / open PR
GitHub->>Detect: run change detection
Detect-->>Build: code change? (affected-apps, helm-apps)
Detect-->>Infra: infra change?
alt Code changes present
Build->>Build: lint, test, build & publish images
Build-->>Deploy: metadata (short-sha, timestamp, affected-apps, helm-apps)
end
alt Infra changes present
Infra->>Infra: terraform fmt / plan/apply
Infra-->>Deploy: infra ready
end
Deploy->>Deploy: helm deploy, register DNS, produce URLs
Deploy-->>Smoke: deployment URLs
Smoke->>Smoke: health checks (web, api)
Smoke-->>E2E: validated URLs
E2E->>E2E: run E2E tests
E2E-->>Promote: on success, trigger image promotion & helm publish
Promote->>GitHub: cache successful SHA
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/job.test.yml (3)
56-56:⚠️ Potential issue | 🔴 Critical
actions/upload-artifact@v6does not exist — workflow will fail.The latest major release is
upload-artifact@v4. Version 6 does not exist; using it will cause the step to fail immediately. Downgrade to@v4.🐛 Proposed fix
- uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@v4
65-65:⚠️ Potential issue | 🔴 Critical
actions/download-artifact@v7does not exist — workflow will fail.The latest major release is
download-artifact@v4, and upload and download versions must match. Update to@v4to pair with the upload step.🐛 Proposed fix
- uses: actions/download-artifact@v7 + uses: actions/download-artifact@v4
78-80:⚠️ Potential issue | 🟠 Major
SonarSource/sonarqube-scan-action@masteris unpinned.Referencing a mutable branch tag is a supply-chain risk — a compromised push to
masterruns in your pipeline. Pin to a specific release tag or a commit SHA.
🟡 Minor comments (5)
.github/workflows/job.helm-cleanup.yml-41-55 (1)
41-55:⚠️ Potential issue | 🟡 MinorMisleading summary: "Status: Deleted" is always written even when the release was absent.
The
|| echo "Release may not exist..."on Line 47 silences the error, but Line 55 unconditionally appends**Status:** Deletedto the step summary. A missing release would report a false success.Capture the exit code to report the real outcome:
🐛 Proposed fix
- - name: Delete Helm release - run: | - echo "Deleting Helm release: ${{ inputs.release-name }} in namespace: ${{ inputs.namespace }}" - helm uninstall "${{ inputs.release-name }}" \ - --namespace "${{ inputs.namespace }}" \ - --wait \ - --timeout 5m0s || echo "Release may not exist or already deleted" - - - name: Output cleanup summary - run: | - echo "### Helm Cleanup Summary :broom:" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Release:** \`${{ inputs.release-name }}\`" >> $GITHUB_STEP_SUMMARY - echo "**Namespace:** \`${{ inputs.namespace }}\`" >> $GITHUB_STEP_SUMMARY - echo "**Status:** Deleted" >> $GITHUB_STEP_SUMMARY + - name: Delete Helm release + id: helm_uninstall + run: | + echo "Deleting Helm release: ${{ inputs.release-name }} in namespace: ${{ inputs.namespace }}" + if helm uninstall "${{ inputs.release-name }}" \ + --namespace "${{ inputs.namespace }}" \ + --wait \ + --timeout 5m0s; then + echo "status=Deleted" >> "$GITHUB_OUTPUT" + else + echo "status=Not found (skipped)" >> "$GITHUB_OUTPUT" + fi + + - name: Output cleanup summary + run: | + { + echo "### Helm Cleanup Summary :broom:" + echo "" + echo "**Release:** \`${{ inputs.release-name }}\`" + echo "**Namespace:** \`${{ inputs.namespace }}\`" + echo "**Status:** ${{ steps.helm_uninstall.outputs.status }}" + } >> "$GITHUB_STEP_SUMMARY"This also addresses the SC2086/SC2129 shellcheck warnings from static analysis.
.github/workflows/jobs/build-and-publish-images/set-image-variables.sh-31-34 (1)
31-34:⚠️ Potential issue | 🟡 Minor
short_shais not validated but is used in image tags for affected apps.If
short_shais empty, affected apps will receive tags likepr-123-orstaging-(trailing hyphen), which are likely invalid or at least unintended. Addshort_shato the validation guard when there are affected apps, or at minimum to the usage check.Proposed fix
- if [ -z "$helm_apps" ] || [ -z "$change_id" ] || [ -z "$application_name" ]; then + if [ -z "$helm_apps" ] || [ -z "$change_id" ] || [ -z "$short_sha" ] || [ -z "$application_name" ]; then echo "Usage: $0 <affected_apps_json> <helm_apps_json> <change_id> <short_sha> <timestamp> <application_name>" exit 1 fi.github/workflows/job.helm-deploy.yml-125-164 (1)
125-164:⚠️ Potential issue | 🟡 MinorDNS registration: quote shell variables and handle empty hostname list.
Line 136 leaves
${{ env.TEAM_NAME }}unquoted inside the kubectl call, and line 142 has an unquoted expansion inside${..}(SC2295). Also, if$HOSTNAMESis empty (no ingresses yet), the loop silently does nothing — consider adding an explicit check or warning.Suggested quoting fixes
- HOSTNAMES=$(kubectl get ingress -n ${{ env.TEAM_NAME }} \ + HOSTNAMES=$(kubectl get ingress -n "${{ env.TEAM_NAME }}" \ -l "app.kubernetes.io/instance=${{ env.RELEASE_NAME }}" \ -o jsonpath='{.items[*].spec.rules[*].host}') + if [ -z "$HOSTNAMES" ]; then + echo "::warning::No ingress hostnames found for release ${{ env.RELEASE_NAME }}" + exit 0 + fi + for HOST in $HOSTNAMES; do - RECORD_NAME="${HOST%.${ZONE}}" + RECORD_NAME="${HOST%."${ZONE}"}"infrastructure/variables.tf-6-16 (1)
6-16:⚠️ Potential issue | 🟡 MinorRedis and Application Insights modules produce redundant resource names with default variables.
With
product = "pip-ss-kv"andcomponent = "cath", the dedicated resource group becomespip-ss-kv-{env}-cath. However, bothredis.tfline 5 andappinsights.tfline 8 hardcode acath-prefix and then append${var.component}, resulting in:
- Redis:
cath-cath-{env}- Application Insights:
cath-cath-appinsightsThis double prefix is likely unintended. The
postgres.tfmodule avoids this by using only"cath-${var.env}", suggesting the hardcoded prefix may be redundant or the variable usage should be reconsidered..github/workflows/workflow.preview.yml-46-50 (1)
46-50:⚠️ Potential issue | 🟡 MinorAvoid deploying when the infra stage was cancelled or timed out.
needs.infrastructure-stage.result != 'failure'permitscancelled, so deploy could proceed after an incomplete infra run. Consider matching the main workflow’s guard (success or skipped).💡 Suggested fix
- if: always() && needs.build-stage.result == 'success' && needs.infrastructure-stage.result != 'failure' + if: | + always() && + needs.build-stage.result == 'success' && + (needs.infrastructure-stage.result == 'success' || needs.infrastructure-stage.result == 'skipped')
🧹 Nitpick comments (17)
apps/postgres/package.json (1)
32-32:http-proxyis unmaintained — consider an alternative.
http-proxy@1.18.1is the latest version on npm, last published 6 years ago. For a simple internal sidecar this is low-risk, but given it's unmaintained you may want to replace it with Node's built-inhttpmodule to avoid carrying an unloved dependency long-term.apps/postgres/health-server.mjs (1)
13-15: Readiness probe does not reflect Prisma Studio's actual availability.
/health/readinessreturns200unconditionally. During the window betweenhealth-server.mjsstarting and Prisma Studio becoming ready, Kubernetes may consider the pod ready and stop directing traffic to the 502 path. If accurate readiness signalling matters, consider pollinglocalhost:5556before returning200.apps/postgres/Dockerfile (2)
21-21:prismaCLI used at runtime but listed as adevDependency.
start.shinvokesnpx prisma migrate deployandnpx prisma studio, so theprismaCLI must be available in the runtime image. It's currently declared as adevDependency, meaningyarn workspaces focus --productionwould break the container. The missing--productionflag is therefore working around this, at the cost of shipping all dev dependencies in the image.Cleaner fix: move
prismatodependenciesand add--productionto thefocuscommand.♻️ Proposed change
- "prisma": "6.19.2" + "prisma": "6.19.2",Move
prismafromdevDependenciestodependenciesinpackage.json, then:-RUN yarn workspaces focus `@hmcts/postgres` +RUN yarn workspaces focus --production `@hmcts/postgres`
3-5: Combine corepack install steps into a singleRUNlayer.Two separate
RUNcommands create unnecessary intermediate layers.♻️ Proposed fix
-RUN npm install -g corepack -RUN corepack enable +RUN npm install -g corepack && corepack enable.github/workflows/job.terraform-fmt.yml (1)
28-33: Consider pinninghmcts/cnp-githubactions-library/terraform-fmtto a specific tag or SHA.The
actions/checkout@v6version is valid and follows GitHub's recommended approach of using major version tags that auto-update within the major release series. However, pinningterraform-fmtfrom@mainto a specific tag or SHA would improve supply-chain stability.apps/web/Dockerfile (1)
19-21:ENV NODE_ENV=productionshould precedeyarn workspaces focus.Minor ordering nit: the
--productionflag handles pruning correctly regardless, but setting the env var before theRUNkeeps the intent explicit and aligns with the api/crons convention.♻️ Proposed reorder
+ENV NODE_ENV=production +ENV NODE_OPTIONS='--conditions=production' + RUN yarn workspaces focus `@hmcts/web` --production -ENV NODE_ENV=production -ENV NODE_OPTIONS='--conditions=production'.github/workflows/job.helm-cleanup.yml (1)
27-33: Staging credentials and cluster are hardcoded, making this job non-reusable.The
release-nameandnamespaceinputs imply a generic cleanup job, butAZURE_CREDENTIALS_SDS_STG,sds-stg-00-rg, andsds-stg-00-akshard-wire it to the staging environment. If cleanup is ever needed for dev or prod, a new duplicate job would be required. Consider parameterising the environment, or at minimum rename to make the staging scope explicit in the job contract..github/workflows/job.smoke-test.yml (2)
93-98: Summary step is a no-op — it only runs when everything already succeeded.Since prior steps don't use
if: always(), this step is unreachable on failure. It's harmless but adds no diagnostic value. Consider removing it, or addingif: always()and printing a conditional pass/fail summary.
31-91: Web and API health-check blocks are near-identical.Consider extracting the retry logic into a small shared shell function or a separate script to reduce duplication. Not blocking, but would simplify future maintenance (e.g., changing retry count or adding headers).
.github/workflows/jobs/build-and-publish-images/set-image-variables.sh (1)
58-91: Pipeline subshell:while readin a pipe runs in a subshell.The
jq ... | while readconstruct means the loop body executes in a subshell. This is fine here because side effects are limited to appending to$GITHUB_ENV(file I/O persists) andecho(stdout). However, if you ever need to propagate a failure or accumulate state from the loop, this pattern will silently swallow it. A process substitution (while read ... < <(jq ...)) would avoid the subshell, but this is non-blocking as-is..github/workflows/job.detect-changes.yml (1)
104-110: Redundant PR base ref check.Lines 105–107 check
github.event.pull_request.base.refand lines 108–110 checkgithub.base_ref. Forpull_requestevents, these resolve to the same value. For non-PR events, both are empty. Unless there's a specific edge case you're targeting (e.g.,pull_request_target), the second branch is dead code..github/workflows/job.build-and-publish-images.yml (4)
46-56: Quote$GITHUB_OUTPUTto satisfy shellcheck SC2086.Flagged by static analysis. While the GHA-provided path won't contain spaces, quoting is good practice and silences the linter.
Proposed fix
- echo "sha=$SHA" >> $GITHUB_OUTPUT + echo "sha=$SHA" >> "$GITHUB_OUTPUT" ... - echo "sha=" >> $GITHUB_OUTPUT + echo "sha=" >> "$GITHUB_OUTPUT"
125-136: Same SC2086 quoting issue for$GITHUB_OUTPUT.Proposed fix
- echo "tag1=pr-${{ env.CHANGE_ID }}-${{ steps.metadata.outputs.short-sha }}" >> $GITHUB_OUTPUT - echo "tag2=pr-${{ env.CHANGE_ID }}" >> $GITHUB_OUTPUT + echo "tag1=pr-${{ env.CHANGE_ID }}-${{ steps.metadata.outputs.short-sha }}" >> "$GITHUB_OUTPUT" + echo "tag2=pr-${{ env.CHANGE_ID }}" >> "$GITHUB_OUTPUT" ... - echo "tag1=${{ env.CHANGE_ID }}-${{ steps.metadata.outputs.short-sha }}" >> $GITHUB_OUTPUT - echo "tag2=${{ env.CHANGE_ID }}" >> $GITHUB_OUTPUT + echo "tag1=${{ env.CHANGE_ID }}-${{ steps.metadata.outputs.short-sha }}" >> "$GITHUB_OUTPUT" + echo "tag2=${{ env.CHANGE_ID }}" >> "$GITHUB_OUTPUT"
138-149: Reusable action pinned to@main.
hmcts/cnp-githubactions-library/container-build-push-openid@maintracks a mutable branch. If this is an organisation convention, it's fine; otherwise consider pinning to a release tag or commit SHA for supply-chain safety.
115-119: Usefetch-depth: 1instead offetch-depth: 0to reduce CI/CD execution time across matrix jobs.The
generate-build-metadata.shscript only requires the remote URL (viagit config) and the current SHA, both of which are available without full history. Fetching only shallow history (depth 1) will speed up the checkout step for each matrix element without impact..github/workflows/stage.infrastructure.yml (1)
25-31: Environment and subscription values are hard-coded to staging.This stage will need parameterisation (environment, subscription, aks-subscription, storage-account) when production Terraform runs are introduced. For now this is functional, but worth noting for the planned multi-environment expansion.
.github/workflows/job.helm-deploy.yml (1)
65-85: Fragile YAML parsing and unquoted variables.Parsing
Chart.yamlwithgrep/awkis brittle — it will break if indentation or key ordering changes. Consider usingyqwhich is available onubuntu-latestrunners. Additionally, shellcheck flags several unquoted variable expansions (SC2086).Suggested improvement
- TEAM_NAME=$(grep -A 1 'annotations:' "$CHART_PATH" | grep 'team:' | awk '{print $2}' | tr -d '"') - APPLICATION_NAME=$(grep '^name:' "$CHART_PATH" | awk '{print $2}' | tr -d '"') + TEAM_NAME=$(yq '.annotations.team' "$CHART_PATH") + APPLICATION_NAME=$(yq '.name' "$CHART_PATH")
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (56)
.github/workflows/e2e.yml.github/workflows/job.build-and-publish-images.yml.github/workflows/job.detect-changes.yml.github/workflows/job.e2e-test.yml.github/workflows/job.helm-cleanup.yml.github/workflows/job.helm-deploy.yml.github/workflows/job.helm-publish.yml.github/workflows/job.lint.yml.github/workflows/job.osv-scanner.yml.github/workflows/job.pr-comment.yml.github/workflows/job.promote-images.yml.github/workflows/job.save-successful-sha.yml.github/workflows/job.smoke-test.yml.github/workflows/job.terraform-fmt.yml.github/workflows/job.terraform.yml.github/workflows/job.test.yml.github/workflows/jobs/build-and-publish-images/detect-affected-apps.sh.github/workflows/jobs/build-and-publish-images/generate-build-metadata.sh.github/workflows/jobs/build-and-publish-images/set-image-variables.sh.github/workflows/jobs/pr-comment/get-deployment-urls.sh.github/workflows/osv-scanner.yml.github/workflows/stage.build.yml.github/workflows/stage.cleanup.yml.github/workflows/stage.deploy.yml.github/workflows/stage.e2e.yml.github/workflows/stage.infrastructure.yml.github/workflows/stage.promote-images.yml.github/workflows/stage.smoke-test.yml.github/workflows/workflow.main.yml.github/workflows/workflow.preview.ymlapps/api/Dockerfileapps/api/helm/Chart.yamlapps/crons/Dockerfileapps/crons/helm/Chart.yamlapps/crons/helm/values.yamlapps/postgres/Dockerfileapps/postgres/health-server.mjsapps/postgres/helm/Chart.yamlapps/postgres/helm/values.yamlapps/postgres/package.jsonapps/postgres/start.shapps/web/Dockerfileapps/web/helm/Chart.yamlhelm/cath-service/Chart.yamlhelm/cath-service/values.preview.template.yamlhelm/cath-service/values.preview.yamlhelm/cath-service/values.template.yamlinfrastructure/.terraform-versioninfrastructure/appinsights.tfinfrastructure/keyvault.tfinfrastructure/main.tfinfrastructure/output.tfinfrastructure/postgres.tfinfrastructure/redis.tfinfrastructure/state.tfinfrastructure/variables.tf
💤 Files with no reviewable changes (2)
- .github/workflows/osv-scanner.yml
- .github/workflows/e2e.yml
| run: | | ||
| echo "### Helm Chart Publish Summary :package:" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $GITHUB_STEP_SUMMARY | ||
| echo "**Chart:** \`cath-service\`" >> $GITHUB_STEP_SUMMARY | ||
| echo "**Version:** \`${CHART_VERSION}\`" >> $GITHUB_STEP_SUMMARY | ||
| echo "**Registry:** \`oci://${{ env.REGISTRY }}/helm\`" >> $GITHUB_STEP_SUMMARY |
There was a problem hiding this comment.
Quote the summary path and consolidate redirects.
actionlint reports SC2086/SC2129 here; this can fail linting and block the workflow.
Proposed change
- name: Output publish summary
run: |
- echo "### Helm Chart Publish Summary :package:" >> $GITHUB_STEP_SUMMARY
- echo "" >> $GITHUB_STEP_SUMMARY
- echo "**Chart:** \`cath-service\`" >> $GITHUB_STEP_SUMMARY
- echo "**Version:** \`${CHART_VERSION}\`" >> $GITHUB_STEP_SUMMARY
- echo "**Registry:** \`oci://${{ env.REGISTRY }}/helm\`" >> $GITHUB_STEP_SUMMARY
+ {
+ echo "### Helm Chart Publish Summary :package:"
+ echo ""
+ echo "**Chart:** \`cath-service\`"
+ echo "**Version:** \`${CHART_VERSION}\`"
+ echo "**Registry:** \`oci://${{ env.REGISTRY }}/helm\`"
+ } >> "$GITHUB_STEP_SUMMARY"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: | | |
| echo "### Helm Chart Publish Summary :package:" >> $GITHUB_STEP_SUMMARY | |
| echo "" >> $GITHUB_STEP_SUMMARY | |
| echo "**Chart:** \`cath-service\`" >> $GITHUB_STEP_SUMMARY | |
| echo "**Version:** \`${CHART_VERSION}\`" >> $GITHUB_STEP_SUMMARY | |
| echo "**Registry:** \`oci://${{ env.REGISTRY }}/helm\`" >> $GITHUB_STEP_SUMMARY | |
| run: | | |
| { | |
| echo "### Helm Chart Publish Summary :package:" | |
| echo "" | |
| echo "**Chart:** \`cath-service\`" | |
| echo "**Version:** \`${CHART_VERSION}\`" | |
| echo "**Registry:** \`oci://${{ env.REGISTRY }}/helm\`" | |
| } >> "$GITHUB_STEP_SUMMARY" |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 70-70: shellcheck reported issue in this script: SC2086:info:1:52: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 70-70: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 70-70: shellcheck reported issue in this script: SC2086:info:3:39: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 70-70: shellcheck reported issue in this script: SC2086:info:4:45: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 70-70: shellcheck reported issue in this script: SC2086:info:5:60: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 70-70: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
| - name: Validate Prisma schema | ||
| run: yarn workspace @hmcts/postgres-prisma exec prisma validate | ||
| env: | ||
| DATABASE_URL: ${{ secrets.DATABASE_URL }} |
There was a problem hiding this comment.
prisma validate will fail on forked PRs where DATABASE_URL is unavailable.
Secrets are not passed to workflows triggered by forked repository PRs. An empty DATABASE_URL will cause this step to fail, blocking lint on all fork contributions.
🐛 Proposed fix
- name: Validate Prisma schema
run: yarn workspace `@hmcts/postgres-prisma` exec prisma validate
+ continue-on-error: true
env:
DATABASE_URL: ${{ secrets.DATABASE_URL }}Alternatively, guard the step: if: secrets.DATABASE_URL != '' (requires the secret to be defined, which is the safer approach).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Validate Prisma schema | |
| run: yarn workspace @hmcts/postgres-prisma exec prisma validate | |
| env: | |
| DATABASE_URL: ${{ secrets.DATABASE_URL }} | |
| - name: Validate Prisma schema | |
| run: yarn workspace `@hmcts/postgres-prisma` exec prisma validate | |
| continue-on-error: true | |
| env: | |
| DATABASE_URL: ${{ secrets.DATABASE_URL }} |
| module "application_insights" { | ||
| source = "git::https://github.com/hmcts/terraform-module-application-insights?ref=4.x" | ||
|
|
||
| env = var.env | ||
| product = var.product | ||
| name = "cath-${var.component}-appinsights" | ||
|
|
||
| resource_group_name = azurerm_resource_group.rg.name | ||
|
|
||
| common_tags = var.common_tags | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
hmcts terraform-module-application-insights v4 releases tags GitHub
💡 Result:
- The repo hmcts/terraform-module-application-insights has no GitHub Releases published (“No releases published”). [1]
- Its default branch is
4.x(i.e., v4 is tracked as a branch, not asv4.*release tags). [1][2] - The Tags/Releases view shows “There aren’t any releases here”, and no
v4.*release tags are listed. [3]
Sources: [1] [2] [3]
Pin the module source to a specific commit SHA.
ref=4.x is a branch reference and is mutable—the module source can change unexpectedly with new commits, weakening reproducibility and supply-chain guarantees. Since the repository has no release tags, pin to an exact commit SHA on the 4.x branch instead.
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 3-13: Ensure Terraform module sources use a commit hash
(CKV_TF_1)
[high] 3-13: Ensure Terraform module sources use a tag with a version number
(CKV_TF_2)
| azurerm.postgres_network = azurerm.postgres_network | ||
| } | ||
|
|
||
| source = "git::https://github.com/hmcts/terraform-module-postgresql-flexible?ref=master" |
There was a problem hiding this comment.
Pin module source to a versioned tag or commit hash.
Same issue as redis.tf — using ref=master is a supply-chain and reproducibility risk.
The CATH infrastructure is in the same STG subscription as the apps, so use AZURE_CREDENTIALS_SDS_STG and DTS-SHAREDSERVICES-STG instead of the DEV equivalents.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/job.terraform.yml (1)
57-57: Pin to a specific commit SHA instead of@main.The
hmcts/cnp-githubactions-libraryrepository has no published releases, so version tags are unavailable. However, the repository's documentation explicitly recommends pinning to a commit SHA for production use to ensure reproducibility and prevent unexpected failures from changes to themainbranch.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/job.terraform.yml.github/workflows/stage.infrastructure.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/stage.infrastructure.yml
- Add postgres-url secret to Terraform (connection string for STG) - Add DATABASE_URL key vault reference to web, api, crons, postgres Helm values for STG/prod (from postgres-url in pip-ss-kv) - Add DATABASE_URL key vault reference to preview templates (from db-url in pip-ss-kv, manually added to dev key vault) - Fix crons: rename db-url to postgres-url to match Terraform secret
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infrastructure/postgres.tf (1)
30-58: Consider setting expiration dates on Key Vault secrets.Static analysis flags
CKV_AZURE_41- secrets without expiration dates. Addingexpiration_dateenables secret rotation compliance and auditing, particularly important for database credentials.♻️ Example fix for one secret
resource "azurerm_key_vault_secret" "postgres_password" { name = "postgres-password" value = module.postgresql.password key_vault_id = data.azurerm_key_vault.key_vault.id + expiration_date = timeadd(timestamp(), "8760h") # 1 year + + lifecycle { + ignore_changes = [expiration_date] + } }
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/helm/values.yamlapps/crons/helm/values.yamlapps/postgres/helm/values.yamlapps/web/helm/values.yamlhelm/cath-service/values.preview.template.yamlhelm/cath-service/values.preview.yamlinfrastructure/postgres.tf
🚧 Files skipped from review as they are similar to previous changes (2)
- helm/cath-service/values.preview.yaml
- apps/postgres/helm/values.yaml
Drop individual postgres-host/user/password/port secrets in favour of the single postgres-url connection string. Prisma only needs DATABASE_URL, not the individual components.
Resolve conflicts with Prisma 7 upgrade: keep Prisma 7 deps (@prisma/adapter-pg, @prisma/client 7.2.0, pg) and retain http-proxy needed for the postgres health server.
The workspace is @hmcts/postgres, not @hmcts/postgres-prisma.
SDS uses different state store names than CNP:
- Resource group: jenkins-state
- Storage account: sdsstate{nonprod|prod}
- Container: tfstate-{environment}
Resource group is jenkins-state-{env} not jenkins-state, and storage
account is sdsstate{env} not sdsstate{storage-account}.
The terraform-deploy action extracts 'cath' from Chart.yaml annotations and passes it as -var product=cath, but Terraform uses product for key vault and resource group naming (pip-ss-kv-stg, not cath-stg).
…ult directly
The terraform-deploy action passes product=cath (from Chart.yaml), so the
resource group needs to be created as cath-{env} rather than looked up.
The key vault remains pip-ss-kv-{env} as it's borrowed from PIP's shared
infrastructure.
Terraform Plan for
|
Take compression dependency and config version bump from master.
…e condition The postinstall hook runs `yarn db:generate` which generates the Prisma client during `yarn install`. Without a `db:generate` script in postgres-prisma, the client was only generated during `yarn build`, causing a race where downstream packages could start tsc before the generated client existed.
The import block caused Terraform apply to recreate the managed identity with a new client ID, breaking the Key Vault CSI mount. Removed the import since Terraform now owns the resource. Reverted preview pipeline back to plan-only to prevent accidental infra changes from PRs.
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
linusnorton
force-pushed
the
feat/deployment-pipeline
branch
from
bd6f0d7 to
299396a
Compare
|
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
3 participants
Summary
Adds full CI/CD deployment pipeline for CATH, adapted from the
expressjs-monorepo-template. Targets SDS clusters (dev/stg/prod) with a dedicatedcathnamespace, using the sharedpip-ss-kvKey Vault.Docker
yarn workspaces focus --production)prisma migrate deploy) and Prisma StudioHelm Charts
helm/cath-service/) aggregating all subcharts + Redis + PostgreSQLteam: cathannotations to all Chart.yaml filesGitHub Actions Pipeline (3-tier)
workflow.main.yml(push to master),workflow.preview.yml(PR)Terraform Infrastructure
business_area = "sds")pip-ss-kv-{env})Key Configuration
sds-dev-00sds-stg-00pip-ss-kv(shared with PIP)AZURE_CREDENTIALS_SDS_DEV/AZURE_CREDENTIALS_SDS_STGhmctspublic.azurecr.io/cath/cath-*Companion PR
A separate PR will be created in
sds-flux-configto set up thecathnamespace with HelmRelease, ImageRepository/Policy, identity, and service account configs.Test plan
docker build -f apps/web/Dockerfile .(and api, crons, postgres)helm dependency update helm/cath-service/cd infrastructure && terraform init && terraform validateyarn testSummary by CodeRabbit
New Features
Chores