Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: migrate webhook runner configuration to SSM #3728

Merged
merged 28 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d48f049
fix: use ssm parameter instead.
GuptaNavdeep1983 Jan 18, 2024
3716e8b
docs: auto update terraform docs
github-actions[bot] Jan 18, 2024
64c9d52
fix: missed files in previous commit.
GuptaNavdeep1983 Jan 18, 2024
1ecb209
Merge branch 'nav/chore/use-ssm' of github.com:philips-labs/terraform…
GuptaNavdeep1983 Jan 18, 2024
e53a4f1
fix: missed files in previous commit.
GuptaNavdeep1983 Jan 18, 2024
d91fbe0
fix: missed files in previous commit.
GuptaNavdeep1983 Jan 18, 2024
5b75756
fix: missed files in previous commit.
GuptaNavdeep1983 Jan 18, 2024
dc0289b
fix: missed files in previous commit.
GuptaNavdeep1983 Jan 18, 2024
01359c6
fix: used caching to avoid redundant ssm calls.
GuptaNavdeep1983 Feb 7, 2024
30d4e6a
Merge branch 'main' into nav/chore/use-ssm
GuptaNavdeep1983 Feb 7, 2024
89a1733
fix: used caching to avoid redundant ssm calls.
GuptaNavdeep1983 Feb 8, 2024
324264a
Merge branch 'main' into nav/chore/use-ssm
npalm Feb 9, 2024
fdd4272
Merge branch 'main' into nav/chore/use-ssm
npalm Feb 12, 2024
8bd0a43
fix: used caching to avoid redundant ssm calls.
GuptaNavdeep1983 Feb 14, 2024
a422671
docs: auto update terraform docs
github-actions[bot] Feb 14, 2024
95b73bc
fix: used caching to avoid redundant ssm calls.
GuptaNavdeep1983 Feb 14, 2024
7346a96
Merge branch 'nav/chore/use-ssm' of github.com:philips-labs/terraform…
GuptaNavdeep1983 Feb 14, 2024
c000ac9
fix: used caching to avoid redundant ssm calls.
GuptaNavdeep1983 Feb 15, 2024
4209efc
docs: auto update terraform docs
github-actions[bot] Feb 15, 2024
11685a4
fix: used caching to avoid redundant ssm calls.
GuptaNavdeep1983 Feb 16, 2024
6b051fb
Merge branch 'nav/chore/use-ssm' of github.com:philips-labs/terraform…
GuptaNavdeep1983 Feb 16, 2024
17979f4
Update main.tf
GuptaNavdeep1983 Feb 21, 2024
382148b
fix: addressed comments.
GuptaNavdeep1983 Feb 21, 2024
0d24662
docs: auto update terraform docs
github-actions[bot] Feb 21, 2024
ec43280
Merge branch 'main' into nav/chore/use-ssm
npalm Feb 23, 2024
a4b36ec
add version constraint for null provider
npalm Feb 26, 2024
ee1c4b6
docs: auto update terraform docs
github-actions[bot] Feb 26, 2024
623e226
Merge branch 'main' into nav/chore/use-ssm
npalm Feb 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 the queues config from SSM if it's not already loaded and cached
if (!Config.matcherConfig) {
GuptaNavdeep1983 marked this conversation as resolved.
Show resolved Hide resolved
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
@@ -1,4 +1,4 @@
import { Webhooks } from '@octokit/webhooks';

Check notice on line 1 in lambdas/functions/webhook/src/webhook/index.test.ts

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Code Duplication

reduced similar code in: 'handles workflow job events with 256 hash signature'. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.
import { getParameter } from '@terraform-aws-github-runner/aws-ssm-util';
import { mocked } from 'jest-mock';
import nock from 'nock';
Expand Down Expand Up @@ -37,18 +37,17 @@
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('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 @@

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 @@

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 @@

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 @@
});

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 @@
});

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 @@
});

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 @@
});

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 @@
});

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 @@
});

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 @@
});

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 @@
});

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 @@
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 @@
});
});
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 @@
});

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 @@
});
});

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);
});
}
Loading
Loading