Skip to content
This repository was archived by the owner on Jan 16, 2025. It is now read-only.

Commit 32492e3

Browse files
GuptaNavdeep1983github-actions[bot]npalm
authored
feat: migrate webhook runner configuration to SSM (#3728)
This PR migrates the confugration for the webhook from environment variables to SSM to avoid the maximum size of environment variables is reached. ## Implementation The webhook will read the configuration from SSM as json string. As long the lambda is hot the configuration is cached to speed-up the lambda time. In cases of configuration changes Lambda resources will be re-created by Terraform to ensure no cached values are used. fix: #3594 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Niek Palm <[email protected]> Co-authored-by: Niek Palm <[email protected]>
1 parent 1487f84 commit 32492e3

24 files changed

+164
-73
lines changed

Diff for: ‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ Talk to the forestkeepers in the `runners-channel` on Slack.
218218
| <a name="input_runners_ssm_housekeeper"></a> [runners\_ssm\_housekeeper](#input\_runners\_ssm\_housekeeper) | Configuration for the SSM housekeeper lambda. This lambda deletes token / JIT config from SSM.<br><br> `schedule_expression`: is used to configure the schedule for the lambda.<br> `enabled`: enable or disable the lambda trigger via the EventBridge.<br> `lambda_timeout`: timeout for the lambda in seconds.<br> `config`: configuration for the lambda function. Token path will be read by default from the module. | <pre>object({<br> schedule_expression = optional(string, "rate(1 day)")<br> enabled = optional(bool, true)<br> lambda_timeout = optional(number, 60)<br> config = object({<br> tokenPath = optional(string)<br> minimumDaysOld = optional(number, 1)<br> dryRun = optional(bool, false)<br> })<br> })</pre> | <pre>{<br> "config": {}<br>}</pre> | no |
219219
| <a name="input_scale_down_schedule_expression"></a> [scale\_down\_schedule\_expression](#input\_scale\_down\_schedule\_expression) | Scheduler expression to check every x for scale down. | `string` | `"cron(*/5 * * * ? *)"` | no |
220220
| <a name="input_scale_up_reserved_concurrent_executions"></a> [scale\_up\_reserved\_concurrent\_executions](#input\_scale\_up\_reserved\_concurrent\_executions) | Amount of reserved concurrent executions for the scale-up lambda function. A value of 0 disables lambda from being triggered and -1 removes any concurrency limitations. | `number` | `1` | no |
221-
| <a name="input_ssm_paths"></a> [ssm\_paths](#input\_ssm\_paths) | The root path used in SSM to store configuration and secrets. | <pre>object({<br> root = optional(string, "github-action-runners")<br> app = optional(string, "app")<br> runners = optional(string, "runners")<br> use_prefix = optional(bool, true)<br> })</pre> | `{}` | no |
221+
| <a name="input_ssm_paths"></a> [ssm\_paths](#input\_ssm\_paths) | The root path used in SSM to store configuration and secrets. | <pre>object({<br> root = optional(string, "github-action-runners")<br> app = optional(string, "app")<br> runners = optional(string, "runners")<br> webhook = optional(string, "webhook")<br> use_prefix = optional(bool, true)<br> })</pre> | `{}` | no |
222222
| <a name="input_state_event_rule_binaries_syncer"></a> [state\_event\_rule\_binaries\_syncer](#input\_state\_event\_rule\_binaries\_syncer) | Option to disable EventBridge Lambda trigger for the binary syncer, useful to stop automatic updates of binary distribution | `string` | `"ENABLED"` | no |
223223
| <a name="input_subnet_ids"></a> [subnet\_ids](#input\_subnet\_ids) | List of subnets in which the action runner instances will be launched. The subnets need to exist in the configured VPC (`vpc_id`), and must reside in different availability zones (see https://github.com/philips-labs/terraform-aws-github-runner/issues/2904) | `list(string)` | n/a | yes |
224224
| <a name="input_syncer_lambda_s3_key"></a> [syncer\_lambda\_s3\_key](#input\_syncer\_lambda\_s3\_key) | S3 key for syncer lambda function. Required if using an S3 bucket to specify lambdas. | `string` | `null` | no |

Diff for: ‎docs/configuration.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ The module uses the AWS System Manager Parameter Store to store configuration fo
2222
| `ssm_paths.root/var.prefix?/app/` | App secrets used by Lambda's |
2323
| `ssm_paths.root/var.prefix?/runners/config/<name>` | Configuration parameters used by runner start script |
2424
| `ssm_paths.root/var.prefix?/runners/tokens/<ec2-instance-id>` | Either JIT configuration (ephemeral runners) or registration tokens (non ephemeral runners) generated by the control plane (scale-up lambda), and consumed by the start script on the runner to activate / register the runner. |
25-
25+
| `ssm_paths.root/var.prefix?/webhook/runner-matcher-config` | Runner matcher config used by webhook to decide the target for the webhook event. |
2626
Available configuration parameters:
2727

2828
| Parameter name | Description |

Diff for: ‎lambdas/functions/control-plane/src/lambda.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ jest.mock('./scale-runners/scale-down');
6464
jest.mock('./pool/pool');
6565
jest.mock('./scale-runners/ssm-housekeeper');
6666
jest.mock('@terraform-aws-github-runner/aws-powertools-util');
67+
jest.mock('@terraform-aws-github-runner/aws-ssm-util');
6768

6869
// Docs for testing async with jest: https://jestjs.io/docs/tutorial-async
6970
describe('Test scale up lambda wrapper.', () => {

Diff for: ‎lambdas/functions/webhook/src/ConfigResolver.ts

+35-11
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,39 @@
1-
import { QueueConfig } from './sqs';
1+
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';
2+
import { RunnerMatcherConfig } from './sqs';
3+
import { logger } from '@terraform-aws-github-runner/aws-powertools-util';
24

35
export class Config {
4-
public repositoryAllowList: Array<string>;
5-
public queuesConfig: Array<QueueConfig>;
6-
public workflowJobEventSecondaryQueue;
7-
8-
constructor() {
9-
const repositoryAllowListEnv = process.env.REPOSITORY_ALLOW_LIST || '[]';
10-
this.repositoryAllowList = JSON.parse(repositoryAllowListEnv) as Array<string>;
11-
const queuesConfigEnv = process.env.RUNNER_CONFIG || '[]';
12-
this.queuesConfig = JSON.parse(queuesConfigEnv) as Array<QueueConfig>;
13-
this.workflowJobEventSecondaryQueue = process.env.SQS_WORKFLOW_JOB_QUEUE || undefined;
6+
repositoryAllowList: Array<string>;
7+
static matcherConfig: Array<RunnerMatcherConfig> | undefined;
8+
static webhookSecret: string | undefined;
9+
workflowJobEventSecondaryQueue: string | undefined;
10+
11+
constructor(repositoryAllowList: Array<string>, workflowJobEventSecondaryQueue: string | undefined) {
12+
this.repositoryAllowList = repositoryAllowList;
13+
14+
this.workflowJobEventSecondaryQueue = workflowJobEventSecondaryQueue;
15+
}
16+
17+
static async load(): Promise<Config> {
18+
const repositoryAllowListEnv = process.env.REPOSITORY_ALLOW_LIST ?? '[]';
19+
const repositoryAllowList = JSON.parse(repositoryAllowListEnv) as Array<string>;
20+
// load parallel config if not cached
21+
if (!Config.matcherConfig) {
22+
const matcherConfigPath =
23+
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH ?? '/github-runner/runner-matcher-config';
24+
const [matcherConfigVal, webhookSecret] = await Promise.all([
25+
getParameter(matcherConfigPath),
26+
getParameter(process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET),
27+
]);
28+
Config.webhookSecret = webhookSecret;
29+
Config.matcherConfig = JSON.parse(matcherConfigVal) as Array<RunnerMatcherConfig>;
30+
logger.debug('Loaded queues config', { matcherConfig: Config.matcherConfig });
31+
}
32+
const workflowJobEventSecondaryQueue = process.env.SQS_WORKFLOW_JOB_QUEUE ?? undefined;
33+
return new Config(repositoryAllowList, workflowJobEventSecondaryQueue);
34+
}
35+
36+
static reset(): void {
37+
Config.matcherConfig = undefined;
1438
}
1539
}

Diff for: ‎lambdas/functions/webhook/src/lambda.test.ts

+6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { mocked } from 'jest-mock';
55
import { githubWebhook } from './lambda';
66
import { handle } from './webhook';
77
import ValidationError from './ValidatonError';
8+
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';
89

910
const event: APIGatewayEvent = {
1011
body: JSON.stringify(''),
@@ -73,8 +74,13 @@ const context: Context = {
7374
};
7475

7576
jest.mock('./webhook');
77+
jest.mock('@terraform-aws-github-runner/aws-ssm-util');
7678

7779
describe('Test scale up lambda wrapper.', () => {
80+
beforeEach(() => {
81+
const mockedGet = mocked(getParameter);
82+
mockedGet.mockResolvedValue('[]');
83+
});
7884
it('Happy flow, resolve.', async () => {
7985
const mock = mocked(handle);
8086
mock.mockImplementation(() => {

Diff for: ‎lambdas/functions/webhook/src/lambda.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ middy(githubWebhook).use(captureLambdaHandler(tracer));
1616

1717
export async function githubWebhook(event: APIGatewayEvent, context: Context): Promise<Response> {
1818
setContext(context, 'lambda.ts');
19-
const config = new Config();
19+
const config = await Config.load();
2020

2121
logger.logEventIfEnabled(event);
2222
logger.debug('Loading config', { config });

Diff for: ‎lambdas/functions/webhook/src/local.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ import { handle } from './webhook';
55
import { Config } from './ConfigResolver';
66

77
const app = express();
8-
const config = new Config();
8+
const config = Config.load();
99

1010
app.use(bodyParser.json());
1111

12-
app.post('/event_handler', (req, res) => {
13-
handle(req.headers, JSON.stringify(req.body), config)
12+
app.post('/event_handler', async (req, res) => {
13+
handle(req.headers, JSON.stringify(req.body), await config)
1414
.then((c) => res.status(c.statusCode).end())
1515
.catch((e) => {
1616
console.log(e);

Diff for: ‎lambdas/functions/webhook/src/sqs/index.test.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { SendMessageCommandInput } from '@aws-sdk/client-sqs';
33
import { ActionRequestMessage, GithubWorkflowEvent, sendActionRequest, sendWebhookEventToWorkflowJobQueue } from '.';
44
import workflowjob_event from '../../test/resources/github_workflowjob_event.json';
55
import { Config } from '../ConfigResolver';
6+
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';
7+
import { mocked } from 'jest-mock';
68

79
const mockSQS = {
810
sendMessage: jest.fn(() => {
@@ -12,6 +14,7 @@ const mockSQS = {
1214
jest.mock('@aws-sdk/client-sqs', () => ({
1315
SQS: jest.fn().mockImplementation(() => mockSQS),
1416
}));
17+
jest.mock('@terraform-aws-github-runner/aws-ssm-util');
1518

1619
describe('Test sending message to SQS.', () => {
1720
const queueUrl = 'https://sqs.eu-west-1.amazonaws.com/123456789/queued-builds';
@@ -74,14 +77,18 @@ describe('Test sending message to SQS.', () => {
7477
QueueUrl: 'https://sqs.eu-west-1.amazonaws.com/123456789/webhook_events_workflow_job_queue',
7578
MessageBody: JSON.stringify(message),
7679
};
80+
beforeEach(() => {
81+
const mockedGet = mocked(getParameter);
82+
mockedGet.mockResolvedValue('[]');
83+
});
7784
afterEach(() => {
7885
jest.clearAllMocks();
7986
});
8087

8188
it('sends webhook events to workflow job queue', async () => {
8289
// Arrange
8390
process.env.SQS_WORKFLOW_JOB_QUEUE = sqsMessage.QueueUrl;
84-
const config = new Config();
91+
const config = await Config.load();
8592

8693
// Act
8794
const result = await sendWebhookEventToWorkflowJobQueue(message, config);
@@ -94,7 +101,7 @@ describe('Test sending message to SQS.', () => {
94101
it('Does not send webhook events to workflow job event copy queue', async () => {
95102
// Arrange
96103
process.env.SQS_WORKFLOW_JOB_QUEUE = '';
97-
const config = new Config();
104+
const config = await Config.load();
98105
// Act
99106
await sendWebhookEventToWorkflowJobQueue(message, config);
100107

@@ -105,7 +112,7 @@ describe('Test sending message to SQS.', () => {
105112
it('Catch the exception when even copy queue throws exception', async () => {
106113
// Arrange
107114
process.env.SQS_WORKFLOW_JOB_QUEUE = sqsMessage.QueueUrl;
108-
const config = new Config();
115+
const config = await Config.load();
109116

110117
const mockSQS = {
111118
sendMessage: jest.fn(() => {

Diff for: ‎lambdas/functions/webhook/src/sqs/index.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ export interface MatcherConfig {
2020
exactMatch: boolean;
2121
}
2222

23-
export type RunnerConfig = QueueConfig[];
23+
export type RunnerConfig = RunnerMatcherConfig[];
2424

25-
export interface QueueConfig {
25+
export interface RunnerMatcherConfig {
2626
matcherConfig: MatcherConfig;
2727
id: string;
2828
arn: string;

Diff for: ‎lambdas/functions/webhook/src/webhook/index.test.ts

+33-25
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,17 @@ describe('handler', () => {
3737
let originalError: Console['error'];
3838
let config: Config;
3939

40-
beforeEach(() => {
40+
beforeEach(async () => {
4141
process.env = { ...cleanEnv };
4242

4343
nock.disableNetConnect();
44-
config = new Config();
4544
originalError = console.error;
4645
console.error = jest.fn();
4746
jest.clearAllMocks();
4847
jest.resetAllMocks();
4948

50-
const mockedGet = mocked(getParameter);
51-
mockedGet.mockResolvedValueOnce(GITHUB_APP_WEBHOOK_SECRET);
49+
mockSSMResponse();
50+
config = await Config.load();
5251
});
5352

5453
afterEach(() => {
@@ -73,8 +72,8 @@ describe('handler', () => {
7372
});
7473

7574
describe('Test for workflowjob event: ', () => {
76-
beforeEach(() => {
77-
config = createConfig(undefined, runnerConfig);
75+
beforeEach(async () => {
76+
config = await createConfig(undefined, runnerConfig);
7877
});
7978

8079
it('handles workflow job events with 256 hash signature', async () => {
@@ -122,7 +121,7 @@ describe('handler', () => {
122121

123122
it('does not handle workflow_job events from unlisted repositories', async () => {
124123
const event = JSON.stringify(workflowjob_event);
125-
config = createConfig(['NotCodertocat/Hello-World']);
124+
config = await createConfig(['NotCodertocat/Hello-World']);
126125
await expect(
127126
handle({ 'X-Hub-Signature-256': await webhooks.sign(event), 'X-GitHub-Event': 'workflow_job' }, event, config),
128127
).rejects.toMatchObject({
@@ -133,7 +132,7 @@ describe('handler', () => {
133132

134133
it('handles workflow_job events without installation id', async () => {
135134
const event = JSON.stringify({ ...workflowjob_event, installation: null });
136-
config = createConfig(['philips-labs/terraform-aws-github-runner']);
135+
config = await createConfig(['philips-labs/terraform-aws-github-runner']);
137136
const resp = await handle(
138137
{ 'X-Hub-Signature-256': await webhooks.sign(event), 'X-GitHub-Event': 'workflow_job' },
139138
event,
@@ -145,7 +144,7 @@ describe('handler', () => {
145144

146145
it('handles workflow_job events from allow listed repositories', async () => {
147146
const event = JSON.stringify(workflowjob_event);
148-
config = createConfig(['philips-labs/terraform-aws-github-runner']);
147+
config = await createConfig(['philips-labs/terraform-aws-github-runner']);
149148
const resp = await handle(
150149
{ 'X-Hub-Signature-256': await webhooks.sign(event), 'X-GitHub-Event': 'workflow_job' },
151150
event,
@@ -156,7 +155,7 @@ describe('handler', () => {
156155
});
157156

158157
it('Check runner labels accept test job', async () => {
159-
config = createConfig(undefined, [
158+
config = await createConfig(undefined, [
160159
{
161160
...runnerConfig[0],
162161
matcherConfig: {
@@ -189,7 +188,7 @@ describe('handler', () => {
189188
});
190189

191190
it('Check runner labels accept job with mixed order.', async () => {
192-
config = createConfig(undefined, [
191+
config = await createConfig(undefined, [
193192
{
194193
...runnerConfig[0],
195194
matcherConfig: {
@@ -222,7 +221,7 @@ describe('handler', () => {
222221
});
223222

224223
it('Check webhook accept jobs where not all labels are provided in job.', async () => {
225-
config = createConfig(undefined, [
224+
config = await createConfig(undefined, [
226225
{
227226
...runnerConfig[0],
228227
matcherConfig: {
@@ -255,7 +254,7 @@ describe('handler', () => {
255254
});
256255

257256
it('Check webhook does not accept jobs where not all labels are supported (single matcher).', async () => {
258-
config = createConfig(undefined, [
257+
config = await createConfig(undefined, [
259258
{
260259
...runnerConfig[0],
261260
matcherConfig: {
@@ -282,7 +281,7 @@ describe('handler', () => {
282281
});
283282

284283
it('Check webhook does not accept jobs where the job labels are spread across label matchers.', async () => {
285-
config = createConfig(undefined, [
284+
config = await createConfig(undefined, [
286285
{
287286
...runnerConfig[0],
288287
matcherConfig: {
@@ -312,7 +311,7 @@ describe('handler', () => {
312311
});
313312

314313
it('Check webhook does not accept jobs where not all labels are supported by the runner.', async () => {
315-
config = createConfig(undefined, [
314+
config = await createConfig(undefined, [
316315
{
317316
...runnerConfig[0],
318317
matcherConfig: {
@@ -346,7 +345,7 @@ describe('handler', () => {
346345
});
347346

348347
it('Check webhook will accept jobs with a single acceptable label.', async () => {
349-
config = createConfig(undefined, [
348+
config = await createConfig(undefined, [
350349
{
351350
...runnerConfig[0],
352351
matcherConfig: {
@@ -380,7 +379,7 @@ describe('handler', () => {
380379
});
381380

382381
it('Check webhook will not accept jobs without correct label when job label check all is false.', async () => {
383-
config = createConfig(undefined, [
382+
config = await createConfig(undefined, [
384383
{
385384
...runnerConfig[0],
386385
matcherConfig: {
@@ -412,7 +411,7 @@ describe('handler', () => {
412411
expect(sendActionRequest).not.toBeCalled();
413412
});
414413
it('Check webhook will accept jobs for specific labels if workflow labels are specific', async () => {
415-
config = createConfig(undefined, [
414+
config = await createConfig(undefined, [
416415
{
417416
...runnerConfig[0],
418417
matcherConfig: {
@@ -454,7 +453,7 @@ describe('handler', () => {
454453
});
455454
});
456455
it('Check webhook will accept jobs for latest labels if workflow labels are not specific', async () => {
457-
config = createConfig(undefined, [
456+
config = await createConfig(undefined, [
458457
{
459458
...runnerConfig[0],
460459
matcherConfig: {
@@ -498,7 +497,7 @@ describe('handler', () => {
498497
});
499498

500499
it('Check webhook will accept jobs when matchers accepts multiple labels.', async () => {
501-
config = createConfig(undefined, [
500+
config = await createConfig(undefined, [
502501
{
503502
...runnerConfig[0],
504503
matcherConfig: {
@@ -599,12 +598,21 @@ describe('canRunJob', () => {
599598
});
600599
});
601600

602-
function createConfig(repositoryAllowList?: string[], runnerConfig?: RunnerConfig): Config {
601+
async function createConfig(repositoryAllowList?: string[], runnerConfig?: RunnerConfig): Promise<Config> {
603602
if (repositoryAllowList) {
604603
process.env.REPOSITORY_ALLOW_LIST = JSON.stringify(repositoryAllowList);
605604
}
606-
if (runnerConfig) {
607-
process.env.RUNNER_CONFIG = JSON.stringify(runnerConfig);
608-
}
609-
return new Config();
605+
Config.reset();
606+
mockSSMResponse(runnerConfig);
607+
return await Config.load();
608+
}
609+
function mockSSMResponse(runnerConfigInput?: RunnerConfig) {
610+
const mockedGet = mocked(getParameter);
611+
mockedGet.mockImplementation((parameter_name) => {
612+
const value =
613+
parameter_name == '/github-runner/runner-matcher-config'
614+
? JSON.stringify(runnerConfigInput ?? runnerConfig)
615+
: GITHUB_APP_WEBHOOK_SECRET;
616+
return Promise.resolve(value);
617+
});
610618
}

0 commit comments

Comments
 (0)