Skip to content

Commit 6d3c6ba

Browse files
edersonbrilhanteedersonbrilhante
committed
refactor(runner-providers): centralize dynamic label selection
1 parent 332eaef commit 6d3c6ba

10 files changed

Lines changed: 197 additions & 136 deletions

File tree

‎lambdas/libs/runner-providers/aws/ec2/src/webhook/dynamic-labels.test.ts‎

Lines changed: 20 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,29 @@
11
import { describe, expect, it } from 'vitest';
22

33
import type { RunnerMatcherConfig } from '../../../../contracts';
4-
import { selectEc2DynamicLabelQueue } from './dynamic-labels';
4+
import { ec2DynamicLabelProvider } from './dynamic-labels';
55

6-
describe('selectEc2DynamicLabelQueue', () => {
7-
it('rejects dynamic labels when the queue disables them', () => {
8-
const queue = runnerQueue('dynamic-labels-disabled');
9-
queue.matcherConfig.enableDynamicLabels = false;
10-
11-
expect(
12-
selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']),
13-
).toBeUndefined();
14-
});
15-
16-
it('accepts dynamic labels when the queue has no policy', () => {
6+
describe('ec2DynamicLabelProvider', () => {
7+
it('returns no violations when the queue has no policy', () => {
178
const queue = runnerQueue('no-policy');
189

19-
expect(selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({
20-
queue,
21-
labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'],
22-
});
10+
expect(getViolations(queue)).toEqual([]);
2311
});
2412

25-
it('skips a policy-rejected queue and returns the next compliant queue', () => {
13+
it('returns violations for labels rejected by the policy', () => {
2614
const strictQueue = runnerQueue('strict');
2715
strictQueue.matcherConfig.awsDynamicLabelsPolicy = {
2816
restricted_keys: {
2917
'instance-type': { allowed: ['m5.*'] },
3018
},
3119
};
32-
const permissiveQueue = runnerQueue('permissive');
3320

34-
expect(
35-
selectEc2DynamicLabelQueue(
36-
[strictQueue, permissiveQueue],
37-
['self-hosted', 'linux'],
38-
['ghr-ec2-instance-type:t3.large'],
39-
),
40-
).toEqual({
41-
queue: permissiveQueue,
42-
labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'],
43-
});
44-
});
45-
46-
it('returns undefined when no queue accepts the dynamic labels', () => {
47-
const strictQueue = runnerQueue('strict');
48-
strictQueue.matcherConfig.awsDynamicLabelsPolicy = {
49-
restricted_keys: {
50-
'instance-type': { allowed: ['m5.*'] },
21+
expect(getViolations(strictQueue)).toEqual([
22+
{
23+
label: 'ghr-ec2-instance-type:t3.large',
24+
reason: "value 't3.large' not in allowed list",
5125
},
52-
};
53-
const disabledQueue = runnerQueue('disabled');
54-
disabledQueue.matcherConfig.enableDynamicLabels = false;
55-
56-
expect(
57-
selectEc2DynamicLabelQueue(
58-
[strictQueue, disabledQueue],
59-
['self-hosted', 'linux'],
60-
['ghr-ec2-instance-type:t3.large'],
61-
),
62-
).toBeUndefined();
26+
]);
6327
});
6428

6529
it('enforces a legacy EC2 dynamic labels policy when the new key is absent', () => {
@@ -68,9 +32,7 @@ describe('selectEc2DynamicLabelQueue', () => {
6832
blocked_keys: ['instance-type'],
6933
};
7034

71-
expect(
72-
selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']),
73-
).toBeUndefined();
35+
expect(getViolations(queue)).toHaveLength(1);
7436
});
7537

7638
it('falls back to the legacy EC2 dynamic labels policy when the new policy is null', () => {
@@ -80,9 +42,7 @@ describe('selectEc2DynamicLabelQueue', () => {
8042
};
8143
queue.matcherConfig.awsDynamicLabelsPolicy = null;
8244

83-
expect(
84-
selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']),
85-
).toBeUndefined();
45+
expect(getViolations(queue)).toHaveLength(1);
8646
});
8747

8848
it('prefers a configured AWS dynamic labels policy over the legacy policy', () => {
@@ -94,13 +54,17 @@ describe('selectEc2DynamicLabelQueue', () => {
9454
blocked_keys: [],
9555
};
9656

97-
expect(selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({
98-
queue,
99-
labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'],
100-
});
57+
expect(getViolations(queue)).toEqual([]);
10158
});
10259
});
10360

61+
function getViolations(queue: RunnerMatcherConfig) {
62+
return ec2DynamicLabelProvider.getViolations({
63+
queue,
64+
labels: ['ghr-ec2-instance-type:t3.large'],
65+
});
66+
}
67+
10468
function runnerQueue(id: string): RunnerMatcherConfig {
10569
return {
10670
id,

‎lambdas/libs/runner-providers/aws/ec2/src/webhook/dynamic-labels.ts‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import { createChildLogger } from '@aws-github-runner/aws-powertools-util';
22

3-
import type { DynamicLabelDispatchTarget, DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts';
3+
import type { DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts';
44
import { violationsAgainstPolicy } from './dynamic-labels-policy';
55

66
const logger = createChildLogger('handler');
77

8-
export type Ec2DynamicLabelDispatchTarget = DynamicLabelDispatchTarget;
9-
108
function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) {
119
const hasLegacyEc2DynamicLabelsPolicy = Object.prototype.hasOwnProperty.call(
1210
queue.matcherConfig,
@@ -23,36 +21,6 @@ function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) {
2321
return queue.matcherConfig.awsDynamicLabelsPolicy;
2422
}
2523

26-
export function selectEc2DynamicLabelQueue(
27-
matches: RunnerMatcherConfig[],
28-
nonGhrLabels: string[],
29-
sanitizedGhrLabels: string[],
30-
): Ec2DynamicLabelDispatchTarget | undefined {
31-
for (const queue of matches) {
32-
if (!queue.matcherConfig.enableDynamicLabels) {
33-
logger.warn(`Queue ${queue.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`);
34-
continue;
35-
}
36-
37-
const violations = violationsAgainstPolicy(sanitizedGhrLabels, resolveEc2DynamicLabelsPolicy(queue));
38-
if (violations.length === 0) {
39-
return {
40-
queue,
41-
labels: [...nonGhrLabels, ...sanitizedGhrLabels],
42-
};
43-
}
44-
45-
for (const violation of violations) {
46-
logger.warn(
47-
`Queue ${queue.id}: dynamic label '${violation.label}' does not match policy (${violation.reason}); trying next match`,
48-
);
49-
}
50-
}
51-
52-
return undefined;
53-
}
54-
5524
export const ec2DynamicLabelProvider: DynamicLabelProvider = {
56-
selectQueue: ({ queue, nonGhrLabels, sanitizedGhrLabels }) =>
57-
selectEc2DynamicLabelQueue([queue], nonGhrLabels, sanitizedGhrLabels),
25+
getViolations: ({ queue, labels }) => violationsAgainstPolicy(labels, resolveEc2DynamicLabelsPolicy(queue)),
5826
};

‎lambdas/libs/runner-providers/contracts.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,13 @@ export interface DynamicLabelDispatchTarget {
4343
labels: string[];
4444
}
4545

46+
export interface DynamicLabelViolation {
47+
label: string;
48+
reason: string;
49+
}
50+
4651
export interface DynamicLabelProvider {
47-
selectQueue(input: {
48-
queue: RunnerMatcherConfig;
49-
nonGhrLabels: string[];
50-
sanitizedGhrLabels: string[];
51-
}): DynamicLabelDispatchTarget | undefined;
52+
getViolations(input: { queue: RunnerMatcherConfig; labels: string[] }): DynamicLabelViolation[];
5253
}
5354

5455
export interface ControlPlaneProviderCapabilities {

‎lambdas/libs/runner-providers/dynamic-labels.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { expect, it } from 'vitest';
2+
3+
import { dynamicLabelsForOtherProvider } from './dynamic-labels';
4+
import { runnerProviderTypes } from './provider-types';
5+
6+
it.each(runnerProviderTypes)('returns labels belonging to providers other than %s', (provider) => {
7+
const providerLabels = runnerProviderTypes.map((type) => `ghr-${type}-size:large`);
8+
9+
expect(dynamicLabelsForOtherProvider(providerLabels, provider)).toEqual(
10+
providerLabels.filter((label) => !label.startsWith(`ghr-${provider}-`)),
11+
);
12+
});

‎lambdas/libs/runner-providers/dynamic-labels.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { runnerProviderTypes } from './provider-types';
2+
import type { RunnerProviderType } from './provider-types';
3+
4+
export function dynamicLabelsForOtherProvider(labels: string[], provider: RunnerProviderType): string[] {
5+
return labels.filter((label) =>
6+
runnerProviderTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)),
7+
);
8+
}

‎lambdas/libs/runner-providers/registry.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,6 @@ it('exposes every configured provider through both capability registries', () =>
3333
unmarkOrphan: expect.any(Function),
3434
terminate: expect.any(Function),
3535
});
36-
expect(webhookProviderRegistry.capability(type, 'dynamicLabels').selectQueue).toEqual(expect.any(Function));
36+
expect(webhookProviderRegistry.capability(type, 'dynamicLabels').getViolations).toEqual(expect.any(Function));
3737
}
3838
});

‎lambdas/libs/runner-providers/templates/provider/provider.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,5 @@ it('exposes every runner provider capability from its lane entry point', () => {
2929
terminate: expect.any(Function),
3030
});
3131
expect(webhookPlugin.type).toBe(webhookProvider.type);
32-
expect(webhookPlugin.capabilities.dynamicLabels.selectQueue).toEqual(expect.any(Function));
32+
expect(webhookPlugin.capabilities.dynamicLabels.getViolations).toEqual(expect.any(Function));
3333
});

‎lambdas/libs/runner-providers/templates/provider/webhook.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ import type { RunnerProviderPlugin } from '../../core';
33
import type { DynamicLabelProvider, WebhookProviderCapabilities, WebhookProviderModule } from '../../contracts';
44

55
export const templateDynamicLabelProvider: DynamicLabelProvider = {
6-
selectQueue: (input) => {
6+
getViolations: (input) => {
77
void input;
8-
// Return a dispatch target when this provider accepts the requested dynamic labels.
9-
return undefined;
8+
// Return violations for dynamic labels this provider does not accept.
9+
return [];
1010
},
1111
};
1212

‎lambdas/libs/runner-providers/webhook.test.ts‎

Lines changed: 86 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,107 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi } from 'vitest';
22

3-
import type { RunnerMatcherConfig } from './contracts';
4-
import type { RunnerProviderType } from './provider-types';
5-
import { selectDynamicLabelQueue } from './webhook';
3+
import type { DynamicLabelProvider, DynamicLabelViolation, RunnerMatcherConfig } from './contracts';
4+
import { createDynamicLabelQueueSelector } from './webhook';
65

7-
describe('selectDynamicLabelQueue', () => {
8-
it('defaults queues without a provider to EC2 dynamic label handling', () => {
9-
const queue = runnerQueue('default-ec2');
6+
type TestProvider = 'ec2' | 'microvm';
107

11-
expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({
8+
describe('createDynamicLabelQueueSelector', () => {
9+
it('returns the first queue accepted by its provider', () => {
10+
const queue = runnerQueue('accepted');
11+
const { selectQueue } = selector();
12+
13+
expect(selectQueue([queue], ['self-hosted', 'linux'], ['ghr-test-size:large'])).toEqual({
1214
queue,
13-
labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'],
15+
labels: ['self-hosted', 'linux', 'ghr-test-size:large'],
1416
});
1517
});
1618

17-
it('normalizes runner provider casing and surrounding whitespace', () => {
18-
const queue = runnerQueue('normalized-ec2');
19-
(queue as unknown as { runnerProvider: string }).runnerProvider = ' EC2 ';
19+
it('skips queues that disable dynamic labels', () => {
20+
const disabledQueue = runnerQueue('disabled');
21+
disabledQueue.matcherConfig.enableDynamicLabels = false;
22+
const enabledQueue = runnerQueue('enabled');
23+
const { getViolations, selectQueue } = selector();
2024

21-
expect(selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({
22-
queue,
23-
labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'],
25+
expect(selectQueue([disabledQueue, enabledQueue], ['self-hosted'], ['ghr-test-size:large'])).toEqual({
26+
queue: enabledQueue,
27+
labels: ['self-hosted', 'ghr-test-size:large'],
28+
});
29+
expect(getViolations).toHaveBeenCalledOnce();
30+
expect(getViolations).toHaveBeenCalledWith({ queue: enabledQueue, labels: ['ghr-test-size:large'] });
31+
});
32+
33+
it('skips queues whose provider reports violations', () => {
34+
const rejectedQueue = runnerQueue('rejected');
35+
const acceptedQueue = runnerQueue('accepted');
36+
const { selectQueue } = selector({
37+
violationsByQueue: {
38+
rejected: [{ label: 'ghr-test-size:large', reason: 'size is unavailable' }],
39+
},
40+
});
41+
42+
expect(selectQueue([rejectedQueue, acceptedQueue], ['self-hosted'], ['ghr-test-size:large'])).toEqual({
43+
queue: acceptedQueue,
44+
labels: ['self-hosted', 'ghr-test-size:large'],
2445
});
2546
});
2647

27-
it.each([['unsupported'], [42]])('throws for unsupported runner provider %j', (runnerProvider) => {
28-
const queue = runnerQueue('unsupported-provider');
29-
(queue as unknown as { runnerProvider: unknown }).runnerProvider = runnerProvider;
48+
it('returns undefined when every provider reports violations', () => {
49+
const queue = runnerQueue('rejected');
50+
const { selectQueue } = selector({
51+
violationsByQueue: {
52+
rejected: [{ label: 'ghr-test-size:large', reason: 'size is unavailable' }],
53+
},
54+
});
3055

31-
expect(() =>
32-
selectDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']),
33-
).toThrow(`Unsupported runner provider type '${String(runnerProvider)}'`);
56+
expect(selectQueue([queue], ['self-hosted'], ['ghr-test-size:large'])).toBeUndefined();
3457
});
58+
59+
/* TODO: Re-enable this scenario when the MicroVM provider is added.
60+
it('skips EC2 and selects the MicroVM queue for MicroVM override labels', () => {
61+
const ec2Queue = runnerQueue('ec2');
62+
const microvmQueue = runnerQueue('microvm');
63+
const imageVersionLabel = 'ghr-microvm-image-version:3.0';
64+
const { getViolations, selectQueue } = selector({
65+
providerByQueue: { ec2: 'ec2', microvm: 'microvm' },
66+
labelsForOtherProvider: (labels, provider) =>
67+
provider === 'ec2' ? labels.filter((label) => label.startsWith('ghr-microvm-')) : [],
68+
});
69+
70+
expect(selectQueue([ec2Queue, microvmQueue], ['self-hosted', 'linux'], [imageVersionLabel])).toEqual({
71+
queue: microvmQueue,
72+
labels: ['self-hosted', 'linux', imageVersionLabel],
73+
});
74+
expect(getViolations).toHaveBeenCalledOnce();
75+
expect(getViolations).toHaveBeenCalledWith({ queue: microvmQueue, labels: [imageVersionLabel] });
76+
});
77+
*/
3578
});
3679

37-
function runnerQueue(id: string, runnerProvider?: RunnerProviderType): RunnerMatcherConfig {
80+
function selector(options?: {
81+
providerByQueue?: Record<string, TestProvider>;
82+
violationsByQueue?: Record<string, DynamicLabelViolation[]>;
83+
labelsForOtherProvider?: (labels: string[], provider: TestProvider) => string[];
84+
}) {
85+
const getViolations = vi.fn<DynamicLabelProvider['getViolations']>(({ queue }) => {
86+
return options?.violationsByQueue?.[queue.id] ?? [];
87+
});
88+
89+
return {
90+
getViolations,
91+
selectQueue: createDynamicLabelQueueSelector<TestProvider>({
92+
resolveProvider: (queue) => ({
93+
type: options?.providerByQueue?.[queue.id] ?? 'ec2',
94+
dynamicLabels: { getViolations },
95+
}),
96+
dynamicLabelsForOtherProvider: options?.labelsForOtherProvider ?? (() => []),
97+
}),
98+
};
99+
}
100+
101+
function runnerQueue(id: string): RunnerMatcherConfig {
38102
return {
39103
id,
40104
arn: `arn:${id}`,
41-
runnerProvider,
42105
matcherConfig: {
43106
labelMatchers: [['self-hosted', 'linux']],
44107
exactMatch: true,

0 commit comments

Comments
 (0)