From 6b0f8259c939c781975b7736feb816904a0edb46 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 15 Apr 2026 15:54:17 +0100 Subject: [PATCH 01/69] Move towards using URL-based env var names. --- .github/workflows/stage-2-test.yaml | 6 +++--- Makefile | 12 ++++++------ README.md | 6 +++--- gateway-api/src/gateway_api/conftest.py | 3 +++ gateway-api/src/gateway_api/pds/client.py | 12 +++++------- gateway-api/src/gateway_api/provider/client.py | 4 ++-- gateway-api/src/gateway_api/sds/client.py | 6 +++--- infrastructure/README.md | 2 +- infrastructure/images/gateway-api/Dockerfile | 6 +++--- 9 files changed, 29 insertions(+), 28 deletions(-) diff --git a/.github/workflows/stage-2-test.yaml b/.github/workflows/stage-2-test.yaml index e7267f40..2adbcd39 100644 --- a/.github/workflows/stage-2-test.yaml +++ b/.github/workflows/stage-2-test.yaml @@ -3,9 +3,9 @@ name: "Test stage" env: BASE_URL: "http://localhost:5000" HOST: "localhost" - STUB_SDS: "true" - STUB_PDS: "true" - STUB_PROVIDER: "true" + SDS_URL: "stub" + PDS_URL: "stub" + PROVIDER_URL: "stub" on: workflow_call: diff --git a/Makefile b/Makefile index 16da7fd5..a398a11c 100644 --- a/Makefile +++ b/Makefile @@ -70,14 +70,14 @@ deploy: clean build # Deploy the project artefact to the target environment @Pip @$(docker) network inspect gateway-local >/dev/null 2>&1 || $(docker) network create gateway-local # Build up list of environment variables to pass to the container @ENVIRONMENT_STRING="" ; \ - if [[ -n "$${STUB_PROVIDER}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e STUB_PROVIDER=$${STUB_PROVIDER}" ; \ + if [[ -n "$${PROVIDER_URL}" ]]; then \ + ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e PROVIDER_URL=$${PROVIDER_URL}" ; \ fi ; \ - if [[ -n "$${STUB_PDS}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e STUB_PDS=$${STUB_PDS}" ; \ + if [[ -n "$${PDS_URL}" ]]; then \ + ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e PDS_URL=$${PDS_URL}" ; \ fi ; \ - if [[ -n "$${STUB_SDS}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e STUB_SDS=$${STUB_SDS}" ; \ + if [[ -n "$${SDS_URL}" ]]; then \ + ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e SDS_URL=$${SDS_URL}" ; \ fi ; \ if [[ -n "$${CDG_DEBUG}" ]]; then \ ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e CDG_DEBUG=$${CDG_DEBUG}" ; \ diff --git a/README.md b/README.md index a237a767..25805e54 100644 --- a/README.md +++ b/README.md @@ -152,9 +152,9 @@ The full API schema is defined in [gateway-api/openapi.yaml](gateway-api/openapi | `HOST` | hostname portion of `BASE_URL` | | `FLASK_HOST` | Host the Flask app binds to | | `FLASK_PORT` | Port the Flask app listens on | -| `STUB_PDS` | `true`, use the stubs/stubs/pds/stub.py to return stubbed responses for PDS FHIR API; otherwise, not. | -| `STUB_SDS` | `true`, use the stubs/stubs/sds/stub.py to return stubbed responses for SDS FHIR API; otherwise, not. | -| `STUB_PROVIDER` | `true`, use the stubs/stubs/provider/stub.py to return stubbed responses for the provider system; otherwise, not. | +| `PDS_URL` | The URL for the PDS FHIR API; set as `stub` to use development stub. | +| `SDS_URL` | The URL for the SDS FHIR API; set as `stub` to use development stub. | +| `PROVIDER_URL` | The URL for the GP Provider; set as `stub` to use development stub. | | `CDG_DEBUG` | `true`, Return additional debug information when the call to the GP provider returns an error. Note if set true causes the unit tests to fail, because expected return values are changed. | Environment variables also control whether stubs are used in place of the real PDS, SDS, and Provider services during local development. diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index ed7844f0..77c5ddbe 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -6,6 +6,7 @@ import pytest import requests +from dotenv import find_dotenv, load_dotenv from fhir.constants import FHIRSystem from flask import Request from requests.structures import CaseInsensitiveDict @@ -13,6 +14,8 @@ from gateway_api.clinical_jwt import JWT +load_dotenv(find_dotenv(usecwd=True)) + @dataclass class FakeResponse: diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 303671bf..323d27ae 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -20,7 +20,6 @@ import os import uuid -from collections.abc import Callable import requests from fhir.r4 import Patient @@ -29,18 +28,17 @@ from gateway_api.common.error import PdsRequestFailedError # TODO [GPCAPIM-359]: Once stub servers/containers made for PDS, SDS and provider -# we should remove the STUB_PDS environment variable and just +# we should remove the PDS_URL environment variable and just # use the stub client -STUB_PDS = os.environ.get("STUB_PDS", "false").lower() == "true" +STUB_PDS = os.environ["PDS_URL"].lower() == "stub" -get: Callable[..., requests.Response] if not STUB_PDS: - get = requests.get + from requests import get else: from stubs.pds.stub import PdsFhirApiStub pds = PdsFhirApiStub() - get = pds.get + get = pds.get # type: ignore class PdsClient: @@ -123,7 +121,7 @@ def search_patient_by_nhs_number( url = f"{self.base_url}/Patient/{nhs_number}" - # This normally calls requests.get, but if STUB_PDS is set it uses the stub. + # This normally calls requests.get, but if PDS_URL is set it uses the stub. response = get( url, headers=headers, diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 731bcafc..13b376e2 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -33,9 +33,9 @@ from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID # TODO [GPCAPIM-359]: Once stub servers/containers made for PDS, SDS and provider -# we should remove the STUB_PROVIDER environment variable and just +# we should remove the PROVIDER_URL environment variable and just # use the stub client -STUB_PROVIDER = os.environ.get("STUB_PROVIDER", "false").lower() == "true" +STUB_PROVIDER = os.environ["PROVIDER_URL"].lower() == "stub" if not STUB_PROVIDER: from requests import post else: diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index 22de0009..436a613f 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -22,9 +22,9 @@ from gateway_api.sds.search_results import SdsSearchResults # TODO [GPCAPIM-359]: Once stub servers/containers made for PDS, SDS and provider -# we should remove the STUB_SDS environment variable and just +# we should remove the SDS_URL environment variable and just # use the stub client -STUB_SDS = os.environ.get("STUB_SDS", "false").lower() == "true" +STUB_SDS = os.environ["SDS_URL"].lower() == "stub" if not STUB_SDS: from requests import get else: @@ -54,7 +54,7 @@ class SdsClient: **Stubbing**: - For testing, set the environment variable ``$STUB_SDS`` to use the + For testing, set the environment variable ``$SDS_URL` to use the :class:`SdsFhirApiStub` instead of making real HTTP requests. **Usage example**:: diff --git a/infrastructure/README.md b/infrastructure/README.md index 4994b8ec..99dd0afe 100644 --- a/infrastructure/README.md +++ b/infrastructure/README.md @@ -60,7 +60,7 @@ A lightweight Alpine-based Python image that runs the Flask application. Built w - A configurable `PYTHON_VERSION` build argument - A non-root user (`gateway_api_user`) -- Stubs enabled by default (`STUB_PDS`, `STUB_SDS`, `STUB_PROVIDER` all set to `true`) +- Stubs enabled by default (`PDS_URL`, `SDS_URL`, `PROVIDER_URL` all set to `stub`) - Flask listening on `0.0.0.0:8080` ### `build-container` diff --git a/infrastructure/images/gateway-api/Dockerfile b/infrastructure/images/gateway-api/Dockerfile index 1a5c94b2..527e0f52 100644 --- a/infrastructure/images/gateway-api/Dockerfile +++ b/infrastructure/images/gateway-api/Dockerfile @@ -26,9 +26,9 @@ WORKDIR /resources/build/gateway-api ENV PYTHONPATH=/resources/build/gateway-api ENV FLASK_HOST="0.0.0.0" ENV FLASK_PORT="8080" -ENV STUB_PDS="true" -ENV STUB_PROVIDER="true" -ENV STUB_SDS="true" +ENV PDS_URL="stub" +ENV PROVIDER_URL="stub" +ENV SDS_URL="stub" ARG COMMIT_VERSION ENV COMMIT_VERSION=$COMMIT_VERSION From a4675a8d29962bc2baeebfa52ac68ccccac821f7 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 15 Apr 2026 21:25:21 +0100 Subject: [PATCH 02/69] Resolve this TODO later. --- .github/workflows/preview-env.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index 2fab475a..fed28082 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -248,6 +248,7 @@ jobs: --services ${{ steps.tf-output.outputs.ecs_service }} \ --region ${{ env.AWS_REGION }} + # TODO: We don't need these anymore, as we are testing against the proxy. - name: Get mTLS certs for testing if: github.event.action != 'closed' id: mtls-certs From 0202c90752232b37d2ff190ff7e1ca5059748ac1 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 15 Apr 2026 21:33:26 +0100 Subject: [PATCH 03/69] Broaden vocabulary. --- .vscode/cspell-dictionary.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/cspell-dictionary.txt b/.vscode/cspell-dictionary.txt index 85932785..1a219d09 100644 --- a/.vscode/cspell-dictionary.txt +++ b/.vscode/cspell-dictionary.txt @@ -3,6 +3,7 @@ fhir getstructuredrecord GPCAPIM gpconnect -searchset +orangebox proxygen +searchset usefixtures From 8d2f8a1af5a8e0c30f54222b2d98580e15ef5e33 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 10:54:42 +0100 Subject: [PATCH 04/69] Log the client details. --- gateway-api/src/gateway_api/pds/client.py | 10 ++++++++++ gateway-api/src/gateway_api/provider/client.py | 12 ++++++++++++ gateway-api/src/gateway_api/sds/client.py | 10 ++++++++++ 3 files changed, 32 insertions(+) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 323d27ae..221d5672 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -33,8 +33,18 @@ STUB_PDS = os.environ["PDS_URL"].lower() == "stub" if not STUB_PDS: + log_details = { + "description": "Using real PDS client", + "pds_url": os.environ["PDS_URL"], + } + print(log_details, flush=True) from requests import get else: + log_details = { + "description": "Using stub PDS client", + "pds_url": "stub", + } + print(log_details, flush=True) from stubs.pds.stub import PdsFhirApiStub pds = PdsFhirApiStub() diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 13b376e2..c9bf5d1c 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -37,8 +37,20 @@ # use the stub client STUB_PROVIDER = os.environ["PROVIDER_URL"].lower() == "stub" if not STUB_PROVIDER: + log_details = { + "description": "Using real GP Provider client", + # TODO: There is a nuance to this: the URL is actually from SDS. + # Find a better wording for this. + "provider_url": os.environ["PROVIDER_URL"], + } + print(log_details, flush=True) from requests import post else: + log_details = { + "description": "Using stub GP Provider client", + "provider_url": "stub", + } + print(log_details, flush=True) from stubs.provider.stub import GpProviderStub provider_stub = GpProviderStub() diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index 436a613f..ce5b64c8 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -26,8 +26,18 @@ # use the stub client STUB_SDS = os.environ["SDS_URL"].lower() == "stub" if not STUB_SDS: + log_details = { + "description": "Using real SDS client", + "sds_url": os.environ["SDS_URL"], + } + print(log_details, flush=True) from requests import get else: + log_details = { + "description": "Using stub SDS client", + "sds_url": "stub", + } + print(log_details, flush=True) from stubs import SdsFhirApiStub sds = SdsFhirApiStub() From dea2c64f3629c2df08a694f9ff4c85c4b0ccf350 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 11:23:19 +0100 Subject: [PATCH 05/69] Use env vars and pass those through an .env file in the make deploy command. --- .github/actions/start-app/action.yaml | 2 +- .github/instructions/copilot-instructions.md | 2 +- Makefile | 22 ++++----------- README.md | 5 ++-- gateway-api/src/gateway_api/app.py | 22 +++++++++++++-- gateway-api/src/gateway_api/conftest.py | 7 +++-- gateway-api/tests/README.md | 4 ++- .../integration/test_get_structured_record.py | 5 ++++ infrastructure/images/gateway-api/Dockerfile | 7 ++--- scripts/env/env.mk | 28 +++++++++++++++++++ scripts/init.mk | 1 + 11 files changed, 74 insertions(+), 31 deletions(-) create mode 100644 scripts/env/env.mk diff --git a/.github/actions/start-app/action.yaml b/.github/actions/start-app/action.yaml index 27b0ba56..d9ee75ca 100644 --- a/.github/actions/start-app/action.yaml +++ b/.github/actions/start-app/action.yaml @@ -4,7 +4,7 @@ inputs: deploy-command: description: "Command to start app" required: false - default: "make deploy" + default: "make deploy-dev" health-path: description: "Health check path" required: false diff --git a/.github/instructions/copilot-instructions.md b/.github/instructions/copilot-instructions.md index 9d2ebc7e..00004bd4 100644 --- a/.github/instructions/copilot-instructions.md +++ b/.github/instructions/copilot-instructions.md @@ -8,7 +8,7 @@ This repository is for handling HTTP requests from "Consumer systems" and forwar We use other NHSE services to assist in the validation and processing of the requests including PDS FHIR API for obtaining GP practice codes for the patient, SDS FHIR API for obtaining the "Provider system" details of that GP practice and Healthcare Worker FHIR API for obtaining details of the requesting practitioner using the "Consumer System" that will then be added to the forwarded request. -`make deploy` will build and start a container running Gateway API at `localhost:5000`. +`make deploy-dev` will build and start a container running Gateway API at `localhost:5000`. After deploying the container locally, `make test` will run all tests and capture their coverage. Note: env variables control the use of stubs for the PDS FHIR API, SDS FHIR API, Healthcare Worker FHIR API and Provider system services. diff --git a/Makefile b/Makefile index a398a11c..b4ed852b 100644 --- a/Makefile +++ b/Makefile @@ -66,29 +66,19 @@ build: build-gateway-api # Build the project artefact @Pipeline publish: # Publish the project artefact @Pipeline # TODO [GPCAPIM-283]: Implement the artefact publishing step -deploy: clean build # Deploy the project artefact to the target environment @Pipeline +deploy: clean build # Build project artefact and deploy locally @Pipeline @$(docker) network inspect gateway-local >/dev/null 2>&1 || $(docker) network create gateway-local # Build up list of environment variables to pass to the container - @ENVIRONMENT_STRING="" ; \ - if [[ -n "$${PROVIDER_URL}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e PROVIDER_URL=$${PROVIDER_URL}" ; \ - fi ; \ - if [[ -n "$${PDS_URL}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e PDS_URL=$${PDS_URL}" ; \ - fi ; \ - if [[ -n "$${SDS_URL}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e SDS_URL=$${SDS_URL}" ; \ - fi ; \ - if [[ -n "$${CDG_DEBUG}" ]]; then \ - ENVIRONMENT_STRING="$${ENVIRONMENT_STRING} -e CDG_DEBUG=$${CDG_DEBUG}" ; \ - fi ; \ if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ echo "Starting using local docker network ..." ; \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local $${ENVIRONMENT_STRING} -d ${IMAGE_NAME} ; \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local --env-file .env -d ${IMAGE_NAME} ; \ else \ - $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 $${ENVIRONMENT_STRING} -d ${IMAGE_NAME} ; \ + $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --env-file .env -d ${IMAGE_NAME} ; \ fi +deploy-%: # Build project artefact and deploy locally as specified environment - mandatory: name=[name of the environment, e.g. 'dev'] @Pipeline + make env-$* deploy + clean:: stop # Clean-up project resources (main) @Operations @echo "Removing Gateway API container..." @$(docker) rm gateway-api || echo "No Gateway API container currently exists." diff --git a/README.md b/README.md index 25805e54..c9767283 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,8 @@ The project uses `make` targets to build, deploy, and manage the application. Ru | --- | --- | | `make dependencies` | Install all project dependencies via Poetry | | `make build` | Type-check, package, and build the Docker image | -| `make deploy` | Build and start the Gateway API container at `localhost:5000` | +| `make deploy` | Build and start the Gateway API container using the environment variables defined in `.env` | +| `make deploy-*` | Create the `.env` file for the `` environment, then build and start the Gateway API container with those variables. | | `make clean` | Stop and remove the Gateway API container | | `make config` | Configure the development environment | @@ -155,7 +156,7 @@ The full API schema is defined in [gateway-api/openapi.yaml](gateway-api/openapi | `PDS_URL` | The URL for the PDS FHIR API; set as `stub` to use development stub. | | `SDS_URL` | The URL for the SDS FHIR API; set as `stub` to use development stub. | | `PROVIDER_URL` | The URL for the GP Provider; set as `stub` to use development stub. | -| `CDG_DEBUG` | `true`, Return additional debug information when the call to the GP provider returns an error. Note if set true causes the unit tests to fail, because expected return values are changed. | +| `CDG_DEBUG` | `true`, return additional debug information when the call to the GP provider returns an error. | Environment variables also control whether stubs are used in place of the real PDS, SDS, and Provider services during local development. diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 4edd0f83..93a1b8e1 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -19,7 +19,6 @@ def get_app_host() -> str: host = os.getenv("FLASK_HOST") if host is None: raise RuntimeError("FLASK_HOST environment variable is not set.") - print(f"Starting Flask app on host: {host}") return host @@ -27,7 +26,6 @@ def get_app_port() -> int: port = os.getenv("FLASK_PORT") if port is None: raise RuntimeError("FLASK_PORT environment variable is not set.") - print(f"Starting Flask app on port: {port}") return int(port) @@ -51,6 +49,23 @@ def log_error(error: AbstractCDGError) -> None: app.logger.error(log_details) +def log_env_vars() -> None: + log_details = { + "description": "Initializing Flask app", + "env_vars": os.environ.items(), + } + app.logger.info(log_details) + + +def log_starting_app(host: str, port: int) -> None: + log_details = { + "description": "Starting Flask app", + "host": host, + "port": port, + } + app.logger.info(log_details) + + @app.route("/patient/$gpc.getstructuredrecord", methods=["POST"]) def get_structured_record() -> Response: log_request_received(request) @@ -86,6 +101,7 @@ def health_check() -> dict[str, str]: if __name__ == "__main__": + log_env_vars() host, port = get_app_host(), get_app_port() - print(f"Version: {os.getenv('COMMIT_VERSION')}") + log_starting_app(host, port) app.run(host=host, port=port) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 77c5ddbe..a0fa64b1 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -1,12 +1,12 @@ """Pytest configuration and shared fixtures for gateway API tests.""" import json +import os from dataclasses import dataclass from typing import Any import pytest import requests -from dotenv import find_dotenv, load_dotenv from fhir.constants import FHIRSystem from flask import Request from requests.structures import CaseInsensitiveDict @@ -14,7 +14,10 @@ from gateway_api.clinical_jwt import JWT -load_dotenv(find_dotenv(usecwd=True)) +# TODO: Do this better. +os.environ["PDS_URL"] = "stub" +os.environ["PROVIDER_URL"] = "not-stub" +os.environ["SDS_URL"] = "stub" @dataclass diff --git a/gateway-api/tests/README.md b/gateway-api/tests/README.md index 7a86fd15..77e6dac0 100644 --- a/gateway-api/tests/README.md +++ b/gateway-api/tests/README.md @@ -27,7 +27,7 @@ tests/ > [!NOTE]
> When running tests the following environment variables need to be provided: > -> - `BASE_URL` - defines the protocol, hostname and port that should used to access the running APIs. Should be included as a URL in the format ::, for example "" if the APIs are available on the "localhost" host via HTTP using port 5000. If running locally in a dev container, using gateway-api as the host will allow the tests to communicate with an instance launched from `make deploy`. +> - `BASE_URL` - defines the protocol, hostname and port that should used to access the running APIs. Should be included as a URL in the format ::, for example "" if the APIs are available on the "localhost" host via HTTP using port 5000. If running locally in a dev container, using gateway-api as the host will allow the tests to communicate with an instance launched from `make deploy-dev`. > - `HOST` - defines the hostname that should be used to access the running APIs. This should match the host portion of the URL provided in the `BASE_URL` environment variable above. ### Install Dependencies (if not using Dev container) @@ -67,6 +67,8 @@ pytest gateway-api/tests/schema/ -v Run `make deploy` before running any tests that hit the real API. +This command requires a `.env` file to set the app's behaviour - see `env.mk` on how to make `.env` files. + - **Requires deployed/running app (`make deploy`):** - Acceptance tests (`tests/acceptance/`) - Integration tests (`tests/integration/`) diff --git a/gateway-api/tests/integration/test_get_structured_record.py b/gateway-api/tests/integration/test_get_structured_record.py index 480ca4f5..752a154f 100644 --- a/gateway-api/tests/integration/test_get_structured_record.py +++ b/gateway-api/tests/integration/test_get_structured_record.py @@ -150,6 +150,11 @@ def test_502_status_code_return_when_provider_returns_error( def test_internal_server_error_message_returned_when_provider_returns_error( self, response_when_provider_returns_error: Response ) -> None: + """ + WARNING: This can fail if the CDG_DEBUG env var is set to true in the + deployed app. + TODO [GPCAPIM-353]: Ensure integration tests are not affected by CDG_DEBUG + """ expected = { "resourceType": "OperationOutcome", "issue": [ diff --git a/infrastructure/images/gateway-api/Dockerfile b/infrastructure/images/gateway-api/Dockerfile index 527e0f52..083fa70b 100644 --- a/infrastructure/images/gateway-api/Dockerfile +++ b/infrastructure/images/gateway-api/Dockerfile @@ -24,15 +24,12 @@ RUN if [ "$INCLUDE_DEV_CERTS" = "true" ] && [ -d /resources/dev-certificates ]; WORKDIR /resources/build/gateway-api ENV PYTHONPATH=/resources/build/gateway-api -ENV FLASK_HOST="0.0.0.0" -ENV FLASK_PORT="8080" -ENV PDS_URL="stub" -ENV PROVIDER_URL="stub" -ENV SDS_URL="stub" ARG COMMIT_VERSION +# TODO: Do we want to do something with this env var? ENV COMMIT_VERSION=$COMMIT_VERSION ARG BUILD_DATE +# TODO: Do we want to do something with this env var? ENV BUILD_DATE=$BUILD_DATE USER gateway_api_user diff --git a/scripts/env/env.mk b/scripts/env/env.mk new file mode 100644 index 00000000..6c93b1f7 --- /dev/null +++ b/scripts/env/env.mk @@ -0,0 +1,28 @@ +.PHONY: env env-% _env + +env: + make env-dev # TODO: Make this interactive + +env-%: # Create .env file with environment variables - optional: name=[name of the environment, e.g. 'dev'] @Configuration + make _env name="$*" # TODO: Implement difference envs + +_env: + echo "# ENVIRONMENT VARIABLES" > .env + echo "# ---------------------" >> .env + echo "# This file is generated by 'make env'/'make env-*'." >> .env + + echo "FLASK_PORT=8080" >> .env + echo "FLASK_HOST=0.0.0.0" >> .env + + echo "BASE_URL=http://gateway-api:8080" >> .env + + echo "PDS_URL=stub" >> .env + echo "PROVIDER_URL=stub" >> .env + echo "SDS_URL=stub" >> .env + + echo "CDG_DEBUG=false" >> .env + +${VERBOSE}.SILENT: \ + _env \ + env \ + env-% \ diff --git a/scripts/init.mk b/scripts/init.mk index bcd51800..39a28d18 100644 --- a/scripts/init.mk +++ b/scripts/init.mk @@ -2,6 +2,7 @@ include scripts/docker/docker.mk include scripts/tests/test.mk +include scripts/env/env.mk -include scripts/terraform/terraform.mk # ============================================================================== From 42043084821a79c56b597c1df2af9ee4703be82d Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 13:34:58 +0100 Subject: [PATCH 06/69] Make number go up. --- gateway-api/src/gateway_api/app.py | 16 ++++--- gateway-api/src/gateway_api/conftest.py | 23 ++++++++++ gateway-api/src/gateway_api/test_app.py | 56 ++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 7 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 93a1b8e1..caef905a 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -15,6 +15,13 @@ app.logger.setLevel("INFO") +def start_app(app: Flask) -> None: + log_env_vars(app) + host, port = get_app_host(), get_app_port() + log_starting_app(app, host, port) + app.run(host=host, port=port) + + def get_app_host() -> str: host = os.getenv("FLASK_HOST") if host is None: @@ -49,7 +56,7 @@ def log_error(error: AbstractCDGError) -> None: app.logger.error(log_details) -def log_env_vars() -> None: +def log_env_vars(app: Flask) -> None: log_details = { "description": "Initializing Flask app", "env_vars": os.environ.items(), @@ -57,7 +64,7 @@ def log_env_vars() -> None: app.logger.info(log_details) -def log_starting_app(host: str, port: int) -> None: +def log_starting_app(app: Flask, host: str, port: int) -> None: log_details = { "description": "Starting Flask app", "host": host, @@ -101,7 +108,4 @@ def health_check() -> dict[str, str]: if __name__ == "__main__": - log_env_vars() - host, port = get_app_host(), get_app_port() - log_starting_app(host, port) - app.run(host=host, port=port) + start_app(app) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index a0fa64b1..ee8c581a 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -3,6 +3,7 @@ import json import os from dataclasses import dataclass +from types import TracebackType from typing import Any import pytest @@ -20,6 +21,28 @@ os.environ["SDS_URL"] = "stub" +class NewEnvVars: + def __init__(self, new_env_vars: dict[str, str]) -> None: + self.new_env_vars = new_env_vars + self.original_env_vars = {key: os.environ.get(key) for key in new_env_vars} + + def __enter__(self) -> "NewEnvVars": + os.environ.update(self.new_env_vars) + return self + + def __exit__( + self, + _type: type[BaseException] | None, + _value: BaseException | None, + _traceback: TracebackType | None, + ) -> None: + for key, value in self.original_env_vars.items(): + if value is not None: + os.environ[key] = value + else: + del os.environ[key] + + @dataclass class FakeResponse: """ diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index e6f808e0..dd3f272e 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -12,7 +12,15 @@ from flask.testing import FlaskClient from pytest_mock import MockerFixture -from gateway_api.app import app, get_app_host, get_app_port +from gateway_api.app import ( + app, + get_app_host, + get_app_port, + log_env_vars, + log_starting_app, + start_app, +) +from gateway_api.conftest import NewEnvVars @pytest.fixture @@ -47,6 +55,52 @@ def test_get_app_port_raises_runtime_error_if_port_not_set(self) -> None: with pytest.raises(RuntimeError): _ = get_app_port() + def test_logging_app_startup_details_on_app_initialization( + self, mocker: MockerFixture + ) -> None: + log_info_mock = mocker.patch.object(app.logger, "info") + + host = "test_host" + port = 1234 + log_starting_app(app, host, port) + + # Check that the app startup details were logged + log_info_mock.assert_called_with( + { + "description": "Starting Flask app", + "host": host, + "port": port, + } + ) + + def test_logging_environment_variables_on_app_initialization( + self, mocker: MockerFixture + ) -> None: + log_info_mock = mocker.patch.object(app.logger, "info") + + log_env_vars(app) + + # Check that the environment variables were logged + log_info_mock.assert_called_with( + { + "description": "Initializing Flask app", + "env_vars": os.environ.items(), + } + ) + + def test_start_app_logs_startup_details(self) -> None: + test_app = Mock() + + test_env_vars = { + "FLASK_HOST": "test_host", + "FLASK_PORT": "1234", + } + + with NewEnvVars(test_env_vars): + start_app(test_app) + + test_app.run.assert_called_with(host="test_host", port=1234) + class TestGetStructuredRecord: @pytest.mark.usefixtures("mock_positive_return_value_from_controller_run") From 899ba50a6df6d6ef222db552d5052230d950e750 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 14:34:56 +0100 Subject: [PATCH 07/69] Make number go up. --- .github/actions/start-app/action.yaml | 1 + gateway-api/src/gateway_api/pds/client.py | 14 ++++---------- gateway-api/src/gateway_api/provider/client.py | 16 ++++------------ gateway-api/src/gateway_api/sds/client.py | 14 ++++---------- scripts/tests/test.mk | 1 + 5 files changed, 14 insertions(+), 32 deletions(-) diff --git a/.github/actions/start-app/action.yaml b/.github/actions/start-app/action.yaml index d9ee75ca..a50a727e 100644 --- a/.github/actions/start-app/action.yaml +++ b/.github/actions/start-app/action.yaml @@ -4,6 +4,7 @@ inputs: deploy-command: description: "Command to start app" required: false + # TODO: rename this make deploy-ci? default: "make deploy-dev" health-path: description: "Health check path" diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 221d5672..4e255f41 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -33,18 +33,8 @@ STUB_PDS = os.environ["PDS_URL"].lower() == "stub" if not STUB_PDS: - log_details = { - "description": "Using real PDS client", - "pds_url": os.environ["PDS_URL"], - } - print(log_details, flush=True) from requests import get else: - log_details = { - "description": "Using stub PDS client", - "pds_url": "stub", - } - print(log_details, flush=True) from stubs.pds.stub import PdsFhirApiStub pds = PdsFhirApiStub() @@ -92,6 +82,8 @@ def __init__( self.timeout = timeout self.ignore_dates = ignore_dates + # TODO: Add logging to show stub behaviour + def _build_headers( self, request_id: str | None = None, @@ -132,6 +124,7 @@ def search_patient_by_nhs_number( url = f"{self.base_url}/Patient/{nhs_number}" # This normally calls requests.get, but if PDS_URL is set it uses the stub. + # TODO: Log request to confirm client behaviour response = get( url, headers=headers, @@ -141,6 +134,7 @@ def search_patient_by_nhs_number( try: response.raise_for_status() + # TODO: Log response to confirm stub behaviour except requests.HTTPError as err: raise PdsRequestFailedError(error_reason=err.response.reason) from err diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index c9bf5d1c..5f403465 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -37,20 +37,8 @@ # use the stub client STUB_PROVIDER = os.environ["PROVIDER_URL"].lower() == "stub" if not STUB_PROVIDER: - log_details = { - "description": "Using real GP Provider client", - # TODO: There is a nuance to this: the URL is actually from SDS. - # Find a better wording for this. - "provider_url": os.environ["PROVIDER_URL"], - } - print(log_details, flush=True) from requests import post else: - log_details = { - "description": "Using stub GP Provider client", - "provider_url": "stub", - } - print(log_details, flush=True) from stubs.provider.stub import GpProviderStub provider_stub = GpProviderStub() @@ -95,6 +83,8 @@ def __init__( self.token = token self.endpoint_path = endpoint_path + # TODO: Add logging to show stub behaviour + def _build_headers(self, trace_id: str) -> dict[str, str]: """ Build the headers required for the GPProvider FHIR API request. @@ -129,6 +119,7 @@ def access_structured_record( base_endpoint = self.provider_endpoint.rstrip("/") + "/" url = urljoin(base_endpoint, self.endpoint_path) + # TODO: Log request to confirm client behaviour response = post( url, headers=headers, @@ -138,6 +129,7 @@ def access_structured_record( try: response.raise_for_status() + # TODOL: Log response to confirm stub behaviour except HTTPError as err: # TODO: GPCAPIM-353 Consider what error information we want to return here. # Post-steel-thread we probably want to log rather than dumping like this diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index ce5b64c8..871e858d 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -26,18 +26,8 @@ # use the stub client STUB_SDS = os.environ["SDS_URL"].lower() == "stub" if not STUB_SDS: - log_details = { - "description": "Using real SDS client", - "sds_url": os.environ["SDS_URL"], - } - print(log_details, flush=True) from requests import get else: - log_details = { - "description": "Using stub SDS client", - "sds_url": "stub", - } - print(log_details, flush=True) from stubs import SdsFhirApiStub sds = SdsFhirApiStub() @@ -101,6 +91,8 @@ def __init__( ) self.api_key = self._get_api_key() + # TODO: Add logging to show stub behaviour + def _build_headers(self, correlation_id: str | None = None) -> dict[str, str]: """ Build mandatory and optional headers for an SDS request. @@ -204,6 +196,7 @@ def _query_sds( if party_key is not None: params["identifier"].append(f"{FHIRSystem.NHS_MHS_PARTY_KEY}|{party_key}") + # TODO: Log request to confirm stub behaviour response = get( url, headers=headers, @@ -213,6 +206,7 @@ def _query_sds( try: response.raise_for_status() + # TODO: Log response to confirm stub behaviour except HTTPError as e: raise SdsRequestFailedError(error_reason=str(e)) from e diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index fd822b1c..a49dd659 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -69,6 +69,7 @@ test: # Run all the test tasks @Testing test-acceptance\ test-schema +# TODO: have _test target="proxy"/"local"? _test: set -e script="./scripts/tests/${name}.sh" From 4f47fa950f39ed931dcf4153d123fcac90a2a3dc Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:12:53 +0100 Subject: [PATCH 08/69] Make number go up. --- gateway-api/src/gateway_api/app.py | 27 +++++++++++++++--- gateway-api/src/gateway_api/conftest.py | 18 ++++++------ gateway-api/src/gateway_api/test_app.py | 38 +++++++++---------------- 3 files changed, 46 insertions(+), 37 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index caef905a..12ce1ebe 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -1,5 +1,7 @@ import os import traceback +from collections.abc import Callable +from typing import Any from flask import Flask, Request, request from flask.wrappers import Response @@ -17,9 +19,22 @@ def start_app(app: Flask) -> None: log_env_vars(app) - host, port = get_app_host(), get_app_port() - log_starting_app(app, host, port) - app.run(host=host, port=port) + app_host = get_env_var("FLASK_HOST", str) + app_port = get_env_var("FLASK_PORT", int) + pds_base_url = get_env_var("PDS_URL", str) + sds_base_url = get_env_var("SDS_URL", str) + log_starting_app(app, app_host, app_port, pds_base_url, sds_base_url) + app.run(host=app_host, port=app_port) + + +def get_env_var(name: str, loader: Callable[[str], Any]) -> Any: + value = os.getenv(name) + if value is None: + raise RuntimeError(f"{name} environment variable is not set.") + try: + return loader(value) + except Exception as e: + raise RuntimeError(f"Error loading {name} environment variable: {e}") from e def get_app_host() -> str: @@ -64,11 +79,15 @@ def log_env_vars(app: Flask) -> None: app.logger.info(log_details) -def log_starting_app(app: Flask, host: str, port: int) -> None: +def log_starting_app( + app: Flask, host: str, port: int, pds_base_url: str, sds_base_url: str +) -> None: log_details = { "description": "Starting Flask app", "host": host, "port": port, + "pds_base_url": pds_base_url, + "sds_base_url": sds_base_url, } app.logger.info(log_details) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index ee8c581a..93ccd86d 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -22,12 +22,18 @@ class NewEnvVars: - def __init__(self, new_env_vars: dict[str, str]) -> None: + def __init__(self, new_env_vars: dict[str, str | None]) -> None: self.new_env_vars = new_env_vars - self.original_env_vars = {key: os.environ.get(key) for key in new_env_vars} + self.original_env_vars = { + key: os.environ.get(key) for key in new_env_vars if key in os.environ + } def __enter__(self) -> "NewEnvVars": - os.environ.update(self.new_env_vars) + for key, value in self.new_env_vars.items(): + if value is None and key in os.environ: + del os.environ[key] + elif value is not None: + os.environ[key] = value return self def __exit__( @@ -36,11 +42,7 @@ def __exit__( _value: BaseException | None, _traceback: TracebackType | None, ) -> None: - for key, value in self.original_env_vars.items(): - if value is not None: - os.environ[key] = value - else: - del os.environ[key] + os.environ.update(self.original_env_vars) @dataclass diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index dd3f272e..474de189 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -14,8 +14,7 @@ from gateway_api.app import ( app, - get_app_host, - get_app_port, + get_env_var, log_env_vars, log_starting_app, start_app, @@ -31,29 +30,14 @@ def client() -> Generator[FlaskClient[Flask]]: class TestAppInitialization: - def test_get_app_host_returns_set_host_name(self) -> None: - os.environ["FLASK_HOST"] = "host_is_set" + def test_get_env_var_when_env_var_is_set(self) -> None: + with NewEnvVars({"FLASK_HOST": "host_is_set"}): + actual = get_env_var("FLASK_HOST", str) + assert actual == "host_is_set" - actual = get_app_host() - assert actual == "host_is_set" - - def test_get_app_host_raises_runtime_error_if_host_name_not_set(self) -> None: - del os.environ["FLASK_HOST"] - - with pytest.raises(RuntimeError): - _ = get_app_host() - - def test_get_app_port_returns_set_port_number(self) -> None: - os.environ["FLASK_PORT"] = "8080" - - actual = get_app_port() - assert actual == 8080 - - def test_get_app_port_raises_runtime_error_if_port_not_set(self) -> None: - del os.environ["FLASK_PORT"] - - with pytest.raises(RuntimeError): - _ = get_app_port() + def test_get_env_var_raises_runtime_error_if_env_var_not_set(self) -> None: + with NewEnvVars({"FLASK_HOST": None}), pytest.raises(RuntimeError): + _ = get_env_var("FLASK_HOST", str) def test_logging_app_startup_details_on_app_initialization( self, mocker: MockerFixture @@ -62,7 +46,9 @@ def test_logging_app_startup_details_on_app_initialization( host = "test_host" port = 1234 - log_starting_app(app, host, port) + pds_base_url = "test_pds_url" + sds_base_url = "test_sds_url" + log_starting_app(app, host, port, pds_base_url, sds_base_url) # Check that the app startup details were logged log_info_mock.assert_called_with( @@ -70,6 +56,8 @@ def test_logging_app_startup_details_on_app_initialization( "description": "Starting Flask app", "host": host, "port": port, + "pds_base_url": pds_base_url, + "sds_base_url": sds_base_url, } ) From d80e86e2739a5a2216683faf6e49b51b5eca2a1f Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:13:31 +0100 Subject: [PATCH 09/69] Make explicit environment variables being passed to container. --- Makefile | 3 ++- README.md | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index b4ed852b..0384262a 100644 --- a/Makefile +++ b/Makefile @@ -68,7 +68,8 @@ publish: # Publish the project artefact @Pipeline deploy: clean build # Build project artefact and deploy locally @Pipeline @$(docker) network inspect gateway-local >/dev/null 2>&1 || $(docker) network create gateway-local - # Build up list of environment variables to pass to the container + @echo "Using environment variables from .env" + @cat .env if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ echo "Starting using local docker network ..." ; \ $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local --env-file .env -d ${IMAGE_NAME} ; \ diff --git a/README.md b/README.md index c9767283..0dfb7912 100644 --- a/README.md +++ b/README.md @@ -130,9 +130,9 @@ The project uses `make` targets to build, deploy, and manage the application. Ru | `make dependencies` | Install all project dependencies via Poetry | | `make build` | Type-check, package, and build the Docker image | | `make deploy` | Build and start the Gateway API container using the environment variables defined in `.env` | -| `make deploy-*` | Create the `.env` file for the `` environment, then build and start the Gateway API container with those variables. | | `make clean` | Stop and remove the Gateway API container | | `make config` | Configure the development environment | +| `make env` | Create a `.env` to be consumed when starting the app, e.g. `make deploy` | ### API Endpoints From 67ef071ff72b226be8e7d8be5b60f6226103fd89 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:25:01 +0100 Subject: [PATCH 10/69] Make mypy happy again. --- gateway-api/src/gateway_api/conftest.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 93ccd86d..f38f7e88 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -2,6 +2,7 @@ import json import os +from collections.abc import Mapping from dataclasses import dataclass from types import TracebackType from typing import Any @@ -22,11 +23,12 @@ class NewEnvVars: - def __init__(self, new_env_vars: dict[str, str | None]) -> None: + def __init__(self, new_env_vars: Mapping[str, str | None]) -> None: self.new_env_vars = new_env_vars - self.original_env_vars = { - key: os.environ.get(key) for key in new_env_vars if key in os.environ - } + self.original_env_vars = {} + for key in new_env_vars: + if key in os.environ: + self.original_env_vars[key] = os.environ[key] def __enter__(self) -> "NewEnvVars": for key, value in self.new_env_vars.items(): From b289500228f6eec2e0147e176b9eca4abebd496d Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 20:51:59 +0100 Subject: [PATCH 11/69] Place env vars in to app config. --- .vscode/cspell-dictionary.txt | 1 + gateway-api/pyproject.toml | 2 +- gateway-api/src/gateway_api/app.py | 45 ++++++++++--------------- gateway-api/src/gateway_api/test_app.py | 43 ++++++++++++----------- 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/.vscode/cspell-dictionary.txt b/.vscode/cspell-dictionary.txt index 1a219d09..6cb2d09a 100644 --- a/.vscode/cspell-dictionary.txt +++ b/.vscode/cspell-dictionary.txt @@ -1,4 +1,5 @@ asid +conftest fhir getstructuredrecord GPCAPIM diff --git a/gateway-api/pyproject.toml b/gateway-api/pyproject.toml index cf3cac15..7770de11 100644 --- a/gateway-api/pyproject.toml +++ b/gateway-api/pyproject.toml @@ -23,7 +23,7 @@ packages = [{include = "gateway_api", from = "src"}, [tool.coverage.run] relative_files = true -omit = ["*/tests/*", "*/features/*", "*/test_*.py"] +omit = ["*/tests/*", "*/features/*", "*/test_*.py", "*/conftest.py"] [tool.coverage.paths] source = [ diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 12ce1ebe..62bb21ab 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -19,12 +19,19 @@ def start_app(app: Flask) -> None: log_env_vars(app) - app_host = get_env_var("FLASK_HOST", str) - app_port = get_env_var("FLASK_PORT", int) - pds_base_url = get_env_var("PDS_URL", str) - sds_base_url = get_env_var("SDS_URL", str) - log_starting_app(app, app_host, app_port, pds_base_url, sds_base_url) - app.run(host=app_host, port=app_port) + configure_app(app) + log_starting_app(app) + app.run(host=app.config["FLASK_HOST"], port=app.config["FLASK_PORT"]) + + +def configure_app(app: Flask) -> None: + config = { + "FLASK_HOST": get_env_var("FLASK_HOST", str), + "FLASK_PORT": get_env_var("FLASK_PORT", int), + "PDS_URL": get_env_var("PDS_URL", str), + "SDS_URL": get_env_var("SDS_URL", str), + } + app.config.update(config) def get_env_var(name: str, loader: Callable[[str], Any]) -> Any: @@ -37,20 +44,6 @@ def get_env_var(name: str, loader: Callable[[str], Any]) -> Any: raise RuntimeError(f"Error loading {name} environment variable: {e}") from e -def get_app_host() -> str: - host = os.getenv("FLASK_HOST") - if host is None: - raise RuntimeError("FLASK_HOST environment variable is not set.") - return host - - -def get_app_port() -> int: - port = os.getenv("FLASK_PORT") - if port is None: - raise RuntimeError("FLASK_PORT environment variable is not set.") - return int(port) - - def log_request_received(request: Request) -> None: log_details = { "description": "Received request", @@ -79,15 +72,13 @@ def log_env_vars(app: Flask) -> None: app.logger.info(log_details) -def log_starting_app( - app: Flask, host: str, port: int, pds_base_url: str, sds_base_url: str -) -> None: +def log_starting_app(app: Flask) -> None: log_details = { "description": "Starting Flask app", - "host": host, - "port": port, - "pds_base_url": pds_base_url, - "sds_base_url": sds_base_url, + "host": app.config["FLASK_HOST"], + "port": app.config["FLASK_PORT"], + "pds_base_url": app.config["PDS_URL"], + "sds_base_url": app.config["SDS_URL"], } app.logger.info(log_details) diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 474de189..336212e0 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -14,9 +14,9 @@ from gateway_api.app import ( app, + configure_app, get_env_var, log_env_vars, - log_starting_app, start_app, ) from gateway_api.conftest import NewEnvVars @@ -39,27 +39,29 @@ def test_get_env_var_raises_runtime_error_if_env_var_not_set(self) -> None: with NewEnvVars({"FLASK_HOST": None}), pytest.raises(RuntimeError): _ = get_env_var("FLASK_HOST", str) - def test_logging_app_startup_details_on_app_initialization( - self, mocker: MockerFixture - ) -> None: - log_info_mock = mocker.patch.object(app.logger, "info") + def test_get_env_var_raises_runtime_error_if_loader_fails(self) -> None: + with NewEnvVars({"FLASK_PORT": "not_an_int"}), pytest.raises(RuntimeError): + _ = get_env_var("FLASK_PORT", int) - host = "test_host" - port = 1234 - pds_base_url = "test_pds_url" - sds_base_url = "test_sds_url" - log_starting_app(app, host, port, pds_base_url, sds_base_url) + def test_configure_app(self) -> None: + test_app = Mock() + config = { + "FLASK_HOST": "test_host", + "FLASK_PORT": "1234", + "PDS_URL": "test_pds_url", + "SDS_URL": "test_sds_url", + } - # Check that the app startup details were logged - log_info_mock.assert_called_with( - { - "description": "Starting Flask app", - "host": host, - "port": port, - "pds_base_url": pds_base_url, - "sds_base_url": sds_base_url, - } - ) + with NewEnvVars(config): + configure_app(test_app) + + expected = { + "FLASK_HOST": "test_host", + "FLASK_PORT": 1234, + "PDS_URL": "test_pds_url", + "SDS_URL": "test_sds_url", + } + test_app.config.update.assert_called_with(expected) def test_logging_environment_variables_on_app_initialization( self, mocker: MockerFixture @@ -78,6 +80,7 @@ def test_logging_environment_variables_on_app_initialization( def test_start_app_logs_startup_details(self) -> None: test_app = Mock() + test_app.config = {} test_env_vars = { "FLASK_HOST": "test_host", From 69a2929a3e3daa5ef353389f9c624f615e19e43a Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:06:17 +0100 Subject: [PATCH 12/69] Pass URLs to controller. --- gateway-api/src/gateway_api/app.py | 4 ++- gateway-api/src/gateway_api/controller.py | 4 +-- gateway-api/src/gateway_api/test_app.py | 11 ++++++- .../src/gateway_api/test_controller.py | 29 ++++++++++++------- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 62bb21ab..49b4c412 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -90,7 +90,9 @@ def get_structured_record() -> Response: response.mirror_headers(request) try: get_structured_record_request = GetStructuredRecordRequest(request) - controller = Controller() + controller = Controller( + pds_base_url=app.config["PDS_URL"], sds_base_url=app.config["SDS_URL"] + ) provider_response = controller.run(request=get_structured_record_request) response.add_provider_response(provider_response) except AbstractCDGError as e: diff --git a/gateway-api/src/gateway_api/controller.py b/gateway-api/src/gateway_api/controller.py index ac37c16e..539bea71 100644 --- a/gateway-api/src/gateway_api/controller.py +++ b/gateway-api/src/gateway_api/controller.py @@ -27,8 +27,8 @@ class Controller: def __init__( self, - pds_base_url: str = PdsClient.SANDBOX_URL, - sds_base_url: str = SdsClient.SANDBOX_URL, + pds_base_url: str, + sds_base_url: str, timeout: int = 10, ) -> None: """ diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 336212e0..5edfbfb5 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -24,7 +24,16 @@ @pytest.fixture def client() -> Generator[FlaskClient[Flask]]: - app.config["TESTING"] = True + with NewEnvVars( + { + "FLASK_HOST": "localhost", + "FLASK_PORT": "5000", + "PDS_URL": "http://test-pds-url", + "SDS_URL": "http://test-sds-url", + } + ): + configure_app(app) + app.config["TESTING"] = True with app.test_client() as client: yield client diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index d6cfb1c1..38201c9a 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -42,12 +42,19 @@ def _create_patient(nhs_number: str, gp_ods_code: str | None) -> Patient: ) +def create_test_controller( + pds_base_url: str = "https://example.test/pds", + sds_base_url: str = "https://example.test/sds", +) -> Controller: + return Controller(pds_base_url=pds_base_url, sds_base_url=sds_base_url) + + def test_controller_run_happy_path_returns_200_status_code( mock_happy_path_get_structured_record_request: Request, ) -> None: request = GetStructuredRecordRequest(mock_happy_path_get_structured_record_request) - controller = Controller() + controller = create_test_controller() actual_response = controller.run(request) assert actual_response.status_code == 200 @@ -59,7 +66,7 @@ def test_controller_run_happy_path_returns_returns_expected_body( ) -> None: request = GetStructuredRecordRequest(mock_happy_path_get_structured_record_request) - controller = Controller() + controller = create_test_controller() actual_response = controller.run(request) assert actual_response.json() == valid_simple_response_payload @@ -74,7 +81,7 @@ def test_get_pds_details_returns_provider_ods_code_for_happy_path( "gateway_api.pds.PdsClient.search_patient_by_nhs_number", return_value=_create_patient(nhs_number, "A12345"), ) - controller = Controller(pds_base_url="https://example.test/pds", timeout=7) + controller = create_test_controller() actual = controller._get_pds_details(auth_token, nhs_number) # noqa: SLF001 testing private method @@ -91,7 +98,7 @@ def test_get_pds_details_raises_no_current_provider_when_ods_code_missing_in_pds return_value=_create_patient(nhs_number, None), ) - controller = Controller() + controller = create_test_controller() with pytest.raises( NoCurrentProviderError, @@ -117,7 +124,7 @@ def test_get_sds_details_returns_consumer_and_provider_details_for_happy_path( side_effect=sds_results, ) - controller = Controller() + controller = create_test_controller() expected = ("ConsumerASID", "ProviderASID", "https://example.provider.org/endpoint") actual = controller._get_sds_details(consumer_ods, provider_ods) # noqa: SLF001 testing private method @@ -135,7 +142,7 @@ def test_get_sds_details_raises_no_organisation_found_when_sds_returns_none( return_value=no_results_for_provider, ) - controller = Controller() + controller = create_test_controller() with pytest.raises( NoOrganisationFoundError, @@ -157,7 +164,7 @@ def test_get_sds_details_raises_no_asid_found_when_sds_returns_empty_asid( return_value=blank_asid_sds_result, ) - controller = Controller() + controller = create_test_controller() with pytest.raises( NoAsidFoundError, @@ -180,7 +187,7 @@ def test_get_sds_details_raises_no_current_endpoint_when_sds_returns_empty_endpo return_value=blank_endpoint_sds_result, ) - controller = Controller() + controller = create_test_controller() with pytest.raises( NoCurrentEndpointError, @@ -207,7 +214,7 @@ def test_get_sds_details_raises_no_org_found_when_sds_returns_none_for_consumer( side_effect=[happy_path_provider_sds_result, none_result_for_consumer], ) - controller = Controller() + controller = create_test_controller() with pytest.raises( NoOrganisationFoundError, @@ -233,7 +240,7 @@ def test_get_sds_details_raises_no_asid_found_when_sds_returns_empty_consumer_as side_effect=[happy_path_provider_sds_result, consumer_asid_blank_sds_result], ) - controller = Controller() + controller = create_test_controller() with pytest.raises( NoAsidFoundError, @@ -337,7 +344,7 @@ def test_controller_creates_jwt_token_with_correct_claims( get_structured_record_request = GetStructuredRecordRequest(request) - controller = Controller() + controller = create_test_controller() controller.run(get_structured_record_request) # Verify that GpProviderClient was called and extract the JWT token From 5268215faabf9d02ff33e42d4f85267085679071 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:13:32 +0100 Subject: [PATCH 13/69] Remove default URLs --- gateway-api/src/gateway_api/pds/client.py | 2 +- .../src/gateway_api/pds/test_client.py | 12 ++++----- gateway-api/src/gateway_api/sds/client.py | 4 +-- .../src/gateway_api/sds/test_client.py | 26 +++++++++---------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 4e255f41..3ffa1199 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -73,7 +73,7 @@ class PdsClient: def __init__( self, auth_token: str, - base_url: str = SANDBOX_URL, + base_url: str, timeout: int = 10, ignore_dates: bool = False, ) -> None: diff --git a/gateway-api/src/gateway_api/pds/test_client.py b/gateway-api/src/gateway_api/pds/test_client.py index 0263ea89..414ceefc 100644 --- a/gateway-api/src/gateway_api/pds/test_client.py +++ b/gateway-api/src/gateway_api/pds/test_client.py @@ -24,7 +24,7 @@ def test_search_patient_by_nhs_number_happy_path( ) mocker.patch("gateway_api.pds.client.get", return_value=happy_path_response) - client = PdsClient(auth_token) + client = PdsClient(auth_token, base_url="https://test.com") patient = client.search_patient_by_nhs_number("9999999999") assert isinstance(patient, Patient) @@ -44,7 +44,7 @@ def test_search_patient_by_nhs_number_has_no_gp_returns_gp_ods_code_none( ) mocker.patch("gateway_api.pds.client.get", return_value=gp_less_response) - client = PdsClient(auth_token) + client = PdsClient(auth_token, base_url="https://test.com") patient = client.search_patient_by_nhs_number("9999999999") assert isinstance(patient, Patient) @@ -67,7 +67,7 @@ def test_search_patient_by_nhs_number_sends_expected_headers( request_id = str(uuid4()) correlation_id = "corr-123" - client = PdsClient(auth_token) + client = PdsClient(auth_token, base_url="https://test.com") _ = client.search_patient_by_nhs_number( "9000000009", request_id=request_id, @@ -96,7 +96,7 @@ def test_search_patient_by_nhs_number_generates_request_id( "gateway_api.pds.client.get", return_value=happy_path_response ) - client = PdsClient(auth_token) + client = PdsClient(auth_token, base_url="https://test.com") _ = client.search_patient_by_nhs_number("9000000009") @@ -117,7 +117,7 @@ def test_search_patient_by_nhs_number_not_found_raises_error( reason="Not Found", ) mocker.patch("gateway_api.pds.client.get", return_value=not_found_response) - pds = PdsClient(auth_token) + pds = PdsClient(auth_token, base_url="https://test.com") with pytest.raises( PdsRequestFailedError, match="PDS FHIR API request failed: Not Found" @@ -140,7 +140,7 @@ def test_search_patient_by_nhs_number_missing_nhs_number_raises_error( ) mocker.patch("gateway_api.pds.client.get", return_value=response) - client = PdsClient(auth_token) + client = PdsClient(auth_token, base_url="https://test.com") with pytest.raises(PdsRequestFailedError) as error: client.search_patient_by_nhs_number("9999999999") diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index 871e858d..82c8c9e8 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -54,7 +54,7 @@ class SdsClient: **Stubbing**: - For testing, set the environment variable ``$SDS_URL` to use the + For testing, set the environment variable ``$SDS_URL`` to use the :class:`SdsFhirApiStub` instead of making real HTTP requests. **Usage example**:: @@ -80,7 +80,7 @@ class SdsClient: def __init__( self, - base_url: str = SANDBOX_URL, + base_url: str, timeout: int = 10, service_interaction_id: str | None = None, ) -> None: diff --git a/gateway-api/src/gateway_api/sds/test_client.py b/gateway-api/src/gateway_api/sds/test_client.py index 76019991..a10bc023 100644 --- a/gateway-api/src/gateway_api/sds/test_client.py +++ b/gateway-api/src/gateway_api/sds/test_client.py @@ -36,7 +36,7 @@ def test_sds_client_get_org_details_success( :param stub: SDS stub fixture. """ - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") result = client.get_org_details(ods_code="PROVIDER") @@ -113,7 +113,7 @@ def test_sds_client_get_org_details_with_endpoint( }, ) - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") result = client.get_org_details(ods_code="TESTORG") assert result is not None @@ -130,7 +130,7 @@ def test_sds_client_sends_correct_headers( :param stub: SDS stub fixture. :param mock_requests_get: Capture fixture for request details. """ - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") correlation_id = "test-correlation-123" client.get_org_details(ods_code="PROVIDER", correlation_id=correlation_id) @@ -152,7 +152,7 @@ def test_sds_client_timeout_parameter( :param stub: SDS stub fixture. :param mock_requests_get: Capture fixture for request details. """ - client = SdsClient(base_url=SdsClient.SANDBOX_URL, timeout=30) + client = SdsClient(base_url="https://test.com", timeout=30) client.get_org_details(ods_code="PROVIDER", timeout=60) @@ -195,7 +195,7 @@ def test_sds_client_custom_service_interaction_id( ) client = SdsClient( - base_url=SdsClient.SANDBOX_URL, + base_url="https://test.com", service_interaction_id=custom_interaction, ) @@ -221,7 +221,7 @@ def test_sds_client_builds_correct_device_query_params( :param stub: SDS stub fixture. :param mock_requests_get: Capture fixture for request details. """ - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") client.get_org_details(ods_code="PROVIDER") @@ -249,7 +249,7 @@ def test_sds_client_extract_party_key_from_device( :param mock_requests_get: Capture fixture for request details. """ # The default seeded PROVIDER device has a party key - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") stub.upsert_device( organization_ods="WITHPARTYKEY", @@ -322,7 +322,7 @@ def get_without_apikey( monkeypatch.setattr("gateway_api.sds.client.get", get_without_apikey) - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") with pytest.raises(SdsRequestFailedError, match="SDS FHIR API request failed"): client.get_org_details(ods_code="PROVIDER") @@ -365,7 +365,7 @@ def test_sds_client_endpoint_entry_without_address_returns_none( }, ) - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") result = client.get_org_details(ods_code="NOADDR") assert result.asid == "111111111111" @@ -381,9 +381,9 @@ def test_sds_client_empty_device_bundle_returns_none_asid() -> None: :param stub: SDS stub fixture. """ - client = SdsClient(base_url=SdsClient.SANDBOX_URL) - # "UNKNOWNORG" has no seeded devices, so the bundle entry list will be empty - result = client.get_org_details(ods_code="UNKNOWNORG", get_endpoint=False) + client = SdsClient(base_url="https://test.com") + # "UNKNOWN_ORG" has no seeded devices, so the bundle entry list will be empty + result = client.get_org_details(ods_code="UNKNOWN_ORG", get_endpoint=False) assert result.asid is None @@ -412,7 +412,7 @@ def test_sds_client_no_endpoint_bundle_entries_returns_none_endpoint( ) # Deliberately do not seed any endpoint for NOENDPOINT - client = SdsClient(base_url=SdsClient.SANDBOX_URL) + client = SdsClient(base_url="https://test.com") result = client.get_org_details(ods_code="NOENDPOINT") assert result.asid == "222222222222" From 3cdecdbb395849ab1f7fa24e33e485cefd5a7732 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:22:52 +0100 Subject: [PATCH 14/69] Tidy up. --- gateway-api/src/gateway_api/pds/client.py | 5 ----- gateway-api/src/gateway_api/sds/client.py | 4 ---- 2 files changed, 9 deletions(-) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 3ffa1199..b848c9a6 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -65,11 +65,6 @@ class PdsClient: print(result) """ - # URLs for different PDS environments. Requires authentication to use live. - SANDBOX_URL = "https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4" - INT_URL = "https://int.api.service.nhs.uk/personal-demographics/FHIR/R4" - PROD_URL = "https://api.service.nhs.uk/personal-demographics/FHIR/R4" - def __init__( self, auth_token: str, diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index 82c8c9e8..f4da64ae 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -71,10 +71,6 @@ class SdsClient: print(f"ASID: {result.asid}, Endpoint: {result.endpoint}") """ - # URLs for different SDS environments. Will move to a config file eventually. - SANDBOX_URL = "https://sandbox.api.service.nhs.uk/spine-directory/FHIR/R4" - INT_URL = "https://int.api.service.nhs.uk/spine-directory/FHIR/R4" - # Default service interaction ID for GP Connect DEFAULT_SERVICE_INTERACTION_ID = ACCESS_RECORD_STRUCTURED_INTERACTION_ID From ba67227f535d12f006d48903cbf79ca38bc47262 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:38:50 +0100 Subject: [PATCH 15/69] Improve make commands to build up env files for different environments. --- scripts/env/env.mk | 19 +++--------------- scripts/env/env.sh | 43 +++++++++++++++++++++++++++++++++++++++++ scripts/env/pds.sh | 21 ++++++++++++++++++++ scripts/env/provider.sh | 18 +++++++++++++++++ scripts/env/sds.sh | 21 ++++++++++++++++++++ 5 files changed, 106 insertions(+), 16 deletions(-) create mode 100755 scripts/env/env.sh create mode 100755 scripts/env/pds.sh create mode 100755 scripts/env/provider.sh create mode 100755 scripts/env/sds.sh diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 6c93b1f7..db7a267e 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -1,26 +1,13 @@ .PHONY: env env-% _env env: - make env-dev # TODO: Make this interactive + make _env env-%: # Create .env file with environment variables - optional: name=[name of the environment, e.g. 'dev'] @Configuration - make _env name="$*" # TODO: Implement difference envs + make _env env="$*" # TODO: Implement difference envs _env: - echo "# ENVIRONMENT VARIABLES" > .env - echo "# ---------------------" >> .env - echo "# This file is generated by 'make env'/'make env-*'." >> .env - - echo "FLASK_PORT=8080" >> .env - echo "FLASK_HOST=0.0.0.0" >> .env - - echo "BASE_URL=http://gateway-api:8080" >> .env - - echo "PDS_URL=stub" >> .env - echo "PROVIDER_URL=stub" >> .env - echo "SDS_URL=stub" >> .env - - echo "CDG_DEBUG=false" >> .env + scripts/env/env.sh "$(env)" ${VERBOSE}.SILENT: \ _env \ diff --git a/scripts/env/env.sh b/scripts/env/env.sh new file mode 100755 index 00000000..772dc476 --- /dev/null +++ b/scripts/env/env.sh @@ -0,0 +1,43 @@ +#!/bin/bash +set -e + +source scripts/env/pds.sh +source scripts/env/sds.sh +source scripts/env/provider.sh + +ENV_FILE=".env" + +if [ -f "$ENV_FILE" ]; then + printf "'%s' already exists. Overwrite? [y/N]: " "$ENV_FILE" + read -r answer + case "$answer" in + [yY]|[yY][eE][sS]) + echo "Overwriting $ENV_FILE..." + ;; + *) + echo "Aborted." + exit 0 + ;; + esac +fi + +PDS_URL=$(prompt_pds_url "$env") +SDS_URL=$(prompt_sds_url "$env") +PROVIDER_URL=$(prompt_provider_url "$env") + +cat > "$ENV_FILE" < Date: Mon, 20 Apr 2026 15:47:29 +0100 Subject: [PATCH 16/69] FLASK env vars are not actually variable. Leave them hardcoded. --- infrastructure/images/gateway-api/Dockerfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/infrastructure/images/gateway-api/Dockerfile b/infrastructure/images/gateway-api/Dockerfile index cfbf2c50..f64e5814 100644 --- a/infrastructure/images/gateway-api/Dockerfile +++ b/infrastructure/images/gateway-api/Dockerfile @@ -23,6 +23,8 @@ RUN if [ "$INCLUDE_DEV_CERTS" = "true" ] && [ -d /resources/dev-certificates ]; WORKDIR /resources/build/gateway-api ENV PYTHONPATH=/resources/build/gateway-api +ENV FLASK_HOST="0.0.0.0" +ENV FLASK_PORT="8080" ARG COMMIT_VERSION # TODO: Do we want to do something with this env var? From a2866c5f26f9fed35bf1909ff3db3a21f34381d1 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:28:25 +0100 Subject: [PATCH 17/69] Enable test environment to be set up easily. --- .env.template | 10 ---- .github/actions/run-test-suite/action.yaml | 26 ++++------ .github/workflows/alpha-integration-env.yml | 37 ++------------ .github/workflows/preview-env.yml | 37 ++------------ .github/workflows/stage-2-test.yaml | 17 ++++--- gateway-api/tests/conftest.py | 50 ++++++++----------- .../tests/schema/test_openapi_schema.py | 11 +++- scripts/env/apigee.sh | 14 ++++++ scripts/env/{env.sh => app-env.sh} | 26 +++------- scripts/env/env.mk | 43 +++++++++++++--- scripts/env/helper.sh | 18 +++++++ scripts/env/pds.sh | 26 ++++------ scripts/env/provider.sh | 23 ++++----- scripts/env/sds.sh | 26 ++++------ scripts/env/target.sh | 46 +++++++++++++++++ scripts/env/test-env.sh | 38 ++++++++++++++ scripts/tests/run-test.sh | 39 ++++----------- 17 files changed, 264 insertions(+), 223 deletions(-) delete mode 100644 .env.template create mode 100644 scripts/env/apigee.sh rename scripts/env/{env.sh => app-env.sh} (53%) create mode 100644 scripts/env/helper.sh create mode 100644 scripts/env/target.sh create mode 100755 scripts/env/test-env.sh diff --git a/.env.template b/.env.template deleted file mode 100644 index b141e2e3..00000000 --- a/.env.template +++ /dev/null @@ -1,10 +0,0 @@ -FLASK_PORT= -FLASK_HOST= - -BASE_URL= -HOST= - -STUB_PDS= -STUB_PROVIDER= -STUB_SDS= - diff --git a/.github/actions/run-test-suite/action.yaml b/.github/actions/run-test-suite/action.yaml index 1113c720..dfbbda99 100644 --- a/.github/actions/run-test-suite/action.yaml +++ b/.github/actions/run-test-suite/action.yaml @@ -9,33 +9,27 @@ inputs: test-type: description: "Type of test to run" required: true - apigee-access-token: - description: "Apigee access token" - required: false - base-url: - description: "The URL of the environment to test" - required: false env: - description: "Environment: local or remote" + description: "Environment to run tests against - see env.mk" required: false - default: "remote" + default: "ci" + options: + - ci + - alpha-int + - pr- runs: using: composite steps: + - name: Set up environment + run: | + make env-test-{{ inputs.env }} + - name: "Run ${{ inputs.test-type }} tests" shell: bash env: - APIGEE_ACCESS_TOKEN: ${{ inputs.apigee-access-token }} - BASE_URL: ${{ inputs.base-url }} - ENV: ${{ inputs.env }} TEST_TYPE: ${{ inputs.test-type }} run: | - if [[ -n "${APIGEE_ACCESS_TOKEN}" ]]; then - echo "::add-mask::${APIGEE_ACCESS_TOKEN}" - fi - - # Clean test artefacts so each suite uploads only its own results rm -rf gateway-api/test-artefacts/* || true mkdir -p gateway-api/test-artefacts make test-"${TEST_TYPE}" diff --git a/.github/workflows/alpha-integration-env.yml b/.github/workflows/alpha-integration-env.yml index fb80f02d..cd2d0a1e 100644 --- a/.github/workflows/alpha-integration-env.yml +++ b/.github/workflows/alpha-integration-env.yml @@ -196,63 +196,34 @@ jobs: echo "http_result=unexpected-status" >> "$GITHUB_OUTPUT" exit 0 - - name: Retrieve Apigee Token - id: apigee-token - shell: bash - run: | - set -euo pipefail - - APIGEE_TOKEN="$(proxygen pytest-nhsd-apim get-token | jq -r '.pytest_nhsd_apim_token' 2>/dev/null)" - if [ -z "$APIGEE_TOKEN" ] || [ "$APIGEE_TOKEN" = "null" ]; then - echo "::error::Failed to retrieve Apigee token" - exit 1 - fi - - echo "::add-mask::$APIGEE_TOKEN" - printf 'apigee-access-token=%s\n' "$APIGEE_TOKEN" >> "$GITHUB_OUTPUT" - echo "Token retrieved successfully (length: ${#APIGEE_TOKEN})" - - name: Run unit tests uses: ./.github/actions/run-test-suite with: test-type: unit - env: local - name: Run contract tests uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-alpha-integration" with: test-type: contract - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: alpha-int - name: Run schema validation tests uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-alpha-integration" with: test-type: schema - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: alpha-int - name: Run integration tests uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-alpha-integration" with: test-type: integration - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: alpha-int - name: Run acceptance tests uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-alpha-integration" with: test-type: acceptance - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: alpha-int - name: Remove mTLS temp files run: rm -f /tmp/client1-key.pem /tmp/client1-cert.pem diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index fed28082..11235e91 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -315,68 +315,39 @@ jobs: # ---------- QUALITY CHECKS (Test Suites) ---------- - - name: Retrieve Apigee Token - id: apigee-token - shell: bash - run: | - set -euo pipefail - - APIGEE_TOKEN="$(proxygen pytest-nhsd-apim get-token | jq -r '.pytest_nhsd_apim_token' 2>/dev/null)" - if [ -z "$APIGEE_TOKEN" ] || [ "$APIGEE_TOKEN" = "null" ]; then - echo "::error::Failed to retrieve Apigee token" - exit 1 - fi - - echo "::add-mask::$APIGEE_TOKEN" - printf 'apigee-access-token=%s\n' "$APIGEE_TOKEN" >> "$GITHUB_OUTPUT" - echo "Token retrieved successfully (length: ${#APIGEE_TOKEN})" - - name: "Run unit tests" if: github.event.action != 'closed' uses: ./.github/actions/run-test-suite with: test-type: unit - env: local - name: "Run contract tests" if: github.event.action != 'closed' uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}" with: test-type: contract - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: pr-${{ github.event.pull_request.number }} - name: "Run schema validation tests" if: github.event.action != 'closed' uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}" with: test-type: schema - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: pr-${{ github.event.pull_request.number }} - name: "Run integration tests" if: github.event.action != 'closed' uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}" with: test-type: integration - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: pr-${{ github.event.pull_request.number }} - name: "Run acceptance tests" if: github.event.action != 'closed' uses: ./.github/actions/run-test-suite - env: - PROXY_BASE_PATH: "clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}" with: test-type: acceptance - apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} - base-url: ${{ env.BASE_URL }} + env: pr-${{ github.event.pull_request.number }} # Cleanup after tests - name: Remove mTLS temp files diff --git a/.github/workflows/stage-2-test.yaml b/.github/workflows/stage-2-test.yaml index 2adbcd39..5411be87 100644 --- a/.github/workflows/stage-2-test.yaml +++ b/.github/workflows/stage-2-test.yaml @@ -1,12 +1,5 @@ name: "Test stage" -env: - BASE_URL: "http://localhost:5000" - HOST: "localhost" - SDS_URL: "stub" - PDS_URL: "stub" - PROVIDER_URL: "stub" - on: workflow_call: inputs: @@ -45,6 +38,8 @@ jobs: uses: ./.github/actions/setup-python-project with: python-version: ${{ inputs.python_version }} + - name: Set environment variables + run: make env-test-ci - name: "Run unit test suite" run: make test-unit - name: "Upload unit test results" @@ -75,6 +70,8 @@ jobs: uses: ./.github/actions/start-app with: python-version: ${{ inputs.python_version }} + - name: Set environment variables + run: make env-test-ci - name: "Run contract tests" run: make test-contract - name: "Upload contract test results" @@ -105,6 +102,8 @@ jobs: uses: ./.github/actions/start-app with: python-version: ${{ inputs.python_version }} + - name: Set environment variables + run: make env-test-ci - name: "Run schema validation tests" run: make test-schema - name: "Upload schema test results" @@ -135,6 +134,8 @@ jobs: uses: ./.github/actions/start-app with: python-version: ${{ inputs.python_version }} + - name: Set environment variables + run: make env-test-ci - name: "Run integration test" run: make test-integration - name: "Upload integration test results" @@ -166,6 +167,8 @@ jobs: with: python-version: ${{ inputs.python_version }} max-seconds: 90 + - name: Set environment variables + run: make env-test-ci - name: "Run acceptance test" run: make test-acceptance - name: "Upload acceptance test results" diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index f77c729d..5d012974 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -6,13 +6,8 @@ import pytest import requests -from dotenv import find_dotenv, load_dotenv from fhir.constants import FHIRSystem -# Load environment variables from .env file in the workspace root -load_dotenv(find_dotenv(usecwd=True)) - - DEFAULT_REQUEST_HEADERS = { "Content-Type": "application/fhir+json", "Ods-from": "CONSUMER", @@ -160,9 +155,8 @@ def simple_request_payload() -> dict[str, Any]: @pytest.fixture -def get_headers(request: pytest.FixtureRequest) -> dict[str, str]: +def get_headers(env: str, request: pytest.FixtureRequest) -> dict[str, str]: """Return auth headers for remote tests, or Apigee token for local.""" - env = request.config.getoption("--env") if env == "local": token = os.environ.get("APIGEE_ACCESS_TOKEN", "") return {"Authorization": f"Bearer {token}"} if token else {} @@ -178,11 +172,12 @@ def get_headers(request: pytest.FixtureRequest) -> dict[str, str]: @pytest.fixture def client( - request: pytest.FixtureRequest, base_url: str, get_headers: dict[str, str] + env: str, + request: pytest.FixtureRequest, + get_headers: dict[str, str], ) -> Client: - env = request.config.getoption("--env") - if env == "local": + base_url = request.getfixturevalue("base_url") return LocalClient(base_url=base_url, auth_headers=get_headers) elif env == "remote": proxy_url = request.getfixturevalue("nhsd_apim_proxy_url") @@ -192,15 +187,22 @@ def client( @pytest.fixture(scope="module") -def base_url() -> str: - """Retrieves the base URL of the currently deployed application.""" - return _fetch_env_variable("BASE_URL", str) +def env() -> str: + return get_env() + + +def get_env() -> str: + try: + _ = _fetch_env_variable("BASE_URL", str) + return "local" + except ValueError: + return "remote" @pytest.fixture(scope="module") -def hostname() -> str: - """Retrieves the hostname of the currently deployed application.""" - return _fetch_env_variable("HOST", str) +def base_url() -> str: + """Retrieves the base URL of the currently deployed application.""" + return _fetch_env_variable("BASE_URL", str) def _fetch_env_variable[T]( @@ -222,19 +224,9 @@ def _get_remote_test_username() -> str: return os.getenv(REMOTE_TEST_USERNAME_ENV_VAR, DEFAULT_REMOTE_TEST_USERNAME) -def pytest_addoption(parser: pytest.Parser) -> None: - parser.addoption( - "--env", - action="store", - default="local", - help="Environment to run tests against", - ) - - -def pytest_collection_modifyitems( - config: pytest.Config, items: list[pytest.Item] -) -> None: - if config.getoption("--env") == "remote": +def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: + is_remote = get_env() == "remote" + if is_remote: for item in items: item.add_marker( pytest.mark.nhsd_apim_authorization( diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index 1a239158..5355b941 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -6,6 +6,7 @@ from pathlib import Path +import pytest import schemathesis import yaml from schemathesis.generation.case import Case @@ -19,7 +20,9 @@ @schema.parametrize() -def test_api_schema_compliance(case: Case, base_url: str) -> None: +def test_api_schema_compliance( + case: Case, env: str, request: pytest.FixtureRequest +) -> None: """Test API endpoints against the OpenAPI schema. Schemathesis automatically generates test cases with: @@ -41,6 +44,12 @@ def test_api_schema_compliance(case: Case, base_url: str) -> None: case.headers["Ods-from"] = "test-ods-code" case.headers["Ssp-TraceID"] = "test-trace-id" + # TODO: Do this better + if env == "local": + base_url = request.getfixturevalue("base_url") + else: + base_url = request.getfixturevalue("nhsd_apim_proxy_url") + case.call_and_validate( base_url=base_url, excluded_checks=[schemathesis.checks.not_a_server_error], diff --git a/scripts/env/apigee.sh b/scripts/env/apigee.sh new file mode 100644 index 00000000..0d082f40 --- /dev/null +++ b/scripts/env/apigee.sh @@ -0,0 +1,14 @@ +#!/bin/bash +set -e + +prompt_apigee_access_token() { + env="$1" + case "$env" in + pr-*|alpha-int) + echo $(./scripts/get_apigee_token.sh) + ;; + *) + echo "" + ;; + esac +} diff --git a/scripts/env/env.sh b/scripts/env/app-env.sh similarity index 53% rename from scripts/env/env.sh rename to scripts/env/app-env.sh index 772dc476..9d63481e 100755 --- a/scripts/env/env.sh +++ b/scripts/env/app-env.sh @@ -4,22 +4,11 @@ set -e source scripts/env/pds.sh source scripts/env/sds.sh source scripts/env/provider.sh +source scripts/env/helper.sh ENV_FILE=".env" -if [ -f "$ENV_FILE" ]; then - printf "'%s' already exists. Overwrite? [y/N]: " "$ENV_FILE" - read -r answer - case "$answer" in - [yY]|[yY][eE][sS]) - echo "Overwriting $ENV_FILE..." - ;; - *) - echo "Aborted." - exit 0 - ;; - esac -fi +confirm_overwrite "$ENV_FILE" PDS_URL=$(prompt_pds_url "$env") SDS_URL=$(prompt_sds_url "$env") @@ -28,14 +17,15 @@ PROVIDER_URL=$(prompt_provider_url "$env") cat > "$ENV_FILE" < to generate a .env.test file for the specified environment. +# See scripts/env/env.mk for the make targets that call this script. +set -e + +source scripts/env/target.sh +source scripts/env/helper.sh +source scripts/env/apigee.sh + +env="$1" +BASE_URL=$(prompt_base_url "$env") +PROXYGEN_API_NAME=$(prompt_proxygen_api_name "$env") +PROXY_BASE_PATH=$(prompt_proxy_base_path "$env") +if [[ -z "$PROXYGEN_API_NAME" || -z "$PROXY_BASE_PATH" ]]; then + PROXY_NAME="" +else + PROXY_NAME="${PROXYGEN_API_NAME}--internal-dev--${PROXY_BASE_PATH}" +fi +APIGEE_ACCESS_TOKEN=$(prompt_apigee_access_token "$env") + +ENV_FILE=".env.test" +overwrite="$2" +if [[ "$overwrite" != "true" ]]; then + confirm_overwrite "$ENV_FILE" +fi + +cat > "$ENV_FILE" <&2 - exit 1 - fi - # Note: TEST_PATH is intentionally unquoted to allow glob expansion - poetry run pytest ${TEST_PATH} --env="remote" -v \ - --api-name="${PROXYGEN_API_NAME}" \ - --proxy-name="${PROXYGEN_API_NAME}--internal-dev--${PROXY_BASE_PATH}" \ - --cov="${COV_PATH}" \ - --cov-report=html:test-artefacts/coverage-html \ - --cov-report=term \ - --junit-xml="test-artefacts/${TEST_TYPE}-tests.xml" \ - --html="test-artefacts/${TEST_TYPE}-tests.html" --self-contained-html -else - echo "[run-test] Branch: local/default path" - echo "[run-test] ENV=${ENV:-local}, TEST_TYPE=${TEST_TYPE}" - echo "[run-test] Running direct tests without APIM proxy options" - poetry run pytest ${TEST_PATH} -v \ - --cov="${COV_PATH}" \ - --cov-report=html:test-artefacts/coverage-html \ - --cov-report=term \ - --junit-xml="test-artefacts/${TEST_TYPE}-tests.xml" \ - --html="test-artefacts/${TEST_TYPE}-tests.html" --self-contained-html -fi +# TODO: Add some logging to prove which URLS are being hit +# TODO: Can we remove the cli args for pytest_nhsd_apim - they _should_ be retrievable from the env +poetry run pytest ${TEST_PATH} -v \ + --api-name="${PROXYGEN_API_NAME}" \ + --proxy-name="${PROXYGEN_API_NAME}--internal-dev--${PROXY_BASE_PATH}" \ + --cov="${COV_PATH}" \ + --cov-report=html:test-artefacts/coverage-html \ + --cov-report=term \ + --junit-xml="test-artefacts/${TEST_TYPE}-tests.xml" \ + --html="test-artefacts/${TEST_TYPE}-tests.html" --self-contained-html # Save coverage data file for merging mv .coverage "test-artefacts/coverage.${TEST_TYPE}" From a0a47d345318cf3e05a650283ab2924f6a69f4c4 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:41:52 +0100 Subject: [PATCH 18/69] Define shell. --- .github/actions/run-test-suite/action.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/run-test-suite/action.yaml b/.github/actions/run-test-suite/action.yaml index dfbbda99..454dfa25 100644 --- a/.github/actions/run-test-suite/action.yaml +++ b/.github/actions/run-test-suite/action.yaml @@ -22,8 +22,8 @@ runs: using: composite steps: - name: Set up environment - run: | - make env-test-{{ inputs.env }} + shell: bash + run: make env-test-{{ inputs.env }} - name: "Run ${{ inputs.test-type }} tests" shell: bash From e6e1c145ef3e2574857e9bcf9583a98b2a303447 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:54:30 +0100 Subject: [PATCH 19/69] Inject env correctly and safely. --- .github/actions/run-test-suite/action.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/actions/run-test-suite/action.yaml b/.github/actions/run-test-suite/action.yaml index 454dfa25..77a26ee8 100644 --- a/.github/actions/run-test-suite/action.yaml +++ b/.github/actions/run-test-suite/action.yaml @@ -23,7 +23,9 @@ runs: steps: - name: Set up environment shell: bash - run: make env-test-{{ inputs.env }} + env: + ENV: ${{ inputs.env }} + run: make env-test-"${ENV}" - name: "Run ${{ inputs.test-type }} tests" shell: bash From 78663fa4c9465510dc5a22605cc37fa56e0d4780 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:06:16 +0100 Subject: [PATCH 20/69] Remove duplicate/unused make commands. --- scripts/tests/test.mk | 52 ------------------------------------------- 1 file changed, 52 deletions(-) diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index a49dd659..897ffb9c 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -97,55 +97,3 @@ ${VERBOSE}.SILENT: \ test-unit \ test-acceptance \ test-schema\ - -# --------------------------------------------------------------------------- -# Local vs remote environment helpers -# --------------------------------------------------------------------------- -# Usage: -# make test-local - activate .env.local, run tests against local -# and api-gateway-mock -# make test-remote - activate .env.remote, obtain an APIGEE token, run -# tests against APIM proxy -# -# .env.local - local env config (ENV=local, BASE_URL,HOST) -# .env.remote - remote env config (ENV=remote, BASE_URL,HOST,PR_NUMBER,PROXYGEN_API_NAME) -# .env - active env, overwritten by env-local/env-remote -# -# The .env file is deterministically overwritten each time you switch -# environments so the test runner always sees a consistent configuration. -# =========================================================================== - -.PHONY: env-local env-remote test-local test-remote - -env-local: - @if [ ! -f .env.local ]; then \ - echo "ERROR: .env.local not found. Needs creating." >&2; \ - exit 1; \ - fi - cp -f .env.local .env - @echo "Activated local environment: .env.local -> .env (ENV=local)" - -env-remote: - @if [ ! -f .env.remote ]; then \ - echo "ERROR: .env.remote not found. Needs creating." >&2; \ - exit 1; \ - fi - cp -f .env.remote .env - @echo "Activated remote environment: .env.remote -> .env (ENV=remote)" - -# Run tests against local -test-local: env-local - @echo "Obtaining APIGEE access token..." - @set -a && source .env && set +a && \ - APIGEE_ACCESS_TOKEN="$$(./scripts/get_apigee_token.sh)" && \ - export APIGEE_ACCESS_TOKEN && \ - $(MAKE) test - -# Run tests against remote, exporting APIGEE_ACCESS_TOKEN only -test-remote: env-remote - @echo "Obtaining APIGEE access token..." - @set -a && source .env && set +a && \ - APIGEE_ACCESS_TOKEN="$$(./scripts/get_apigee_token.sh)" && \ - BASE_URL="$${BASE_URL}-pr-$${PR_NUMBER}" && \ - export APIGEE_ACCESS_TOKEN BASE_URL && \ - $(MAKE) test From 15528f8f59bb4b2e0a6f8ed694f5a80ef78ed681 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:14:19 +0100 Subject: [PATCH 21/69] Add simple logging to clients to demonstrate URLs being used. --- gateway-api/src/gateway_api/pds/client.py | 22 +++++++++++++--- .../src/gateway_api/provider/client.py | 26 ++++++++++++++++--- gateway-api/src/gateway_api/sds/client.py | 24 ++++++++++++++--- 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index b848c9a6..6d978f85 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -18,6 +18,7 @@ malformed upstream data (or malformed test fixtures) and should be corrected at source. """ +import logging import os import uuid @@ -40,6 +41,8 @@ pds = PdsFhirApiStub() get = pds.get # type: ignore +logger = logging.getLogger(__name__) + class PdsClient: """ @@ -77,7 +80,11 @@ def __init__( self.timeout = timeout self.ignore_dates = ignore_dates - # TODO: Add logging to show stub behaviour + log_details = { + "description": "Initialized PdsClient", + "base_url": self.base_url, + } + logger.info(log_details) def _build_headers( self, @@ -118,18 +125,27 @@ def search_patient_by_nhs_number( url = f"{self.base_url}/Patient/{nhs_number}" + log_details = { + "description": "PDS request", + "url": url, + "headers": headers, + } + logger.info(log_details) # This normally calls requests.get, but if PDS_URL is set it uses the stub. - # TODO: Log request to confirm client behaviour response = get( url, headers=headers, params={}, timeout=timeout or self.timeout, ) + log_details = { + "description": "PDS response received", + "status_code": str(response.status_code), + } + logger.info(log_details) try: response.raise_for_status() - # TODO: Log response to confirm stub behaviour except requests.HTTPError as err: raise PdsRequestFailedError(error_reason=err.response.reason) from err diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 5f403465..31d76a92 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -22,6 +22,7 @@ The response from the provider FHIR API. """ +import logging import os from urllib.parse import urljoin @@ -44,6 +45,8 @@ provider_stub = GpProviderStub() post = provider_stub.post # type: ignore +logger = logging.getLogger(__name__) + # Default endpoint path for access record structured interaction (standard GP Connect) ARS_ENDPOINT_PATH = "Patient/$gpc.getstructuredrecord" TIMEOUT: int | None = None # None used for quicker dev, adjust as needed @@ -83,7 +86,14 @@ def __init__( self.token = token self.endpoint_path = endpoint_path - # TODO: Add logging to show stub behaviour + log_details = { + "description": "Initialized GpProviderClient", + "provider_endpoint": self.provider_endpoint, + "provider_asid": self.provider_asid, + "consumer_asid": self.consumer_asid, + "endpoint_path": self.endpoint_path, + } + logger.info(log_details) def _build_headers(self, trace_id: str) -> dict[str, str]: """ @@ -119,17 +129,27 @@ def access_structured_record( base_endpoint = self.provider_endpoint.rstrip("/") + "/" url = urljoin(base_endpoint, self.endpoint_path) - # TODO: Log request to confirm client behaviour + log_details = { + "description": "GPProvider FHIR API request", + "url": url, + "headers": headers, + } + logger.info(log_details) + response = post( url, headers=headers, data=body, timeout=TIMEOUT, ) + log_details = { + "description": "GPProvider FHIR API response received", + "status_code": str(response.status_code), + } + logger.info(log_details) try: response.raise_for_status() - # TODOL: Log response to confirm stub behaviour except HTTPError as err: # TODO: GPCAPIM-353 Consider what error information we want to return here. # Post-steel-thread we probably want to log rather than dumping like this diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index f4da64ae..de5a5432 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -8,6 +8,7 @@ from __future__ import annotations +import logging import os from enum import StrEnum from typing import Any @@ -33,6 +34,8 @@ sds = SdsFhirApiStub() get = sds.get # type: ignore +logger = logging.getLogger(__name__) + class SdsResourceType(StrEnum): """SDS FHIR resource types.""" @@ -87,7 +90,12 @@ def __init__( ) self.api_key = self._get_api_key() - # TODO: Add logging to show stub behaviour + log_details = { + "description": "Initialized SdsClient", + "base_url": self.base_url, + "service_interaction_id": self.service_interaction_id, + } + logger.info(log_details) def _build_headers(self, correlation_id: str | None = None) -> dict[str, str]: """ @@ -192,17 +200,27 @@ def _query_sds( if party_key is not None: params["identifier"].append(f"{FHIRSystem.NHS_MHS_PARTY_KEY}|{party_key}") - # TODO: Log request to confirm stub behaviour + log_details = { + "description": "SDS request", + "url": url, + "headers": headers, + "params": params, + } + logger.info(log_details) response = get( url, headers=headers, params=params, timeout=timeout or self.timeout, ) + log_details = { + "description": "SDS response received", + "status_code": str(response.status_code), + } + logger.info(log_details) try: response.raise_for_status() - # TODO: Log response to confirm stub behaviour except HTTPError as e: raise SdsRequestFailedError(error_reason=str(e)) from e From 629c53c65638b9dfd192d5e741ea53e929729727 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:14:39 +0100 Subject: [PATCH 22/69] Remove unused pipeline env var. --- .github/workflows/alpha-integration-env.yml | 1 - .github/workflows/preview-env.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/alpha-integration-env.yml b/.github/workflows/alpha-integration-env.yml index cd2d0a1e..100e1aa6 100644 --- a/.github/workflows/alpha-integration-env.yml +++ b/.github/workflows/alpha-integration-env.yml @@ -14,7 +14,6 @@ env: TF_STATE_KEY: "dev/preview/alpha-integration.tfstate" BRANCH_NAME: "alpha-integration" ALB_RULE_PRIORITY: "900" - BASE_URL: "https://internal-dev.api.service.nhs.uk/clinical-data-gateway-api-poc-alpha-integration" python_version: "3.14" PROXYGEN_API_NAME: ${{ vars.PROXYGEN_API_NAME }} diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index 11235e91..23d614df 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -10,7 +10,6 @@ env: ECR_REPOSITORY_NAME: "whoami" TF_STATE_BUCKET: "cds-cdg-dev-tfstate-900119715266" PREVIEW_STATE_PREFIX: "dev/preview/" - BASE_URL: "https://internal-dev.api.service.nhs.uk/clinical-data-gateway-api-poc-pr-${{ github.event.pull_request.number }}" python_version: "3.14" PROXYGEN_API_NAME: ${{ vars.PROXYGEN_API_NAME }} PR_NUMBER: ${{ github.event.pull_request.number }} From 71cf237d2548ccbc72b3c912b2d617458f1af8c9 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:21:25 +0100 Subject: [PATCH 23/69] Don't need to do some TODOs --- infrastructure/images/gateway-api/Dockerfile | 2 -- scripts/tests/test.mk | 1 - 2 files changed, 3 deletions(-) diff --git a/infrastructure/images/gateway-api/Dockerfile b/infrastructure/images/gateway-api/Dockerfile index f64e5814..a15f4e50 100644 --- a/infrastructure/images/gateway-api/Dockerfile +++ b/infrastructure/images/gateway-api/Dockerfile @@ -27,10 +27,8 @@ ENV FLASK_HOST="0.0.0.0" ENV FLASK_PORT="8080" ARG COMMIT_VERSION -# TODO: Do we want to do something with this env var? ENV COMMIT_VERSION=$COMMIT_VERSION ARG BUILD_DATE -# TODO: Do we want to do something with this env var? ENV BUILD_DATE=$BUILD_DATE USER gateway_api_user diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index 359dce92..fec7dbbf 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -75,7 +75,6 @@ test-load-initial: # Run your load tests from scripts/test/load @Testing test-load-ui: # Run your load tests from scripts/test/load @Testing with UI enabled @UI=true $(MAKE) _test name="load"' -# TODO: have _test target="proxy"/"local"? _test: set -e script="./scripts/tests/${name}.sh" From 0c29fb20c924662774b8a9eee9589032f0228ff4 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 09:48:39 +0100 Subject: [PATCH 24/69] Resolve some TODOs --- .github/actions/start-app/action.yaml | 3 +-- .github/workflows/preview-env.yml | 1 - scripts/env/env.mk | 2 +- scripts/tests/run-test.sh | 1 - 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/actions/start-app/action.yaml b/.github/actions/start-app/action.yaml index a50a727e..27f8e177 100644 --- a/.github/actions/start-app/action.yaml +++ b/.github/actions/start-app/action.yaml @@ -4,8 +4,7 @@ inputs: deploy-command: description: "Command to start app" required: false - # TODO: rename this make deploy-ci? - default: "make deploy-dev" + default: "make deploy-ci" health-path: description: "Health check path" required: false diff --git a/.github/workflows/preview-env.yml b/.github/workflows/preview-env.yml index 74515b70..f467826c 100644 --- a/.github/workflows/preview-env.yml +++ b/.github/workflows/preview-env.yml @@ -247,7 +247,6 @@ jobs: --services ${{ steps.tf-output.outputs.ecs_service }} \ --region ${{ env.AWS_REGION }} - # TODO: We don't need these anymore, as we are testing against the proxy. - name: Get mTLS certs for testing if: github.event.action != 'closed' id: mtls-certs diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 2206a703..ab5a61a6 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -1,6 +1,6 @@ .PHONY: env env-% _env -env env-dev: # Create .env file with environment variables for development environment (stubs) +env env-dev env-ci: # Create .env file with environment variables for development environment (stubs) make _env env="dev" env-orangebox: # Create .env file that will have the app send requests to the provider "orangebox", stubs otherwise. diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index e77d699d..8668773f 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -45,7 +45,6 @@ else fi # TODO: Add some logging to prove which URLS are being hit -# TODO: Can we remove the cli args for pytest_nhsd_apim - they _should_ be retrievable from the env poetry run pytest ${TEST_PATH} -v \ --api-name="${PROXYGEN_API_NAME}" \ --proxy-name="${PROXYGEN_API_NAME}--internal-dev--${PROXY_BASE_PATH}" \ From 3dd80ffe6177fb8e2371144a3c1bf74b14824b28 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 11:20:28 +0100 Subject: [PATCH 25/69] Handle unit test enviroment variables better. --- README.md | 2 ++ gateway-api/src/gateway_api/conftest.py | 5 ----- scripts/env/env.mk | 1 + scripts/tests/run-test.sh | 6 ++++++ 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0dfb7912..5b327ea4 100644 --- a/README.md +++ b/README.md @@ -147,6 +147,8 @@ The full API schema is defined in [gateway-api/openapi.yaml](gateway-api/openapi ### Environment Variables + + | Variable | Description | | --- | --- | | `BASE_URL` | Protocol, hostname and port for the running API (e.g. `http://localhost:5000`, or `http://gateway-api:8080` from within the devcontainer) | diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index f38f7e88..80145467 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -16,11 +16,6 @@ from gateway_api.clinical_jwt import JWT -# TODO: Do this better. -os.environ["PDS_URL"] = "stub" -os.environ["PROVIDER_URL"] = "not-stub" -os.environ["SDS_URL"] = "stub" - class NewEnvVars: def __init__(self, new_env_vars: Mapping[str, str | None]) -> None: diff --git a/scripts/env/env.mk b/scripts/env/env.mk index ab5a61a6..4f29913b 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -20,6 +20,7 @@ env-test-local: # Create .env.test file that will have tests send requests to th env-test-ci: # Create .env.test file that will have tests send requests to a CI-local app. make _env-test env="ci" overwrite=true + make env-ci env-test-pr-%: # Create .env.test file that will have tests send requests to a proxy deployed for the PR. make _env-test env="pr-$*" overwrite=true diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index 8668773f..3b05647a 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -32,6 +32,12 @@ else fi source .env.test +if [[ "$TEST_TYPE" = "unit" ]]; then + set -a + source .env + set +a +fi + cd gateway-api mkdir -p test-artefacts From c82811f484588aa8a8a3a776ee3d9027c4d0db48 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:02:50 +0100 Subject: [PATCH 26/69] Simplify local vs remote paths. --- .vscode/cspell-dictionary.txt | 1 + gateway-api/tests/conftest.py | 120 ++++-------------- .../tests/schema/test_openapi_schema.py | 11 +- scripts/env/env.mk | 3 +- scripts/env/target.sh | 12 ++ scripts/env/test-env.sh | 2 + scripts/tests/run-test.sh | 2 +- 7 files changed, 42 insertions(+), 109 deletions(-) diff --git a/.vscode/cspell-dictionary.txt b/.vscode/cspell-dictionary.txt index bfd7e5ce..a15065cc 100644 --- a/.vscode/cspell-dictionary.txt +++ b/.vscode/cspell-dictionary.txt @@ -36,6 +36,7 @@ eamodio errstr Farsley fhir +getfixturevalue getstructuredrecord gocloc GPCAPIM diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 470e8814..149b160f 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -3,7 +3,7 @@ import copy import os from datetime import timedelta -from typing import Any, Protocol, cast +from typing import Any, cast import pytest import requests @@ -30,93 +30,18 @@ } -class Client(Protocol): - """Protocol defining the interface for HTTP clients.""" - - base_url: str - - def send_to_get_structured_record_endpoint( - self, payload: str, headers: dict[str, str] | None = None - ) -> requests.Response: - """ - Send a request to the get_structured_record endpoint with the given NHS number. - """ - ... - - def send_health_check(self) -> requests.Response: - """ - Send a health check request to the API. - """ - ... - - def send_post_to_path( - self, - path: str, - payload: str, - headers: dict[str, str] | None = None, - ) -> requests.Response: - """Send a POST request to the given API path.""" - ... - - -class LocalClient: - """HTTP client that sends requests directly to the API. - - Includes optional Apigee Authorization headers when an access token is provided. - """ - - def __init__( - self, - base_url: str, - auth_headers: dict[str, str], - timeout: timedelta = timedelta(seconds=35), - ): - self.base_url = base_url - self._auth_headers = auth_headers - self._timeout = timeout.total_seconds() - - def send_to_get_structured_record_endpoint( - self, payload: str, headers: dict[str, str] | None = None - ) -> requests.Response: - return self.send_post_to_path( - path="/patient/$gpc.getstructuredrecord", - payload=payload, - headers=headers, - ) - - def send_post_to_path( - self, - path: str, - payload: str, - headers: dict[str, str] | None = None, - ) -> requests.Response: - url = f"{self.base_url}/{path.lstrip('/')}" - default_headers = self._auth_headers | DEFAULT_REQUEST_HEADERS - if headers: - default_headers.update(headers) - - return requests.post( - url=url, - data=payload, - headers=default_headers, - timeout=self._timeout, - ) - - def send_health_check(self) -> requests.Response: - url = f"{self.base_url}/health" - return requests.get(url=url, headers=self._auth_headers, timeout=self._timeout) - - -class RemoteClient: - """HTTP client for remote testing via the APIM proxy.""" +class Client: + """Client for sending HTTP requests""" def __init__( self, base_url: str, + health_endpoint: str, auth_headers: dict[str, str], timeout: timedelta = timedelta(seconds=35), ): self.base_url = base_url + self.health_endpoint = health_endpoint self._auth_headers = auth_headers self._timeout = timeout.total_seconds() @@ -149,7 +74,7 @@ def send_post_to_path( ) def send_health_check(self) -> requests.Response: - url = f"{self.base_url}/_status" + url = f"{self.base_url}/{self.health_endpoint}" return requests.get(url=url, headers=self._auth_headers, timeout=self._timeout) @@ -176,18 +101,13 @@ def get_headers(env: str, request: pytest.FixtureRequest) -> dict[str, str]: @pytest.fixture def client( - env: str, - request: pytest.FixtureRequest, + base_url: str, + health_endpoint: str, get_headers: dict[str, str], ) -> Client: - if env == "local": - base_url = request.getfixturevalue("base_url") - return LocalClient(base_url=base_url, auth_headers=get_headers) - elif env == "remote": - proxy_url = request.getfixturevalue("nhsd_apim_proxy_url") - return RemoteClient(base_url=proxy_url, auth_headers=get_headers) - else: - raise ValueError(f"Unknown env: {env}") + return Client( + base_url=base_url, health_endpoint=health_endpoint, auth_headers=get_headers + ) @pytest.fixture(scope="module") @@ -196,21 +116,27 @@ def env() -> str: def get_env() -> str: - try: - _ = _fetch_env_variable("BASE_URL", str) - return "local" - except ValueError: - return "remote" + return _fetch_env_variable("TARGET_ENV", str) @pytest.fixture(scope="module") -def base_url() -> str: +def base_url(request: pytest.FixtureRequest, env: str) -> str: """Retrieves the base URL of the currently deployed application.""" + if env == "remote": + return str(request.getfixturevalue("nhsd_apim_proxy_url")) return _fetch_env_variable("BASE_URL", str) +@pytest.fixture(scope="module") +def health_endpoint(env: str) -> str: + if env == "local": + return "health" + return "_status" + + def _fetch_env_variable[T]( name: str, + # TODO: Can we do this better? t: type[T], # NOQA ARG001 This is actually used for type hinting ) -> T: value = os.getenv(name) diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index 5355b941..1a239158 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -6,7 +6,6 @@ from pathlib import Path -import pytest import schemathesis import yaml from schemathesis.generation.case import Case @@ -20,9 +19,7 @@ @schema.parametrize() -def test_api_schema_compliance( - case: Case, env: str, request: pytest.FixtureRequest -) -> None: +def test_api_schema_compliance(case: Case, base_url: str) -> None: """Test API endpoints against the OpenAPI schema. Schemathesis automatically generates test cases with: @@ -44,12 +41,6 @@ def test_api_schema_compliance( case.headers["Ods-from"] = "test-ods-code" case.headers["Ssp-TraceID"] = "test-trace-id" - # TODO: Do this better - if env == "local": - base_url = request.getfixturevalue("base_url") - else: - base_url = request.getfixturevalue("nhsd_apim_proxy_url") - case.call_and_validate( base_url=base_url, excluded_checks=[schemathesis.checks.not_a_server_error], diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 4f29913b..9158ba8d 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -17,10 +17,11 @@ env-int: # Create .env file with environment variables for integration environme env-test-local: # Create .env.test file that will have tests send requests to the local app. make _env-test env="local" + make env-dev # Ensure unit tests run with stub environment variables env-test-ci: # Create .env.test file that will have tests send requests to a CI-local app. make _env-test env="ci" overwrite=true - make env-ci + make env-ci # Ensure unit tests run with stub environment variables env-test-pr-%: # Create .env.test file that will have tests send requests to a proxy deployed for the PR. make _env-test env="pr-$*" overwrite=true diff --git a/scripts/env/target.sh b/scripts/env/target.sh index 23f94fdc..b352d900 100644 --- a/scripts/env/target.sh +++ b/scripts/env/target.sh @@ -44,3 +44,15 @@ prompt_proxy_base_path() { esac } +prompt_target_env() { + env="$1" + case "$env" in + *) + echo "local" + ;; + pr-*|alpha-int) + echo "remote" + ;; + esac +} + diff --git a/scripts/env/test-env.sh b/scripts/env/test-env.sh index 2ab2e2aa..a31e6114 100755 --- a/scripts/env/test-env.sh +++ b/scripts/env/test-env.sh @@ -17,6 +17,7 @@ else PROXY_NAME="${PROXYGEN_API_NAME}--internal-dev--${PROXY_BASE_PATH}" fi APIGEE_ACCESS_TOKEN=$(prompt_apigee_access_token "$env") +TARGET_ENV=$(prompt_target_env "$env") ENV_FILE=".env.test" overwrite="$2" @@ -32,6 +33,7 @@ PROXYGEN_API_NAME=${PROXYGEN_API_NAME} PROXY_NAME=${PROXY_NAME} PROXY_BASE_PATH=$PROXY_BASE_PATH BASE_URL=$BASE_URL +TARGET_ENV=$TARGET_ENV set +a EOF diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index 3b05647a..1c274ee7 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -41,7 +41,7 @@ fi cd gateway-api mkdir -p test-artefacts -echo "Running ${TEST_TYPE} tests..." +echo "Running ${TEST_TYPE} tests against ${TARGET_ENV} environment..." # Set coverage path based on test type if [[ "$TEST_TYPE" = "unit" ]]; then From 247881f6704a719d41e86d0ec266685b0c53bf15 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 13:14:05 +0100 Subject: [PATCH 27/69] Always overwrite. --- scripts/env/app-env.sh | 3 --- scripts/env/env.mk | 8 ++++---- scripts/env/helper.sh | 18 ------------------ scripts/env/test-env.sh | 5 ----- 4 files changed, 4 insertions(+), 30 deletions(-) delete mode 100644 scripts/env/helper.sh diff --git a/scripts/env/app-env.sh b/scripts/env/app-env.sh index 9d63481e..a2f44044 100755 --- a/scripts/env/app-env.sh +++ b/scripts/env/app-env.sh @@ -4,12 +4,9 @@ set -e source scripts/env/pds.sh source scripts/env/sds.sh source scripts/env/provider.sh -source scripts/env/helper.sh ENV_FILE=".env" -confirm_overwrite "$ENV_FILE" - PDS_URL=$(prompt_pds_url "$env") SDS_URL=$(prompt_sds_url "$env") PROVIDER_URL=$(prompt_provider_url "$env") diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 9158ba8d..6a81bc7d 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -20,20 +20,20 @@ env-test-local: # Create .env.test file that will have tests send requests to th make env-dev # Ensure unit tests run with stub environment variables env-test-ci: # Create .env.test file that will have tests send requests to a CI-local app. - make _env-test env="ci" overwrite=true + make _env-test env="ci" make env-ci # Ensure unit tests run with stub environment variables env-test-pr-%: # Create .env.test file that will have tests send requests to a proxy deployed for the PR. - make _env-test env="pr-$*" overwrite=true + make _env-test env="pr-$*" env-test-alpha-int: # Create .env.test file that will have tests send requests to the alpha integration environment. - make _env-test env="alpha-int" overwrite=true + make _env-test env="alpha-int" _env: scripts/env/app-env.sh "$(env)" _env-test: - scripts/env/test-env.sh "$(env)" "$(overwrite)" + scripts/env/test-env.sh "$(env)" ${VERBOSE}.SILENT: \ _env \ diff --git a/scripts/env/helper.sh b/scripts/env/helper.sh deleted file mode 100644 index 7627b375..00000000 --- a/scripts/env/helper.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/bash - -confirm_overwrite() { - local file="$1" - if [ -f "$file" ]; then - printf "'%s' already exists. Overwrite? [y/N]: " "$file" - read -r answer - case "$answer" in - [yY]|[yY][eE][sS]) - echo "Overwriting $file..." - ;; - *) - echo "Aborted." - exit 0 - ;; - esac - fi -} diff --git a/scripts/env/test-env.sh b/scripts/env/test-env.sh index a31e6114..663ff9dd 100755 --- a/scripts/env/test-env.sh +++ b/scripts/env/test-env.sh @@ -4,7 +4,6 @@ set -e source scripts/env/target.sh -source scripts/env/helper.sh source scripts/env/apigee.sh env="$1" @@ -20,10 +19,6 @@ APIGEE_ACCESS_TOKEN=$(prompt_apigee_access_token "$env") TARGET_ENV=$(prompt_target_env "$env") ENV_FILE=".env.test" -overwrite="$2" -if [[ "$overwrite" != "true" ]]; then - confirm_overwrite "$ENV_FILE" -fi cat > "$ENV_FILE" < Date: Wed, 22 Apr 2026 13:32:53 +0100 Subject: [PATCH 28/69] Update docs. Improve env-ci output. --- README.md | 31 +++++++++++++++++++++++++------ scripts/env/env.mk | 6 +++++- scripts/env/test-env.sh | 1 - 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 5b327ea4..f220eae5 100644 --- a/README.md +++ b/README.md @@ -147,20 +147,39 @@ The full API schema is defined in [gateway-api/openapi.yaml](gateway-api/openapi ### Environment Variables - +Make commands help build the `.env` and `.env.test` files used to populate the required environment variables. + +#### .env + +`make deploy` will feed the `.env` variables in to the app's environment. + +Environment variables control whether stubs are used in place of the real PDS, SDS, and Provider services during local development. | Variable | Description | | --- | --- | -| `BASE_URL` | Protocol, hostname and port for the running API (e.g. `http://localhost:5000`, or `http://gateway-api:8080` from within the devcontainer) | -| `HOST` | hostname portion of `BASE_URL` | -| `FLASK_HOST` | Host the Flask app binds to | -| `FLASK_PORT` | Port the Flask app listens on | | `PDS_URL` | The URL for the PDS FHIR API; set as `stub` to use development stub. | +| `PDS_API_TOKEN`| Leave unset in development environment. | +| `PDS_API_SECRET`| Leave unset in development environment. | +| `PDS_API_KID`| Leave unset in development environment. | | `SDS_URL` | The URL for the SDS FHIR API; set as `stub` to use development stub. | +| `SDS_API_TOKEN`| Leave unset in development environment. | | `PROVIDER_URL` | The URL for the GP Provider; set as `stub` to use development stub. | | `CDG_DEBUG` | `true`, return additional debug information when the call to the GP provider returns an error. | -Environment variables also control whether stubs are used in place of the real PDS, SDS, and Provider services during local development. +See `make env-*` in `scripts/env/app/env.mk` for the commands that will write these variables to a file. + +_Note: `FLASK_HOST` and `FLASK_PORT` are hardcoded in to the Dockerfile. These are for container, and do not need adjusting._ + +#### .env.test + +| Variable | Description | +| --- | --- | +| `BASE_URL` | Protocol, hostname and port for the running API (e.g. `http://localhost:5000`, or `http://gateway-api:8080` from within the devcontainer) | +| `APIGEE_ACCESS_TOKEN` | An access token to Apigee API used by `pytest_nhds_apim`, fed from the environment variables at run time. | +| `PROXYGEN_API_NAME` | The name of the API defined in Proxygen. Used by `pytest_nhsd_apim` to run tests against, fed in the CLI arguments in `make test-*` | +| `PROXY_BASE_PATH` | The suffix of the proxy instance being deployed. Used by `pytest_nhsd_apim` to run tests against, fed in the CLI arguments in `make test-*` | +| `BASE_URL` | Set if targeting a locally deployed application; otherwise, leave unset. | +| `TARGET_ENV` | Either `local` or `remote`, to inform the HTTP client used by the tests how to behave - e.g. add auth headers, etc. | ## Testing diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 6a81bc7d..2359ef7a 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -1,8 +1,11 @@ .PHONY: env env-% _env -env env-dev env-ci: # Create .env file with environment variables for development environment (stubs) +env env-dev: # Create .env file with environment variables for development environment (stubs) make _env env="dev" +env-ci: # Create .env file with environment variables for CI environment (stubs) + make _env env="ci" + env-orangebox: # Create .env file that will have the app send requests to the provider "orangebox", stubs otherwise. make _env env="orangebox" @@ -39,6 +42,7 @@ ${VERBOSE}.SILENT: \ _env \ env \ env-dev \ + env-ci \ env-orangebox \ env-sandbox-pds \ env-sandbox-sds \ diff --git a/scripts/env/test-env.sh b/scripts/env/test-env.sh index 663ff9dd..5fe7d1b2 100755 --- a/scripts/env/test-env.sh +++ b/scripts/env/test-env.sh @@ -25,7 +25,6 @@ cat > "$ENV_FILE" < Date: Wed, 22 Apr 2026 13:39:04 +0100 Subject: [PATCH 29/69] Tidy up. --- scripts/env/{app-env.sh => app/env.sh} | 12 ++++++------ scripts/env/{ => app}/pds.sh | 2 +- scripts/env/{ => app}/provider.sh | 2 +- scripts/env/{ => app}/sds.sh | 2 +- scripts/env/env.mk | 5 +++-- scripts/env/{ => test}/apigee.sh | 2 +- scripts/env/{test-env.sh => test/env.sh} | 14 +++++++------- scripts/env/{ => test}/target.sh | 8 ++++---- 8 files changed, 24 insertions(+), 23 deletions(-) rename scripts/env/{app-env.sh => app/env.sh} (59%) rename scripts/env/{ => app}/pds.sh (93%) rename scripts/env/{ => app}/provider.sh (92%) rename scripts/env/{ => app}/sds.sh (93%) rename scripts/env/{ => test}/apigee.sh (83%) rename scripts/env/{test-env.sh => test/env.sh} (69%) rename scripts/env/{ => test}/target.sh (88%) diff --git a/scripts/env/app-env.sh b/scripts/env/app/env.sh similarity index 59% rename from scripts/env/app-env.sh rename to scripts/env/app/env.sh index a2f44044..b867b763 100755 --- a/scripts/env/app-env.sh +++ b/scripts/env/app/env.sh @@ -1,15 +1,15 @@ #!/bin/bash set -e -source scripts/env/pds.sh -source scripts/env/sds.sh -source scripts/env/provider.sh +source scripts/env/app/pds.sh +source scripts/env/app/sds.sh +source scripts/env/app/provider.sh ENV_FILE=".env" -PDS_URL=$(prompt_pds_url "$env") -SDS_URL=$(prompt_sds_url "$env") -PROVIDER_URL=$(prompt_provider_url "$env") +PDS_URL=$(get_pds_url "$env") +SDS_URL=$(get_sds_url "$env") +PROVIDER_URL=$(get_provider_url "$env") cat > "$ENV_FILE" < Date: Wed, 22 Apr 2026 13:52:16 +0100 Subject: [PATCH 30/69] Place the test user in to the env variables. --- README.md | 1 + gateway-api/tests/conftest.py | 19 ++++++------------- scripts/env/test/env.sh | 6 ++++++ scripts/env/test/target.sh | 6 +++--- scripts/env/test/user.sh | 11 +++++++++++ 5 files changed, 27 insertions(+), 16 deletions(-) create mode 100644 scripts/env/test/user.sh diff --git a/README.md b/README.md index f220eae5..c03a3d06 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,7 @@ _Note: `FLASK_HOST` and `FLASK_PORT` are hardcoded in to the Dockerfile. These a | `PROXY_BASE_PATH` | The suffix of the proxy instance being deployed. Used by `pytest_nhsd_apim` to run tests against, fed in the CLI arguments in `make test-*` | | `BASE_URL` | Set if targeting a locally deployed application; otherwise, leave unset. | | `TARGET_ENV` | Either `local` or `remote`, to inform the HTTP client used by the tests how to behave - e.g. add auth headers, etc. | +| `REMOTE_TEST_USERNAME` | The test user through which the tests will be authenticated against when run against a remote target. | ## Testing diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 149b160f..0b19d228 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -2,6 +2,7 @@ import copy import os +from collections.abc import Callable from datetime import timedelta from typing import Any, cast @@ -119,7 +120,7 @@ def get_env() -> str: return _fetch_env_variable("TARGET_ENV", str) -@pytest.fixture(scope="module") +@pytest.fixture def base_url(request: pytest.FixtureRequest, env: str) -> str: """Retrieves the base URL of the currently deployed application.""" if env == "remote": @@ -134,24 +135,16 @@ def health_endpoint(env: str) -> str: return "_status" -def _fetch_env_variable[T]( - name: str, - # TODO: Can we do this better? - t: type[T], # NOQA ARG001 This is actually used for type hinting -) -> T: +def _fetch_env_variable[T](name: str, parser: Callable[[str], T]) -> T: value = os.getenv(name) - if not value: + if value is None: raise ValueError(f"{name} environment variable is not set.") - return cast("T", value) - - -REMOTE_TEST_USERNAME_ENV_VAR = "REMOTE_TEST_USERNAME" -DEFAULT_REMOTE_TEST_USERNAME = "656005750101" + return parser(value) def _get_remote_test_username() -> str: """Return the username to use for remote tests, allowing override via env.""" - return os.getenv(REMOTE_TEST_USERNAME_ENV_VAR, DEFAULT_REMOTE_TEST_USERNAME) + return _fetch_env_variable("REMOTE_TEST_USERNAME", str) def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: diff --git a/scripts/env/test/env.sh b/scripts/env/test/env.sh index eb2582c9..ff3cc857 100755 --- a/scripts/env/test/env.sh +++ b/scripts/env/test/env.sh @@ -5,6 +5,7 @@ set -e source scripts/env/test/target.sh source scripts/env/test/apigee.sh +source scripts/env/test/user.sh env="$1" BASE_URL=$(get_base_url "$env") @@ -17,6 +18,7 @@ else fi APIGEE_ACCESS_TOKEN=$(get_apigee_access_token "$env") TARGET_ENV=$(get_target_env "$env") +REMOTE_TEST_USERNAME=$(get_test_user "$env") ENV_FILE=".env.test" @@ -26,8 +28,12 @@ set -a APIGEE_ACCESS_TOKEN=${APIGEE_ACCESS_TOKEN} PROXYGEN_API_NAME=${PROXYGEN_API_NAME} PROXY_BASE_PATH=$PROXY_BASE_PATH + BASE_URL=$BASE_URL + TARGET_ENV=$TARGET_ENV + +REMOTE_TEST_USERNAME=$REMOTE_TEST_USERNAME set +a EOF diff --git a/scripts/env/test/target.sh b/scripts/env/test/target.sh index b5e8feb6..e17c804e 100644 --- a/scripts/env/test/target.sh +++ b/scripts/env/test/target.sh @@ -47,12 +47,12 @@ get_proxy_base_path() { get_target_env() { env="$1" case "$env" in - *) - echo "local" - ;; pr-*|alpha-int) echo "remote" ;; + *) + echo "local" + ;; esac } diff --git a/scripts/env/test/user.sh b/scripts/env/test/user.sh new file mode 100644 index 00000000..6dc5296c --- /dev/null +++ b/scripts/env/test/user.sh @@ -0,0 +1,11 @@ +#!/bin/bash +set -e + +get_test_user() { + env="$1" + case "$env" in + pr-*|alpha-int) + echo "656005750101" + ;; + esac +} From 928d6294e463ce2b0a2d0ddbf1b90306287fbb5d Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:40:54 +0100 Subject: [PATCH 31/69] Tidy up. --- gateway-api/tests/conftest.py | 11 ++++++----- gateway-api/tests/schema/test_openapi_schema.py | 2 ++ scripts/tests/run-test.sh | 3 +++ 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 0b19d228..70828676 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -121,18 +121,19 @@ def get_env() -> str: @pytest.fixture -def base_url(request: pytest.FixtureRequest, env: str) -> str: +def base_url(env: str, request: pytest.FixtureRequest) -> str: """Retrieves the base URL of the currently deployed application.""" if env == "remote": return str(request.getfixturevalue("nhsd_apim_proxy_url")) return _fetch_env_variable("BASE_URL", str) -@pytest.fixture(scope="module") +@pytest.fixture def health_endpoint(env: str) -> str: if env == "local": return "health" - return "_status" + else: + return "_status" def _fetch_env_variable[T](name: str, parser: Callable[[str], T]) -> T: @@ -148,8 +149,8 @@ def _get_remote_test_username() -> str: def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: - is_remote = get_env() == "remote" - if is_remote: + target_is_remote = get_env() == "remote" + if target_is_remote: for item in items: item.add_marker( pytest.mark.nhsd_apim_authorization( diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index 1a239158..5619d805 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -8,6 +8,7 @@ import schemathesis import yaml +from hypothesis import HealthCheck, settings from schemathesis.generation.case import Case from schemathesis.openapi import from_dict @@ -19,6 +20,7 @@ @schema.parametrize() +@settings(suppress_health_check=[HealthCheck.function_scoped_fixture]) def test_api_schema_compliance(case: Case, base_url: str) -> None: """Test API endpoints against the OpenAPI schema. diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index 1c274ee7..0918ed9a 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -42,6 +42,9 @@ cd gateway-api mkdir -p test-artefacts echo "Running ${TEST_TYPE} tests against ${TARGET_ENV} environment..." +if [[ "$PROXY_BASE_PATH" != "none" ]]; then + echo "Using proxy base path: ${PROXY_BASE_PATH}" +fi # Set coverage path based on test type if [[ "$TEST_TYPE" = "unit" ]]; then From 04092fd6fa7d71447c411a49bafbe8487c36d3e7 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:52:16 +0100 Subject: [PATCH 32/69] Make linter happy. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 713ae66b..b907e191 100644 --- a/README.md +++ b/README.md @@ -226,10 +226,10 @@ To be able to use the `load-tests` or `local-tests`, you will need to have Proxy - Please follow the [guide here](https://nhsd-confluence.digital.nhs.uk/spaces/DCA/pages/1236046532/Proxygen) - When the devcontainer has built, the Proxygen configuration is created on the host at `~/gateway/ptl/.proxygen` and bind-mounted into the container as `~/.proxygen`. -- For *settings*: +- For _settings_: - Set the `api` value - `endpoint_url` and `spec_output_format` should be already set. -- For *credentials*: +- For _credentials_: - Set the `base_url`, `client_secret`, `password` and `username` values. - Remove unused fields. From 0b7e5832725d00e56b5d0432f4a1cfbe1613abc1 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 14:56:12 +0100 Subject: [PATCH 33/69] More tightly define type. --- gateway-api/src/gateway_api/app.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 49b4c412..386e4973 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -1,7 +1,6 @@ import os import traceback from collections.abc import Callable -from typing import Any from flask import Flask, Request, request from flask.wrappers import Response @@ -34,12 +33,12 @@ def configure_app(app: Flask) -> None: app.config.update(config) -def get_env_var(name: str, loader: Callable[[str], Any]) -> Any: +def get_env_var[T](name: str, parser: Callable[[str], T]) -> T: value = os.getenv(name) if value is None: raise RuntimeError(f"{name} environment variable is not set.") try: - return loader(value) + return parser(value) except Exception as e: raise RuntimeError(f"Error loading {name} environment variable: {e}") from e From bda2bb64b2ac750048905411f2354af0dc01f9c8 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:00:11 +0100 Subject: [PATCH 34/69] Doesn't seem to 'omit' --- gateway-api/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway-api/pyproject.toml b/gateway-api/pyproject.toml index ce7fee4e..779738b9 100644 --- a/gateway-api/pyproject.toml +++ b/gateway-api/pyproject.toml @@ -23,7 +23,7 @@ packages = [{include = "gateway_api", from = "src"}, [tool.coverage.run] relative_files = true -omit = ["*/tests/*", "*/features/*", "*/test_*.py", "*/conftest.py"] +omit = ["*/tests/*", "*/features/*", "*/test_*.py"] [tool.coverage.paths] source = [ From 55a234b6c1fa9aafc5e5bbbfe9913da0c3a1b71e Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:01:34 +0100 Subject: [PATCH 35/69] Resolved to do. --- scripts/tests/run-test.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index 0918ed9a..c06f03f2 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -53,7 +53,6 @@ else COV_PATH="src/gateway_api" fi -# TODO: Add some logging to prove which URLS are being hit poetry run pytest ${TEST_PATH} -v \ --api-name="${PROXYGEN_API_NAME}" \ --proxy-name="${PROXYGEN_API_NAME}--internal-dev--${PROXY_BASE_PATH}" \ From bab15e1fd854eab478e6a9b094cea11a4f4609cc Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:04:13 +0100 Subject: [PATCH 36/69] Place make env file eariler in commands to run. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b907e191..c1f436ef 100644 --- a/README.md +++ b/README.md @@ -130,10 +130,10 @@ The project uses `make` targets to build, deploy, and manage the application. Ru | --- | --- | | `make dependencies` | Install all project dependencies via Poetry | | `make build` | Type-check, package, and build the Docker image | +| `make env` | Create a `.env` to be consumed when starting the app, e.g. `make deploy` | | `make deploy` | Build and start the Gateway API container using the environment variables defined in `.env` | | `make clean` | Stop and remove the Gateway API container | | `make config` | Configure the development environment | -| `make env` | Create a `.env` to be consumed when starting the app, e.g. `make deploy` | ### API Endpoints From cff2eff248f3a9722b5ed28eab19390411d23dd5 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 15:33:46 +0100 Subject: [PATCH 37/69] Update creation of .env.test. steps. --- gateway-api/tests/README.md | 143 +++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 68 deletions(-) diff --git a/gateway-api/tests/README.md b/gateway-api/tests/README.md index 65fa4414..9cc116b0 100644 --- a/gateway-api/tests/README.md +++ b/gateway-api/tests/README.md @@ -23,12 +23,6 @@ tests/ ``` ## Running Tests -> -> [!NOTE]
-> When running tests the following environment variables need to be provided: -> -> - `BASE_URL` - defines the protocol, hostname and port that should used to access the running APIs. Should be included as a URL in the format ::, for example "" if the APIs are available on the "localhost" host via HTTP using port 5000. If running locally in a dev container, using gateway-api as the host will allow the tests to communicate with an instance launched from `make deploy-dev`. -> - `HOST` - defines the hostname that should be used to access the running APIs. This should match the host portion of the URL provided in the `BASE_URL` environment variable above. ### Install Dependencies (if not using Dev container) @@ -39,6 +33,19 @@ cd gateway-api poetry sync ``` +### Setup `.env.test` file + +When `make test-*` is called, the script will place `source` the variables in `.env.test` before running the tests. + +Use the following commands to create the appropriate `.env.test` file for the target environment. + +* `make env-test-local` to write a `.env.test` file for testing a locally deployed application, from within the dev container. +* `make env-test-ci` to write a `.env.test` file for testing a locally deployed application, from outside the dev container. +* `make env-test-pr-` to write a `.env.test` file for testing an application deployed behind a PR proxy. +* `make env-test-alpha-int` to write a `.env.test` file for testing an application deployed to the "alpha integration" environment. + +_Note: Unit tests require the `.env` file, as these tests do not test a deployed application`_ + ### Run All Tests (with Verbose Output) From the root of the repository: @@ -69,14 +76,14 @@ Run `make deploy` before running any tests that hit the real API. This command requires a `.env` file to set the app's behaviour - see `env.mk` on how to make `.env` files. -- **Requires deployed/running app (`make deploy`):** - - Acceptance tests (`tests/acceptance/`) - - Integration tests (`tests/integration/`) - - Schema validation tests (`tests/schema/`) - - Provider contract tests (`tests/contract/test_provider_contract.py`) -- **Does not require deployed/running app:** - - Unit tests - - Consumer contract tests (`tests/contract/test_consumer_contract.py`) - these run against a Pact mock server +* **Requires deployed/running app (`make deploy`):** + * Acceptance tests (`tests/acceptance/`) + * Integration tests (`tests/integration/`) + * Schema validation tests (`tests/schema/`) + * Provider contract tests (`tests/contract/test_provider_contract.py`) +* **Does not require deployed/running app:** + * Unit tests + * Consumer contract tests (`tests/contract/test_consumer_contract.py`) - these run against a Pact mock server ## Test Types @@ -86,9 +93,9 @@ behaviour-driven development (BDD) tests using pytest-bdd and Gherkin syntax. Th **Structure:** -- **Feature files** (`features/*.feature`): Written in Gherkin, these define scenarios in plain language -- **Step definitions** (`steps/*.py`): Python implementations that map Gherkin steps to actual test code -- **Test bindings** (`scenarios/test_*.py`): Link scenarios to pytest test functions using `@scenario` decorator +* **Feature files** (`features/*.feature`): Written in Gherkin, these define scenarios in plain language +* **Step definitions** (`steps/*.py`): Python implementations that map Gherkin steps to actual test code +* **Test bindings** (`scenarios/test_*.py`): Link scenarios to pytest test functions using `@scenario` decorator **How it works:** @@ -116,23 +123,23 @@ Integration tests that validate the APIs’ behaviour through HTTP requests. The **How it works:** -- Tests use the `Client` class from `conftest.py` to interact with the API -- The client sends HTTP POST requests to the APIs -- Tests verify response status codes, headers, and response bodies -- Tests validate both successful requests and error handling +* Tests use the `Client` class from `conftest.py` to interact with the API +* The client sends HTTP POST requests to the APIs +* Tests verify response status codes, headers, and response bodies +* Tests validate both successful requests and error handling **Example test cases:** -- Successful "hello world" responses -- Error handling for missing or empty payloads -- Error handling for non-existent resources -- Content-Type header validation +* Successful "hello world" responses +* Error handling for missing or empty payloads +* Error handling for non-existent resources +* Content-Type header validation **Key difference from acceptance tests:** -- Integration tests use direct pytest assertions without Gherkin syntax -- More focused on testing specific API behaviours and edge cases -- Uses the same `Client` fixture as acceptance tests but with standard pytest structure +* Integration tests use direct pytest assertions without Gherkin syntax +* More focused on testing specific API behaviours and edge cases +* Uses the same `Client` fixture as acceptance tests but with standard pytest structure ### Schema Validation Tests (`schema/`) @@ -140,14 +147,14 @@ Property-based API schema validation tests using Schemathesis. These tests autom **How it works:** -- Loads the OpenAPI schema from `openapi.yaml` -- Uses the `base_url` fixture to test against the running API -- Automatically generates test cases including: - - Valid inputs - - Edge cases - - Boundary values - - Invalid inputs -- Validates that responses match the schema definitions +* Loads the OpenAPI schema from `openapi.yaml` +* Uses the `base_url` fixture to test against the running API +* Automatically generates test cases including: + * Valid inputs + * Edge cases + * Boundary values + * Invalid inputs +* Validates that responses match the schema definitions ### Contract Testing with Pact (`contract/`) @@ -158,17 +165,17 @@ There are two types of tests in this folder. The stub tests (contained in the st **How it works (Pact-based tests):** 1. **Consumer Tests** (`test_consumer_contract.py`): - - Define what the consumer EXPECTS from the API - - Test against a **mock Pact server** (not the real API) - - The mock server responds based on the defined expectations - - Generates a pact contract file (`GatewayAPIConsumer-GatewayAPIProvider.json`) with all interactions - - **Key point:** These tests don't call the real API + * Define what the consumer EXPECTS from the API + * Test against a **mock Pact server** (not the real API) + * The mock server responds based on the defined expectations + * Generates a pact contract file (`GatewayAPIConsumer-GatewayAPIProvider.json`) with all interactions + * **Key point:** These tests don't call the real API 2. **Provider Integration Tests** (`test_provider_contract.py`): - - Verify the **actual deployed API** implementation - - Read the pact contract file generated by consumer tests - - Verify that the real API implementation satisfies the consumer's expectations - - **Key point:** This is where the real API gets tested + * Verify the **actual deployed API** implementation + * Read the pact contract file generated by consumer tests + * Verify that the real API implementation satisfies the consumer's expectations + * **Key point:** This is where the real API gets tested **The Flow:** @@ -180,11 +187,11 @@ Consumer Test → Mock Pact Server → Contract File (JSON) **Why this approach as opposed to a standard integration test?** -- **Explicit contract documentation** - The pact file is a versioned artefact that documents the API contract -- **Contract evolution tracking** - Because of the above - Git diffs will show exactly how API contracts change over time +* **Explicit contract documentation** - The pact file is a versioned artefact that documents the API contract +* **Contract evolution tracking** - Because of the above - Git diffs will show exactly how API contracts change over time -- **Consumer-driven development** - Consumers define their needs; providers verify they meet them -- **Prevents breaking changes** - Provider tests fail if changes break existing consumer expectations +* **Consumer-driven development** - Consumers define their needs; providers verify they meet them +* **Prevents breaking changes** - Provider tests fail if changes break existing consumer expectations ### Contract Testing Workflow @@ -196,18 +203,18 @@ Consumer tests generate the pact contract files in `tests/contract/pacts/` (e.g. **Key points:** -- The pact contract file represents the contract between the consumer and provider -- This file is committed to version control so you can track contract changes through git diffs -- The `pact.write_file()` call merges interactions (updates existing or adds new ones) -- Interactions with the same description get replaced; different descriptions get added +* The pact contract file represents the contract between the consumer and provider +* This file is committed to version control so you can track contract changes through git diffs +* The `pact.write_file()` call merges interactions (updates existing or adds new ones) +* Interactions with the same description get replaced; different descriptions get added ## Shared Fixtures Shared fixtures in `tests/conftest.py` are available across all test types: -- **`base_url`**: The base URL of the deployed Lambda function (from `BASE_URL` environment variable highlighted above) -- **`host`**: The hostname of the deployed application (from `HOST` environment variable highlighted above) -- **`client`**: An HTTP client instance for sending requests to the APIs +* **`base_url`**: The base URL of the deployed Lambda function (from `BASE_URL` environment variable highlighted above) +* **`host`**: The hostname of the deployed application (from `HOST` environment variable highlighted above) +* **`client`**: An HTTP client instance for sending requests to the APIs ### Load Testing with Locust ('load/') @@ -255,8 +262,8 @@ JUnit XML format is used for CI/CD integration and test result summaries. All re **All Test Types (Unit, Contract, Schema, Integration, Acceptance):** -- Generated with `--junit-xml=test-artefacts/{type}-tests.xml` -- Contains test results, execution times, and failure details +* Generated with `--junit-xml=test-artefacts/{type}-tests.xml` +* Contains test results, execution times, and failure details ### HTML Test Reports @@ -264,12 +271,12 @@ Human-readable HTML reports for detailed test analysis: **Tests using pytest (Unit, Contract, Schema, Integration, Acceptance):** -- Generated with `--html=test-artefacts/{type}-tests.html --self-contained-html` -- Self-contained HTML files with embedded CSS/JavaScript -- Include: - - Test results with pass/fail status - - Execution times - - Test metadata +* Generated with `--html=test-artefacts/{type}-tests.html --self-contained-html` +* Self-contained HTML files with embedded CSS/JavaScript +* Include: + * Test results with pass/fail status + * Execution times + * Test metadata ### CI/CD Report Artefacts @@ -291,10 +298,10 @@ Each test execution script (`scripts/tests/*.sh`) collects coverage data indepen **Unit, Contract, Schema, Integration, and Acceptance Tests** (pytest-based): -- Use `pytest-cov` plugin with `--cov=src/gateway_api` flag -- Generate individual coverage data files: `.coverage` -- Each test type saves its coverage file as `coverage.{type}` (e.g., `coverage.unit`, `coverage.contract`, `coverage.schema`) -- Produce HTML reports for local viewing and terminal output for CI logs +* Use `pytest-cov` plugin with `--cov=src/gateway_api` flag +* Generate individual coverage data files: `.coverage` +* Each test type saves its coverage file as `coverage.{type}` (e.g., `coverage.unit`, `coverage.contract`, `coverage.schema`) +* Produce HTML reports for local viewing and terminal output for CI logs ### CI/CD Coverage Workflow From 5fc19a2dc49219a6b58b35888003649bdfe02c07 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:10:17 +0100 Subject: [PATCH 38/69] Add an orangebox environment that would reflect env-organebox behaviour. --- .../Steel_Thread/Access_Structured_Record.yml | 10 ++++++---- .../collections/Steel_Thread/environments/PR_Proxy.yml | 8 +++++--- .../collections/Steel_Thread/environments/local.yml | 6 +++++- .../Steel_Thread/environments/orangebox.yml | 8 ++++++++ 4 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 bruno/gateway-api/collections/Steel_Thread/environments/orangebox.yml diff --git a/bruno/gateway-api/collections/Steel_Thread/Access_Structured_Record.yml b/bruno/gateway-api/collections/Steel_Thread/Access_Structured_Record.yml index 4b78bd26..265eaf36 100644 --- a/bruno/gateway-api/collections/Steel_Thread/Access_Structured_Record.yml +++ b/bruno/gateway-api/collections/Steel_Thread/Access_Structured_Record.yml @@ -1,15 +1,14 @@ info: - name: "Access Record Structured" + name: Access Record Structured type: http seq: 1 http: method: POST url: "{{base_url}}/patient/$gpc.getstructuredrecord" - auth: inherit headers: - name: ODS-from - value: A12345 + value: "{{ods_code}}" body: type: json data: |- @@ -20,11 +19,12 @@ http: "name": "patientNHSNumber", "valueIdentifier": { "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "9999999999" + "value": "{{nhs_number}}" } } ] } + auth: inherit runtime: scripts: @@ -37,3 +37,5 @@ runtime: settings: encodeUrl: true timeout: 0 + followRedirects: true + maxRedirects: 5 diff --git a/bruno/gateway-api/collections/Steel_Thread/environments/PR_Proxy.yml b/bruno/gateway-api/collections/Steel_Thread/environments/PR_Proxy.yml index be3fe52c..ae44dff4 100644 --- a/bruno/gateway-api/collections/Steel_Thread/environments/PR_Proxy.yml +++ b/bruno/gateway-api/collections/Steel_Thread/environments/PR_Proxy.yml @@ -1,10 +1,12 @@ +name: PR_Proxy variables: - name: base_url value: https://{{apigee_env}}.api.service.nhs.uk/{{proxy_base_path}} - enabled: true - name: apigee_env value: internal-dev - enabled: true - name: proxy_base_path value: clinical-data-gateway-api-poc-pr-{{process.env.PR_NUMBER}} - enabled: true + - name: nhs_number + value: "9999999999" + - name: ods_code + value: A12345 diff --git a/bruno/gateway-api/collections/Steel_Thread/environments/local.yml b/bruno/gateway-api/collections/Steel_Thread/environments/local.yml index a7b23db8..d5620fc4 100644 --- a/bruno/gateway-api/collections/Steel_Thread/environments/local.yml +++ b/bruno/gateway-api/collections/Steel_Thread/environments/local.yml @@ -1,4 +1,8 @@ +name: local variables: - name: base_url value: http://localhost:5000 - enabled: true + - name: nhs_number + value: "9999999999" + - name: ods_code + value: A12345 diff --git a/bruno/gateway-api/collections/Steel_Thread/environments/orangebox.yml b/bruno/gateway-api/collections/Steel_Thread/environments/orangebox.yml new file mode 100644 index 00000000..807c3a47 --- /dev/null +++ b/bruno/gateway-api/collections/Steel_Thread/environments/orangebox.yml @@ -0,0 +1,8 @@ +name: orangebox +variables: + - name: nhs_number + value: "9690937278" + - name: base_url + value: http://localhost:5000 + - name: ods_code + value: S55555 From 4082b479f3a0569d8bf6fbc325afcc54749e4e5b Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:11:00 +0100 Subject: [PATCH 39/69] Correctly print out logs. --- gateway-api/src/gateway_api/pds/client.py | 3 +++ gateway-api/src/gateway_api/provider/client.py | 3 +++ gateway-api/src/gateway_api/sds/client.py | 3 +++ 3 files changed, 9 insertions(+) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 6d978f85..d206ffb7 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -24,6 +24,7 @@ import requests from fhir.r4 import Patient +from flask.logging import default_handler from pydantic import ValidationError from gateway_api.common.error import PdsRequestFailedError @@ -42,6 +43,8 @@ get = pds.get # type: ignore logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) +logger.addHandler(default_handler) class PdsClient: diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 31d76a92..4f4dc0ef 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -26,6 +26,7 @@ import os from urllib.parse import urljoin +from flask.logging import default_handler from requests import HTTPError, Response from gateway_api.clinical_jwt import JWT, JWTValidator @@ -46,6 +47,8 @@ post = provider_stub.post # type: ignore logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) +logger.addHandler(default_handler) # Default endpoint path for access record structured interaction (standard GP Connect) ARS_ENDPOINT_PATH = "Patient/$gpc.getstructuredrecord" diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index de5a5432..96185901 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -16,6 +16,7 @@ from fhir import Resource from fhir.constants import FHIRSystem from fhir.r4 import Bundle, Device, Endpoint +from flask.logging import default_handler from requests import HTTPError from gateway_api.common.error import SdsRequestFailedError @@ -35,6 +36,8 @@ get = sds.get # type: ignore logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) +logger.addHandler(default_handler) class SdsResourceType(StrEnum): From f9be0036b39faefafa82c4961d9693d039ec44de Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:11:30 +0100 Subject: [PATCH 40/69] Make orange box test patient FHIR compliant. --- .../stubs/stubs/data/patients/orange_box_trigger_9690937278.json | 1 + 1 file changed, 1 insertion(+) diff --git a/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json b/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json index 1660e792..1d738cbe 100644 --- a/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json +++ b/gateway-api/stubs/stubs/data/patients/orange_box_trigger_9690937278.json @@ -27,6 +27,7 @@ "type": "Organization", "identifier": { "value": "S55555", + "system": "https://fhir.nhs.uk/Id/ods-organization-code", "period": {"start": "2020-01-01", "end": "9999-12-31"} } } From 5ded7c5de63f271163b428dac1641903a3319235 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:16:54 +0100 Subject: [PATCH 41/69] Update FHIR Parameters resource to include required name field. --- gateway-api/src/fhir/stu3/elements/parameters.py | 3 ++- gateway-api/src/fhir/stu3/elements/test_elements.py | 13 ++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/gateway-api/src/fhir/stu3/elements/parameters.py b/gateway-api/src/fhir/stu3/elements/parameters.py index f04a72e3..a45fe85a 100644 --- a/gateway-api/src/fhir/stu3/elements/parameters.py +++ b/gateway-api/src/fhir/stu3/elements/parameters.py @@ -1,6 +1,6 @@ from abc import ABC from dataclasses import dataclass -from typing import Annotated +from typing import Annotated, Literal from pydantic import Field @@ -15,6 +15,7 @@ class Parameters(Resource, resource_type="Parameters"): class Parameter(ABC): """A FHIR STU3 Parameter resource.""" + name: Annotated[Literal["patientNHSNumber"], Field(frozen=True)] valueIdentifier: Annotated[PatientIdentifier, Field(frozen=True)] parameter: Annotated[list[Parameter], Field(frozen=True, min_length=1)] diff --git a/gateway-api/src/fhir/stu3/elements/test_elements.py b/gateway-api/src/fhir/stu3/elements/test_elements.py index fcb25dfd..651b796f 100644 --- a/gateway-api/src/fhir/stu3/elements/test_elements.py +++ b/gateway-api/src/fhir/stu3/elements/test_elements.py @@ -8,6 +8,7 @@ class TestParameters: def test_create(self) -> None: """Test creating a Parameters resource.""" parameter = Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=PatientIdentifier( value="9000000009", ), @@ -26,11 +27,13 @@ def test_create(self) -> None: def test_create_with_multiple_parameters(self) -> None: """Test creating a Parameters resource with multiple parameters.""" param_a = Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=PatientIdentifier( value="9000000009", ), ) param_b = Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=PatientIdentifier( value="9000000017", ), @@ -53,6 +56,7 @@ def test_model_validate_valid(self) -> None: "resourceType": "Parameters", "parameter": [ { + "name": "patientNHSNumber", "valueIdentifier": { "system": "https://fhir.nhs.uk/Id/nhs-number", "value": "9000000009", @@ -87,6 +91,7 @@ def test_model_validate_with_wrong_resource_type_raises_error(self) -> None: "resourceType": "Patient", "parameter": [ { + "name": "patientNHSNumber", "valueIdentifier": { "system": "https://fhir.nhs.uk/Id/nhs-number", "value": "9000000009", @@ -110,6 +115,7 @@ def test_model_validate_with_invalid_identifier_system_raises_error(self) -> Non "resourceType": "Parameters", "parameter": [ { + "name": "patientNHSNumber", "valueIdentifier": { "system": "https://example.org/invalid", "value": "9000000009", @@ -143,6 +149,7 @@ def test_model_dump_json_roundtrip(self) -> None: params = Parameters.create( parameter=[ Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=PatientIdentifier( value="9000000009", ), @@ -164,6 +171,7 @@ def test_is_frozen(self) -> None: params = Parameters.create( parameter=[ Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=PatientIdentifier( value="9000000009", ), @@ -181,7 +189,9 @@ def test_create(self) -> None: identifier = PatientIdentifier( value="9000000009", ) - parameter = Parameters.Parameter(valueIdentifier=identifier) + parameter = Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=identifier + ) assert parameter.valueIdentifier == identifier, ( "valueIdentifier should match the provided identifier" @@ -196,6 +206,7 @@ def test_create(self) -> None: def test_is_frozen(self) -> None: """Test that Parameter fields are frozen (immutable).""" parameter = Parameters.Parameter( + name="patientNHSNumber", valueIdentifier=PatientIdentifier( value="9000000009", ), From 03aff726380bc3b9a6fe8962707001b02a63ea18 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 20:17:49 +0100 Subject: [PATCH 42/69] Add explicit return statements. --- scripts/env/app/pds.sh | 3 +++ scripts/env/app/provider.sh | 3 +++ scripts/env/app/sds.sh | 3 +++ scripts/env/test/apigee.sh | 2 ++ scripts/env/test/target.sh | 10 ++++++++++ scripts/env/test/user.sh | 1 + 6 files changed, 22 insertions(+) diff --git a/scripts/env/app/pds.sh b/scripts/env/app/pds.sh index f048dff0..a654db38 100755 --- a/scripts/env/app/pds.sh +++ b/scripts/env/app/pds.sh @@ -6,12 +6,15 @@ get_pds_url() { case "$env" in sandbox-pds) echo "https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4/" + return 0 ;; int) echo "https://int.api.service.nhs.uk/personal-demographics/FHIR/R4/" + return 0 ;; *) echo "stub" + return 0 ;; esac } diff --git a/scripts/env/app/provider.sh b/scripts/env/app/provider.sh index 2bf5efe0..397f597a 100755 --- a/scripts/env/app/provider.sh +++ b/scripts/env/app/provider.sh @@ -6,12 +6,15 @@ get_provider_url() { case "$env" in int) echo "how do we test integration with _all_ providers?" + return 0 ;; orangebox) echo "https://orange.testlab.nhs.uk/B82617/STU3/1/gpconnect/structured/fhir/" + return 0 ;; *) echo "stub" + return 0 ;; esac } diff --git a/scripts/env/app/sds.sh b/scripts/env/app/sds.sh index 22c31b5e..6c4246c9 100755 --- a/scripts/env/app/sds.sh +++ b/scripts/env/app/sds.sh @@ -6,12 +6,15 @@ get_sds_url() { case "$env" in sandbox-sds) echo "https://sandbox.api.service.nhs.uk/spine-directory/FHIR/R4" + return 0 ;; int) echo "https://int.api.service.nhs.uk/spine-directory/FHIR/R4" + return 0 ;; *) echo "stub" + return 0 ;; esac } diff --git a/scripts/env/test/apigee.sh b/scripts/env/test/apigee.sh index 95d19672..782b4f41 100644 --- a/scripts/env/test/apigee.sh +++ b/scripts/env/test/apigee.sh @@ -6,9 +6,11 @@ get_apigee_access_token() { case "$env" in pr-*|alpha-int) echo $(./scripts/get_apigee_token.sh) + return 0 ;; *) echo "" + return 0 ;; esac } diff --git a/scripts/env/test/target.sh b/scripts/env/test/target.sh index e17c804e..08857cc5 100644 --- a/scripts/env/test/target.sh +++ b/scripts/env/test/target.sh @@ -6,12 +6,15 @@ get_base_url() { case "$env" in local) echo "http://gateway-api:8080" + return 0 ;; ci) echo "http://localhost:5000" + return 0 ;; *) echo "" + return 0 ;; esac } @@ -21,9 +24,11 @@ get_proxygen_api_name() { case "$env" in pr-*|alpha-int) echo "clinical-data-gateway-api-poc" + return 0 ;; *) echo "" + return 0 ;; esac } @@ -34,12 +39,15 @@ get_proxy_base_path() { pr-*) pr_number="${env#pr-}" echo "clinical-data-gateway-api-poc-pr-${pr_number}" + return 0 ;; alpha-int) echo "clinical-data-gateway-api-poc-alpha-integration" + return 0 ;; *) echo "" + return 0 ;; esac } @@ -49,9 +57,11 @@ get_target_env() { case "$env" in pr-*|alpha-int) echo "remote" + return 0 ;; *) echo "local" + return 0 ;; esac } diff --git a/scripts/env/test/user.sh b/scripts/env/test/user.sh index 6dc5296c..3f79f488 100644 --- a/scripts/env/test/user.sh +++ b/scripts/env/test/user.sh @@ -6,6 +6,7 @@ get_test_user() { case "$env" in pr-*|alpha-int) echo "656005750101" + return 0 ;; esac } From 80bedaf480b1ea920120f4a3ddcac46370dd9a9c Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Wed, 22 Apr 2026 20:24:54 +0100 Subject: [PATCH 43/69] Add a default case for the test user - none. It should only be needed when explicitly called upon. --- scripts/env/test/user.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/env/test/user.sh b/scripts/env/test/user.sh index 3f79f488..f67c21f8 100644 --- a/scripts/env/test/user.sh +++ b/scripts/env/test/user.sh @@ -8,5 +8,9 @@ get_test_user() { echo "656005750101" return 0 ;; + *) + echo "" + return 0 + ;; esac } From 762b10b1dc9975ef2079d4cf7f746d562f17ae28 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 10:29:14 +0100 Subject: [PATCH 44/69] Add local secret handling for int --- .gitignore | 3 ++ .secrets/.gitkeep | 0 .secrets/pds/.gitkeep | 0 .secrets/sds/.gitkeep | 0 scripts/env/app/env.sh | 14 ++++++--- scripts/env/app/pds.sh | 60 +++++++++++++++++++++++++++++++++++++ scripts/env/app/provider.sh | 4 +-- scripts/env/app/sds.sh | 20 +++++++++++++ scripts/env/env.mk | 10 ++++++- 9 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 .secrets/.gitkeep create mode 100644 .secrets/pds/.gitkeep create mode 100644 .secrets/sds/.gitkeep diff --git a/.gitignore b/.gitignore index 3ab02b93..0510ea9e 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,6 @@ gateway-api/test-artefacts/ **/.env.* **/.DS_Store **/.terraform.lock.hcl + +# Any file within .secrets +.secrets/** diff --git a/.secrets/.gitkeep b/.secrets/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/.secrets/pds/.gitkeep b/.secrets/pds/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/.secrets/sds/.gitkeep b/.secrets/sds/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/scripts/env/app/env.sh b/scripts/env/app/env.sh index b867b763..9678c633 100755 --- a/scripts/env/app/env.sh +++ b/scripts/env/app/env.sh @@ -8,21 +8,27 @@ source scripts/env/app/provider.sh ENV_FILE=".env" PDS_URL=$(get_pds_url "$env") +PDS_API_TOKEN=$(get_pds_api_token "$env") +PDS_API_SECRET=$(get_pds_api_secret "$env") +PDS_API_KID=$(get_pds_api_kid "$env") + SDS_URL=$(get_sds_url "$env") +SDS_API_TOKEN=$(get_sds_api_token "$env") + PROVIDER_URL=$(get_provider_url "$env") cat > "$ENV_FILE" <&2 + return 1 + fi + ;; + *) + echo "" + return 0 + ;; + esac +} + +get_pds_api_secret() { + env="$1" + secret_file=".secrets/pds/api_secret" + case "$env" in + int|int-pds) + if [ -f "$secret_file" ]; then + cat "$secret_file" + return 0 + else + printf "Warning: $secret_file not found." >&2 + return 1 + fi + ;; + *) + echo "" + return 0 + ;; + esac +} + +get_pds_api_kid() { + env="$1" + secret_file=".secrets/pds/api_kid" + case "$env" in + int|int-pds) + if [ -f "$secret_file" ]; then + cat "$secret_file" + return 0 + else + printf "Warning: $secret_file not found." >&2 + return 1 + fi + ;; + *) + echo "" + return 0 + ;; + esac +} diff --git a/scripts/env/app/provider.sh b/scripts/env/app/provider.sh index 397f597a..f6655d47 100755 --- a/scripts/env/app/provider.sh +++ b/scripts/env/app/provider.sh @@ -4,8 +4,8 @@ set -e get_provider_url() { env="$1" case "$env" in - int) - echo "how do we test integration with _all_ providers?" + int|int-pds|int-sds) + echo "how_do_we_do_this?" return 0 ;; orangebox) diff --git a/scripts/env/app/sds.sh b/scripts/env/app/sds.sh index 6c4246c9..17ef3149 100755 --- a/scripts/env/app/sds.sh +++ b/scripts/env/app/sds.sh @@ -18,3 +18,23 @@ get_sds_url() { ;; esac } + +get_sds_api_token() { + env="$1" + secret_file=".secrets/sds/api_token" + case "$env" in + int|int-sds) + if [ -f "$secret_file" ]; then + cat "$secret_file" + return 0 + else + printf "Warning: $secret_file not found." >&2 + return 1 + fi + ;; + *) + echo "" + return 0 + ;; + esac +} diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 08265025..8b4d1824 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -15,9 +15,15 @@ env-sandbox-pds: # Create .env file that will have the app send requests to the env-sandbox-sds: # Create .env file that will have the app send requests to the SDS sandbox environment (sandbox SDS), stubs otherwise. make _env env="sandbox-sds" -env-int: # Create .env file with environment variables for integration environment +env-int: # Create .env file that will have the app send requests to the SDS and PDS integration environments. make _env env="int" +env-int-pds: # Create .env file that will have the app send requests to the PDS integration environment (int PDS), stubs otherwise. + make _env env="int-pds" + +env-int-sds: # Create .env file that will have the app send requests to the SDS integration environment (int SDS), stubs otherwise. + make _env env="int-sds" + env-test-local: # Create .env.test file that will have tests send requests to the local app. make _env-test env="local" make env-dev # Ensure unit tests run with stub environment variables @@ -47,6 +53,8 @@ ${VERBOSE}.SILENT: \ env-sandbox-pds \ env-sandbox-sds \ env-int \ + env-int-pds \ + env-int-sds \ _env-test \ env-test-local \ env-test-ci \ From f9f13192522c54299db25eb5724e029effa86ea1 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:04:36 +0100 Subject: [PATCH 45/69] Removing noise from "make deploy" output --- Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile b/Makefile index 0384262a..9ca1fb33 100644 --- a/Makefile +++ b/Makefile @@ -68,8 +68,6 @@ publish: # Publish the project artefact @Pipeline deploy: clean build # Build project artefact and deploy locally @Pipeline @$(docker) network inspect gateway-local >/dev/null 2>&1 || $(docker) network create gateway-local - @echo "Using environment variables from .env" - @cat .env if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \ echo "Starting using local docker network ..." ; \ $(docker) run --platform linux/amd64 --name gateway-api -p 5000:8080 --network gateway-local --env-file .env -d ${IMAGE_NAME} ; \ From 5c99f533871f77d7b8f7928a362611a67b1f7d43 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:06:20 +0100 Subject: [PATCH 46/69] Only log pertinent, non-sensitive env vars. --- gateway-api/src/gateway_api/app.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 386e4973..2c9cc39d 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -64,9 +64,14 @@ def log_error(error: AbstractCDGError) -> None: def log_env_vars(app: Flask) -> None: + env_vars = { + key: value + for key, value in os.environ.items() + if key in {"FLASK_HOST", "FLASK_PORT", "PDS_URL", "SDS_URL"} + } log_details = { "description": "Initializing Flask app", - "env_vars": os.environ.items(), + "env_vars": env_vars, } app.logger.info(log_details) From dd2c50b8109e86527308633abc567fa110b80498 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:08:37 +0100 Subject: [PATCH 47/69] Do not log sensitive headers. --- gateway-api/src/gateway_api/app.py | 7 ++++++- gateway-api/src/gateway_api/sds/client.py | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 2c9cc39d..750ba3c2 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -44,11 +44,16 @@ def get_env_var[T](name: str, parser: Callable[[str], T]) -> T: def log_request_received(request: Request) -> None: + sanitized_headers = { + key: value + for key, value in request.headers.items() + if key.lower() != "authorization" + } log_details = { "description": "Received request", "method": request.method, "path": request.path, - "headers": dict(request.headers), + "headers": sanitized_headers, } app.logger.info(log_details) diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index 96185901..e633a197 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -206,7 +206,6 @@ def _query_sds( log_details = { "description": "SDS request", "url": url, - "headers": headers, "params": params, } logger.info(log_details) From b5445c520275422e72910fbda8abaaa752a6c184 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:10:21 +0100 Subject: [PATCH 48/69] clean up. --- scripts/tests/test.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index fec7dbbf..e9f4752f 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -73,7 +73,7 @@ test-load-initial: # Run your load tests from scripts/test/load @Testing @$(MAKE) _test name="load" test-load-ui: # Run your load tests from scripts/test/load @Testing with UI enabled - @UI=true $(MAKE) _test name="load"' + @UI=true $(MAKE) _test name="load" _test: set -e From 519efda8f34f14553baa59ac14118d9c454388eb Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:12:46 +0100 Subject: [PATCH 49/69] Remove env vars that did not exist before fiddling with the env vars. --- gateway-api/src/gateway_api/conftest.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 80145467..4bd4ceb3 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -39,6 +39,9 @@ def __exit__( _value: BaseException | None, _traceback: TracebackType | None, ) -> None: + for key in self.new_env_vars: + if key in os.environ: + del os.environ[key] os.environ.update(self.original_env_vars) From 057a4c4d67fb757507fad82dd64aaf9f9cda48c8 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:15:55 +0100 Subject: [PATCH 50/69] Options not supported at this level. --- .github/actions/run-test-suite/action.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/actions/run-test-suite/action.yaml b/.github/actions/run-test-suite/action.yaml index 77a26ee8..2eee7d00 100644 --- a/.github/actions/run-test-suite/action.yaml +++ b/.github/actions/run-test-suite/action.yaml @@ -10,13 +10,9 @@ inputs: description: "Type of test to run" required: true env: - description: "Environment to run tests against - see env.mk" + description: "Environment to run tests against: ci, alpha-int, pr- - see env.mk" required: false default: "ci" - options: - - ci - - alpha-int - - pr- runs: using: composite From 548c49d2822721ce891f7584ac5abbf37d232110 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:17:03 +0100 Subject: [PATCH 51/69] Do not log sensitive headers. --- gateway-api/src/gateway_api/provider/client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 4f4dc0ef..4325dfe2 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -135,7 +135,6 @@ def access_structured_record( log_details = { "description": "GPProvider FHIR API request", "url": url, - "headers": headers, } logger.info(log_details) From 158f4c07292001cff1aeb0239b8926685841e38a Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:18:51 +0100 Subject: [PATCH 52/69] Stub for int environment, until 397 is worked. --- scripts/env/app/provider.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/env/app/provider.sh b/scripts/env/app/provider.sh index f6655d47..557997e5 100755 --- a/scripts/env/app/provider.sh +++ b/scripts/env/app/provider.sh @@ -5,7 +5,8 @@ get_provider_url() { env="$1" case "$env" in int|int-pds|int-sds) - echo "how_do_we_do_this?" + # TODO [GPCAPIM-397]: Update this. + echo "stub" return 0 ;; orangebox) From c22c719c413884b9646dbf93453a1a1fd29b337a Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 11:22:55 +0100 Subject: [PATCH 53/69] Provide useful error messages if env files do not exist. --- scripts/tests/run-test.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index c06f03f2..cdd229d4 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -31,8 +31,17 @@ else TEST_PATH="tests/${TEST_TYPE}/" fi +if [[ ! -d "$TEST_PATH" ]]; then + echo "Error: Test path '$TEST_PATH' does not exist" >&2 + exit 1 +fi source .env.test + if [[ "$TEST_TYPE" = "unit" ]]; then + if [[ ! -f ".env" ]]; then + echo "Error: .env file not found. Please run 'make env-' to generate it." >&2 + exit 1 + fi set -a source .env set +a From d04316155e8bad92d3b4a8f54600a502be150b12 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:08:41 +0100 Subject: [PATCH 54/69] Provide explanation around the expected secrets. --- .gitignore | 1 + .secrets/.gitkeep | 0 .secrets/README.md | 19 +++++++++++++++++++ 3 files changed, 20 insertions(+) delete mode 100644 .secrets/.gitkeep create mode 100644 .secrets/README.md diff --git a/.gitignore b/.gitignore index 0510ea9e..8e9f79dc 100644 --- a/.gitignore +++ b/.gitignore @@ -29,3 +29,4 @@ gateway-api/test-artefacts/ # Any file within .secrets .secrets/** +!.secrets/README.md diff --git a/.secrets/.gitkeep b/.secrets/.gitkeep deleted file mode 100644 index e69de29b..00000000 diff --git a/.secrets/README.md b/.secrets/README.md new file mode 100644 index 00000000..9b8b5967 --- /dev/null +++ b/.secrets/README.md @@ -0,0 +1,19 @@ +# Secrets + +This directory is used to store secrets. + +The secrets are accessed through `make env-` which sets the secrets required for PDS FHIR API and SDS FHIR API to `.env` file, which is then fed in to the locally deployed application through `make deploy`. + +## PDS + +PDS FHIR API requires [signed JWT for application-resrtictecd access](https://digital.nhs.uk/developer/guides-and-documentation/security-and-authorisation/application-restricted-restful-apis-signed-jwt-authentication). As such, the following three secrets enable the Gateway API to authenticate: + +* `.secrets/pds/api_token` - the API key of the application through which the Gateway API will consume NHSE APIs. +* `.secrets/pds/api_secret` - the private key of the public/private key pair created for application identified by `api_token` +* `.secrets/pds/api_kid` - the key identifier for the private/public key pair used for the `api_secret`. + +## SDS + +SDS FHIR API requires [API key authentication](https://digital.nhs.uk/developer/guides-and-documentation/security-and-authorisation/application-restricted-restful-apis-api-key-authentication) for application-restricted access. As such, the only secret required is + +* `.secrets/sds/api_token` - the API key of the application through which the Gateway API will consume NHSE APIs. From 7a4fc64d34d352f077ced225788ba44b9fc2b3c1 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 14:11:16 +0100 Subject: [PATCH 55/69] Correct env file check. --- scripts/tests/run-test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/tests/run-test.sh b/scripts/tests/run-test.sh index cdd229d4..608b74e2 100755 --- a/scripts/tests/run-test.sh +++ b/scripts/tests/run-test.sh @@ -31,8 +31,8 @@ else TEST_PATH="tests/${TEST_TYPE}/" fi -if [[ ! -d "$TEST_PATH" ]]; then - echo "Error: Test path '$TEST_PATH' does not exist" >&2 +if [[ ! -f ".env.test" ]]; then + echo "Error: .env.test file not found. Please run 'make env-test-' to generate it." >&2 exit 1 fi source .env.test From f84e63e884a07906858a4778581f6159b4941328 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 15:24:46 +0100 Subject: [PATCH 56/69] Refine logger setup; test url that instantiates the client is being used on the GET requests. --- gateway-api/src/gateway_api/app.py | 22 +++++++++++++------ gateway-api/src/gateway_api/pds/client.py | 11 ++++------ .../src/gateway_api/pds/test_client.py | 19 ++++++++++++++++ .../src/gateway_api/provider/client.py | 11 ++++------ gateway-api/src/gateway_api/sds/client.py | 11 ++++------ .../src/gateway_api/sds/test_client.py | 21 ++++++++++++++++-- gateway-api/src/gateway_api/test_app.py | 16 +++++++++----- scripts/env/app/env.sh | 4 ++++ scripts/env/app/logging.sh | 18 +++++++++++++++ scripts/tests/run-test.sh | 2 +- 10 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 scripts/env/app/logging.sh diff --git a/gateway-api/src/gateway_api/app.py b/gateway-api/src/gateway_api/app.py index 750ba3c2..0ff3f695 100644 --- a/gateway-api/src/gateway_api/app.py +++ b/gateway-api/src/gateway_api/app.py @@ -1,6 +1,7 @@ import os import traceback from collections.abc import Callable +from logging import basicConfig, getLogger from flask import Flask, Request, request from flask.wrappers import Response @@ -13,16 +14,23 @@ ) app = Flask(__name__) -app.logger.setLevel("INFO") +_logger = getLogger(__name__) def start_app(app: Flask) -> None: - log_env_vars(app) + setup_logging() + log_env_vars() configure_app(app) log_starting_app(app) app.run(host=app.config["FLASK_HOST"], port=app.config["FLASK_PORT"]) +def setup_logging() -> None: + basicConfig( + level=os.getenv("LOG_LEVEL", "INFO"), + ) + + def configure_app(app: Flask) -> None: config = { "FLASK_HOST": get_env_var("FLASK_HOST", str), @@ -55,7 +63,7 @@ def log_request_received(request: Request) -> None: "path": request.path, "headers": sanitized_headers, } - app.logger.info(log_details) + _logger.info(log_details) def log_error(error: AbstractCDGError) -> None: @@ -65,10 +73,10 @@ def log_error(error: AbstractCDGError) -> None: "error_message": str(error), "traceback": traceback.format_exc(), } - app.logger.error(log_details) + _logger.error(log_details) -def log_env_vars(app: Flask) -> None: +def log_env_vars() -> None: env_vars = { key: value for key, value in os.environ.items() @@ -78,7 +86,7 @@ def log_env_vars(app: Flask) -> None: "description": "Initializing Flask app", "env_vars": env_vars, } - app.logger.info(log_details) + _logger.info(log_details) def log_starting_app(app: Flask) -> None: @@ -89,7 +97,7 @@ def log_starting_app(app: Flask) -> None: "pds_base_url": app.config["PDS_URL"], "sds_base_url": app.config["SDS_URL"], } - app.logger.info(log_details) + _logger.info(log_details) @app.route("/patient/$gpc.getstructuredrecord", methods=["POST"]) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index d206ffb7..76db1664 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -24,7 +24,6 @@ import requests from fhir.r4 import Patient -from flask.logging import default_handler from pydantic import ValidationError from gateway_api.common.error import PdsRequestFailedError @@ -42,9 +41,7 @@ pds = PdsFhirApiStub() get = pds.get # type: ignore -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -logger.addHandler(default_handler) +_logger = logging.getLogger(__name__) class PdsClient: @@ -87,7 +84,7 @@ def __init__( "description": "Initialized PdsClient", "base_url": self.base_url, } - logger.info(log_details) + _logger.info(log_details) def _build_headers( self, @@ -133,7 +130,7 @@ def search_patient_by_nhs_number( "url": url, "headers": headers, } - logger.info(log_details) + _logger.info(log_details) # This normally calls requests.get, but if PDS_URL is set it uses the stub. response = get( url, @@ -145,7 +142,7 @@ def search_patient_by_nhs_number( "description": "PDS response received", "status_code": str(response.status_code), } - logger.info(log_details) + _logger.info(log_details) try: response.raise_for_status() diff --git a/gateway-api/src/gateway_api/pds/test_client.py b/gateway-api/src/gateway_api/pds/test_client.py index 414ceefc..aee33bf2 100644 --- a/gateway-api/src/gateway_api/pds/test_client.py +++ b/gateway-api/src/gateway_api/pds/test_client.py @@ -147,3 +147,22 @@ def test_search_patient_by_nhs_number_missing_nhs_number_raises_error( assert "'identifier'" in str(error.value) assert "too_short" in str(error.value) + + +def test_search_patient_respects_url( + auth_token: str, + mocker: MockerFixture, + happy_path_pds_response_body: dict[str, Any], +) -> None: + happy_path_response = FakeResponse( + status_code=200, headers={}, _json=happy_path_pds_response_body + ) + mocked_get = mocker.patch( + "gateway_api.pds.client.get", return_value=happy_path_response + ) + + client = PdsClient(auth_token, base_url="https://a.different.url/base") + _ = client.search_patient_by_nhs_number("9000000009") + + actual_url = mocked_get.call_args.args[0] + assert actual_url == "https://a.different.url/base/Patient/9000000009" diff --git a/gateway-api/src/gateway_api/provider/client.py b/gateway-api/src/gateway_api/provider/client.py index 4325dfe2..338e6093 100644 --- a/gateway-api/src/gateway_api/provider/client.py +++ b/gateway-api/src/gateway_api/provider/client.py @@ -26,7 +26,6 @@ import os from urllib.parse import urljoin -from flask.logging import default_handler from requests import HTTPError, Response from gateway_api.clinical_jwt import JWT, JWTValidator @@ -46,9 +45,7 @@ provider_stub = GpProviderStub() post = provider_stub.post # type: ignore -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -logger.addHandler(default_handler) +_logger = logging.getLogger(__name__) # Default endpoint path for access record structured interaction (standard GP Connect) ARS_ENDPOINT_PATH = "Patient/$gpc.getstructuredrecord" @@ -96,7 +93,7 @@ def __init__( "consumer_asid": self.consumer_asid, "endpoint_path": self.endpoint_path, } - logger.info(log_details) + _logger.info(log_details) def _build_headers(self, trace_id: str) -> dict[str, str]: """ @@ -136,7 +133,7 @@ def access_structured_record( "description": "GPProvider FHIR API request", "url": url, } - logger.info(log_details) + _logger.info(log_details) response = post( url, @@ -148,7 +145,7 @@ def access_structured_record( "description": "GPProvider FHIR API response received", "status_code": str(response.status_code), } - logger.info(log_details) + _logger.info(log_details) try: response.raise_for_status() diff --git a/gateway-api/src/gateway_api/sds/client.py b/gateway-api/src/gateway_api/sds/client.py index e633a197..19f57d3b 100644 --- a/gateway-api/src/gateway_api/sds/client.py +++ b/gateway-api/src/gateway_api/sds/client.py @@ -16,7 +16,6 @@ from fhir import Resource from fhir.constants import FHIRSystem from fhir.r4 import Bundle, Device, Endpoint -from flask.logging import default_handler from requests import HTTPError from gateway_api.common.error import SdsRequestFailedError @@ -35,9 +34,7 @@ sds = SdsFhirApiStub() get = sds.get # type: ignore -logger = logging.getLogger(__name__) -logger.setLevel(logging.INFO) -logger.addHandler(default_handler) +_logger = logging.getLogger(__name__) class SdsResourceType(StrEnum): @@ -98,7 +95,7 @@ def __init__( "base_url": self.base_url, "service_interaction_id": self.service_interaction_id, } - logger.info(log_details) + _logger.info(log_details) def _build_headers(self, correlation_id: str | None = None) -> dict[str, str]: """ @@ -208,7 +205,7 @@ def _query_sds( "url": url, "params": params, } - logger.info(log_details) + _logger.info(log_details) response = get( url, headers=headers, @@ -219,7 +216,7 @@ def _query_sds( "description": "SDS response received", "status_code": str(response.status_code), } - logger.info(log_details) + _logger.info(log_details) try: response.raise_for_status() diff --git a/gateway-api/src/gateway_api/sds/test_client.py b/gateway-api/src/gateway_api/sds/test_client.py index a10bc023..8bd25514 100644 --- a/gateway-api/src/gateway_api/sds/test_client.py +++ b/gateway-api/src/gateway_api/sds/test_client.py @@ -2,13 +2,14 @@ Unit tests for :mod:`gateway_api.sds_search`. """ -from __future__ import annotations - import pytest from fhir.constants import FHIRSystem +from fhir.r4.resources.bundle import Bundle +from pytest_mock import MockerFixture from stubs.sds.stub import SdsFhirApiStub from gateway_api.common.error import SdsRequestFailedError +from gateway_api.conftest import FakeResponse from gateway_api.get_structured_record import ACCESS_RECORD_STRUCTURED_INTERACTION_ID from gateway_api.sds import ( SdsClient, @@ -417,3 +418,19 @@ def test_sds_client_no_endpoint_bundle_entries_returns_none_endpoint( assert result.asid == "222222222222" assert result.endpoint is None + + +def test_sds_client_respects_url( + mocker: MockerFixture, +) -> None: + empty_bundle = Bundle.empty("searchset").model_dump() + mocked_get = mocker.patch( + "gateway_api.sds.client.get", + return_value=FakeResponse(status_code=200, headers={}, _json=empty_bundle), + ) + + client = SdsClient(base_url="https://a.different.url/base") + _ = client.get_org_details(ods_code="A12345", get_endpoint=False) + + actual_url = mocked_get.call_args.args[0] + assert actual_url == "https://a.different.url/base/Device" diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 5edfbfb5..39f755e1 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -1,7 +1,6 @@ """Unit tests for the Flask app endpoints.""" import json -import os from collections.abc import Generator from copy import copy from typing import Any @@ -75,15 +74,22 @@ def test_configure_app(self) -> None: def test_logging_environment_variables_on_app_initialization( self, mocker: MockerFixture ) -> None: - log_info_mock = mocker.patch.object(app.logger, "info") + log_mock_info = mocker.patch("gateway_api.app._logger.info") - log_env_vars(app) + config = { + "FLASK_HOST": "test_host", + "FLASK_PORT": "1234", + "PDS_URL": "test_pds_url", + "SDS_URL": "test_sds_url", + } + with NewEnvVars(config): + log_env_vars() # Check that the environment variables were logged - log_info_mock.assert_called_with( + log_mock_info.assert_called_with( { "description": "Initializing Flask app", - "env_vars": os.environ.items(), + "env_vars": config, } ) diff --git a/scripts/env/app/env.sh b/scripts/env/app/env.sh index 9678c633..25d74972 100755 --- a/scripts/env/app/env.sh +++ b/scripts/env/app/env.sh @@ -4,6 +4,7 @@ set -e source scripts/env/app/pds.sh source scripts/env/app/sds.sh source scripts/env/app/provider.sh +source scripts/env/app/logging.sh ENV_FILE=".env" @@ -17,6 +18,8 @@ SDS_API_TOKEN=$(get_sds_api_token "$env") PROVIDER_URL=$(get_provider_url "$env") +LOG_LEVEL=$(get_log_level "$env") + cat > "$ENV_FILE" < Date: Thu, 23 Apr 2026 15:27:04 +0100 Subject: [PATCH 57/69] Rename class to better reflect behaviour. --- gateway-api/src/gateway_api/conftest.py | 4 ++-- gateway-api/src/gateway_api/test_app.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gateway-api/src/gateway_api/conftest.py b/gateway-api/src/gateway_api/conftest.py index 4bd4ceb3..99548b5c 100644 --- a/gateway-api/src/gateway_api/conftest.py +++ b/gateway-api/src/gateway_api/conftest.py @@ -17,7 +17,7 @@ from gateway_api.clinical_jwt import JWT -class NewEnvVars: +class ScopedEnvVars: def __init__(self, new_env_vars: Mapping[str, str | None]) -> None: self.new_env_vars = new_env_vars self.original_env_vars = {} @@ -25,7 +25,7 @@ def __init__(self, new_env_vars: Mapping[str, str | None]) -> None: if key in os.environ: self.original_env_vars[key] = os.environ[key] - def __enter__(self) -> "NewEnvVars": + def __enter__(self) -> "ScopedEnvVars": for key, value in self.new_env_vars.items(): if value is None and key in os.environ: del os.environ[key] diff --git a/gateway-api/src/gateway_api/test_app.py b/gateway-api/src/gateway_api/test_app.py index 39f755e1..140a8905 100644 --- a/gateway-api/src/gateway_api/test_app.py +++ b/gateway-api/src/gateway_api/test_app.py @@ -18,12 +18,12 @@ log_env_vars, start_app, ) -from gateway_api.conftest import NewEnvVars +from gateway_api.conftest import ScopedEnvVars @pytest.fixture def client() -> Generator[FlaskClient[Flask]]: - with NewEnvVars( + with ScopedEnvVars( { "FLASK_HOST": "localhost", "FLASK_PORT": "5000", @@ -39,16 +39,16 @@ def client() -> Generator[FlaskClient[Flask]]: class TestAppInitialization: def test_get_env_var_when_env_var_is_set(self) -> None: - with NewEnvVars({"FLASK_HOST": "host_is_set"}): + with ScopedEnvVars({"FLASK_HOST": "host_is_set"}): actual = get_env_var("FLASK_HOST", str) assert actual == "host_is_set" def test_get_env_var_raises_runtime_error_if_env_var_not_set(self) -> None: - with NewEnvVars({"FLASK_HOST": None}), pytest.raises(RuntimeError): + with ScopedEnvVars({"FLASK_HOST": None}), pytest.raises(RuntimeError): _ = get_env_var("FLASK_HOST", str) def test_get_env_var_raises_runtime_error_if_loader_fails(self) -> None: - with NewEnvVars({"FLASK_PORT": "not_an_int"}), pytest.raises(RuntimeError): + with ScopedEnvVars({"FLASK_PORT": "not_an_int"}), pytest.raises(RuntimeError): _ = get_env_var("FLASK_PORT", int) def test_configure_app(self) -> None: @@ -60,7 +60,7 @@ def test_configure_app(self) -> None: "SDS_URL": "test_sds_url", } - with NewEnvVars(config): + with ScopedEnvVars(config): configure_app(test_app) expected = { @@ -82,7 +82,7 @@ def test_logging_environment_variables_on_app_initialization( "PDS_URL": "test_pds_url", "SDS_URL": "test_sds_url", } - with NewEnvVars(config): + with ScopedEnvVars(config): log_env_vars() # Check that the environment variables were logged @@ -102,7 +102,7 @@ def test_start_app_logs_startup_details(self) -> None: "FLASK_PORT": "1234", } - with NewEnvVars(test_env_vars): + with ScopedEnvVars(test_env_vars): start_app(test_app) test_app.run.assert_called_with(host="test_host", port=1234) From 3775cb59ecfc1cda894f00d56652aa5b30af157d Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:40:49 +0100 Subject: [PATCH 58/69] Test url that instantiates the controller is being used on the GET requests. --- .../src/gateway_api/test_controller.py | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index 38201c9a..f329af0b 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -358,3 +358,94 @@ def test_controller_creates_jwt_token_with_correct_claims( # Verify the requesting organization matches the consumer ODS assert jwt_token.requesting_organization["identifier"][0]["value"] == consumer_ods + + +def test_controller_respects_pds_url( + mocker: MockerFixture, + happy_path_pds_response_body: dict[str, Any], +) -> None: + """ + Test that the controller uses the PDS URL provided in the constructor. + """ + mocked_get_pds = mocker.patch( + "gateway_api.pds.client.get", + return_value=FakeResponse( + status_code=200, headers={}, _json=happy_path_pds_response_body + ), + ) + custom_pds_url = "https://a.different.url/base" + + controller = create_test_controller(pds_base_url=custom_pds_url) + controller._get_pds_details("auth_token", "9000000009") + + actual_pds_url = mocked_get_pds.call_args.args[0] + assert actual_pds_url == "https://a.different.url/base/Patient/9000000009" + + +@pytest.fixture +def mock_request_with_headers(valid_simple_request_payload: dict[str, Any]) -> Request: + headers = { + "Ssp-TraceID": "test-trace-id", + "ODS-from": "test-ods", + } + return create_mock_request(headers, valid_simple_request_payload) + + +def test_controller_respects_sds_url( + mocker: MockerFixture, +) -> None: + """ + Test that the controller uses the SDS URL provided in the constructor. + """ + device_bundle = { + "resourceType": "Bundle", + "type": "searchset", + "total": 1, + "entry": [ + { + "fullUrl": "https://example.test/sds/Device/1", + "resource": { + "resourceType": "Device", + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhsSpineASID", + "value": "test-asid", + }, + { + "system": "https://fhir.nhs.uk/Id/nhsMhsPartyKey", + "value": "test-party-key", + }, + ], + }, + } + ], + } + endpoint_bundle = { + "resourceType": "Bundle", + "type": "searchset", + "total": 1, + "entry": [ + { + "fullUrl": "https://example.test/sds/Endpoint/1", + "resource": { + "resourceType": "Endpoint", + "address": "https://example.provider.org/endpoint", + }, + } + ], + } + mocked_get_sds = mocker.patch( + "gateway_api.sds.client.get", + side_effect=[ + FakeResponse(status_code=200, headers={}, _json=device_bundle), + FakeResponse(status_code=200, headers={}, _json=endpoint_bundle), + FakeResponse(status_code=200, headers={}, _json=device_bundle), + ], + ) + custom_sds_url = "https://a.different.url/base" + + controller = create_test_controller(sds_base_url=custom_sds_url) + controller._get_sds_details("test-ods-1", "test-ods-2") + + actual_sds_url = mocked_get_sds.call_args.args[0] + assert actual_sds_url == "https://a.different.url/base/Device" From c98e561f4f59b4aae5179fb0d0a0cb17a8d08d23 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:43:55 +0100 Subject: [PATCH 59/69] Remove unused fixture. --- gateway-api/src/gateway_api/test_controller.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/gateway-api/src/gateway_api/test_controller.py b/gateway-api/src/gateway_api/test_controller.py index f329af0b..0cb660ef 100644 --- a/gateway-api/src/gateway_api/test_controller.py +++ b/gateway-api/src/gateway_api/test_controller.py @@ -382,15 +382,6 @@ def test_controller_respects_pds_url( assert actual_pds_url == "https://a.different.url/base/Patient/9000000009" -@pytest.fixture -def mock_request_with_headers(valid_simple_request_payload: dict[str, Any]) -> Request: - headers = { - "Ssp-TraceID": "test-trace-id", - "ODS-from": "test-ods", - } - return create_mock_request(headers, valid_simple_request_payload) - - def test_controller_respects_sds_url( mocker: MockerFixture, ) -> None: From 8c5c2feb5ef3033d882c5e70a523c8f90f4e8583 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:51:35 +0100 Subject: [PATCH 60/69] Provider a more helpful warning. --- gateway-api/tests/integration/test_get_structured_record.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gateway-api/tests/integration/test_get_structured_record.py b/gateway-api/tests/integration/test_get_structured_record.py index 752a154f..c3324622 100644 --- a/gateway-api/tests/integration/test_get_structured_record.py +++ b/gateway-api/tests/integration/test_get_structured_record.py @@ -152,8 +152,8 @@ def test_internal_server_error_message_returned_when_provider_returns_error( ) -> None: """ WARNING: This can fail if the CDG_DEBUG env var is set to true in the - deployed app. - TODO [GPCAPIM-353]: Ensure integration tests are not affected by CDG_DEBUG + deployed app - see GpProviderClient.access_structured_record(). + TODO [GPCAPIM-401]: Ensure integration tests are not affected by CDG_DEBUG """ expected = { "resourceType": "OperationOutcome", From 12122962d5879284a38afc54b3087bfdc850c7bf Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:13:09 +0100 Subject: [PATCH 61/69] Explain what this decorator is doing. --- gateway-api/tests/schema/test_openapi_schema.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gateway-api/tests/schema/test_openapi_schema.py b/gateway-api/tests/schema/test_openapi_schema.py index 5619d805..1ee0f06a 100644 --- a/gateway-api/tests/schema/test_openapi_schema.py +++ b/gateway-api/tests/schema/test_openapi_schema.py @@ -19,6 +19,13 @@ schema = from_dict(schema_dict) +# Schemathesis warns you that this test is running a function-scoped fixture - a fixture +# that is executed once per test function, not per test case. Given schemathesis +# generates multiple test cases from the same test function, this means that the fixture +# will be executed only once for all generated cases, this can be a problem for certain +# fixtures. However, in this case, as base_url should not change during a test run, +# using the cached value for each test case is acceptable. +# See https://hypothesis.readthedocs.io/en/latest/reference/api.html#hypothesis.HealthCheck.function_scoped_fixture @schema.parametrize() @settings(suppress_health_check=[HealthCheck.function_scoped_fixture]) def test_api_schema_compliance(case: Case, base_url: str) -> None: From b02ac58d35ee969416b7a6e90e519be07b07409b Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:15:44 +0100 Subject: [PATCH 62/69] Update fixture name. --- gateway-api/tests/conftest.py | 6 +++--- gateway-api/tests/contract/test_provider_contract.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gateway-api/tests/conftest.py b/gateway-api/tests/conftest.py index 70828676..3a73f619 100644 --- a/gateway-api/tests/conftest.py +++ b/gateway-api/tests/conftest.py @@ -85,7 +85,7 @@ def simple_request_payload() -> dict[str, Any]: @pytest.fixture -def get_headers(env: str, request: pytest.FixtureRequest) -> dict[str, str]: +def headers(env: str, request: pytest.FixtureRequest) -> dict[str, str]: """Return auth headers for remote tests, or Apigee token for local.""" if env == "local": token = os.environ.get("APIGEE_ACCESS_TOKEN", "") @@ -104,10 +104,10 @@ def get_headers(env: str, request: pytest.FixtureRequest) -> dict[str, str]: def client( base_url: str, health_endpoint: str, - get_headers: dict[str, str], + headers: dict[str, str], ) -> Client: return Client( - base_url=base_url, health_endpoint=health_endpoint, auth_headers=get_headers + base_url=base_url, health_endpoint=health_endpoint, auth_headers=headers ) diff --git a/gateway-api/tests/contract/test_provider_contract.py b/gateway-api/tests/contract/test_provider_contract.py index ac231f25..1fa29b1b 100644 --- a/gateway-api/tests/contract/test_provider_contract.py +++ b/gateway-api/tests/contract/test_provider_contract.py @@ -12,7 +12,7 @@ from tests.conftest import Client -def test_provider_honors_consumer_contract(get_headers: Any, client: Client) -> None: +def test_provider_honors_consumer_contract(headers: Any, client: Client) -> None: host = urlparse(client.base_url).hostname @@ -22,7 +22,7 @@ def test_provider_honors_consumer_contract(get_headers: Any, client: Client) -> verifier.add_transport(url=client.base_url) # So the Verifier can authenticate with the APIM proxy - verifier.add_custom_headers(get_headers) + verifier.add_custom_headers(headers) verifier.add_source( "tests/contract/pacts/GatewayAPIConsumer-GatewayAPIProvider.json" From 71ea52315a2e619073e20c474d32991d75aa0bc1 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:29:06 +0100 Subject: [PATCH 63/69] Remove toggling of sandbox/int services --- scripts/env/app/pds.sh | 8 ++++---- scripts/env/app/sds.sh | 4 ++-- scripts/env/env.mk | 16 +++------------- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/scripts/env/app/pds.sh b/scripts/env/app/pds.sh index 1fb4b589..147e7d27 100755 --- a/scripts/env/app/pds.sh +++ b/scripts/env/app/pds.sh @@ -4,7 +4,7 @@ set -e get_pds_url() { env="$1" case "$env" in - sandbox-pds) + sandbox) echo "https://sandbox.api.service.nhs.uk/personal-demographics/FHIR/R4/" return 0 ;; @@ -23,7 +23,7 @@ get_pds_api_token() { env="$1" secret_file=".secrets/pds/api_token" case "$env" in - int|int-pds) + int) if [ -f "$secret_file" ]; then cat "$secret_file" return 0 @@ -43,7 +43,7 @@ get_pds_api_secret() { env="$1" secret_file=".secrets/pds/api_secret" case "$env" in - int|int-pds) + int) if [ -f "$secret_file" ]; then cat "$secret_file" return 0 @@ -63,7 +63,7 @@ get_pds_api_kid() { env="$1" secret_file=".secrets/pds/api_kid" case "$env" in - int|int-pds) + int) if [ -f "$secret_file" ]; then cat "$secret_file" return 0 diff --git a/scripts/env/app/sds.sh b/scripts/env/app/sds.sh index 17ef3149..a68cfdc0 100755 --- a/scripts/env/app/sds.sh +++ b/scripts/env/app/sds.sh @@ -4,7 +4,7 @@ set -e get_sds_url() { env="$1" case "$env" in - sandbox-sds) + sandbox) echo "https://sandbox.api.service.nhs.uk/spine-directory/FHIR/R4" return 0 ;; @@ -23,7 +23,7 @@ get_sds_api_token() { env="$1" secret_file=".secrets/sds/api_token" case "$env" in - int|int-sds) + int) if [ -f "$secret_file" ]; then cat "$secret_file" return 0 diff --git a/scripts/env/env.mk b/scripts/env/env.mk index 8b4d1824..55b8983b 100644 --- a/scripts/env/env.mk +++ b/scripts/env/env.mk @@ -9,21 +9,12 @@ env-ci: # Create .env file with environment variables for CI environment (stubs) env-orangebox: # Create .env file that will have the app send requests to the provider "orangebox", stubs otherwise. make _env env="orangebox" -env-sandbox-pds: # Create .env file that will have the app send requests to the PDS sandbox environment (sandbox PDS), stubs otherwise. - make _env env="sandbox-pds" - -env-sandbox-sds: # Create .env file that will have the app send requests to the SDS sandbox environment (sandbox SDS), stubs otherwise. - make _env env="sandbox-sds" +env-sandbox: # Create .env file that will have the app send requests to the SDS and PDS sandbox environment, stubs otherwise. + make _env env="sandbox" env-int: # Create .env file that will have the app send requests to the SDS and PDS integration environments. make _env env="int" -env-int-pds: # Create .env file that will have the app send requests to the PDS integration environment (int PDS), stubs otherwise. - make _env env="int-pds" - -env-int-sds: # Create .env file that will have the app send requests to the SDS integration environment (int SDS), stubs otherwise. - make _env env="int-sds" - env-test-local: # Create .env.test file that will have tests send requests to the local app. make _env-test env="local" make env-dev # Ensure unit tests run with stub environment variables @@ -50,8 +41,7 @@ ${VERBOSE}.SILENT: \ env-dev \ env-ci \ env-orangebox \ - env-sandbox-pds \ - env-sandbox-sds \ + env-sandbox \ env-int \ env-int-pds \ env-int-sds \ From f04a48923e9319e1de8b5278e164a9deff387760 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Thu, 23 Apr 2026 23:41:35 +0100 Subject: [PATCH 64/69] Remove toggling of sandbox/int services --- scripts/env/app/provider.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/env/app/provider.sh b/scripts/env/app/provider.sh index 557997e5..800fb6a2 100755 --- a/scripts/env/app/provider.sh +++ b/scripts/env/app/provider.sh @@ -4,7 +4,7 @@ set -e get_provider_url() { env="$1" case "$env" in - int|int-pds|int-sds) + int) # TODO [GPCAPIM-397]: Update this. echo "stub" return 0 From f92a196e7e8c8a941ef2ce9fa10d934d4e20b398 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:19:01 +0100 Subject: [PATCH 65/69] Use [[ over [ --- scripts/env/app/pds.sh | 6 +++--- scripts/env/app/sds.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/env/app/pds.sh b/scripts/env/app/pds.sh index 147e7d27..8054b96f 100755 --- a/scripts/env/app/pds.sh +++ b/scripts/env/app/pds.sh @@ -24,7 +24,7 @@ get_pds_api_token() { secret_file=".secrets/pds/api_token" case "$env" in int) - if [ -f "$secret_file" ]; then + if [[ -f "$secret_file" ]]; then cat "$secret_file" return 0 else @@ -44,7 +44,7 @@ get_pds_api_secret() { secret_file=".secrets/pds/api_secret" case "$env" in int) - if [ -f "$secret_file" ]; then + if [[ -f "$secret_file" ]]; then cat "$secret_file" return 0 else @@ -64,7 +64,7 @@ get_pds_api_kid() { secret_file=".secrets/pds/api_kid" case "$env" in int) - if [ -f "$secret_file" ]; then + if [[ -f "$secret_file" ]]; then cat "$secret_file" return 0 else diff --git a/scripts/env/app/sds.sh b/scripts/env/app/sds.sh index a68cfdc0..0d4df765 100755 --- a/scripts/env/app/sds.sh +++ b/scripts/env/app/sds.sh @@ -24,7 +24,7 @@ get_sds_api_token() { secret_file=".secrets/sds/api_token" case "$env" in int) - if [ -f "$secret_file" ]; then + if [[ -f "$secret_file" ]]; then cat "$secret_file" return 0 else From 55c536269c9eb34a571996125b1e1f53faa3c8d5 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:21:24 +0100 Subject: [PATCH 66/69] Make pipeline fail. --- gateway-api/tests/integration/test_get_structured_record.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway-api/tests/integration/test_get_structured_record.py b/gateway-api/tests/integration/test_get_structured_record.py index c3324622..2316c68c 100644 --- a/gateway-api/tests/integration/test_get_structured_record.py +++ b/gateway-api/tests/integration/test_get_structured_record.py @@ -20,7 +20,7 @@ def test_happy_path_returns_200( response = client.send_to_get_structured_record_endpoint( json.dumps(simple_request_payload) ) - assert response.status_code == 200 + assert response.status_code == 500 def test_happy_path_returns_correct_message( self, From 67300cc7e6d95f8bd96316e2b33a68b792c5313e Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:22:58 +0100 Subject: [PATCH 67/69] Revert "Make pipeline fail." This reverts commit 55c536269c9eb34a571996125b1e1f53faa3c8d5. --- gateway-api/tests/integration/test_get_structured_record.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway-api/tests/integration/test_get_structured_record.py b/gateway-api/tests/integration/test_get_structured_record.py index 2316c68c..c3324622 100644 --- a/gateway-api/tests/integration/test_get_structured_record.py +++ b/gateway-api/tests/integration/test_get_structured_record.py @@ -20,7 +20,7 @@ def test_happy_path_returns_200( response = client.send_to_get_structured_record_endpoint( json.dumps(simple_request_payload) ) - assert response.status_code == 500 + assert response.status_code == 200 def test_happy_path_returns_correct_message( self, From 316efb0c21f3882c4da1d4ffef189f405ff18fd9 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 27 Apr 2026 09:44:38 +0100 Subject: [PATCH 68/69] update vocab. --- scripts/config/vale/styles/config/vocabularies/words/accept.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/config/vale/styles/config/vocabularies/words/accept.txt b/scripts/config/vale/styles/config/vocabularies/words/accept.txt index b7fcab2c..39633a18 100644 --- a/scripts/config/vale/styles/config/vocabularies/words/accept.txt +++ b/scripts/config/vale/styles/config/vocabularies/words/accept.txt @@ -13,6 +13,7 @@ coreutils customisability CVEs? Cyber +cybersecurity Dependabot [Dd]ev dotfiles From d48aec9bacfe2eb663398abc4b5fd1780677c825 Mon Sep 17 00:00:00 2001 From: David Hamill <109090521+davidhamill1-nhs@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:29:42 +0100 Subject: [PATCH 69/69] Don't log headers --- gateway-api/src/gateway_api/pds/client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/gateway-api/src/gateway_api/pds/client.py b/gateway-api/src/gateway_api/pds/client.py index 76db1664..b228c8bc 100644 --- a/gateway-api/src/gateway_api/pds/client.py +++ b/gateway-api/src/gateway_api/pds/client.py @@ -128,7 +128,6 @@ def search_patient_by_nhs_number( log_details = { "description": "PDS request", "url": url, - "headers": headers, } _logger.info(log_details) # This normally calls requests.get, but if PDS_URL is set it uses the stub.