Skip to content

Commit c1f5765

Browse files
bhansell1bhansell1
committed
CCM-10547: isolate clients to test suites
1 parent 6a8bf78 commit c1f5765

File tree

22 files changed

+264
-79
lines changed

22 files changed

+264
-79
lines changed

‎infrastructure/terraform/components/sandbox/outputs.tf‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ output "api_base_url" {
22
value = module.backend_api.api_base_url
33
}
44

5+
output "client_ssm_path_prefix" {
6+
value = module.backend_api.client_ssm_path_prefix
7+
}
8+
59
output "cognito_user_pool_id" {
610
value = aws_cognito_user_pool.sandbox.id
711
}

‎infrastructure/terraform/modules/backend-api/locals.tf‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ locals {
77
pdfjs_layer_zip = abspath("${local.lambdas_source_code_dir}/layers/pdfjs/dist/layer/pdfjs-layer.zip")
88
pdfjs_layer_lockfile = abspath("${local.lambdas_source_code_dir}/layers/pdfjs/package-lock.json")
99

10+
client_ssm_path_prefix = "/${var.csi}/clients"
11+
1012
openapi_spec = templatefile("${path.module}/spec.tmpl.json", {
1113
APIG_EXECUTION_ROLE_ARN = aws_iam_role.api_gateway_execution_role.arn
1214
AUTHORIZER_LAMBDA_ARN = module.authorizer_lambda.function_arn
@@ -39,7 +41,7 @@ locals {
3941
}
4042

4143
backend_lambda_environment_variables = {
42-
CLIENT_CONFIG_SSM_KEY_PREFIX = "${var.csi}/clients"
44+
CLIENT_CONFIG_SSM_KEY_PREFIX = local.client_ssm_path_prefix
4345
DEFAULT_LETTER_SUPPLIER = local.default_letter_supplier_name
4446
ENVIRONMENT = var.environment
4547
NODE_OPTIONS = "--enable-source-maps"

‎infrastructure/terraform/modules/backend-api/module_create_letter_template_lambda.tf‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,15 @@ data "aws_iam_policy_document" "create_letter_template_lambda_policy" {
7979
"${module.s3bucket_quarantine.arn}/pdf-template/*",
8080
]
8181
}
82+
83+
statement {
84+
sid = "AllowSSMParameterRead"
85+
effect = "Allow"
86+
actions = [
87+
"ssm:GetParameter",
88+
]
89+
resources = [
90+
"arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter${local.client_ssm_path_prefix}/*",
91+
]
92+
}
8293
}

‎infrastructure/terraform/modules/backend-api/module_create_template_lambda.tf‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,15 @@ data "aws_iam_policy_document" "create_template_lambda_policy" {
6464
var.kms_key_arn
6565
]
6666
}
67+
68+
statement {
69+
sid = "AllowSSMParameterRead"
70+
effect = "Allow"
71+
actions = [
72+
"ssm:GetParameter",
73+
]
74+
resources = [
75+
"arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter${local.client_ssm_path_prefix}/*",
76+
]
77+
}
6778
}

‎infrastructure/terraform/modules/backend-api/module_lambda_request_proof.tf‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,15 @@ data "aws_iam_policy_document" "request_proof_lambda_policy" {
7777
var.kms_key_arn
7878
]
7979
}
80+
81+
statement {
82+
sid = "AllowSSMParameterRead"
83+
effect = "Allow"
84+
actions = [
85+
"ssm:GetParameter",
86+
]
87+
resources = [
88+
"arn:aws:ssm:${var.region}:${var.aws_account_id}:parameter${local.client_ssm_path_prefix}/*",
89+
]
90+
}
8091
}

‎infrastructure/terraform/modules/backend-api/outputs.tf‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ output "api_base_url" {
22
value = aws_api_gateway_stage.main.invoke_url
33
}
44

5+
output "client_ssm_path_prefix" {
6+
value = local.client_ssm_path_prefix
7+
}
8+
59
output "download_bucket_name" {
610
value = module.s3bucket_download.id
711
}

‎lambdas/backend-api/src/__tests__/templates/api/create-letter.test.ts‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { createHandler } from '@backend-api/templates/api/create-letter';
22
import { mock } from 'jest-mock-extended';
3-
import {
4-
CreateUpdateTemplate,
5-
ClientConfiguration,
6-
TemplateDto,
7-
} from 'nhs-notify-backend-client';
3+
import { CreateUpdateTemplate, TemplateDto } from 'nhs-notify-backend-client';
84
import {
95
APIGatewayProxyEvent,
106
APIGatewayProxyResult,
@@ -16,16 +12,12 @@ import {
1612
} from 'nhs-notify-web-template-management-test-helper-utils';
1713
import { TemplateClient } from '@backend-api/templates/app/template-client';
1814

19-
jest.mock('nhs-notify-backend-client/src/client-configuration-client');
20-
2115
const setup = () => {
2216
const templateClient = mock<TemplateClient>();
2317

24-
const clientConfigurationFetch = jest.mocked(ClientConfiguration.fetch);
25-
2618
const handler = createHandler({ templateClient });
2719

28-
return { handler, mocks: { templateClient, clientConfigurationFetch } };
20+
return { handler, mocks: { templateClient } };
2921
};
3022

3123
const userId = '8B892046';
@@ -50,12 +42,6 @@ describe('create-letter', () => {
5042
test('successfully handles multipart form input and forwards PDF and CSV', async () => {
5143
const { handler, mocks } = setup();
5244

53-
mocks.clientConfigurationFetch.mockResolvedValueOnce(
54-
mock<ClientConfiguration>({
55-
campaignId: 'campaignId',
56-
})
57-
);
58-
5945
const pdfFilename = 'template.pdf';
6046
const csvFilename = 'data.csv';
6147
const pdfType = 'application/pdf';
@@ -130,11 +116,8 @@ describe('create-letter', () => {
130116
initialTemplate,
131117
{ userId, clientId },
132118
new File([pdf], pdfFilename, { type: pdfType }),
133-
new File([csv], csvFilename, { type: csvType }),
134-
'campaignId'
119+
new File([csv], csvFilename, { type: csvType })
135120
);
136-
137-
expect(mocks.clientConfigurationFetch).toHaveBeenCalledWith('example');
138121
});
139122

140123
test('successfully handles multipart form input without test data', async () => {
@@ -270,7 +253,6 @@ describe('create-letter', () => {
270253
initialTemplate,
271254
{ userId, clientId: undefined },
272255
new File([pdf], pdfFilename, { type: pdfType }),
273-
undefined,
274256
undefined
275257
);
276258
});
@@ -452,7 +434,6 @@ describe('create-letter', () => {
452434
initialTemplate,
453435
{ userId, clientId },
454436
new File([pdf], pdfFilename, { type: pdfType }),
455-
undefined,
456437
undefined
457438
);
458439
});

‎lambdas/backend-api/src/__tests__/templates/api/create.test.ts‎

Lines changed: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,15 @@
11
import type { APIGatewayProxyEvent, Context } from 'aws-lambda';
22
import { mock } from 'jest-mock-extended';
3-
import {
4-
TemplateDto,
5-
CreateUpdateTemplate,
6-
ClientConfiguration,
7-
} from 'nhs-notify-backend-client';
3+
import { TemplateDto, CreateUpdateTemplate } from 'nhs-notify-backend-client';
84
import { createHandler } from '@backend-api/templates/api/create';
95
import { TemplateClient } from '@backend-api/templates/app/template-client';
106

11-
jest.mock('nhs-notify-backend-client/src/client-configuration-client');
12-
137
const setup = () => {
148
const templateClient = mock<TemplateClient>();
159

16-
const clientConfigurationFetch = jest.mocked(ClientConfiguration.fetch);
17-
1810
const handler = createHandler({ templateClient });
1911

20-
return { handler, mocks: { templateClient, clientConfigurationFetch } };
12+
return { handler, mocks: { templateClient } };
2113
};
2214

2315
describe('Template API - Create', () => {
@@ -80,8 +72,7 @@ describe('Template API - Create', () => {
8072

8173
expect(mocks.templateClient.createTemplate).toHaveBeenCalledWith(
8274
{},
83-
{ userId: 'sub', clientId: 'nhs-notify-client-id' },
84-
undefined
75+
{ userId: 'sub', clientId: 'nhs-notify-client-id' }
8576
);
8677
});
8778

@@ -114,20 +105,13 @@ describe('Template API - Create', () => {
114105

115106
expect(mocks.templateClient.createTemplate).toHaveBeenCalledWith(
116107
{ id: 1 },
117-
{ userId: 'sub', clientId: 'nhs-notify-client-id' },
118-
undefined
108+
{ userId: 'sub', clientId: 'nhs-notify-client-id' }
119109
);
120110
});
121111

122112
test('should return template', async () => {
123113
const { handler, mocks } = setup();
124114

125-
mocks.clientConfigurationFetch.mockResolvedValueOnce(
126-
mock<ClientConfiguration>({
127-
campaignId: 'campaignId',
128-
})
129-
);
130-
131115
const create: CreateUpdateTemplate = {
132116
name: 'updated-name',
133117
message: 'message',
@@ -159,14 +143,10 @@ describe('Template API - Create', () => {
159143
body: JSON.stringify({ statusCode: 201, template: response }),
160144
});
161145

162-
expect(mocks.templateClient.createTemplate).toHaveBeenCalledWith(
163-
create,
164-
{
165-
userId: 'sub',
166-
clientId: 'notify-client-id',
167-
},
168-
'campaignId'
169-
);
146+
expect(mocks.templateClient.createTemplate).toHaveBeenCalledWith(create, {
147+
userId: 'sub',
148+
clientId: 'notify-client-id',
149+
});
170150
});
171151

172152
test('should return template when no clientId in auth context', async () => {

‎lambdas/backend-api/src/__tests__/templates/infra/client-config-repository.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ function setup() {
2323

2424
const mockCSI = 'test-csi';
2525
const mockClientId = 'test-client-123';
26-
const mockKey = `${mockCSI}/clients/${mockClientId}`;
26+
const mockKey = `${mockCSI}/${mockClientId}`;
2727

2828
const validNotifyClient: NotifyClient = {
2929
campaignId: 'campaign-123',
@@ -254,7 +254,7 @@ describe('ClientConfigRepository', () => {
254254
expect(result).toBeUndefined();
255255
});
256256

257-
it('should propagate SSM client errors', async () => {
257+
it('should return undefined SSM client errors', async () => {
258258
const {
259259
mocks: { ssmClient, cache, logger },
260260
} = setup();
@@ -270,9 +270,9 @@ describe('ClientConfigRepository', () => {
270270

271271
ssmClient.on(GetParameterCommand).rejectsOnce(ssmError);
272272

273-
await expect(repository.get(mockClientId)).rejects.toThrow(
274-
'SSM service unavailable'
275-
);
273+
const client = await repository.get(mockClientId);
274+
275+
expect(client).toBeUndefined();
276276
});
277277
});
278278

‎lambdas/backend-api/src/templates/infra/client-config-repository.ts‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,28 @@ export class ClientConfigRepository {
2727
return notifyClient;
2828
}
2929

30-
const config = await this.ssmClient.send(
31-
new GetParameterCommand({
32-
Name: key,
33-
})
34-
);
30+
let config: string | undefined;
31+
32+
try {
33+
const response = await this.ssmClient.send(
34+
new GetParameterCommand({
35+
Name: key,
36+
})
37+
);
38+
config = response.Parameter?.Value;
39+
} catch (error) {
40+
this.logger.error('failed to obtain client configuration', {
41+
error,
42+
clientId,
43+
key,
44+
});
45+
return;
46+
}
3547

36-
const { data, error, success } = $NotifyClientProcessed.safeParse(
37-
config.Parameter?.Value
38-
);
48+
const { data, error, success } = $NotifyClientProcessed.safeParse(config);
3949

4050
if (!success) {
41-
this.logger.error('failed to obtain client configuration', error, {
51+
this.logger.error('Failed to parse client configuration', error, {
4252
clientId,
4353
});
4454
return;

0 commit comments

Comments
 (0)