Address review comments

Author: stevebux

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",

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';

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<Context>(), 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<Context>(), 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<Context>(), 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<Deps> {

lambdas/api-handler/src/handlers/get-status.ts

@@ -1,34 +1,30 @@
 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();
       await s3HealthCheck(deps.s3Client);
 
       deps.logger.info({
         description: 'Healthcheck passed',
-        supplierId: commonHeadersResult.value.supplierId
+        correlationId
       });
 
       return {
         statusCode: 200,
         body: '{}'
       };
     } catch (error) {
-      return mapErrorToResponse(error, commonHeadersResult.value.correlationId, deps.logger);
+      return mapErrorToResponse(error, correlationId || '', deps.logger);
     }
   }
 }