From 77c3202cb0ca43c88483acd6790f8ea3f132def9 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Wed, 22 Oct 2025 16:33:33 +0100 Subject: [PATCH 01/11] CCM-12649 Get status healthcheck endpoint --- .../terraform/components/api/README.md | 1 + .../iam_role_api_gateway_execution_role.tf | 3 +- .../terraform/components/api/locals.tf | 1 + .../api/module_lambda_get_status.tf | 78 +++++++++++++++++ .../components/api/resources/spec.tmpl.json | 33 ++++++++ .../datastore/src/__test__/heathcheck.test.ts | 38 +++++++++ internal/datastore/src/healthcheck.ts | 13 +++ internal/datastore/src/index.ts | 1 + .../src/config/__tests__/deps.test.ts | 2 +- lambdas/api-handler/src/config/deps.ts | 17 +++- .../__tests__/get-letter-data.test.ts | 4 +- .../src/handlers/__tests__/get-letter.test.ts | 2 +- .../handlers/__tests__/get-letters.test.ts | 2 +- .../src/handlers/__tests__/get_status.test.ts | 83 +++++++++++++++++++ .../handlers/__tests__/patch-letter.test.ts | 2 +- .../api-handler/src/handlers/get-status.ts | 42 ++++++++++ lambdas/api-handler/src/index.ts | 2 + .../__tests__/letter-operations.test.ts | 2 +- 18 files changed, 316 insertions(+), 10 deletions(-) create mode 100644 infrastructure/terraform/components/api/module_lambda_get_status.tf create mode 100644 internal/datastore/src/__test__/heathcheck.test.ts create mode 100644 internal/datastore/src/healthcheck.ts create mode 100644 lambdas/api-handler/src/handlers/__tests__/get_status.test.ts create mode 100644 lambdas/api-handler/src/handlers/get-status.ts diff --git a/infrastructure/terraform/components/api/README.md b/infrastructure/terraform/components/api/README.md index 1b60ce0ac..bde845f78 100644 --- a/infrastructure/terraform/components/api/README.md +++ b/infrastructure/terraform/components/api/README.md @@ -37,6 +37,7 @@ No requirements. | [get\_letter](#module\_get\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.24/terraform-lambda.zip | n/a | | [get\_letter\_data](#module\_get\_letter\_data) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.24/terraform-lambda.zip | n/a | | [get\_letters](#module\_get\_letters) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.24/terraform-lambda.zip | n/a | +| [get\_status](#module\_get\_status) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.24/terraform-lambda.zip | n/a | | [kms](#module\_kms) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-kms.zip | n/a | | [logging\_bucket](#module\_logging\_bucket) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.20/terraform-s3bucket.zip | n/a | | [patch\_letter](#module\_patch\_letter) | https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.24/terraform-lambda.zip | n/a | diff --git a/infrastructure/terraform/components/api/iam_role_api_gateway_execution_role.tf b/infrastructure/terraform/components/api/iam_role_api_gateway_execution_role.tf index c4cd1b169..11c961788 100644 --- a/infrastructure/terraform/components/api/iam_role_api_gateway_execution_role.tf +++ b/infrastructure/terraform/components/api/iam_role_api_gateway_execution_role.tf @@ -52,7 +52,8 @@ data "aws_iam_policy_document" "api_gateway_execution_policy" { module.get_letter.function_arn, module.get_letters.function_arn, module.patch_letter.function_arn, - module.get_letter_data.function_arn + module.get_letter_data.function_arn, + module.get_status.function_arn ] } } diff --git a/infrastructure/terraform/components/api/locals.tf b/infrastructure/terraform/components/api/locals.tf index 5a2b4aa56..641b40dec 100644 --- a/infrastructure/terraform/components/api/locals.tf +++ b/infrastructure/terraform/components/api/locals.tf @@ -12,6 +12,7 @@ locals { GET_LETTERS_LAMBDA_ARN = module.get_letters.function_arn GET_LETTER_DATA_LAMBDA_ARN = module.get_letter_data.function_arn PATCH_LETTER_LAMBDA_ARN = module.patch_letter.function_arn + GET_STATUS_LAMBDA_ARN = module.get_status.function_arn }) destination_arn = "arn:aws:logs:${var.region}:${var.shared_infra_account_id}:destination:nhs-main-obs-firehose-logs" diff --git a/infrastructure/terraform/components/api/module_lambda_get_status.tf b/infrastructure/terraform/components/api/module_lambda_get_status.tf new file mode 100644 index 000000000..be93702c9 --- /dev/null +++ b/infrastructure/terraform/components/api/module_lambda_get_status.tf @@ -0,0 +1,78 @@ +module "get_status" { + source = "https://github.com/NHSDigital/nhs-notify-shared-modules/releases/download/v2.0.24/terraform-lambda.zip" + + function_name = "get_status" + description = "Healthcheck for service" + + aws_account_id = var.aws_account_id + component = var.component + environment = var.environment + project = var.project + region = var.region + group = var.group + + log_retention_in_days = var.log_retention_in_days + kms_key_arn = module.kms.key_arn + + iam_policy_document = { + body = data.aws_iam_policy_document.get_status_lambda.json + } + + function_s3_bucket = local.acct.s3_buckets["lambda_function_artefacts"]["id"] + function_code_base_path = local.aws_lambda_functions_dir_path + function_code_dir = "api-handler/dist" + function_include_common = true + handler_function_name = "getStatus" + runtime = "nodejs22.x" + memory = 128 + timeout = 5 + log_level = var.log_level + + force_lambda_code_deploy = var.force_lambda_code_deploy + enable_lambda_insights = false + + send_to_firehose = true + log_destination_arn = local.destination_arn + log_subscription_role_arn = local.acct.log_subscription_role_arn + + lambda_env_vars = merge(local.common_lambda_env_vars, { + MAX_LIMIT = var.max_get_limit + }) +} + +data "aws_iam_policy_document" "get_status_lambda" { + statement { + sid = "KMSPermissions" + effect = "Allow" + + actions = [ + "kms:Decrypt", + "kms:GenerateDataKey", + ] + + resources = [ + module.kms.key_arn, ## Requires shared kms module + ] + } + + statement { + sid = "AllowDynamoDBAccess" + effect = "Allow" + + actions = [ + "dynamodb:DescribeTable" + ] + + resources = [ + aws_dynamodb_table.letters.arn, + "${aws_dynamodb_table.letters.arn}/index/supplierStatus-index" + ] + } + + + statement { + sid = "S3ListAllMyBuckets" + actions = ["s3:ListAllMyBuckets"] + resources = ["arn:aws:s3:::*"] + } +} diff --git a/infrastructure/terraform/components/api/resources/spec.tmpl.json b/infrastructure/terraform/components/api/resources/spec.tmpl.json index 50896857a..f9b6141fc 100644 --- a/infrastructure/terraform/components/api/resources/spec.tmpl.json +++ b/infrastructure/terraform/components/api/resources/spec.tmpl.json @@ -23,6 +23,39 @@ }, "openapi": "3.0.1", "paths": { + "/_status": { + "get": { + "operationId": "getStatusId", + "responses": { + "200": { + "description": "Empty body" + }, + "500": { + "description": "Server error" + } + }, + "security": [ + { + "LambdaAuthorizer": [] + } + ], + "summary": "Healthcheck endpoint", + "x-amazon-apigateway-integration": { + "contentHandling": "CONVERT_TO_TEXT", + "credentials": "${APIG_EXECUTION_ROLE_ARN}", + "httpMethod": "POST", + "passthroughBehavior": "WHEN_NO_TEMPLATES", + "responses": { + ".*": { + "statusCode": "200" + } + }, + "timeoutInMillis": 29000, + "type": "AWS_PROXY", + "uri": "arn:aws:apigateway:${AWS_REGION}:lambda:path/2015-03-31/functions/${GET_STATUS_LAMBDA_ARN}/invocations" + } + } + }, "/letters": { "get": { "description": "Returns 200 OK with paginated letter ids.", diff --git a/internal/datastore/src/__test__/heathcheck.test.ts b/internal/datastore/src/__test__/heathcheck.test.ts new file mode 100644 index 000000000..ade1b1b18 --- /dev/null +++ b/internal/datastore/src/__test__/heathcheck.test.ts @@ -0,0 +1,38 @@ +import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; +import { DBHealthcheck } from "../healthcheck"; +import { createTables, DBContext, deleteTables, setupDynamoDBContainer } from "./db"; + +// Database tests can take longer, especially with setup and teardown +jest.setTimeout(30000); + +describe('DBHealthcheck', () => { + + let db: DBContext; + + beforeAll(async () => { + db = await setupDynamoDBContainer(); + }); + + beforeEach(async () => { + await createTables(db); + }); + + afterEach(async () => { + await deleteTables(db); + }); + + it('passes when the database is available', async () => { + const dbHealthCheck = new DBHealthcheck(db.docClient, db.config); + await dbHealthCheck.check(); + }); + + it('fails when the database is unavailable', async () => { + const realFunction = db.docClient.send; + db.docClient.send = jest.fn().mockImplementation(() => { throw new Error('Failed to send')}); + + const dbHealthCheck = new DBHealthcheck(db.docClient, db.config); + await expect(dbHealthCheck.check()).rejects.toThrow(); + + db.docClient.send = realFunction; + }); +}); diff --git a/internal/datastore/src/healthcheck.ts b/internal/datastore/src/healthcheck.ts new file mode 100644 index 000000000..39c91c619 --- /dev/null +++ b/internal/datastore/src/healthcheck.ts @@ -0,0 +1,13 @@ +import { DescribeTableCommand } from "@aws-sdk/client-dynamodb"; +import { DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; +import { LetterRepositoryConfig } from "./letter-repository"; + +export class DBHealthcheck { + constructor(readonly ddbClient: DynamoDBDocumentClient, + readonly config: LetterRepositoryConfig) {} + + async check(): Promise { + await this.ddbClient.send(new DescribeTableCommand({ + TableName: this.config.lettersTableName})); + } +} diff --git a/internal/datastore/src/index.ts b/internal/datastore/src/index.ts index 3c0ebfaf5..bad5ad2ed 100644 --- a/internal/datastore/src/index.ts +++ b/internal/datastore/src/index.ts @@ -1,3 +1,4 @@ export * from './types'; export * from './letter-repository'; +export * from './healthcheck'; export * from './types'; diff --git a/lambdas/api-handler/src/config/__tests__/deps.test.ts b/lambdas/api-handler/src/config/__tests__/deps.test.ts index 2028db885..7bec2677a 100644 --- a/lambdas/api-handler/src/config/__tests__/deps.test.ts +++ b/lambdas/api-handler/src/config/__tests__/deps.test.ts @@ -1,4 +1,3 @@ - import type { Deps } from '../deps'; describe('createDependenciesContainer', () => { @@ -24,6 +23,7 @@ describe('createDependenciesContainer', () => { // Repo client jest.mock('../../../../../internal/datastore', () => ({ LetterRepository: jest.fn(), + DBHealthcheck: jest.fn() })); // Env diff --git a/lambdas/api-handler/src/config/deps.ts b/lambdas/api-handler/src/config/deps.ts index 6f9c3e84a..26028814f 100644 --- a/lambdas/api-handler/src/config/deps.ts +++ b/lambdas/api-handler/src/config/deps.ts @@ -2,13 +2,14 @@ import { S3Client } from "@aws-sdk/client-s3"; import { DynamoDBClient } from '@aws-sdk/client-dynamodb'; import { DynamoDBDocumentClient } from '@aws-sdk/lib-dynamodb'; import pino from 'pino'; -import { LetterRepository } from '../../../../internal/datastore'; +import { LetterRepository, DBHealthcheck } from '../../../../internal/datastore'; import { envVars, EnvVars } from "../config/env"; export type Deps = { s3Client: S3Client; letterRepo: LetterRepository; - logger: pino.Logger, + dbHealthcheck: DBHealthcheck; + logger: pino.Logger; env: EnvVars }; @@ -23,6 +24,17 @@ function createLetterRepository(log: pino.Logger, envVars: EnvVars): LetterRepos return new LetterRepository(docClient, log, config); } +function createDBHealthcheck(envVars: EnvVars): DBHealthcheck { + const ddbClient = new DynamoDBClient({}); + const docClient = DynamoDBDocumentClient.from(ddbClient); + const config = { + lettersTableName: envVars.LETTERS_TABLE_NAME, + ttlHours: envVars.LETTER_TTL_HOURS + }; + + return new DBHealthcheck(docClient, config); +} + export function createDependenciesContainer(): Deps { const log = pino(); @@ -30,6 +42,7 @@ export function createDependenciesContainer(): Deps { return { s3Client: new S3Client(), letterRepo: createLetterRepository(log, envVars), + dbHealthcheck: createDBHealthcheck(envVars), logger: log, env: envVars }; diff --git a/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts b/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts index 5b6509a8b..9111bbb6e 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts @@ -18,7 +18,7 @@ import { makeApiGwEvent } from './utils/test-utils'; import { ValidationError } from '../../errors'; import * as errors from '../../contracts/errors'; import { createGetLetterDataHandler } from '../get-letter-data'; -import { S3Client } from '@aws-sdk/client-s3'; +import { Destination, S3Client } from '@aws-sdk/client-s3'; import pino from 'pino'; import { LetterRepository } from '../../../../../internal/datastore/src'; import { EnvVars } from '../../config/env'; @@ -37,7 +37,7 @@ describe('API Lambda handler', () => { LETTER_TTL_HOURS: 12960, DOWNLOAD_URL_TTL_SECONDS: 60 } as unknown as EnvVars - } + } as Deps; beforeEach(() => { jest.clearAllMocks(); diff --git a/lambdas/api-handler/src/handlers/__tests__/get-letter.test.ts b/lambdas/api-handler/src/handlers/__tests__/get-letter.test.ts index 99b01b743..a21325a50 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get-letter.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get-letter.test.ts @@ -28,7 +28,7 @@ describe('API Lambda handler', () => { DOWNLOAD_URL_TTL_SECONDS: 60, MAX_LIMIT: 2500 } as unknown as EnvVars - }; + } as Deps; beforeEach(() => { jest.clearAllMocks(); diff --git a/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts b/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts index 73b528969..973fbcf57 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get-letters.test.ts @@ -38,7 +38,7 @@ describe('API Lambda handler', () => { DOWNLOAD_URL_TTL_SECONDS: 60, MAX_LIMIT: 2500 } as unknown as EnvVars - } + } as Deps; beforeEach(() => { jest.clearAllMocks(); diff --git a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts new file mode 100644 index 000000000..3340d9694 --- /dev/null +++ b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts @@ -0,0 +1,83 @@ +import { S3Client } from "@aws-sdk/client-s3"; +import { DBHealthcheck } from "../../../../../internal/datastore/src"; +import pino from "pino"; +import { Deps } from "../../config/deps"; +import { makeApiGwEvent } from "./utils/test-utils"; +import { mockDeep } from "jest-mock-extended"; +import { Context } from "aws-lambda"; +import { createGetStatusHandler } from "../get-status"; + +describe('API Lambda handler', () => { + it('passes if S3 and DynamoDB are available', async() => { + + const event = makeApiGwEvent({path: '/_status', + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId', 'x-request-id': 'requestId'} + }); + + const getLetterDataHandler = createGetStatusHandler(getMockedDeps()); + const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); + + expect(result).toEqual({ + statusCode: 200, + body: JSON.stringify({}, null, 2), + }); + }); + + it('fails if S3 is unavailable', async() => { + const mockedDeps = getMockedDeps(); + mockedDeps.s3Client.send = jest.fn().mockRejectedValue(new Error('unexpected error')); + + const event = makeApiGwEvent({path: '/_status', + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId', 'x-request-id': 'requestId'} + }); + + const getLetterDataHandler = createGetStatusHandler(mockedDeps); + const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); + + expect(result).toEqual(expect.objectContaining({ + statusCode: 500 + })); + }); + + + it('fails if DynamoDB is unavailable', async() => { + const mockedDeps = getMockedDeps(); + mockedDeps.dbHealthcheck.check = jest.fn().mockRejectedValue(new Error('unexpected error')); + + const event = makeApiGwEvent({path: '/_status', + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId', 'x-request-id': 'requestId'} + }); + + const getLetterDataHandler = createGetStatusHandler(mockedDeps); + const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); + + expect(result).toEqual(expect.objectContaining({ + statusCode: 500 + })); + }); + + it('fails if request ID is absent', async() => { + const event = makeApiGwEvent({path: '/_status', + headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + }); + + const getLetterDataHandler = createGetStatusHandler(getMockedDeps()); + const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); + + expect(result).toEqual(expect.objectContaining({ + statusCode: 500 + })); + }); + + function getMockedDeps(): jest.Mocked { + return { + s3Client: { send: jest.fn()} as unknown as S3Client, + dbHealthcheck: {check: jest.fn()} as unknown as DBHealthcheck, + logger: { info: jest.fn(), error: jest.fn() } as unknown as pino.Logger, + env: { + SUPPLIER_ID_HEADER: 'nhsd-supplier-id', + APIM_CORRELATION_HEADER: 'nhsd-correlation-id' + } + } as Deps; + } +}); diff --git a/lambdas/api-handler/src/handlers/__tests__/patch-letter.test.ts b/lambdas/api-handler/src/handlers/__tests__/patch-letter.test.ts index 8c96ce79d..b134d95b3 100644 --- a/lambdas/api-handler/src/handlers/__tests__/patch-letter.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/patch-letter.test.ts @@ -57,7 +57,7 @@ describe('patchLetter API Handler', () => { LETTER_TTL_HOURS: 12960, DOWNLOAD_URL_TTL_SECONDS: 60 } as unknown as EnvVars - } + } as Deps; it('returns 200 OK with updated resource', async () => { const event = makeApiGwEvent({ diff --git a/lambdas/api-handler/src/handlers/get-status.ts b/lambdas/api-handler/src/handlers/get-status.ts new file mode 100644 index 000000000..d13d6f615 --- /dev/null +++ b/lambdas/api-handler/src/handlers/get-status.ts @@ -0,0 +1,42 @@ +import { APIGatewayProxyHandler } from "aws-lambda"; +import { Deps } from "../config/deps"; +import { ListBucketsCommand, S3Client } from "@aws-sdk/client-s3"; +import { validateCommonHeaders } from "../utils/validation"; +import { mapErrorToResponse } from "../mappers/error-mapper"; + +export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { + + return async(event) => { + + const commonHeadersResult = validateCommonHeaders(event.headers, deps); + + if (!commonHeadersResult.ok) { + return mapErrorToResponse(commonHeadersResult.error, commonHeadersResult.correlationId, deps.logger); + } + + try { + await deps.dbHealthcheck.check(); + await s3HealthCheck(deps.s3Client); + + deps.logger.info({ + description: 'Healthcheck passed', + supplierId: commonHeadersResult.value.supplierId + }); + + return { + statusCode: 200, + body: JSON.stringify({}, null, 2) + }; + } catch (error) { + return mapErrorToResponse(error, commonHeadersResult.value.correlationId, deps.logger); + } + } +} + + +async function s3HealthCheck(s3Client: S3Client) { + const command: ListBucketsCommand = new ListBucketsCommand({ + MaxBuckets: 1 + }); + await s3Client.send(command); +} diff --git a/lambdas/api-handler/src/index.ts b/lambdas/api-handler/src/index.ts index bca91bc92..c5f382114 100644 --- a/lambdas/api-handler/src/index.ts +++ b/lambdas/api-handler/src/index.ts @@ -3,6 +3,7 @@ import { createGetLetterHandler } from "./handlers/get-letter"; import { createGetLetterDataHandler } from "./handlers/get-letter-data"; import { createGetLettersHandler } from "./handlers/get-letters"; import { createPatchLetterHandler } from "./handlers/patch-letter"; +import { createGetStatusHandler } from "./handlers/get-status"; const container = createDependenciesContainer(); @@ -10,3 +11,4 @@ export const getLetter = createGetLetterHandler(container); export const getLetterData = createGetLetterDataHandler(container); export const getLetters = createGetLettersHandler(container); export const patchLetter = createPatchLetterHandler(container); +export const getStatus = createGetStatusHandler(container); diff --git a/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts b/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts index 7c24c9582..284931e8e 100644 --- a/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts +++ b/lambdas/api-handler/src/services/__tests__/letter-operations.test.ts @@ -175,7 +175,7 @@ describe('getLetterDataUrl function', () => { APIM_CORRELATION_HEADER: 'nhsd-correlation-id', DOWNLOAD_URL_TTL_SECONDS: 60 }; - const deps: Deps = { s3Client, letterRepo, logger, env }; + const deps: Deps = { s3Client, letterRepo, logger, env } as Deps; it('should return pre signed url successfully', async () => { From 16f802ce8c77c3c10d4908d3c0a665f9d5be88c3 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Wed, 29 Oct 2025 14:25:01 +0000 Subject: [PATCH 02/11] Address review comments --- .../terraform/components/api/locals.tf | 14 ++++++------- .../api/module_lambda_get_status.tf | 4 ---- lambdas/api-handler/src/config/deps.ts | 21 +++++++++++-------- .../src/handlers/__tests__/get_status.test.ts | 2 +- .../api-handler/src/handlers/get-status.ts | 4 ++-- 5 files changed, 22 insertions(+), 23 deletions(-) diff --git a/infrastructure/terraform/components/api/locals.tf b/infrastructure/terraform/components/api/locals.tf index 641b40dec..01785925a 100644 --- a/infrastructure/terraform/components/api/locals.tf +++ b/infrastructure/terraform/components/api/locals.tf @@ -5,14 +5,14 @@ locals { root_domain_nameservers = local.acct.route53_zone_nameservers["supplier-api"] openapi_spec = templatefile("${path.module}/resources/spec.tmpl.json", { - APIG_EXECUTION_ROLE_ARN = aws_iam_role.api_gateway_execution_role.arn - AWS_REGION = var.region - AUTHORIZER_LAMBDA_ARN = module.authorizer_lambda.function_arn - GET_LETTER_LAMBDA_ARN = module.get_letter.function_arn - GET_LETTERS_LAMBDA_ARN = module.get_letters.function_arn + APIG_EXECUTION_ROLE_ARN = aws_iam_role.api_gateway_execution_role.arn + AWS_REGION = var.region + AUTHORIZER_LAMBDA_ARN = module.authorizer_lambda.function_arn + GET_LETTER_LAMBDA_ARN = module.get_letter.function_arn + GET_LETTERS_LAMBDA_ARN = module.get_letters.function_arn GET_LETTER_DATA_LAMBDA_ARN = module.get_letter_data.function_arn - PATCH_LETTER_LAMBDA_ARN = module.patch_letter.function_arn - GET_STATUS_LAMBDA_ARN = module.get_status.function_arn + GET_STATUS_LAMBDA_ARN = module.get_status.function_arn + PATCH_LETTER_LAMBDA_ARN = module.patch_letter.function_arn }) destination_arn = "arn:aws:logs:${var.region}:${var.shared_infra_account_id}:destination:nhs-main-obs-firehose-logs" diff --git a/infrastructure/terraform/components/api/module_lambda_get_status.tf b/infrastructure/terraform/components/api/module_lambda_get_status.tf index be93702c9..4f6a45309 100644 --- a/infrastructure/terraform/components/api/module_lambda_get_status.tf +++ b/infrastructure/terraform/components/api/module_lambda_get_status.tf @@ -34,10 +34,6 @@ module "get_status" { send_to_firehose = true log_destination_arn = local.destination_arn log_subscription_role_arn = local.acct.log_subscription_role_arn - - lambda_env_vars = merge(local.common_lambda_env_vars, { - MAX_LIMIT = var.max_get_limit - }) } data "aws_iam_policy_document" "get_status_lambda" { diff --git a/lambdas/api-handler/src/config/deps.ts b/lambdas/api-handler/src/config/deps.ts index 26028814f..440b1a8c5 100644 --- a/lambdas/api-handler/src/config/deps.ts +++ b/lambdas/api-handler/src/config/deps.ts @@ -13,36 +13,39 @@ export type Deps = { env: EnvVars }; -function createLetterRepository(log: pino.Logger, envVars: EnvVars): LetterRepository { +function createDocumentClient(): DynamoDBDocumentClient { const ddbClient = new DynamoDBClient({}); - const docClient = DynamoDBDocumentClient.from(ddbClient); + return DynamoDBDocumentClient.from(ddbClient); +} + + +function createLetterRepository(documentClient: DynamoDBDocumentClient, log: pino.Logger, envVars: EnvVars): LetterRepository { const config = { lettersTableName: envVars.LETTERS_TABLE_NAME, ttlHours: envVars.LETTER_TTL_HOURS }; - return new LetterRepository(docClient, log, config); + return new LetterRepository(documentClient, log, config); } -function createDBHealthcheck(envVars: EnvVars): DBHealthcheck { - const ddbClient = new DynamoDBClient({}); - const docClient = DynamoDBDocumentClient.from(ddbClient); +function createDBHealthcheck(documentClient: DynamoDBDocumentClient, envVars: EnvVars): DBHealthcheck { const config = { lettersTableName: envVars.LETTERS_TABLE_NAME, ttlHours: envVars.LETTER_TTL_HOURS }; - return new DBHealthcheck(docClient, config); + return new DBHealthcheck(documentClient, config); } export function createDependenciesContainer(): Deps { const log = pino(); + const documentClient = createDocumentClient(); return { s3Client: new S3Client(), - letterRepo: createLetterRepository(log, envVars), - dbHealthcheck: createDBHealthcheck(envVars), + letterRepo: createLetterRepository(documentClient, log, envVars), + dbHealthcheck: createDBHealthcheck(documentClient, envVars), logger: log, env: envVars }; diff --git a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts index 3340d9694..3356852de 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts @@ -19,7 +19,7 @@ describe('API Lambda handler', () => { expect(result).toEqual({ statusCode: 200, - body: JSON.stringify({}, null, 2), + body: '{}', }); }); diff --git a/lambdas/api-handler/src/handlers/get-status.ts b/lambdas/api-handler/src/handlers/get-status.ts index d13d6f615..7f41beb58 100644 --- a/lambdas/api-handler/src/handlers/get-status.ts +++ b/lambdas/api-handler/src/handlers/get-status.ts @@ -25,11 +25,11 @@ export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { return { statusCode: 200, - body: JSON.stringify({}, null, 2) + body: '{}' }; } catch (error) { return mapErrorToResponse(error, commonHeadersResult.value.correlationId, deps.logger); - } + } } } From aa26b4215729dcdce35a4e0253f8df5c7cc72e71 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Wed, 29 Oct 2025 15:54:22 +0000 Subject: [PATCH 03/11] Fix following merge with main --- lambdas/api-handler/src/config/deps.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lambdas/api-handler/src/config/deps.ts b/lambdas/api-handler/src/config/deps.ts index 450b62646..c1695e156 100644 --- a/lambdas/api-handler/src/config/deps.ts +++ b/lambdas/api-handler/src/config/deps.ts @@ -23,7 +23,7 @@ function createDocumentClient(): DynamoDBDocumentClient { function createLetterRepository(documentClient: DynamoDBDocumentClient, log: pino.Logger, envVars: EnvVars): LetterRepository { const config = { lettersTableName: envVars.LETTERS_TABLE_NAME, - ttlHours: envVars.LETTER_TTL_HOURS + lettersTtlHours: envVars.LETTER_TTL_HOURS }; return new LetterRepository(documentClient, log, config); From 0ad56328eff70129b9c7ec3f51ddf63159b4c83c Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Thu, 30 Oct 2025 10:53:26 +0000 Subject: [PATCH 04/11] Avoid leaking system details in 500 error message --- .../api-handler/src/mappers/__tests__/error-mapper.test.ts | 4 ++-- lambdas/api-handler/src/mappers/error-mapper.ts | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts b/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts index 5110e32bc..8f944577d 100644 --- a/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts +++ b/lambdas/api-handler/src/mappers/__tests__/error-mapper.test.ts @@ -49,7 +49,7 @@ describe("mapErrorToResponse", () => { }); it("should map generic Error to InternalServerError response", () => { - const err = new Error("Something broke"); + const err = new Error("Low level error message"); const res = mapErrorToResponse(err, 'correlationId', { info: jest.fn(), error: jest.fn() } as unknown as Logger); @@ -58,7 +58,7 @@ describe("mapErrorToResponse", () => { "errors": [ { "code": "NOTIFY_INTERNAL_SERVER_ERROR", - "detail": "Something broke", + "detail": "Unexpected error", "id": "correlationId", "links": { "about": "https://digital.nhs.uk/developer/api-catalogue/nhs-notify-supplier" diff --git a/lambdas/api-handler/src/mappers/error-mapper.ts b/lambdas/api-handler/src/mappers/error-mapper.ts index 99493234b..84c144a3c 100644 --- a/lambdas/api-handler/src/mappers/error-mapper.ts +++ b/lambdas/api-handler/src/mappers/error-mapper.ts @@ -15,9 +15,6 @@ export function mapErrorToResponse(error: unknown, correlationId: string | undef } else if (error instanceof NotFoundError) { logger.info({ err: error }, `Not found error correlationId=${correlationId}`); return buildResponseFromErrorCode(ApiErrorCode.NotFound, error.detail, correlationId); - } else if (error instanceof Error) { - logger.error({ err: error }, `Internal server error correlationId=${correlationId}`); - return buildResponseFromErrorCode(ApiErrorCode.InternalServerError, error.message, correlationId); } else { logger.error({ err: error }, `Internal server error (non-Error thrown) correlationId=${correlationId}`); return buildResponseFromErrorCode(ApiErrorCode.InternalServerError, "Unexpected error", correlationId); From dcc4fc945f43224f4059e2460959ac74424ac563 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Thu, 30 Oct 2025 10:54:05 +0000 Subject: [PATCH 05/11] Tidy up @internal paths --- lambdas/api-handler/src/config/deps.ts | 2 +- lambdas/api-handler/src/handlers/__tests__/get_status.test.ts | 2 +- lambdas/api-handler/src/handlers/__tests__/post-mi.test.ts | 2 +- lambdas/api-handler/src/mappers/__tests__/mi-mapper.test.ts | 2 +- lambdas/api-handler/src/mappers/mi-mapper.ts | 2 +- lambdas/api-handler/src/services/mi-operations.ts | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lambdas/api-handler/src/config/deps.ts b/lambdas/api-handler/src/config/deps.ts index c1695e156..e8b07bde3 100644 --- a/lambdas/api-handler/src/config/deps.ts +++ b/lambdas/api-handler/src/config/deps.ts @@ -2,7 +2,7 @@ import { S3Client } from "@aws-sdk/client-s3"; import { DynamoDBClient } from '@aws-sdk/client-dynamodb'; import { DynamoDBDocumentClient } from '@aws-sdk/lib-dynamodb'; import pino from 'pino'; -import { LetterRepository, MIRepository, DBHealthcheck } from '../../../../internal/datastore'; +import { LetterRepository, MIRepository, DBHealthcheck } from '@internal/datastore'; import { envVars, EnvVars } from "../config/env"; export type Deps = { diff --git a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts index 3356852de..f95287817 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts @@ -1,5 +1,5 @@ import { S3Client } from "@aws-sdk/client-s3"; -import { DBHealthcheck } from "../../../../../internal/datastore/src"; +import { DBHealthcheck } from "@internal/datastore/src"; import pino from "pino"; import { Deps } from "../../config/deps"; import { makeApiGwEvent } from "./utils/test-utils"; diff --git a/lambdas/api-handler/src/handlers/__tests__/post-mi.test.ts b/lambdas/api-handler/src/handlers/__tests__/post-mi.test.ts index d7d942e14..10897fb76 100644 --- a/lambdas/api-handler/src/handlers/__tests__/post-mi.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/post-mi.test.ts @@ -4,7 +4,7 @@ import { makeApiGwEvent } from "./utils/test-utils"; import { PostMIRequest, PostMIResponse } from "../../contracts/mi"; import * as miService from '../../services/mi-operations'; import pino from 'pino'; -import { MIRepository } from "../../../../../internal/datastore/src"; +import { MIRepository } from "@internal/datastore/src"; import { Deps } from "../../config/deps"; import { EnvVars } from "../../config/env"; import { createPostMIHandler } from "../post-mi"; diff --git a/lambdas/api-handler/src/mappers/__tests__/mi-mapper.test.ts b/lambdas/api-handler/src/mappers/__tests__/mi-mapper.test.ts index be1a55c15..e36555fe5 100644 --- a/lambdas/api-handler/src/mappers/__tests__/mi-mapper.test.ts +++ b/lambdas/api-handler/src/mappers/__tests__/mi-mapper.test.ts @@ -1,4 +1,4 @@ -import { MIBase } from "../../../../../internal/datastore/src"; +import { MIBase } from "@internal/datastore/src"; import { IncomingMI, PostMIRequest } from "../../contracts/mi"; import { mapToMI, mapToPostMIResponse } from "../mi-mapper"; diff --git a/lambdas/api-handler/src/mappers/mi-mapper.ts b/lambdas/api-handler/src/mappers/mi-mapper.ts index 83848e1a2..1dc980d5a 100644 --- a/lambdas/api-handler/src/mappers/mi-mapper.ts +++ b/lambdas/api-handler/src/mappers/mi-mapper.ts @@ -1,4 +1,4 @@ -import { MIBase } from "../../../../internal/datastore/src"; +import { MIBase } from "@internal/datastore/src"; import { IncomingMI, PostMIRequest as PostMIRequest, PostMIResponse, PostMIResponseSchema } from "../contracts/mi"; export function mapToMI(request: PostMIRequest, supplierId: string): IncomingMI { diff --git a/lambdas/api-handler/src/services/mi-operations.ts b/lambdas/api-handler/src/services/mi-operations.ts index 2c574b676..d1cc207c1 100644 --- a/lambdas/api-handler/src/services/mi-operations.ts +++ b/lambdas/api-handler/src/services/mi-operations.ts @@ -1,4 +1,4 @@ -import { MIRepository } from "../../../../internal/datastore/src/mi-repository"; +import { MIRepository } from "@internal/datastore/src/mi-repository"; import { IncomingMI, PostMIResponse } from "../contracts/mi"; import { mapToPostMIResponse } from "../mappers/mi-mapper"; From 77ddbdd09c361dc1f628efeba92cede36d819389 Mon Sep 17 00:00:00 2001 From: Tim Ireland Date: Thu, 30 Oct 2025 12:03:29 +0000 Subject: [PATCH 06/11] Update .gitleaksignore added gitleaks ignore --- .gitleaksignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitleaksignore b/.gitleaksignore index 803deb3d7..996b43887 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -15,3 +15,4 @@ b1f85a7faf54eaf66074d7a6daa093aefe6b3ebe:sdk/python/pyproject.toml:ipv4:25 93e54b6baa390529aab08d9fd956837f7bb3f30:src/src.sln:ipv4:3 493e54b6baa390529aab08d9fd956837f7bb3f30:src/src.sln:ipv4:3 d8aaf7e033bf78fff491caa148897be266b60f67:src/src.sln:ipv4:3 +e12407e09151898bfd8d049d57eee9db9977d56b:.github/copilot-instructions.md:generic-api-key:213 From 2dffb63a87779680bd0d7433204489028c1af985 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Thu, 30 Oct 2025 13:07:55 +0000 Subject: [PATCH 07/11] Fix logging in error mapper --- lambdas/api-handler/src/mappers/error-mapper.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lambdas/api-handler/src/mappers/error-mapper.ts b/lambdas/api-handler/src/mappers/error-mapper.ts index 84c144a3c..e374d6dfe 100644 --- a/lambdas/api-handler/src/mappers/error-mapper.ts +++ b/lambdas/api-handler/src/mappers/error-mapper.ts @@ -15,6 +15,9 @@ export function mapErrorToResponse(error: unknown, correlationId: string | undef } else if (error instanceof NotFoundError) { logger.info({ err: error }, `Not found error correlationId=${correlationId}`); return buildResponseFromErrorCode(ApiErrorCode.NotFound, error.detail, correlationId); + } else if (error instanceof Error) { + logger.error({ err: error }, `Internal server error correlationId=${correlationId}`); + return buildResponseFromErrorCode(ApiErrorCode.InternalServerError, "Unexpected error", correlationId); } else { logger.error({ err: error }, `Internal server error (non-Error thrown) correlationId=${correlationId}`); return buildResponseFromErrorCode(ApiErrorCode.InternalServerError, "Unexpected error", correlationId); From 9bc9ad30101c5b59267ffc0d734f277f85bb83f6 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Fri, 31 Oct 2025 14:13:16 +0000 Subject: [PATCH 08/11] Address review comments --- .../components/api/resources/spec.tmpl.json | 5 ---- .../__tests__/get-letter-data.test.ts | 2 +- .../src/handlers/__tests__/get_status.test.ts | 30 +++++++++---------- .../api-handler/src/handlers/get-status.ts | 12 +++----- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/infrastructure/terraform/components/api/resources/spec.tmpl.json b/infrastructure/terraform/components/api/resources/spec.tmpl.json index 017a075a3..b72e0a380 100644 --- a/infrastructure/terraform/components/api/resources/spec.tmpl.json +++ b/infrastructure/terraform/components/api/resources/spec.tmpl.json @@ -34,11 +34,6 @@ "description": "Server error" } }, - "security": [ - { - "LambdaAuthorizer": [] - } - ], "summary": "Healthcheck endpoint", "x-amazon-apigateway-integration": { "contentHandling": "CONVERT_TO_TEXT", diff --git a/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts b/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts index 5f4e3ae77..38c339edc 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get-letter-data.test.ts @@ -18,7 +18,7 @@ import { makeApiGwEvent } from './utils/test-utils'; import { ValidationError } from '../../errors'; import * as errors from '../../contracts/errors'; import { createGetLetterDataHandler } from '../get-letter-data'; -import { Destination, S3Client } from '@aws-sdk/client-s3'; +import { S3Client } from '@aws-sdk/client-s3'; import pino from 'pino'; import { LetterRepository } from '@internal/datastore/src'; import { EnvVars } from '../../config/env'; diff --git a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts index f95287817..307fcd656 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts @@ -11,7 +11,7 @@ describe('API Lambda handler', () => { it('passes if S3 and DynamoDB are available', async() => { const event = makeApiGwEvent({path: '/_status', - headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId', 'x-request-id': 'requestId'} + headers: {'Nhsd-Correlation-Id': 'correlationId'} }); const getLetterDataHandler = createGetStatusHandler(getMockedDeps()); @@ -28,15 +28,14 @@ describe('API Lambda handler', () => { mockedDeps.s3Client.send = jest.fn().mockRejectedValue(new Error('unexpected error')); const event = makeApiGwEvent({path: '/_status', - headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId', 'x-request-id': 'requestId'} + headers: {'Nhsd-Correlation-Id': 'correlationId'} }); const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result).toEqual(expect.objectContaining({ - statusCode: 500 - })); + expect(result!.statusCode).toBe(500); + expect(JSON.parse(result!.body).errors[0].id).toBe('correlationId'); }); @@ -45,28 +44,29 @@ describe('API Lambda handler', () => { mockedDeps.dbHealthcheck.check = jest.fn().mockRejectedValue(new Error('unexpected error')); const event = makeApiGwEvent({path: '/_status', - headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId', 'x-request-id': 'requestId'} + headers: {'Nhsd-Correlation-Id': 'correlationId'} }); const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result).toEqual(expect.objectContaining({ - statusCode: 500 - })); + expect(result!.statusCode).toBe(500); + expect(JSON.parse(result!.body).errors[0].id).toBe('correlationId'); }); - it('fails if request ID is absent', async() => { + it('allows the correlation ID to be absent', async() => { + const mockedDeps = getMockedDeps(); + mockedDeps.dbHealthcheck.check = jest.fn().mockRejectedValue(new Error('unexpected error')); + const event = makeApiGwEvent({path: '/_status', - headers: {'nhsd-supplier-id': 'supplier1', 'nhsd-correlation-id': 'correlationId'} + headers: {} }); - const getLetterDataHandler = createGetStatusHandler(getMockedDeps()); + const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result).toEqual(expect.objectContaining({ - statusCode: 500 - })); + expect(result!.statusCode).toBe(500); + expect(JSON.parse(result!.body).errors[0].id).toBeDefined(); }); function getMockedDeps(): jest.Mocked { diff --git a/lambdas/api-handler/src/handlers/get-status.ts b/lambdas/api-handler/src/handlers/get-status.ts index 7f41beb58..cbdb639d9 100644 --- a/lambdas/api-handler/src/handlers/get-status.ts +++ b/lambdas/api-handler/src/handlers/get-status.ts @@ -1,18 +1,14 @@ import { APIGatewayProxyHandler } from "aws-lambda"; import { Deps } from "../config/deps"; import { ListBucketsCommand, S3Client } from "@aws-sdk/client-s3"; -import { validateCommonHeaders } from "../utils/validation"; import { mapErrorToResponse } from "../mappers/error-mapper"; export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { return async(event) => { - const commonHeadersResult = validateCommonHeaders(event.headers, deps); - - if (!commonHeadersResult.ok) { - return mapErrorToResponse(commonHeadersResult.error, commonHeadersResult.correlationId, deps.logger); - } + const correlationId = Object.entries(event.headers) + .find(([headerName, _]) => headerName.toLowerCase() === deps.env.APIM_CORRELATION_HEADER)?.[1]; try { await deps.dbHealthcheck.check(); @@ -20,7 +16,7 @@ export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { deps.logger.info({ description: 'Healthcheck passed', - supplierId: commonHeadersResult.value.supplierId + correlationId }); return { @@ -28,7 +24,7 @@ export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { body: '{}' }; } catch (error) { - return mapErrorToResponse(error, commonHeadersResult.value.correlationId, deps.logger); + return mapErrorToResponse(error, correlationId || '', deps.logger); } } } From 77bf03cba379ee6c388813f3122d13c822ada89e Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Fri, 31 Oct 2025 16:36:16 +0000 Subject: [PATCH 09/11] Terraform fix --- .../terraform/components/api/module_lambda_get_status.tf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/infrastructure/terraform/components/api/module_lambda_get_status.tf b/infrastructure/terraform/components/api/module_lambda_get_status.tf index 4f6a45309..37e33d5f8 100644 --- a/infrastructure/terraform/components/api/module_lambda_get_status.tf +++ b/infrastructure/terraform/components/api/module_lambda_get_status.tf @@ -34,6 +34,8 @@ module "get_status" { send_to_firehose = true log_destination_arn = local.destination_arn log_subscription_role_arn = local.acct.log_subscription_role_arn + + lambda_env_vars = merge(local.common_lambda_env_vars, {}) } data "aws_iam_policy_document" "get_status_lambda" { From 54da309ca50af876733f680df1eb4b636285d449 Mon Sep 17 00:00:00 2001 From: Steve Buxton Date: Mon, 3 Nov 2025 09:23:34 +0000 Subject: [PATCH 10/11] Allow for no headers --- .../src/handlers/__tests__/get_status.test.ts | 28 ++++++------------- .../api-handler/src/handlers/get-status.ts | 10 ++----- 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts index 307fcd656..09175d379 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts @@ -11,7 +11,7 @@ describe('API Lambda handler', () => { it('passes if S3 and DynamoDB are available', async() => { const event = makeApiGwEvent({path: '/_status', - headers: {'Nhsd-Correlation-Id': 'correlationId'} + headers: undefined }); const getLetterDataHandler = createGetStatusHandler(getMockedDeps()); @@ -28,14 +28,15 @@ describe('API Lambda handler', () => { mockedDeps.s3Client.send = jest.fn().mockRejectedValue(new Error('unexpected error')); const event = makeApiGwEvent({path: '/_status', - headers: {'Nhsd-Correlation-Id': 'correlationId'} + headers: undefined }); const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result!.statusCode).toBe(500); - expect(JSON.parse(result!.body).errors[0].id).toBe('correlationId'); + expect(result).toEqual(expect.objectContaining({ + statusCode: 500 + })); }); @@ -50,24 +51,11 @@ describe('API Lambda handler', () => { const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result!.statusCode).toBe(500); - expect(JSON.parse(result!.body).errors[0].id).toBe('correlationId'); + expect(result).toEqual(expect.objectContaining({ + statusCode: 500 + })); }); - it('allows the correlation ID to be absent', async() => { - const mockedDeps = getMockedDeps(); - mockedDeps.dbHealthcheck.check = jest.fn().mockRejectedValue(new Error('unexpected error')); - - const event = makeApiGwEvent({path: '/_status', - headers: {} - }); - - const getLetterDataHandler = createGetStatusHandler(mockedDeps); - const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - - expect(result!.statusCode).toBe(500); - expect(JSON.parse(result!.body).errors[0].id).toBeDefined(); - }); function getMockedDeps(): jest.Mocked { return { diff --git a/lambdas/api-handler/src/handlers/get-status.ts b/lambdas/api-handler/src/handlers/get-status.ts index cbdb639d9..f589eb987 100644 --- a/lambdas/api-handler/src/handlers/get-status.ts +++ b/lambdas/api-handler/src/handlers/get-status.ts @@ -5,18 +5,14 @@ import { mapErrorToResponse } from "../mappers/error-mapper"; export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { - return async(event) => { - - const correlationId = Object.entries(event.headers) - .find(([headerName, _]) => headerName.toLowerCase() === deps.env.APIM_CORRELATION_HEADER)?.[1]; + return async(_) => { try { await deps.dbHealthcheck.check(); await s3HealthCheck(deps.s3Client); deps.logger.info({ - description: 'Healthcheck passed', - correlationId + description: 'Healthcheck passed' }); return { @@ -24,7 +20,7 @@ export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { body: '{}' }; } catch (error) { - return mapErrorToResponse(error, correlationId || '', deps.logger); + return mapErrorToResponse(error, undefined, deps.logger); } } } From e345ec68b6d810ab276e9a89b710ddadc970ae5b Mon Sep 17 00:00:00 2001 From: Francisco Videira Date: Fri, 14 Nov 2025 15:29:39 +0000 Subject: [PATCH 11/11] Change response shape --- .../components/api/resources/spec.tmpl.json | 2 +- .../src/handlers/__tests__/get_status.test.ts | 16 +++++++++------- lambdas/api-handler/src/handlers/get-status.ts | 8 ++++++-- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/infrastructure/terraform/components/api/resources/spec.tmpl.json b/infrastructure/terraform/components/api/resources/spec.tmpl.json index b72e0a380..490d2dab8 100644 --- a/infrastructure/terraform/components/api/resources/spec.tmpl.json +++ b/infrastructure/terraform/components/api/resources/spec.tmpl.json @@ -28,7 +28,7 @@ "operationId": "getStatusId", "responses": { "200": { - "description": "Empty body" + "description": "OK" }, "500": { "description": "Server error" diff --git a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts index 09175d379..8fae2419e 100644 --- a/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts +++ b/lambdas/api-handler/src/handlers/__tests__/get_status.test.ts @@ -19,7 +19,7 @@ describe('API Lambda handler', () => { expect(result).toEqual({ statusCode: 200, - body: '{}', + body: JSON.stringify({ code: 200 }, null, 2) }); }); @@ -34,9 +34,10 @@ describe('API Lambda handler', () => { const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result).toEqual(expect.objectContaining({ - statusCode: 500 - })); + expect(result).toEqual({ + statusCode: 500, + body: JSON.stringify({ code: 500 }, null, 2) + }); }); @@ -51,9 +52,10 @@ describe('API Lambda handler', () => { const getLetterDataHandler = createGetStatusHandler(mockedDeps); const result = await getLetterDataHandler(event, mockDeep(), jest.fn()); - expect(result).toEqual(expect.objectContaining({ - statusCode: 500 - })); + expect(result).toEqual({ + statusCode: 500, + body: JSON.stringify({ code: 500 }, null, 2) + }); }); diff --git a/lambdas/api-handler/src/handlers/get-status.ts b/lambdas/api-handler/src/handlers/get-status.ts index f589eb987..81bbf9038 100644 --- a/lambdas/api-handler/src/handlers/get-status.ts +++ b/lambdas/api-handler/src/handlers/get-status.ts @@ -17,10 +17,14 @@ export function createGetStatusHandler(deps: Deps): APIGatewayProxyHandler { return { statusCode: 200, - body: '{}' + body: JSON.stringify({ code: 200 }, null, 2) }; } catch (error) { - return mapErrorToResponse(error, undefined, deps.logger); + deps.logger.error({ err: error }, 'Status endpoint error, services not available'); + return { + statusCode: 500, + body: JSON.stringify({ code: 500 }, null, 2) + }; } } }