diff --git a/.github/actions/run-test-suite/action.yaml b/.github/actions/run-test-suite/action.yaml index d84e9ae5..a1eac393 100644 --- a/.github/actions/run-test-suite/action.yaml +++ b/.github/actions/run-test-suite/action.yaml @@ -17,6 +17,26 @@ inputs: description: "Environment: local or remote" required: false default: "remote" + apim_client_key_secret_name: + description: "Secret name of the APIM client certificate key (if needed)" + required: false + default: "" + apim_client_cert_secret_name: + description: "Secret name of the APIM client certificate (if needed)" + required: false + default: "" + pdm_mock_document_url: + description: "PDM mock document URL (if needed)" + required: false + default: "" + mns_mock_events_url: + description: "MNS mock events URL (if needed)" + required: false + default: "" + pdm_bundle_url: + description: "PDM bundle URL (if needed)" + required: false + default: "" runs: using: composite @@ -27,11 +47,31 @@ runs: APIGEE_ACCESS_TOKEN: ${{ inputs.apigee-access-token }} ENV: ${{ inputs.env }} TEST_TYPE: ${{ inputs.test-type }} + CLIENT_KEY_NAME: ${{ inputs.apim_client_key_secret_name }} + CLIENT_CERT_NAME: ${{ inputs.apim_client_cert_secret_name }} + PDM_MOCK_DOCUMENT_URL: ${{ inputs.pdm_mock_document_url }} + MNS_MOCK_EVENTS_URL: ${{ inputs.mns_mock_events_url }} + PDM_BUNDLE_URL: ${{ inputs.pdm_bundle_url }} run: | if [[ -n "${APIGEE_ACCESS_TOKEN}" ]]; then echo "::add-mask::${APIGEE_ACCESS_TOKEN}" fi - make test-${TEST_TYPE} + + if [[ -n "${CLIENT_KEY_NAME}" ]]; then + SECRET_NAME=${CLIENT_KEY_NAME//\//_} + SECRET_NAME=${SECRET_NAME//-/_} + echo "Using APIM client certificate key from name: ${SECRET_NAME}" + CLIENT_KEY=${!SECRET_NAME} + fi + + if [[ -n "${CLIENT_CERT_NAME}" ]]; then + SECRET_NAME=${CLIENT_CERT_NAME//\//_} + SECRET_NAME=${SECRET_NAME//-/_} + echo "Using APIM client certificate from name: ${SECRET_NAME}" + CLIENT_CERT=${!SECRET_NAME} + fi + + CLIENT_KEY=${CLIENT_KEY} CLIENT_CERT=${CLIENT_CERT} make test-${TEST_TYPE} - name: "Upload ${{ inputs.test-type }} test results" if: always() diff --git a/.github/workflows/preview-env.yaml b/.github/workflows/preview-env.yaml index 0af97e48..f8e634ed 100644 --- a/.github/workflows/preview-env.yaml +++ b/.github/workflows/preview-env.yaml @@ -685,9 +685,17 @@ jobs: - name: "Run integration tests" if: github.event.action != 'closed' uses: ./.github/actions/run-test-suite + env: + API_MTLS_CERT: ${{ secrets.API_MTLS_CERT }} + API_MTLS_KEY: ${{ secrets.API_MTLS_KEY }} with: test-type: integration apigee-access-token: ${{ steps.apigee-token.outputs.apigee-access-token }} + apim_client_cert_secret_name: "${{ env.API_MTLS_CERT || '/cds/pathology/dev/mtls/client1-key-public' }}" + apim_client_key_secret_name: "${{ env.API_MTLS_KEY || '/cds/pathology/dev/mtls/client1-key-secret' }}" + pdm_mock_document_url: ${{ steps.names.outputs.mock_preview_url }}/pdm/mock/Bundle + mns_mock_events_url: ${{ steps.names.outputs.mock_preview_url }}/mns/mock/event + pdm_bundle_url: ${{ steps.names.outputs.mock_preview_url }}/pdm/FHIR/R4/Bundle - name: "Run acceptance tests" if: github.event.action != 'closed' diff --git a/infrastructure/images/api-gateway-mock/resources/server.py b/infrastructure/images/api-gateway-mock/resources/server.py index f6918366..cae77f10 100644 --- a/infrastructure/images/api-gateway-mock/resources/server.py +++ b/infrastructure/images/api-gateway-mock/resources/server.py @@ -44,7 +44,7 @@ def forward_request(path_params): x_correlation_id = request.headers.get("X-Correlation-ID", "") forwarded_headers = {k.lower(): v for k, v in request.headers.items()} - forwarded_headers["nhsd-correlation-id"] = x_correlation_id + forwarded_headers["nhsd-correlation-id"] = f".{x_correlation_id}.test" response = requests.post( f"{TARGET_URL}/2015-03-31/functions/function/invocations", @@ -61,8 +61,9 @@ def forward_request(path_params): }, "httpMethod": request.method, "rawPath": f"/{path_params}", - "rawQueryString": "", + "rawQueryString": request.query_string.decode("utf-8"), "pathParameters": {"proxy": path_params}, + "queryStringParameters": request.args.to_dict(), }, headers={"Content-Type": "application/json"}, timeout=120, @@ -75,11 +76,13 @@ def forward_request(path_params): app.logger.info("response: %s", response.text) response_data = response.json() + headers = {"x-correlation-id": x_correlation_id} | response_data.get("headers", {}) + output = ( ( response_data["body"], response_data["statusCode"], - response_data["headers"], + headers, ) if "body" in response_data else (response_data, 500, {"Content-Type": "text/plain"}) diff --git a/mocks/src/pdm_mock/handler.py b/mocks/src/pdm_mock/handler.py index b14945f3..52be4cdf 100644 --- a/mocks/src/pdm_mock/handler.py +++ b/mocks/src/pdm_mock/handler.py @@ -114,7 +114,7 @@ def handle_post_request(payload: dict[str, Any]) -> PDMResponse: item: DocumentItem = { "sessionId": document_id, "expiresAt": int(time()) + 600, - "document": json.dumps(created_document), + "document": json.dumps(payload), "type": "pdm_document", } diff --git a/pathology-api/openapi.yaml b/pathology-api/openapi.yaml index fc3347fc..57f59926 100644 --- a/pathology-api/openapi.yaml +++ b/pathology-api/openapi.yaml @@ -234,10 +234,10 @@ paths: identifier: system: "https://fhir.nhs.uk/Id/nhs-number" value: "9999999999" - extension: - - url: "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition" - valueReference: - reference: "ServiceRequest" + extension: + - url: "http://hl7.eu/fhir/StructureDefinition/composition-basedOn-order-or-requisition" + valueReference: + reference: "ServiceRequest" - fullUrl: "ServiceRequest" resource: resourceType: ServiceRequest @@ -252,8 +252,8 @@ paths: resource: resourceType: Organization identifier: - system: "https://fhir.nhs.uk/Id/ods-organization-code" - value: "A12345" + - system: "https://fhir.nhs.uk/Id/ods-organization-code" + value: "A12345" - fullUrl: "Patient" resource: resourceType: Patient @@ -367,8 +367,8 @@ paths: resource: resourceType: Organization identifier: - system: "https://fhir.nhs.uk/Id/ods-organization-code" - value: "A12345" + - system: "https://fhir.nhs.uk/Id/ods-organization-code" + value: "A12345" - fullUrl: "Patient" resource: resourceType: Patient @@ -567,20 +567,23 @@ components: - Organization example: Organization identifier: - type: object - required: - - system - - value - properties: - system: - type: string - enum: - - "https://fhir.nhs.uk/Id/ods-organization-code" - example: "https://fhir.nhs.uk/Id/ods-organization-code" - value: - type: string - example: "A12345" - description: ODS code of the requesting organisation + type: array + minItems: 1 + items: + type: object + required: + - system + - value + properties: + system: + type: string + enum: + - "https://fhir.nhs.uk/Id/ods-organization-code" + example: "https://fhir.nhs.uk/Id/ods-organization-code" + value: + type: string + example: "A12345" + description: ODS code of the requesting organisation OperationOutcome: type: object required: diff --git a/pathology-api/tests/conftest.py b/pathology-api/tests/conftest.py index 3fde7e72..99833973 100644 --- a/pathology-api/tests/conftest.py +++ b/pathology-api/tests/conftest.py @@ -1,8 +1,9 @@ """Pytest configuration and shared fixtures for pathology API tests.""" import os +from collections.abc import Callable from datetime import timedelta -from typing import Any, Literal, Protocol, cast +from typing import Any, Literal, Protocol import pytest import requests @@ -58,7 +59,7 @@ def __init__( self, lambda_url: str, headers: dict[str, str] | None = None, - timeout: timedelta = timedelta(seconds=1), + timeout: timedelta = timedelta(seconds=10), ): self._lambda_url = lambda_url self._default_headers = {"Content-Type": "application/fhir+json"} | ( @@ -209,15 +210,32 @@ def _send( @pytest.fixture(scope="module") -def base_url() -> str: +def fetch_env_variable[T]() -> Callable[[str, type[T]], T]: + def _fetch_env_variable(name: str, required_type: type[T]) -> T: + value = os.getenv(name) + if not value: + raise ValueError(f"{name} environment variable is not set.") + + if not isinstance(value, required_type): + raise ValueError( + f"{name} environment variable is not required type {required_type}" + ) + + return value + + return _fetch_env_variable + + +@pytest.fixture(scope="module") +def base_url(fetch_env_variable: Callable[[str, type[str]], str]) -> str: """Retrieves the base URL of the currently deployed application.""" - return _fetch_env_variable("BASE_URL", str) + return fetch_env_variable("BASE_URL", str) @pytest.fixture -def hostname() -> str: +def hostname(fetch_env_variable: Callable[[str, type[str]], str]) -> str: """Retrieves the hostname of the currently deployed application.""" - return _fetch_env_variable("HOST", str) + return fetch_env_variable("HOST", str) @pytest.fixture @@ -280,13 +298,6 @@ def _create_remote_client(request: pytest.FixtureRequest) -> RemoteClient: ) -def _fetch_env_variable[T](name: str, _: type[T]) -> T: - value = os.getenv(name) - if not value: - raise ValueError(f"{name} environment variable is not set.") - return cast("T", value) - - def pytest_addoption(parser: pytest.Parser) -> None: parser.addoption( "--env", diff --git a/pathology-api/tests/integration/conftest.py b/pathology-api/tests/integration/conftest.py new file mode 100644 index 00000000..2b51b8b4 --- /dev/null +++ b/pathology-api/tests/integration/conftest.py @@ -0,0 +1,67 @@ +import os +import tempfile +from collections.abc import Callable, Generator +from datetime import timedelta + +import pytest + +from tests.mock_client import CertificateDetails, MNSMockClient, PDMMockClient + + +@pytest.fixture(scope="module") +def client_cert() -> Generator[CertificateDetails | None, None, None]: + client_cert = os.getenv("CLIENT_CERT") + client_key = os.getenv("CLIENT_KEY") + + if client_cert and client_key: + with ( + tempfile.NamedTemporaryFile(delete=True) as cert_file, + tempfile.NamedTemporaryFile(delete=True) as key_file, + ): + cert_file.write(client_cert.encode()) + cert_file.flush() + key_file.write(client_key.encode()) + key_file.flush() + yield { + "cert_path": cert_file.name, + "key_path": key_file.name, + } + else: + yield None + + +@pytest.fixture(scope="module") +def pdm_mock_document_url(fetch_env_variable: Callable[[str, type[str]], str]) -> str: + return fetch_env_variable("PDM_MOCK_DOCUMENT_URL", str) + + +@pytest.fixture(scope="module") +def mns_mock_events_url(fetch_env_variable: Callable[[str, type[str]], str]) -> str: + return fetch_env_variable("MNS_MOCK_EVENTS_URL", str) + + +@pytest.fixture(scope="module") +def pdm_bundle_url(fetch_env_variable: Callable[[str, type[str]], str]) -> str: + return fetch_env_variable("PDM_BUNDLE_URL", str) + + +@pytest.fixture(scope="module") +def pdm_mock_client( + client_cert: CertificateDetails | None, pdm_mock_document_url: str +) -> PDMMockClient: + return PDMMockClient( + document_url=pdm_mock_document_url, + timeout=timedelta(seconds=5), + client_cert=client_cert, + ) + + +@pytest.fixture(scope="module") +def mns_mock_client( + client_cert: CertificateDetails | None, mns_mock_events_url: str +) -> MNSMockClient: + return MNSMockClient( + events_url=mns_mock_events_url, + timeout=timedelta(seconds=5), + client_cert=client_cert, + ) diff --git a/pathology-api/tests/integration/test_endpoints.py b/pathology-api/tests/integration/test_endpoints.py index 66141796..08d4c4c6 100644 --- a/pathology-api/tests/integration/test_endpoints.py +++ b/pathology-api/tests/integration/test_endpoints.py @@ -1,6 +1,7 @@ """Integration tests for the pathology API using pytest.""" import json +import uuid from collections.abc import Callable from typing import Any, Literal @@ -12,13 +13,21 @@ from pydantic import BaseModel, HttpUrl from tests.conftest import Client +from tests.mock_client import MNSMockClient, PDMMockClient class TestBundleEndpoint: def test_bundle_returns_200( - self, client: Client, build_valid_test_result: Callable[[str, str], Bundle] + self, + client: Client, + build_valid_test_result: Callable[[str, str], Bundle], + pdm_mock_client: PDMMockClient, + mns_mock_client: MNSMockClient, + pdm_bundle_url: str, ) -> None: - bundle = build_valid_test_result("nhs_number", "ods_code") + subject = "subject-" + str(uuid.uuid4()) + requesting_ods_code = "ods_code" + bundle = build_valid_test_result(subject, requesting_ods_code) response = client.send( data=bundle.model_dump_json(by_alias=True), @@ -53,6 +62,25 @@ def test_bundle_returns_200( assert response.headers["etag"] == 'W/"1"' + sent_request = pdm_mock_client.retrieve_sent_request(response_bundle.id) + assert sent_request == bundle.model_dump(by_alias=True, exclude_none=True) + + published_events = mns_mock_client.retrieve_sent_messages(subject) + assert len(published_events) == 1 + + published_event = published_events[0] + assert published_event["subject"] == subject + assert published_event["dataref"] == pdm_bundle_url + "/" + response_bundle.id + assert published_event["filtering"] == { + "requestingOrganisationODS": requesting_ods_code + } + assert ( + published_event["type"] + == "pathology-laboratory-reporting-test-result-stored-1" + ) + assert published_event["source"] == "uk.nhs.pathology-laboratory-reporting" + assert published_event["specversion"] == "1.0" + def test_no_payload_returns_error(self, client: Client) -> None: response = client.send_without_payload( request_method="POST", path="FHIR/R4/Bundle" diff --git a/pathology-api/tests/mock_client.py b/pathology-api/tests/mock_client.py new file mode 100644 index 00000000..743c24b3 --- /dev/null +++ b/pathology-api/tests/mock_client.py @@ -0,0 +1,61 @@ +from datetime import timedelta +from typing import Any, TypedDict, cast + +import requests + + +class CertificateDetails(TypedDict): + cert_path: str + key_path: str + + +class PDMMockClient: + def __init__( + self, + document_url: str, + timeout: timedelta, + client_cert: CertificateDetails | None, + ): + self._document_url = document_url + self._timeout = timeout + self._client_cert = client_cert + + def retrieve_sent_request(self, request_id: str) -> Any: + certs = ( + (self._client_cert["cert_path"], self._client_cert["key_path"]) + if self._client_cert + else None + ) + + response = requests.get( + self._document_url + "/" + request_id, + timeout=self._timeout.total_seconds(), + cert=certs, + ) + return response.json() + + +class MNSMockClient: + def __init__( + self, + events_url: str, + timeout: timedelta, + client_cert: CertificateDetails | None, + ): + self._events_url = events_url + self._timeout = timeout + self._client_cert = client_cert + + def retrieve_sent_messages(self, subject: str) -> list[Any]: + certs = ( + (self._client_cert["cert_path"], self._client_cert["key_path"]) + if self._client_cert + else None + ) + + response = requests.get( + self._events_url + "?subject=" + subject, + timeout=self._timeout.total_seconds(), + cert=certs, + ) + return cast("list[Any]", response.json().get("events", [])) diff --git a/scripts/fetch_secret.sh b/scripts/fetch_secret.sh new file mode 100755 index 00000000..485205de --- /dev/null +++ b/scripts/fetch_secret.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash +set -euo pipefail + +secretName="$1" + +echo "Retrieving secret from AWS Secrets Manager: $secretName ..." >&2 + +SECRET_VALUE=$(aws secretsmanager get-secret-value --secret-id "$secretName" --query 'SecretString' --output text) +echo "${SECRET_VALUE}" diff --git a/scripts/tests/test.mk b/scripts/tests/test.mk index c53b0cf6..b5046954 100644 --- a/scripts/tests/test.mk +++ b/scripts/tests/test.mk @@ -138,8 +138,9 @@ env-remote: # Run tests against local lambda test-local: env-local + @echo "Running test stage: $${stage:-all}" @set -a && source .env && set +a && \ - $(MAKE) test + $(MAKE) test$(if $(stage),-$(stage),) # Run tests against remote lambda, exporting APIGEE_ACCESS_TOKEN only test-remote: env-remote @@ -147,6 +148,8 @@ test-remote: env-remote @echo "Obtaining APIGEE access token..." @set -a && source .env && set +a && \ APIGEE_ACCESS_TOKEN="$$(./scripts/get_apigee_token.sh)" && \ + CLIENT_CERT="$$(./scripts/fetch_secret.sh "$$APIM_CLIENT_CERT_SECRET_NAME")" && \ + CLIENT_KEY="$$(./scripts/fetch_secret.sh "$$APIM_CLIENT_KEY_SECRET_NAME")" && \ BASE_URL="$${BASE_URL}-pr-$${PR_NUMBER}" && \ - export APIGEE_ACCESS_TOKEN BASE_URL && \ + export APIGEE_ACCESS_TOKEN CLIENT_CERT CLIENT_KEY BASE_URL && \ $(MAKE) test$(if $(stage),-$(stage),)