Skip to content
This repository was archived by the owner on Jan 16, 2025. It is now read-only.

Commit 98b1560

Browse files
PerGonnpalmstuartp44
authored
fix(lambda): Prevent scale-up lambda from starting runner for user repo if org level runners is enabled (#3909)
We run a setup where we only have "Org Level Runners" (`enable_organization_runners: true`). When creating an action job in a repository that does not belong to an Organization (User level repository), that ScaleUp lambda will happily create a new VM for it. Well, this new VM in turn, will never be able to register itself in GitHub because it cannot register itself as a "Org Level Runner" as such concept does not exist at the user level. This VM will then become a "zombie" VM and will also cause issues with the ScaleDown lambda (described in `ToBeCreatedIssue`). In this PR, we will pass the "Repo Owner Type" all the way from the Webhook lambda to the ScaleUp lambda (by adding a new field in the SQS payload). Then, the ScaleUp lambda can decide to drop the request if the "Repo Owner Type" is not of the type `Organization` and `enable_organization_runners` is set to `true`. --------- Co-authored-by: Niek Palm <[email protected]> Co-authored-by: Stuart Pearson <[email protected]>
1 parent 01c1515 commit 98b1560

File tree

7 files changed

+32
-0
lines changed

7 files changed

+32
-0
lines changed

Diff for: lambdas/functions/control-plane/src/lambda.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const body: ActionRequestMessage = {
1515
installationId: 1,
1616
repositoryName: 'name',
1717
repositoryOwner: 'owner',
18+
repoOwnerType: 'Organization',
1819
};
1920

2021
const sqsRecord: SQSRecord = {

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

+10
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const TEST_DATA: scaleUpModule.ActionRequestMessage = {
5353
repositoryName: 'hello-world',
5454
repositoryOwner: 'Codertocat',
5555
installationId: 2,
56+
repoOwnerType: 'Organization',
5657
};
5758

5859
// installationId 0 means no installationId is set.
@@ -62,6 +63,7 @@ const TEST_DATA_WITH_ZERO_INSTALL_ID: scaleUpModule.ActionRequestMessage = {
6263
repositoryName: 'hello-world',
6364
repositoryOwner: 'Codertocat',
6465
installationId: 0,
66+
repoOwnerType: 'Organization',
6567
};
6668

6769
const cleanEnv = process.env;
@@ -305,6 +307,14 @@ describe('scaleUp with GHES', () => {
305307
expect(mockOctokit.paginate).toHaveBeenCalledTimes(1);
306308
});
307309

310+
it('Discards event if it is a User repo and org level runners is enabled', async () => {
311+
process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
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();
316+
});
317+
308318
it('create SSM parameter for runner group id if it doesnt exist', async () => {
309319
mockSSMClient.on(GetParameterCommand).rejects();
310320
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);

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

+15
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export interface ActionRequestMessage {
2727
repositoryName: string;
2828
repositoryOwner: string;
2929
installationId: number;
30+
repoOwnerType: string;
3031
}
3132

3233
interface CreateGitHubRunnerConfig {
@@ -250,6 +251,16 @@ export async function scaleUp(eventSource: string, payload: ActionRequestMessage
250251
`Please ensure you have enabled workflow_job events.`,
251252
);
252253
}
254+
255+
if (!isValidRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel)) {
256+
logger.warn(
257+
`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
258+
`organization and organization runners are enabled. This is not supported. Not scaling up for this event.` +
259+
`Not throwing error to prevent re-queueing and just ignoring the event.`,
260+
);
261+
return;
262+
}
263+
253264
const ephemeral = ephemeralEnabled && payload.eventType === 'workflow_job';
254265
const runnerType = enableOrgLevel ? 'Org' : 'Repo';
255266
const runnerOwner = enableOrgLevel ? payload.repositoryOwner : `${payload.repositoryOwner}/${payload.repositoryName}`;
@@ -341,6 +352,10 @@ async function createStartRunnerConfig(
341352
}
342353
}
343354

355+
function isValidRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean): boolean {
356+
return !(enableOrgLevel && payload.repoOwnerType !== 'Organization');
357+
}
358+
344359
function addDelay(instances: string[]) {
345360
const delay = async (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
346361
const ssmParameterStoreMaxThroughput = 40;

Diff for: lambdas/functions/webhook/src/sqs/index.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('Test sending message to SQS.', () => {
2828
repositoryOwner: 'owner',
2929
queueId: queueUrl,
3030
queueFifo: false,
31+
repoOwnerType: 'Organization',
3132
};
3233

3334
afterEach(() => {

Diff for: lambdas/functions/webhook/src/sqs/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface ActionRequestMessage {
1313
installationId: number;
1414
queueId: string;
1515
queueFifo: boolean;
16+
repoOwnerType: string;
1617
}
1718

1819
export interface MatcherConfig {

Diff for: lambdas/functions/webhook/src/webhook/index.test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,7 @@ describe('handler', () => {
450450
installationId: 0,
451451
queueId: 'ubuntu-queue-id',
452452
queueFifo: false,
453+
repoOwnerType: 'Organization',
453454
});
454455
});
455456
it('Check webhook will accept jobs for latest labels if workflow labels are not specific', async () => {
@@ -492,6 +493,7 @@ describe('handler', () => {
492493
installationId: 0,
493494
queueId: 'ubuntu-queue-id',
494495
queueFifo: false,
496+
repoOwnerType: 'Organization',
495497
});
496498
});
497499
});
@@ -531,6 +533,7 @@ describe('handler', () => {
531533
installationId: 0,
532534
queueId: 'ubuntu-queue-id',
533535
queueFifo: false,
536+
repoOwnerType: 'Organization',
534537
});
535538
});
536539

Diff for: lambdas/functions/webhook/src/webhook/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ async function handleWorkflowJob(
5353
installationId: installationId,
5454
queueId: queue.id,
5555
queueFifo: queue.fifo,
56+
repoOwnerType: body.repository.owner.type,
5657
});
5758
logger.info(`Successfully queued job for ${body.repository.full_name} to the queue ${queue.id}`);
5859
return { statusCode: 201 };

0 commit comments

Comments
 (0)