Maintenance: Sync latest template repo upstream#52
Conversation
There was a problem hiding this comment.
Pull request overview
Syncs this repository with upstream changes from the NHS Notify repository template, updating build/dev tooling, CI workflows, and repo scaffolding to match the latest template conventions.
Changes:
- Introduces/updates Terraform, Docker, and Trivy helper scripts/Make targets aligned to the template.
- Updates GitHub workflows/actions and pre-commit hooks (including new TODO enforcement).
- Refreshes JS/tooling configuration (Sonar properties, ESLint rule tweak, dependency bumps) and removes the example lambda scaffolding.
Reviewed changes
Copilot reviewed 40 out of 44 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/jekyll-devcontainer/Makefile | Adds devcontainer build/publish/debug targets for the Jekyll devcontainer. |
| scripts/terraform/trivy-scan.sh | Adds a Trivy wrapper supporting native vs Docker execution. |
| scripts/terraform/terraform.mk | Replaces Terraform make targets with TFScaffold-oriented operations plus fmt/validate helpers. |
| scripts/init.mk | Adjusts dependency install loop to use $(MAKE) for recursion. |
| scripts/githooks/check-todos.sh | Adds a git hook script to enforce TODO usage (with Jira ticket filtering). |
| scripts/githooks/check-terraform-format.sh | Updates terraform formatting hook to call terraform-fmt / terraform-fmt-check targets. |
| scripts/githooks/check-file-format.sh | Switches native formatter command to editorconfig-checker. |
| scripts/git-repo/branch-protection.sh | Updates branch protection ruleset payload to include required status checks. |
| scripts/git-repo/auto-link.sh | Adjusts Jira autolink creation payload. |
| scripts/docker/docker.mk | Replaces Docker targets with NHS Notify container build/push/login workflow for ECR/GHCR. |
| scripts/docker/docker.lib.sh | Adds NHS Notify-specific container helpers (ECR/GHCR login, container build/push, image naming). |
| scripts/config/vale/styles/config/vocabularies/words/accept.txt | Expands accepted vocabulary for Vale checks. |
| scripts/config/trivy.yaml | Extends Trivy config skip list to include node_modules. |
| scripts/config/sonar-scanner.properties | Re-points Sonar sources/tests configuration to packages/ and adjusts coverage/test patterns. |
| scripts/config/pre-commit.yaml | Bumps pre-commit-hooks rev and adds a local TODO enforcement hook; adjusts JSON formatting exclusions. |
| scripts/config/.repository-template-sync-merge | Adds workflow glob to files merged during template sync. |
| scripts/config/.repository-template-sync-ignore | Expands template sync ignore list for repo-specific files/paths. |
| package.json | Updates dev dependencies, adds an override, and removes lambdas workspace / root start script. |
| package-lock.json | Regenerates lockfile for updated deps/workspaces. |
| lambdas/example-lambda/tsconfig.json | Removes example lambda scaffold file. |
| lambdas/example-lambda/src/index.ts | Removes example lambda handler. |
| lambdas/example-lambda/src/tests/index.test.ts | Removes example lambda unit test. |
| lambdas/example-lambda/package.json | Removes example lambda package definition/scripts. |
| lambdas/example-lambda/jest.config.ts | Removes example lambda Jest config. |
| lambdas/example-lambda/.gitignore | Removes example lambda ignore rules. |
| lambdas/example-lambda/.eslintignore | Removes example lambda ESLint ignore file. |
| infrastructure/terraform/bin/terraform.sh | Updates pre.sh invocation style, asdf plugin command, and backend config generation. |
| eslint.config.mjs | Tightens no-relative-import-paths rule from off to error. |
| AGENTS.md | Rewrites agent guidance to match newer template structure and repo layout notes. |
| .github/workflows/stage-3-build.yaml | Removes TODO markers from placeholder workflow steps. |
| .github/workflows/stage-1-commit.yaml | Adds TODO usage check job; adds ASDF setup for terraform linting; includes commented Trivy stubs. |
| .github/workflows/scorecard.yml | Updates checkout pin and enables additional permissions/token configuration. |
| .github/workflows/release_created.disabled | Adds a disabled release workflow template. |
| .github/workflows/pr_destroy_dynamic_env.disabled | Adds a disabled PR environment destroy workflow template. |
| .github/workflows/pr_create_dynamic_env.disabled | Adds a disabled PR environment create workflow template. |
| .github/workflows/pr_closed.disabled | Adds a disabled PR-closed deploy workflow template. |
| .github/scripts/dispatch_internal_repo_workflow.sh | Adds a script to dispatch and wait for a workflow run in nhs-notify-internal. |
| .github/actions/trivy-package/action.yaml | Adds a commented-out composite action stub for Trivy package scanning. |
| .github/actions/trivy-iac/action.yaml | Adds a commented-out composite action stub for Trivy IaC scanning. |
| .github/actions/lint-terraform/action.yaml | Updates terraform lint action to install terraform with asdf and validate via terraform-validate-all. |
| .github/actions/check-todo-usage/action.yaml | Adds composite action wrapper for the TODO usage hook. |
| .github/SECURITY.md | Updates contact emails to mailto links and replaces placeholder address. |
| .github/PULL_REQUEST_TEMPLATE.md | Adds a commented note referencing skip-trivy-package label usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| project=$(or ${project}, nhs) \ | ||
| region=$(or ${region}, eu-west-2) \ | ||
| group=$(or ${group}, dev) \ | ||
| opts=$(or ${opts}, ) |
There was a problem hiding this comment.
opts=$(or ${opts}, ) defaults to a single space when opts is unset, which makes $(if $(opts),...) evaluate as true and always passes a stray -- to terraform.sh. Default opts to truly empty (no whitespace) so the -- $(opts) segment is only added when options are provided.
| if [ "${PUBLISH_CONTAINER_IMAGE:-true}" = "true" ]; then | ||
| echo "Pushing to ECR..." | ||
| echo "Pushing ${DOCKER_IMAGE}..." | ||
| docker push "${DOCKER_IMAGE}" | ||
| echo "Push complete." |
There was a problem hiding this comment.
docker-push-container uses DOCKER_IMAGE without validating it is set; if it's empty, docker push will fail with a confusing error. Add a guard that errors clearly when DOCKER_IMAGE is missing.
| clean:: # Remove container image and resources - required: DOCKER_IMAGE @Development | ||
| source scripts/docker/docker.lib.sh; \ | ||
| DOCKER_IMAGE="$${DOCKER_IMAGE:-}" \ | ||
| docker-clean |
There was a problem hiding this comment.
This clean target is documented as requiring DOCKER_IMAGE, but it passes an empty default (DOCKER_IMAGE="$${DOCKER_IMAGE:-}") into docker-clean, which will attempt to docker rmi ":<tag>". Add validation (or a safe no-op) when DOCKER_IMAGE is unset.
| @@ -0,0 +1,21 @@ | |||
| build: | |||
| make -C ../../ Makefile version | |||
There was a problem hiding this comment.
make -C ../../ Makefile version treats Makefile as a target, not a file path, and will fail unless there is a Makefile target defined. If the intention is to run the version target in the repo root, this should likely be $(MAKE) -C ../../ version (and/or reference the correct target name).
| group=$(or ${group}, dev) \ | ||
| build_id=$(or ${build_id}, ) \ | ||
| opts=$(or ${opts}, ) |
There was a problem hiding this comment.
build_id=$(or ${build_id}, ) defaults to a single space, so $(if $(build_id),--build-id $(build_id),) expands to --build-id with no value when build_id is omitted. This is likely to break terraform.sh argument parsing; make the default truly empty and only emit --build-id when a non-empty value is provided.
| echo " overrideProjectName: $overrideProjectName" | ||
| echo " overrideRoleName: $overrideRoleName" | ||
| echo " targetProject: $targetProject" |
There was a problem hiding this comment.
targetProject is echoed and passed into the jq payload, but it is never set from CLI args or defaults, so it will always be empty. Either add argument parsing for it (and document it) or remove these references to avoid confusion.
| echo "Building container image..." | ||
| docker buildx build \ | ||
| -f docker/Dockerfile \ | ||
| --platform=linux/amd64 \ | ||
| --provenance=false \ | ||
| --sbom=false \ | ||
| --build-arg BASE_IMAGE="${BASE_IMAGE}" \ | ||
| -t "${DOCKER_IMAGE}" \ | ||
| --load \ |
There was a problem hiding this comment.
docker-build-container uses -t "${DOCKER_IMAGE}" but never validates that DOCKER_IMAGE is set/non-empty, so callers can end up running an invalid docker buildx command. Add an explicit check for DOCKER_IMAGE early (similar to the BASE_IMAGE check).
| # | ||
| # All arguments are required except terraformAction, and internalRef. | ||
| # Example: | ||
| # ./dispatch_internal_repo_workflow.sh \ | ||
| # --infraRepoName "nhs-notify-dns" \ | ||
| # --releaseVersion "v1.2.3" \ | ||
| # --targetWorkflow "deploy.yaml" \ | ||
| # --targetEnvironment "prod" \ | ||
| # --targetComponent "web" \ | ||
| # --targetAccountGroup "core" \ | ||
| # --terraformAction "apply" \ | ||
| # --internalRef "main" \ | ||
| # --overrides "tf_var=someString" \ | ||
| # --overrideProjectName nhs \ | ||
| # --overrideRoleName nhs-service-iam-role | ||
|
|
There was a problem hiding this comment.
The script claims “All arguments are required except terraformAction, and internalRef.” but there is no validation after parsing, so it can dispatch with empty required inputs and fail later with hard-to-debug API responses. Add explicit checks for required args (infraRepoName, releaseVersion, targetWorkflow, targetEnvironment, targetComponent, targetAccountGroup) with a clear usage message.
| # Pre-commit git hook to scan for secrets hard-coded in the codebase. This is a | ||
| # gitleaks command wrapper. It will run gitleaks natively if it is installed, | ||
| # otherwise it will run it in a Docker container. | ||
| # | ||
| # Usage: | ||
| # $ [options] ./scan-secrets.sh | ||
| # |
There was a problem hiding this comment.
Header comments/usage text describe this as a secrets/gitleaks wrapper (scan-secrets.sh) but the script actually searches for TODO. This is misleading for maintainers—update the description and usage to reflect the TODO scanning behavior.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Pull in upstream changes from the repo template and reconcile with local repo config.
Context
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.