From 2158076ceaeddd09b0fec392b36fcd2b75af7092 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Fri, 17 Apr 2026 12:51:44 +0100 Subject: [PATCH 01/18] feat: Introduce Mock PDS Lambda for MNS integration testing - Added a new Lambda function to simulate PDS responses for patient lookups. - Implemented rate limiting with configurable average and spike thresholds. - Updated existing MNS publisher to utilize the new PDS base URL environment variable. - Enhanced tests to cover scenarios with the mock PDS service, including rate limit handling. - Created performance testing scripts to validate MNS behavior under load with the mock PDS. - Updated infrastructure to route PDS calls in the ref environment to the mock endpoint. --- .../environments/dev/ref/variables.tfvars | 3 + infrastructure/instance/id_sync_lambda.tf | 1 + infrastructure/instance/mns_publisher.tf | 1 + infrastructure/instance/mock_pds.tf | 240 ++++++++++++++++++ .../mns_publisher/mns_publisher_lambda.tf | 1 + .../modules/mns_publisher/variables.tf | 6 + infrastructure/instance/variables.tf | 36 +++ .../tests/test_lambda_handler.py | 51 ++++ lambdas/mock_pds/Dockerfile | 23 ++ lambdas/mock_pds/README.md | 18 ++ lambdas/mock_pds/poetry.lock | 39 +++ lambdas/mock_pds/pyproject.toml | 15 ++ lambdas/mock_pds/src/lambda_handler.py | 49 ++++ lambdas/mock_pds/src/mock_pds_service.py | 115 +++++++++ lambdas/mock_pds/src/rate_limiter.py | 64 +++++ .../mock_pds/tests/test_mock_pds_service.py | 92 +++++++ .../src/common/api_clients/get_pds_details.py | 27 +- .../src/common/api_clients/pds_service.py | 27 +- .../api_clients/test_pds_details.py | 13 + .../api_clients/test_pds_service.py | 17 ++ mns_perf_with_mocked_pds_4bb74b98.plan.md | 112 ++++++++ tests/perf_tests/Makefile | 21 +- tests/perf_tests/Readme.md | 29 +++ tests/perf_tests/src/locustfile.py | 49 +++- 24 files changed, 1033 insertions(+), 16 deletions(-) create mode 100644 infrastructure/instance/mock_pds.tf create mode 100644 lambdas/mock_pds/Dockerfile create mode 100644 lambdas/mock_pds/README.md create mode 100644 lambdas/mock_pds/poetry.lock create mode 100644 lambdas/mock_pds/pyproject.toml create mode 100644 lambdas/mock_pds/src/lambda_handler.py create mode 100644 lambdas/mock_pds/src/mock_pds_service.py create mode 100644 lambdas/mock_pds/src/rate_limiter.py create mode 100644 lambdas/mock_pds/tests/test_mock_pds_service.py create mode 100644 mns_perf_with_mocked_pds_4bb74b98.plan.md diff --git a/infrastructure/instance/environments/dev/ref/variables.tfvars b/infrastructure/instance/environments/dev/ref/variables.tfvars index e6256cc114..16ab05b6e4 100644 --- a/infrastructure/instance/environments/dev/ref/variables.tfvars +++ b/infrastructure/instance/environments/dev/ref/variables.tfvars @@ -3,6 +3,9 @@ immunisation_account_id = "345594581768" dspp_core_account_id = "603871901111" pds_environment = "ref" mns_environment = "dev" +mock_pds_enabled = true +mock_pds_average_rate_limit = 125 +mock_pds_spike_rate_limit = 450 error_alarm_notifications_enabled = true create_mesh_processor = false has_sub_environment_scope = true diff --git a/infrastructure/instance/id_sync_lambda.tf b/infrastructure/instance/id_sync_lambda.tf index 322448532a..885fc806a6 100644 --- a/infrastructure/instance/id_sync_lambda.tf +++ b/infrastructure/instance/id_sync_lambda.tf @@ -290,6 +290,7 @@ resource "aws_lambda_function" "id_sync_lambda" { variables = { IEDS_TABLE_NAME = aws_dynamodb_table.events-dynamodb-table.name PDS_ENV = var.pds_environment + PDS_BASE_URL = local.mock_pds_base_url SPLUNK_FIREHOSE_NAME = module.splunk.firehose_stream_name } } diff --git a/infrastructure/instance/mns_publisher.tf b/infrastructure/instance/mns_publisher.tf index d094919885..51b20394f7 100644 --- a/infrastructure/instance/mns_publisher.tf +++ b/infrastructure/instance/mns_publisher.tf @@ -15,6 +15,7 @@ module "mns_publisher" { secrets_manager_policy_path = "${local.policy_path}/secret_manager.json" account_id = data.aws_caller_identity.current.account_id pds_environment = var.pds_environment + pds_base_url = local.mock_pds_base_url mns_environment = var.mns_environment private_subnet_ids = local.private_subnet_ids diff --git a/infrastructure/instance/mock_pds.tf b/infrastructure/instance/mock_pds.tf new file mode 100644 index 0000000000..c43083410f --- /dev/null +++ b/infrastructure/instance/mock_pds.tf @@ -0,0 +1,240 @@ +locals { + mock_pds_lambda_dir = abspath("${path.root}/../../lambdas/mock_pds") + mock_pds_lambda_files = fileset(local.mock_pds_lambda_dir, "**") + mock_pds_lambda_dir_sha = sha1(join("", [for f in local.mock_pds_lambda_files : filesha1("${local.mock_pds_lambda_dir}/${f}")])) + mock_pds_lambda_name = "${local.short_prefix}-mock-pds-lambda" + mock_pds_base_url = var.mock_pds_enabled ? "${aws_lambda_function_url.mock_pds_lambda_url[0].function_url}Patient" : "" +} + +resource "aws_ecr_repository" "mock_pds_lambda_repository" { + count = var.mock_pds_enabled ? 1 : 0 + + image_scanning_configuration { + scan_on_push = true + } + + name = "${local.short_prefix}-mock-pds-repo" + force_delete = local.is_temp +} + +module "mock_pds_docker_image" { + count = var.mock_pds_enabled ? 1 : 0 + + source = "terraform-aws-modules/lambda/aws//modules/docker-build" + version = "8.7.0" + docker_file_path = "./mock_pds/Dockerfile" + create_ecr_repo = false + ecr_repo = aws_ecr_repository.mock_pds_lambda_repository[0].name + ecr_repo_lifecycle_policy = jsonencode({ + "rules" : [ + { + "rulePriority" : 1, + "description" : "Keep only the last 2 images", + "selection" : { + "tagStatus" : "any", + "countType" : "imageCountMoreThan", + "countNumber" : 2 + }, + "action" : { + "type" : "expire" + } + } + ] + }) + + platform = "linux/amd64" + use_image_tag = false + source_path = abspath("${path.root}/../../lambdas") + triggers = { + dir_sha = local.mock_pds_lambda_dir_sha + } +} + +resource "aws_ecr_repository_policy" "mock_pds_lambda_ecr_image_retrieval_policy" { + count = var.mock_pds_enabled ? 1 : 0 + + repository = aws_ecr_repository.mock_pds_lambda_repository[0].name + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + "Sid" : "LambdaECRImageRetrievalPolicy", + "Effect" : "Allow", + "Principal" : { + "Service" : "lambda.amazonaws.com" + }, + "Action" : [ + "ecr:BatchGetImage", + "ecr:DeleteRepositoryPolicy", + "ecr:GetDownloadUrlForLayer", + "ecr:GetRepositoryPolicy", + "ecr:SetRepositoryPolicy" + ], + "Condition" : { + "StringLike" : { + "aws:sourceArn" : "arn:aws:lambda:${var.aws_region}:${var.immunisation_account_id}:function:${local.mock_pds_lambda_name}" + } + } + } + ] + }) +} + +resource "aws_iam_role" "mock_pds_lambda_exec_role" { + count = var.mock_pds_enabled ? 1 : 0 + + name = "${local.mock_pds_lambda_name}-exec-role" + assume_role_policy = jsonencode({ + Version = "2012-10-17", + Statement = [{ + Effect = "Allow", + Sid = "", + Principal = { + Service = "lambda.amazonaws.com" + }, + Action = "sts:AssumeRole" + }] + }) +} + +resource "aws_iam_policy" "mock_pds_lambda_exec_policy" { + count = var.mock_pds_enabled ? 1 : 0 + + name = "${local.mock_pds_lambda_name}-exec-policy" + policy = jsonencode({ + Version = "2012-10-17", + Statement = [ + { + Effect = "Allow" + Action = [ + "logs:CreateLogGroup", + "logs:CreateLogStream", + "logs:PutLogEvents" + ] + Resource = "arn:aws:logs:${var.aws_region}:${var.immunisation_account_id}:log-group:/aws/lambda/${local.mock_pds_lambda_name}:*" + }, + { + Effect = "Allow", + Action = [ + "ec2:CreateNetworkInterface", + "ec2:DescribeNetworkInterfaces", + "ec2:DeleteNetworkInterface" + ], + Resource = "*" + } + ] + }) +} + +resource "aws_iam_policy" "mock_pds_lambda_kms_access_policy" { + count = var.mock_pds_enabled ? 1 : 0 + + name = "${local.mock_pds_lambda_name}-kms-policy" + description = "Allow mock PDS Lambda to decrypt environment variables" + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Effect = "Allow" + Action = [ + "kms:Decrypt" + ] + Resource = data.aws_kms_key.existing_lambda_encryption_key.arn + } + ] + }) +} + +resource "aws_iam_role_policy_attachment" "mock_pds_lambda_exec_policy_attachment" { + count = var.mock_pds_enabled ? 1 : 0 + + role = aws_iam_role.mock_pds_lambda_exec_role[0].name + policy_arn = aws_iam_policy.mock_pds_lambda_exec_policy[0].arn +} + +resource "aws_iam_role_policy_attachment" "mock_pds_lambda_kms_policy_attachment" { + count = var.mock_pds_enabled ? 1 : 0 + + role = aws_iam_role.mock_pds_lambda_exec_role[0].name + policy_arn = aws_iam_policy.mock_pds_lambda_kms_access_policy[0].arn +} + +resource "aws_cloudwatch_log_group" "mock_pds_lambda_log_group" { + count = var.mock_pds_enabled ? 1 : 0 + + name = "/aws/lambda/${local.mock_pds_lambda_name}" + retention_in_days = 30 +} + +resource "aws_lambda_function" "mock_pds_lambda" { + count = var.mock_pds_enabled ? 1 : 0 + + function_name = local.mock_pds_lambda_name + role = aws_iam_role.mock_pds_lambda_exec_role[0].arn + package_type = "Image" + image_uri = module.mock_pds_docker_image[0].image_uri + architectures = ["x86_64"] + timeout = 30 + + vpc_config { + subnet_ids = local.private_subnet_ids + security_group_ids = [data.aws_security_group.existing_securitygroup.id] + } + + environment { + variables = { + REDIS_HOST = data.aws_elasticache_cluster.existing_redis.cache_nodes[0].address + REDIS_PORT = tostring(data.aws_elasticache_cluster.existing_redis.port) + MOCK_PDS_AVERAGE_LIMIT = tostring(var.mock_pds_average_rate_limit) + MOCK_PDS_AVERAGE_WINDOW_SECONDS = tostring(var.mock_pds_average_window_seconds) + MOCK_PDS_SPIKE_LIMIT = tostring(var.mock_pds_spike_rate_limit) + MOCK_PDS_SPIKE_WINDOW_SECONDS = tostring(var.mock_pds_spike_window_seconds) + MOCK_PDS_GP_ODS_CODE = var.mock_pds_gp_ods_code + } + } + + kms_key_arn = data.aws_kms_key.existing_lambda_encryption_key.arn + + depends_on = [ + aws_cloudwatch_log_group.mock_pds_lambda_log_group, + aws_iam_policy.mock_pds_lambda_exec_policy + ] +} + +resource "aws_lambda_function_url" "mock_pds_lambda_url" { + count = var.mock_pds_enabled ? 1 : 0 + + function_name = aws_lambda_function.mock_pds_lambda[0].function_name + authorization_type = "NONE" +} + +resource "aws_lambda_permission" "mock_pds_lambda_url_invoke" { + count = var.mock_pds_enabled ? 1 : 0 + + statement_id = "AllowPublicInvokeFunctionUrl" + action = "lambda:InvokeFunctionUrl" + function_name = aws_lambda_function.mock_pds_lambda[0].function_name + principal = "*" + function_url_auth_type = "NONE" +} + +resource "aws_cloudwatch_log_metric_filter" "mock_pds_throttle_logs" { + count = var.mock_pds_enabled ? 1 : 0 + + name = "${local.short_prefix}-MockPdsThrottleLogs" + pattern = "Mock PDS rate limit exceeded" + log_group_name = aws_cloudwatch_log_group.mock_pds_lambda_log_group[0].name + + metric_transformation { + name = "${local.short_prefix}-MockPdsThrottleRequests" + namespace = "${local.short_prefix}-MockPds" + value = "1" + } +} + +output "mock_pds_function_url" { + value = var.mock_pds_enabled ? aws_lambda_function_url.mock_pds_lambda_url[0].function_url : null + description = "Function URL for the mock PDS endpoint." +} \ No newline at end of file diff --git a/infrastructure/instance/modules/mns_publisher/mns_publisher_lambda.tf b/infrastructure/instance/modules/mns_publisher/mns_publisher_lambda.tf index 3c3c127f24..1263ef4ad1 100644 --- a/infrastructure/instance/modules/mns_publisher/mns_publisher_lambda.tf +++ b/infrastructure/instance/modules/mns_publisher/mns_publisher_lambda.tf @@ -196,6 +196,7 @@ resource "aws_lambda_function" "mns_publisher_lambda" { IMMUNIZATION_ENV = var.resource_scope, IMMUNIZATION_BASE_PATH = var.imms_base_path PDS_ENV = var.pds_environment + PDS_BASE_URL = var.pds_base_url MNS_ENV = var.mns_environment } } diff --git a/infrastructure/instance/modules/mns_publisher/variables.tf b/infrastructure/instance/modules/mns_publisher/variables.tf index 3857b9b13f..d5229e9f03 100644 --- a/infrastructure/instance/modules/mns_publisher/variables.tf +++ b/infrastructure/instance/modules/mns_publisher/variables.tf @@ -94,6 +94,12 @@ variable "pds_environment" { type = string } +variable "pds_base_url" { + type = string + default = "" + description = "Optional override for the PDS base URL, used by ref to route to the mock PDS endpoint." +} + variable "account_id" { type = string description = "AWS account ID used for IAM policy templating (e.g., Secrets Manager ARNs)." diff --git a/infrastructure/instance/variables.tf b/infrastructure/instance/variables.tf index 39e7b7e07a..295d79b8cb 100644 --- a/infrastructure/instance/variables.tf +++ b/infrastructure/instance/variables.tf @@ -79,6 +79,42 @@ variable "mns_environment" { default = "int" } +variable "mock_pds_enabled" { + description = "Enable the ref-only mock PDS endpoint and route PDS consumers to it." + type = bool + default = false +} + +variable "mock_pds_average_rate_limit" { + description = "Average mock PDS request rate, in requests per second." + type = number + default = 125 +} + +variable "mock_pds_average_window_seconds" { + description = "Average mock PDS rate limiting window in seconds." + type = number + default = 60 +} + +variable "mock_pds_spike_rate_limit" { + description = "Spike mock PDS request rate, in requests per second." + type = number + default = 450 +} + +variable "mock_pds_spike_window_seconds" { + description = "Spike mock PDS rate limiting window in seconds." + type = number + default = 1 +} + +variable "mock_pds_gp_ods_code" { + description = "Deterministic GP ODS code returned by the mock PDS service." + type = string + default = "Y12345" +} + variable "mesh_no_invocation_period_seconds" { description = "The maximum duration the MESH Processor Lambda can go without being invoked before the no-invocation alarm is triggered." type = number diff --git a/lambdas/mns_publisher/tests/test_lambda_handler.py b/lambdas/mns_publisher/tests/test_lambda_handler.py index 2c45682f14..373ceaeed5 100644 --- a/lambdas/mns_publisher/tests/test_lambda_handler.py +++ b/lambdas/mns_publisher/tests/test_lambda_handler.py @@ -6,6 +6,7 @@ import responses from moto import mock_aws +import common.api_clients.get_pds_details as get_pds_details_module from lambda_handler import lambda_handler from process_records import extract_trace_ids, process_record, process_records from test_utils import generate_private_key_b64, load_sample_sqs_event @@ -246,6 +247,7 @@ class TestLambdaHandlerIntegration(unittest.TestCase): def setUp(self): """Set up mocked AWS services and test data.""" self.sample_sqs_record = load_sample_sqs_event() + get_pds_details_module._pds_service = None self.secrets_client = boto3.client("secretsmanager", region_name="eu-west-2") self.secrets_client.create_secret( Name="imms/pds/int/jwt-secrets", @@ -254,6 +256,9 @@ def setUp(self): ), ) + def tearDown(self): + get_pds_details_module._pds_service = None + @responses.activate @patch("common.api_clients.authentication.AppRestrictedAuth.get_access_token") @patch("process_records.logger") @@ -379,3 +384,49 @@ def test_successful_notification_creation_with_expired_gp(self, mock_logger, moc self.assertEqual(mns_payload["filtering"]["subjectage"], 21) mock_logger.info.assert_any_call("Successfully processed all 1 messages") + + @responses.activate + @patch.dict("os.environ", {"PDS_BASE_URL": "https://mock-pds.example/Patient"}, clear=False) + @patch("process_records._get_runtime_mns_service") + @patch("process_records.logger") + def test_successful_notification_creation_with_mock_pds_base_url(self, mock_logger, mock_get_mns): + responses.add( + responses.GET, + "https://mock-pds.example/Patient/9481152782", + json={"generalPractitioner": [{"identifier": {"value": "Y12345", "period": {"start": "2024-01-01"}}}]}, + status=200, + ) + + mock_mns_service = Mock() + mock_get_mns.return_value = mock_mns_service + + sqs_event = {"Records": [self.sample_sqs_record]} + result = lambda_handler(sqs_event, Mock()) + + self.assertEqual(result, {"batchItemFailures": []}) + mock_mns_service.publish_notification.assert_called_once() + mns_payload = mock_mns_service.publish_notification.call_args.args[0] + self.assertEqual(mns_payload["filtering"]["generalpractitioner"], "Y12345") + mock_logger.info.assert_any_call("Successfully processed all 1 messages") + + @responses.activate + @patch.dict("os.environ", {"PDS_BASE_URL": "https://mock-pds.example/Patient"}, clear=False) + @patch("process_records._get_runtime_mns_service") + @patch("process_records.logger") + def test_mock_pds_rate_limit_results_in_batch_failure(self, mock_logger, mock_get_mns): + responses.add( + responses.GET, + "https://mock-pds.example/Patient/9481152782", + json={"code": 429, "message": "Mock PDS rate limit has been exceeded"}, + status=429, + ) + + mock_mns_service = Mock() + mock_get_mns.return_value = mock_mns_service + + sqs_event = {"Records": [self.sample_sqs_record]} + result = lambda_handler(sqs_event, Mock()) + + self.assertEqual(len(result["batchItemFailures"]), 1) + mock_mns_service.publish_notification.assert_not_called() + mock_logger.warning.assert_called_with("Batch completed with 1 failures") diff --git a/lambdas/mock_pds/Dockerfile b/lambdas/mock_pds/Dockerfile new file mode 100644 index 0000000000..6e7a092428 --- /dev/null +++ b/lambdas/mock_pds/Dockerfile @@ -0,0 +1,23 @@ +FROM public.ecr.aws/lambda/python:3.11 AS base + +RUN mkdir -p /home/appuser && \ + echo 'appuser:x:1001:1001::/home/appuser:/sbin/nologin' >> /etc/passwd && \ + echo 'appuser:x:1001:' >> /etc/group && \ + chown -R 1001:1001 /home/appuser && pip install "poetry~=2.1.4" + +COPY ./mock_pds/poetry.lock ./mock_pds/pyproject.toml ./ + +WORKDIR /var/task +RUN poetry config virtualenvs.create false && poetry install --no-interaction --no-ansi --no-root --only main + +FROM base AS build + +WORKDIR /var/task + +COPY ./mock_pds/src . + +RUN chmod 644 $(find . -type f) && chmod 755 $(find . -type d) + +USER 1001:1001 + +CMD ["lambda_handler.lambda_handler"] \ No newline at end of file diff --git a/lambdas/mock_pds/README.md b/lambdas/mock_pds/README.md new file mode 100644 index 0000000000..d0202ff0a8 --- /dev/null +++ b/lambdas/mock_pds/README.md @@ -0,0 +1,18 @@ +# Mock PDS Lambda + +This Lambda exposes a deterministic mock PDS endpoint for ref-only integration and performance testing. + +It supports: + +- `GET /Patient/{nhs_number}` for patient lookups used by MNS and id-sync. +- Redis-backed average and spike rate limiting with a fixed response contract. + +Environment variables: + +- `MOCK_PDS_AVERAGE_LIMIT` +- `MOCK_PDS_AVERAGE_WINDOW_SECONDS` +- `MOCK_PDS_SPIKE_LIMIT` +- `MOCK_PDS_SPIKE_WINDOW_SECONDS` +- `MOCK_PDS_GP_ODS_CODE` +- `REDIS_HOST` +- `REDIS_PORT` diff --git a/lambdas/mock_pds/poetry.lock b/lambdas/mock_pds/poetry.lock new file mode 100644 index 0000000000..6a34806349 --- /dev/null +++ b/lambdas/mock_pds/poetry.lock @@ -0,0 +1,39 @@ +# This file is automatically @generated by Poetry 2.1.4 and should not be changed by hand. + +[[package]] +name = "async-timeout" +version = "5.0.1" +description = "Timeout context manager for asyncio programs" +optional = false +python-versions = ">=3.8" +groups = ["main"] +markers = "python_full_version < \"3.11.3\"" +files = [ + {file = "async_timeout-5.0.1-py3-none-any.whl", hash = "sha256:39e3809566ff85354557ec2398b55e096c8364bacac9405a7a1fa429e77fe76c"}, + {file = "async_timeout-5.0.1.tar.gz", hash = "sha256:d9321a7a3d5a6a5e187e824d2fa0793ce379a202935782d555d6e9d2735677d3"}, +] + +[[package]] +name = "redis" +version = "6.4.0" +description = "Python client for Redis database and key-value store" +optional = false +python-versions = ">=3.9" +groups = ["main"] +files = [ + {file = "redis-6.4.0-py3-none-any.whl", hash = "sha256:f0544fa9604264e9464cdf4814e7d4830f74b165d52f2a330a760a88dd248b7f"}, + {file = "redis-6.4.0.tar.gz", hash = "sha256:b01bc7282b8444e28ec36b261df5375183bb47a07eb9c603f284e89cbc5ef010"}, +] + +[package.dependencies] +async-timeout = {version = ">=4.0.3", markers = "python_full_version < \"3.11.3\""} + +[package.extras] +hiredis = ["hiredis (>=3.2.0)"] +jwt = ["pyjwt (>=2.9.0)"] +ocsp = ["cryptography (>=36.0.1)", "pyopenssl (>=20.0.1)", "requests (>=2.31.0)"] + +[metadata] +lock-version = "2.1" +python-versions = "~3.11" +content-hash = "5d3a40c5ca25affeefbf1b1b07bc8003305056af1804c0b29f4f02c2ac64f00d" diff --git a/lambdas/mock_pds/pyproject.toml b/lambdas/mock_pds/pyproject.toml new file mode 100644 index 0000000000..2e82d4ce05 --- /dev/null +++ b/lambdas/mock_pds/pyproject.toml @@ -0,0 +1,15 @@ +[tool.poetry] +name = "mock-pds" +version = "1.0.0" +description = "Mock PDS endpoint for ref integration and performance testing" +authors = ["VED Team "] +readme = "README.md" +packages = [{include = "src"}] + +[tool.poetry.dependencies] +python = "~3.11" +redis = "^6.1.0" + +[build-system] +requires = ["poetry-core >= 1.5.0"] +build-backend = "poetry.core.masonry.api" \ No newline at end of file diff --git a/lambdas/mock_pds/src/lambda_handler.py b/lambdas/mock_pds/src/lambda_handler.py new file mode 100644 index 0000000000..594389df66 --- /dev/null +++ b/lambdas/mock_pds/src/lambda_handler.py @@ -0,0 +1,49 @@ +import logging +import os + +import redis + +from mock_pds_service import MockPdsService +from rate_limiter import FixedWindowRateLimiter + +logger = logging.getLogger() +logger.setLevel(logging.INFO) + +_mock_pds_service: MockPdsService | None = None + + +def get_mock_pds_service() -> MockPdsService: + global _mock_pds_service + + if _mock_pds_service is None: + redis_client = redis.Redis( + host=os.environ["REDIS_HOST"], + port=int(os.getenv("REDIS_PORT", "6379")), + decode_responses=True, + ) + rate_limiter = FixedWindowRateLimiter( + redis_client=redis_client, + key_prefix="mock-pds", + average_limit=int(os.getenv("MOCK_PDS_AVERAGE_LIMIT", "125")), + average_window_seconds=int(os.getenv("MOCK_PDS_AVERAGE_WINDOW_SECONDS", "60")), + spike_limit=int(os.getenv("MOCK_PDS_SPIKE_LIMIT", "450")), + spike_window_seconds=int(os.getenv("MOCK_PDS_SPIKE_WINDOW_SECONDS", "1")), + ) + _mock_pds_service = MockPdsService( + rate_limiter=rate_limiter, + gp_ods_code=os.getenv("MOCK_PDS_GP_ODS_CODE", "Y12345"), + ) + + return _mock_pds_service + + +def lambda_handler(event, context): + try: + return get_mock_pds_service().handle(event) + except Exception: + logger.exception("Mock PDS failed to handle request") + return { + "statusCode": 500, + "headers": {"Content-Type": "application/json"}, + "body": '{"code": 500, "message": "Mock PDS encountered an unexpected error"}', + } diff --git a/lambdas/mock_pds/src/mock_pds_service.py b/lambdas/mock_pds/src/mock_pds_service.py new file mode 100644 index 0000000000..692ab420ed --- /dev/null +++ b/lambdas/mock_pds/src/mock_pds_service.py @@ -0,0 +1,115 @@ +import json +import logging +from http import HTTPStatus + +from rate_limiter import FixedWindowRateLimiter + +logger = logging.getLogger() +logger.setLevel(logging.INFO) + +FHIR_JSON_CONTENT_TYPE = "application/fhir+json" +JSON_CONTENT_TYPE = "application/json" +RATE_LIMIT_MESSAGE = "Mock PDS rate limit has been exceeded" + + +class MockPdsService: + def __init__(self, rate_limiter: FixedWindowRateLimiter, gp_ods_code: str): + self.rate_limiter = rate_limiter + self.gp_ods_code = gp_ods_code + + def handle(self, event: dict) -> dict: + method = self._get_method(event) + if method != "GET": + return self._json_response( + HTTPStatus.METHOD_NOT_ALLOWED, + {"code": HTTPStatus.METHOD_NOT_ALLOWED, "message": "Method not allowed"}, + ) + + nhs_number = self._extract_patient_id(event) + if not nhs_number: + return self._json_response( + HTTPStatus.BAD_REQUEST, + {"code": HTTPStatus.BAD_REQUEST, "message": "Patient id is required"}, + ) + + decision = self.rate_limiter.check("patient-lookup") + if not decision.allowed: + logger.warning( + "Mock PDS rate limit exceeded for %s window: count=%s limit=%s window_seconds=%s", + decision.window_name, + decision.count, + decision.limit, + decision.window_seconds, + ) + return self._json_response( + HTTPStatus.TOO_MANY_REQUESTS, + {"code": HTTPStatus.TOO_MANY_REQUESTS, "message": RATE_LIMIT_MESSAGE}, + ) + + logger.info("Mock PDS served patient lookup for nhs_number=%s", nhs_number) + return self._json_response( + HTTPStatus.OK, + self._build_patient(nhs_number), + content_type=FHIR_JSON_CONTENT_TYPE, + ) + + def _build_patient(self, nhs_number: str) -> dict: + suffix = nhs_number[-4:] if nhs_number else "0000" + day = max(1, (int(suffix[-2:]) % 28)) + month = max(1, (int(suffix[:2]) % 12)) + + return { + "resourceType": "Patient", + "id": nhs_number, + "identifier": [ + { + "system": "https://fhir.nhs.uk/Id/nhs-number", + "value": nhs_number, + } + ], + "birthDate": f"1985-{month:02d}-{day:02d}", + "gender": "unknown", + "name": [ + { + "family": f"Mock-{suffix}", + "given": ["Ref", "Patient"], + } + ], + "generalPractitioner": [ + { + "identifier": { + "system": "https://fhir.nhs.uk/Id/ods-organization-code", + "value": self.gp_ods_code, + "period": { + "start": "2024-01-01", + }, + } + } + ], + } + + @staticmethod + def _get_method(event: dict) -> str: + request_context = event.get("requestContext", {}) + http_context = request_context.get("http", {}) + return http_context.get("method") or event.get("httpMethod") or "GET" + + @staticmethod + def _extract_patient_id(event: dict) -> str | None: + path = ( + event.get("rawPath") or event.get("path") or event.get("requestContext", {}).get("http", {}).get("path", "") + ) + normalized_path = path.rstrip("/") + + if "/Patient/" not in normalized_path: + return None + + return normalized_path.rsplit("/Patient/", maxsplit=1)[-1] or None + + @staticmethod + def _json_response(status_code: int, body: dict, content_type: str = JSON_CONTENT_TYPE) -> dict: + return { + "statusCode": status_code, + "headers": {"Content-Type": content_type}, + "body": json.dumps(body), + } diff --git a/lambdas/mock_pds/src/rate_limiter.py b/lambdas/mock_pds/src/rate_limiter.py new file mode 100644 index 0000000000..7388deab1c --- /dev/null +++ b/lambdas/mock_pds/src/rate_limiter.py @@ -0,0 +1,64 @@ +import time +from dataclasses import dataclass + +import redis + + +@dataclass(frozen=True) +class RateLimitDecision: + allowed: bool + window_name: str + count: int + limit: int + window_seconds: int + + +class FixedWindowRateLimiter: + def __init__( + self, + redis_client: redis.Redis, + key_prefix: str, + average_limit: int, + average_window_seconds: int, + spike_limit: int, + spike_window_seconds: int, + ): + self.redis_client = redis_client + self.key_prefix = key_prefix + self.average_limit = average_limit + self.average_window_seconds = average_window_seconds + self.spike_limit = spike_limit + self.spike_window_seconds = spike_window_seconds + + def check(self, scope: str) -> RateLimitDecision: + average_decision = self._evaluate_window( + scope=scope, + window_name="average", + limit=self.average_limit * self.average_window_seconds, + window_seconds=self.average_window_seconds, + ) + if not average_decision.allowed: + return average_decision + + return self._evaluate_window( + scope=scope, + window_name="spike", + limit=self.spike_limit, + window_seconds=self.spike_window_seconds, + ) + + def _evaluate_window(self, scope: str, window_name: str, limit: int, window_seconds: int) -> RateLimitDecision: + current_window = int(time.time() // window_seconds) + key = f"{self.key_prefix}:{scope}:{window_name}:{current_window}" + pipeline = self.redis_client.pipeline() + pipeline.incr(key) + pipeline.expire(key, window_seconds + 1) + count, _ = pipeline.execute() + + return RateLimitDecision( + allowed=count <= limit, + window_name=window_name, + count=count, + limit=limit, + window_seconds=window_seconds, + ) diff --git a/lambdas/mock_pds/tests/test_mock_pds_service.py b/lambdas/mock_pds/tests/test_mock_pds_service.py new file mode 100644 index 0000000000..9247569b71 --- /dev/null +++ b/lambdas/mock_pds/tests/test_mock_pds_service.py @@ -0,0 +1,92 @@ +import json +import unittest +from unittest.mock import Mock, patch + +from lambda_handler import get_mock_pds_service, lambda_handler +from mock_pds_service import RATE_LIMIT_MESSAGE, MockPdsService +from rate_limiter import FixedWindowRateLimiter, RateLimitDecision + + +class TestMockPdsService(unittest.TestCase): + def setUp(self): + self.rate_limiter = Mock(spec=FixedWindowRateLimiter) + self.rate_limiter.check.return_value = RateLimitDecision( + allowed=True, + window_name="spike", + count=1, + limit=450, + window_seconds=1, + ) + self.service = MockPdsService(self.rate_limiter, "Y12345") + + def test_returns_mock_patient_payload(self): + response = self.service.handle({"rawPath": "/Patient/9481152782", "requestContext": {"http": {"method": "GET"}}}) + + self.assertEqual(response["statusCode"], 200) + self.assertEqual(response["headers"]["Content-Type"], "application/fhir+json") + body = json.loads(response["body"]) + self.assertEqual(body["resourceType"], "Patient") + self.assertEqual(body["id"], "9481152782") + self.assertEqual(body["generalPractitioner"][0]["identifier"]["value"], "Y12345") + + def test_returns_429_when_rate_limit_exceeded(self): + self.rate_limiter.check.return_value = RateLimitDecision( + allowed=False, + window_name="spike", + count=451, + limit=450, + window_seconds=1, + ) + + response = self.service.handle({"rawPath": "/Patient/9481152782", "requestContext": {"http": {"method": "GET"}}}) + + self.assertEqual(response["statusCode"], 429) + self.assertEqual(json.loads(response["body"]), {"code": 429, "message": RATE_LIMIT_MESSAGE}) + + def test_rejects_non_get_requests(self): + response = self.service.handle( + {"rawPath": "/Patient/9481152782", "requestContext": {"http": {"method": "POST"}}} + ) + + self.assertEqual(response["statusCode"], 405) + + +class TestLambdaHandler(unittest.TestCase): + def tearDown(self): + get_mock_pds_service.__globals__["_mock_pds_service"] = None + + @patch.dict( + "os.environ", + { + "REDIS_HOST": "mock-redis", + "MOCK_PDS_AVERAGE_LIMIT": "125", + "MOCK_PDS_AVERAGE_WINDOW_SECONDS": "60", + "MOCK_PDS_SPIKE_LIMIT": "450", + "MOCK_PDS_SPIKE_WINDOW_SECONDS": "1", + }, + clear=False, + ) + @patch("lambda_handler.redis.Redis") + def test_lambda_handler_uses_cached_service(self, mock_redis): + mock_service = Mock() + mock_service.handle.return_value = {"statusCode": 200} + + with patch("lambda_handler.MockPdsService", return_value=mock_service): + first_response = lambda_handler( + {"rawPath": "/Patient/123", "requestContext": {"http": {"method": "GET"}}}, None + ) + second_response = lambda_handler( + {"rawPath": "/Patient/456", "requestContext": {"http": {"method": "GET"}}}, None + ) + + self.assertEqual(first_response, {"statusCode": 200}) + self.assertEqual(second_response, {"statusCode": 200}) + mock_redis.assert_called_once_with(host="mock-redis", port=6379, decode_responses=True) + + @patch("lambda_handler.get_mock_pds_service") + def test_lambda_handler_returns_500_on_unhandled_error(self, mock_get_service): + mock_get_service.return_value.handle.side_effect = RuntimeError("boom") + + response = lambda_handler({"rawPath": "/Patient/123", "requestContext": {"http": {"method": "GET"}}}, None) + + self.assertEqual(response["statusCode"], 500) diff --git a/lambdas/shared/src/common/api_clients/get_pds_details.py b/lambdas/shared/src/common/api_clients/get_pds_details.py index 7f728c81e8..2f5766f832 100644 --- a/lambdas/shared/src/common/api_clients/get_pds_details.py +++ b/lambdas/shared/src/common/api_clients/get_pds_details.py @@ -9,19 +9,32 @@ from common.api_clients.pds_service import PdsService from common.clients import get_secrets_manager_client, logger -PDS_ENV = os.getenv("PDS_ENV", "int") - _pds_service: PdsService | None = None +def get_pds_environment() -> str: + return os.getenv("PDS_ENV", "int") + + +def get_pds_base_url() -> str | None: + base_url = os.getenv("PDS_BASE_URL", "").strip() + return base_url or None + + def get_pds_service() -> PdsService: global _pds_service if _pds_service is None: - authenticator = AppRestrictedAuth( - secret_manager_client=get_secrets_manager_client(), - environment=PDS_ENV, - ) - _pds_service = PdsService(authenticator, PDS_ENV) + pds_environment = get_pds_environment() + pds_base_url = get_pds_base_url() + authenticator = None + + if pds_base_url is None: + authenticator = AppRestrictedAuth( + secret_manager_client=get_secrets_manager_client(), + environment=pds_environment, + ) + + _pds_service = PdsService(authenticator, pds_environment, base_url=pds_base_url) return _pds_service diff --git a/lambdas/shared/src/common/api_clients/pds_service.py b/lambdas/shared/src/common/api_clients/pds_service.py index 90db22f957..1b73af8861 100644 --- a/lambdas/shared/src/common/api_clients/pds_service.py +++ b/lambdas/shared/src/common/api_clients/pds_service.py @@ -7,25 +7,40 @@ class PdsService: - def __init__(self, authenticator: AppRestrictedAuth, environment): + def __init__( + self, + authenticator: AppRestrictedAuth | None, + environment: str, + base_url: str | None = None, + ): logger.info(f"PdsService init: {environment}") self.authenticator = authenticator - self.base_url = ( + self.base_url = self._resolve_base_url(environment, base_url) + + logger.info(f"PDS Service URL: {self.base_url}") + + @staticmethod + def _resolve_base_url(environment: str, base_url: str | None) -> str: + if base_url: + return base_url.rstrip("/") + + return ( f"https://{environment}.api.service.nhs.uk/personal-demographics/FHIR/R4/Patient" if environment != "prod" else "https://api.service.nhs.uk/personal-demographics/FHIR/R4/Patient" ) - logger.info(f"PDS Service URL: {self.base_url}") - def get_patient_details(self, patient_id: str) -> dict | None: - access_token = self.authenticator.get_access_token() request_headers = { - "Authorization": f"Bearer {access_token}", "X-Request-ID": str(uuid.uuid4()), "X-Correlation-ID": str(uuid.uuid4()), } + + if self.authenticator is not None: + access_token = self.authenticator.get_access_token() + request_headers["Authorization"] = f"Bearer {access_token}" + response = request_with_retry_backoff("GET", f"{self.base_url}/{patient_id}", headers=request_headers) if response.status_code == 200: diff --git a/lambdas/shared/tests/test_common/api_clients/test_pds_details.py b/lambdas/shared/tests/test_common/api_clients/test_pds_details.py index e58b430ee8..b5f7fb01b4 100644 --- a/lambdas/shared/tests/test_common/api_clients/test_pds_details.py +++ b/lambdas/shared/tests/test_common/api_clients/test_pds_details.py @@ -88,3 +88,16 @@ def test_reuses_same_pds_service_instance(self): self.mock_auth_class.assert_called_once() self.mock_pds_service_class.assert_called_once() self.assertEqual(self.mock_pds_service_instance.get_patient_details.call_count, 2) + + @patch.dict("os.environ", {"PDS_ENV": "ref", "PDS_BASE_URL": "https://mock-pds.example/Patient"}, clear=False) + def test_uses_base_url_override_without_authenticator(self): + pds_get_patient_details(self.test_patient_id) + + self.mock_auth_class.assert_not_called() + self.mock_pds_service_class.assert_called_once_with( + None, + "ref", + base_url="https://mock-pds.example/Patient", + ) + + self.mock_pds_service_instance.get_patient_details.assert_called_once_with(self.test_patient_id) diff --git a/lambdas/shared/tests/test_common/api_clients/test_pds_service.py b/lambdas/shared/tests/test_common/api_clients/test_pds_service.py index 48f7a8e250..2f58aeb875 100644 --- a/lambdas/shared/tests/test_common/api_clients/test_pds_service.py +++ b/lambdas/shared/tests/test_common/api_clients/test_pds_service.py @@ -77,3 +77,20 @@ def test_env_mapping(self): env = "prod" service = PdsService(None, env) self.assertTrue(env not in service.base_url) + + def test_custom_base_url_override(self): + service = PdsService(None, "ref", base_url="https://mock-pds.example/Patient/") + + self.assertEqual(service.base_url, "https://mock-pds.example/Patient") + + @responses.activate + def test_get_patient_details_without_authenticator(self): + patient_id = "900000009" + pds_url = f"https://mock-pds.example/Patient/{patient_id}" + responses.add(responses.GET, pds_url, json={"id": patient_id}, status=200) + pds_service = PdsService(None, "ref", base_url="https://mock-pds.example/Patient") + + patient = pds_service.get_patient_details(patient_id) + + self.assertEqual(patient, {"id": patient_id}) + self.assertNotIn("Authorization", responses.calls[0].request.headers) diff --git a/mns_perf_with_mocked_pds_4bb74b98.plan.md b/mns_perf_with_mocked_pds_4bb74b98.plan.md new file mode 100644 index 0000000000..ae7303c875 --- /dev/null +++ b/mns_perf_with_mocked_pds_4bb74b98.plan.md @@ -0,0 +1,112 @@ +--- +name: MNS perf with mocked PDS +overview: Design and implement a mocked PDS path in ref so MNS integration/performance testing can run without hitting real PDS, with explicit rate-limit behavior (125 average, 450 spike) and repeatable load tests that quantify current service limits. +todos: + - id: add-mock-pds-service + content: Create mock PDS Lambda/API that returns deterministic patient + GP ODS data and standardized 429 response contract. + status: pending + - id: wire-ref-routing + content: Update infrastructure and ref environment config so PDS calls in ref route to mock endpoint while preserving non-ref behavior. + status: pending + - id: implement-rate-limit-policy + content: Implement and configure average/spike limiting (125/450 rps) with observable metrics and deterministic testability. + status: pending + - id: add-functional-tests + content: Add unit/integration tests covering successful data retrieval and 429 rate-limit scenarios for MNS publishing dependency on PDS. + status: pending + - id: extend-locust-scenarios + content: Add Locust traffic profiles (average, spike, ramp) and commands to quantify throughput, latency, and 429 behavior. + status: pending + - id: publish-runbook-guidance + content: Update existing perf docs with execution steps, expected outputs, and interpretation for campaign-capacity decisions. + status: pending +isProject: false +--- + +# Plan: Performance test MNS with mocked PDS in ref + +## Objectives + +- Provide a **mocked PDS Lambda/service** in `ref` so MNS flows can retrieve required patient/GP (ODS) data without calling real PDS. +- Enforce and validate rate limits aligned to acceptance criteria: + - average threshold: `125 rps` + - spike threshold: `450 rps` + - overflow response: HTTP `429`, body/message `Mock PDS rate limit has been exceeded` +- Run repeatable load tests to determine current throughput ceiling and failure characteristics for campaign peaks. + +## Existing architecture to leverage + +- MNS publisher resolves GP data via shared PDS client in [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py) and [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py). +- Notification payload construction (including GP ODS use) is in [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/mns_publisher/src/create_notification.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/mns_publisher/src/create_notification.py). +- Ref-targeted performance tooling already exists in Locust at [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py). + +## Delivery approach + +1. **Introduce Mock PDS API Lambda** + - Add a dedicated Lambda that returns deterministic Patient payloads containing required demographic + GP ODS fields for NHS number lookup. + - Support two behaviors via env/config: + - normal mode: return valid mock PDS patient data + - throttled mode: return `429` once configured limit is exceeded + - Keep schema compatible with current parser paths in MNS/id-sync code to avoid functional drift. + +2. **Wire ref to mock endpoint, keep other envs safe** + - Add infra for mock PDS API (Lambda + API Gateway route) in instance modules. + - In `ref` tfvars, route `PDS_ENV`/PDS base URL resolution to the mock endpoint only. + - Preserve existing behavior for non-ref environments unless explicitly configured. + +3. **Implement rate-limiting policy in mock** + - Use API Gateway throttling and/or in-Lambda token-bucket counters (final choice based on easiest deterministic assertion in tests) to guarantee: + - sustained traffic over `125 rps` observable as throttling trend + - spike over `450 rps` promptly returns `429` + - Standardize response body contract: `{ "code": 429, "message": "Mock PDS rate limit has been exceeded" }`. + +4. **Add automated validation tests** + - Unit tests for mock response shape and error contract. + - Integration tests for MNS publisher path proving: + - Scenario 1: required PDS data is returned in ref via mock (including ODS usage path) + - Scenario 2: limit breaches produce `429` with expected message. + +5. **Extend perf tests for campaign-style load** + - Add Locust scenarios to drive traffic profiles around: + - baseline (`~125 rps`) + - spike (`>450 rps` burst) + - stepped ramp (to identify knee point and failure ratio) + - Capture and report: success %, 429 %, latency p50/p95/p99, and MNS publish success under each load shape. + +6. **Operational guardrails and rollout** + - Add runbook notes in existing perf test docs for executing ref tests safely. + - Ensure alarms/log queries identify throttle onset and downstream effects in MNS publisher. + - Keep mock enabled as mandatory integration path while real PDS endpoint is off-limits. + +## Suggested implementation sequence by file area + +- Shared PDS client behavior switch: + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py) + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py) +- New mock PDS lambda + tests under `lambdas/` (new module mirrored to existing lambda structure). +- Infra wiring (API/Lambda/env mapping): + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance) + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance/environments/dev/ref/variables.tfvars`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance/environments/dev/ref/variables.tfvars) +- Perf scripts/docs: + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py) + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/README.md`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/README.md) + - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/Makefile`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/Makefile) + +## Flow diagram + +```mermaid +flowchart LR +clientLoad[LocustLoadInRef] --> api[ImmsApiRef] +api --> events[EventsAndDeltaFlow] +events --> mnsPublisher[MnsPublisherLambda] +mnsPublisher --> pdsMock[MockPdsApiLambda] +pdsMock -->|"200 patient data with GP ODS"| mnsPublisher +pdsMock -->|"429 Mock PDS rate limit has been exceeded"| mnsPublisher +mnsPublisher --> mnsTarget[MnsPublishPath] +``` + +## Acceptance criteria mapping + +- **Scenario 1:** mock service in `ref` returns required patient data for MNS publishing path, including GP ODS extraction path. +- **Scenario 2:** calls above configured `450 rps` (and sustained over `125 rps`) return HTTP `429` with exact message contract. +- **Outcome:** measured traffic limits and failure envelope documented from repeatable load profiles for Autumn/Spring campaign readiness decisions. diff --git a/tests/perf_tests/Makefile b/tests/perf_tests/Makefile index d15b7705e3..46d2a55863 100644 --- a/tests/perf_tests/Makefile +++ b/tests/perf_tests/Makefile @@ -1,4 +1,21 @@ +LOCUST_FILE ?= src/locustfile.py +LOCUST ?= poetry run locust -f $(LOCUST_FILE) +RESULTS_DIR ?= results +RESULTS_PREFIX ?= $(RESULTS_DIR)/$(PERF_LOAD_PROFILE) + test: - poetry run locust -f src/locustfile.py + $(LOCUST) + +baseline: + mkdir -p $(RESULTS_DIR) + PERF_LOAD_PROFILE=baseline $(LOCUST) CreateUser --headless --only-summary --csv $(RESULTS_PREFIX) + +spike: + mkdir -p $(RESULTS_DIR) + PERF_LOAD_PROFILE=spike $(LOCUST) CreateUser --headless --only-summary --csv $(RESULTS_PREFIX) + +ramp: + mkdir -p $(RESULTS_DIR) + PERF_LOAD_PROFILE=ramp $(LOCUST) CreateUser --headless --only-summary --csv $(RESULTS_PREFIX) -.PHONY: test \ No newline at end of file +.PHONY: test baseline spike ramp \ No newline at end of file diff --git a/tests/perf_tests/Readme.md b/tests/perf_tests/Readme.md index d33fb8abe9..f833a3f5fb 100644 --- a/tests/perf_tests/Readme.md +++ b/tests/perf_tests/Readme.md @@ -8,3 +8,32 @@ To run them, ensure you have the `PERF_CREATE_RPS_PER_USER` : numeric env vars set, and call `make test`. + +For MNS-with-mocked-PDS capacity work, use the `CreateUser` profile so downstream publishing and PDS lookup activity is exercised. + +Available load profiles: + +- `make baseline`: holds traffic around the average acceptance threshold. Defaults to `125 rps` for `300s`. +- `make spike`: warms up at the average threshold, bursts above the spike threshold, then recovers. Defaults to `125 rps`, then `460 rps`, then back to `125 rps`. +- `make ramp`: increases traffic in fixed steps to identify the knee point and error envelope. Defaults to `50 rps` start, `25 rps` increments, `60s` per step, stopping after `500 rps`. + +Supported environment variables: + +- `PERF_LOAD_PROFILE`: `baseline`, `spike`, or `ramp`. +- `PERF_BASELINE_RPS`, `PERF_BASELINE_DURATION_SECONDS` +- `PERF_SPIKE_WARMUP_RPS`, `PERF_SPIKE_RPS`, `PERF_SPIKE_WARMUP_SECONDS`, `PERF_SPIKE_DURATION_SECONDS`, `PERF_SPIKE_RECOVERY_SECONDS` +- `PERF_RAMP_START_RPS`, `PERF_RAMP_STEP_RPS`, `PERF_RAMP_MAX_RPS`, `PERF_RAMP_STEP_DURATION_SECONDS` +- `RESULTS_DIR`: output directory for Locust CSV summaries. + +Each headless profile writes Locust CSV output to `results/*`. Review: + +- request counts and failures to quantify the percentage of 429 responses +- `*_stats.csv` for p50/p95/p99 latency +- `*_failures.csv` for error mix and throttle onset timing + +Suggested ref runbook: + +1. Run `make baseline` and confirm downstream create flow is stable at the average threshold. +2. Run `make spike` and check whether 429 responses are isolated to the burst window and whether MNS publish failures remain within expected limits. +3. Run `make ramp` to find the first step where latency, failures, or 429 volume becomes operationally unacceptable. +4. Record success rate, 429 rate, and p95/p99 latency from the generated CSV files for campaign-capacity decisions. diff --git a/tests/perf_tests/src/locustfile.py b/tests/perf_tests/src/locustfile.py index 5841a6a748..7c8956c5df 100644 --- a/tests/perf_tests/src/locustfile.py +++ b/tests/perf_tests/src/locustfile.py @@ -1,4 +1,5 @@ import json +import math import os import random import uuid @@ -6,7 +7,7 @@ from urllib.parse import urlencode import pandas as pd -from locust import HttpUser, constant_throughput, task +from locust import HttpUser, LoadTestShape, constant_throughput, task from common.api_clients.authentication import AppRestrictedAuth from common.clients import get_secrets_manager_client @@ -22,6 +23,18 @@ PERF_SUPPLIER_SYSTEM = os.getenv("PERF_SUPPLIER_SYSTEM", "EMIS").upper() PERF_CREATE_TASK_RPS_PER_USER = float(os.getenv("PERF_CREATE_RPS_PER_USER", "1")) +PERF_LOAD_PROFILE = os.getenv("PERF_LOAD_PROFILE", "").strip().lower() +PERF_BASELINE_RPS = int(os.getenv("PERF_BASELINE_RPS", "125")) +PERF_BASELINE_DURATION_SECONDS = int(os.getenv("PERF_BASELINE_DURATION_SECONDS", "300")) +PERF_SPIKE_WARMUP_RPS = int(os.getenv("PERF_SPIKE_WARMUP_RPS", "125")) +PERF_SPIKE_RPS = int(os.getenv("PERF_SPIKE_RPS", "460")) +PERF_SPIKE_WARMUP_SECONDS = int(os.getenv("PERF_SPIKE_WARMUP_SECONDS", "120")) +PERF_SPIKE_DURATION_SECONDS = int(os.getenv("PERF_SPIKE_DURATION_SECONDS", "60")) +PERF_SPIKE_RECOVERY_SECONDS = int(os.getenv("PERF_SPIKE_RECOVERY_SECONDS", "120")) +PERF_RAMP_START_RPS = int(os.getenv("PERF_RAMP_START_RPS", "50")) +PERF_RAMP_STEP_RPS = int(os.getenv("PERF_RAMP_STEP_RPS", "25")) +PERF_RAMP_MAX_RPS = int(os.getenv("PERF_RAMP_MAX_RPS", "500")) +PERF_RAMP_STEP_DURATION_SECONDS = int(os.getenv("PERF_RAMP_STEP_DURATION_SECONDS", "60")) IMMUNIZATION_TARGETS = [ "3IN1", @@ -68,6 +81,40 @@ def _load_valid_patients(): VALID_PATIENT_IDS = _load_valid_patients() +def _users_for_target_rps(target_rps: int) -> int: + per_user_rps = PERF_CREATE_TASK_RPS_PER_USER if PERF_CREATE_TASK_RPS_PER_USER > 0 else 1 + return max(1, math.ceil(target_rps / per_user_rps)) + + +class CampaignCapacityShape(LoadTestShape): + abstract = PERF_LOAD_PROFILE not in {"baseline", "spike", "ramp"} + + def tick(self): + run_time = self.get_run_time() + + if PERF_LOAD_PROFILE == "baseline": + if run_time >= PERF_BASELINE_DURATION_SECONDS: + return None + target_rps = PERF_BASELINE_RPS + elif PERF_LOAD_PROFILE == "spike": + if run_time < PERF_SPIKE_WARMUP_SECONDS: + target_rps = PERF_SPIKE_WARMUP_RPS + elif run_time < PERF_SPIKE_WARMUP_SECONDS + PERF_SPIKE_DURATION_SECONDS: + target_rps = PERF_SPIKE_RPS + elif run_time < PERF_SPIKE_WARMUP_SECONDS + PERF_SPIKE_DURATION_SECONDS + PERF_SPIKE_RECOVERY_SECONDS: + target_rps = PERF_SPIKE_WARMUP_RPS + else: + return None + else: + current_step = int(run_time // PERF_RAMP_STEP_DURATION_SECONDS) + target_rps = PERF_RAMP_START_RPS + (current_step * PERF_RAMP_STEP_RPS) + if target_rps > PERF_RAMP_MAX_RPS: + return None + + user_count = _users_for_target_rps(target_rps) + return user_count, user_count + + class BaseImmunizationUser(HttpUser): abstract = True From 1f5dfc5965d87f1befba5134c25fa05201f31e35 Mon Sep 17 00:00:00 2001 From: Thomas-Boyle Date: Fri, 17 Apr 2026 13:01:59 +0100 Subject: [PATCH 02/18] refactor: Simplify MockPdsService and improve error handling - Streamlined the initialization of MockPdsService by consolidating parameters. - Replaced multiple response methods with a unified error handling approach. - Enhanced readability by reducing redundant code in the patient lookup logic. - Updated tests to utilize helper functions for event creation and decision mocking. --- lambdas/mock_pds/src/lambda_handler.py | 7 +- lambdas/mock_pds/src/mock_pds_service.py | 66 ++++++------------- lambdas/mock_pds/src/rate_limiter.py | 32 ++++----- .../mock_pds/tests/test_mock_pds_service.py | 42 +++++------- .../src/common/api_clients/get_pds_details.py | 32 +++------ .../src/common/api_clients/pds_service.py | 32 ++------- 6 files changed, 63 insertions(+), 148 deletions(-) diff --git a/lambdas/mock_pds/src/lambda_handler.py b/lambdas/mock_pds/src/lambda_handler.py index 594389df66..1d4da28482 100644 --- a/lambdas/mock_pds/src/lambda_handler.py +++ b/lambdas/mock_pds/src/lambda_handler.py @@ -14,7 +14,6 @@ def get_mock_pds_service() -> MockPdsService: global _mock_pds_service - if _mock_pds_service is None: redis_client = redis.Redis( host=os.environ["REDIS_HOST"], @@ -29,11 +28,7 @@ def get_mock_pds_service() -> MockPdsService: spike_limit=int(os.getenv("MOCK_PDS_SPIKE_LIMIT", "450")), spike_window_seconds=int(os.getenv("MOCK_PDS_SPIKE_WINDOW_SECONDS", "1")), ) - _mock_pds_service = MockPdsService( - rate_limiter=rate_limiter, - gp_ods_code=os.getenv("MOCK_PDS_GP_ODS_CODE", "Y12345"), - ) - + _mock_pds_service = MockPdsService(rate_limiter, os.getenv("MOCK_PDS_GP_ODS_CODE", "Y12345")) return _mock_pds_service diff --git a/lambdas/mock_pds/src/mock_pds_service.py b/lambdas/mock_pds/src/mock_pds_service.py index 692ab420ed..3503dbb027 100644 --- a/lambdas/mock_pds/src/mock_pds_service.py +++ b/lambdas/mock_pds/src/mock_pds_service.py @@ -8,7 +8,6 @@ logger.setLevel(logging.INFO) FHIR_JSON_CONTENT_TYPE = "application/fhir+json" -JSON_CONTENT_TYPE = "application/json" RATE_LIMIT_MESSAGE = "Mock PDS rate limit has been exceeded" @@ -18,19 +17,12 @@ def __init__(self, rate_limiter: FixedWindowRateLimiter, gp_ods_code: str): self.gp_ods_code = gp_ods_code def handle(self, event: dict) -> dict: - method = self._get_method(event) - if method != "GET": - return self._json_response( - HTTPStatus.METHOD_NOT_ALLOWED, - {"code": HTTPStatus.METHOD_NOT_ALLOWED, "message": "Method not allowed"}, - ) + if self._get_method(event) != "GET": + return self._error(HTTPStatus.METHOD_NOT_ALLOWED, "Method not allowed") nhs_number = self._extract_patient_id(event) if not nhs_number: - return self._json_response( - HTTPStatus.BAD_REQUEST, - {"code": HTTPStatus.BAD_REQUEST, "message": "Patient id is required"}, - ) + return self._error(HTTPStatus.BAD_REQUEST, "Patient id is required") decision = self.rate_limiter.check("patient-lookup") if not decision.allowed: @@ -41,48 +33,29 @@ def handle(self, event: dict) -> dict: decision.limit, decision.window_seconds, ) - return self._json_response( - HTTPStatus.TOO_MANY_REQUESTS, - {"code": HTTPStatus.TOO_MANY_REQUESTS, "message": RATE_LIMIT_MESSAGE}, - ) + return self._error(HTTPStatus.TOO_MANY_REQUESTS, RATE_LIMIT_MESSAGE) logger.info("Mock PDS served patient lookup for nhs_number=%s", nhs_number) - return self._json_response( - HTTPStatus.OK, - self._build_patient(nhs_number), - content_type=FHIR_JSON_CONTENT_TYPE, - ) + return self._response(HTTPStatus.OK, self._build_patient(nhs_number), FHIR_JSON_CONTENT_TYPE) def _build_patient(self, nhs_number: str) -> dict: - suffix = nhs_number[-4:] if nhs_number else "0000" - day = max(1, (int(suffix[-2:]) % 28)) - month = max(1, (int(suffix[:2]) % 12)) + suffix = (nhs_number or "0000")[-4:] + day = max(1, int(suffix[-2:]) % 28) + month = max(1, int(suffix[:2]) % 12) return { "resourceType": "Patient", "id": nhs_number, - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": nhs_number, - } - ], + "identifier": [{"system": "https://fhir.nhs.uk/Id/nhs-number", "value": nhs_number}], "birthDate": f"1985-{month:02d}-{day:02d}", "gender": "unknown", - "name": [ - { - "family": f"Mock-{suffix}", - "given": ["Ref", "Patient"], - } - ], + "name": [{"family": f"Mock-{suffix}", "given": ["Ref", "Patient"]}], "generalPractitioner": [ { "identifier": { "system": "https://fhir.nhs.uk/Id/ods-organization-code", "value": self.gp_ods_code, - "period": { - "start": "2024-01-01", - }, + "period": {"start": "2024-01-01"}, } } ], @@ -90,24 +63,23 @@ def _build_patient(self, nhs_number: str) -> dict: @staticmethod def _get_method(event: dict) -> str: - request_context = event.get("requestContext", {}) - http_context = request_context.get("http", {}) - return http_context.get("method") or event.get("httpMethod") or "GET" + return event.get("requestContext", {}).get("http", {}).get("method") or event.get("httpMethod") or "GET" @staticmethod def _extract_patient_id(event: dict) -> str | None: path = ( event.get("rawPath") or event.get("path") or event.get("requestContext", {}).get("http", {}).get("path", "") - ) - normalized_path = path.rstrip("/") - - if "/Patient/" not in normalized_path: + ).rstrip("/") + if "/Patient/" not in path: return None + return path.rsplit("/Patient/", maxsplit=1)[-1] or None - return normalized_path.rsplit("/Patient/", maxsplit=1)[-1] or None + @classmethod + def _error(cls, status: HTTPStatus, message: str) -> dict: + return cls._response(status, {"code": int(status), "message": message}) @staticmethod - def _json_response(status_code: int, body: dict, content_type: str = JSON_CONTENT_TYPE) -> dict: + def _response(status_code: int, body: dict, content_type: str = "application/json") -> dict: return { "statusCode": status_code, "headers": {"Content-Type": content_type}, diff --git a/lambdas/mock_pds/src/rate_limiter.py b/lambdas/mock_pds/src/rate_limiter.py index 7388deab1c..84c760a227 100644 --- a/lambdas/mock_pds/src/rate_limiter.py +++ b/lambdas/mock_pds/src/rate_limiter.py @@ -25,31 +25,21 @@ def __init__( ): self.redis_client = redis_client self.key_prefix = key_prefix - self.average_limit = average_limit - self.average_window_seconds = average_window_seconds - self.spike_limit = spike_limit - self.spike_window_seconds = spike_window_seconds - - def check(self, scope: str) -> RateLimitDecision: - average_decision = self._evaluate_window( - scope=scope, - window_name="average", - limit=self.average_limit * self.average_window_seconds, - window_seconds=self.average_window_seconds, + self._windows = ( + ("average", average_limit * average_window_seconds, average_window_seconds), + ("spike", spike_limit, spike_window_seconds), ) - if not average_decision.allowed: - return average_decision - return self._evaluate_window( - scope=scope, - window_name="spike", - limit=self.spike_limit, - window_seconds=self.spike_window_seconds, - ) + def check(self, scope: str) -> RateLimitDecision: + decision = None + for name, limit, seconds in self._windows: + decision = self._evaluate_window(scope, name, limit, seconds) + if not decision.allowed: + break + return decision def _evaluate_window(self, scope: str, window_name: str, limit: int, window_seconds: int) -> RateLimitDecision: - current_window = int(time.time() // window_seconds) - key = f"{self.key_prefix}:{scope}:{window_name}:{current_window}" + key = f"{self.key_prefix}:{scope}:{window_name}:{int(time.time() // window_seconds)}" pipeline = self.redis_client.pipeline() pipeline.incr(key) pipeline.expire(key, window_seconds + 1) diff --git a/lambdas/mock_pds/tests/test_mock_pds_service.py b/lambdas/mock_pds/tests/test_mock_pds_service.py index 9247569b71..ad51bde34b 100644 --- a/lambdas/mock_pds/tests/test_mock_pds_service.py +++ b/lambdas/mock_pds/tests/test_mock_pds_service.py @@ -7,20 +7,22 @@ from rate_limiter import FixedWindowRateLimiter, RateLimitDecision +def _event(method: str = "GET", nhs_number: str = "9481152782") -> dict: + return {"rawPath": f"/Patient/{nhs_number}", "requestContext": {"http": {"method": method}}} + + +def _decision(allowed: bool, count: int) -> RateLimitDecision: + return RateLimitDecision(allowed=allowed, window_name="spike", count=count, limit=450, window_seconds=1) + + class TestMockPdsService(unittest.TestCase): def setUp(self): self.rate_limiter = Mock(spec=FixedWindowRateLimiter) - self.rate_limiter.check.return_value = RateLimitDecision( - allowed=True, - window_name="spike", - count=1, - limit=450, - window_seconds=1, - ) + self.rate_limiter.check.return_value = _decision(True, 1) self.service = MockPdsService(self.rate_limiter, "Y12345") def test_returns_mock_patient_payload(self): - response = self.service.handle({"rawPath": "/Patient/9481152782", "requestContext": {"http": {"method": "GET"}}}) + response = self.service.handle(_event()) self.assertEqual(response["statusCode"], 200) self.assertEqual(response["headers"]["Content-Type"], "application/fhir+json") @@ -30,23 +32,15 @@ def test_returns_mock_patient_payload(self): self.assertEqual(body["generalPractitioner"][0]["identifier"]["value"], "Y12345") def test_returns_429_when_rate_limit_exceeded(self): - self.rate_limiter.check.return_value = RateLimitDecision( - allowed=False, - window_name="spike", - count=451, - limit=450, - window_seconds=1, - ) + self.rate_limiter.check.return_value = _decision(False, 451) - response = self.service.handle({"rawPath": "/Patient/9481152782", "requestContext": {"http": {"method": "GET"}}}) + response = self.service.handle(_event()) self.assertEqual(response["statusCode"], 429) self.assertEqual(json.loads(response["body"]), {"code": 429, "message": RATE_LIMIT_MESSAGE}) def test_rejects_non_get_requests(self): - response = self.service.handle( - {"rawPath": "/Patient/9481152782", "requestContext": {"http": {"method": "POST"}}} - ) + response = self.service.handle(_event(method="POST")) self.assertEqual(response["statusCode"], 405) @@ -72,12 +66,8 @@ def test_lambda_handler_uses_cached_service(self, mock_redis): mock_service.handle.return_value = {"statusCode": 200} with patch("lambda_handler.MockPdsService", return_value=mock_service): - first_response = lambda_handler( - {"rawPath": "/Patient/123", "requestContext": {"http": {"method": "GET"}}}, None - ) - second_response = lambda_handler( - {"rawPath": "/Patient/456", "requestContext": {"http": {"method": "GET"}}}, None - ) + first_response = lambda_handler(_event(nhs_number="123"), None) + second_response = lambda_handler(_event(nhs_number="456"), None) self.assertEqual(first_response, {"statusCode": 200}) self.assertEqual(second_response, {"statusCode": 200}) @@ -87,6 +77,6 @@ def test_lambda_handler_uses_cached_service(self, mock_redis): def test_lambda_handler_returns_500_on_unhandled_error(self, mock_get_service): mock_get_service.return_value.handle.side_effect = RuntimeError("boom") - response = lambda_handler({"rawPath": "/Patient/123", "requestContext": {"http": {"method": "GET"}}}, None) + response = lambda_handler(_event(nhs_number="123"), None) self.assertEqual(response["statusCode"], 500) diff --git a/lambdas/shared/src/common/api_clients/get_pds_details.py b/lambdas/shared/src/common/api_clients/get_pds_details.py index 2f5766f832..3c032e14d9 100644 --- a/lambdas/shared/src/common/api_clients/get_pds_details.py +++ b/lambdas/shared/src/common/api_clients/get_pds_details.py @@ -12,38 +12,24 @@ _pds_service: PdsService | None = None -def get_pds_environment() -> str: - return os.getenv("PDS_ENV", "int") - - -def get_pds_base_url() -> str | None: - base_url = os.getenv("PDS_BASE_URL", "").strip() - return base_url or None - - def get_pds_service() -> PdsService: global _pds_service if _pds_service is None: - pds_environment = get_pds_environment() - pds_base_url = get_pds_base_url() - authenticator = None - - if pds_base_url is None: - authenticator = AppRestrictedAuth( - secret_manager_client=get_secrets_manager_client(), - environment=pds_environment, - ) - - _pds_service = PdsService(authenticator, pds_environment, base_url=pds_base_url) - + environment = os.getenv("PDS_ENV", "int") + base_url = os.getenv("PDS_BASE_URL", "").strip() or None + authenticator = ( + None + if base_url + else AppRestrictedAuth(secret_manager_client=get_secrets_manager_client(), environment=environment) + ) + _pds_service = PdsService(authenticator, environment, base_url=base_url) return _pds_service # Get Patient details from external service PDS using NHS number from MNS notification def pds_get_patient_details(nhs_number: str) -> dict: try: - patient = get_pds_service().get_patient_details(nhs_number) - return patient + return get_pds_service().get_patient_details(nhs_number) except Exception as e: msg = "Error retrieving patient details from PDS" logger.exception(msg) diff --git a/lambdas/shared/src/common/api_clients/pds_service.py b/lambdas/shared/src/common/api_clients/pds_service.py index 1b73af8861..42f952b4ec 100644 --- a/lambdas/shared/src/common/api_clients/pds_service.py +++ b/lambdas/shared/src/common/api_clients/pds_service.py @@ -15,38 +15,20 @@ def __init__( ): logger.info(f"PdsService init: {environment}") self.authenticator = authenticator - - self.base_url = self._resolve_base_url(environment, base_url) - + host = "api.service.nhs.uk" if environment == "prod" else f"{environment}.api.service.nhs.uk" + self.base_url = base_url.rstrip("/") if base_url else f"https://{host}/personal-demographics/FHIR/R4/Patient" logger.info(f"PDS Service URL: {self.base_url}") - @staticmethod - def _resolve_base_url(environment: str, base_url: str | None) -> str: - if base_url: - return base_url.rstrip("/") - - return ( - f"https://{environment}.api.service.nhs.uk/personal-demographics/FHIR/R4/Patient" - if environment != "prod" - else "https://api.service.nhs.uk/personal-demographics/FHIR/R4/Patient" - ) - def get_patient_details(self, patient_id: str) -> dict | None: - request_headers = { - "X-Request-ID": str(uuid.uuid4()), - "X-Correlation-ID": str(uuid.uuid4()), - } - + headers = {"X-Request-ID": str(uuid.uuid4()), "X-Correlation-ID": str(uuid.uuid4())} if self.authenticator is not None: - access_token = self.authenticator.get_access_token() - request_headers["Authorization"] = f"Bearer {access_token}" + headers["Authorization"] = f"Bearer {self.authenticator.get_access_token()}" - response = request_with_retry_backoff("GET", f"{self.base_url}/{patient_id}", headers=request_headers) + response = request_with_retry_backoff("GET", f"{self.base_url}/{patient_id}", headers=headers) if response.status_code == 200: return response.json() - elif response.status_code == 404: + if response.status_code == 404: logger.info("Patient not found") return None - else: - raise_error_response(response) + raise_error_response(response) From d7421df6039681d56933380803ac40a74b47b772 Mon Sep 17 00:00:00 2001 From: Thomas-Boyle Date: Fri, 17 Apr 2026 13:03:39 +0100 Subject: [PATCH 03/18] chore: Remove MNS performance testing plan for mocked PDS - Deleted the performance testing plan document for MNS integration with mocked PDS. - This document outlined objectives, delivery approaches, and existing architecture for implementing a mocked PDS service. --- mns_perf_with_mocked_pds_4bb74b98.plan.md | 112 ---------------------- 1 file changed, 112 deletions(-) delete mode 100644 mns_perf_with_mocked_pds_4bb74b98.plan.md diff --git a/mns_perf_with_mocked_pds_4bb74b98.plan.md b/mns_perf_with_mocked_pds_4bb74b98.plan.md deleted file mode 100644 index ae7303c875..0000000000 --- a/mns_perf_with_mocked_pds_4bb74b98.plan.md +++ /dev/null @@ -1,112 +0,0 @@ ---- -name: MNS perf with mocked PDS -overview: Design and implement a mocked PDS path in ref so MNS integration/performance testing can run without hitting real PDS, with explicit rate-limit behavior (125 average, 450 spike) and repeatable load tests that quantify current service limits. -todos: - - id: add-mock-pds-service - content: Create mock PDS Lambda/API that returns deterministic patient + GP ODS data and standardized 429 response contract. - status: pending - - id: wire-ref-routing - content: Update infrastructure and ref environment config so PDS calls in ref route to mock endpoint while preserving non-ref behavior. - status: pending - - id: implement-rate-limit-policy - content: Implement and configure average/spike limiting (125/450 rps) with observable metrics and deterministic testability. - status: pending - - id: add-functional-tests - content: Add unit/integration tests covering successful data retrieval and 429 rate-limit scenarios for MNS publishing dependency on PDS. - status: pending - - id: extend-locust-scenarios - content: Add Locust traffic profiles (average, spike, ramp) and commands to quantify throughput, latency, and 429 behavior. - status: pending - - id: publish-runbook-guidance - content: Update existing perf docs with execution steps, expected outputs, and interpretation for campaign-capacity decisions. - status: pending -isProject: false ---- - -# Plan: Performance test MNS with mocked PDS in ref - -## Objectives - -- Provide a **mocked PDS Lambda/service** in `ref` so MNS flows can retrieve required patient/GP (ODS) data without calling real PDS. -- Enforce and validate rate limits aligned to acceptance criteria: - - average threshold: `125 rps` - - spike threshold: `450 rps` - - overflow response: HTTP `429`, body/message `Mock PDS rate limit has been exceeded` -- Run repeatable load tests to determine current throughput ceiling and failure characteristics for campaign peaks. - -## Existing architecture to leverage - -- MNS publisher resolves GP data via shared PDS client in [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py) and [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py). -- Notification payload construction (including GP ODS use) is in [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/mns_publisher/src/create_notification.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/mns_publisher/src/create_notification.py). -- Ref-targeted performance tooling already exists in Locust at [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py). - -## Delivery approach - -1. **Introduce Mock PDS API Lambda** - - Add a dedicated Lambda that returns deterministic Patient payloads containing required demographic + GP ODS fields for NHS number lookup. - - Support two behaviors via env/config: - - normal mode: return valid mock PDS patient data - - throttled mode: return `429` once configured limit is exceeded - - Keep schema compatible with current parser paths in MNS/id-sync code to avoid functional drift. - -2. **Wire ref to mock endpoint, keep other envs safe** - - Add infra for mock PDS API (Lambda + API Gateway route) in instance modules. - - In `ref` tfvars, route `PDS_ENV`/PDS base URL resolution to the mock endpoint only. - - Preserve existing behavior for non-ref environments unless explicitly configured. - -3. **Implement rate-limiting policy in mock** - - Use API Gateway throttling and/or in-Lambda token-bucket counters (final choice based on easiest deterministic assertion in tests) to guarantee: - - sustained traffic over `125 rps` observable as throttling trend - - spike over `450 rps` promptly returns `429` - - Standardize response body contract: `{ "code": 429, "message": "Mock PDS rate limit has been exceeded" }`. - -4. **Add automated validation tests** - - Unit tests for mock response shape and error contract. - - Integration tests for MNS publisher path proving: - - Scenario 1: required PDS data is returned in ref via mock (including ODS usage path) - - Scenario 2: limit breaches produce `429` with expected message. - -5. **Extend perf tests for campaign-style load** - - Add Locust scenarios to drive traffic profiles around: - - baseline (`~125 rps`) - - spike (`>450 rps` burst) - - stepped ramp (to identify knee point and failure ratio) - - Capture and report: success %, 429 %, latency p50/p95/p99, and MNS publish success under each load shape. - -6. **Operational guardrails and rollout** - - Add runbook notes in existing perf test docs for executing ref tests safely. - - Ensure alarms/log queries identify throttle onset and downstream effects in MNS publisher. - - Keep mock enabled as mandatory integration path while real PDS endpoint is off-limits. - -## Suggested implementation sequence by file area - -- Shared PDS client behavior switch: - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/pds_service.py) - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/lambdas/shared/src/common/api_clients/get_pds_details.py) -- New mock PDS lambda + tests under `lambdas/` (new module mirrored to existing lambda structure). -- Infra wiring (API/Lambda/env mapping): - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance) - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance/environments/dev/ref/variables.tfvars`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/infrastructure/instance/environments/dev/ref/variables.tfvars) -- Perf scripts/docs: - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/src/locustfile.py) - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/README.md`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/README.md) - - [`/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/Makefile`](/Users/thomasboyle/Documents/VEDS Immunisation FHIR API/immunisation-fhir-api/tests/perf_tests/Makefile) - -## Flow diagram - -```mermaid -flowchart LR -clientLoad[LocustLoadInRef] --> api[ImmsApiRef] -api --> events[EventsAndDeltaFlow] -events --> mnsPublisher[MnsPublisherLambda] -mnsPublisher --> pdsMock[MockPdsApiLambda] -pdsMock -->|"200 patient data with GP ODS"| mnsPublisher -pdsMock -->|"429 Mock PDS rate limit has been exceeded"| mnsPublisher -mnsPublisher --> mnsTarget[MnsPublishPath] -``` - -## Acceptance criteria mapping - -- **Scenario 1:** mock service in `ref` returns required patient data for MNS publishing path, including GP ODS extraction path. -- **Scenario 2:** calls above configured `450 rps` (and sustained over `125 rps`) return HTTP `429` with exact message contract. -- **Outcome:** measured traffic limits and failure envelope documented from repeatable load profiles for Autumn/Spring campaign readiness decisions. From 98dc17b7454aac09bd90276594a624fe05178a0d Mon Sep 17 00:00:00 2001 From: Thomas-Boyle Date: Fri, 17 Apr 2026 13:17:08 +0100 Subject: [PATCH 04/18] refactor: Optimize Mock PDS service initialization in lambda_handler - Removed the global service retrieval function and directly initialized the MockPdsService with a Redis client and rate limiter. - Updated the lambda_handler to use the initialized service directly, improving performance and readability. - Adjusted unit tests to accommodate the new service initialization approach, ensuring proper mocking and error handling. --- lambdas/mock_pds/src/lambda_handler.py | 37 ++++++++----------- .../mock_pds/tests/test_mock_pds_service.py | 31 ++++++++++------ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/lambdas/mock_pds/src/lambda_handler.py b/lambdas/mock_pds/src/lambda_handler.py index 1d4da28482..a1d3cd47a4 100644 --- a/lambdas/mock_pds/src/lambda_handler.py +++ b/lambdas/mock_pds/src/lambda_handler.py @@ -9,32 +9,25 @@ logger = logging.getLogger() logger.setLevel(logging.INFO) -_mock_pds_service: MockPdsService | None = None - - -def get_mock_pds_service() -> MockPdsService: - global _mock_pds_service - if _mock_pds_service is None: - redis_client = redis.Redis( - host=os.environ["REDIS_HOST"], - port=int(os.getenv("REDIS_PORT", "6379")), - decode_responses=True, - ) - rate_limiter = FixedWindowRateLimiter( - redis_client=redis_client, - key_prefix="mock-pds", - average_limit=int(os.getenv("MOCK_PDS_AVERAGE_LIMIT", "125")), - average_window_seconds=int(os.getenv("MOCK_PDS_AVERAGE_WINDOW_SECONDS", "60")), - spike_limit=int(os.getenv("MOCK_PDS_SPIKE_LIMIT", "450")), - spike_window_seconds=int(os.getenv("MOCK_PDS_SPIKE_WINDOW_SECONDS", "1")), - ) - _mock_pds_service = MockPdsService(rate_limiter, os.getenv("MOCK_PDS_GP_ODS_CODE", "Y12345")) - return _mock_pds_service +_redis_client = redis.Redis( + host=os.environ["REDIS_HOST"], + port=int(os.getenv("REDIS_PORT", "6379")), + decode_responses=True, +) +_rate_limiter = FixedWindowRateLimiter( + redis_client=_redis_client, + key_prefix="mock-pds", + average_limit=int(os.getenv("MOCK_PDS_AVERAGE_LIMIT", "125")), + average_window_seconds=int(os.getenv("MOCK_PDS_AVERAGE_WINDOW_SECONDS", "60")), + spike_limit=int(os.getenv("MOCK_PDS_SPIKE_LIMIT", "450")), + spike_window_seconds=int(os.getenv("MOCK_PDS_SPIKE_WINDOW_SECONDS", "1")), +) +_mock_pds_service = MockPdsService(_rate_limiter, os.getenv("MOCK_PDS_GP_ODS_CODE", "Y12345")) def lambda_handler(event, context): try: - return get_mock_pds_service().handle(event) + return _mock_pds_service.handle(event) except Exception: logger.exception("Mock PDS failed to handle request") return { diff --git a/lambdas/mock_pds/tests/test_mock_pds_service.py b/lambdas/mock_pds/tests/test_mock_pds_service.py index ad51bde34b..02c73ec288 100644 --- a/lambdas/mock_pds/tests/test_mock_pds_service.py +++ b/lambdas/mock_pds/tests/test_mock_pds_service.py @@ -1,8 +1,13 @@ +import importlib import json +import os import unittest from unittest.mock import Mock, patch -from lambda_handler import get_mock_pds_service, lambda_handler +os.environ.setdefault("REDIS_HOST", "test-redis-host") +os.environ.setdefault("REDIS_PORT", "6379") + +import lambda_handler as lambda_handler_module from mock_pds_service import RATE_LIMIT_MESSAGE, MockPdsService from rate_limiter import FixedWindowRateLimiter, RateLimitDecision @@ -47,7 +52,7 @@ def test_rejects_non_get_requests(self): class TestLambdaHandler(unittest.TestCase): def tearDown(self): - get_mock_pds_service.__globals__["_mock_pds_service"] = None + importlib.reload(lambda_handler_module) @patch.dict( "os.environ", @@ -60,23 +65,25 @@ def tearDown(self): }, clear=False, ) - @patch("lambda_handler.redis.Redis") - def test_lambda_handler_uses_cached_service(self, mock_redis): + @patch("mock_pds_service.MockPdsService") + @patch("redis.Redis") + def test_lambda_handler_uses_cached_service(self, mock_redis, mock_pds_cls): mock_service = Mock() mock_service.handle.return_value = {"statusCode": 200} + mock_pds_cls.return_value = mock_service - with patch("lambda_handler.MockPdsService", return_value=mock_service): - first_response = lambda_handler(_event(nhs_number="123"), None) - second_response = lambda_handler(_event(nhs_number="456"), None) + importlib.reload(lambda_handler_module) + first_response = lambda_handler_module.lambda_handler(_event(nhs_number="123"), None) + second_response = lambda_handler_module.lambda_handler(_event(nhs_number="456"), None) self.assertEqual(first_response, {"statusCode": 200}) self.assertEqual(second_response, {"statusCode": 200}) mock_redis.assert_called_once_with(host="mock-redis", port=6379, decode_responses=True) - @patch("lambda_handler.get_mock_pds_service") - def test_lambda_handler_returns_500_on_unhandled_error(self, mock_get_service): - mock_get_service.return_value.handle.side_effect = RuntimeError("boom") - - response = lambda_handler(_event(nhs_number="123"), None) + def test_lambda_handler_returns_500_on_unhandled_error(self): + mock_svc = Mock() + mock_svc.handle.side_effect = RuntimeError("boom") + with patch.object(lambda_handler_module, "_mock_pds_service", mock_svc): + response = lambda_handler_module.lambda_handler(_event(nhs_number="123"), None) self.assertEqual(response["statusCode"], 500) From d7692be395c278f19189d25d0baad8c9352913a5 Mon Sep 17 00:00:00 2001 From: Thomas-Boyle Date: Fri, 17 Apr 2026 13:29:15 +0100 Subject: [PATCH 05/18] feat: Enhance Mock PDS service with additional coverage reporting and tests - Updated sonar-project.properties to include coverage report for mock_pds. - Added new unit tests for Mock PDS service to improve test coverage and handle edge cases. - Introduced a new test file for rate limiting functionality in the Mock PDS service. - Updated the quality-checks workflow to run tests for the new mock_pds service. --- .github/workflows/quality-checks.yml | 11 ++ lambdas/mock_pds/poetry.lock | 123 +++++++++++++++++- lambdas/mock_pds/pyproject.toml | 1 + .../mock_pds/tests/test_mock_pds_service.py | 41 ++++++ lambdas/mock_pds/tests/test_rate_limiter.py | 87 +++++++++++++ .../api_clients/test_pds_details.py | 11 ++ sonar-project.properties | 2 +- 7 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 lambdas/mock_pds/tests/test_rate_limiter.py diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 2b97a03671..4edb916a54 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -184,6 +184,17 @@ jobs: poetry run coverage run --source=src -m unittest discover || echo "mns_publisher tests failed" >> ../../failed_tests.txt poetry run coverage xml -o ../../mns_publisher-coverage.xml + - name: Run unittest with coverage-mock-pds + working-directory: lambdas/mock_pds + id: mock_pds + env: + PYTHONPATH: ${{ env.LAMBDA_PATH }}/mock_pds/src:${{ env.LAMBDA_PATH }}/mock_pds/tests + continue-on-error: true + run: | + poetry install + poetry run coverage run --source=src -m unittest discover -s tests -v || echo "mock_pds tests failed" >> ../../failed_tests.txt + poetry run coverage xml -o ../../mock_pds-coverage.xml + - name: Run unittest with coverage-mns-subscription working-directory: lambdas/mns_subscription id: mns_subscription diff --git a/lambdas/mock_pds/poetry.lock b/lambdas/mock_pds/poetry.lock index 6a34806349..af2cf3d91d 100644 --- a/lambdas/mock_pds/poetry.lock +++ b/lambdas/mock_pds/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.1.4 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.3.2 and should not be changed by hand. [[package]] name = "async-timeout" @@ -13,6 +13,125 @@ files = [ {file = "async_timeout-5.0.1.tar.gz", hash = "sha256:d9321a7a3d5a6a5e187e824d2fa0793ce379a202935782d555d6e9d2735677d3"}, ] +[[package]] +name = "coverage" +version = "7.13.5" +description = "Code coverage measurement for Python" +optional = false +python-versions = ">=3.10" +groups = ["main"] +files = [ + {file = "coverage-7.13.5-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:e0723d2c96324561b9aa76fb982406e11d93cdb388a7a7da2b16e04719cf7ca5"}, + {file = "coverage-7.13.5-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:52f444e86475992506b32d4e5ca55c24fc88d73bcbda0e9745095b28ef4dc0cf"}, + {file = "coverage-7.13.5-cp310-cp310-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:704de6328e3d612a8f6c07000a878ff38181ec3263d5a11da1db294fa6a9bdf8"}, + {file = "coverage-7.13.5-cp310-cp310-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:a1a6d79a14e1ec1832cabc833898636ad5f3754a678ef8bb4908515208bf84f4"}, + {file = "coverage-7.13.5-cp310-cp310-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:79060214983769c7ba3f0cee10b54c97609dca4d478fa1aa32b914480fd5738d"}, + {file = "coverage-7.13.5-cp310-cp310-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:356e76b46783a98c2a2fe81ec79df4883a1e62895ea952968fb253c114e7f930"}, + {file = "coverage-7.13.5-cp310-cp310-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:0cef0cdec915d11254a7f549c1170afecce708d30610c6abdded1f74e581666d"}, + {file = "coverage-7.13.5-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:dc022073d063b25a402454e5712ef9e007113e3a676b96c5f29b2bda29352f40"}, + {file = "coverage-7.13.5-cp310-cp310-musllinux_1_2_i686.whl", hash = "sha256:9b74db26dfea4f4e50d48a4602207cd1e78be33182bc9cbf22da94f332f99878"}, + {file = "coverage-7.13.5-cp310-cp310-musllinux_1_2_ppc64le.whl", hash = "sha256:ad146744ca4fd09b50c482650e3c1b1f4dfa1d4792e0a04a369c7f23336f0400"}, + {file = "coverage-7.13.5-cp310-cp310-musllinux_1_2_riscv64.whl", hash = "sha256:c555b48be1853fe3997c11c4bd521cdd9a9612352de01fa4508f16ec341e6fe0"}, + {file = "coverage-7.13.5-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:7034b5c56a58ae5e85f23949d52c14aca2cfc6848a31764995b7de88f13a1ea0"}, + {file = "coverage-7.13.5-cp310-cp310-win32.whl", hash = "sha256:eb7fdf1ef130660e7415e0253a01a7d5a88c9c4d158bcf75cbbd922fd65a5b58"}, + {file = "coverage-7.13.5-cp310-cp310-win_amd64.whl", hash = "sha256:3e1bb5f6c78feeb1be3475789b14a0f0a5b47d505bfc7267126ccbd50289999e"}, + {file = "coverage-7.13.5-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:66a80c616f80181f4d643b0f9e709d97bcea413ecd9631e1dedc7401c8e6695d"}, + {file = "coverage-7.13.5-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:145ede53ccbafb297c1c9287f788d1bc3efd6c900da23bf6931b09eafc931587"}, + {file = "coverage-7.13.5-cp311-cp311-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:0672854dc733c342fa3e957e0605256d2bf5934feeac328da9e0b5449634a642"}, + {file = "coverage-7.13.5-cp311-cp311-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:ec10e2a42b41c923c2209b846126c6582db5e43a33157e9870ba9fb70dc7854b"}, + {file = "coverage-7.13.5-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:be3d4bbad9d4b037791794ddeedd7d64a56f5933a2c1373e18e9e568b9141686"}, + {file = "coverage-7.13.5-cp311-cp311-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:4d2afbc5cc54d286bfb54541aa50b64cdb07a718227168c87b9e2fb8f25e1743"}, + {file = "coverage-7.13.5-cp311-cp311-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:3ad050321264c49c2fa67bb599100456fc51d004b82534f379d16445da40fb75"}, + {file = "coverage-7.13.5-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:7300c8a6d13335b29bb76d7651c66af6bd8658517c43499f110ddc6717bfc209"}, + {file = "coverage-7.13.5-cp311-cp311-musllinux_1_2_i686.whl", hash = "sha256:eb07647a5738b89baab047f14edd18ded523de60f3b30e75c2acc826f79c839a"}, + {file = "coverage-7.13.5-cp311-cp311-musllinux_1_2_ppc64le.whl", hash = "sha256:9adb6688e3b53adffefd4a52d72cbd8b02602bfb8f74dcd862337182fd4d1a4e"}, + {file = "coverage-7.13.5-cp311-cp311-musllinux_1_2_riscv64.whl", hash = "sha256:7c8d4bc913dd70b93488d6c496c77f3aff5ea99a07e36a18f865bca55adef8bd"}, + {file = "coverage-7.13.5-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:0e3c426ffc4cd952f54ee9ffbdd10345709ecc78a3ecfd796a57236bfad0b9b8"}, + {file = "coverage-7.13.5-cp311-cp311-win32.whl", hash = "sha256:259b69bb83ad9894c4b25be2528139eecba9a82646ebdda2d9db1ba28424a6bf"}, + {file = "coverage-7.13.5-cp311-cp311-win_amd64.whl", hash = "sha256:258354455f4e86e3e9d0d17571d522e13b4e1e19bf0f8596bcf9476d61e7d8a9"}, + {file = "coverage-7.13.5-cp311-cp311-win_arm64.whl", hash = "sha256:bff95879c33ec8da99fc9b6fe345ddb5be6414b41d6d1ad1c8f188d26f36e028"}, + {file = "coverage-7.13.5-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:460cf0114c5016fa841214ff5564aa4864f11948da9440bc97e21ad1f4ba1e01"}, + {file = "coverage-7.13.5-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:0e223ce4b4ed47f065bfb123687686512e37629be25cc63728557ae7db261422"}, + {file = "coverage-7.13.5-cp312-cp312-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:6e3370441f4513c6252bf042b9c36d22491142385049243253c7e48398a15a9f"}, + {file = "coverage-7.13.5-cp312-cp312-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:03ccc709a17a1de074fb1d11f217342fb0d2b1582ed544f554fc9fc3f07e95f5"}, + {file = "coverage-7.13.5-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:3f4818d065964db3c1c66dc0fbdac5ac692ecbc875555e13374fdbe7eedb4376"}, + {file = "coverage-7.13.5-cp312-cp312-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:012d5319e66e9d5a218834642d6c35d265515a62f01157a45bcc036ecf947256"}, + {file = "coverage-7.13.5-cp312-cp312-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:8dd02af98971bdb956363e4827d34425cb3df19ee550ef92855b0acb9c7ce51c"}, + {file = "coverage-7.13.5-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:f08fd75c50a760c7eb068ae823777268daaf16a80b918fa58eea888f8e3919f5"}, + {file = "coverage-7.13.5-cp312-cp312-musllinux_1_2_i686.whl", hash = "sha256:843ea8643cf967d1ac7e8ecd4bb00c99135adf4816c0c0593fdcc47b597fcf09"}, + {file = "coverage-7.13.5-cp312-cp312-musllinux_1_2_ppc64le.whl", hash = "sha256:9d44d7aa963820b1b971dbecd90bfe5fe8f81cff79787eb6cca15750bd2f79b9"}, + {file = "coverage-7.13.5-cp312-cp312-musllinux_1_2_riscv64.whl", hash = "sha256:7132bed4bd7b836200c591410ae7d97bf7ae8be6fc87d160b2bd881df929e7bf"}, + {file = "coverage-7.13.5-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:a698e363641b98843c517817db75373c83254781426e94ada3197cabbc2c919c"}, + {file = "coverage-7.13.5-cp312-cp312-win32.whl", hash = "sha256:bdba0a6b8812e8c7df002d908a9a2ea3c36e92611b5708633c50869e6d922fdf"}, + {file = "coverage-7.13.5-cp312-cp312-win_amd64.whl", hash = "sha256:d2c87e0c473a10bffe991502eac389220533024c8082ec1ce849f4218dded810"}, + {file = "coverage-7.13.5-cp312-cp312-win_arm64.whl", hash = "sha256:bf69236a9a81bdca3bff53796237aab096cdbf8d78a66ad61e992d9dac7eb2de"}, + {file = "coverage-7.13.5-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:5ec4af212df513e399cf11610cc27063f1586419e814755ab362e50a85ea69c1"}, + {file = "coverage-7.13.5-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:941617e518602e2d64942c88ec8499f7fbd49d3f6c4327d3a71d43a1973032f3"}, + {file = "coverage-7.13.5-cp313-cp313-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:da305e9937617ee95c2e39d8ff9f040e0487cbf1ac174f777ed5eddd7a7c1f26"}, + {file = "coverage-7.13.5-cp313-cp313-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:78e696e1cc714e57e8b25760b33a8b1026b7048d270140d25dafe1b0a1ee05a3"}, + {file = "coverage-7.13.5-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:02ca0eed225b2ff301c474aeeeae27d26e2537942aa0f87491d3e147e784a82b"}, + {file = "coverage-7.13.5-cp313-cp313-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:04690832cbea4e4663d9149e05dba142546ca05cb1848816760e7f58285c970a"}, + {file = "coverage-7.13.5-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:0590e44dd2745c696a778f7bab6aa95256de2cbc8b8cff4f7db8ff09813d6969"}, + {file = "coverage-7.13.5-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:d7cfad2d6d81dd298ab6b89fe72c3b7b05ec7544bdda3b707ddaecff8d25c161"}, + {file = "coverage-7.13.5-cp313-cp313-musllinux_1_2_i686.whl", hash = "sha256:e092b9499de38ae0fbfbc603a74660eb6ff3e869e507b50d85a13b6db9863e15"}, + {file = "coverage-7.13.5-cp313-cp313-musllinux_1_2_ppc64le.whl", hash = "sha256:48c39bc4a04d983a54a705a6389512883d4a3b9862991b3617d547940e9f52b1"}, + {file = "coverage-7.13.5-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:2d3807015f138ffea1ed9afeeb8624fd781703f2858b62a8dd8da5a0994c57b6"}, + {file = "coverage-7.13.5-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:ee2aa19e03161671ec964004fb74b2257805d9710bf14a5c704558b9d8dbaf17"}, + {file = "coverage-7.13.5-cp313-cp313-win32.whl", hash = "sha256:ce1998c0483007608c8382f4ff50164bfc5bd07a2246dd272aa4043b75e61e85"}, + {file = "coverage-7.13.5-cp313-cp313-win_amd64.whl", hash = "sha256:631efb83f01569670a5e866ceb80fe483e7c159fac6f167e6571522636104a0b"}, + {file = "coverage-7.13.5-cp313-cp313-win_arm64.whl", hash = "sha256:f4cd16206ad171cbc2470dbea9103cf9a7607d5fe8c242fdf1edf36174020664"}, + {file = "coverage-7.13.5-cp313-cp313t-macosx_10_13_x86_64.whl", hash = "sha256:0428cbef5783ad91fe240f673cc1f76b25e74bbfe1a13115e4aa30d3f538162d"}, + {file = "coverage-7.13.5-cp313-cp313t-macosx_11_0_arm64.whl", hash = "sha256:e0b216a19534b2427cc201a26c25da4a48633f29a487c61258643e89d28200c0"}, + {file = "coverage-7.13.5-cp313-cp313t-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:972a9cd27894afe4bc2b1480107054e062df08e671df7c2f18c205e805ccd806"}, + {file = "coverage-7.13.5-cp313-cp313t-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:4b59148601efcd2bac8c4dbf1f0ad6391693ccf7a74b8205781751637076aee3"}, + {file = "coverage-7.13.5-cp313-cp313t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:505d7083c8b0c87a8fa8c07370c285847c1f77739b22e299ad75a6af6c32c5c9"}, + {file = "coverage-7.13.5-cp313-cp313t-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:60365289c3741e4db327e7baff2a4aaacf22f788e80fa4683393891b70a89fbd"}, + {file = "coverage-7.13.5-cp313-cp313t-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:1b88c69c8ef5d4b6fe7dea66d6636056a0f6a7527c440e890cf9259011f5e606"}, + {file = "coverage-7.13.5-cp313-cp313t-musllinux_1_2_aarch64.whl", hash = "sha256:5b13955d31d1633cf9376908089b7cebe7d15ddad7aeaabcbe969a595a97e95e"}, + {file = "coverage-7.13.5-cp313-cp313t-musllinux_1_2_i686.whl", hash = "sha256:f70c9ab2595c56f81a89620e22899eea8b212a4041bd728ac6f4a28bf5d3ddd0"}, + {file = "coverage-7.13.5-cp313-cp313t-musllinux_1_2_ppc64le.whl", hash = "sha256:084b84a8c63e8d6fc7e3931b316a9bcafca1458d753c539db82d31ed20091a87"}, + {file = "coverage-7.13.5-cp313-cp313t-musllinux_1_2_riscv64.whl", hash = "sha256:ad14385487393e386e2ea988b09d62dd42c397662ac2dabc3832d71253eee479"}, + {file = "coverage-7.13.5-cp313-cp313t-musllinux_1_2_x86_64.whl", hash = "sha256:7f2c47b36fe7709a6e83bfadf4eefb90bd25fbe4014d715224c4316f808e59a2"}, + {file = "coverage-7.13.5-cp313-cp313t-win32.whl", hash = "sha256:67e9bc5449801fad0e5dff329499fb090ba4c5800b86805c80617b4e29809b2a"}, + {file = "coverage-7.13.5-cp313-cp313t-win_amd64.whl", hash = "sha256:da86cdcf10d2519e10cabb8ac2de03da1bcb6e4853790b7fbd48523332e3a819"}, + {file = "coverage-7.13.5-cp313-cp313t-win_arm64.whl", hash = "sha256:0ecf12ecb326fe2c339d93fc131816f3a7367d223db37817208905c89bded911"}, + {file = "coverage-7.13.5-cp314-cp314-macosx_10_15_x86_64.whl", hash = "sha256:fbabfaceaeb587e16f7008f7795cd80d20ec548dc7f94fbb0d4ec2e038ce563f"}, + {file = "coverage-7.13.5-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:9bb2a28101a443669a423b665939381084412b81c3f8c0fcfbac57f4e30b5b8e"}, + {file = "coverage-7.13.5-cp314-cp314-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:bd3a2fbc1c6cccb3c5106140d87cc6a8715110373ef42b63cf5aea29df8c217a"}, + {file = "coverage-7.13.5-cp314-cp314-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:6c36ddb64ed9d7e496028d1d00dfec3e428e0aabf4006583bb1839958d280510"}, + {file = "coverage-7.13.5-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:380e8e9084d8eb38db3a9176a1a4f3c0082c3806fa0dc882d1d87abc3c789247"}, + {file = "coverage-7.13.5-cp314-cp314-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:e808af52a0513762df4d945ea164a24b37f2f518cbe97e03deaa0ee66139b4d6"}, + {file = "coverage-7.13.5-cp314-cp314-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:e301d30dd7e95ae068671d746ba8c34e945a82682e62918e41b2679acd2051a0"}, + {file = "coverage-7.13.5-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:800bc829053c80d240a687ceeb927a94fd108bbdc68dfbe505d0d75ab578a882"}, + {file = "coverage-7.13.5-cp314-cp314-musllinux_1_2_i686.whl", hash = "sha256:0b67af5492adb31940ee418a5a655c28e48165da5afab8c7fa6fd72a142f8740"}, + {file = "coverage-7.13.5-cp314-cp314-musllinux_1_2_ppc64le.whl", hash = "sha256:c9136ff29c3a91e25b1d1552b5308e53a1e0653a23e53b6366d7c2dcbbaf8a16"}, + {file = "coverage-7.13.5-cp314-cp314-musllinux_1_2_riscv64.whl", hash = "sha256:cff784eef7f0b8f6cb28804fbddcfa99f89efe4cc35fb5627e3ac58f91ed3ac0"}, + {file = "coverage-7.13.5-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:68a4953be99b17ac3c23b6efbc8a38330d99680c9458927491d18700ef23ded0"}, + {file = "coverage-7.13.5-cp314-cp314-win32.whl", hash = "sha256:35a31f2b1578185fbe6aa2e74cea1b1d0bbf4c552774247d9160d29b80ed56cc"}, + {file = "coverage-7.13.5-cp314-cp314-win_amd64.whl", hash = "sha256:2aa055ae1857258f9e0045be26a6d62bdb47a72448b62d7b55f4820f361a2633"}, + {file = "coverage-7.13.5-cp314-cp314-win_arm64.whl", hash = "sha256:1b11eef33edeae9d142f9b4358edb76273b3bfd30bc3df9a4f95d0e49caf94e8"}, + {file = "coverage-7.13.5-cp314-cp314t-macosx_10_15_x86_64.whl", hash = "sha256:10a0c37f0b646eaff7cce1874c31d1f1ccb297688d4c747291f4f4c70741cc8b"}, + {file = "coverage-7.13.5-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:b5db73ba3c41c7008037fa731ad5459fc3944cb7452fc0aa9f822ad3533c583c"}, + {file = "coverage-7.13.5-cp314-cp314t-manylinux1_i686.manylinux_2_28_i686.manylinux_2_5_i686.whl", hash = "sha256:750db93a81e3e5a9831b534be7b1229df848b2e125a604fe6651e48aa070e5f9"}, + {file = "coverage-7.13.5-cp314-cp314t-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl", hash = "sha256:9ddb4f4a5479f2539644be484da179b653273bca1a323947d48ab107b3ed1f29"}, + {file = "coverage-7.13.5-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:d8a7a2049c14f413163e2bdabd37e41179b1d1ccb10ffc6ccc4b7a718429c607"}, + {file = "coverage-7.13.5-cp314-cp314t-manylinux2014_ppc64le.manylinux_2_17_ppc64le.manylinux_2_28_ppc64le.whl", hash = "sha256:e1c85e0b6c05c592ea6d8768a66a254bfb3874b53774b12d4c89c481eb78cb90"}, + {file = "coverage-7.13.5-cp314-cp314t-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:777c4d1eff1b67876139d24288aaf1817f6c03d6bae9c5cc8d27b83bcfe38fe3"}, + {file = "coverage-7.13.5-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:6697e29b93707167687543480a40f0db8f356e86d9f67ddf2e37e2dfd91a9dab"}, + {file = "coverage-7.13.5-cp314-cp314t-musllinux_1_2_i686.whl", hash = "sha256:8fdf453a942c3e4d99bd80088141c4c6960bb232c409d9c3558e2dbaa3998562"}, + {file = "coverage-7.13.5-cp314-cp314t-musllinux_1_2_ppc64le.whl", hash = "sha256:32ca0c0114c9834a43f045a87dcebd69d108d8ffb666957ea65aa132f50332e2"}, + {file = "coverage-7.13.5-cp314-cp314t-musllinux_1_2_riscv64.whl", hash = "sha256:8769751c10f339021e2638cd354e13adeac54004d1941119b2c96fe5276d45ea"}, + {file = "coverage-7.13.5-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:cec2d83125531bd153175354055cdb7a09987af08a9430bd173c937c6d0fba2a"}, + {file = "coverage-7.13.5-cp314-cp314t-win32.whl", hash = "sha256:0cd9ed7a8b181775459296e402ca4fb27db1279740a24e93b3b41942ebe4b215"}, + {file = "coverage-7.13.5-cp314-cp314t-win_amd64.whl", hash = "sha256:301e3b7dfefecaca37c9f1aa6f0049b7d4ab8dd933742b607765d757aca77d43"}, + {file = "coverage-7.13.5-cp314-cp314t-win_arm64.whl", hash = "sha256:9dacc2ad679b292709e0f5fc1ac74a6d4d5562e424058962c7bb0c658ad25e45"}, + {file = "coverage-7.13.5-py3-none-any.whl", hash = "sha256:34b02417cf070e173989b3db962f7ed56d2f644307b2cf9d5a0f258e13084a61"}, + {file = "coverage-7.13.5.tar.gz", hash = "sha256:c81f6515c4c40141f83f502b07bbfa5c240ba25bbe73da7b33f1e5b6120ff179"}, +] + +[package.extras] +toml = ["tomli ; python_full_version <= \"3.11.0a6\""] + [[package]] name = "redis" version = "6.4.0" @@ -36,4 +155,4 @@ ocsp = ["cryptography (>=36.0.1)", "pyopenssl (>=20.0.1)", "requests (>=2.31.0)" [metadata] lock-version = "2.1" python-versions = "~3.11" -content-hash = "5d3a40c5ca25affeefbf1b1b07bc8003305056af1804c0b29f4f02c2ac64f00d" +content-hash = "0e7f8698fcaf597c39319073cc8d6cec4d52d3c55ca8151c2c44192723d20aa7" diff --git a/lambdas/mock_pds/pyproject.toml b/lambdas/mock_pds/pyproject.toml index 2e82d4ce05..99cca3dd38 100644 --- a/lambdas/mock_pds/pyproject.toml +++ b/lambdas/mock_pds/pyproject.toml @@ -9,6 +9,7 @@ packages = [{include = "src"}] [tool.poetry.dependencies] python = "~3.11" redis = "^6.1.0" +coverage = "^7.13.2" [build-system] requires = ["poetry-core >= 1.5.0"] diff --git a/lambdas/mock_pds/tests/test_mock_pds_service.py b/lambdas/mock_pds/tests/test_mock_pds_service.py index 02c73ec288..8134b1df6e 100644 --- a/lambdas/mock_pds/tests/test_mock_pds_service.py +++ b/lambdas/mock_pds/tests/test_mock_pds_service.py @@ -49,6 +49,47 @@ def test_rejects_non_get_requests(self): self.assertEqual(response["statusCode"], 405) + def test_returns_400_when_patient_id_missing(self): + response = self.service.handle({"rawPath": "/Patient/", "requestContext": {"http": {"method": "GET"}}}) + + self.assertEqual(response["statusCode"], 400) + self.assertEqual(json.loads(response["body"]), {"code": 400, "message": "Patient id is required"}) + + def test_returns_400_when_path_has_no_patient_segment(self): + response = self.service.handle({"rawPath": "/metadata", "requestContext": {"http": {"method": "GET"}}}) + + self.assertEqual(response["statusCode"], 400) + + def test_accepts_http_method_from_api_gateway_rest_shape(self): + event = {"path": "/Patient/9481152782", "httpMethod": "GET"} + response = self.service.handle(event) + + self.assertEqual(response["statusCode"], 200) + + def test_defaults_to_get_when_method_absent(self): + event = {"rawPath": "/Patient/9481152782"} + response = self.service.handle(event) + + self.assertEqual(response["statusCode"], 200) + + def test_extracts_patient_from_path_when_raw_path_missing(self): + event = {"path": "/Patient/9912003888", "httpMethod": "GET"} + response = self.service.handle(event) + + self.assertEqual(json.loads(response["body"])["id"], "9912003888") + + def test_extracts_patient_from_request_context_http_path(self): + event = {"requestContext": {"http": {"method": "GET", "path": "/Patient/1111111111"}}} + response = self.service.handle(event) + + self.assertEqual(json.loads(response["body"])["id"], "1111111111") + + def test_build_patient_handles_short_nhs_number(self): + body = self.service._build_patient("12") + + self.assertEqual(body["id"], "12") + self.assertEqual(body["birthDate"][:5], "1985-") + class TestLambdaHandler(unittest.TestCase): def tearDown(self): diff --git a/lambdas/mock_pds/tests/test_rate_limiter.py b/lambdas/mock_pds/tests/test_rate_limiter.py new file mode 100644 index 0000000000..f78630960d --- /dev/null +++ b/lambdas/mock_pds/tests/test_rate_limiter.py @@ -0,0 +1,87 @@ +import unittest +from unittest.mock import MagicMock, patch + +from rate_limiter import FixedWindowRateLimiter + + +class TestFixedWindowRateLimiter(unittest.TestCase): + def _limiter(self, redis_client: MagicMock, **kwargs) -> FixedWindowRateLimiter: + defaults = { + "key_prefix": "pfx", + "average_limit": 2, + "average_window_seconds": 60, + "spike_limit": 10, + "spike_window_seconds": 1, + } + defaults.update(kwargs) + return FixedWindowRateLimiter(redis_client, **defaults) + + def _pipeline_mock(self, execute_result: list): + pipeline = MagicMock() + pipeline.incr.return_value = pipeline + pipeline.expire.return_value = pipeline + pipeline.execute.return_value = execute_result + return pipeline + + def test_check_allows_when_both_windows_under_limit(self): + redis_client = MagicMock() + pipe_avg = self._pipeline_mock([1, True]) + pipe_spike = self._pipeline_mock([2, True]) + redis_client.pipeline.side_effect = [pipe_avg, pipe_spike] + + limiter = self._limiter(redis_client) + decision = limiter.check("scope-a") + + self.assertTrue(decision.allowed) + self.assertEqual(decision.window_name, "spike") + self.assertEqual(decision.count, 2) + self.assertEqual(redis_client.pipeline.call_count, 2) + + def test_check_denies_on_average_window(self): + redis_client = MagicMock() + pipe_avg = self._pipeline_mock([121, True]) + redis_client.pipeline.return_value = pipe_avg + + limiter = self._limiter(redis_client, average_limit=2, average_window_seconds=60) + decision = limiter.check("scope-b") + + self.assertFalse(decision.allowed) + self.assertEqual(decision.window_name, "average") + self.assertEqual(decision.count, 121) + self.assertEqual(decision.limit, 120) + self.assertEqual(decision.window_seconds, 60) + redis_client.pipeline.assert_called_once() + + def test_check_denies_on_spike_after_average_passes(self): + redis_client = MagicMock() + pipe_avg = self._pipeline_mock([1, True]) + pipe_spike = self._pipeline_mock([11, True]) + redis_client.pipeline.side_effect = [pipe_avg, pipe_spike] + + limiter = self._limiter(redis_client, spike_limit=10) + decision = limiter.check("scope-c") + + self.assertFalse(decision.allowed) + self.assertEqual(decision.window_name, "spike") + self.assertEqual(decision.count, 11) + self.assertEqual(decision.limit, 10) + + def test_evaluate_window_uses_time_bucket_in_key(self): + redis_client = MagicMock() + pipeline = self._pipeline_mock([1, True]) + redis_client.pipeline.return_value = pipeline + + limiter = self._limiter(redis_client) + fixed_t = 1_700_000_000 + window_seconds = 60 + expected_bucket = int(fixed_t // window_seconds) + + with patch("rate_limiter.time.time", return_value=fixed_t): + decision = limiter._evaluate_window("s", "average", 120, window_seconds) + + self.assertTrue(decision.allowed) + self.assertEqual(decision.count, 1) + pipeline.incr.assert_called_once() + incr_key = pipeline.incr.call_args[0][0] + self.assertIn(f":s:average:{expected_bucket}", incr_key) + pipeline.expire.assert_called_once_with(incr_key, window_seconds + 1) diff --git a/lambdas/shared/tests/test_common/api_clients/test_pds_details.py b/lambdas/shared/tests/test_common/api_clients/test_pds_details.py index b5f7fb01b4..103e4e90ce 100644 --- a/lambdas/shared/tests/test_common/api_clients/test_pds_details.py +++ b/lambdas/shared/tests/test_common/api_clients/test_pds_details.py @@ -101,3 +101,14 @@ def test_uses_base_url_override_without_authenticator(self): ) self.mock_pds_service_instance.get_patient_details.assert_called_once_with(self.test_patient_id) + + @patch.dict("os.environ", {"PDS_ENV": "ref", "PDS_BASE_URL": " "}, clear=False) + def test_whitespace_only_base_url_uses_authenticator(self): + pds_get_patient_details(self.test_patient_id) + + self.mock_auth_class.assert_called_once() + self.mock_pds_service_class.assert_called_once_with( + self.mock_auth_instance, + "ref", + base_url=None, + ) diff --git a/sonar-project.properties b/sonar-project.properties index 281ab9433e..e5372bde64 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -4,7 +4,7 @@ sonar.organization=nhsdigital sonar.host.url=https://sonarcloud.io sonar.python.version=3.11 sonar.exclusions=**/proxies/**,**/utilities/scripts/**,**/infrastructure/account/**,**/infrastructure/instance/**,**/terraform_aws_backup/**,**/tests/** -sonar.python.coverage.reportPaths=backend-coverage.xml,delta-coverage.xml,ack-lambda-coverage.xml,filenameprocessor-coverage.xml,recordforwarder-coverage.xml,recordprocessor-coverage.xml,mesh_processor-coverage.xml,redis_sync-coverage.xml,mns_subscription-coverage.xml,id_sync-coverage.xml,shared-coverage.xml,batchprocessorfilter-coverage.xml,mns_publisher-coverage.xml +sonar.python.coverage.reportPaths=backend-coverage.xml,delta-coverage.xml,ack-lambda-coverage.xml,filenameprocessor-coverage.xml,recordforwarder-coverage.xml,recordprocessor-coverage.xml,mesh_processor-coverage.xml,redis_sync-coverage.xml,mns_subscription-coverage.xml,id_sync-coverage.xml,shared-coverage.xml,batchprocessorfilter-coverage.xml,mns_publisher-coverage.xml,mock_pds-coverage.xml sonar.cpd.exclusions=**/Dockerfile sonar.issue.ignore.multicriteria=exclude_http_urls,exclude_writable_dirs,exclude_force_dict sonar.issue.ignore.multicriteria.exclude_http_urls.ruleKey=python:S5332 From ec65ea0c925008cb5d74928d7452ef943b0af555 Mon Sep 17 00:00:00 2001 From: Thomas-Boyle Date: Fri, 17 Apr 2026 13:45:07 +0100 Subject: [PATCH 06/18] feat: Add Mock PDS Lambda and ECR repository configuration - Introduced a new Mock PDS Lambda function to simulate PDS responses. - Created an ECR repository for the Mock PDS service with appropriate policies. - Updated deployment workflows to include mock_pds in build flags and image overrides. - Enhanced infrastructure configuration to support the new Mock PDS service, including image URI handling and environment variable setup. --- .github/workflows/deploy-backend.yml | 12 ++- .github/workflows/pr-deploy-and-test.yml | 2 +- .github/workflows/pr-teardown.yml | 3 +- infrastructure/account/mock_pds_ecr_repo.tf | 33 ++++++++ infrastructure/instance/mock_pds.tf | 84 +-------------------- infrastructure/instance/variables.tf | 11 +++ 6 files changed, 59 insertions(+), 86 deletions(-) create mode 100644 infrastructure/account/mock_pds_ecr_repo.tf diff --git a/.github/workflows/deploy-backend.yml b/.github/workflows/deploy-backend.yml index a376e28a1b..180d404bda 100644 --- a/.github/workflows/deploy-backend.yml +++ b/.github/workflows/deploy-backend.yml @@ -9,7 +9,7 @@ on: lambda_build_flags: description: > JSON map of lambda_name -> force-build flag. - e.g. {"recordprocessor":true,"ack-backend":false} + e.g. {"recordprocessor":true,"ack-backend":false,"mock_pds":false} required: false type: string default: "{}" @@ -73,14 +73,14 @@ on: lambda_build_flags: description: > JSON map of lambda_name -> force-build flag. - e.g. {"recordprocessor":true,"ack-backend":false} + e.g. {"recordprocessor":true,"ack-backend":false,"mock_pds":false} required: false type: string default: "{}" lambda_image_overrides: description: > JSON map of lambda_name -> immutable image selector for reuse mode. - e.g. {"recordprocessor":"internal-dev-git-abc123","ack-backend":"123456789012.dkr.ecr.eu-west-2.amazonaws.com/imms-ackbackend-repo@sha256:..."} + e.g. {"recordprocessor":"internal-dev-git-abc123","ack-backend":"123456789012.dkr.ecr.eu-west-2.amazonaws.com/imms-ackbackend-repo@sha256:...","mock_pds":"123456789012.dkr.ecr.eu-west-2.amazonaws.com/imms-mock-pds-repo@sha256:..."} required: false type: string default: "{}" @@ -130,6 +130,12 @@ jobs: dockerfile_path: lambdas/ack_backend/Dockerfile lambda_paths: | lambdas/ack_backend/ + - lambda_name: mock_pds + tf_var_suffix: mock_pds + ecr_repository: imms-mock-pds-repo + dockerfile_path: lambdas/mock_pds/Dockerfile + lambda_paths: | + lambdas/mock_pds/ uses: ./.github/workflows/deploy-lambda-artifact.yml with: lambda_name: ${{ matrix.lambda_name }} diff --git a/.github/workflows/pr-deploy-and-test.yml b/.github/workflows/pr-deploy-and-test.yml index 1f0cfbad62..d117905072 100644 --- a/.github/workflows/pr-deploy-and-test.yml +++ b/.github/workflows/pr-deploy-and-test.yml @@ -22,7 +22,7 @@ jobs: apigee_environment: internal-dev lambda_build_flags: >- ${{ (github.event.action == 'opened' || github.event.action == 'reopened') - && '{"recordprocessor":true,"ack-backend":true}' + && '{"recordprocessor":true,"ack-backend":true,"mock_pds":true}' || '{}' }} diff_base_sha: ${{ github.event.action == 'synchronize' && github.event.before || github.event.pull_request.base.sha }} diff_head_sha: ${{ github.event.pull_request.head.sha }} diff --git a/.github/workflows/pr-teardown.yml b/.github/workflows/pr-teardown.yml index 5d57cb92d2..e946396f57 100644 --- a/.github/workflows/pr-teardown.yml +++ b/.github/workflows/pr-teardown.yml @@ -55,6 +55,7 @@ jobs: } echo "TF_VAR_recordprocessor_image_uri=$(resolve_or_placeholder recordprocessor_image_uri imms-recordprocessor-repo)" >> $GITHUB_ENV echo "TF_VAR_ack_backend_image_uri=$(resolve_or_placeholder ack_backend_image_uri imms-ackbackend-repo)" >> $GITHUB_ENV + echo "TF_VAR_mock_pds_image_uri=$(resolve_or_placeholder mock_pds_image_uri imms-mock-pds-repo)" >> $GITHUB_ENV - name: Install poetry run: pip install poetry==2.1.4 @@ -129,6 +130,6 @@ jobs: --output json } - for repository_name in imms-recordprocessor-repo imms-ackbackend-repo; do + for repository_name in imms-recordprocessor-repo imms-ackbackend-repo imms-mock-pds-repo; do cleanup_repo_by_prefix "${repository_name}" done diff --git a/infrastructure/account/mock_pds_ecr_repo.tf b/infrastructure/account/mock_pds_ecr_repo.tf new file mode 100644 index 0000000000..52c7cf1602 --- /dev/null +++ b/infrastructure/account/mock_pds_ecr_repo.tf @@ -0,0 +1,33 @@ +resource "aws_ecr_repository" "mock_pds_repository" { + image_scanning_configuration { + scan_on_push = true + } + image_tag_mutability = "IMMUTABLE" + name = "imms-mock-pds-repo" +} + +resource "aws_ecr_repository_policy" "mock_pds_repository_lambda_image_retrieval_policy" { + repository = aws_ecr_repository.mock_pds_repository.name + + policy = jsonencode({ + Version = "2012-10-17" + Statement = [ + { + Sid = "LambdaECRImageRetrievalPolicy" + Effect = "Allow" + Principal = { + Service = "lambda.amazonaws.com" + } + Action = [ + "ecr:BatchGetImage", + "ecr:GetDownloadUrlForLayer" + ] + Condition = { + StringLike = { + "aws:sourceArn" = "arn:aws:lambda:${var.aws_region}:${var.imms_account_id}:function:imms-*-mock-pds-lambda" + } + } + } + ] + }) +} diff --git a/infrastructure/instance/mock_pds.tf b/infrastructure/instance/mock_pds.tf index c43083410f..9995b6186b 100644 --- a/infrastructure/instance/mock_pds.tf +++ b/infrastructure/instance/mock_pds.tf @@ -1,84 +1,6 @@ locals { - mock_pds_lambda_dir = abspath("${path.root}/../../lambdas/mock_pds") - mock_pds_lambda_files = fileset(local.mock_pds_lambda_dir, "**") - mock_pds_lambda_dir_sha = sha1(join("", [for f in local.mock_pds_lambda_files : filesha1("${local.mock_pds_lambda_dir}/${f}")])) - mock_pds_lambda_name = "${local.short_prefix}-mock-pds-lambda" - mock_pds_base_url = var.mock_pds_enabled ? "${aws_lambda_function_url.mock_pds_lambda_url[0].function_url}Patient" : "" -} - -resource "aws_ecr_repository" "mock_pds_lambda_repository" { - count = var.mock_pds_enabled ? 1 : 0 - - image_scanning_configuration { - scan_on_push = true - } - - name = "${local.short_prefix}-mock-pds-repo" - force_delete = local.is_temp -} - -module "mock_pds_docker_image" { - count = var.mock_pds_enabled ? 1 : 0 - - source = "terraform-aws-modules/lambda/aws//modules/docker-build" - version = "8.7.0" - docker_file_path = "./mock_pds/Dockerfile" - create_ecr_repo = false - ecr_repo = aws_ecr_repository.mock_pds_lambda_repository[0].name - ecr_repo_lifecycle_policy = jsonencode({ - "rules" : [ - { - "rulePriority" : 1, - "description" : "Keep only the last 2 images", - "selection" : { - "tagStatus" : "any", - "countType" : "imageCountMoreThan", - "countNumber" : 2 - }, - "action" : { - "type" : "expire" - } - } - ] - }) - - platform = "linux/amd64" - use_image_tag = false - source_path = abspath("${path.root}/../../lambdas") - triggers = { - dir_sha = local.mock_pds_lambda_dir_sha - } -} - -resource "aws_ecr_repository_policy" "mock_pds_lambda_ecr_image_retrieval_policy" { - count = var.mock_pds_enabled ? 1 : 0 - - repository = aws_ecr_repository.mock_pds_lambda_repository[0].name - - policy = jsonencode({ - Version = "2012-10-17" - Statement = [ - { - "Sid" : "LambdaECRImageRetrievalPolicy", - "Effect" : "Allow", - "Principal" : { - "Service" : "lambda.amazonaws.com" - }, - "Action" : [ - "ecr:BatchGetImage", - "ecr:DeleteRepositoryPolicy", - "ecr:GetDownloadUrlForLayer", - "ecr:GetRepositoryPolicy", - "ecr:SetRepositoryPolicy" - ], - "Condition" : { - "StringLike" : { - "aws:sourceArn" : "arn:aws:lambda:${var.aws_region}:${var.immunisation_account_id}:function:${local.mock_pds_lambda_name}" - } - } - } - ] - }) + mock_pds_lambda_name = "${local.short_prefix}-mock-pds-lambda" + mock_pds_base_url = var.mock_pds_enabled ? "${aws_lambda_function_url.mock_pds_lambda_url[0].function_url}Patient" : "" } resource "aws_iam_role" "mock_pds_lambda_exec_role" { @@ -174,7 +96,7 @@ resource "aws_lambda_function" "mock_pds_lambda" { function_name = local.mock_pds_lambda_name role = aws_iam_role.mock_pds_lambda_exec_role[0].arn package_type = "Image" - image_uri = module.mock_pds_docker_image[0].image_uri + image_uri = var.mock_pds_image_uri architectures = ["x86_64"] timeout = 30 diff --git a/infrastructure/instance/variables.tf b/infrastructure/instance/variables.tf index e62057c27c..8fe014947c 100644 --- a/infrastructure/instance/variables.tf +++ b/infrastructure/instance/variables.tf @@ -161,6 +161,17 @@ variable "ack_backend_image_uri" { } } +variable "mock_pds_image_uri" { + description = "Immutable URI of the mock PDS Lambda container image in ECR. Required when mock_pds_enabled is true; supplied by CI/CD." + type = string + default = "" + + validation { + condition = !var.mock_pds_enabled || trimspace(var.mock_pds_image_uri) != "" + error_message = "mock_pds_image_uri must be provided when mock_pds_enabled is true." + } +} + locals { prefix = "${var.project_name}-${var.service}-${var.sub_environment}" short_prefix = "${var.project_short_name}-${var.sub_environment}" From e18d51098254b394ed1349a3927e86464d547724 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Fri, 17 Apr 2026 13:57:52 +0100 Subject: [PATCH 07/18] chore: Add module comment for ECR repository in mock PDS configuration --- infrastructure/account/mock_pds_ecr_repo.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructure/account/mock_pds_ecr_repo.tf b/infrastructure/account/mock_pds_ecr_repo.tf index 52c7cf1602..8fafcd82a3 100644 --- a/infrastructure/account/mock_pds_ecr_repo.tf +++ b/infrastructure/account/mock_pds_ecr_repo.tf @@ -5,7 +5,7 @@ resource "aws_ecr_repository" "mock_pds_repository" { image_tag_mutability = "IMMUTABLE" name = "imms-mock-pds-repo" } - +# Module for building and pushing Docker image to ECR resource "aws_ecr_repository_policy" "mock_pds_repository_lambda_image_retrieval_policy" { repository = aws_ecr_repository.mock_pds_repository.name From 611e304a77f991f9f9b92b24148cdc803ef1a522 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Mon, 20 Apr 2026 10:24:30 +0100 Subject: [PATCH 08/18] Add expected_commit_id input to E2E test workflow for improved commit tracking --- .github/workflows/pr-deploy-and-test.yml | 1 + .github/workflows/run-e2e-automation-tests.yml | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-deploy-and-test.yml b/.github/workflows/pr-deploy-and-test.yml index d117905072..3e67697ff4 100644 --- a/.github/workflows/pr-deploy-and-test.yml +++ b/.github/workflows/pr-deploy-and-test.yml @@ -48,6 +48,7 @@ jobs: sub_environment: pr-${{github.event.pull_request.number}} service_under_test: all suite_to_run: ${{ matrix.required_test_suite }} + expected_commit_id: ${{ github.event.pull_request.head.sha }} secrets: APIGEE_PASSWORD: ${{ secrets.APIGEE_PASSWORD }} APIGEE_BASIC_AUTH_TOKEN: ${{ secrets.APIGEE_BASIC_AUTH_TOKEN }} diff --git a/.github/workflows/run-e2e-automation-tests.yml b/.github/workflows/run-e2e-automation-tests.yml index c1d7b2b8fa..e448373f0a 100644 --- a/.github/workflows/run-e2e-automation-tests.yml +++ b/.github/workflows/run-e2e-automation-tests.yml @@ -18,6 +18,10 @@ on: suite_to_run: required: true type: string + expected_commit_id: + required: false + type: string + default: "" secrets: APIGEE_PASSWORD: required: true @@ -72,6 +76,11 @@ on: description: Set to true if you want the MNS validation to be performed as part of the tests. please keep in mind it will increase execution time. default: false type: boolean + expected_commit_id: + description: Optional commit SHA expected from the deployed _status endpoint. + required: false + type: string + default: "" env: APIGEE_AUTH_ENV: ${{ inputs.apigee_environment == 'int' && inputs.apigee_environment || 'internal-dev' }} @@ -81,7 +90,7 @@ env: SERVICE_BASE_PATH: ${{ startsWith(inputs.sub_environment, 'pr-') && format('immunisation-fhir-api/FHIR/R4-{0}', inputs.sub_environment) || 'immunisation-fhir-api/FHIR/R4' }} PROXY_NAME: ${{ startsWith(inputs.sub_environment, 'pr-') && format('immunisation-fhir-api-{0}', inputs.sub_environment) || format('immunisation-fhir-api-{0}', inputs.apigee_environment) }} STATUS_API_KEY: ${{ secrets.STATUS_API_KEY }} - SOURCE_COMMIT_ID: ${{ github.sha }} + SOURCE_COMMIT_ID: ${{ inputs.expected_commit_id || github.sha }} MNS_VALIDATION_REQUIRED: ${{ inputs.mns_validation_required || startsWith(inputs.sub_environment, 'pr-') || inputs.apigee_environment == 'internal-dev' }} jobs: From 87d68b30afe6b15f04684bbd86616e6e4ff983ad Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Mon, 20 Apr 2026 11:00:35 +0100 Subject: [PATCH 09/18] Add expected_commit_id input to E2E test workflow for improved commit tracking --- .github/workflows/pr-deploy-and-test.yml | 3 +++ .github/workflows/run-e2e-automation-tests.yml | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-deploy-and-test.yml b/.github/workflows/pr-deploy-and-test.yml index 3e67697ff4..4633695259 100644 --- a/.github/workflows/pr-deploy-and-test.yml +++ b/.github/workflows/pr-deploy-and-test.yml @@ -39,8 +39,10 @@ jobs: include: - apigee_environment_name: internal-dev required_test_suite: smoke + require_matching_commit_id: true - apigee_environment_name: internal-dev-sandbox required_test_suite: sandbox + require_matching_commit_id: false uses: ./.github/workflows/run-e2e-automation-tests.yml with: apigee_environment: ${{ matrix.apigee_environment_name }} @@ -49,6 +51,7 @@ jobs: service_under_test: all suite_to_run: ${{ matrix.required_test_suite }} expected_commit_id: ${{ github.event.pull_request.head.sha }} + require_matching_commit_id: ${{ matrix.require_matching_commit_id }} secrets: APIGEE_PASSWORD: ${{ secrets.APIGEE_PASSWORD }} APIGEE_BASIC_AUTH_TOKEN: ${{ secrets.APIGEE_BASIC_AUTH_TOKEN }} diff --git a/.github/workflows/run-e2e-automation-tests.yml b/.github/workflows/run-e2e-automation-tests.yml index e448373f0a..d9f85b21eb 100644 --- a/.github/workflows/run-e2e-automation-tests.yml +++ b/.github/workflows/run-e2e-automation-tests.yml @@ -22,6 +22,10 @@ on: required: false type: string default: "" + require_matching_commit_id: + required: false + type: boolean + default: true secrets: APIGEE_PASSWORD: required: true @@ -81,6 +85,11 @@ on: required: false type: string default: "" + require_matching_commit_id: + description: Whether the _status endpoint commitId must match the expected commit SHA. + required: false + type: boolean + default: true env: APIGEE_AUTH_ENV: ${{ inputs.apigee_environment == 'int' && inputs.apigee_environment || 'internal-dev' }} @@ -91,6 +100,7 @@ env: PROXY_NAME: ${{ startsWith(inputs.sub_environment, 'pr-') && format('immunisation-fhir-api-{0}', inputs.sub_environment) || format('immunisation-fhir-api-{0}', inputs.apigee_environment) }} STATUS_API_KEY: ${{ secrets.STATUS_API_KEY }} SOURCE_COMMIT_ID: ${{ inputs.expected_commit_id || github.sha }} + REQUIRE_MATCHING_COMMIT_ID: ${{ inputs.require_matching_commit_id }} MNS_VALIDATION_REQUIRED: ${{ inputs.mns_validation_required || startsWith(inputs.sub_environment, 'pr-') || inputs.apigee_environment == 'internal-dev' }} jobs: @@ -121,11 +131,15 @@ jobs: if [[ "${response_code}" -eq 200 ]] && [[ "${response_body}" == "OK" ]] && [[ "${status}" == "pass" ]]; then echo "Status test successful" + if [[ "${REQUIRE_MATCHING_COMMIT_ID}" != "true" ]]; then + echo "Skipping commit hash validation for ${APIGEE_ENVIRONMENT}" + break + fi if [[ "${commitId}" == "${SOURCE_COMMIT_ID}" ]]; then echo "Commit hash test successful" break else - echo "Waiting for ${endpoint} to return the correct commit hash..." + echo "Waiting for ${endpoint} to return the correct commit hash... expected ${SOURCE_COMMIT_ID}, got ${commitId}" fi else echo "Waiting for ${endpoint} to return a 200 response with 'OK' body..." From d4a04fa0a3030e0f5ad6d7b563ce482bc8acc7f7 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Mon, 20 Apr 2026 14:55:57 +0100 Subject: [PATCH 10/18] Remove mock_pds entry from lambda_image_overrides example in deploy-backend.yml --- .github/workflows/deploy-backend.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy-backend.yml b/.github/workflows/deploy-backend.yml index 180d404bda..e22c15ecf0 100644 --- a/.github/workflows/deploy-backend.yml +++ b/.github/workflows/deploy-backend.yml @@ -80,7 +80,7 @@ on: lambda_image_overrides: description: > JSON map of lambda_name -> immutable image selector for reuse mode. - e.g. {"recordprocessor":"internal-dev-git-abc123","ack-backend":"123456789012.dkr.ecr.eu-west-2.amazonaws.com/imms-ackbackend-repo@sha256:...","mock_pds":"123456789012.dkr.ecr.eu-west-2.amazonaws.com/imms-mock-pds-repo@sha256:..."} + e.g. {"recordprocessor":"internal-dev-git-abc123","ack-backend":"123456789012.dkr.ecr.eu-west-2.amazonaws.com/imms-ackbackend-repo@sha256:..."} required: false type: string default: "{}" From 813a0e06be328b26cd3d40d58d3cff0e70f37151 Mon Sep 17 00:00:00 2001 From: Thomas-Boyle Date: Mon, 20 Apr 2026 15:07:24 +0100 Subject: [PATCH 11/18] Add mock_pds_enabled variable to dev environment configuration --- infrastructure/instance/environments/dev/pr/variables.tfvars | 1 + 1 file changed, 1 insertion(+) diff --git a/infrastructure/instance/environments/dev/pr/variables.tfvars b/infrastructure/instance/environments/dev/pr/variables.tfvars index 130fea83cc..0fc03251ce 100644 --- a/infrastructure/instance/environments/dev/pr/variables.tfvars +++ b/infrastructure/instance/environments/dev/pr/variables.tfvars @@ -6,3 +6,4 @@ mns_environment = "dev" error_alarm_notifications_enabled = false create_mesh_processor = false has_sub_environment_scope = true +mock_pds_enabled = true From d5d0318e128a58ca6ee8f68f2ddc55ae22af0201 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Tue, 21 Apr 2026 09:26:56 +0100 Subject: [PATCH 12/18] Set ref mns_environment to int --- infrastructure/instance/environments/dev/ref/variables.tfvars | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructure/instance/environments/dev/ref/variables.tfvars b/infrastructure/instance/environments/dev/ref/variables.tfvars index 16ab05b6e4..6a60ac334a 100644 --- a/infrastructure/instance/environments/dev/ref/variables.tfvars +++ b/infrastructure/instance/environments/dev/ref/variables.tfvars @@ -2,7 +2,7 @@ environment = "dev" immunisation_account_id = "345594581768" dspp_core_account_id = "603871901111" pds_environment = "ref" -mns_environment = "dev" +mns_environment = "int" mock_pds_enabled = true mock_pds_average_rate_limit = 125 mock_pds_spike_rate_limit = 450 From b74a32c1601a8ed3f5ea3710e49b707209a6e930 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Wed, 22 Apr 2026 08:43:59 +0100 Subject: [PATCH 13/18] Update expected_commit_id to use github.sha in E2E tests --- .github/workflows/pr-deploy-and-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-deploy-and-test.yml b/.github/workflows/pr-deploy-and-test.yml index 4633695259..907fe2deeb 100644 --- a/.github/workflows/pr-deploy-and-test.yml +++ b/.github/workflows/pr-deploy-and-test.yml @@ -50,7 +50,7 @@ jobs: sub_environment: pr-${{github.event.pull_request.number}} service_under_test: all suite_to_run: ${{ matrix.required_test_suite }} - expected_commit_id: ${{ github.event.pull_request.head.sha }} + expected_commit_id: ${{ github.sha }} require_matching_commit_id: ${{ matrix.require_matching_commit_id }} secrets: APIGEE_PASSWORD: ${{ secrets.APIGEE_PASSWORD }} From bdf2fe478de925a0b0de5b47b678587177e298ba Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Wed, 22 Apr 2026 09:34:41 +0100 Subject: [PATCH 14/18] Disable mock PDS in development environment --- infrastructure/instance/environments/dev/pr/variables.tfvars | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructure/instance/environments/dev/pr/variables.tfvars b/infrastructure/instance/environments/dev/pr/variables.tfvars index 0fc03251ce..d7a8bf9b18 100644 --- a/infrastructure/instance/environments/dev/pr/variables.tfvars +++ b/infrastructure/instance/environments/dev/pr/variables.tfvars @@ -6,4 +6,4 @@ mns_environment = "dev" error_alarm_notifications_enabled = false create_mesh_processor = false has_sub_environment_scope = true -mock_pds_enabled = true +mock_pds_enabled = false From aae62833ee468fb85ed4c71a2ae62a5288ef754f Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Wed, 29 Apr 2026 10:47:24 +0100 Subject: [PATCH 15/18] Refactor PDS service caching logic and update coverage dependency --- .github/workflows/quality-checks.yml | 2 +- lambdas/mock_pds/pyproject.toml | 2 +- .../src/common/api_clients/get_pds_details.py | 12 +++-- .../api_clients/test_pds_details.py | 45 +++++++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 4edb916a54..305b553676 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -188,7 +188,7 @@ jobs: working-directory: lambdas/mock_pds id: mock_pds env: - PYTHONPATH: ${{ env.LAMBDA_PATH }}/mock_pds/src:${{ env.LAMBDA_PATH }}/mock_pds/tests + PYTHONPATH: ${{ env.LAMBDA_PATH }}/mock_pds/src continue-on-error: true run: | poetry install diff --git a/lambdas/mock_pds/pyproject.toml b/lambdas/mock_pds/pyproject.toml index 99cca3dd38..9ca813e030 100644 --- a/lambdas/mock_pds/pyproject.toml +++ b/lambdas/mock_pds/pyproject.toml @@ -9,7 +9,7 @@ packages = [{include = "src"}] [tool.poetry.dependencies] python = "~3.11" redis = "^6.1.0" -coverage = "^7.13.2" +coverage = "^7.13.5" [build-system] requires = ["poetry-core >= 1.5.0"] diff --git a/lambdas/shared/src/common/api_clients/get_pds_details.py b/lambdas/shared/src/common/api_clients/get_pds_details.py index 3c032e14d9..437d6d6875 100644 --- a/lambdas/shared/src/common/api_clients/get_pds_details.py +++ b/lambdas/shared/src/common/api_clients/get_pds_details.py @@ -10,19 +10,23 @@ from common.clients import get_secrets_manager_client, logger _pds_service: PdsService | None = None +_pds_service_config: tuple[str, str | None] | None = None def get_pds_service() -> PdsService: - global _pds_service - if _pds_service is None: - environment = os.getenv("PDS_ENV", "int") - base_url = os.getenv("PDS_BASE_URL", "").strip() or None + global _pds_service, _pds_service_config + environment = os.getenv("PDS_ENV", "int") + base_url = os.getenv("PDS_BASE_URL", "").strip() or None + config = (environment, base_url) + + if _pds_service is None or _pds_service_config != config: authenticator = ( None if base_url else AppRestrictedAuth(secret_manager_client=get_secrets_manager_client(), environment=environment) ) _pds_service = PdsService(authenticator, environment, base_url=base_url) + _pds_service_config = config return _pds_service diff --git a/lambdas/shared/tests/test_common/api_clients/test_pds_details.py b/lambdas/shared/tests/test_common/api_clients/test_pds_details.py index 103e4e90ce..8021a1e952 100644 --- a/lambdas/shared/tests/test_common/api_clients/test_pds_details.py +++ b/lambdas/shared/tests/test_common/api_clients/test_pds_details.py @@ -9,6 +9,7 @@ class TestGetPdsPatientDetails(unittest.TestCase): def setUp(self): self.test_patient_id = "9912003888" get_pds_service.__globals__["_pds_service"] = None + get_pds_service.__globals__["_pds_service_config"] = None self.logger_patcher = patch("common.api_clients.get_pds_details.logger") self.mock_logger = self.logger_patcher.start() @@ -25,6 +26,7 @@ def setUp(self): def tearDown(self): get_pds_service.__globals__["_pds_service"] = None + get_pds_service.__globals__["_pds_service_config"] = None patch.stopall() def test_pds_get_patient_details_success(self): @@ -112,3 +114,46 @@ def test_whitespace_only_base_url_uses_authenticator(self): "ref", base_url=None, ) + + @patch.dict("os.environ", {"PDS_ENV": "ref", "PDS_BASE_URL": "https://mock-pds-v1.example/Patient"}, clear=False) + def test_rebuilds_cached_service_when_base_url_changes(self): + pds_get_patient_details(self.test_patient_id) + + self.mock_pds_service_class.reset_mock() + self.mock_pds_service_instance.get_patient_details.reset_mock() + new_instance = MagicMock() + self.mock_pds_service_class.return_value = new_instance + + with patch.dict("os.environ", {"PDS_BASE_URL": "https://mock-pds-v2.example/Patient"}, clear=False): + pds_get_patient_details("1234567890") + + self.mock_auth_class.assert_not_called() + self.mock_pds_service_class.assert_called_once_with( + None, + "ref", + base_url="https://mock-pds-v2.example/Patient", + ) + new_instance.get_patient_details.assert_called_once_with("1234567890") + + @patch.dict("os.environ", {"PDS_ENV": "int", "PDS_BASE_URL": ""}, clear=False) + def test_rebuilds_cached_service_when_environment_changes(self): + pds_get_patient_details(self.test_patient_id) + + self.mock_pds_service_class.reset_mock() + self.mock_auth_class.reset_mock() + self.mock_pds_service_instance.get_patient_details.reset_mock() + new_auth = MagicMock() + new_service = MagicMock() + self.mock_auth_class.return_value = new_auth + self.mock_pds_service_class.return_value = new_service + + with patch.dict("os.environ", {"PDS_ENV": "ref", "PDS_BASE_URL": ""}, clear=False): + pds_get_patient_details("1234567890") + + self.mock_auth_class.assert_called_once_with(secret_manager_client=unittest.mock.ANY, environment="ref") + self.mock_pds_service_class.assert_called_once_with( + new_auth, + "ref", + base_url=None, + ) + new_service.get_patient_details.assert_called_once_with("1234567890") From 0dff83c0d2f370567a46a3254bc94620794fab08 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Wed, 29 Apr 2026 11:39:14 +0100 Subject: [PATCH 16/18] Update Dockerfile to set environment variables for Poetry installation --- .github/workflows/quality-checks.yml | 1 + lambdas/mock_pds/Dockerfile | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index e962715e7e..7ec2b8036f 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -9,6 +9,7 @@ on: env: SHARED_PATH: ${{ github.workspace }}/lambdas/shared LAMBDA_PATH: ${{ github.workspace }}/lambdas + POETRY_INSTALLER_ONLY_BINARY: ":all:" jobs: lint-specification: diff --git a/lambdas/mock_pds/Dockerfile b/lambdas/mock_pds/Dockerfile index 6e7a092428..bfee75d660 100644 --- a/lambdas/mock_pds/Dockerfile +++ b/lambdas/mock_pds/Dockerfile @@ -1,9 +1,12 @@ FROM public.ecr.aws/lambda/python:3.11 AS base +ENV PIP_ONLY_BINARY=:all: \ + POETRY_INSTALLER_ONLY_BINARY=:all: + RUN mkdir -p /home/appuser && \ echo 'appuser:x:1001:1001::/home/appuser:/sbin/nologin' >> /etc/passwd && \ echo 'appuser:x:1001:' >> /etc/group && \ - chown -R 1001:1001 /home/appuser && pip install "poetry~=2.1.4" + chown -R 1001:1001 /home/appuser && pip install --only-binary :all: "poetry==2.1.4" COPY ./mock_pds/poetry.lock ./mock_pds/pyproject.toml ./ From 708197e9807deba64d429fbebcf0804a3efe365a Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Wed, 29 Apr 2026 11:50:00 +0100 Subject: [PATCH 17/18] Refactor Dockerfile to install Poetry using hashed requirements for security --- lambdas/mock_pds/Dockerfile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lambdas/mock_pds/Dockerfile b/lambdas/mock_pds/Dockerfile index bfee75d660..8d57f83c9c 100644 --- a/lambdas/mock_pds/Dockerfile +++ b/lambdas/mock_pds/Dockerfile @@ -6,7 +6,10 @@ ENV PIP_ONLY_BINARY=:all: \ RUN mkdir -p /home/appuser && \ echo 'appuser:x:1001:1001::/home/appuser:/sbin/nologin' >> /etc/passwd && \ echo 'appuser:x:1001:' >> /etc/group && \ - chown -R 1001:1001 /home/appuser && pip install --only-binary :all: "poetry==2.1.4" + chown -R 1001:1001 /home/appuser && \ + printf 'poetry==2.1.4 --hash=sha256:0019b64d33fed9184a332f7fad60ca47aace4d6a0e9c635cdea21b76e96f32ce\n' > /tmp/poetry-requirements.txt && \ + pip install --only-binary :all: --require-hashes -r /tmp/poetry-requirements.txt && \ + rm -f /tmp/poetry-requirements.txt COPY ./mock_pds/poetry.lock ./mock_pds/pyproject.toml ./ From 3aa57e0913faddf4883da9a08e4207922b206aa3 Mon Sep 17 00:00:00 2001 From: amarauzoma Date: Wed, 29 Apr 2026 12:06:16 +0100 Subject: [PATCH 18/18] Refactor Dockerfile to improve readability of Poetry installation command --- lambdas/mock_pds/Dockerfile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lambdas/mock_pds/Dockerfile b/lambdas/mock_pds/Dockerfile index 8d57f83c9c..a0a8ac9602 100644 --- a/lambdas/mock_pds/Dockerfile +++ b/lambdas/mock_pds/Dockerfile @@ -7,7 +7,10 @@ RUN mkdir -p /home/appuser && \ echo 'appuser:x:1001:1001::/home/appuser:/sbin/nologin' >> /etc/passwd && \ echo 'appuser:x:1001:' >> /etc/group && \ chown -R 1001:1001 /home/appuser && \ - printf 'poetry==2.1.4 --hash=sha256:0019b64d33fed9184a332f7fad60ca47aace4d6a0e9c635cdea21b76e96f32ce\n' > /tmp/poetry-requirements.txt && \ + printf '%s %s\n' \ + 'poetry==2.1.4' \ + '--hash=sha256:0019b64d33fed9184a332f7fad60ca47aace4d6a0e9c635cdea21b76e96f32ce' \ + > /tmp/poetry-requirements.txt && \ pip install --only-binary :all: --require-hashes -r /tmp/poetry-requirements.txt && \ rm -f /tmp/poetry-requirements.txt