Skip to content

Commit 54403da

Browse files
edersonbrilhanteedersonbrilhante
committed
refactor(storage): extract webhook matcher config
1 parent 4726813 commit 54403da

16 files changed

Lines changed: 369 additions & 188 deletions

‎lambdas/functions/webhook/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"@aws-github-runner/aws-powertools-util": "*",
3232
"@aws-github-runner/aws-ssm-util": "*",
3333
"@aws-github-runner/compute-providers": "*",
34+
"@aws-github-runner/storage-providers": "*",
3435
"@aws-sdk/client-sqs": "^3.1009.0",
3536
"@middy/core": "^6.4.5",
3637
"@octokit/rest": "22.0.1",

‎lambdas/functions/webhook/src/ConfigLoader.test.ts‎

Lines changed: 42 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1-
import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util';
1+
import { getParameter } from '@aws-github-runner/aws-ssm-util';
2+
import { getRunnerMatcherConfigStore, type RunnerMatcherConfigStore } from '@aws-github-runner/storage-providers';
23
import { ConfigWebhook, ConfigWebhookEventBridge, ConfigDispatcher } from './ConfigLoader';
34

45
import { logger } from '@aws-github-runner/aws-powertools-util';
56
import { RunnerMatcherConfig } from './sqs';
67
import { describe, it, expect, beforeEach, vi } from 'vitest';
78

89
vi.mock('@aws-github-runner/aws-ssm-util');
10+
vi.mock('@aws-github-runner/storage-providers');
11+
12+
const runnerMatcherConfigStore = {
13+
get: vi.fn(),
14+
} satisfies RunnerMatcherConfigStore;
915

1016
describe('ConfigLoader Tests', () => {
1117
beforeEach(() => {
@@ -14,6 +20,7 @@ describe('ConfigLoader Tests', () => {
1420
ConfigWebhookEventBridge.reset();
1521
ConfigDispatcher.reset();
1622
logger.setLogLevel('DEBUG');
23+
vi.mocked(getRunnerMatcherConfigStore).mockReturnValue(runnerMatcherConfigStore);
1724

1825
// clear process.env
1926
for (const key of Object.keys(process.env)) {
@@ -24,7 +31,6 @@ describe('ConfigLoader Tests', () => {
2431
describe('Check base object', () => {
2532
function setupConfiguration(): void {
2633
process.env.EVENT_BUS_NAME = 'event-bus';
27-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
2834
process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret';
2935
const matcherConfig = [
3036
{
@@ -36,15 +42,8 @@ describe('ConfigLoader Tests', () => {
3642
},
3743
},
3844
];
39-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
40-
if (paramPath === '/path/to/matcher/config') {
41-
return JSON.stringify(matcherConfig);
42-
}
43-
if (paramPath === '/path/to/webhook/secret') {
44-
return 'secret';
45-
}
46-
return '';
47-
});
45+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig));
46+
vi.mocked(getParameter).mockResolvedValue('secret');
4847
}
4948

5049
it('should return the same instance of ConfigWebhook (singleton)', async () => {
@@ -53,7 +52,8 @@ describe('ConfigLoader Tests', () => {
5352
const config2 = await ConfigWebhook.load();
5453

5554
expect(config1).toBe(config2);
56-
expect(getParameter).toHaveBeenCalledTimes(2);
55+
expect(getParameter).toHaveBeenCalledOnce();
56+
expect(runnerMatcherConfigStore.get).toHaveBeenCalledOnce();
5757
});
5858

5959
it('should return the same instance of ConfigWebhookEventBridge (singleton)', async () => {
@@ -63,6 +63,7 @@ describe('ConfigLoader Tests', () => {
6363

6464
expect(config1).toBe(config2);
6565
expect(getParameter).toHaveBeenCalledTimes(1);
66+
expect(runnerMatcherConfigStore.get).not.toHaveBeenCalled();
6667
});
6768

6869
it('should return the same instance of ConfigDispatcher (singleton)', async () => {
@@ -71,7 +72,8 @@ describe('ConfigLoader Tests', () => {
7172
const config2 = await ConfigDispatcher.load();
7273

7374
expect(config1).toBe(config2);
74-
expect(getParameter).toHaveBeenCalledTimes(1);
75+
expect(getParameter).not.toHaveBeenCalled();
76+
expect(runnerMatcherConfigStore.get).toHaveBeenCalledOnce();
7577
});
7678

7779
it('should filter secrets from being logged', async () => {
@@ -95,7 +97,6 @@ describe('ConfigLoader Tests', () => {
9597
it('should load config successfully', async () => {
9698
process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]';
9799
process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret';
98-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
99100
const matcherConfig = [
100101
{
101102
id: '1',
@@ -106,15 +107,8 @@ describe('ConfigLoader Tests', () => {
106107
},
107108
},
108109
];
109-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
110-
if (paramPath === '/path/to/matcher/config') {
111-
return JSON.stringify(matcherConfig);
112-
}
113-
if (paramPath === '/path/to/webhook/secret') {
114-
return 'secret';
115-
}
116-
return '';
117-
});
110+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig));
111+
vi.mocked(getParameter).mockResolvedValue('secret');
118112

119113
const config: ConfigWebhook = await ConfigWebhook.load();
120114

@@ -124,7 +118,6 @@ describe('ConfigLoader Tests', () => {
124118
});
125119

126120
it('should load config successfully', async () => {
127-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
128121
process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret';
129122
const matcherConfig = [
130123
{
@@ -136,15 +129,8 @@ describe('ConfigLoader Tests', () => {
136129
},
137130
},
138131
];
139-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
140-
if (paramPath === '/path/to/matcher/config') {
141-
return JSON.stringify(matcherConfig);
142-
}
143-
if (paramPath === '/path/to/webhook/secret') {
144-
return 'secret';
145-
}
146-
return '';
147-
});
132+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig));
133+
vi.mocked(getParameter).mockResolvedValue('secret');
148134

149135
const config: ConfigWebhook = await ConfigWebhook.load();
150136

@@ -155,74 +141,42 @@ describe('ConfigLoader Tests', () => {
155141
});
156142

157143
it('should throw error if config loading fails', async () => {
158-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
159-
160-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
161-
if (paramPath === '/path/to/matcher/config') {
162-
throw new Error('Failed to load matcher config');
163-
}
164-
return '';
165-
});
144+
runnerMatcherConfigStore.get.mockRejectedValue(
145+
new Error(
146+
'Failed to load parameter for matcherConfig from path /path/to/matcher/config: Failed to load matcher config',
147+
),
148+
);
149+
vi.mocked(getParameter).mockResolvedValue('');
166150

167151
await expect(ConfigWebhook.load()).rejects.toThrow(
168152
'Failed to load config: Failed to load parameter for matcherConfig from path /path/to/matcher/config: Failed to load matcher config',
169153
);
170154
});
171155

172-
it('should load config successfully from multiple paths', async () => {
173-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config-1:/path/to/matcher/config-2';
156+
it('should load combined matcher config returned by the store', async () => {
174157
process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret';
175158

176-
const partialMatcher1 =
177-
'[{"id":"1","arn":"arn:aws:sqs:queue1","matcherConfig":{"labelMatchers":[["a"]],"exactMatch":true}}';
178-
const partialMatcher2 =
179-
',{"id":"2","arn":"arn:aws:sqs:queue2","matcherConfig":{"labelMatchers":[["b"]],"exactMatch":true}}]';
180-
181159
const combinedMatcherConfig = [
182160
{ id: '1', arn: 'arn:aws:sqs:queue1', matcherConfig: { labelMatchers: [['a']], exactMatch: true } },
183161
{ id: '2', arn: 'arn:aws:sqs:queue2', matcherConfig: { labelMatchers: [['b']], exactMatch: true } },
184162
];
185-
186-
// Mock getParameters for batch fetching multiple paths
187-
vi.mocked(getParameters).mockResolvedValue(
188-
new Map([
189-
['/path/to/matcher/config-1', partialMatcher1],
190-
['/path/to/matcher/config-2', partialMatcher2],
191-
]),
192-
);
193-
194-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
195-
if (paramPath === '/path/to/webhook/secret') return 'secret';
196-
return '';
197-
});
163+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(combinedMatcherConfig));
164+
vi.mocked(getParameter).mockResolvedValue('secret');
198165

199166
const config: ConfigWebhook = await ConfigWebhook.load();
200167

201168
expect(config.matcherConfig).toEqual(combinedMatcherConfig);
202169
expect(config.webhookSecret).toBe('secret');
203170
});
204171

205-
it('should throw error if config loading fails from multiple paths', async () => {
206-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config-1:/path/to/matcher/config-2';
172+
it('should propagate an error from the matcher config store', async () => {
207173
process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret';
208-
209-
const partialMatcher1 =
210-
'[{"id":"1","arn":"arn:aws:sqs:queue1","matcherConfig":{"labelMatchers":[["a"]],"exactMatch":true}}';
211-
const partialMatcher2 =
212-
',{"id":"2","arn":"arn:aws:sqs:queue2","matcherConfig":{"labelMatchers":[["b"]],"exactMatch":true}}';
213-
214-
// Mock getParameters for batch fetching - returns incomplete JSON that will fail to parse
215-
vi.mocked(getParameters).mockResolvedValue(
216-
new Map([
217-
['/path/to/matcher/config-1', partialMatcher1],
218-
['/path/to/matcher/config-2', partialMatcher2],
219-
]),
174+
runnerMatcherConfigStore.get.mockRejectedValue(
175+
new Error(
176+
"Failed to load/parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196",
177+
),
220178
);
221-
222-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
223-
if (paramPath === '/path/to/webhook/secret') return 'secret';
224-
return '';
225-
});
179+
vi.mocked(getParameter).mockResolvedValue('secret');
226180

227181
await expect(ConfigWebhook.load()).rejects.toThrow(
228182
"Failed to load config: Failed to load/parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196",
@@ -248,6 +202,7 @@ describe('ConfigLoader Tests', () => {
248202
expect(config.allowedEvents).toEqual(['push', 'pull_request']);
249203
expect(config.eventBusName).toBe('event-bus');
250204
expect(config.webhookSecret).toBe('secret');
205+
expect(runnerMatcherConfigStore.get).not.toHaveBeenCalled();
251206
});
252207

253208
it('should throw error if config loading fails', async () => {
@@ -264,7 +219,6 @@ describe('ConfigLoader Tests', () => {
264219
describe('ConfigDispatcher', () => {
265220
it('should load config successfully', async () => {
266221
process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]';
267-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
268222

269223
const matcherConfig: RunnerMatcherConfig[] = [
270224
{
@@ -276,40 +230,22 @@ describe('ConfigLoader Tests', () => {
276230
},
277231
},
278232
];
279-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
280-
if (paramPath === '/path/to/matcher/config') {
281-
return JSON.stringify(matcherConfig);
282-
}
283-
return '';
284-
});
233+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig));
285234

286235
const config: ConfigDispatcher = await ConfigDispatcher.load();
287236

288237
expect(config.repositoryAllowList).toEqual(['repo1', 'repo2']);
289238
expect(config.matcherConfig).toEqual(matcherConfig);
290239
});
291240

292-
it('should load config successfully from multiple paths with repo allow list', async () => {
241+
it('should load combined matcher config returned by the store with repo allow list', async () => {
293242
process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]';
294-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config-1:/path/to/matcher/config-2';
295-
296-
const partial1 =
297-
'[{"id":"1","arn":"arn:aws:sqs:queue1","matcherConfig":{"labelMatchers":[["x"]],"exactMatch":true}}';
298-
const partial2 =
299-
',{"id":"2","arn":"arn:aws:sqs:queue2","matcherConfig":{"labelMatchers":[["y"]],"exactMatch":true}}]';
300243

301244
const combined: RunnerMatcherConfig[] = [
302245
{ id: '1', arn: 'arn:aws:sqs:queue1', matcherConfig: { labelMatchers: [['x']], exactMatch: true } },
303246
{ id: '2', arn: 'arn:aws:sqs:queue2', matcherConfig: { labelMatchers: [['y']], exactMatch: true } },
304247
];
305-
306-
// Mock getParameters for batch fetching multiple paths
307-
vi.mocked(getParameters).mockResolvedValue(
308-
new Map([
309-
['/path/to/matcher/config-1', partial1],
310-
['/path/to/matcher/config-2', partial2],
311-
]),
312-
);
248+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(combined));
313249

314250
const config: ConfigDispatcher = await ConfigDispatcher.load();
315251

@@ -318,18 +254,15 @@ describe('ConfigLoader Tests', () => {
318254
});
319255

320256
it('should throw error if config loading fails', async () => {
321-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
322-
throw new Error(`Parameter ${paramPath} not found`);
323-
});
257+
runnerMatcherConfigStore.get.mockRejectedValue(new Error('Matcher config store is unavailable'));
324258

325259
await expect(ConfigDispatcher.load()).rejects.toThrow(
326-
'Failed to load config: Failed to load parameter for matcherConfig from path undefined: Parameter undefined not found',
260+
'Failed to load config: Matcher config store is unavailable',
327261
);
328262
});
329263

330264
it('should rely on default when optionals are not set.', async () => {
331265
process.env.ACCEPT_EVENTS = 'null';
332-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
333266
const matcherConfig: RunnerMatcherConfig[] = [
334267
{
335268
arn: 'arn:aws:sqs:eu-central-1:123456:npalm-default-queued-builds',
@@ -340,12 +273,7 @@ describe('ConfigLoader Tests', () => {
340273
},
341274
},
342275
];
343-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
344-
if (paramPath === '/path/to/matcher/config') {
345-
return JSON.stringify(matcherConfig);
346-
}
347-
return '';
348-
});
276+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig));
349277

350278
const config: ConfigDispatcher = await ConfigDispatcher.load();
351279

@@ -355,14 +283,7 @@ describe('ConfigLoader Tests', () => {
355283

356284
it('should throw an error if runner matcher config is empty.', async () => {
357285
process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]';
358-
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config';
359-
360-
vi.mocked(getParameter).mockImplementation(async (paramPath: string) => {
361-
if (paramPath === '/path/to/matcher/config') {
362-
return JSON.stringify('');
363-
}
364-
return '';
365-
});
286+
runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(''));
366287

367288
await expect(ConfigDispatcher.load()).rejects.toThrow('Failed to load config: Matcher config is empty');
368289
});

0 commit comments

Comments
 (0)