Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the hometest-app Terragrunt deployment flow to optionally bake WireMock-related authentication configuration into the SPA build, and disables WireMock for the UAT environment.
Changes:
- Extend
scripts/build-spa.shto accept WireMock-related build-time environment variables and reflect them in cache hashing / build output. - Update
poc/hometest-app/app.hclTerragruntbefore_hookto pass WireMock settings into the SPA build step. - Set
enable_wiremock = falsefor thepoc/hometest-app/uatenvironment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| scripts/build-spa.sh | Adds WireMock env var handling to the SPA build and updates the build cache inputs. |
| infrastructure/environments/poc/hometest-app/app.hcl | Passes WireMock config into the SPA build hook and derives enablement from the child config. |
| infrastructure/environments/poc/hometest-app/uat/terragrunt.hcl | Disables WireMock in UAT via enable_wiremock = false. |
| all_hashes+="BACKEND_URL:${BACKEND_URL}|" | ||
|
|
||
| # Include wiremock config in hash — toggling wiremock changes build-time env vars | ||
| all_hashes+="ENABLE_WIREMOCK:${ENABLE_WIREMOCK}|" |
There was a problem hiding this comment.
The SPA build cache hash includes ENABLE_WIREMOCK but not WIREMOCK_BASE_URL, even though the built output changes when NHS_LOGIN_AUTHORIZE_URL (derived from the base URL) changes. This can cause Terragrunt to skip rebuilds when the WireMock URL changes. Include WIREMOCK_BASE_URL (or the full computed NHS_LOGIN_AUTHORIZE_URL) in calculate_source_hash().
| all_hashes+="ENABLE_WIREMOCK:${ENABLE_WIREMOCK}|" | |
| all_hashes+="ENABLE_WIREMOCK:${ENABLE_WIREMOCK}|" | |
| all_hashes+="WIREMOCK_BASE_URL:${WIREMOCK_BASE_URL}|" |
| # When wiremock is enabled, bake auth-related env vars into the SPA build | ||
| if [[ "$ENABLE_WIREMOCK" == "true" ]]; then | ||
| export USE_WIREMOCK_AUTH="true" | ||
| export NHS_LOGIN_AUTHORIZE_URL="${WIREMOCK_BASE_URL}/authorize" | ||
| echo " USE_WIREMOCK_AUTH=$USE_WIREMOCK_AUTH" | ||
| echo " NHS_LOGIN_AUTHORIZE_URL=$NHS_LOGIN_AUTHORIZE_URL" | ||
| fi |
There was a problem hiding this comment.
When ENABLE_WIREMOCK is true, the script constructs NHS_LOGIN_AUTHORIZE_URL from WIREMOCK_BASE_URL but doesn't validate that WIREMOCK_BASE_URL is set/non-empty. This can silently bake an invalid '/authorize' URL into the build. Add a guard that errors out (or defaults safely) when wiremock is enabled but WIREMOCK_BASE_URL is missing.
| # WireMock configuration (set via environment variables by Terragrunt hook) | ||
| ENABLE_WIREMOCK="${ENABLE_WIREMOCK:-false}" | ||
| WIREMOCK_BASE_URL="${WIREMOCK_BASE_URL:-}" | ||
|
|
There was a problem hiding this comment.
These new environment variables are required to control WireMock build-time behavior, but the script's usage/help text only documents FORCE_SPA_REBUILD. Please update the usage output to include ENABLE_WIREMOCK and WIREMOCK_BASE_URL so running the script outside Terragrunt is self-explanatory.
|
|
||
| # WireMock build configuration — controls SPA build-time env vars | ||
| # Reads enable_wiremock from the child terragrunt.hcl inputs (e.g. enable_wiremock = true) | ||
| _enable_wiremock = trimspace(run_cmd("bash", "-c", "grep -qE '^[[:space:]]*enable_wiremock[[:space:]]*=[[:space:]]*true' '${get_terragrunt_dir()}/terragrunt.hcl' 2>/dev/null && echo true || echo false")) |
There was a problem hiding this comment.
_enable_wiremock is derived by shelling out to grep and parsing terragrunt.hcl as plain text. This is fragile (e.g., it won't work if enable_wiremock is set via a local/expression, moved to a shared include, or formatted differently) and adds an implicit dependency on bash/grep during config evaluation. Prefer a Terragrunt-native approach (e.g., define a local enable_wiremock in child configs and reference that here, or pass it via an env var that the hook consumes) rather than scraping HCL with grep.
| _enable_wiremock = trimspace(run_cmd("bash", "-c", "grep -qE '^[[:space:]]*enable_wiremock[[:space:]]*=[[:space:]]*true' '${get_terragrunt_dir()}/terragrunt.hcl' 2>/dev/null && echo true || echo false")) | |
| # Use Terragrunt-native parsing instead of shelling out to grep. | |
| child_config = read_terragrunt_config("${get_terragrunt_dir()}/terragrunt.hcl") | |
| _enable_wiremock = lower(tostring(try(local.child_config.inputs.enable_wiremock, false))) |
| # When wiremock is enabled (via domain.hcl), USE_WIREMOCK_AUTH and NHS_LOGIN_AUTHORIZE_URL | ||
| # are baked into the SPA build. |
There was a problem hiding this comment.
The comment says wiremock is enabled "via domain.hcl", but the hook is actually driven by the enable_wiremock input (and _enable_wiremock is parsed from terragrunt.hcl). This is misleading documentation; update the comment to reflect the real toggle source.
| # When wiremock is enabled (via domain.hcl), USE_WIREMOCK_AUTH and NHS_LOGIN_AUTHORIZE_URL | |
| # are baked into the SPA build. | |
| # When wiremock is enabled via the enable_wiremock Terragrunt input (local._enable_wiremock), | |
| # USE_WIREMOCK_AUTH and NHS_LOGIN_AUTHORIZE_URL are baked into the SPA build. |
| @@ -23,5 +23,5 @@ include "app" { | |||
|
|
|||
| inputs = { | |||
| # WireMock - enabled for dev to stub 3rd-party APIs and support Playwright tests | |||
There was a problem hiding this comment.
This comment implies WireMock is generally "enabled for dev", but this is the uat environment config and enable_wiremock is being set to false. Consider rewording the comment to state that WireMock is disabled in UAT (and why), so readers don't have to reconcile the mismatch.
| # WireMock - enabled for dev to stub 3rd-party APIs and support Playwright tests | |
| # WireMock - disabled in UAT; only enabled in dev to stub 3rd-party APIs and support Playwright tests |
|
| # WireMock build configuration — controls SPA build-time env vars | ||
| # Reads enable_wiremock from the child terragrunt.hcl inputs (e.g. enable_wiremock = true) | ||
| _enable_wiremock = trimspace(run_cmd("bash", "-c", "grep -qE '^[[:space:]]*enable_wiremock[[:space:]]*=[[:space:]]*true' '${get_terragrunt_dir()}/terragrunt.hcl' 2>/dev/null && echo true || echo false")) | ||
| _wiremock_base_url = "https://wiremock-${local.environment}.${local.base_domain}" |
There was a problem hiding this comment.
This derives enable_wiremock by grepping the child terragrunt.hcl. That’s brittle (breaks if the value is set via an expression, different formatting, or moved to a different file) and adds an implicit coupling between config structure and behavior. Prefer passing wiremock build flags explicitly to the hook from the child config (or from a dedicated, machine-readable file like domain.hcl) instead of parsing HCL with grep.
| # WireMock build configuration — controls SPA build-time env vars | |
| # Reads enable_wiremock from the child terragrunt.hcl inputs (e.g. enable_wiremock = true) | |
| _enable_wiremock = trimspace(run_cmd("bash", "-c", "grep -qE '^[[:space:]]*enable_wiremock[[:space:]]*=[[:space:]]*true' '${get_terragrunt_dir()}/terragrunt.hcl' 2>/dev/null && echo true || echo false")) | |
| _wiremock_base_url = "https://wiremock-${local.environment}.${local.base_domain}" | |
| # WireMock build configuration — controls SPA build-time env vars. | |
| # WireMock flags are read from domain.hcl locals to avoid parsing HCL via grep. | |
| _enable_wiremock = lookup(local._domain_overrides, "enable_wiremock", false) | |
| _wiremock_base_url = lookup(local._domain_overrides, "wiremock_base_url", "https://wiremock-${local.environment}.${local.base_domain}") |
Description
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.