From 8138a116b80e2e440ceea7db1d1ce9f2c64cc246 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Thu, 6 Aug 2026 19:44:10 +0200 Subject: [PATCH 1/9] refactor(compute-providers): isolate EC2 provider handling --- .../control-plane/src/pool/pool.test.ts | 4 +- .../src/scale-runners/scale-up.test.ts | 6 +- .../src/runners/aws-dynamic-labels-policy.ts | 1 - .../webhook/src/runners/aws-dynamic-labels.ts | 29 ----- .../webhook/src/runners/dispatch.test.ts | 113 ++++++------------ .../functions/webhook/src/runners/dispatch.ts | 4 +- .../aws/dynamic-labels-policy.ts | 61 ++++++++++ .../ec2/src/webhook/dynamic-labels-policy.ts | 54 +-------- .../ec2/src/webhook/dynamic-labels.test.ts | 58 +++++++++ .../compute-providers/provider-types.test.ts | 11 +- .../compute-providers/webhook.test.ts} | 16 +-- lambdas/libs/compute-providers/webhook.ts | 28 ++++- 12 files changed, 205 insertions(+), 180 deletions(-) delete mode 100644 lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts delete mode 100644 lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts create mode 100644 lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts rename lambdas/{functions/webhook/src/runners/aws-dynamic-labels.test.ts => libs/compute-providers/webhook.test.ts} (72%) diff --git a/lambdas/functions/control-plane/src/pool/pool.test.ts b/lambdas/functions/control-plane/src/pool/pool.test.ts index 568403c3be..ee41d77b41 100644 --- a/lambdas/functions/control-plane/src/pool/pool.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool.test.ts @@ -247,8 +247,8 @@ describe('Test simple pool.', () => { }); it('Rejects unsupported pool provider types.', async () => { - await expect(adjust({ poolSize: 10, type: 'microvm' })).rejects.toThrow( - "Unsupported compute provider type 'microvm'", + await expect(adjust({ poolSize: 10, type: 'unsupported-provider' })).rejects.toThrow( + "Unsupported compute provider type 'unsupported-provider'", ); expect(mockListRunners).not.toHaveBeenCalled(); }); 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 eb6899ff79..9df79ceac1 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 @@ -2157,9 +2157,11 @@ describe('compute provider selection', () => { }); it('rejects unsupported scale-up provider types', async () => { - process.env.COMPUTE_PROVIDER_TYPE = 'microvm'; + process.env.COMPUTE_PROVIDER_TYPE = 'unsupported-provider'; - await expect(scaleUpModule.scaleUp(TEST_DATA)).rejects.toThrow("Unsupported compute provider type 'microvm'"); + await expect(scaleUpModule.scaleUp(TEST_DATA)).rejects.toThrow( + "Unsupported compute provider type 'unsupported-provider'", + ); expect(mockedAppAuth).not.toHaveBeenCalled(); }); }); diff --git a/lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts b/lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts deleted file mode 100644 index 98bba55b30..0000000000 --- a/lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts +++ /dev/null @@ -1 +0,0 @@ -export type { AwsDynamicLabelsPolicy, AwsDynamicLabelsValueRule } from '@aws-github-runner/compute-providers'; diff --git a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts b/lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts deleted file mode 100644 index 418697398a..0000000000 --- a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; -import type { DynamicLabelDispatchTarget } from '@aws-github-runner/compute-providers'; -import { normalizeComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; -import { webhookProviderRegistry } from '@aws-github-runner/compute-providers/webhook'; - -import type { RunnerMatcherConfig } from '../sqs'; - -const logger = createChildLogger('handler'); - -export function selectAwsDynamicLabelQueue( - matches: RunnerMatcherConfig[], - nonGhrLabels: string[], - sanitizedGhrLabels: string[], -): DynamicLabelDispatchTarget | undefined { - for (const queue of matches) { - const provider = normalizeComputeProviderType(queue.computeProvider); - const dynamicLabels = provider ? webhookProviderRegistry.capability(provider, 'dynamicLabels') : undefined; - - if (!dynamicLabels) { - logger.warn(`Queue ${queue.id} has unsupported compute provider '${provider ?? String(queue.computeProvider)}'`); - continue; - } - - const target = dynamicLabels.selectQueue({ queue, nonGhrLabels, sanitizedGhrLabels }); - if (target) return target; - } - - return undefined; -} diff --git a/lambdas/functions/webhook/src/runners/dispatch.test.ts b/lambdas/functions/webhook/src/runners/dispatch.test.ts index ae571da9d8..bb2cdc7cce 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.test.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.test.ts @@ -1,4 +1,5 @@ import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { selectDynamicLabelQueue } from '@aws-github-runner/compute-providers/webhook'; import nock from 'nock'; import { WorkflowJobEvent } from '@octokit/webhooks-types'; @@ -14,6 +15,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; vi.mock('../sqs'); vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/compute-providers/webhook', () => ({ + selectDynamicLabelQueue: vi.fn(), +})); const GITHUB_APP_WEBHOOK_SECRET = 'TEST_SECRET'; @@ -246,7 +250,14 @@ describe('Dispatcher', () => { describe('per-matcher dynamic labels handling', () => { const baseRunner = runnerConfig[0]; - it('strips invalid ghr- labels (too long, bad chars) before policy and dispatch', async () => { + beforeEach(() => { + vi.mocked(selectDynamicLabelQueue).mockImplementation((matches, nonGhrLabels, sanitizedGhrLabels) => ({ + queue: matches[0], + labels: [...nonGhrLabels, ...sanitizedGhrLabels], + })); + }); + + it('strips invalid ghr- labels before provider selection and dispatch', async () => { const longLabel = 'ghr-' + 'a'.repeat(125); // 129 chars config = await createConfig(undefined, [ { @@ -276,19 +287,25 @@ describe('Dispatcher', () => { } as unknown as WorkflowJobEvent; const resp = await dispatch(event, 'workflow_job', config); expect(resp.statusCode).toBe(201); + expect(selectDynamicLabelQueue).toHaveBeenCalledWith( + [expect.objectContaining({ id: baseRunner.id })], + ['self-hosted', 'linux'], + ['ghr-valid:value', 'ghr-list:value;another'], + ); expect(sendActionRequest).toHaveBeenCalledWith( expect.objectContaining({ labels: ['self-hosted', 'linux', 'ghr-valid:value', 'ghr-list:value;another'] }), ); }); - it('rejects the job (202) when the only matching runner has enableDynamicLabels=false', async () => { + it('rejects the job when no provider accepts the dynamic labels', async () => { + vi.mocked(selectDynamicLabelQueue).mockReturnValue(undefined); config = await createConfig(undefined, [ { ...baseRunner, matcherConfig: { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, - enableDynamicLabels: false, + enableDynamicLabels: true, }, }, ]); @@ -296,7 +313,7 @@ describe('Dispatcher', () => { ...workFlowJobEvent, workflow_job: { ...workFlowJobEvent.workflow_job, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + labels: ['self-hosted', 'linux', 'ghr-provider-setting:value'], }, } as unknown as WorkflowJobEvent; const resp = await dispatch(event, 'workflow_job', config); @@ -304,50 +321,20 @@ describe('Dispatcher', () => { expect(sendActionRequest).not.toHaveBeenCalled(); }); - it('keeps dynamic labels when the matched runner enables them and has no policy', async () => { + it('dispatches to the queue and labels returned by the provider selector', async () => { config = await createConfig(undefined, [ { ...baseRunner, + id: 'first', 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('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, - awsDynamicLabelsPolicy: { - restricted_keys: { - 'instance-type': { allowed: ['m5.*'] }, - }, - }, - }, - }, { ...baseRunner, - id: 'permissive', + id: 'selected', matcherConfig: { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, @@ -355,61 +342,29 @@ describe('Dispatcher', () => { }, }, ]); + + vi.mocked(selectDynamicLabelQueue).mockImplementation((matches) => ({ + queue: matches[1], + labels: ['self-hosted', 'linux', 'ghr-provider-setting:normalized'], + })); + const event = { ...workFlowJobEvent, workflow_job: { ...workFlowJobEvent.workflow_job, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + labels: ['self-hosted', 'linux', 'ghr-provider-setting:requested'], }, } 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'], + queueId: 'selected', + labels: ['self-hosted', 'linux', 'ghr-provider-setting:normalized'], }), ); }); - 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, - awsDynamicLabelsPolicy: { - 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('forwards non-dynamic jobs as-is to the first match', async () => { config = await createConfig(undefined, [ { @@ -419,7 +374,6 @@ describe('Dispatcher', () => { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, enableDynamicLabels: true, - awsDynamicLabelsPolicy: {}, }, }, ]); @@ -435,6 +389,7 @@ describe('Dispatcher', () => { expect(sendActionRequest).toHaveBeenCalledWith( expect.objectContaining({ queueId: 'first', labels: ['self-hosted', 'linux'] }), ); + expect(selectDynamicLabelQueue).not.toHaveBeenCalled(); }); }); }); diff --git a/lambdas/functions/webhook/src/runners/dispatch.ts b/lambdas/functions/webhook/src/runners/dispatch.ts index 47c1f1bfc0..da6dc01221 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.ts @@ -1,11 +1,11 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; +import { selectDynamicLabelQueue } from '@aws-github-runner/compute-providers/webhook'; import { WorkflowJobEvent } from '@octokit/webhooks-types'; import { Response } from '../lambda'; import { RunnerMatcherConfig, sendActionRequest } from '../sqs'; import ValidationError from '../ValidationError'; import { ConfigDispatcher, ConfigWebhook, QueueSelectionStrategy } from '../ConfigLoader'; -import { selectAwsDynamicLabelQueue } from './aws-dynamic-labels'; import { canRunJob, splitWorkflowJobLabels } from './labels'; const logger = createChildLogger('handler'); @@ -84,7 +84,7 @@ async function handleWorkflowJob( // Dynamic labels present: prefer the first provider-compliant queue. The // queue selection strategy applies to standard jobs only; dynamic-label jobs // always use the first compliant queue. - const dynamicTarget = selectAwsDynamicLabelQueue(matches, nonGhrLabels, sanitizedGhrLabels); + const dynamicTarget = selectDynamicLabelQueue(matches, nonGhrLabels, sanitizedGhrLabels); if (dynamicTarget) { targets = [dynamicTarget.queue]; diff --git a/lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts b/lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts new file mode 100644 index 0000000000..64b7507add --- /dev/null +++ b/lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts @@ -0,0 +1,61 @@ +import type { AwsDynamicLabelsPolicy } from '../contracts'; + +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((pattern) => globToRegExp(pattern).test(value)); +} + +function evaluateLabel(label: string, policy: AwsDynamicLabelsPolicy, labelPrefix: string): string | null { + const stripped = label.slice(labelPrefix.length); + const colonIndex = stripped.indexOf(':'); + const key = colonIndex === -1 ? stripped : stripped.slice(0, colonIndex); + const value = colonIndex === -1 ? undefined : stripped.slice(colonIndex + 1); + + if (policy.blocked_keys?.includes(key)) { + return `key '${key}' is in blocked_keys`; + } + + const rule = policy.restricted_keys?.[key]; + if (!rule || 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 valueNumber = Number(value); + const maximum = Number(rule.max); + if (!Number.isFinite(valueNumber) || !Number.isFinite(maximum)) { + return `max set but value '${value}' or max '${rule.max}' is not numeric`; + } + if (valueNumber > maximum) { + return `value '${value}' exceeds max '${rule.max}'`; + } + } + + return null; +} + +export function violationsAgainstAwsDynamicLabelsPolicy( + labels: string[], + policy: AwsDynamicLabelsPolicy | null | undefined, + labelPrefix: string, +): { label: string; reason: string }[] { + if (!policy) return []; + + const violations: { label: string; reason: string }[] = []; + for (const label of labels) { + if (!label.startsWith(labelPrefix)) continue; + const reason = evaluateLabel(label, policy, labelPrefix); + if (reason) violations.push({ label, reason }); + } + return violations; +} diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts index a9b919c7bd..8babbadd55 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts @@ -1,4 +1,5 @@ import type { AwsDynamicLabelsPolicy, AwsDynamicLabelsValueRule } from '../../../../contracts'; +import { violationsAgainstAwsDynamicLabelsPolicy } from '../../../dynamic-labels-policy'; export type Ec2DynamicLabelsValueRule = AwsDynamicLabelsValueRule; @@ -10,50 +11,6 @@ export type Ec2DynamicLabelsValueRule = AwsDynamicLabelsValueRule; */ export type Ec2DynamicLabelsPolicy = AwsDynamicLabelsPolicy; -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. @@ -62,12 +19,5 @@ 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; + return violationsAgainstAwsDynamicLabelsPolicy(labels, policy, 'ghr-ec2-'); } diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts index 400807554f..9b0cd07924 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts @@ -4,6 +4,64 @@ import type { RunnerMatcherConfig } from '../../../../contracts'; import { selectEc2DynamicLabelQueue } from './dynamic-labels'; describe('selectEc2DynamicLabelQueue', () => { + it('rejects dynamic labels when the queue disables them', () => { + const queue = runnerQueue('dynamic-labels-disabled'); + queue.matcherConfig.enableDynamicLabels = false; + + expect( + selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), + ).toBeUndefined(); + }); + + it('accepts dynamic labels when the queue has no policy', () => { + const queue = runnerQueue('no-policy'); + + expect(selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ + queue, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }); + }); + + it('skips a policy-rejected queue and returns the next compliant queue', () => { + const strictQueue = runnerQueue('strict'); + strictQueue.matcherConfig.awsDynamicLabelsPolicy = { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + const permissiveQueue = runnerQueue('permissive'); + + expect( + selectEc2DynamicLabelQueue( + [strictQueue, permissiveQueue], + ['self-hosted', 'linux'], + ['ghr-ec2-instance-type:t3.large'], + ), + ).toEqual({ + queue: permissiveQueue, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }); + }); + + it('returns undefined when no queue accepts the dynamic labels', () => { + const strictQueue = runnerQueue('strict'); + strictQueue.matcherConfig.awsDynamicLabelsPolicy = { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + const disabledQueue = runnerQueue('disabled'); + disabledQueue.matcherConfig.enableDynamicLabels = false; + + expect( + selectEc2DynamicLabelQueue( + [strictQueue, disabledQueue], + ['self-hosted', 'linux'], + ['ghr-ec2-instance-type:t3.large'], + ), + ).toBeUndefined(); + }); + it('enforces a legacy EC2 dynamic labels policy when the new key is absent', () => { const queue = runnerQueue('legacy-ec2-policy'); queue.matcherConfig.ec2DynamicLabelsPolicy = { diff --git a/lambdas/libs/compute-providers/provider-types.test.ts b/lambdas/libs/compute-providers/provider-types.test.ts index 76111897ab..746274e5cc 100644 --- a/lambdas/libs/compute-providers/provider-types.test.ts +++ b/lambdas/libs/compute-providers/provider-types.test.ts @@ -23,9 +23,12 @@ describe('compute provider normalization', () => { expect(normalizeComputeProviderType(type)).toBe(expected); }); - it.each([[' Unknown '], ['microvm'], [null], [1]])('returns undefined for unsupported provider type %j', (type) => { - expect(normalizeComputeProviderType(type)).toBeUndefined(); - }); + it.each([[' Unknown '], ['unsupported-provider'], [null], [1]])( + 'returns undefined for unsupported provider type %j', + (type) => { + expect(normalizeComputeProviderType(type)).toBeUndefined(); + }, + ); }); describe('compute provider resolution', () => { @@ -38,7 +41,7 @@ describe('compute provider resolution', () => { expect(resolveComputeProviderType(type)).toBe(expected); }); - it.each([[' Unknown '], ['microvm'], [null], [1]])('rejects unsupported provider type %j', (type) => { + it.each([[' Unknown '], ['unsupported-provider'], [null], [1]])('rejects unsupported provider type %j', (type) => { expect(() => resolveComputeProviderType(type)).toThrow(`Unsupported compute provider type '${String(type)}'`); }); }); diff --git a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.test.ts b/lambdas/libs/compute-providers/webhook.test.ts similarity index 72% rename from lambdas/functions/webhook/src/runners/aws-dynamic-labels.test.ts rename to lambdas/libs/compute-providers/webhook.test.ts index 790d4c2989..2007248b18 100644 --- a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.test.ts +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -1,14 +1,14 @@ -import type { ComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; import { describe, expect, it } from 'vitest'; -import type { RunnerMatcherConfig } from '../sqs'; -import { selectAwsDynamicLabelQueue } from './aws-dynamic-labels'; +import type { RunnerMatcherConfig } from './contracts'; +import type { ComputeProviderType } from './provider-types'; +import { selectDynamicLabelQueue } from './webhook'; -describe('selectAwsDynamicLabelQueue', () => { +describe('selectDynamicLabelQueue', () => { it('defaults queues without a provider to EC2 dynamic label handling', () => { const queue = runnerQueue('default-ec2'); - expect(selectAwsDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ + expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ queue, labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], }); @@ -18,7 +18,7 @@ describe('selectAwsDynamicLabelQueue', () => { const queue = runnerQueue('normalized-ec2'); (queue as unknown as { computeProvider: string }).computeProvider = ' EC2 '; - expect(selectAwsDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ + expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ queue, labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], }); @@ -30,7 +30,7 @@ describe('selectAwsDynamicLabelQueue', () => { const ec2Queue = runnerQueue('ec2'); expect( - selectAwsDynamicLabelQueue( + selectDynamicLabelQueue( [unsupportedQueue, ec2Queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'], @@ -46,7 +46,7 @@ describe('selectAwsDynamicLabelQueue', () => { (queue as unknown as { computeProvider: number }).computeProvider = 42; expect( - selectAwsDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), + selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), ).toBeUndefined(); }); }); diff --git a/lambdas/libs/compute-providers/webhook.ts b/lambdas/libs/compute-providers/webhook.ts index ee80a54203..4c70c6a74c 100644 --- a/lambdas/libs/compute-providers/webhook.ts +++ b/lambdas/libs/compute-providers/webhook.ts @@ -1,8 +1,34 @@ +import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; + import { createComputeProviderRegistry } from './core'; -import type { WebhookProviderCapabilities } from './contracts'; +import type { DynamicLabelDispatchTarget, RunnerMatcherConfig, WebhookProviderCapabilities } from './contracts'; +import { normalizeComputeProviderType } from './provider-types'; import { enabledWebhookProviders } from './providers.config.webhook'; +const logger = createChildLogger('compute-provider-webhook'); + export const webhookProviderRegistry = createComputeProviderRegistry( enabledWebhookProviders.map((provider) => provider.createPlugin()), ); + +export function selectDynamicLabelQueue( + matches: RunnerMatcherConfig[], + nonGhrLabels: string[], + sanitizedGhrLabels: string[], +): DynamicLabelDispatchTarget | undefined { + for (const queue of matches) { + const provider = normalizeComputeProviderType(queue.computeProvider); + const dynamicLabels = provider ? webhookProviderRegistry.capability(provider, 'dynamicLabels') : undefined; + + if (!dynamicLabels) { + logger.warn(`Queue ${queue.id} has unsupported compute provider '${provider ?? String(queue.computeProvider)}'`); + continue; + } + + const target = dynamicLabels.selectQueue({ queue, nonGhrLabels, sanitizedGhrLabels }); + if (target) return target; + } + + return undefined; +} From f91cb1370b897ed21bb6026feebb93db4d6e36c6 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Thu, 6 Aug 2026 21:16:34 +0200 Subject: [PATCH 2/9] refactor(compute-providers): resolve provider types strictly --- .../compute-providers/provider-types.test.ts | 43 ++++++------------- .../libs/compute-providers/provider-types.ts | 16 +++---- .../libs/compute-providers/webhook.test.ts | 27 +++--------- lambdas/libs/compute-providers/webhook.ts | 15 ++----- 4 files changed, 29 insertions(+), 72 deletions(-) diff --git a/lambdas/libs/compute-providers/provider-types.test.ts b/lambdas/libs/compute-providers/provider-types.test.ts index 746274e5cc..9f6f4a981e 100644 --- a/lambdas/libs/compute-providers/provider-types.test.ts +++ b/lambdas/libs/compute-providers/provider-types.test.ts @@ -1,11 +1,15 @@ import { describe, expect, it } from 'vitest'; -import { - defaultComputeProvider, - normalizeComputeProviderType, - resolveComputeProviderType, - computeProviderTypes, -} from './provider-types'; +import { computeProviderTypes, defaultComputeProvider, resolveComputeProviderType } from './provider-types'; + +const defaultProviderInputs = [undefined, '', ' '] as const; +const supportedProviderCases = computeProviderTypes.flatMap( + (provider) => + [ + [provider, provider], + [` ${provider.toUpperCase()} `, provider], + ] as const, +); describe('compute provider configuration', () => { it('defines an explicit default provider', () => { @@ -13,31 +17,12 @@ describe('compute provider configuration', () => { }); }); -describe('compute provider normalization', () => { - it.each([ - [undefined, 'ec2'], - ['', 'ec2'], - [' ', 'ec2'], - [' EC2 ', 'ec2'], - ])('normalizes provider type %j to %j', (type, expected) => { - expect(normalizeComputeProviderType(type)).toBe(expected); +describe('compute provider resolution', () => { + it.each(defaultProviderInputs)('resolves default provider input %j', (type) => { + expect(resolveComputeProviderType(type)).toBe(defaultComputeProvider); }); - it.each([[' Unknown '], ['unsupported-provider'], [null], [1]])( - 'returns undefined for unsupported provider type %j', - (type) => { - expect(normalizeComputeProviderType(type)).toBeUndefined(); - }, - ); -}); - -describe('compute provider resolution', () => { - it.each([ - [undefined, 'ec2'], - ['', 'ec2'], - [' ', 'ec2'], - [' EC2 ', 'ec2'], - ])('resolves provider type %j to %j', (type, expected) => { + it.each(supportedProviderCases)('resolves provider type %j to %j', (type, expected) => { expect(resolveComputeProviderType(type)).toBe(expected); }); diff --git a/lambdas/libs/compute-providers/provider-types.ts b/lambdas/libs/compute-providers/provider-types.ts index dcac6c5769..64d7be8e5f 100644 --- a/lambdas/libs/compute-providers/provider-types.ts +++ b/lambdas/libs/compute-providers/provider-types.ts @@ -4,21 +4,19 @@ export type ComputeProviderType = (typeof computeProviderTypes)[number]; export const defaultComputeProvider = 'ec2' satisfies ComputeProviderType; -export function normalizeComputeProviderType(type: unknown): ComputeProviderType | undefined { +export function resolveComputeProviderType(type: unknown): ComputeProviderType { if (type === undefined) return defaultComputeProvider; - if (typeof type !== 'string') return undefined; + if (typeof type !== 'string') { + throw new Error(`Unsupported compute provider type '${String(type)}'`); + } const normalizedType = type.trim().toLowerCase(); if (!normalizedType) return defaultComputeProvider; - return computeProviderTypes.find((computeProviderType) => computeProviderType === normalizedType); -} - -export function resolveComputeProviderType(type: unknown): ComputeProviderType { - const normalizedType = normalizeComputeProviderType(type); - if (!normalizedType) { + const computeProviderType = computeProviderTypes.find((provider) => provider === normalizedType); + if (!computeProviderType) { throw new Error(`Unsupported compute provider type '${String(type)}'`); } - return normalizedType; + return computeProviderType; } diff --git a/lambdas/libs/compute-providers/webhook.test.ts b/lambdas/libs/compute-providers/webhook.test.ts index 2007248b18..b46e365246 100644 --- a/lambdas/libs/compute-providers/webhook.test.ts +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -24,30 +24,13 @@ describe('selectDynamicLabelQueue', () => { }); }); - it('skips an unsupported provider strategy and selects the next supported queue', () => { - const unsupportedQueue = runnerQueue('unsupported-provider'); - (unsupportedQueue as unknown as { computeProvider: string }).computeProvider = 'unsupported'; - const ec2Queue = runnerQueue('ec2'); + it.each([['unsupported'], [42]])('throws for unsupported compute provider %j', (computeProvider) => { + const queue = runnerQueue('unsupported-provider'); + (queue as unknown as { computeProvider: unknown }).computeProvider = computeProvider; - expect( - selectDynamicLabelQueue( - [unsupportedQueue, ec2Queue], - ['self-hosted', 'linux'], - ['ghr-ec2-instance-type:t3.large'], - ), - ).toEqual({ - queue: ec2Queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); - - it('rejects a malformed non-string compute provider without throwing', () => { - const queue = runnerQueue('malformed-provider'); - (queue as unknown as { computeProvider: number }).computeProvider = 42; - - expect( + expect(() => selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); + ).toThrow(`Unsupported compute provider type '${String(computeProvider)}'`); }); }); diff --git a/lambdas/libs/compute-providers/webhook.ts b/lambdas/libs/compute-providers/webhook.ts index 4c70c6a74c..2b3414777a 100644 --- a/lambdas/libs/compute-providers/webhook.ts +++ b/lambdas/libs/compute-providers/webhook.ts @@ -1,13 +1,9 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; - import { createComputeProviderRegistry } from './core'; import type { DynamicLabelDispatchTarget, RunnerMatcherConfig, WebhookProviderCapabilities } from './contracts'; -import { normalizeComputeProviderType } from './provider-types'; +import { resolveComputeProviderType } from './provider-types'; import { enabledWebhookProviders } from './providers.config.webhook'; -const logger = createChildLogger('compute-provider-webhook'); - export const webhookProviderRegistry = createComputeProviderRegistry( enabledWebhookProviders.map((provider) => provider.createPlugin()), ); @@ -18,13 +14,8 @@ export function selectDynamicLabelQueue( sanitizedGhrLabels: string[], ): DynamicLabelDispatchTarget | undefined { for (const queue of matches) { - const provider = normalizeComputeProviderType(queue.computeProvider); - const dynamicLabels = provider ? webhookProviderRegistry.capability(provider, 'dynamicLabels') : undefined; - - if (!dynamicLabels) { - logger.warn(`Queue ${queue.id} has unsupported compute provider '${provider ?? String(queue.computeProvider)}'`); - continue; - } + const provider = resolveComputeProviderType(queue.computeProvider); + const dynamicLabels = webhookProviderRegistry.capability(provider, 'dynamicLabels'); const target = dynamicLabels.selectQueue({ queue, nonGhrLabels, sanitizedGhrLabels }); if (target) return target; From fd78317c1caf6c7f62ae928bb473b5c5bad99156 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Thu, 13 Aug 2026 00:28:54 +0200 Subject: [PATCH 3/9] refactor(compute-providers): centralize dynamic label selection --- .../ec2/src/webhook/dynamic-labels.test.ts | 76 ++++--------- .../aws/ec2/src/webhook/dynamic-labels.ts | 36 +----- lambdas/libs/compute-providers/contracts.ts | 11 +- .../compute-providers/dynamic-labels.test.ts | 12 ++ .../libs/compute-providers/dynamic-labels.ts | 8 ++ .../libs/compute-providers/registry.test.ts | 2 +- .../templates/provider/provider.test.ts | 2 +- .../templates/provider/webhook.ts | 6 +- .../libs/compute-providers/webhook.test.ts | 106 ++++++++++++++---- lambdas/libs/compute-providers/webhook.ts | 71 +++++++++--- 10 files changed, 195 insertions(+), 135 deletions(-) create mode 100644 lambdas/libs/compute-providers/dynamic-labels.test.ts create mode 100644 lambdas/libs/compute-providers/dynamic-labels.ts diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts index 9b0cd07924..99c1844a5f 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts @@ -1,65 +1,29 @@ import { describe, expect, it } from 'vitest'; import type { RunnerMatcherConfig } from '../../../../contracts'; -import { selectEc2DynamicLabelQueue } from './dynamic-labels'; +import { ec2DynamicLabelProvider } from './dynamic-labels'; -describe('selectEc2DynamicLabelQueue', () => { - it('rejects dynamic labels when the queue disables them', () => { - const queue = runnerQueue('dynamic-labels-disabled'); - queue.matcherConfig.enableDynamicLabels = false; - - expect( - selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); - }); - - it('accepts dynamic labels when the queue has no policy', () => { +describe('ec2DynamicLabelProvider', () => { + it('returns no violations when the queue has no policy', () => { const queue = runnerQueue('no-policy'); - expect(selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); + expect(getViolations(queue)).toEqual([]); }); - it('skips a policy-rejected queue and returns the next compliant queue', () => { + it('returns violations for labels rejected by the policy', () => { const strictQueue = runnerQueue('strict'); strictQueue.matcherConfig.awsDynamicLabelsPolicy = { restricted_keys: { 'instance-type': { allowed: ['m5.*'] }, }, }; - const permissiveQueue = runnerQueue('permissive'); - expect( - selectEc2DynamicLabelQueue( - [strictQueue, permissiveQueue], - ['self-hosted', 'linux'], - ['ghr-ec2-instance-type:t3.large'], - ), - ).toEqual({ - queue: permissiveQueue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); - - it('returns undefined when no queue accepts the dynamic labels', () => { - const strictQueue = runnerQueue('strict'); - strictQueue.matcherConfig.awsDynamicLabelsPolicy = { - restricted_keys: { - 'instance-type': { allowed: ['m5.*'] }, + expect(getViolations(strictQueue)).toEqual([ + { + label: 'ghr-ec2-instance-type:t3.large', + reason: "value 't3.large' not in allowed list", }, - }; - const disabledQueue = runnerQueue('disabled'); - disabledQueue.matcherConfig.enableDynamicLabels = false; - - expect( - selectEc2DynamicLabelQueue( - [strictQueue, disabledQueue], - ['self-hosted', 'linux'], - ['ghr-ec2-instance-type:t3.large'], - ), - ).toBeUndefined(); + ]); }); it('enforces a legacy EC2 dynamic labels policy when the new key is absent', () => { @@ -68,9 +32,7 @@ describe('selectEc2DynamicLabelQueue', () => { blocked_keys: ['instance-type'], }; - expect( - selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); + expect(getViolations(queue)).toHaveLength(1); }); it('falls back to the legacy EC2 dynamic labels policy when the new policy is null', () => { @@ -80,9 +42,7 @@ describe('selectEc2DynamicLabelQueue', () => { }; queue.matcherConfig.awsDynamicLabelsPolicy = null; - expect( - selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); + expect(getViolations(queue)).toHaveLength(1); }); it('prefers a configured AWS dynamic labels policy over the legacy policy', () => { @@ -94,13 +54,17 @@ describe('selectEc2DynamicLabelQueue', () => { blocked_keys: [], }; - expect(selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); + expect(getViolations(queue)).toEqual([]); }); }); +function getViolations(queue: RunnerMatcherConfig) { + return ec2DynamicLabelProvider.getViolations({ + queue, + labels: ['ghr-ec2-instance-type:t3.large'], + }); +} + function runnerQueue(id: string): RunnerMatcherConfig { return { id, diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts index 6ddf5b8fbb..5e671da189 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts @@ -1,12 +1,10 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; -import type { DynamicLabelDispatchTarget, DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts'; +import type { DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts'; import { violationsAgainstPolicy } from './dynamic-labels-policy'; const logger = createChildLogger('handler'); -export type Ec2DynamicLabelDispatchTarget = DynamicLabelDispatchTarget; - function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) { const hasLegacyEc2DynamicLabelsPolicy = Object.prototype.hasOwnProperty.call( queue.matcherConfig, @@ -23,36 +21,6 @@ function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) { return queue.matcherConfig.awsDynamicLabelsPolicy; } -export function selectEc2DynamicLabelQueue( - matches: RunnerMatcherConfig[], - nonGhrLabels: string[], - sanitizedGhrLabels: string[], -): Ec2DynamicLabelDispatchTarget | undefined { - for (const queue of matches) { - if (!queue.matcherConfig.enableDynamicLabels) { - logger.warn(`Queue ${queue.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`); - continue; - } - - const violations = violationsAgainstPolicy(sanitizedGhrLabels, resolveEc2DynamicLabelsPolicy(queue)); - if (violations.length === 0) { - return { - queue, - labels: [...nonGhrLabels, ...sanitizedGhrLabels], - }; - } - - for (const violation of violations) { - logger.warn( - `Queue ${queue.id}: dynamic label '${violation.label}' does not match policy (${violation.reason}); trying next match`, - ); - } - } - - return undefined; -} - export const ec2DynamicLabelProvider: DynamicLabelProvider = { - selectQueue: ({ queue, nonGhrLabels, sanitizedGhrLabels }) => - selectEc2DynamicLabelQueue([queue], nonGhrLabels, sanitizedGhrLabels), + getViolations: ({ queue, labels }) => violationsAgainstPolicy(labels, resolveEc2DynamicLabelsPolicy(queue)), }; diff --git a/lambdas/libs/compute-providers/contracts.ts b/lambdas/libs/compute-providers/contracts.ts index 617789ec10..85e99f3949 100644 --- a/lambdas/libs/compute-providers/contracts.ts +++ b/lambdas/libs/compute-providers/contracts.ts @@ -43,12 +43,13 @@ export interface DynamicLabelDispatchTarget { labels: string[]; } +export interface DynamicLabelViolation { + label: string; + reason: string; +} + export interface DynamicLabelProvider { - selectQueue(input: { - queue: RunnerMatcherConfig; - nonGhrLabels: string[]; - sanitizedGhrLabels: string[]; - }): DynamicLabelDispatchTarget | undefined; + getViolations(input: { queue: RunnerMatcherConfig; labels: string[] }): DynamicLabelViolation[]; } export interface ControlPlaneProviderCapabilities { diff --git a/lambdas/libs/compute-providers/dynamic-labels.test.ts b/lambdas/libs/compute-providers/dynamic-labels.test.ts new file mode 100644 index 0000000000..0eacc9cf62 --- /dev/null +++ b/lambdas/libs/compute-providers/dynamic-labels.test.ts @@ -0,0 +1,12 @@ +import { expect, it } from 'vitest'; + +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; +import { computeProviderTypes } from './provider-types'; + +it.each(computeProviderTypes)('returns labels belonging to providers other than %s', (provider) => { + const providerLabels = computeProviderTypes.map((type) => `ghr-${type}-size:large`); + + expect(dynamicLabelsForOtherProvider(providerLabels, provider)).toEqual( + providerLabels.filter((label) => !label.startsWith(`ghr-${provider}-`)), + ); +}); diff --git a/lambdas/libs/compute-providers/dynamic-labels.ts b/lambdas/libs/compute-providers/dynamic-labels.ts new file mode 100644 index 0000000000..97db9517d3 --- /dev/null +++ b/lambdas/libs/compute-providers/dynamic-labels.ts @@ -0,0 +1,8 @@ +import { computeProviderTypes } from './provider-types'; +import type { ComputeProviderType } from './provider-types'; + +export function dynamicLabelsForOtherProvider(labels: string[], provider: ComputeProviderType): string[] { + return labels.filter((label) => + computeProviderTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)), + ); +} diff --git a/lambdas/libs/compute-providers/registry.test.ts b/lambdas/libs/compute-providers/registry.test.ts index 3c95dcaca4..93227831cd 100644 --- a/lambdas/libs/compute-providers/registry.test.ts +++ b/lambdas/libs/compute-providers/registry.test.ts @@ -33,6 +33,6 @@ it('exposes every configured provider through both capability registries', () => unmarkOrphan: expect.any(Function), terminate: expect.any(Function), }); - expect(webhookProviderRegistry.capability(type, 'dynamicLabels').selectQueue).toEqual(expect.any(Function)); + expect(webhookProviderRegistry.capability(type, 'dynamicLabels').getViolations).toEqual(expect.any(Function)); } }); diff --git a/lambdas/libs/compute-providers/templates/provider/provider.test.ts b/lambdas/libs/compute-providers/templates/provider/provider.test.ts index 816b2f9cfc..2644fc4f2a 100644 --- a/lambdas/libs/compute-providers/templates/provider/provider.test.ts +++ b/lambdas/libs/compute-providers/templates/provider/provider.test.ts @@ -29,5 +29,5 @@ it('exposes every compute provider capability from its compute-provider entry po terminate: expect.any(Function), }); expect(webhookPlugin.type).toBe(webhookProvider.type); - expect(webhookPlugin.capabilities.dynamicLabels.selectQueue).toEqual(expect.any(Function)); + expect(webhookPlugin.capabilities.dynamicLabels.getViolations).toEqual(expect.any(Function)); }); diff --git a/lambdas/libs/compute-providers/templates/provider/webhook.ts b/lambdas/libs/compute-providers/templates/provider/webhook.ts index 86e59da7b3..31c522c588 100644 --- a/lambdas/libs/compute-providers/templates/provider/webhook.ts +++ b/lambdas/libs/compute-providers/templates/provider/webhook.ts @@ -3,10 +3,10 @@ import type { ComputeProviderPlugin } from '../../core'; import type { DynamicLabelProvider, WebhookProviderCapabilities, WebhookProviderModule } from '../../contracts'; export const templateDynamicLabelProvider: DynamicLabelProvider = { - selectQueue: (input) => { + getViolations: (input) => { void input; - // Return a dispatch target when this provider accepts the requested dynamic labels. - return undefined; + // Return violations for dynamic labels this provider does not accept. + return []; }, }; diff --git a/lambdas/libs/compute-providers/webhook.test.ts b/lambdas/libs/compute-providers/webhook.test.ts index b46e365246..4316c3aca5 100644 --- a/lambdas/libs/compute-providers/webhook.test.ts +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -1,44 +1,106 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; -import type { RunnerMatcherConfig } from './contracts'; +import type { DynamicLabelProvider, DynamicLabelViolation, RunnerMatcherConfig } from './contracts'; import type { ComputeProviderType } from './provider-types'; -import { selectDynamicLabelQueue } from './webhook'; +import { createDynamicLabelQueueSelector } from './webhook'; -describe('selectDynamicLabelQueue', () => { - it('defaults queues without a provider to EC2 dynamic label handling', () => { - const queue = runnerQueue('default-ec2'); +describe('createDynamicLabelQueueSelector', () => { + it('returns the first queue accepted by its provider', () => { + const queue = runnerQueue('accepted'); + const { selectQueue } = selector(); - expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ + expect(selectQueue([queue], ['self-hosted', 'linux'], ['ghr-test-size:large'])).toEqual({ queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + labels: ['self-hosted', 'linux', 'ghr-test-size:large'], }); }); - it('normalizes compute provider casing and surrounding whitespace', () => { - const queue = runnerQueue('normalized-ec2'); - (queue as unknown as { computeProvider: string }).computeProvider = ' EC2 '; + it('skips queues that disable dynamic labels', () => { + const disabledQueue = runnerQueue('disabled'); + disabledQueue.matcherConfig.enableDynamicLabels = false; + const enabledQueue = runnerQueue('enabled'); + const { getViolations, selectQueue } = selector(); - expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + expect(selectQueue([disabledQueue, enabledQueue], ['self-hosted'], ['ghr-test-size:large'])).toEqual({ + queue: enabledQueue, + labels: ['self-hosted', 'ghr-test-size:large'], + }); + expect(getViolations).toHaveBeenCalledOnce(); + expect(getViolations).toHaveBeenCalledWith({ queue: enabledQueue, labels: ['ghr-test-size:large'] }); + }); + + it('skips queues whose provider reports violations', () => { + const rejectedQueue = runnerQueue('rejected'); + const acceptedQueue = runnerQueue('accepted'); + const { selectQueue } = selector({ + violationsByQueue: { + rejected: [{ label: 'ghr-test-size:large', reason: 'size is unavailable' }], + }, + }); + + expect(selectQueue([rejectedQueue, acceptedQueue], ['self-hosted'], ['ghr-test-size:large'])).toEqual({ + queue: acceptedQueue, + labels: ['self-hosted', 'ghr-test-size:large'], + }); + }); + + it('returns undefined when every provider reports violations', () => { + const queue = runnerQueue('rejected'); + const { selectQueue } = selector({ + violationsByQueue: { + rejected: [{ label: 'ghr-test-size:large', reason: 'size is unavailable' }], + }, }); + + expect(selectQueue([queue], ['self-hosted'], ['ghr-test-size:large'])).toBeUndefined(); }); - it.each([['unsupported'], [42]])('throws for unsupported compute provider %j', (computeProvider) => { - const queue = runnerQueue('unsupported-provider'); - (queue as unknown as { computeProvider: unknown }).computeProvider = computeProvider; + /* TODO: Re-enable this scenario when the MicroVM provider is added. + it('skips EC2 and selects the MicroVM queue for MicroVM override labels', () => { + const ec2Queue = runnerQueue('ec2'); + const microvmQueue = runnerQueue('microvm'); + const imageVersionLabel = 'ghr-microvm-image-version:3.0'; + const { getViolations, selectQueue } = selector({ + providerByQueue: { ec2: 'ec2', microvm: 'microvm' }, + labelsForOtherProvider: (labels, provider) => + provider === 'ec2' ? labels.filter((label) => label.startsWith('ghr-microvm-')) : [], + }); - expect(() => - selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toThrow(`Unsupported compute provider type '${String(computeProvider)}'`); + expect(selectQueue([ec2Queue, microvmQueue], ['self-hosted', 'linux'], [imageVersionLabel])).toEqual({ + queue: microvmQueue, + labels: ['self-hosted', 'linux', imageVersionLabel], + }); + expect(getViolations).toHaveBeenCalledOnce(); + expect(getViolations).toHaveBeenCalledWith({ queue: microvmQueue, labels: [imageVersionLabel] }); }); + */ }); -function runnerQueue(id: string, computeProvider?: ComputeProviderType): RunnerMatcherConfig { +function selector(options?: { + providerByQueue?: Record; + violationsByQueue?: Record; + labelsForOtherProvider?: (labels: string[], provider: ComputeProviderType) => string[]; +}) { + const getViolations = vi.fn(({ queue }) => { + return options?.violationsByQueue?.[queue.id] ?? []; + }); + + return { + getViolations, + selectQueue: createDynamicLabelQueueSelector({ + resolveProvider: (queue) => ({ + type: options?.providerByQueue?.[queue.id] ?? 'ec2', + dynamicLabels: { getViolations }, + }), + dynamicLabelsForOtherProvider: options?.labelsForOtherProvider ?? (() => []), + }), + }; +} + +function runnerQueue(id: string): RunnerMatcherConfig { return { id, arn: `arn:${id}`, - computeProvider, matcherConfig: { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, diff --git a/lambdas/libs/compute-providers/webhook.ts b/lambdas/libs/compute-providers/webhook.ts index 2b3414777a..1aa0a7b5e6 100644 --- a/lambdas/libs/compute-providers/webhook.ts +++ b/lambdas/libs/compute-providers/webhook.ts @@ -1,25 +1,70 @@ +import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; + import { createComputeProviderRegistry } from './core'; -import type { DynamicLabelDispatchTarget, RunnerMatcherConfig, WebhookProviderCapabilities } from './contracts'; +import type { + DynamicLabelDispatchTarget, + DynamicLabelProvider, + RunnerMatcherConfig, + WebhookProviderCapabilities, +} from './contracts'; +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; import { resolveComputeProviderType } from './provider-types'; import { enabledWebhookProviders } from './providers.config.webhook'; +const logger = createChildLogger('handler'); + export const webhookProviderRegistry = createComputeProviderRegistry( enabledWebhookProviders.map((provider) => provider.createPlugin()), ); -export function selectDynamicLabelQueue( - matches: RunnerMatcherConfig[], - nonGhrLabels: string[], - sanitizedGhrLabels: string[], -): DynamicLabelDispatchTarget | undefined { - for (const queue of matches) { - const provider = resolveComputeProviderType(queue.computeProvider); - const dynamicLabels = webhookProviderRegistry.capability(provider, 'dynamicLabels'); +export function createDynamicLabelQueueSelector(dependencies: { + resolveProvider(queue: RunnerMatcherConfig): { type: TProvider; dynamicLabels: DynamicLabelProvider }; + dynamicLabelsForOtherProvider(labels: string[], provider: TProvider): string[]; +}) { + return ( + matches: RunnerMatcherConfig[], + nonGhrLabels: string[], + sanitizedGhrLabels: string[], + ): DynamicLabelDispatchTarget | undefined => { + for (const queue of matches) { + const { type: provider, dynamicLabels } = dependencies.resolveProvider(queue); + + if (!queue.matcherConfig.enableDynamicLabels) { + logger.warn( + `Queue ${queue.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`, + ); + continue; + } - const target = dynamicLabels.selectQueue({ queue, nonGhrLabels, sanitizedGhrLabels }); - if (target) return target; - } + const labelsForOtherProvider = dependencies.dynamicLabelsForOtherProvider(sanitizedGhrLabels, provider); + if (labelsForOtherProvider.length > 0) { + logger.warn(`Queue ${queue.id}: dynamic labels target another compute provider; trying next match`, { + dynamicLabels: labelsForOtherProvider, + }); + continue; + } - return undefined; + const violations = dynamicLabels.getViolations({ queue, labels: sanitizedGhrLabels }); + if (violations.length === 0) { + return { queue, labels: [...nonGhrLabels, ...sanitizedGhrLabels] }; + } + + for (const violation of violations) { + logger.warn( + `Queue ${queue.id}: dynamic label '${violation.label}' is not accepted (${violation.reason}); trying next match`, + ); + } + } + + return undefined; + }; } + +export const selectDynamicLabelQueue = createDynamicLabelQueueSelector({ + resolveProvider: (queue) => { + const type = resolveComputeProviderType(queue.computeProvider); + return { type, dynamicLabels: webhookProviderRegistry.capability(type, 'dynamicLabels') }; + }, + dynamicLabelsForOtherProvider, +}); From 4fc3939fd85329384f3b4a6031b32566f330e0cb Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 14 Aug 2026 15:27:34 +0200 Subject: [PATCH 4/9] test(compute-providers): cover dynamic label selection --- .../compute-providers/dynamic-labels.test.ts | 10 +++-- .../libs/compute-providers/dynamic-labels.ts | 12 ++++-- .../libs/compute-providers/webhook.test.ts | 39 ++++++++++++++++++- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/lambdas/libs/compute-providers/dynamic-labels.test.ts b/lambdas/libs/compute-providers/dynamic-labels.test.ts index 0eacc9cf62..e93b9fa264 100644 --- a/lambdas/libs/compute-providers/dynamic-labels.test.ts +++ b/lambdas/libs/compute-providers/dynamic-labels.test.ts @@ -1,10 +1,12 @@ import { expect, it } from 'vitest'; -import { dynamicLabelsForOtherProvider } from './dynamic-labels'; -import { computeProviderTypes } from './provider-types'; +import { createDynamicLabelsForOtherProvider } from './dynamic-labels'; -it.each(computeProviderTypes)('returns labels belonging to providers other than %s', (provider) => { - const providerLabels = computeProviderTypes.map((type) => `ghr-${type}-size:large`); +const providerTypes = ['alpha', 'beta'] as const; +const dynamicLabelsForOtherProvider = createDynamicLabelsForOtherProvider(providerTypes); + +it.each(providerTypes)('returns labels belonging to providers other than %s', (provider) => { + const providerLabels = providerTypes.map((type) => `ghr-${type}-size:large`); expect(dynamicLabelsForOtherProvider(providerLabels, provider)).toEqual( providerLabels.filter((label) => !label.startsWith(`ghr-${provider}-`)), diff --git a/lambdas/libs/compute-providers/dynamic-labels.ts b/lambdas/libs/compute-providers/dynamic-labels.ts index 97db9517d3..8ac72757c8 100644 --- a/lambdas/libs/compute-providers/dynamic-labels.ts +++ b/lambdas/libs/compute-providers/dynamic-labels.ts @@ -1,8 +1,12 @@ import { computeProviderTypes } from './provider-types'; import type { ComputeProviderType } from './provider-types'; -export function dynamicLabelsForOtherProvider(labels: string[], provider: ComputeProviderType): string[] { - return labels.filter((label) => - computeProviderTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)), - ); +export function createDynamicLabelsForOtherProvider(providerTypes: readonly TProvider[]) { + return (labels: string[], provider: TProvider): string[] => + labels.filter((label) => + providerTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)), + ); } + +export const dynamicLabelsForOtherProvider = + createDynamicLabelsForOtherProvider(computeProviderTypes); diff --git a/lambdas/libs/compute-providers/webhook.test.ts b/lambdas/libs/compute-providers/webhook.test.ts index 4316c3aca5..f3120a3b59 100644 --- a/lambdas/libs/compute-providers/webhook.test.ts +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -2,7 +2,44 @@ import { describe, expect, it, vi } from 'vitest'; import type { DynamicLabelProvider, DynamicLabelViolation, RunnerMatcherConfig } from './contracts'; import type { ComputeProviderType } from './provider-types'; -import { createDynamicLabelQueueSelector } from './webhook'; +import { createDynamicLabelQueueSelector, selectDynamicLabelQueue } from './webhook'; + +describe('selectDynamicLabelQueue', () => { + it('defaults queues without a provider to EC2 dynamic label handling', () => { + const queue = runnerQueue('default-ec2'); + + expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ + queue, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }); + }); + + it('normalizes compute provider casing and surrounding whitespace', () => { + const queue = runnerQueue('normalized-ec2'); + (queue as unknown as { computeProvider: string }).computeProvider = ' EC2 '; + + expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ + queue, + labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + }); + }); + + it.each([ + ['unsupported string', 'unsupported-provider'], + ['non-string', 42], + ])('strictly rejects an %s compute provider', (_description, computeProvider) => { + const invalidQueue = runnerQueue('invalid-provider'); + (invalidQueue as unknown as { computeProvider: unknown }).computeProvider = computeProvider; + + expect(() => + selectDynamicLabelQueue( + [invalidQueue, runnerQueue('valid-ec2')], + ['self-hosted', 'linux'], + ['ghr-ec2-instance-type:t3.large'], + ), + ).toThrow(`Unsupported compute provider type '${String(computeProvider)}'`); + }); +}); describe('createDynamicLabelQueueSelector', () => { it('returns the first queue accepted by its provider', () => { From 7adbb527b8279195f6e7eb1564bddd121314e6bd Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 14 Aug 2026 15:51:50 +0200 Subject: [PATCH 5/9] test(compute-providers): share webhook provider contract --- .../compute-providers/aws/ec2/webhook.test.ts | 7 ++ .../test/webhook-provider-contract.ts | 58 ++++++++++++++++ lambdas/libs/compute-providers/tsconfig.json | 2 +- .../libs/compute-providers/webhook.test.ts | 66 ++++++------------- 4 files changed, 87 insertions(+), 46 deletions(-) create mode 100644 lambdas/libs/compute-providers/aws/ec2/webhook.test.ts create mode 100644 lambdas/libs/compute-providers/test/webhook-provider-contract.ts diff --git a/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts new file mode 100644 index 0000000000..d6557d3c88 --- /dev/null +++ b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts @@ -0,0 +1,7 @@ +import { defineWebhookProviderContractTests } from '../../test/webhook-provider-contract'; +import { provider } from './webhook'; + +defineWebhookProviderContractTests({ + provider, + acceptedDynamicLabels: ['ghr-ec2-instance-type:t3.large'], +}); diff --git a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts new file mode 100644 index 0000000000..a970e5f10e --- /dev/null +++ b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest'; + +import type { RunnerMatcherConfig, WebhookProviderModule } from '../contracts'; +import { defaultComputeProvider } from '../provider-types'; +import type { ComputeProviderType } from '../provider-types'; +import { selectDynamicLabelQueue } from '../webhook'; + +interface WebhookProviderContractOptions { + provider: WebhookProviderModule; + acceptedDynamicLabels: readonly [string, ...string[]]; +} + +export function defineWebhookProviderContractTests({ + provider, + acceptedDynamicLabels, +}: WebhookProviderContractOptions): void { + const nonGhrLabels = ['self-hosted', 'linux']; + const dynamicLabels = [...acceptedDynamicLabels]; + + function expectProviderSelected(queue: RunnerMatcherConfig) { + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toEqual({ + queue, + labels: [...nonGhrLabels, ...dynamicLabels], + }); + } + + describe(`${provider.type} webhook provider contract`, () => { + it('selects an explicitly configured provider through the production registry', () => { + expectProviderSelected(runnerQueue(`${provider.type}-configured`, provider.type)); + }); + + it('normalizes provider configuration before registry selection', () => { + const queue = runnerQueue(`${provider.type}-normalized`); + (queue as unknown as { computeProvider: string }).computeProvider = ` ${provider.type.toUpperCase()} `; + + expectProviderSelected(queue); + }); + + if (provider.type === defaultComputeProvider) { + it('selects the default provider when the queue omits provider configuration', () => { + expectProviderSelected(runnerQueue(`${provider.type}-default`)); + }); + } + }); +} + +function runnerQueue(id: string, computeProvider?: ComputeProviderType): RunnerMatcherConfig { + return { + id, + arn: `arn:${id}`, + computeProvider, + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + }, + }; +} diff --git a/lambdas/libs/compute-providers/tsconfig.json b/lambdas/libs/compute-providers/tsconfig.json index 52d55867fe..51beb73b87 100644 --- a/lambdas/libs/compute-providers/tsconfig.json +++ b/lambdas/libs/compute-providers/tsconfig.json @@ -1,5 +1,5 @@ { "extends": "../../tsconfig.json", - "include": ["*.ts", "core/**/*", "aws/**/*", "templates/**/*"], + "include": ["*.ts", "core/**/*", "aws/**/*", "templates/**/*", "test/**/*"], "exclude": ["aws/**/*.test.ts"] } diff --git a/lambdas/libs/compute-providers/webhook.test.ts b/lambdas/libs/compute-providers/webhook.test.ts index f3120a3b59..983da7d028 100644 --- a/lambdas/libs/compute-providers/webhook.test.ts +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -1,29 +1,14 @@ import { describe, expect, it, vi } from 'vitest'; import type { DynamicLabelProvider, DynamicLabelViolation, RunnerMatcherConfig } from './contracts'; -import type { ComputeProviderType } from './provider-types'; +import { createDynamicLabelsForOtherProvider } from './dynamic-labels'; import { createDynamicLabelQueueSelector, selectDynamicLabelQueue } from './webhook'; -describe('selectDynamicLabelQueue', () => { - it('defaults queues without a provider to EC2 dynamic label handling', () => { - const queue = runnerQueue('default-ec2'); - - expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); - - it('normalizes compute provider casing and surrounding whitespace', () => { - const queue = runnerQueue('normalized-ec2'); - (queue as unknown as { computeProvider: string }).computeProvider = ' EC2 '; - - expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); +const testProviderTypes = ['alpha', 'beta'] as const; +type TestProviderType = (typeof testProviderTypes)[number]; +const dynamicLabelsForOtherProvider = createDynamicLabelsForOtherProvider(testProviderTypes); +describe('selectDynamicLabelQueue', () => { it.each([ ['unsupported string', 'unsupported-provider'], ['non-string', 42], @@ -31,13 +16,9 @@ describe('selectDynamicLabelQueue', () => { const invalidQueue = runnerQueue('invalid-provider'); (invalidQueue as unknown as { computeProvider: unknown }).computeProvider = computeProvider; - expect(() => - selectDynamicLabelQueue( - [invalidQueue, runnerQueue('valid-ec2')], - ['self-hosted', 'linux'], - ['ghr-ec2-instance-type:t3.large'], - ), - ).toThrow(`Unsupported compute provider type '${String(computeProvider)}'`); + expect(() => selectDynamicLabelQueue([invalidQueue], [], [])).toThrow( + `Unsupported compute provider type '${String(computeProvider)}'`, + ); }); }); @@ -92,31 +73,26 @@ describe('createDynamicLabelQueueSelector', () => { expect(selectQueue([queue], ['self-hosted'], ['ghr-test-size:large'])).toBeUndefined(); }); - /* TODO: Re-enable this scenario when the MicroVM provider is added. - it('skips EC2 and selects the MicroVM queue for MicroVM override labels', () => { - const ec2Queue = runnerQueue('ec2'); - const microvmQueue = runnerQueue('microvm'); - const imageVersionLabel = 'ghr-microvm-image-version:3.0'; + it('selects the queue targeted by provider-specific labels', () => { + const alphaQueue = runnerQueue('alpha'); + const betaQueue = runnerQueue('beta'); + const betaLabel = 'ghr-beta-size:large'; const { getViolations, selectQueue } = selector({ - providerByQueue: { ec2: 'ec2', microvm: 'microvm' }, - labelsForOtherProvider: (labels, provider) => - provider === 'ec2' ? labels.filter((label) => label.startsWith('ghr-microvm-')) : [], + providerByQueue: { alpha: 'alpha', beta: 'beta' }, }); - expect(selectQueue([ec2Queue, microvmQueue], ['self-hosted', 'linux'], [imageVersionLabel])).toEqual({ - queue: microvmQueue, - labels: ['self-hosted', 'linux', imageVersionLabel], + expect(selectQueue([alphaQueue, betaQueue], ['self-hosted', 'linux'], [betaLabel])).toEqual({ + queue: betaQueue, + labels: ['self-hosted', 'linux', betaLabel], }); expect(getViolations).toHaveBeenCalledOnce(); - expect(getViolations).toHaveBeenCalledWith({ queue: microvmQueue, labels: [imageVersionLabel] }); + expect(getViolations).toHaveBeenCalledWith({ queue: betaQueue, labels: [betaLabel] }); }); - */ }); function selector(options?: { - providerByQueue?: Record; + providerByQueue?: Record; violationsByQueue?: Record; - labelsForOtherProvider?: (labels: string[], provider: ComputeProviderType) => string[]; }) { const getViolations = vi.fn(({ queue }) => { return options?.violationsByQueue?.[queue.id] ?? []; @@ -124,12 +100,12 @@ function selector(options?: { return { getViolations, - selectQueue: createDynamicLabelQueueSelector({ + selectQueue: createDynamicLabelQueueSelector({ resolveProvider: (queue) => ({ - type: options?.providerByQueue?.[queue.id] ?? 'ec2', + type: options?.providerByQueue?.[queue.id] ?? 'alpha', dynamicLabels: { getViolations }, }), - dynamicLabelsForOtherProvider: options?.labelsForOtherProvider ?? (() => []), + dynamicLabelsForOtherProvider, }), }; } From 51ba155e38351ada5ad861bbf86f36dfcc11152f Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 14 Aug 2026 16:18:05 +0200 Subject: [PATCH 6/9] refactor(compute-providers): simplify provider label filtering --- .../compute-providers/dynamic-labels.test.ts | 5 ++--- .../libs/compute-providers/dynamic-labels.ts | 17 ++++++++--------- lambdas/libs/compute-providers/webhook.test.ts | 6 +++--- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/lambdas/libs/compute-providers/dynamic-labels.test.ts b/lambdas/libs/compute-providers/dynamic-labels.test.ts index e93b9fa264..88aa39689c 100644 --- a/lambdas/libs/compute-providers/dynamic-labels.test.ts +++ b/lambdas/libs/compute-providers/dynamic-labels.test.ts @@ -1,14 +1,13 @@ import { expect, it } from 'vitest'; -import { createDynamicLabelsForOtherProvider } from './dynamic-labels'; +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; const providerTypes = ['alpha', 'beta'] as const; -const dynamicLabelsForOtherProvider = createDynamicLabelsForOtherProvider(providerTypes); it.each(providerTypes)('returns labels belonging to providers other than %s', (provider) => { const providerLabels = providerTypes.map((type) => `ghr-${type}-size:large`); - expect(dynamicLabelsForOtherProvider(providerLabels, provider)).toEqual( + expect(dynamicLabelsForOtherProvider(providerLabels, provider, providerTypes)).toEqual( providerLabels.filter((label) => !label.startsWith(`ghr-${provider}-`)), ); }); diff --git a/lambdas/libs/compute-providers/dynamic-labels.ts b/lambdas/libs/compute-providers/dynamic-labels.ts index 8ac72757c8..3c72d77966 100644 --- a/lambdas/libs/compute-providers/dynamic-labels.ts +++ b/lambdas/libs/compute-providers/dynamic-labels.ts @@ -1,12 +1,11 @@ import { computeProviderTypes } from './provider-types'; -import type { ComputeProviderType } from './provider-types'; -export function createDynamicLabelsForOtherProvider(providerTypes: readonly TProvider[]) { - return (labels: string[], provider: TProvider): string[] => - labels.filter((label) => - providerTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)), - ); +export function dynamicLabelsForOtherProvider( + labels: string[], + provider: string, + providerTypes: readonly string[] = computeProviderTypes, +): string[] { + return labels.filter((label) => + providerTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)), + ); } - -export const dynamicLabelsForOtherProvider = - createDynamicLabelsForOtherProvider(computeProviderTypes); diff --git a/lambdas/libs/compute-providers/webhook.test.ts b/lambdas/libs/compute-providers/webhook.test.ts index 983da7d028..7ec7343f97 100644 --- a/lambdas/libs/compute-providers/webhook.test.ts +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -1,12 +1,11 @@ import { describe, expect, it, vi } from 'vitest'; import type { DynamicLabelProvider, DynamicLabelViolation, RunnerMatcherConfig } from './contracts'; -import { createDynamicLabelsForOtherProvider } from './dynamic-labels'; +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; import { createDynamicLabelQueueSelector, selectDynamicLabelQueue } from './webhook'; const testProviderTypes = ['alpha', 'beta'] as const; type TestProviderType = (typeof testProviderTypes)[number]; -const dynamicLabelsForOtherProvider = createDynamicLabelsForOtherProvider(testProviderTypes); describe('selectDynamicLabelQueue', () => { it.each([ @@ -105,7 +104,8 @@ function selector(options?: { type: options?.providerByQueue?.[queue.id] ?? 'alpha', dynamicLabels: { getViolations }, }), - dynamicLabelsForOtherProvider, + dynamicLabelsForOtherProvider: (labels, provider) => + dynamicLabelsForOtherProvider(labels, provider, testProviderTypes), }), }; } From 046a6caf18003163ceb75d880179d63eb9aeb4e9 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 14 Aug 2026 16:24:15 +0200 Subject: [PATCH 7/9] test(compute-providers): cover disabled dynamic labels --- .../compute-providers/test/webhook-provider-contract.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts index a970e5f10e..759329880b 100644 --- a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts +++ b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts @@ -29,6 +29,13 @@ export function defineWebhookProviderContractTests { + const queue = runnerQueue(`${provider.type}-disabled`, provider.type); + queue.matcherConfig.enableDynamicLabels = false; + + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toBeUndefined(); + }); + it('normalizes provider configuration before registry selection', () => { const queue = runnerQueue(`${provider.type}-normalized`); (queue as unknown as { computeProvider: string }).computeProvider = ` ${provider.type.toUpperCase()} `; From 9116fec00edd2ee4781ecb68e9d127b1a2d8f5da Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 14 Aug 2026 16:32:05 +0200 Subject: [PATCH 8/9] test(compute-providers): cover AWS dynamic label policy --- lambdas/libs/compute-providers/aws/ec2/webhook.test.ts | 5 +++++ .../compute-providers/test/webhook-provider-contract.ts | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts index d6557d3c88..7fa5d4ffa5 100644 --- a/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts @@ -4,4 +4,9 @@ import { provider } from './webhook'; defineWebhookProviderContractTests({ provider, acceptedDynamicLabels: ['ghr-ec2-instance-type:t3.large'], + applyRejectingPolicy: (queue) => { + queue.matcherConfig.awsDynamicLabelsPolicy = { + blocked_keys: ['instance-type'], + }; + }, }); diff --git a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts index 759329880b..50651cbfe0 100644 --- a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts +++ b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts @@ -8,11 +8,13 @@ import { selectDynamicLabelQueue } from '../webhook'; interface WebhookProviderContractOptions { provider: WebhookProviderModule; acceptedDynamicLabels: readonly [string, ...string[]]; + applyRejectingPolicy(queue: RunnerMatcherConfig): void; } export function defineWebhookProviderContractTests({ provider, acceptedDynamicLabels, + applyRejectingPolicy, }: WebhookProviderContractOptions): void { const nonGhrLabels = ['self-hosted', 'linux']; const dynamicLabels = [...acceptedDynamicLabels]; @@ -36,6 +38,13 @@ export function defineWebhookProviderContractTests { + const queue = runnerQueue(`${provider.type}-policy-rejected`, provider.type); + applyRejectingPolicy(queue); + + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toBeUndefined(); + }); + it('normalizes provider configuration before registry selection', () => { const queue = runnerQueue(`${provider.type}-normalized`); (queue as unknown as { computeProvider: string }).computeProvider = ` ${provider.type.toUpperCase()} `; From 40f69f6cd6b66ec225d45b26d30f0324409ce3e9 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 14 Aug 2026 16:38:41 +0200 Subject: [PATCH 9/9] test(compute-providers): cover restricted AWS policy --- .../compute-providers/aws/ec2/webhook.test.ts | 25 +++++++++++++++---- .../test/webhook-provider-contract.ts | 21 ++++++++++------ 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts index 7fa5d4ffa5..755831fb91 100644 --- a/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts @@ -4,9 +4,24 @@ import { provider } from './webhook'; defineWebhookProviderContractTests({ provider, acceptedDynamicLabels: ['ghr-ec2-instance-type:t3.large'], - applyRejectingPolicy: (queue) => { - queue.matcherConfig.awsDynamicLabelsPolicy = { - blocked_keys: ['instance-type'], - }; - }, + rejectingPolicies: [ + { + name: 'blocked keys', + apply: (queue) => { + queue.matcherConfig.awsDynamicLabelsPolicy = { + blocked_keys: ['instance-type'], + }; + }, + }, + { + name: 'restricted keys', + apply: (queue) => { + queue.matcherConfig.awsDynamicLabelsPolicy = { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + }, + }, + ], }); diff --git a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts index 50651cbfe0..dd4e3097b0 100644 --- a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts +++ b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts @@ -5,16 +5,21 @@ import { defaultComputeProvider } from '../provider-types'; import type { ComputeProviderType } from '../provider-types'; import { selectDynamicLabelQueue } from '../webhook'; +interface RejectingPolicyCase { + name: string; + apply(queue: RunnerMatcherConfig): void; +} + interface WebhookProviderContractOptions { provider: WebhookProviderModule; acceptedDynamicLabels: readonly [string, ...string[]]; - applyRejectingPolicy(queue: RunnerMatcherConfig): void; + rejectingPolicies: readonly [RejectingPolicyCase, ...RejectingPolicyCase[]]; } export function defineWebhookProviderContractTests({ provider, acceptedDynamicLabels, - applyRejectingPolicy, + rejectingPolicies, }: WebhookProviderContractOptions): void { const nonGhrLabels = ['self-hosted', 'linux']; const dynamicLabels = [...acceptedDynamicLabels]; @@ -38,12 +43,14 @@ export function defineWebhookProviderContractTests { - const queue = runnerQueue(`${provider.type}-policy-rejected`, provider.type); - applyRejectingPolicy(queue); + for (const policy of rejectingPolicies) { + it(`skips the provider when its ${policy.name} policy rejects the labels`, () => { + const queue = runnerQueue(`${provider.type}-policy-rejected`, provider.type); + policy.apply(queue); - expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toBeUndefined(); - }); + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toBeUndefined(); + }); + } it('normalizes provider configuration before registry selection', () => { const queue = runnerQueue(`${provider.type}-normalized`);