Skip to content

Commit 10576a3

Browse files
committed
address comments
1 parent e0c518f commit 10576a3

File tree

2 files changed

+14
-21
lines changed

2 files changed

+14
-21
lines changed

lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,6 @@ const TEST_DATA: scaleUpModule.ActionRequestMessage = {
5656
repoOwnerType: "Organization",
5757
};
5858

59-
const TEST_DATA_USER_REPO: scaleUpModule.ActionRequestMessage = {
60-
id: 1,
61-
eventType: 'workflow_job',
62-
repositoryName: 'hello-world',
63-
repositoryOwner: 'Octocat',
64-
installationId: 2,
65-
repoOwnerType: "User",
66-
};
67-
6859
// installationId 0 means no installationId is set.
6960
const TEST_DATA_WITH_ZERO_INSTALL_ID: scaleUpModule.ActionRequestMessage = {
7061
id: 3,
@@ -316,9 +307,12 @@ describe('scaleUp with GHES', () => {
316307
expect(mockOctokit.paginate).toHaveBeenCalledTimes(1);
317308
});
318309

319-
it('Throws error if it is a User repo and org level runners is enabled', () => {
310+
it('Discards event if it is a User repo and org level runners is enabled', async () => {
320311
process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
321-
expect(() => scaleUpModule.scaleUp('aws:sqs', TEST_DATA_USER_REPO)).rejects.toBeInstanceOf(Error);
312+
const USER_REPO_TEST_DATA = { ...TEST_DATA };
313+
USER_REPO_TEST_DATA.repoOwnerType = 'User';
314+
await scaleUpModule.scaleUp('aws:sqs', USER_REPO_TEST_DATA);
315+
expect(createRunner).not.toHaveBeenCalled();
322316
});
323317

324318
it('create SSM parameter for runner group id if it doesnt exist', async () => {

lambdas/functions/control-plane/src/scale-runners/scale-up.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,12 @@ export async function scaleUp(eventSource: string, payload: ActionRequestMessage
252252
);
253253
}
254254

255-
validateRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel);
255+
if (!isValidRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel)) {
256+
logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
257+
`organization and organization runners are enabled. This is not supported. Not scaling up for this event.` +
258+
`Not throwing error to prevent re-queueing and just ignoring the event.`);
259+
return;
260+
}
256261

257262
const ephemeral = ephemeralEnabled && payload.eventType === 'workflow_job';
258263
const runnerType = enableOrgLevel ? 'Org' : 'Repo';
@@ -345,15 +350,9 @@ async function createStartRunnerConfig(
345350
}
346351
}
347352

348-
function validateRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) {
349-
if (enableOrgLevel && payload.repoOwnerType !== 'Organization') {
350-
logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
351-
`organization and organization runners are enabled. This is not supported. Not scaling up for this event.`);
352-
throw Error(
353-
`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
354-
`organization and organization runners are enabled. This is not supported. Not scaling up for this event.`,
355-
);
356-
}
353+
function isValidRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) : boolean {
354+
return !(enableOrgLevel && payload.repoOwnerType !== 'Organization');
355+
357356
}
358357

359358
function addDelay(instances: string[]) {

0 commit comments

Comments
 (0)