From 405b11db828234bfb1eb8482493a25505ce59a34 Mon Sep 17 00:00:00 2001 From: Niek Palm Date: Mon, 10 Jan 2022 11:09:36 +0100 Subject: [PATCH] feat: Strict label check and replace disable_check_wokflow_job_labels by opt in enable_workflow_job_labels_check (#1591) * Check strict labels * feat: Replace disable_check_wokflow_job_labels by opt in enable_workflow_job_labels_check, and check all labels. * Make check strict * update docs * cleanup --- README.md | 6 +- examples/ephemeral/main.tf | 11 +- examples/windows/main.tf | 2 +- main.tf | 6 +- modules/runners/logging.tf | 6 +- modules/runners/main.tf | 16 +-- modules/runners/scale-down.tf | 4 +- modules/runners/variables.tf | 2 +- modules/webhook/README.md | 3 +- .../lambdas/webhook/src/lambda.test.ts | 108 ++++++++++++++++++ modules/webhook/lambdas/webhook/src/lambda.ts | 16 ++- .../webhook/src/webhook/handler.test.ts | 64 +++-------- .../lambdas/webhook/src/webhook/handler.ts | 79 +++++++------ modules/webhook/variables.tf | 10 +- modules/webhook/webhook.tf | 16 +-- variables.tf | 8 +- 16 files changed, 233 insertions(+), 124 deletions(-) create mode 100644 modules/webhook/lambdas/webhook/src/lambda.test.ts diff --git a/README.md b/README.md index c235c979c5..32a9e7052c 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ Besides these permissions, the lambdas also need permission to CloudWatch (for l To be able to support a number of use-cases the module has quite a lot configuration options. We try to choose reasonable defaults. The several examples also shows for the main cases how to configure the runners. - Org vs Repo level. You can configure the module to connect the runners in GitHub on a org level and share the runners in your org. Or set the runners on repo level. The module will install the runner to the repo. This can be multiple repo's but runners are not shared between repo's. -- Checkrun vs Workflow job event. You can configure the webhook in GitHub to send checkrun or workflow job events to the webhook. Workflow job events are introduced by GitHub in September 2021 and are designed to support scalable runners. We advise when possible to use the workflow job event, you can set `disable_check_wokflow_job_labels = true` to disable the label check. +- Checkrun vs Workflow job event. You can configure the webhook in GitHub to send checkrun or workflow job events to the webhook. Workflow job events are introduced by GitHub in September 2021 and are designed to support scalable runners. We advise when possible to use the workflow job event, you can set `runner_enable_workflow_job_labels_check = true` to let the webhook only accept jobs based on the labels configured. The webhook will check the custom labels provided via the variable `runner_extra_labels` and the GitHub managed labels, "self-hosted", OS and architecture. The OS and architecture are derived from the settings. By default the check is disabled. - Linux vs Windows. you can configure the os types linux and win. Linux will be used by default. - Re-use vs Ephemeral. By default runners are re-used for till detected idle, once idle they will be removed from the pool. To improve security we are introducing ephemeral runners. Those runners are only used for one job. Ephemeral runners are only working in combination with the workflow job event. We also suggest to use a pre-build AMI to improve the start time of jobs. - GitHub cloud vs GitHub enterprise server (GHES). The runner support GitHub cloud as well GitHub enterprise service. For GHES we rely on our community to test and support. We have no possibility to test ourselves on GHES. @@ -382,7 +382,6 @@ In case the setup does not work as intended follow the trace of events: | [cloudwatch\_config](#input\_cloudwatch\_config) | (optional) Replaces the module default cloudwatch log config. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html for details. | `string` | `null` | no | | [create\_service\_linked\_role\_spot](#input\_create\_service\_linked\_role\_spot) | (optional) create the serviced linked role for spot instances that is required by the scale-up lambda. | `bool` | `false` | no | | [delay\_webhook\_event](#input\_delay\_webhook\_event) | The number of seconds the event accepted by the webhook is invisible on the queue before the scale up lambda will receive the event. | `number` | `30` | no | -| [disable\_check\_wokflow\_job\_labels](#input\_disable\_check\_wokflow\_job\_labels) | Disable the the check of workflow labels for received workflow job events. | `bool` | `false` | no | | [enable\_cloudwatch\_agent](#input\_enable\_cloudwatch\_agent) | Enabling the cloudwatch agent on the ec2 runner instances, the runner contains default config. Configuration can be overridden via `cloudwatch_config`. | `bool` | `true` | no | | [enable\_ephemeral\_runners](#input\_enable\_ephemeral\_runners) | Enable ephemeral runners, runners will only be used once. | `bool` | `false` | no | | [enable\_organization\_runners](#input\_enable\_organization\_runners) | Register runners to organization, instead of repo level | `bool` | `false` | no | @@ -426,7 +425,8 @@ In case the setup does not work as intended follow the trace of events: | [runner\_boot\_time\_in\_minutes](#input\_runner\_boot\_time\_in\_minutes) | The minimum time for an EC2 runner to boot and register as a runner. | `number` | `5` | no | | [runner\_ec2\_tags](#input\_runner\_ec2\_tags) | Map of tags that will be added to the launch template instance tag specificatons. | `map(string)` | `{}` | no | | [runner\_egress\_rules](#input\_runner\_egress\_rules) | List of egress rules for the GitHub runner instances. | 
list(object({
    cidr_blocks      = list(string)
    ipv6_cidr_blocks = list(string)
    prefix_list_ids  = list(string)
    from_port        = number
    protocol         = string
    security_groups  = list(string)
    self             = bool
    to_port          = number
    description      = string
  }))
| 
[
  {
    "cidr_blocks": [
      "0.0.0.0/0"
    ],
    "description": null,
    "from_port": 0,
    "ipv6_cidr_blocks": [
      "::/0"
    ],
    "prefix_list_ids": null,
    "protocol": "-1",
    "security_groups": null,
    "self": null,
    "to_port": 0
  }
]
| no | -| [runner\_extra\_labels](#input\_runner\_extra\_labels) | Extra labels for the runners (GitHub). Separate each label by a comma | `string` | `""` | no | +| [runner\_enable\_workflow\_job\_labels\_check](#input\_runner\_enable\_workflow\_job\_labels\_check) | If set to true all labels in the workflow job even are matched agaist the custom labels and GitHub labels (os, architecture and `self-hosted`). When the labels are not matching the event is dropped at the webhook. | `bool` | `false` | no | +| [runner\_extra\_labels](#input\_runner\_extra\_labels) | Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided. | `string` | `""` | no | | [runner\_group\_name](#input\_runner\_group\_name) | Name of the runner group. | `string` | `"Default"` | no | | [runner\_iam\_role\_managed\_policy\_arns](#input\_runner\_iam\_role\_managed\_policy\_arns) | Attach AWS or customer-managed IAM policies (by ARN) to the runner IAM role | `list(string)` | `[]` | no | | [runner\_log\_files](#input\_runner\_log\_files) | (optional) Replaces the module default cloudwatch log config. See https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-Configuration-File-Details.html for details. | 
list(object({
    log_group_name   = string
    prefix_log_group = bool
    file_path        = string
    log_stream_name  = string
  }))
| `null` | no | diff --git a/examples/ephemeral/main.tf b/examples/ephemeral/main.tf index d53c0f7869..b9d553823f 100644 --- a/examples/ephemeral/main.tf +++ b/examples/ephemeral/main.tf @@ -35,6 +35,9 @@ module "runners" { enable_organization_runners = true runner_extra_labels = "default,example" + # enable workflow labels check + # runner_enable_workflow_job_labels_check = true + # enable access to the runners via SSM enable_ssm_on_runners = true @@ -55,12 +58,12 @@ module "runners" { enable_ephemeral_runners = true # configure your pre-built AMI - # enabled_userdata = false - # ami_filter = { name = ["github-runner-amzn2-x86_64-2021*"] } - # ami_owners = [data.aws_caller_identity.current.account_id] + enabled_userdata = false + ami_filter = { name = ["github-runner-amzn2-x86_64-2021*"] } + ami_owners = [data.aws_caller_identity.current.account_id] # Enable logging - # log_level = "debug" + log_level = "debug" # Setup a dead letter queue, by default scale up lambda will kepp retrying to process event in case of scaling error. # redrive_policy_build_queue = { diff --git a/examples/windows/main.tf b/examples/windows/main.tf index d79edcb59e..0a1d770c64 100644 --- a/examples/windows/main.tf +++ b/examples/windows/main.tf @@ -31,7 +31,7 @@ module "runners" { runner_extra_labels = "default,example" # Set the OS to Windows - runner_os = "win" + runner_os = "windows" # we need to give the runner time to start because this is windows. runner_boot_time_in_minutes = 20 diff --git a/main.tf b/main.tf index db29db37fb..76ce82833d 100644 --- a/main.tf +++ b/main.tf @@ -67,8 +67,10 @@ module "webhook" { lambda_zip = var.webhook_lambda_zip lambda_timeout = var.webhook_lambda_timeout logging_retention_in_days = var.logging_retention_in_days - runner_extra_labels = var.runner_extra_labels - disable_check_wokflow_job_labels = var.disable_check_wokflow_job_labels + + # labels + enable_workflow_job_labels_check = var.runner_enable_workflow_job_labels_check + runner_labels = "self-hosted,${var.runner_os},${var.runner_architecture},${var.runner_extra_labels}" role_path = var.role_path role_permissions_boundary = var.role_permissions_boundary diff --git a/modules/runners/logging.tf b/modules/runners/logging.tf index 3d37ef804f..6bd0843fdb 100644 --- a/modules/runners/logging.tf +++ b/modules/runners/logging.tf @@ -12,19 +12,19 @@ locals { { "log_group_name" : "user_data", "prefix_log_group" : true, - "file_path" : var.runner_os == "win" ? "C:/UserData.log" : "/var/log/user-data.log", + "file_path" : var.runner_os == "windows" ? "C:/UserData.log" : "/var/log/user-data.log", "log_stream_name" : "{instance_id}" }, { "log_group_name" : "runner", "prefix_log_group" : true, - "file_path" : var.runner_os == "win" ? "C:/actions-runner/_diag/Runner_*.log" : "/home/runners/actions-runner/_diag/Runner_**.log", + "file_path" : var.runner_os == "windows" ? "C:/actions-runner/_diag/Runner_*.log" : "/home/runners/actions-runner/_diag/Runner_**.log", "log_stream_name" : "{instance_id}" }, { "log_group_name" : "runner-startup", "prefix_log_group" : true, - "file_path" : var.runner_os == "win" ? "C:/runner-startup.log" : "/var/log/runner-startup.log", + "file_path" : var.runner_os == "windows" ? "C:/runner-startup.log" : "/var/log/runner-startup.log", "log_stream_name" : "{instance_id}" } ] diff --git a/modules/runners/main.tf b/modules/runners/main.tf index cf73ab22bf..6c539bcfe4 100644 --- a/modules/runners/main.tf +++ b/modules/runners/main.tf @@ -16,23 +16,23 @@ locals { kms_key_arn = var.kms_key_arn != null ? var.kms_key_arn : "" default_ami = { - "win" = { name = ["Windows_Server-20H2-English-Core-ContainersLatest-*"] } - "linux" = var.runner_architecture == "arm64" ? { name = ["amzn2-ami-hvm-2*-arm64-gp2"] } : { name = ["amzn2-ami-hvm-2.*-x86_64-ebs"] } + "windows" = { name = ["Windows_Server-20H2-English-Core-ContainersLatest-*"] } + "linux" = var.runner_architecture == "arm64" ? { name = ["amzn2-ami-hvm-2*-arm64-gp2"] } : { name = ["amzn2-ami-hvm-2.*-x86_64-ebs"] } } default_userdata_template = { - "win" = "${path.module}/templates/user-data.ps1" - "linux" = "${path.module}/templates/user-data.sh" + "windows" = "${path.module}/templates/user-data.ps1" + "linux" = "${path.module}/templates/user-data.sh" } userdata_install_runner = { - "win" = "${path.module}/templates/install-runner.ps1" - "linux" = "${path.module}/templates/install-runner.sh" + "windows" = "${path.module}/templates/install-runner.ps1" + "linux" = "${path.module}/templates/install-runner.sh" } userdata_start_runner = { - "win" = "${path.module}/templates/start-runner.ps1" - "linux" = "${path.module}/templates/start-runner.sh" + "windows" = "${path.module}/templates/start-runner.ps1" + "linux" = "${path.module}/templates/start-runner.sh" } ami_filter = coalesce(var.ami_filter, local.default_ami[var.runner_os]) diff --git a/modules/runners/scale-down.tf b/modules/runners/scale-down.tf index 042535cc7b..44b2ae1749 100644 --- a/modules/runners/scale-down.tf +++ b/modules/runners/scale-down.tf @@ -1,8 +1,8 @@ locals { # Windows Runners can take their sweet time to do anything min_runtime_defaults = { - "win" = 15 - "linux" = 5 + "windows" = 15 + "linux" = 5 } } resource "aws_lambda_function" "scale_down" { diff --git a/modules/runners/variables.tf b/modules/runners/variables.tf index 7b6606b91c..59d2791200 100644 --- a/modules/runners/variables.tf +++ b/modules/runners/variables.tf @@ -96,7 +96,7 @@ variable "runner_os" { default = "linux" validation { - condition = contains(["linux", "win"], var.runner_os) + condition = contains(["linux", "windows"], var.runner_os) error_message = "Valid values for runner_os are (linux, win)." } } diff --git a/modules/webhook/README.md b/modules/webhook/README.md index 2430972b5e..61f359e322 100644 --- a/modules/webhook/README.md +++ b/modules/webhook/README.md @@ -74,6 +74,7 @@ No modules. |------|-------------|------|---------|:--------:| | [aws\_region](#input\_aws\_region) | AWS region. | `string` | n/a | yes | | [disable\_check\_wokflow\_job\_labels](#input\_disable\_check\_wokflow\_job\_labels) | Disable the the check of workflow labels. | `bool` | `false` | no | +| [enable\_workflow\_job\_labels\_check](#input\_enable\_workflow\_job\_labels\_check) | If set to true all labels in the workflow job even are matched agaist the custom labels and GitHub labels (os, architecture and `self-hosted`). When the labels are not matching the event is dropped at the webhook. | `bool` | `false` | no | | [environment](#input\_environment) | A name that identifies the environment, used as prefix and for tagging. | `string` | n/a | yes | | [github\_app\_webhook\_secret\_arn](#input\_github\_app\_webhook\_secret\_arn) | n/a | `string` | n/a | yes | | [kms\_key\_arn](#input\_kms\_key\_arn) | Optional CMK Key ARN to be used for Parameter Store. | `string` | `null` | no | @@ -86,7 +87,7 @@ No modules. | [repository\_white\_list](#input\_repository\_white\_list) | List of repositories allowed to use the github app | `list(string)` | `[]` | no | | [role\_path](#input\_role\_path) | The path that will be added to the role; if not set, the environment name will be used. | `string` | `null` | no | | [role\_permissions\_boundary](#input\_role\_permissions\_boundary) | Permissions boundary that will be added to the created role for the lambda. | `string` | `null` | no | -| [runner\_extra\_labels](#input\_runner\_extra\_labels) | Extra labels for the runners (GitHub). Separate each label by a comma | `string` | `""` | no | +| [runner\_labels](#input\_runner\_labels) | Labels for the runners (GitHub). Separate each label by a comma. Labels are used to check events when `runner_enable_workflow_job_labels_check` is set to `true`. | `string` | `""` | no | | [sqs\_build\_queue](#input\_sqs\_build\_queue) | SQS queue to publish accepted build events. | 
object({
    id  = string
    arn = string
  })
| n/a | yes | | [sqs\_build\_queue\_fifo](#input\_sqs\_build\_queue\_fifo) | Enable a FIFO queue to remain the order of events received by the webhook. Suggest to set to true for repo level runners. | `bool` | `false` | no | | [tags](#input\_tags) | Map of tags that will be added to created resources. By default resources will be tagged with name and environment. | `map(string)` | `{}` | no | diff --git a/modules/webhook/lambdas/webhook/src/lambda.test.ts b/modules/webhook/lambdas/webhook/src/lambda.test.ts new file mode 100644 index 0000000000..e1f710d6f8 --- /dev/null +++ b/modules/webhook/lambdas/webhook/src/lambda.test.ts @@ -0,0 +1,108 @@ +import { APIGatewayEvent, Context } from 'aws-lambda'; +import { mocked } from 'ts-jest/utils'; +import { githubWebhook } from './lambda'; +import { handle } from './webhook/handler'; +import { logger } from './webhook/logger'; + +const event: APIGatewayEvent = { + body: JSON.stringify(''), + headers: { abc: undefined }, + httpMethod: '', + isBase64Encoded: false, + multiValueHeaders: { abc: undefined }, + multiValueQueryStringParameters: null, + path: '', + pathParameters: null, + queryStringParameters: null, + stageVariables: null, + resource: '', + requestContext: { + authorizer: null, + accountId: '123456789012', + resourceId: '123456', + stage: 'prod', + requestId: 'c6af9ac6-7b61-11e6-9a41-93e8deadbeef', + requestTime: '09/Apr/2015:12:34:56 +0000', + requestTimeEpoch: 1428582896000, + identity: { + cognitoIdentityPoolId: null, + accountId: null, + cognitoIdentityId: null, + caller: null, + accessKey: null, + sourceIp: '127.0.0.1', + cognitoAuthenticationType: null, + cognitoAuthenticationProvider: null, + userArn: null, + userAgent: 'Custom User Agent String', + user: null, + clientCert: null, + apiKey: null, + apiKeyId: null, + principalOrgId: null, + }, + path: '/prod/path/to/resource', + resourcePath: '/{proxy+}', + httpMethod: 'POST', + apiId: '1234567890', + protocol: 'HTTP/1.1', + }, +}; + +const context: Context = { + awsRequestId: '1', + callbackWaitsForEmptyEventLoop: false, + functionName: '', + functionVersion: '', + getRemainingTimeInMillis: () => 0, + invokedFunctionArn: '', + logGroupName: '', + logStreamName: '', + memoryLimitInMB: '', + done: () => { + return; + }, + fail: () => { + return; + }, + succeed: () => { + return; + }, +}; + +jest.mock('./webhook/handler'); + +describe('Test scale up lambda wrapper.', () => { + it('Happy flow, resolve.', async () => { + const mock = mocked(handle); + mock.mockImplementation(() => { + return new Promise((resolve) => { + resolve({ statusCode: 200 }); + }); + }); + + const result = await githubWebhook(event, context); + expect(result).toEqual({ statusCode: 200 }); + }); + + it('An expected error, resolve.', async () => { + const mock = mocked(handle); + mock.mockImplementation(() => { + return new Promise((resolve) => { + resolve({ statusCode: 400 }); + }); + }); + + const result = await githubWebhook(event, context); + expect(result).toEqual({ statusCode: 400 }); + }); + + it('Errors are not thrown.', async () => { + const mock = mocked(handle); + const logSpy = jest.spyOn(logger, 'error'); + mock.mockRejectedValue(new Error('some error')); + const result = await githubWebhook(event, context); + expect(result).toMatchObject({ statusCode: 500 }); + expect(logSpy).toBeCalledTimes(1); + }); +}); diff --git a/modules/webhook/lambdas/webhook/src/lambda.ts b/modules/webhook/lambdas/webhook/src/lambda.ts index 232f74000e..07cad39a74 100644 --- a/modules/webhook/lambdas/webhook/src/lambda.ts +++ b/modules/webhook/lambdas/webhook/src/lambda.ts @@ -6,14 +6,18 @@ export interface Response { statusCode: number; body?: string; } - -export const githubWebhook = async (event: APIGatewayEvent, context: Context, callback: Callback): Promise => { +export async function githubWebhook(event: APIGatewayEvent, context: Context): Promise { logger.setSettings({ requestId: context.awsRequestId }); logger.debug(JSON.stringify(event)); + let result: Response; try { - const response = await handle(event.headers, event.body as string); - callback(null, response); + result = await handle(event.headers, event.body as string); } catch (e) { - callback(e as Error); + logger.error(e); + result = { + statusCode: 500, + body: 'Check the Lambda logs for the error details.', + }; } -}; + return result; +} diff --git a/modules/webhook/lambdas/webhook/src/webhook/handler.test.ts b/modules/webhook/lambdas/webhook/src/webhook/handler.test.ts index 76bbf737a3..d2fc9eafc9 100644 --- a/modules/webhook/lambdas/webhook/src/webhook/handler.test.ts +++ b/modules/webhook/lambdas/webhook/src/webhook/handler.test.ts @@ -120,8 +120,9 @@ describe('handler', () => { expect(sendActionRequest).toBeCalled(); }); - it('Check runner labels (test)', async () => { - process.env.RUNNER_LABELS = '["test"]'; + it('Check runner labels accept test job', async () => { + process.env.RUNNER_LABELS = '["self-hosted", "test"]'; + process.env.ENABLE_WORKFLOW_JOB_LABELS_CHECK = 'true'; const event = JSON.stringify({ ...workflowjob_event, workflow_job: { @@ -137,13 +138,14 @@ describe('handler', () => { expect(sendActionRequest).toBeCalled(); }); - it('Check runner labels (test)', async () => { - process.env.RUNNER_LABELS = '["test"]'; + it('Check runner labels accept job with mixed order.', async () => { + process.env.RUNNER_LABELS = '["linux", "test", "self-hosted"]'; + process.env.ENABLE_WORKFLOW_JOB_LABELS_CHECK = 'true'; const event = JSON.stringify({ ...workflowjob_event, workflow_job: { ...workflowjob_event.workflow_job, - labels: ['self-hosted', 'test'], + labels: ['linux', 'self-hosted', 'test'], }, }); const resp = await handle( @@ -154,8 +156,9 @@ describe('handler', () => { expect(sendActionRequest).toBeCalled(); }); - it('Check runner a self hosted runner will run a job marked with only self-hosted', async () => { - process.env.RUNNER_LABELS = '["test", "test2"]'; + it('Check webhook does not accept jobs where not all labels are provided in job.', async () => { + process.env.RUNNER_LABELS = '["self-hosted", "test", "test2"]'; + process.env.ENABLE_WORKFLOW_JOB_LABELS_CHECK = 'true'; const event = JSON.stringify({ ...workflowjob_event, workflow_job: { @@ -167,51 +170,18 @@ describe('handler', () => { { 'X-Hub-Signature': await webhooks.sign(event), 'X-GitHub-Event': 'workflow_job' }, event, ); - expect(resp.statusCode).toBe(201); - expect(sendActionRequest).toBeCalled(); - }); - - it('Check runner labels for a strict job (2 labels should match)', async () => { - process.env.RUNNER_LABELS = '["test", "test2"]'; - const event = JSON.stringify({ - ...workflowjob_event, - workflow_job: { - ...workflowjob_event.workflow_job, - labels: ['self-hosted', 'linux', 'test', 'test2'], - }, - }); - const resp = await handle( - { 'X-Hub-Signature': await webhooks.sign(event), 'X-GitHub-Event': 'workflow_job' }, - event, - ); - expect(resp.statusCode).toBe(201); - expect(sendActionRequest).toBeCalled(); + expect(resp.statusCode).toBe(202); + expect(sendActionRequest).not.toBeCalled; }); - it('Check event is accepted for disabled workflow check', async () => { - process.env.DISABLE_CHECK_WORKFLOW_JOB_LABELS = 'true'; - process.env.RUNNER_LABELS = '["test", "no-check"]'; - const event = JSON.stringify({ - ...workflowjob_event, - workflow_job: { - ...workflowjob_event.workflow_job, - labels: ['self-hosted', 'linux', 'test', 'test2'], - }, - }); - const resp = await handle( - { 'X-Hub-Signature': await webhooks.sign(event), 'X-GitHub-Event': 'workflow_job' }, - event, - ); - expect(resp.statusCode).toBe(201); - expect(sendActionRequest).toBeCalled(); - }); - it('Check not allowed runner label is declined', async () => { - process.env.RUNNER_LABELS = '["test"]'; + it('Check webhook does not accept jobs where not all labels are supported by the runner.', async () => { + process.env.RUNNER_LABELS = '["self-hosted", "x64", "linux", "test"]'; + process.env.ENABLE_WORKFLOW_JOB_LABELS_CHECK = 'true'; const event = JSON.stringify({ ...workflowjob_event, workflow_job: { ...workflowjob_event.workflow_job, - labels: ['self-hosted', 'linux', 'not_allowed'], + labels: ['self-hosted', 'linux', 'x64', 'test', 'gpu'], }, }); const resp = await handle( @@ -219,7 +189,7 @@ describe('handler', () => { event, ); expect(resp.statusCode).toBe(202); - expect(sendActionRequest).not.toBeCalled(); + expect(sendActionRequest).not.toBeCalled; }); }); diff --git a/modules/webhook/lambdas/webhook/src/webhook/handler.ts b/modules/webhook/lambdas/webhook/src/webhook/handler.ts index 7e56480e2b..5dfb61d68a 100644 --- a/modules/webhook/lambdas/webhook/src/webhook/handler.ts +++ b/modules/webhook/lambdas/webhook/src/webhook/handler.ts @@ -10,6 +10,8 @@ const supportedEvents = ['check_run', 'workflow_job']; const logger = rootLogger.getChildLogger(); export async function handle(headers: IncomingHttpHeaders, body: string): Promise { + const { environment, repositoryWhiteList, enableWorkflowLabelCheck, runnerLabels } = readEnvironmentVariables(); + // ensure header keys lower case since github headers can contain capitals. for (const key in headers) { headers[key.toLowerCase()] = headers[key]; @@ -18,12 +20,13 @@ export async function handle(headers: IncomingHttpHeaders, body: string): Promis const githubEvent = headers['x-github-event'] as string; let response: Response = { - statusCode: await verifySignature(githubEvent, headers['x-hub-signature'] as string, body), + statusCode: await verifySignature(githubEvent, headers, body, environment), }; if (response.statusCode != 200) { return response; } + const payload = JSON.parse(body); LogFields.fields.event = githubEvent; LogFields.fields.repository = payload.repository.full_name; @@ -48,7 +51,7 @@ export async function handle(headers: IncomingHttpHeaders, body: string): Promis LogFields.fields.completed_at = payload[githubEvent]?.completed_at; LogFields.fields.conclusion = payload[githubEvent]?.conclusion; - if (isRepoNotAllowed(payload.repository.full_name)) { + if (isRepoNotAllowed(payload.repository.full_name, repositoryWhiteList)) { logger.error(`Received event from unauthorized repository ${payload.repository.full_name}`, LogFields.print()); return { statusCode: 403, @@ -58,7 +61,12 @@ export async function handle(headers: IncomingHttpHeaders, body: string): Promis logger.info(`Processing Github event`, LogFields.print()); if (githubEvent == 'workflow_job') { - response = await handleWorkflowJob(payload as WorkflowJobEvent, githubEvent); + response = await handleWorkflowJob( + payload as WorkflowJobEvent, + githubEvent, + enableWorkflowLabelCheck, + runnerLabels, + ); } else if (githubEvent == 'check_run') { response = await handleCheckRun(payload as CheckRunEvent, githubEvent); } @@ -66,7 +74,24 @@ export async function handle(headers: IncomingHttpHeaders, body: string): Promis return response; } -async function verifySignature(githubEvent: string, signature: string, body: string): Promise { +function readEnvironmentVariables() { + const environment = process.env.ENVIRONMENT; + const enableWorkflowLabelCheckEnv = process.env.ENABLE_WORKFLOW_JOB_LABELS_CHECK || 'false'; + const enableWorkflowLabelCheck = JSON.parse(enableWorkflowLabelCheckEnv) as boolean; + const repositoryWhiteListEnv = process.env.REPOSITORY_WHITE_LIST || '[]'; + const repositoryWhiteList = JSON.parse(repositoryWhiteListEnv) as Array; + const runnerLabelsEnv = process.env.RUNNER_LABELS || '[]'; + const runnerLabels = JSON.parse(runnerLabelsEnv) as Array; + return { environment, repositoryWhiteList, enableWorkflowLabelCheck, runnerLabels }; +} + +async function verifySignature( + githubEvent: string, + headers: IncomingHttpHeaders, + body: string, + environment: string, +): Promise { + const signature = headers['x-hub-signature'] as string; if (!signature) { logger.error( "Github event doesn't have signature. This webhook requires a secret to be configured.", @@ -75,7 +100,7 @@ async function verifySignature(githubEvent: string, signature: string, body: str return 500; } - const secret = await getParameterValue(process.env.ENVIRONMENT as string, 'github_app_webhook_secret'); + const secret = await getParameterValue(environment, 'github_app_webhook_secret'); const webhooks = new Webhooks({ secret: secret, @@ -87,10 +112,13 @@ async function verifySignature(githubEvent: string, signature: string, body: str return 200; } -async function handleWorkflowJob(body: WorkflowJobEvent, githubEvent: string): Promise { - const disableCheckWorkflowJobLabelsEnv = process.env.DISABLE_CHECK_WORKFLOW_JOB_LABELS || 'false'; - const disableCheckWorkflowJobLabels = JSON.parse(disableCheckWorkflowJobLabelsEnv) as boolean; - if (!disableCheckWorkflowJobLabels && !canRunJob(body)) { +async function handleWorkflowJob( + body: WorkflowJobEvent, + githubEvent: string, + enableWorkflowLabelCheck: boolean, + runnerLabels: string[], +): Promise { + if (enableWorkflowLabelCheck && !canRunJob(body, runnerLabels)) { logger.warn( `Received event contains runner labels '${body.workflow_job.labels}' that are not accepted.`, LogFields.print(), @@ -138,34 +166,21 @@ function getInstallationId(body: WorkflowJobEvent | CheckRunEvent) { return installationId; } -function isRepoNotAllowed(repo_full_name: string): boolean { - const repositoryWhiteListEnv = process.env.REPOSITORY_WHITE_LIST || '[]'; - const repositoryWhiteList = JSON.parse(repositoryWhiteListEnv) as Array; - - return repositoryWhiteList.length > 0 && !repositoryWhiteList.includes(repo_full_name); +function isRepoNotAllowed(repoFullName: string, repositoryWhiteList: string[]): boolean { + return repositoryWhiteList.length > 0 && !repositoryWhiteList.includes(repoFullName); } -function canRunJob(job: WorkflowJobEvent): boolean { - const runnerLabelsEnv = process.env.RUNNER_LABELS || '[]'; - const runnerLabels = new Set(JSON.parse(runnerLabelsEnv) as Array); - - // ensure the self-hosted label is in the list. - runnerLabels.add('self-hosted'); +function canRunJob(job: WorkflowJobEvent, runnerLabels: string[]): boolean { const workflowJobLabels = job.workflow_job.labels; - - // eslint-disable-next-line max-len - // GitHub managed labels: https://docs.github.com/en/actions/hosting-your-own-runners/using-self-hosted-runners-in-a-workflow#using-default-labels-to-route-jobs - const githubManagedLabels = ['self-hosted', 'linux', 'macOS', 'windows', 'x64', 'ARM', 'ARM64']; - // Remove GitHub managed labels - const customWorkflowJobLabels = workflowJobLabels.filter((l) => githubManagedLabels.indexOf(l) < 0); - - const runnerMatch = customWorkflowJobLabels.every((l) => runnerLabels.has(l)); + const runnerMatch = runnerLabels.every((l) => workflowJobLabels.includes(l)); + const jobMatch = workflowJobLabels.every((l) => runnerLabels.includes(l)); + const match = jobMatch && runnerMatch; logger.debug( - `Received workflow job event with labels: '${JSON.stringify(job.workflow_job.labels)}'. The event does ${ - runnerMatch ? '' : 'NOT ' - }match the configured labels: '${Array.from(runnerLabels).join(',')}'`, + `Received workflow job event with labels: '${JSON.stringify(workflowJobLabels)}'. The event does ${ + match ? '' : 'NOT ' + }match the runner labels: '${Array.from(runnerLabels).join(',')}'`, LogFields.print(), ); - return runnerMatch; + return match; } diff --git a/modules/webhook/variables.tf b/modules/webhook/variables.tf index 1eb17c9d38..aa7b777397 100644 --- a/modules/webhook/variables.tf +++ b/modules/webhook/variables.tf @@ -83,12 +83,18 @@ variable "kms_key_arn" { default = null } -variable "runner_extra_labels" { - description = "Extra labels for the runners (GitHub). Separate each label by a comma" +variable "runner_labels" { + description = "Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided." type = string default = "" } +variable "enable_workflow_job_labels_check" { + description = "If set to true all labels in the workflow job even are matched agaist the custom labels and GitHub labels (os, architecture and `self-hosted`). When the labels are not matching the event is dropped at the webhook." + type = bool + default = false +} + variable "log_type" { description = "Logging format for lambda logging. Valid values are 'json', 'pretty', 'hidden'. " type = string diff --git a/modules/webhook/webhook.tf b/modules/webhook/webhook.tf index 8ea42b2d28..bd2ae54cda 100644 --- a/modules/webhook/webhook.tf +++ b/modules/webhook/webhook.tf @@ -12,14 +12,14 @@ resource "aws_lambda_function" "webhook" { environment { variables = { - DISABLE_CHECK_WORKFLOW_JOB_LABELS = var.disable_check_wokflow_job_labels - ENVIRONMENT = var.environment - LOG_LEVEL = var.log_level - LOG_TYPE = var.log_type - REPOSITORY_WHITE_LIST = jsonencode(var.repository_white_list) - RUNNER_LABELS = jsonencode(split(",", var.runner_extra_labels)) - SQS_URL_WEBHOOK = var.sqs_build_queue.id - SQS_IS_FIFO = var.sqs_build_queue_fifo + ENABLE_WORKFLOW_JOB_LABELS_CHECK = var.enable_workflow_job_labels_check + ENVIRONMENT = var.environment + LOG_LEVEL = var.log_level + LOG_TYPE = var.log_type + REPOSITORY_WHITE_LIST = jsonencode(var.repository_white_list) + RUNNER_LABELS = jsonencode(split(",", var.runner_labels)) + SQS_URL_WEBHOOK = var.sqs_build_queue.id + SQS_IS_FIFO = var.sqs_build_queue_fifo } } diff --git a/variables.tf b/variables.tf index 3ff01c3bd5..7c744f70bb 100644 --- a/variables.tf +++ b/variables.tf @@ -58,7 +58,7 @@ variable "runner_boot_time_in_minutes" { } variable "runner_extra_labels" { - description = "Extra labels for the runners (GitHub). Separate each label by a comma" + description = "Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided." type = string default = "" } @@ -473,8 +473,8 @@ variable "log_level" { } } -variable "disable_check_wokflow_job_labels" { - description = "Disable the the check of workflow labels for received workflow job events." +variable "runner_enable_workflow_job_labels_check" { + description = "If set to true all labels in the workflow job even are matched agaist the custom labels and GitHub labels (os, architecture and `self-hosted`). When the labels are not matching the event is dropped at the webhook." type = bool default = false } @@ -507,7 +507,7 @@ variable "runner_os" { default = "linux" validation { - condition = contains(["linux", "win"], var.runner_os) + condition = contains(["linux", "windows"], var.runner_os) error_message = "Valid values for runner_os are (linux, win)." } }