New: [AEA-6536] - Switched to new zero downtime deployment system#1864
Open
MatthewPopat-NHS wants to merge 54 commits intomainfrom
Open
New: [AEA-6536] - Switched to new zero downtime deployment system#1864MatthewPopat-NHS wants to merge 54 commits intomainfrom
MatthewPopat-NHS wants to merge 54 commits intomainfrom
Conversation
Contributor
|
This PR is linked to a ticket in an NHS Digital JIRA Project. Here's a handy link to the ticket: AEA-6269 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the build/deploy toolchain to support the new zero-downtime deployment approach by shifting CDK execution to a new “MainDeploymentApp” model, simplifying CloudFront routing/functions, and updating CI/CD and devcontainer workflows accordingly.
Changes:
- Reworked CDK apps/stacks/constructs to use
@nhsdigital/eps-cdk-constructs+ a newMainDeploymentAppentrypoint. - Simplified CloudFront routing (new
/api/*and/oauth2/*paths) and refactored CloudFront Functions/log delivery. - Updated CI workflows/devcontainer tooling and removed legacy guard/license-check scripts and some CDK tests/config.
Reviewed changes
Copilot reviewed 88 out of 95 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/run_sync.sh | Uses new npm run cdk-watch flow for local sync |
| scripts/run_cfn_guard.sh | Removed CFN Guard script |
| scripts/check_python_licenses.sh | Removed python license check script |
| packages/trackerUserInfoLambda/package.json | Removed check-licenses script |
| packages/testingSupport/clearActiveSessions/package.json | Removed check-licenses script |
| packages/sessionManagementLambda/package.json | Removed check-licenses script |
| packages/selectedRoleLambda/package.json | Removed check-licenses script |
| packages/prescriptionListLambda/src/handler.ts | Removes roleId env var usage |
| packages/prescriptionListLambda/package.json | Removed check-licenses script |
| packages/prescriptionDetailsLambda/package.json | Removed check-licenses script |
| packages/patientSearchLambda/package.json | Removed check-licenses script |
| packages/cpt-ui/vite-env.d.ts | Updates Vite env typings |
| packages/cpt-ui/src/helpers/awsRum.tsx | RUM config behavior changed/hardcoded |
| packages/cpt-ui/src/constants/environment.ts | Redirects now derived from CloudFront domain |
| packages/cpt-ui/package.json | Keeps Jest test script |
| packages/cpt-ui/jest.setup.ts | Updates mocked env config |
| packages/cpt-ui/tests/awsRumHelper.test.tsx | Updates expectations for RUM config |
| packages/cpt-ui/tests/LoginPage.test.tsx | Updates mocked env config |
| packages/cpt-ui/tests/EpsPrescriptionList.test.tsx | Updates mocked env/RUM config |
| packages/common/testing/package.json | Removed check-licenses script |
| packages/common/pdsClient/package.json | Removed check-licenses script |
| packages/common/middyErrorHandler/package.json | Removed check-licenses script |
| packages/common/lambdaUtils/package.json | Removed check-licenses script |
| packages/common/dynamoFunctions/package.json | Removed check-licenses script |
| packages/common/doHSClient/package.json | Removed check-licenses script |
| packages/common/commonTypes/package.json | Removed check-licenses script |
| packages/common/authFunctions/package.json | Removed check-licenses script |
| packages/cognito/package.json | Removed check-licenses script |
| packages/cloudfrontFunctions/tests/testS3StaticContentUriRewrite.test.ts | Simplified rewrite test cases |
| packages/cloudfrontFunctions/tests/testS3404ModifyStatusCode.test.ts | Removed test file |
| packages/cloudfrontFunctions/tests/testGenericStripPathUriRewrite.test.ts | Removed test file |
| packages/cloudfrontFunctions/tests/testGenericS3FixedObjectUriRewrite.test.ts | Removed test file |
| packages/cloudfrontFunctions/src/s3StaticContentUriRewrite.js | Simplified static content URI rewrite |
| packages/cloudfrontFunctions/src/s3404ModifyStatusCode.js | Removed CloudFront function |
| packages/cloudfrontFunctions/src/genericStripPathUriRewrite.js | Removed CloudFront function |
| packages/cloudfrontFunctions/src/genericS3FixedObjectUriRewrite.js | Removed CloudFront function |
| packages/cloudfrontFunctions/src/cloudfront/package.json | Removed test mock module |
| packages/cloudfrontFunctions/src/cloudfront/index.js | Removed test mock module |
| packages/cloudfrontFunctions/package.json | Removes local mock dependency; license script removed |
| packages/cdk/tests/src/dummyLambda.ts | Removed CDK test helper |
| packages/cdk/tests/functionConstruct.test.ts | Removed CDK construct tests |
| packages/cdk/tests/cdk-nag.test.ts | Removed CDK nag test |
| packages/cdk/stacks/UsCertsStack.ts | Stack props/resources refactor (new props model) |
| packages/cdk/stacks/StatefulResourcesStack.ts | Stack props/resources refactor; exports reduced |
| packages/cdk/resources/usRegionLogGroups.ts | Optional CSOC destination logic |
| packages/cdk/resources/ukRegionLogGroups.ts | Optional CSOC destination logic |
| packages/cdk/resources/api/oauth2Functions.ts | Migrate to TypescriptLambdaFunction and new props |
| packages/cdk/resources/api/apiFunctions.ts | Migrate to TypescriptLambdaFunction and new props |
| packages/cdk/resources/WebApplicationFirewall.ts | New allow-list model + rule wiring changes |
| packages/cdk/resources/StaticContentBucket.ts | New KMS + bucket policies; auto-delete enabled |
| packages/cdk/resources/SharedSecrets.ts | Secrets now injected from config (incl. JWT key) |
| packages/cdk/resources/Rum.ts | RUM resource refactor; config changes |
| packages/cdk/resources/RestApiGateway/RestApiGatewayMethods.ts | API routes now under /api/* |
| packages/cdk/resources/RestApiGateway/OAuth2ApiGatewayMethods.ts | OAuth2 routes now under /oauth2/* |
| packages/cdk/resources/LambdaFunction/helpers.ts | Removed legacy lambda helpers |
| packages/cdk/resources/LambdaFunction.ts | Removed legacy lambda construct |
| packages/cdk/resources/Dynamodb.ts | Removes state/session-state tables; refactors policies |
| packages/cdk/resources/Cognito.ts | Consolidates OIDC config into OidcConfig |
| packages/cdk/resources/CloudfrontLogDelivery.ts | Destination-only; source/delivery moved elsewhere |
| packages/cdk/resources/CloudfrontDistribution.ts | Rebuilds distribution behaviors/functions/logging |
| packages/cdk/resources/CloudfrontBehaviors.ts | Removed legacy behaviors construct |
| packages/cdk/resources/Cloudfront/CloudfrontFunction.ts | Removed legacy CF function wrapper |
| packages/cdk/policies/s3/AllowStaticContentPolicyStatements.ts | Removed policy helper |
| packages/cdk/policies/kms/AllowStaticBucketKmsKeyAccessPolicy.ts | Removed policy helper |
| packages/cdk/package.json | Adds new CDK scripts/deps; removes tests/license script |
| packages/cdk/nagSuppressions.ts | Switches to safe suppression helpers; refactors paths |
| packages/cdk/jest.debug.config.ts | Removed Jest config |
| packages/cdk/jest.config.ts | Removed Jest config |
| packages/cdk/cdk.json | Adds acknowledged issue numbers to config |
| packages/cdk/bin/utils/appUtils.ts | Removed legacy CFN Guard metadata helper |
| packages/cdk/bin/StatelessResourcesApp.ts | Removed legacy CDK app |
| packages/cdk/bin/StatefulResourcesApp.ts | Removed legacy CDK app |
| packages/cdk/bin/MainDeploymentApp.ts | New CDK entrypoint using eps constructs/utilities |
| packages/cdk/.gitignore | Removed cdk.out ignore (package-level) |
| packages/CIS2SignOutLambda/package.json | Removed check-licenses script |
| package.json | Removes root license-check script and deps adjustments |
| cdk.context.json | Removed context file |
| README.md | Removes license-check documentation; updates workflows doc |
| Makefile | Replaces many custom targets with common.mk + new CDK vars |
| .tool-versions.asdf | Removed |
| .tool-versions | Removed |
| .pre-commit-config.yaml | Updates actionlint hook invocation |
| .gitignore | Removes cfn_guard_output/ ignore |
| .github/workflows/run_regression_tests.yml | Moves to pinned devcontainer-based execution |
| .github/workflows/release.yml | Migrates to common devcontainer workflows |
| .github/workflows/pull_request.yml | Migrates to common devcontainer workflows |
| .github/workflows/link_dev_website.yml | Simplifies PR URL linking logic |
| .github/workflows/ci.yml | Migrates to common devcontainer workflows |
| .github/workflows/cdk_package_code.yml | Migrates packaging to pinned devcontainer |
| .devcontainer/devcontainer.json | New devcontainer definition (pinned EPS image) |
| .devcontainer/Dockerfile | Uses prebuilt EPS devcontainer image |
| .cfnlintrc | Removed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MatthewPopat-NHS
had a problem deploying
to
dev-pr
GitHub Actions
Failure
MatthewPopat-NHS
had a problem deploying
to
dev-pr
GitHub Actions
Failure
MatthewPopat-NHS
had a problem deploying
to
dev-pr
GitHub Actions
Failure
MatthewPopat-NHS
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the repo’s deployment approach to a new “zero downtime” model by restructuring CDK apps/stacks, CloudFront routing, and local developer workflows.
Changes:
- Refactors CDK into a new app entrypoint (
MainDeploymentApp) with new stacks (e.g.,FrontDoorStack,UsStatelessStack) and revised CloudFront/API routing. - Updates CPT UI build/dev setup to use Vite config composition and new env wiring (
VITE_cloudfrontBaseUrl). - Simplifies/updates GitHub workflows and tooling scripts to match the new deployment flow.
Reviewed changes
Copilot reviewed 63 out of 71 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cpt-ui/vite.local.config.ts | Adds local Vite config that hydrates env from CF exports and proxies API calls. |
| packages/cdk/bin/MainDeploymentApp.ts | Introduces new main CDK app entrypoint using shared create-all-stacks logic. |
| packages/cdk/bin/LocalToPrDeploymentApp.ts | Adds local-to-PR CDK watcher app entrypoint. |
| packages/cdk/bin/CreateAllStacks.ts | Centralizes stack construction and environment-specific config derivation. |
| packages/cpt-ui/package.json | Switches dev/compile scripts to new Vite config and adds deployment-utils dependency. |
| Makefile | Reworks CDK targets, watch/synth flows, and environment exports. |
| packages/cdk/stacks/* / packages/cdk/resources/* | Adds new stacks/resources and refactors distribution/WAF/logging/static deploy logic. |
| .github/workflows/* | Updates PR/release pipelines for new packaging/deployment system. |
| npm run dev --workspace packages/cpt-ui | ||
|
|
||
| react-build: | ||
| export BASE_PATH=/site && npm run build --workspace packages/cpt-ui |
There was a problem hiding this comment.
react-build runs npm run build --workspace packages/cpt-ui, but packages/cpt-ui/package.json no longer defines a build script (it was removed in this PR). This target will now fail; update it to call the correct script (e.g., npm run compile) or reintroduce a build script in the workspace.
Suggested change
| export BASE_PATH=/site && npm run build --workspace packages/cpt-ui | |
| export BASE_PATH=/site && npm run compile --workspace packages/cpt-ui |
Comment on lines
+29
to
+33
| "/api": { | ||
| target: `https://${serviceName}.dev.eps.national.nhs.uk/ `, | ||
| changeOrigin: true, | ||
| secure: false | ||
| } |
There was a problem hiding this comment.
The proxy target URL has a trailing space (after the /), which will produce an invalid/incorrect upstream URL and can break local API calls. Also secure: false disables TLS certificate verification for an HTTPS target; prefer leaving verification enabled unless there's a specific dev-only cert issue (and document/guard it).
Comment on lines
+6
to
+16
| const exports = await getCloudFormationExports() | ||
| const prId = process.env.PULL_REQUEST_ID | ||
| const serviceName = `cpt-ui-pr-${prId}` | ||
| const env = { | ||
| VITE_userPoolId: getCFConfigValue(exports, `${serviceName}:userPool:Id`), | ||
| VITE_userPoolClientId: getCFConfigValue(exports, `${serviceName}:userPoolClient:Id`), | ||
| VITE_hostedLoginDomain: `${serviceName}.auth.eu-west-2.amazoncognito.com`, | ||
| VITE_cloudfrontBaseUrl: "http://localhost:3000", | ||
| VITE_TARGET_ENVIRONMENT: "dev-pr", | ||
| VITE_COMMIT_ID: "static-pr", | ||
| VITE_VERSION_NUMBER: `PR-${prId}`, |
There was a problem hiding this comment.
PULL_REQUEST_ID is used to build serviceName/VERSION_NUMBER, but there’s no validation/fallback if it’s missing. If it’s undefined, this config will query exports for cpt-ui-pr-undefined and fail in a non-obvious way; consider failing fast with a clear error when PULL_REQUEST_ID isn’t set.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Details