Skip to content

Commit

Permalink
feat: migrate webhook runner configuration to SSM (#3728)
Browse files Browse the repository at this point in the history
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 <npalm@users.noreply.github.com>
Co-authored-by: Niek Palm <niek.palm@philips.com>
  • Loading branch information
4 people committed Feb 26, 2024
1 parent 1487f84 commit 32492e3
Show file tree
Hide file tree
Showing 24 changed files with 164 additions and 73 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ Talk to the forestkeepers in the `runners-channel` on Slack.
| <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 |
| <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 |
| <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 |
| <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 |
| <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 |
| <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 |
| <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 |
| <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 |
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ The module uses the AWS System Manager Parameter Store to store configuration fo
| `ssm_paths.root/var.prefix?/app/` | App secrets used by Lambda's |
| `ssm_paths.root/var.prefix?/runners/config/<name>` | Configuration parameters used by runner start script |
| `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. |

| `ssm_paths.root/var.prefix?/webhook/runner-matcher-config` | Runner matcher config used by webhook to decide the target for the webhook event. |
Available configuration parameters:

| Parameter name | Description |
Expand Down
1 change: 1 addition & 0 deletions lambdas/functions/control-plane/src/lambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ jest.mock('./scale-runners/scale-down');
jest.mock('./pool/pool');
jest.mock('./scale-runners/ssm-housekeeper');
jest.mock('@terraform-aws-github-runner/aws-powertools-util');
jest.mock('@terraform-aws-github-runner/aws-ssm-util');

// Docs for testing async with jest: https://jestjs.io/docs/tutorial-async
describe('Test scale up lambda wrapper.', () => {
Expand Down
46 changes: 35 additions & 11 deletions lambdas/functions/webhook/src/ConfigResolver.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,39 @@
import { QueueConfig } from './sqs';
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';
import { RunnerMatcherConfig } from './sqs';
import { logger } from '@terraform-aws-github-runner/aws-powertools-util';

export class Config {
public repositoryAllowList: Array<string>;
public queuesConfig: Array<QueueConfig>;
public workflowJobEventSecondaryQueue;

constructor() {
const repositoryAllowListEnv = process.env.REPOSITORY_ALLOW_LIST || '[]';
this.repositoryAllowList = JSON.parse(repositoryAllowListEnv) as Array<string>;
const queuesConfigEnv = process.env.RUNNER_CONFIG || '[]';
this.queuesConfig = JSON.parse(queuesConfigEnv) as Array<QueueConfig>;
this.workflowJobEventSecondaryQueue = process.env.SQS_WORKFLOW_JOB_QUEUE || undefined;
repositoryAllowList: Array<string>;
static matcherConfig: Array<RunnerMatcherConfig> | undefined;
static webhookSecret: string | undefined;
workflowJobEventSecondaryQueue: string | undefined;

constructor(repositoryAllowList: Array<string>, workflowJobEventSecondaryQueue: string | undefined) {
this.repositoryAllowList = repositoryAllowList;

this.workflowJobEventSecondaryQueue = workflowJobEventSecondaryQueue;
}

static async load(): Promise<Config> {
const repositoryAllowListEnv = process.env.REPOSITORY_ALLOW_LIST ?? '[]';
const repositoryAllowList = JSON.parse(repositoryAllowListEnv) as Array<string>;
// load parallel config if not cached
if (!Config.matcherConfig) {
const matcherConfigPath =
process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH ?? '/github-runner/runner-matcher-config';
const [matcherConfigVal, webhookSecret] = await Promise.all([
getParameter(matcherConfigPath),
getParameter(process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET),
]);
Config.webhookSecret = webhookSecret;
Config.matcherConfig = JSON.parse(matcherConfigVal) as Array<RunnerMatcherConfig>;
logger.debug('Loaded queues config', { matcherConfig: Config.matcherConfig });
}
const workflowJobEventSecondaryQueue = process.env.SQS_WORKFLOW_JOB_QUEUE ?? undefined;
return new Config(repositoryAllowList, workflowJobEventSecondaryQueue);
}

static reset(): void {
Config.matcherConfig = undefined;
}
}
6 changes: 6 additions & 0 deletions lambdas/functions/webhook/src/lambda.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { mocked } from 'jest-mock';
import { githubWebhook } from './lambda';
import { handle } from './webhook';
import ValidationError from './ValidatonError';
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';

const event: APIGatewayEvent = {
body: JSON.stringify(''),
Expand Down Expand Up @@ -73,8 +74,13 @@ const context: Context = {
};

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

describe('Test scale up lambda wrapper.', () => {
beforeEach(() => {
const mockedGet = mocked(getParameter);
mockedGet.mockResolvedValue('[]');
});
it('Happy flow, resolve.', async () => {
const mock = mocked(handle);
mock.mockImplementation(() => {
Expand Down
2 changes: 1 addition & 1 deletion lambdas/functions/webhook/src/lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ middy(githubWebhook).use(captureLambdaHandler(tracer));

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

logger.logEventIfEnabled(event);
logger.debug('Loading config', { config });
Expand Down
6 changes: 3 additions & 3 deletions lambdas/functions/webhook/src/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import { handle } from './webhook';
import { Config } from './ConfigResolver';

const app = express();
const config = new Config();
const config = Config.load();

app.use(bodyParser.json());

app.post('/event_handler', (req, res) => {
handle(req.headers, JSON.stringify(req.body), config)
app.post('/event_handler', async (req, res) => {
handle(req.headers, JSON.stringify(req.body), await config)
.then((c) => res.status(c.statusCode).end())
.catch((e) => {
console.log(e);
Expand Down
13 changes: 10 additions & 3 deletions lambdas/functions/webhook/src/sqs/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { SendMessageCommandInput } from '@aws-sdk/client-sqs';
import { ActionRequestMessage, GithubWorkflowEvent, sendActionRequest, sendWebhookEventToWorkflowJobQueue } from '.';
import workflowjob_event from '../../test/resources/github_workflowjob_event.json';
import { Config } from '../ConfigResolver';
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';
import { mocked } from 'jest-mock';

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

describe('Test sending message to SQS.', () => {
const queueUrl = 'https://sqs.eu-west-1.amazonaws.com/123456789/queued-builds';
Expand Down Expand Up @@ -74,14 +77,18 @@ describe('Test sending message to SQS.', () => {
QueueUrl: 'https://sqs.eu-west-1.amazonaws.com/123456789/webhook_events_workflow_job_queue',
MessageBody: JSON.stringify(message),
};
beforeEach(() => {
const mockedGet = mocked(getParameter);
mockedGet.mockResolvedValue('[]');
});
afterEach(() => {
jest.clearAllMocks();
});

it('sends webhook events to workflow job queue', async () => {
// Arrange
process.env.SQS_WORKFLOW_JOB_QUEUE = sqsMessage.QueueUrl;
const config = new Config();
const config = await Config.load();

// Act
const result = await sendWebhookEventToWorkflowJobQueue(message, config);
Expand All @@ -94,7 +101,7 @@ describe('Test sending message to SQS.', () => {
it('Does not send webhook events to workflow job event copy queue', async () => {
// Arrange
process.env.SQS_WORKFLOW_JOB_QUEUE = '';
const config = new Config();
const config = await Config.load();
// Act
await sendWebhookEventToWorkflowJobQueue(message, config);

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

const mockSQS = {
sendMessage: jest.fn(() => {
Expand Down
4 changes: 2 additions & 2 deletions lambdas/functions/webhook/src/sqs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ export interface MatcherConfig {
exactMatch: boolean;
}

export type RunnerConfig = QueueConfig[];
export type RunnerConfig = RunnerMatcherConfig[];

export interface QueueConfig {
export interface RunnerMatcherConfig {
matcherConfig: MatcherConfig;
id: string;
arn: string;
Expand Down
58 changes: 33 additions & 25 deletions lambdas/functions/webhook/src/webhook/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,17 @@ describe('handler', () => {
let originalError: Console['error'];
let config: Config;

beforeEach(() => {
beforeEach(async () => {
process.env = { ...cleanEnv };

nock.disableNetConnect();
config = new Config();
originalError = console.error;
console.error = jest.fn();
jest.clearAllMocks();
jest.resetAllMocks();

const mockedGet = mocked(getParameter);
mockedGet.mockResolvedValueOnce(GITHUB_APP_WEBHOOK_SECRET);
mockSSMResponse();
config = await Config.load();
});

afterEach(() => {
Expand All @@ -73,8 +72,8 @@ describe('handler', () => {
});

describe('Test for workflowjob event: ', () => {
beforeEach(() => {
config = createConfig(undefined, runnerConfig);
beforeEach(async () => {
config = await createConfig(undefined, runnerConfig);
});

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

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

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

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

it('Check runner labels accept test job', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -189,7 +188,7 @@ describe('handler', () => {
});

it('Check runner labels accept job with mixed order.', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -222,7 +221,7 @@ describe('handler', () => {
});

it('Check webhook accept jobs where not all labels are provided in job.', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -255,7 +254,7 @@ describe('handler', () => {
});

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

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

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

it('Check webhook will accept jobs with a single acceptable label.', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -380,7 +379,7 @@ describe('handler', () => {
});

it('Check webhook will not accept jobs without correct label when job label check all is false.', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -412,7 +411,7 @@ describe('handler', () => {
expect(sendActionRequest).not.toBeCalled();
});
it('Check webhook will accept jobs for specific labels if workflow labels are specific', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -454,7 +453,7 @@ describe('handler', () => {
});
});
it('Check webhook will accept jobs for latest labels if workflow labels are not specific', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -498,7 +497,7 @@ describe('handler', () => {
});

it('Check webhook will accept jobs when matchers accepts multiple labels.', async () => {
config = createConfig(undefined, [
config = await createConfig(undefined, [
{
...runnerConfig[0],
matcherConfig: {
Expand Down Expand Up @@ -599,12 +598,21 @@ describe('canRunJob', () => {
});
});

function createConfig(repositoryAllowList?: string[], runnerConfig?: RunnerConfig): Config {
async function createConfig(repositoryAllowList?: string[], runnerConfig?: RunnerConfig): Promise<Config> {
if (repositoryAllowList) {
process.env.REPOSITORY_ALLOW_LIST = JSON.stringify(repositoryAllowList);
}
if (runnerConfig) {
process.env.RUNNER_CONFIG = JSON.stringify(runnerConfig);
}
return new Config();
Config.reset();
mockSSMResponse(runnerConfig);
return await Config.load();
}
function mockSSMResponse(runnerConfigInput?: RunnerConfig) {
const mockedGet = mocked(getParameter);
mockedGet.mockImplementation((parameter_name) => {
const value =
parameter_name == '/github-runner/runner-matcher-config'
? JSON.stringify(runnerConfigInput ?? runnerConfig)
: GITHUB_APP_WEBHOOK_SECRET;
return Promise.resolve(value);
});
}

0 comments on commit 32492e3

Please sign in to comment.