Skip to content

Commit 8018959

Browse files
feat: add bidirectionalLabelMatch option and deprecate exactMatch
Introduce a new bidirectionalLabelMatch option that performs strict two-way label matching (runner labels must equal workflow labels as a set). This preserves the existing exactMatch behavior (unidirectional subset check) to avoid breaking changes. The bidirectionalLabelMatch option for runner label matching requires labels to be identical in both directions. Previously, a runner with labels [A, B, C, D] would match a job requesting [A, B, C] when exactMatch was true. Now, bidirectionalLabelMatch=true requires the labels to be exactly identical - the runner will only match if the job requests exactly [A, B, C, D]. This change affects users who have runners with extra labels (e.g., on-demand) that were previously matching jobs not explicitly requesting those labels. After this change, such runners will only be used when jobs explicitly request all of the runner labels. Before: Job [A,B,C] + Runner [A,B,C,D] + exactMatch=true -> Match After: Job [A,B,C] + Runner [A,B,C,D] + exactMatch=true -> No Match ExactMatch was suppose to have this behaviour, but the avoid breaking changes, the variable will be deprecated to give users time to migrate. To migrate, use bidirectionalLabelMatch instead of exactMatch in your runner configs. Then either: 1. Remove extra labels from runner configurations, or 2. Add the extra labels to your workflow job runs-on Signed-off-by: Brend Smits <brend.smits@philips.com> Co-authored-by: Stuart Pearson <stuart.pearson@philips.com>
1 parent 07bd193 commit 8018959

File tree

12 files changed

+134
-23
lines changed

12 files changed

+134
-23
lines changed

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,11 @@ Join our discord community via [this invite link](https://discord.gg/bxgXW8jJGh)
126126
| <a name="input_enable_job_queued_check"></a> [enable\_job\_queued\_check](#input\_enable\_job\_queued\_check) | Only scale if the job event received by the scale up lambda is in the queued state. By default enabled for non ephemeral runners and disabled for ephemeral. Set this variable to overwrite the default behavior. | `bool` | `null` | no |
127127
| <a name="input_enable_managed_runner_security_group"></a> [enable\_managed\_runner\_security\_group](#input\_enable\_managed\_runner\_security\_group) | Enables creation of the default managed security group. Unmanaged security groups can be specified via `runner_additional_security_group_ids`. | `bool` | `true` | no |
128128
| <a name="input_enable_organization_runners"></a> [enable\_organization\_runners](#input\_enable\_organization\_runners) | Register runners to organization, instead of repo level | `bool` | `false` | no |
129+
| <a name="input_enable_runner_bidirectional_label_match"></a> [enable\_runner\_bidirectional\_label\_match](#input\_enable\_runner\_bidirectional\_label\_match) | If set to true, the runner labels and workflow job labels must be an exact two-way match (same set, any order, no extras or missing labels). This is stricter than `enable_runner_workflow_job_labels_check_all` which only checks that workflow labels are a subset of runner labels. When false, if __any__ label matches it will trigger the webhook. | `bool` | `false` | no |
129130
| <a name="input_enable_runner_binaries_syncer"></a> [enable\_runner\_binaries\_syncer](#input\_enable\_runner\_binaries\_syncer) | Option to disable the lambda to sync GitHub runner distribution, useful when using a pre-build AMI. | `bool` | `true` | no |
130131
| <a name="input_enable_runner_detailed_monitoring"></a> [enable\_runner\_detailed\_monitoring](#input\_enable\_runner\_detailed\_monitoring) | Should detailed monitoring be enabled for the runner. Set this to true if you want to use detailed monitoring. See https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-cloudwatch-new.html for details. | `bool` | `false` | no |
131132
| <a name="input_enable_runner_on_demand_failover_for_errors"></a> [enable\_runner\_on\_demand\_failover\_for\_errors](#input\_enable\_runner\_on\_demand\_failover\_for\_errors) | Enable on-demand failover. For example to fall back to on demand when no spot capacity is available the variable can be set to `InsufficientInstanceCapacity`. When not defined the default behavior is to retry later. | `list(string)` | `[]` | no |
132-
| <a name="input_enable_runner_workflow_job_labels_check_all"></a> [enable\_runner\_workflow\_job\_labels\_check\_all](#input\_enable\_runner\_workflow\_job\_labels\_check\_all) | If set to true all labels in the workflow job must match the GitHub labels (os, architecture and `self-hosted`). When false if __any__ label matches it will trigger the webhook. | `bool` | `true` | no |
133+
| <a name="input_enable_runner_workflow_job_labels_check_all"></a> [enable\_runner\_workflow\_job\_labels\_check\_all](#input\_enable\_runner\_workflow\_job\_labels\_check\_all) | DEPRECATED: Use `enable_runner_bidirectional_label_match` instead. If set to true all labels in the workflow job must match the GitHub labels (os, architecture and `self-hosted`). When false if __any__ label matches it will trigger the webhook. Note: this only checks that workflow labels are a subset of runner labels, not the reverse. | `bool` | `true` | no |
133134
| <a name="input_enable_ssm_on_runners"></a> [enable\_ssm\_on\_runners](#input\_enable\_ssm\_on\_runners) | Enable to allow access to the runner instances for debugging purposes via SSM. Note that this adds additional permissions to the runner instances. | `bool` | `false` | no |
134135
| <a name="input_enable_user_data_debug_logging_runner"></a> [enable\_user\_data\_debug\_logging\_runner](#input\_enable\_user\_data\_debug\_logging\_runner) | Option to enable debug logging for user-data, this logs all secrets as well. | `bool` | `false` | no |
135136
| <a name="input_enable_userdata"></a> [enable\_userdata](#input\_enable\_userdata) | Should the userdata script be enabled for the runner. Set this to false if you are using your own prebuilt AMI. | `bool` | `true` | no |

lambdas/functions/webhook/src/ConfigLoader.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ describe('ConfigLoader Tests', () => {
165165
});
166166

167167
await expect(ConfigWebhook.load()).rejects.toThrow(
168-
'Failed to load config: Failed to load parameter for matcherConfig from path /path/to/matcher/config: Failed to load matcher config', // eslint-disable-line max-len
168+
'Failed to load config: Failed to load parameter for matcherConfig from path /path/to/matcher/config: Failed to load matcher config',
169169
);
170170
});
171171

@@ -213,7 +213,7 @@ describe('ConfigLoader Tests', () => {
213213
});
214214

215215
await expect(ConfigWebhook.load()).rejects.toThrow(
216-
"Failed to load config: Failed to parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196", // eslint-disable-line max-len
216+
"Failed to load config: Failed to parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196",
217217
);
218218
});
219219
});
@@ -244,7 +244,7 @@ describe('ConfigLoader Tests', () => {
244244
});
245245

246246
await expect(ConfigWebhookEventBridge.load()).rejects.toThrow(
247-
'Failed to load config: Environment variable for eventBusName is not set and no default value provided., Failed to load parameter for webhookSecret from path undefined: Parameter undefined not found', // eslint-disable-line max-len
247+
'Failed to load config: Environment variable for eventBusName is not set and no default value provided., Failed to load parameter for webhookSecret from path undefined: Parameter undefined not found',
248248
);
249249
});
250250
});
@@ -309,7 +309,7 @@ describe('ConfigLoader Tests', () => {
309309
});
310310

311311
await expect(ConfigDispatcher.load()).rejects.toThrow(
312-
'Failed to load config: Failed to load parameter for matcherConfig from path undefined: Parameter undefined not found', // eslint-disable-line max-len
312+
'Failed to load config: Failed to load parameter for matcherConfig from path undefined: Parameter undefined not found',
313313
);
314314
});
315315

lambdas/functions/webhook/src/ConfigLoader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ abstract class BaseConfig {
6161
this.loadProperty(propertyName, value);
6262
})
6363
.catch((error) => {
64-
const errorMessage = `Failed to load parameter for ${String(propertyName)} from path ${paramPath}: ${(error as Error).message}`; // eslint-disable-line max-len
64+
const errorMessage = `Failed to load parameter for ${String(propertyName)} from path ${paramPath}: ${(error as Error).message}`;
6565
this.configLoadingErrors.push(errorMessage);
6666
});
6767
}

lambdas/functions/webhook/src/runners/dispatch.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,86 @@ describe('Dispatcher', () => {
225225
const runnerLabels = [['self-hosted', 'linux', 'x64']];
226226
expect(canRunJob(workflowLabels, runnerLabels, false)).toBe(true);
227227
});
228+
229+
it('should match when runner has more labels than workflow requests with exactMatch=true (unidirectional).', () => {
230+
const workflowLabels = ['self-hosted', 'linux', 'x64', 'staging', 'ubuntu-2404'];
231+
const runnerLabels = [['self-hosted', 'linux', 'x64', 'staging', 'ubuntu-2404', 'on-demand']];
232+
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true);
233+
});
234+
235+
it('should match when labels are exactly identical with exactMatch=true.', () => {
236+
const workflowLabels = ['self-hosted', 'linux', 'on-demand'];
237+
const runnerLabels = [['self-hosted', 'linux', 'on-demand']];
238+
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true);
239+
});
240+
241+
it('should match with exactMatch=true when labels are in different order.', () => {
242+
const workflowLabels = ['linux', 'self-hosted', 'x64'];
243+
const runnerLabels = [['self-hosted', 'linux', 'x64']];
244+
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true);
245+
});
246+
247+
it('should match with exactMatch=true when labels are completely shuffled.', () => {
248+
const workflowLabels = ['x64', 'ubuntu-latest', 'self-hosted', 'linux'];
249+
const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']];
250+
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true);
251+
});
252+
253+
it('should match with exactMatch=false when labels are in different order.', () => {
254+
const workflowLabels = ['gpu', 'self-hosted'];
255+
const runnerLabels = [['self-hosted', 'gpu']];
256+
expect(canRunJob(workflowLabels, runnerLabels, false)).toBe(true);
257+
});
258+
259+
// bidirectionalLabelMatch tests
260+
it('should NOT match when runner has more labels than workflow requests (bidirectionalLabelMatch=true).', () => {
261+
const workflowLabels = ['self-hosted', 'linux', 'x64', 'staging', 'ubuntu-2404'];
262+
const runnerLabels = [['self-hosted', 'linux', 'x64', 'staging', 'ubuntu-2404', 'on-demand']];
263+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(false);
264+
});
265+
266+
it('should NOT match when workflow has more labels than runner (bidirectionalLabelMatch=true).', () => {
267+
const workflowLabels = ['self-hosted', 'linux', 'x64', 'ubuntu-latest', 'gpu'];
268+
const runnerLabels = [['self-hosted', 'linux', 'x64']];
269+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(false);
270+
});
271+
272+
it('should match when labels are exactly identical with bidirectionalLabelMatch=true.', () => {
273+
const workflowLabels = ['self-hosted', 'linux', 'on-demand'];
274+
const runnerLabels = [['self-hosted', 'linux', 'on-demand']];
275+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(true);
276+
});
277+
278+
it('should match with bidirectionalLabelMatch=true when labels are in different order.', () => {
279+
const workflowLabels = ['linux', 'self-hosted', 'x64'];
280+
const runnerLabels = [['self-hosted', 'linux', 'x64']];
281+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(true);
282+
});
283+
284+
it('should match with bidirectionalLabelMatch=true when labels are completely shuffled.', () => {
285+
const workflowLabels = ['x64', 'ubuntu-latest', 'self-hosted', 'linux'];
286+
const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']];
287+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(true);
288+
});
289+
290+
it('should match with bidirectionalLabelMatch=true ignoring case.', () => {
291+
const workflowLabels = ['Self-Hosted', 'Linux', 'X64'];
292+
const runnerLabels = [['self-hosted', 'linux', 'x64']];
293+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(true);
294+
});
295+
296+
it('should NOT match empty workflow labels with bidirectionalLabelMatch=true.', () => {
297+
const workflowLabels: string[] = [];
298+
const runnerLabels = [['self-hosted', 'linux', 'x64']];
299+
expect(canRunJob(workflowLabels, runnerLabels, false, true)).toBe(false);
300+
});
301+
302+
it('bidirectionalLabelMatch takes precedence over exactMatch when both are true.', () => {
303+
const workflowLabels = ['self-hosted', 'linux', 'x64'];
304+
const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']];
305+
// exactMatch alone would accept this (runner has extra labels), but bidirectional should reject
306+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(false);
307+
});
228308
});
229309
});
230310

lambdas/functions/webhook/src/runners/dispatch.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,21 @@ async function handleWorkflowJob(
4242
`Job ID: ${body.workflow_job.id}, Job Name: ${body.workflow_job.name}, ` +
4343
`Run ID: ${body.workflow_job.run_id}, Labels: ${JSON.stringify(body.workflow_job.labels)}`,
4444
);
45-
// sort the queuesConfig by order of matcher config exact match, with all true matches lined up ahead.
45+
// sort the queuesConfig by order of matcher config exact/bidirectional match, with all true matches lined up ahead.
4646
matcherConfig.sort((a, b) => {
47-
return a.matcherConfig.exactMatch === b.matcherConfig.exactMatch ? 0 : a.matcherConfig.exactMatch ? -1 : 1;
47+
const aStrict = a.matcherConfig.bidirectionalLabelMatch || a.matcherConfig.exactMatch;
48+
const bStrict = b.matcherConfig.bidirectionalLabelMatch || b.matcherConfig.exactMatch;
49+
return aStrict === bStrict ? 0 : aStrict ? -1 : 1;
4850
});
4951
for (const queue of matcherConfig) {
50-
if (canRunJob(body.workflow_job.labels, queue.matcherConfig.labelMatchers, queue.matcherConfig.exactMatch)) {
52+
if (
53+
canRunJob(
54+
body.workflow_job.labels,
55+
queue.matcherConfig.labelMatchers,
56+
queue.matcherConfig.exactMatch,
57+
queue.matcherConfig.bidirectionalLabelMatch,
58+
)
59+
) {
5160
await sendActionRequest({
5261
id: body.workflow_job.id,
5362
repositoryName: body.repository.name,
@@ -80,14 +89,24 @@ export function canRunJob(
8089
workflowJobLabels: string[],
8190
runnerLabelsMatchers: string[][],
8291
workflowLabelCheckAll: boolean,
92+
bidirectionalLabelMatch = false,
8393
): boolean {
8494
runnerLabelsMatchers = runnerLabelsMatchers.map((runnerLabel) => {
8595
return runnerLabel.map((label) => label.toLowerCase());
8696
});
87-
const matchLabels = workflowLabelCheckAll
88-
? runnerLabelsMatchers.some((rl) => workflowJobLabels.every((wl) => rl.includes(wl.toLowerCase())))
89-
: runnerLabelsMatchers.some((rl) => workflowJobLabels.some((wl) => rl.includes(wl.toLowerCase())));
90-
const match = workflowJobLabels.length === 0 ? !matchLabels : matchLabels;
97+
98+
let match: boolean;
99+
if (bidirectionalLabelMatch) {
100+
const workflowLabelsLower = workflowJobLabels.map((wl) => wl.toLowerCase());
101+
match = runnerLabelsMatchers.some(
102+
(rl) => workflowLabelsLower.every((wl) => rl.includes(wl)) && rl.every((r) => workflowLabelsLower.includes(r)),
103+
);
104+
} else {
105+
const matchLabels = workflowLabelCheckAll
106+
? runnerLabelsMatchers.some((rl) => workflowJobLabels.every((wl) => rl.includes(wl.toLowerCase())))
107+
: runnerLabelsMatchers.some((rl) => workflowJobLabels.some((wl) => rl.includes(wl.toLowerCase())));
108+
match = workflowJobLabels.length === 0 ? !matchLabels : matchLabels;
109+
}
91110

92111
logger.debug(
93112
`Received workflow job event with labels: '${JSON.stringify(workflowJobLabels)}'. The event does ${

lambdas/functions/webhook/src/sqs/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export interface ActionRequestMessage {
1717
export interface MatcherConfig {
1818
labelMatchers: string[][];
1919
exactMatch: boolean;
20+
bidirectionalLabelMatch?: boolean;
2021
}
2122

2223
export type RunnerConfig = RunnerMatcherConfig[];

main.tf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ module "webhook" {
114114
matcherConfig : {
115115
labelMatchers : [local.runner_labels]
116116
exactMatch : var.enable_runner_workflow_job_labels_check_all
117+
bidirectionalLabelMatch : var.enable_runner_bidirectional_label_match
117118
}
118119
}
119120
}

modules/multi-runner/README.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

modules/multi-runner/variables.tf

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,10 @@ variable "multi_runner_config" {
181181
}), {})
182182
})
183183
matcherConfig = object({
184-
labelMatchers = list(list(string))
185-
exactMatch = optional(bool, false)
186-
priority = optional(number, 999)
184+
labelMatchers = list(list(string))
185+
exactMatch = optional(bool, false)
186+
bidirectionalLabelMatch = optional(bool, false)
187+
priority = optional(number, 999)
187188
})
188189
redrive_build_queue = optional(object({
189190
enabled = bool
@@ -252,7 +253,8 @@ variable "multi_runner_config" {
252253
}
253254
matcherConfig: {
254255
labelMatchers: "The list of list of labels supported by the runner configuration. `[[self-hosted, linux, x64, example]]`"
255-
exactMatch: "If set to true all labels in the workflow job must match the GitHub labels (os, architecture and `self-hosted`). When false if __any__ workflow label matches it will trigger the webhook."
256+
exactMatch: "DEPRECATED: Use `bidirectionalLabelMatch` instead. If set to true all labels in the workflow job must match the GitHub labels (os, architecture and `self-hosted`). When false if __any__ workflow label matches it will trigger the webhook. Note: this only checks that workflow labels are a subset of runner labels, not the reverse."
257+
bidirectionalLabelMatch: "If set to true, the runner labels and workflow job labels must be an exact two-way match (same set, any order, no extras or missing labels). This is stricter than `exactMatch` which only checks that workflow labels are a subset of runner labels. When false, if __any__ workflow label matches it will trigger the webhook."
256258
priority: "If set it defines the priority of the matcher, the matcher with the lowest priority will be evaluated first. Default is 999, allowed values 0-999."
257259
}
258260
redrive_build_queue: "Set options to attach (optional) a dead letter queue to the build queue, the queue between the webhook and the scale up lambda. You have the following options. 1. Disable by setting `enabled` to false. 2. Enable by setting `enabled` to `true`, `maxReceiveCount` to a number of max retries."

modules/webhook/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ yarn run dist
8787
| <a name="input_repository_white_list"></a> [repository\_white\_list](#input\_repository\_white\_list) | List of github repository full names (owner/repo\_name) that will be allowed to use the github app. Leave empty for no filtering. | `list(string)` | `[]` | no |
8888
| <a name="input_role_path"></a> [role\_path](#input\_role\_path) | The path that will be added to the role; if not set, the environment name will be used. | `string` | `null` | no |
8989
| <a name="input_role_permissions_boundary"></a> [role\_permissions\_boundary](#input\_role\_permissions\_boundary) | Permissions boundary that will be added to the created role for the lambda. | `string` | `null` | no |
90-
| <a name="input_runner_matcher_config"></a> [runner\_matcher\_config](#input\_runner\_matcher\_config) | SQS queue to publish accepted build events based on the runner type. When exact match is disabled the webhook accepts the event if one of the workflow job labels is part of the matcher. The priority defines the order the matchers are applied. | <pre>map(object({<br/> arn = string<br/> id = string<br/> matcherConfig = object({<br/> labelMatchers = list(list(string))<br/> exactMatch = bool<br/> priority = optional(number, 999)<br/> })<br/> }))</pre> | n/a | yes |
90+
| <a name="input_runner_matcher_config"></a> [runner\_matcher\_config](#input\_runner\_matcher\_config) | SQS queue to publish accepted build events based on the runner type. When exact match is disabled the webhook accepts the event if one of the workflow job labels is part of the matcher. The priority defines the order the matchers are applied. | <pre>map(object({<br/> arn = string<br/> id = string<br/> matcherConfig = object({<br/> labelMatchers = list(list(string))<br/> exactMatch = bool<br/> bidirectionalLabelMatch = optional(bool, false)<br/> priority = optional(number, 999)<br/> })<br/> }))</pre> | n/a | yes |
9191
| <a name="input_ssm_paths"></a> [ssm\_paths](#input\_ssm\_paths) | The root path used in SSM to store configuration and secrets. | <pre>object({<br/> root = string<br/> webhook = string<br/> })</pre> | n/a | yes |
9292
| <a name="input_tags"></a> [tags](#input\_tags) | Map of tags that will be added to created resources. By default resources will be tagged with name and environment. | `map(string)` | `{}` | no |
9393
| <a name="input_tracing_config"></a> [tracing\_config](#input\_tracing\_config) | Configuration for lambda tracing. | <pre>object({<br/> mode = optional(string, null)<br/> capture_http_requests = optional(bool, false)<br/> capture_error = optional(bool, false)<br/> })</pre> | `{}` | no |

0 commit comments

Comments
 (0)