Skip to content

Commit 7f4238f

Browse files
fix!: make exactMatch truly bidirectional for label matching
BREAKING CHANGE: The exactMatch option for runner label matching now 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, exactMatch=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 To migrate, 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 7f4238f

File tree

4 files changed

+32
-11
lines changed

4 files changed

+32
-11
lines changed

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: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,10 @@ describe('Dispatcher', () => {
190190
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true);
191191
});
192192

193-
it('should accept job with an exact match and runner supports requested capabilities.', () => {
193+
it('should NOT accept job with exact match when runner has extra labels.', () => {
194194
const workflowLabels = ['self-hosted', 'linux', 'x64'];
195195
const runnerLabels = [['self-hosted', 'linux', 'x64', 'ubuntu-latest']];
196-
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(true);
196+
expect(canRunJob(workflowLabels, runnerLabels, true)).toBe(false);
197197
});
198198

199199
it('should NOT accept job with an exact match and runner not matching requested capabilities.', () => {
@@ -225,6 +225,18 @@ describe('Dispatcher', () => {
225225
const runnerLabels = [['self-hosted', 'linux', 'x64']];
226226
expect(canRunJob(workflowLabels, runnerLabels, false)).toBe(true);
227227
});
228+
229+
it('should NOT match when runner has more labels than workflow requests (exactMatch=true).', () => {
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(false);
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+
});
228240
});
229241
});
230242

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,19 @@ export function canRunJob(
8484
runnerLabelsMatchers = runnerLabelsMatchers.map((runnerLabel) => {
8585
return runnerLabel.map((label) => label.toLowerCase());
8686
});
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;
87+
const workflowLabelsLower = workflowJobLabels.map((wl) => wl.toLowerCase());
88+
89+
let match: boolean;
90+
if (workflowLabelCheckAll) {
91+
match = runnerLabelsMatchers.some(
92+
(rl) => workflowLabelsLower.every((wl) => rl.includes(wl)) && rl.every((r) => workflowLabelsLower.includes(r)),
93+
);
94+
} else {
95+
const matchLabels = runnerLabelsMatchers.some((rl) =>
96+
workflowJobLabels.some((wl) => rl.includes(wl.toLowerCase())),
97+
);
98+
match = workflowJobLabels.length === 0 ? !matchLabels : matchLabels;
99+
}
91100

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

0 commit comments

Comments
 (0)