diff --git a/README.md b/README.md index cf8546637f..91122fcbe8 100644 --- a/README.md +++ b/README.md @@ -119,9 +119,10 @@ Join our discord community via [this invite link](https://discord.gg/bxgXW8jJGh) | [create\_service\_linked\_role\_spot](#input\_create\_service\_linked\_role\_spot) | (optional) create the service 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\_runner\_autoupdate](#input\_disable\_runner\_autoupdate) | Disable the auto update of the github runner agent. Be aware there is a grace period of 30 days, see also the [GitHub article](https://github.blog/changelog/2022-02-01-github-actions-self-hosted-runners-can-now-disable-automatic-updates/) | `bool` | `false` | no | +| [ec2\_dynamic\_labels\_policy](#input\_ec2\_dynamic\_labels\_policy) | Experimental! Can be removed / changed without trigger a major release.
Optional policy for dynamic EC2 override labels evaluated by the webhook
dispatcher. Only effective when `enable_dynamic_labels = true`.

Jobs whose EC2 dynamic labels violate the policy are rejected with a 202 and a
warning is logged.

Evaluation:
1. Keys in `blocked_keys` are always rejected.
2. Keys in `restricted_keys` are allowed only when their value passes the rule.
3. Keys not listed in `blocked_keys` or `restricted_keys` are allowed.

Schema:
- `blocked_keys`: keys to reject outright.
- `restricted_keys`: map of key to value rule:
`{ allowed = [globs], denied = [globs], max = number|string }`.

Keys use the `ghr-ec2-*` dynamic label suffix, not the full label. For example, use
`instance-type` for `ghr-ec2-instance-type`. | `any` | `null` | no | | [enable\_ami\_housekeeper](#input\_enable\_ami\_housekeeper) | Option to disable the lambda to clean up old AMIs. | `bool` | `false` | no | | [enable\_cloudwatch\_agent](#input\_enable\_cloudwatch\_agent) | Enables the cloudwatch agent on the ec2 runner instances. The runner uses a default config that can be overridden via `cloudwatch_config`. | `bool` | `true` | no | -| [enable\_dynamic\_labels](#input\_enable\_dynamic\_labels) | Experimental! Can be removed / changed without trigger a major release. Enable dynamic EC2 configs based on workflow job labels. When enabled, jobs can request specific configs via the 'gh-ec2-:' label (e.g., 'gh-ec2-instance-type:t3.large'). When enabled, labels starting with `ghr-` are ignored during webhook label matching. | `bool` | `false` | no | +| [enable\_dynamic\_labels](#input\_enable\_dynamic\_labels) | Experimental! Can be removed / changed without trigger a major release. Enable dynamic EC2 configs based on workflow job labels. When enabled, jobs can request specific configs via the 'ghr-ec2-:' label (e.g., 'ghr-ec2-instance-type:t3.large'). When enabled, labels starting with `ghr-` are ignored during webhook label matching. | `bool` | `false` | no | | [enable\_ephemeral\_runners](#input\_enable\_ephemeral\_runners) | Enable ephemeral runners, runners will only be used once. | `bool` | `false` | no | | [enable\_jit\_config](#input\_enable\_jit\_config) | Overwrite the default behavior for JIT configuration. By default JIT configuration is enabled for ephemeral runners and disabled for non-ephemeral runners. In case of GHES check first if the JIT config API is available. In case you are upgrading from 3.x to 4.x you can set `enable_jit_config` to `false` to avoid a breaking change when having your own AMI. | `bool` | `null` | no | | [enable\_job\_queued\_check](#input\_enable\_job\_queued\_check) | Only scale if the job event received by the scale up lambda is in the queued state. By default enabled for non ephemeral runners and disabled for ephemeral. Set this variable to overwrite the default behavior. | `bool` | `null` | no | diff --git a/docs/configuration.md b/docs/configuration.md index 6c8f33aaf9..153617e0a1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -336,8 +336,7 @@ Below is an example of the log messages created. [!WARNING] **Security implication:** Dynamic labels are extracted from the `runs-on` labels in incoming `workflow_job` webhook events. These labels originate from what -users define in their workflow files. Any user with permission to create or modify workflows can inject arbitrary EC2 configuration values — including instance types, AMI IDs, subnet IDs, EBS volumes, placement settings, and more. **These values are not sanitized or validated** against an allowlist before being passed to the EC2 CreateFleet API. This means a malicious or careless workflow author could, for example: -- +users define in their workflow files. Any user with permission to create or modify workflows can inject arbitrary EC2 configuration values — including instance types, AMI IDs, subnet IDs, EBS volumes, placement settings, and more. Unless constrained with `ec2_dynamic_labels_policy`, these values are not validated against label-specific rules before being passed to the EC2 CreateFleet API. This means a malicious or careless workflow author could, for example: - Launch expensive instance types (e.g., `p5.48xlarge`) to inflate costs - Override the AMI (`ghr-ec2-image-id`) to boot a compromised image @@ -368,10 +367,32 @@ module "runners" { ... enable_dynamic_labels = true + ec2_dynamic_labels_policy = { + blocked_keys = ["image-id", "subnet-id"] + + restricted_keys = { + "instance-type" = { + allowed = ["m5.*", "c5.*"] + denied = ["m5.metal"] + } + + "ebs-volume-size" = { + max = 200 + } + } + } ... } ``` +The policy is evaluated by dynamic label key: + +1. Keys in `blocked_keys` are always rejected. +2. Keys in `restricted_keys` are allowed only when their value passes the rule. +3. Keys not listed in `blocked_keys` or `restricted_keys` are allowed. + +Policy keys use the dynamic label suffix, not the full label. For example, use `instance-type` for `ghr-ec2-instance-type`. + #### Custom identity labels Any label matching `ghr-:` (where `` does **not** start with `ec2-`) is a custom identity label. These labels have no effect on EC2 instance configuration but are included in the runner matching hash. Use them to: diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts index 1770dfa09a..4a9835de30 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts @@ -595,7 +595,6 @@ describe('scaleUp with GHES', () => { describe('Dynamic EC2 Configuration', () => { beforeEach(() => { process.env.ENABLE_ORGANIZATION_RUNNERS = 'true'; - process.env.ENABLE_DYNAMIC_LABELS = 'true'; process.env.ENABLE_EPHEMERAL_RUNNERS = 'true'; process.env.ENABLE_JOB_QUEUED_CHECK = 'false'; process.env.RUNNER_LABELS = 'base-label'; @@ -690,29 +689,6 @@ describe('scaleUp with GHES', () => { ); }); - it('does not process EC2 labels when ENABLE_DYNAMIC_LABELS is disabled', async () => { - process.env.ENABLE_DYNAMIC_LABELS = 'false'; - - const testDataWithEc2Labels = [ - { - ...TEST_DATA_SINGLE, - labels: ['ghr-ec2-instance-type:c5.4xlarge'], - messageId: 'test-7', - }, - ]; - - await scaleUpModule.scaleUp(testDataWithEc2Labels); - - // Should ignore EC2 labels and use default instance types - expect(createRunner).toBeCalledWith( - expect.objectContaining({ - ec2instanceCriteria: expect.objectContaining({ - instanceTypes: ['t3.medium', 't3.large'], - }), - }), - ); - }); - it('handles multiple EC2 labels correctly', async () => { const testDataWithMultipleEc2Labels = [ { diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts index b742264842..510fc60186 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts @@ -336,7 +336,6 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise l.startsWith('ghr-'))?.slice('ghr-'.length); - - if (dynamicLabels) { - const dynamicLabelsHash = labelsHash(labels); - key = `${key}/${dynamicLabelsHash}`; - } + if (labels?.some((l) => l.startsWith('ghr-'))) { + const dynamicLabelsHash = labelsHash(labels); + key = `${key}/${dynamicLabelsHash}`; } let entry = validMessages.get(key); @@ -457,27 +452,23 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise 0 && dynamicLabelsEnabled) { - logger.debug('Dynamic EC2 config enabled, processing labels', { labels: messages[0].labels }); - - const dynamicEC2Labels = messages[0].labels?.map((l) => l.trim()).filter((l) => l.startsWith('ghr-ec2-')) ?? []; - const allDynamicLabels = messages[0].labels?.map((l) => l.trim()).filter((l) => l.startsWith('ghr-')) ?? []; - - if (allDynamicLabels.length > 0) { - runnerLabels = runnerLabels ? `${runnerLabels},${allDynamicLabels.join(',')}` : allDynamicLabels.join(','); - - logger.debug('Updated runner labels', { runnerLabels }); - - if (dynamicEC2Labels.length > 0) { - ec2OverrideConfig = parseEc2OverrideConfig(dynamicEC2Labels); - if (ec2OverrideConfig) { - logger.debug('EC2 override config parsed from labels', { - ec2OverrideConfig, - }); - } + const messageLabels = messages.length > 0 ? (messages[0].labels ?? []) : []; + const dynamicEC2Labels = messageLabels.map((l) => l.trim()).filter((l) => l.startsWith('ghr-ec2-')); + const nonEc2DynamicLabels = messageLabels + .map((l) => l.trim()) + .filter((l) => l.startsWith('ghr-') && !l.startsWith('ghr-ec2-')); + const allDynamicLabels = [...nonEc2DynamicLabels, ...dynamicEC2Labels]; + + if (allDynamicLabels.length > 0) { + logger.debug('Dynamic labels present on message', { labels: allDynamicLabels }); + runnerLabels = runnerLabels ? `${runnerLabels},${allDynamicLabels.join(',')}` : allDynamicLabels.join(','); + logger.debug('Updated runner labels', { runnerLabels }); + + if (dynamicEC2Labels.length > 0) { + ec2OverrideConfig = parseEc2OverrideConfig(dynamicEC2Labels); + if (ec2OverrideConfig) { + logger.debug('EC2 override config parsed from labels', { ec2OverrideConfig }); } - } else { - logger.debug('No dynamic labels found on message'); } } @@ -822,8 +813,8 @@ async function createJitConfig( * - ghr-ec2-accelerator-count-max: - Set maximum accelerator count * - ghr-ec2-accelerator-manufacturers: - Accelerator manufacturers (comma-separated: nvidia,amd,amazon-web-services,xilinx) * - ghr-ec2-accelerator-names: - Specific accelerator names (comma-separated) - * - ghr-ec2-accelerator-memory-mib-min: - Min accelerator total memory in MiB - * - ghr-ec2-accelerator-memory-mib-max: - Max accelerator total memory in MiB + * - ghr-ec2-accelerator-total-memory-mib-min: - Min accelerator total memory in MiB + * - ghr-ec2-accelerator-total-memory-mib-max: - Max accelerator total memory in MiB * * Instance Requirements (Network & Storage): * - ghr-ec2-network-interface-count-min: - Min network interfaces diff --git a/lambdas/functions/webhook/src/ConfigLoader.ts b/lambdas/functions/webhook/src/ConfigLoader.ts index df7b159495..e77a92b16e 100644 --- a/lambdas/functions/webhook/src/ConfigLoader.ts +++ b/lambdas/functions/webhook/src/ConfigLoader.ts @@ -130,11 +130,9 @@ export class ConfigWebhook extends MatcherAwareConfig { repositoryAllowList: string[] = []; webhookSecret: string = ''; workflowJobEventSecondaryQueue: string = ''; - enableDynamicLabels: boolean = false; async loadConfig(): Promise { this.loadEnvVar(process.env.REPOSITORY_ALLOW_LIST, 'repositoryAllowList', []); - this.loadEnvVar(process.env.ENABLE_DYNAMIC_LABELS, 'enableDynamicLabels', false); await Promise.all([ this.loadMatcherConfig(process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH), @@ -164,11 +162,9 @@ export class ConfigWebhookEventBridge extends BaseConfig { export class ConfigDispatcher extends MatcherAwareConfig { repositoryAllowList: string[] = []; workflowJobEventSecondaryQueue: string = ''; // Deprecated - enableDynamicLabels: boolean = false; async loadConfig(): Promise { this.loadEnvVar(process.env.REPOSITORY_ALLOW_LIST, 'repositoryAllowList', []); - this.loadEnvVar(process.env.ENABLE_DYNAMIC_LABELS, 'enableDynamicLabels', false); await this.loadMatcherConfig(process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH); validateRunnerMatcherConfig(this); diff --git a/lambdas/functions/webhook/src/modules.d.ts b/lambdas/functions/webhook/src/modules.d.ts index 9d73cf5815..76a72660c0 100644 --- a/lambdas/functions/webhook/src/modules.d.ts +++ b/lambdas/functions/webhook/src/modules.d.ts @@ -5,7 +5,6 @@ declare namespace NodeJS { PARAMETER_GITHUB_APP_WEBHOOK_SECRET: string; PARAMETER_RUNNER_MATCHER_CONFIG_PATH: string; REPOSITORY_ALLOW_LIST: string; - ENABLE_DYNAMIC_LABELS: string; RUNNER_LABELS: string; ACCEPT_EVENTS: string; } diff --git a/lambdas/functions/webhook/src/runners/dispatch.test.ts b/lambdas/functions/webhook/src/runners/dispatch.test.ts index 203e0979b7..182c8b0024 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.test.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.test.ts @@ -183,110 +183,223 @@ describe('Dispatcher', () => { it('should accept job with an exact match and identical labels.', () => { const workflowLabels = ['self-hosted', 'linux', 'x64', 'ubuntu-latest']; const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']]; - expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(true); + expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true); }); it('should accept job with an exact match and identical labels, ignoring cases.', () => { const workflowLabels = ['self-Hosted', 'Linux', 'X64', 'ubuntu-Latest']; const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']]; - expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(true); + expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true); }); it('should accept job with an exact match and runner supports requested capabilities.', () => { const workflowLabels = ['self-hosted', 'linux', 'x64']; const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']]; - expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(true); + expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true); }); it('should NOT accept job with an exact match and runner not matching requested capabilities.', () => { const workflowLabels = ['self-hosted', 'linux', 'x64', 'ubuntu-latest']; const runnerLabels = [['self-hosted', 'linux', 'x64']]; - expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(false); + expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(false); }); it('should accept job with for a non exact match. Any label that matches will accept the job.', () => { const workflowLabels = ['self-hosted', 'linux', 'x64', 'ubuntu-latest', 'gpu']; const runnerLabels = [['gpu']]; - expect(canRunJob(workflowLabels, runnerLabels, false, false)).toBe(true); + expect(canRunJob(workflowLabels, runnerLabels, false)).toBe(true); }); it('should NOT accept job with for an exact match. Not all requested capabilities are supported.', () => { const workflowLabels = ['self-hosted', 'linux', 'x64', 'ubuntu-latest', 'gpu']; const runnerLabels = [['gpu']]; - expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(false); - }); - - it('should filter out ghr- labels when enableDynamicLabels is true.', () => { - const workflowLabels = ['self-hosted', 'linux', 'x64', 'ghr-ec2-instance-type:t3.large', 'ghr-run-id:12345']; - const runnerLabels = [['self-hosted', 'linux', 'x64']]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); - }); - - it('should NOT filter out ghr- labels when enableDynamicLabels is false.', () => { - const workflowLabels = ['self-hosted', 'linux', 'x64', 'ghr-ec2-instance-type:t3.large']; - const runnerLabels = [['self-hosted', 'linux', 'x64']]; - expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(false); + expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(false); }); }); - describe('sanitizeGhrLabels (via canRunJob with enableDynamicLabels=true)', () => { - it('should keep valid ghr- labels with allowed characters (alphanumeric, dot, dash, colon, slash)', () => { - const workflowLabels = ['self-hosted', 'ghr-ec2-instance-type:t3.large']; - const runnerLabels = [['self-hosted', 'ghr-ec2-instance-type:t3.large']]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); - }); + describe('per-matcher dynamic labels handling', () => { + const baseRunner = runnerConfig[0]; - it('should keep ghr- labels with path-like values containing slashes', () => { - const workflowLabels = ['self-hosted', 'ghr-image:my/custom/image']; - const runnerLabels = [['self-hosted', 'ghr-image:my/custom/image']]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); - }); - - it('should strip ghr- labels that exceed 128 characters', () => { - const longLabel = 'ghr-' + 'a'.repeat(125); // 129 chars total, exceeds 128 - const workflowLabels = ['self-hosted', 'linux', longLabel]; - const runnerLabels = [['self-hosted', 'linux']]; - // Long label is stripped, remaining labels match - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); + it('strips invalid ghr- labels (too long, bad chars) before policy and dispatch', async () => { + const longLabel = 'ghr-' + 'a'.repeat(125); // 129 chars + config = await createConfig(undefined, [ + { + ...baseRunner, + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + }, + }, + ]); + const event = { + ...workFlowJobEvent, + workflow_job: { + ...workFlowJobEvent.workflow_job, + labels: ['self-hosted', 'linux', 'ghr-valid:value', 'ghr-bad label', longLabel], + }, + } as unknown as WorkflowJobEvent; + const resp = await dispatch(event, 'workflow_job', config); + expect(resp.statusCode).toBe(201); + expect(sendActionRequest).toHaveBeenCalledWith( + expect.objectContaining({ labels: ['self-hosted', 'linux', 'ghr-valid:value'] }), + ); }); - it('should keep ghr- labels that are exactly 128 characters', () => { - const exactLabel = 'ghr-' + 'a'.repeat(124); // exactly 128 chars - const workflowLabels = ['self-hosted', exactLabel]; - const runnerLabels = [['self-hosted', exactLabel]]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); + it('rejects the job (202) when the only matching runner has enableDynamicLabels=false', async () => { + config = await createConfig(undefined, [ + { + ...baseRunner, + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: false, + }, + }, + ]); + const event = { + ...workFlowJobEvent, + workflow_job: { + ...workFlowJobEvent.workflow_job, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }, + } as unknown as WorkflowJobEvent; + const resp = await dispatch(event, 'workflow_job', config); + expect(resp.statusCode).toBe(202); + expect(sendActionRequest).not.toHaveBeenCalled(); }); - it('should strip ghr- labels with invalid characters (spaces)', () => { - const workflowLabels = ['self-hosted', 'linux', 'ghr-bad label']; - const runnerLabels = [['self-hosted', 'linux']]; - // Invalid label is stripped, remaining labels match - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); + it('keeps dynamic labels when the matched runner enables them and has no policy', async () => { + config = await createConfig(undefined, [ + { + ...baseRunner, + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + }, + }, + ]); + const event = { + ...workFlowJobEvent, + workflow_job: { + ...workFlowJobEvent.workflow_job, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }, + } as unknown as WorkflowJobEvent; + const resp = await dispatch(event, 'workflow_job', config); + expect(resp.statusCode).toBe(201); + expect(sendActionRequest).toHaveBeenCalledWith( + expect.objectContaining({ labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'] }), + ); }); - it('should strip ghr- labels with invalid characters (special chars)', () => { - const workflowLabels = ['self-hosted', 'linux', 'ghr-inject;rm -rf']; - const runnerLabels = [['self-hosted', 'linux']]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); + it('skips a matching runner whose policy rejects the dynamic labels and uses the next compliant one', async () => { + config = await createConfig(undefined, [ + { + ...baseRunner, + id: 'strict', + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + ec2DynamicLabelsPolicy: { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }, + }, + }, + { + ...baseRunner, + id: 'permissive', + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + }, + }, + ]); + const event = { + ...workFlowJobEvent, + workflow_job: { + ...workFlowJobEvent.workflow_job, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }, + } as unknown as WorkflowJobEvent; + const resp = await dispatch(event, 'workflow_job', config); + expect(resp.statusCode).toBe(201); + expect(sendActionRequest).toHaveBeenCalledWith( + expect.objectContaining({ + queueId: 'permissive', + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }), + ); }); - it('should never strip non-ghr labels regardless of content', () => { - const workflowLabels = ['self-hosted', 'linux', 'x64']; - const runnerLabels = [['self-hosted', 'linux', 'x64']]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); + it('rejects the job (202) when no runner accepts the policy', async () => { + config = await createConfig(undefined, [ + { + ...baseRunner, + id: 'first', + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + ec2DynamicLabelsPolicy: { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }, + }, + }, + { + ...baseRunner, + id: 'second', + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: false, + }, + }, + ]); + const event = { + ...workFlowJobEvent, + workflow_job: { + ...workFlowJobEvent.workflow_job, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }, + } as unknown as WorkflowJobEvent; + const resp = await dispatch(event, 'workflow_job', config); + expect(resp.statusCode).toBe(202); + expect(sendActionRequest).not.toHaveBeenCalled(); }); - it('should handle a mix of valid ghr-, invalid ghr-, and regular labels', () => { - const longLabel = 'ghr-' + 'x'.repeat(125); // 129 chars, will be stripped - const workflowLabels = [ - 'self-hosted', - 'linux', - 'ghr-valid:value', // valid, kept - 'ghr-bad label', // invalid chars, stripped - longLabel, // too long, stripped - ]; - const runnerLabels = [['self-hosted', 'linux', 'ghr-valid:value']]; - expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true); + it('forwards non-dynamic jobs as-is to the first match', async () => { + config = await createConfig(undefined, [ + { + ...baseRunner, + id: 'first', + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + ec2DynamicLabelsPolicy: {}, + }, + }, + ]); + const event = { + ...workFlowJobEvent, + workflow_job: { + ...workFlowJobEvent.workflow_job, + labels: ['self-hosted', 'linux'], + }, + } as unknown as WorkflowJobEvent; + const resp = await dispatch(event, 'workflow_job', config); + expect(resp.statusCode).toBe(201); + expect(sendActionRequest).toHaveBeenCalledWith( + expect.objectContaining({ queueId: 'first', labels: ['self-hosted', 'linux'] }), + ); }); }); }); diff --git a/lambdas/functions/webhook/src/runners/dispatch.ts b/lambdas/functions/webhook/src/runners/dispatch.ts index e7f4dcb623..6c54dea70b 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.ts @@ -5,9 +5,13 @@ import { Response } from '../lambda'; import { RunnerMatcherConfig, sendActionRequest } from '../sqs'; import ValidationError from '../ValidationError'; import { ConfigDispatcher, ConfigWebhook } from '../ConfigLoader'; +import { violationsAgainstPolicy } from './dynamic-labels-policy'; const logger = createChildLogger('handler'); +const GHR_LABEL_MAX_LENGTH = 128; +const GHR_LABEL_VALUE_PATTERN = /^[a-zA-Z0-9._/\-:]+$/; + export async function dispatch( event: WorkflowJobEvent, eventType: string, @@ -15,7 +19,7 @@ export async function dispatch( ): Promise { validateRepoInAllowList(event, config); - return await handleWorkflowJob(event, eventType, config.matcherConfig!, config.enableDynamicLabels); + return await handleWorkflowJob(event, eventType, config.matcherConfig!); } function validateRepoInAllowList(event: WorkflowJobEvent, config: ConfigDispatcher) { @@ -29,7 +33,6 @@ async function handleWorkflowJob( body: WorkflowJobEvent, githubEvent: string, matcherConfig: Array, - enableDynamicLabels: boolean, ): Promise { if (body.action !== 'queued') { return { @@ -43,39 +46,88 @@ async function handleWorkflowJob( `Job ID: ${body.workflow_job.id}, Job Name: ${body.workflow_job.name}, ` + `Run ID: ${body.workflow_job.run_id}, Labels: ${JSON.stringify(body.workflow_job.labels)}`, ); - // sort the queuesConfig by order of matcher config exact match, with all true matches lined up ahead. + + // Sort queues by priority (exactMatch first), as before. matcherConfig.sort((a, b) => { return a.matcherConfig.exactMatch === b.matcherConfig.exactMatch ? 0 : a.matcherConfig.exactMatch ? -1 : 1; }); - for (const queue of matcherConfig) { - if ( - canRunJob( - body.workflow_job.labels, - queue.matcherConfig.labelMatchers, - queue.matcherConfig.exactMatch, - enableDynamicLabels, - ) - ) { - await sendActionRequest({ - id: body.workflow_job.id, - repositoryName: body.repository.name, - repositoryOwner: body.repository.owner.login, - eventType: githubEvent, - installationId: body.installation?.id ?? 0, - queueId: queue.id, - repoOwnerType: body.repository.owner.type, - labels: body.workflow_job.labels, + + const allLabels = body.workflow_job.labels; + const ghrLabels = allLabels.filter((l) => l.startsWith('ghr-')); + const sanitizedGhrLabels = sanitizeGhrLabels(ghrLabels); + const nonGhrLabels = allLabels.filter((l) => !l.startsWith('ghr-')); + const hasDynamicLabels = sanitizedGhrLabels.length > 0; + + // 1. Collect all queues whose non-dynamic labels match the job. + const matches: RunnerMatcherConfig[] = matcherConfig.filter((q) => + canRunJob(nonGhrLabels, q.matcherConfig.labelMatchers, q.matcherConfig.exactMatch), + ); + + if (matches.length === 0) { + return notAccepted(body); + } + + // 2. Pick a queue. + let chosen: RunnerMatcherConfig; + let labelsToSend: string[]; + + if (!hasDynamicLabels) { + // No dynamic labels in the job: take the first match, forward as-is. + chosen = matches[0]; + labelsToSend = nonGhrLabels; + } else { + // Dynamic labels present: prefer the first match that has dynamic labels + // enabled AND accepts these labels under its policy. + let compliant: RunnerMatcherConfig | undefined; + for (const q of matches) { + if (!q.matcherConfig.enableDynamicLabels) { + logger.warn(`Queue ${q.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`); + continue; + } + const violations = violationsAgainstPolicy(sanitizedGhrLabels, q.matcherConfig.ec2DynamicLabelsPolicy); + if (violations.length === 0) { + compliant = q; + break; + } + for (const v of violations) { + logger.warn(`Queue ${q.id}: dynamic label '${v.label}' does not match policy (${v.reason}); trying next match`); + } + } + + if (compliant) { + chosen = compliant; + labelsToSend = [...nonGhrLabels, ...sanitizedGhrLabels]; + } else { + // No queue accepts the dynamic labels under its policy: refuse the job. + logger.warn(`No queue accepts the dynamic labels for this job; not dispatching`, { + dynamicLabels: sanitizedGhrLabels, }); - logger.info( - `Successfully dispatched job for ${body.repository.full_name} to the queue ${queue.id} - ` + - `Job ID: ${body.workflow_job.id}, Job Name: ${body.workflow_job.name}, Run ID: ${body.workflow_job.run_id}`, - ); - return { - statusCode: 201, - body: `Successfully queued job for ${body.repository.full_name} to the queue ${queue.id}`, - }; + return notAccepted(body); } } + + await sendActionRequest({ + id: body.workflow_job.id, + repositoryName: body.repository.name, + repositoryOwner: body.repository.owner.login, + eventType: githubEvent, + installationId: body.installation?.id ?? 0, + queueId: chosen.id, + repoOwnerType: body.repository.owner.type, + labels: labelsToSend, + }); + + logger.info( + `Successfully dispatched job for ${body.repository.full_name} to the queue ${chosen.id} - ` + + `Job ID: ${body.workflow_job.id}, Job Name: ${body.workflow_job.name}, Run ID: ${body.workflow_job.run_id}`, + ); + return { + statusCode: 201, + body: `Successfully queued job for ${body.repository.full_name} to the queue ${chosen.id}`, + }; +} + +function notAccepted(body: WorkflowJobEvent): Response { const notAcceptedErrorMsg = `Received event contains runner labels '${body.workflow_job.labels}' from '${ body.repository.full_name }' that are not accepted.`; @@ -86,47 +138,38 @@ async function handleWorkflowJob( } function sanitizeGhrLabels(labels: string[]): string[] { - const GHR_LABEL_MAX_LENGTH = 128; - const GHR_LABEL_VALUE_PATTERN = /^[a-zA-Z0-9._/\-:]+$/; - - return labels - .map((label) => { - if (!label.startsWith('ghr-')) return label; - - if (label.length > GHR_LABEL_MAX_LENGTH) { - logger.warn('Dynamic label exceeds max length, stripping', { label: label.substring(0, 40) }); - return null; - } - if (!GHR_LABEL_VALUE_PATTERN.test(label)) { - logger.warn('Dynamic label contains invalid characters, stripping', { label }); - return null; - } - return null; - }) - .filter((l): l is string => l !== null); + return labels.filter((label) => { + if (label.length > GHR_LABEL_MAX_LENGTH) { + logger.warn('Dynamic label exceeds max length, stripping', { label: label.substring(0, 40) }); + return false; + } + if (!GHR_LABEL_VALUE_PATTERN.test(label)) { + logger.warn('Dynamic label contains invalid characters, stripping', { label }); + return false; + } + return true; + }); } +/** + * Pure label match against a runner's `labelMatchers`. Caller is expected to + * pass only non-dynamic labels. + */ export function canRunJob( workflowJobLabels: string[], runnerLabelsMatchers: string[][], workflowLabelCheckAll: boolean, - enableDynamicLabels: boolean, ): boolean { - // Filter out ghr- labels only and sanitize them if dynamic labels is enabled, otherwise keep all labels as is for matching. - const sanitizedLabels = enableDynamicLabels ? sanitizeGhrLabels(workflowJobLabels) : workflowJobLabels; - - runnerLabelsMatchers = runnerLabelsMatchers.map((runnerLabel) => { - return runnerLabel.map((label) => label.toLowerCase()); - }); + const lowered = runnerLabelsMatchers.map((rl) => rl.map((l) => l.toLowerCase())); const matchLabels = workflowLabelCheckAll - ? runnerLabelsMatchers.some((rl) => sanitizedLabels.every((wl) => rl.includes(wl.toLowerCase()))) - : runnerLabelsMatchers.some((rl) => sanitizedLabels.some((wl) => rl.includes(wl.toLowerCase()))); - const match = sanitizedLabels.length === 0 ? !matchLabels : matchLabels; + ? lowered.some((rl) => workflowJobLabels.every((wl) => rl.includes(wl.toLowerCase()))) + : lowered.some((rl) => workflowJobLabels.some((wl) => rl.includes(wl.toLowerCase()))); + const match = workflowJobLabels.length === 0 ? !matchLabels : matchLabels; logger.debug( `Received workflow job event with labels: '${JSON.stringify(workflowJobLabels)}'. The event does ${ match ? '' : 'NOT ' - }match the runner labels: '${Array.from(runnerLabelsMatchers).join(',')}'`, + }match the runner labels: '${Array.from(lowered).join(',')}'`, ); return match; } diff --git a/lambdas/functions/webhook/src/runners/dynamic-labels-policy.test.ts b/lambdas/functions/webhook/src/runners/dynamic-labels-policy.test.ts new file mode 100644 index 0000000000..d9d3c9325d --- /dev/null +++ b/lambdas/functions/webhook/src/runners/dynamic-labels-policy.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect } from 'vitest'; + +import { violationsAgainstPolicy, type Ec2DynamicLabelsPolicy } from './dynamic-labels-policy'; + +describe('violationsAgainstPolicy', () => { + it('returns [] when policy is null/undefined', () => { + expect(violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large'], null)).toEqual([]); + expect(violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large'], undefined)).toEqual([]); + }); + + it('accepts any key when no policy entries match', () => { + const policy: Ec2DynamicLabelsPolicy = {}; + expect(violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large', 'ghr-ec2-image-id:ami-1'], policy)).toEqual([]); + }); + + it('accepts keys not listed in blocked_keys or restricted_keys', () => { + const policy: Ec2DynamicLabelsPolicy = { + blocked_keys: ['image-id'], + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + expect(violationsAgainstPolicy(['ghr-ec2-ebs-volume-size:300'], policy)).toEqual([]); + }); + + it('flags keys in blocked_keys', () => { + const policy: Ec2DynamicLabelsPolicy = { blocked_keys: ['image-id'] }; + const v = violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large', 'ghr-ec2-image-id:ami-1'], policy); + expect(v).toHaveLength(1); + expect(v[0].label).toBe('ghr-ec2-image-id:ami-1'); + }); + + it('blocked_keys takes precedence over restricted_keys', () => { + const policy: Ec2DynamicLabelsPolicy = { + blocked_keys: ['image-id'], + restricted_keys: { + 'image-id': { allowed: ['ami-*'] }, + }, + }; + const v = violationsAgainstPolicy(['ghr-ec2-image-id:ami-1'], policy); + expect(v).toHaveLength(1); + expect(v[0].label).toBe('ghr-ec2-image-id:ami-1'); + }); + + it('restricted key allowed glob with `*`', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'instance-type': { allowed: ['m5.*', 'c5.*'] } } }; + const v = violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large', 'ghr-ec2-instance-type:r5.large'], policy); + expect(v).toHaveLength(1); + expect(v[0].label).toBe('ghr-ec2-instance-type:r5.large'); + }); + + it('restricted key allowed glob with `?`', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'image-id': { allowed: ['ami-?????????'] } } }; + const v = violationsAgainstPolicy(['ghr-ec2-image-id:ami-123456789', 'ghr-ec2-image-id:ami-12345'], policy); + expect(v).toHaveLength(1); + expect(v[0].label).toBe('ghr-ec2-image-id:ami-12345'); + }); + + it('escapes regex metacharacters in patterns', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'instance-type': { allowed: ['m5.large'] } } }; + const v = violationsAgainstPolicy(['ghr-ec2-instance-type:m5xlarge'], policy); + expect(v).toHaveLength(1); + }); + + it('empty allowed list is treated as no constraint', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'instance-type': { allowed: [] } } }; + expect(violationsAgainstPolicy(['ghr-ec2-instance-type:any'], policy)).toEqual([]); + }); + + it('denied glob flags matches', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'instance-type': { denied: ['*.metal*'] } } }; + const v = violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large', 'ghr-ec2-instance-type:m5.metal'], policy); + expect(v).toHaveLength(1); + expect(v[0].label).toBe('ghr-ec2-instance-type:m5.metal'); + }); + + it('max flags values that exceed', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'ebs-volume-size': { max: 200 } } }; + const v = violationsAgainstPolicy(['ghr-ec2-ebs-volume-size:100', 'ghr-ec2-ebs-volume-size:300'], policy); + expect(v).toHaveLength(1); + expect(v[0].label).toBe('ghr-ec2-ebs-volume-size:300'); + }); + + it('max flags when value is not numeric', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'instance-type': { max: 100 } } }; + const v = violationsAgainstPolicy(['ghr-ec2-instance-type:m5.large'], policy); + expect(v).toHaveLength(1); + }); + + it('empty rule object accepts any value', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'instance-type': {} } }; + expect(violationsAgainstPolicy(['ghr-ec2-instance-type:any'], policy)).toEqual([]); + }); + + it('accepts a value-less label whose key is not blocked', () => { + const policy: Ec2DynamicLabelsPolicy = { restricted_keys: { 'no-device': {} } }; + expect(violationsAgainstPolicy(['ghr-ec2-no-device'], policy)).toEqual([]); + }); + + it('flags a value-less label when blocked_keys includes it', () => { + const policy: Ec2DynamicLabelsPolicy = { blocked_keys: ['no-device'] }; + const v = violationsAgainstPolicy(['ghr-ec2-no-device'], policy); + expect(v).toHaveLength(1); + }); + + it('returns a reason per violating label', () => { + const policy: Ec2DynamicLabelsPolicy = { + blocked_keys: ['image-id'], + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + const v = violationsAgainstPolicy( + ['ghr-ec2-instance-type:r5.large', 'ghr-ec2-image-id:ami-x', 'ghr-ec2-instance-type:m5.large'], + policy, + ); + expect(v).toHaveLength(2); + expect(v[0].label).toBe('ghr-ec2-instance-type:r5.large'); + expect(v[1].label).toBe('ghr-ec2-image-id:ami-x'); + }); +}); diff --git a/lambdas/functions/webhook/src/runners/dynamic-labels-policy.ts b/lambdas/functions/webhook/src/runners/dynamic-labels-policy.ts new file mode 100644 index 0000000000..54003690b7 --- /dev/null +++ b/lambdas/functions/webhook/src/runners/dynamic-labels-policy.ts @@ -0,0 +1,78 @@ +export interface Ec2DynamicLabelsValueRule { + allowed?: string[]; + denied?: string[]; + max?: number | string; +} + +/** + * EC2 dynamic labels policy schema. `blocked_keys` rejects keys outright; + * `restricted_keys` applies optional per-key value rules. Keys use the + * `` segment of a `ghr-ec2-:` label in the same hyphenated + * form as the labels themselves (e.g. `instance-type`). + */ +export interface Ec2DynamicLabelsPolicy { + blocked_keys?: string[]; + restricted_keys?: Record; +} + +function globToRegExp(glob: string): RegExp { + const escaped = glob.replace(/[.+^${}()|[\]\\]/g, '\\$&'); + const pattern = escaped.replace(/\*/g, '.*').replace(/\?/g, '.'); + return new RegExp(`^${pattern}$`); +} + +function matchesAny(value: string, patterns: string[] | undefined): boolean { + if (!patterns || patterns.length === 0) return false; + return patterns.some((p) => globToRegExp(p).test(value)); +} + +function evaluateLabel(label: string, policy: Ec2DynamicLabelsPolicy): string | null { + const stripped = label.replace(/^ghr-ec2-/, ''); + const colonIdx = stripped.indexOf(':'); + const key = colonIdx === -1 ? stripped : stripped.slice(0, colonIdx); + const value = colonIdx === -1 ? undefined : stripped.slice(colonIdx + 1); + + if (policy.blocked_keys?.includes(key)) { + return `key '${key}' is in blocked_keys`; + } + + const rule = policy.restricted_keys?.[key]; + if (!rule) return null; + if (value === undefined) return null; + + if (rule.allowed && rule.allowed.length > 0 && !matchesAny(value, rule.allowed)) { + return `value '${value}' not in allowed list`; + } + if (rule.denied && matchesAny(value, rule.denied)) { + return `value '${value}' in denied list`; + } + if (rule.max !== undefined && rule.max !== null) { + const valueNum = Number(value); + const maxNum = Number(rule.max); + if (!Number.isFinite(valueNum) || !Number.isFinite(maxNum)) { + return `max set but value '${value}' or max '${rule.max}' is not numeric`; + } + if (valueNum > maxNum) { + return `value '${value}' exceeds max '${rule.max}'`; + } + } + return null; +} + +/** + * Inspects the labels and returns the rejection reasons for any `ghr-ec2-*` + * label that violates the policy. Non-`ghr-ec2-*` labels are ignored. + */ +export function violationsAgainstPolicy( + labels: string[], + policy: Ec2DynamicLabelsPolicy | null | undefined, +): { label: string; reason: string }[] { + if (!policy) return []; + const violations: { label: string; reason: string }[] = []; + for (const label of labels) { + if (!label.startsWith('ghr-ec2-')) continue; + const reason = evaluateLabel(label, policy); + if (reason) violations.push({ label, reason }); + } + return violations; +} diff --git a/lambdas/functions/webhook/src/sqs/index.ts b/lambdas/functions/webhook/src/sqs/index.ts index ecf31f1cfd..e891f6cf1b 100644 --- a/lambdas/functions/webhook/src/sqs/index.ts +++ b/lambdas/functions/webhook/src/sqs/index.ts @@ -2,6 +2,8 @@ import { SQS, SendMessageCommandInput } from '@aws-sdk/client-sqs'; import { WorkflowJobEvent } from '@octokit/webhooks-types'; import { createChildLogger, getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; +import { Ec2DynamicLabelsPolicy } from '../runners/dynamic-labels-policy'; + const logger = createChildLogger('sqs'); export interface ActionRequestMessage { @@ -18,6 +20,8 @@ export interface ActionRequestMessage { export interface MatcherConfig { labelMatchers: string[][]; exactMatch: boolean; + enableDynamicLabels?: boolean; + ec2DynamicLabelsPolicy?: Ec2DynamicLabelsPolicy | null; } export type RunnerConfig = RunnerMatcherConfig[]; diff --git a/main.tf b/main.tf index 24255f423a..49bdd89a2f 100644 --- a/main.tf +++ b/main.tf @@ -114,6 +114,8 @@ module "webhook" { matcherConfig : { labelMatchers : [local.runner_labels] exactMatch : var.enable_runner_workflow_job_labels_check_all + enableDynamicLabels : var.enable_dynamic_labels + ec2DynamicLabelsPolicy : var.ec2_dynamic_labels_policy } } } @@ -137,7 +139,6 @@ module "webhook" { logging_retention_in_days = var.logging_retention_in_days logging_kms_key_id = var.logging_kms_key_id log_class = var.log_class - enable_dynamic_labels = var.enable_dynamic_labels role_path = var.role_path role_permissions_boundary = var.role_permissions_boundary @@ -187,7 +188,6 @@ module "runners" { github_app_parameters = local.github_app_parameters enable_organization_runners = var.enable_organization_runners enable_ephemeral_runners = var.enable_ephemeral_runners - enable_dynamic_labels = var.enable_dynamic_labels enable_job_queued_check = var.enable_job_queued_check enable_jit_config = var.enable_jit_config enable_on_demand_failover_for_errors = var.enable_runner_on_demand_failover_for_errors diff --git a/modules/multi-runner/README.md b/modules/multi-runner/README.md index 65cb4e5359..24edf4525c 100644 --- a/modules/multi-runner/README.md +++ b/modules/multi-runner/README.md @@ -127,7 +127,6 @@ module "multi-runner" { | [aws\_region](#input\_aws\_region) | AWS region. | `string` | n/a | yes | | [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 | | [enable\_ami\_housekeeper](#input\_enable\_ami\_housekeeper) | Option to disable the lambda to clean up old AMIs. | `bool` | `false` | no | -| [enable\_dynamic\_labels](#input\_enable\_dynamic\_labels) | Experimental! Can be removed / changed without trigger a major release. Enable dynamic labels with 'ghr-' prefix. When enabled, jobs can use 'ghr-ec2-:' labels to dynamically configure EC2 instances (e.g., 'ghr-ec2-instance-type:t3.large') and 'ghr-run-