From 7a37d5bceab88b63b5d29fcf60a1f450ec78ab31 Mon Sep 17 00:00:00 2001
From: Pedro Goncalves <pedro.goncalves@schibsted.com>
Date: Mon, 13 May 2024 23:17:18 +0200
Subject: [PATCH 1/6] fix(lambda): Prevent scale-up lambda from starting runner
 for user repo if org level runners is enabled

---
 .../functions/control-plane/src/lambda.test.ts   |  1 +
 .../src/scale-runners/scale-up.test.ts           | 16 ++++++++++++++++
 .../control-plane/src/scale-runners/scale-up.ts  | 11 +++++++++++
 lambdas/functions/webhook/src/sqs/index.test.ts  |  1 +
 lambdas/functions/webhook/src/sqs/index.ts       |  1 +
 .../functions/webhook/src/webhook/index.test.ts  |  3 +++
 lambdas/functions/webhook/src/webhook/index.ts   |  1 +
 7 files changed, 34 insertions(+)

diff --git a/lambdas/functions/control-plane/src/lambda.test.ts b/lambdas/functions/control-plane/src/lambda.test.ts
index f4d28ccd79..fbc4002e5f 100644
--- a/lambdas/functions/control-plane/src/lambda.test.ts
+++ b/lambdas/functions/control-plane/src/lambda.test.ts
@@ -15,6 +15,7 @@ const body: ActionRequestMessage = {
   installationId: 1,
   repositoryName: 'name',
   repositoryOwner: 'owner',
+  repoOwnerType: "Organization",
 };
 
 const sqsRecord: SQSRecord = {
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
index f53a998f01..2bc1e5e0a2 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
@@ -53,6 +53,16 @@ const TEST_DATA: scaleUpModule.ActionRequestMessage = {
   repositoryName: 'hello-world',
   repositoryOwner: 'Codertocat',
   installationId: 2,
+  repoOwnerType: "Organization",
+};
+
+const TEST_DATA_USER_REPO: scaleUpModule.ActionRequestMessage = {
+  id: 1,
+  eventType: 'workflow_job',
+  repositoryName: 'hello-world',
+  repositoryOwner: 'Octocat',
+  installationId: 2,
+  repoOwnerType: "User",
 };
 
 // installationId 0 means no installationId is set.
@@ -62,6 +72,7 @@ const TEST_DATA_WITH_ZERO_INSTALL_ID: scaleUpModule.ActionRequestMessage = {
   repositoryName: 'hello-world',
   repositoryOwner: 'Codertocat',
   installationId: 0,
+  repoOwnerType: "Organization",
 };
 
 const cleanEnv = process.env;
@@ -305,6 +316,11 @@ describe('scaleUp with GHES', () => {
       expect(mockOctokit.paginate).toHaveBeenCalledTimes(1);
     });
 
+    it('Throws error if it is a User repo and org level runners is enabled', () => {
+      process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
+      expect(() => scaleUpModule.scaleUp('aws:sqs', TEST_DATA_USER_REPO)).rejects.toBeInstanceOf(Error);
+    });
+
     it('create SSM parameter for runner group id if it doesnt exist', async () => {
       mockSSMClient.on(GetParameterCommand).rejects();
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index 0c184ff673..3f404c9931 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -27,6 +27,7 @@ export interface ActionRequestMessage {
   repositoryName: string;
   repositoryOwner: string;
   installationId: number;
+  repoOwnerType: string;
 }
 
 interface CreateGitHubRunnerConfig {
@@ -250,6 +251,16 @@ export async function scaleUp(eventSource: string, payload: ActionRequestMessage
         `Please ensure you have enabled workflow_job events.`,
     );
   }
+
+  if (enableOrgLevel && payload.repoOwnerType !== 'Organization') {
+    logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
+    `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`);
+    throw Error(
+      `Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
+    `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`,
+    );
+  }
+
   const ephemeral = ephemeralEnabled && payload.eventType === 'workflow_job';
   const runnerType = enableOrgLevel ? 'Org' : 'Repo';
   const runnerOwner = enableOrgLevel ? payload.repositoryOwner : `${payload.repositoryOwner}/${payload.repositoryName}`;
diff --git a/lambdas/functions/webhook/src/sqs/index.test.ts b/lambdas/functions/webhook/src/sqs/index.test.ts
index c8b66adcb5..c138ec0b08 100644
--- a/lambdas/functions/webhook/src/sqs/index.test.ts
+++ b/lambdas/functions/webhook/src/sqs/index.test.ts
@@ -26,6 +26,7 @@ describe('Test sending message to SQS.', () => {
     repositoryOwner: 'owner',
     queueId: queueUrl,
     queueFifo: false,
+    repoOwnerType: 'Organization',
   };
 
   afterEach(() => {
diff --git a/lambdas/functions/webhook/src/sqs/index.ts b/lambdas/functions/webhook/src/sqs/index.ts
index 8a1e8e20af..0bcce77b9f 100644
--- a/lambdas/functions/webhook/src/sqs/index.ts
+++ b/lambdas/functions/webhook/src/sqs/index.ts
@@ -13,6 +13,7 @@ export interface ActionRequestMessage {
   installationId: number;
   queueId: string;
   queueFifo: boolean;
+  repoOwnerType: string;
 }
 
 export interface MatcherConfig {
diff --git a/lambdas/functions/webhook/src/webhook/index.test.ts b/lambdas/functions/webhook/src/webhook/index.test.ts
index cacbde23f5..1405b0e5c3 100644
--- a/lambdas/functions/webhook/src/webhook/index.test.ts
+++ b/lambdas/functions/webhook/src/webhook/index.test.ts
@@ -450,6 +450,7 @@ describe('handler', () => {
         installationId: 0,
         queueId: 'ubuntu-queue-id',
         queueFifo: false,
+        repoOwnerType: "Organization",
       });
     });
     it('Check webhook will accept jobs for latest labels if workflow labels are not specific', async () => {
@@ -492,6 +493,7 @@ describe('handler', () => {
         installationId: 0,
         queueId: 'ubuntu-queue-id',
         queueFifo: false,
+        repoOwnerType: "Organization",
       });
     });
   });
@@ -531,6 +533,7 @@ describe('handler', () => {
       installationId: 0,
       queueId: 'ubuntu-queue-id',
       queueFifo: false,
+      repoOwnerType: "Organization",
     });
   });
 
diff --git a/lambdas/functions/webhook/src/webhook/index.ts b/lambdas/functions/webhook/src/webhook/index.ts
index 92daf8fea2..f9406ab7b8 100644
--- a/lambdas/functions/webhook/src/webhook/index.ts
+++ b/lambdas/functions/webhook/src/webhook/index.ts
@@ -53,6 +53,7 @@ async function handleWorkflowJob(
           installationId: installationId,
           queueId: queue.id,
           queueFifo: queue.fifo,
+          repoOwnerType: body.repository.owner.type,
         });
         logger.info(`Successfully queued job for ${body.repository.full_name} to the queue ${queue.id}`);
         return { statusCode: 201 };

From 0abb2ff497a9b3c7e46717a022903f8fecf0e7be Mon Sep 17 00:00:00 2001
From: Pedro Goncalves <pedro.goncalves@schibsted.com>
Date: Mon, 13 May 2024 23:45:19 +0200
Subject: [PATCH 2/6] bug(lambda): Prevent scale-up lambda from starting runner
 for user repo if org level runners is enabled

---
 .../src/scale-runners/scale-up.ts             | 21 ++++++++++++-------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index 3f404c9931..e9149bb2e3 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -252,14 +252,7 @@ export async function scaleUp(eventSource: string, payload: ActionRequestMessage
     );
   }
 
-  if (enableOrgLevel && payload.repoOwnerType !== 'Organization') {
-    logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
-    `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`);
-    throw Error(
-      `Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
-    `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`,
-    );
-  }
+  validateRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel);
 
   const ephemeral = ephemeralEnabled && payload.eventType === 'workflow_job';
   const runnerType = enableOrgLevel ? 'Org' : 'Repo';
@@ -352,6 +345,18 @@ async function createStartRunnerConfig(
   }
 }
 
+function validateRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) {
+  if (enableOrgLevel && payload.repoOwnerType !== 'Organization') {
+    logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
+      `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`);
+    throw Error(
+      `Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
+      `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`,
+    );
+  }
+
+}
+
 function addDelay(instances: string[]) {
   const delay = async (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
   const ssmParameterStoreMaxThroughput = 40;

From e0c518ff1c2a09efb3c480cbf57174378e981078 Mon Sep 17 00:00:00 2001
From: Pedro Goncalves <pedro.goncalves@schibsted.com>
Date: Mon, 13 May 2024 23:48:33 +0200
Subject: [PATCH 3/6] bug(lambda): Prevent scale-up lambda from starting runner
 for user repo if org level runners is enabled

---
 lambdas/functions/control-plane/src/scale-runners/scale-up.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index e9149bb2e3..7e80f545b6 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -354,7 +354,6 @@ function validateRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, e
       `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`,
     );
   }
-
 }
 
 function addDelay(instances: string[]) {

From 10576a3439114f4dcd36938ab76ead4039e0afbc Mon Sep 17 00:00:00 2001
From: Pedro Goncalves <pedro.goncalves@schibsted.com>
Date: Thu, 30 May 2024 09:31:31 +0200
Subject: [PATCH 4/6] address comments

---
 .../src/scale-runners/scale-up.test.ts        | 16 +++++-----------
 .../src/scale-runners/scale-up.ts             | 19 +++++++++----------
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
index 2bc1e5e0a2..4dc2d0b140 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
@@ -56,15 +56,6 @@ const TEST_DATA: scaleUpModule.ActionRequestMessage = {
   repoOwnerType: "Organization",
 };
 
-const TEST_DATA_USER_REPO: scaleUpModule.ActionRequestMessage = {
-  id: 1,
-  eventType: 'workflow_job',
-  repositoryName: 'hello-world',
-  repositoryOwner: 'Octocat',
-  installationId: 2,
-  repoOwnerType: "User",
-};
-
 // installationId 0 means no installationId is set.
 const TEST_DATA_WITH_ZERO_INSTALL_ID: scaleUpModule.ActionRequestMessage = {
   id: 3,
@@ -316,9 +307,12 @@ describe('scaleUp with GHES', () => {
       expect(mockOctokit.paginate).toHaveBeenCalledTimes(1);
     });
 
-    it('Throws error if it is a User repo and org level runners is enabled', () => {
+    it('Discards event if it is a User repo and org level runners is enabled', async () => {
       process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
-      expect(() => scaleUpModule.scaleUp('aws:sqs', TEST_DATA_USER_REPO)).rejects.toBeInstanceOf(Error);
+      const USER_REPO_TEST_DATA = { ...TEST_DATA };
+      USER_REPO_TEST_DATA.repoOwnerType = 'User';
+      await scaleUpModule.scaleUp('aws:sqs', USER_REPO_TEST_DATA);
+      expect(createRunner).not.toHaveBeenCalled();
     });
 
     it('create SSM parameter for runner group id if it doesnt exist', async () => {
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index 7e80f545b6..b0c48337f9 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -252,7 +252,12 @@ export async function scaleUp(eventSource: string, payload: ActionRequestMessage
     );
   }
 
-  validateRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel);
+  if (!isValidRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel)) {
+    logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
+      `organization and organization runners are enabled. This is not supported. Not scaling up for this event.` +
+    `Not throwing error to prevent re-queueing and just ignoring the event.`);
+    return;
+  }
 
   const ephemeral = ephemeralEnabled && payload.eventType === 'workflow_job';
   const runnerType = enableOrgLevel ? 'Org' : 'Repo';
@@ -345,15 +350,9 @@ async function createStartRunnerConfig(
   }
 }
 
-function validateRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) {
-  if (enableOrgLevel && payload.repoOwnerType !== 'Organization') {
-    logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
-      `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`);
-    throw Error(
-      `Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
-      `organization and organization runners are enabled. This is not supported. Not scaling up for this event.`,
-    );
-  }
+function isValidRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) : boolean {
+  return !(enableOrgLevel && payload.repoOwnerType !== 'Organization');
+
 }
 
 function addDelay(instances: string[]) {

From 94bf0a1cc88606d7b9726555ee39f8a650d578ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20Gon=C3=A7alves?= <PerGon@users.noreply.github.com>
Date: Thu, 30 May 2024 09:39:56 +0200
Subject: [PATCH 5/6] Update scale-up.ts

---
 lambdas/functions/control-plane/src/scale-runners/scale-up.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index b19fc14b50..a8c56f786e 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -352,7 +352,6 @@ async function createStartRunnerConfig(
 
 function isValidRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) : boolean {
   return !(enableOrgLevel && payload.repoOwnerType !== 'Organization');
-
 }
 
 function addDelay(instances: string[]) {

From e7ceab787672d371d6570efcb9896c344dcb4d30 Mon Sep 17 00:00:00 2001
From: Pedro Goncalves <pedro.goncalves@schibsted.com>
Date: Wed, 7 Aug 2024 09:54:46 +0200
Subject: [PATCH 6/6] format code

---
 lambdas/functions/control-plane/src/lambda.test.ts     |  2 +-
 .../control-plane/src/scale-runners/scale-up.test.ts   |  4 ++--
 .../control-plane/src/scale-runners/scale-up.ts        | 10 ++++++----
 lambdas/functions/webhook/src/webhook/index.test.ts    |  6 +++---
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/lambdas/functions/control-plane/src/lambda.test.ts b/lambdas/functions/control-plane/src/lambda.test.ts
index fbc4002e5f..ab87a5c968 100644
--- a/lambdas/functions/control-plane/src/lambda.test.ts
+++ b/lambdas/functions/control-plane/src/lambda.test.ts
@@ -15,7 +15,7 @@ const body: ActionRequestMessage = {
   installationId: 1,
   repositoryName: 'name',
   repositoryOwner: 'owner',
-  repoOwnerType: "Organization",
+  repoOwnerType: 'Organization',
 };
 
 const sqsRecord: SQSRecord = {
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
index 0101ea7e47..1382052baf 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts
@@ -53,7 +53,7 @@ const TEST_DATA: scaleUpModule.ActionRequestMessage = {
   repositoryName: 'hello-world',
   repositoryOwner: 'Codertocat',
   installationId: 2,
-  repoOwnerType: "Organization",
+  repoOwnerType: 'Organization',
 };
 
 // installationId 0 means no installationId is set.
@@ -63,7 +63,7 @@ const TEST_DATA_WITH_ZERO_INSTALL_ID: scaleUpModule.ActionRequestMessage = {
   repositoryName: 'hello-world',
   repositoryOwner: 'Codertocat',
   installationId: 0,
-  repoOwnerType: "Organization",
+  repoOwnerType: 'Organization',
 };
 
 const cleanEnv = process.env;
diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
index 60f938da04..26cb538445 100644
--- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
+++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts
@@ -253,9 +253,11 @@ export async function scaleUp(eventSource: string, payload: ActionRequestMessage
   }
 
   if (!isValidRepoOwnerTypeIfOrgLevelEnabled(payload, enableOrgLevel)) {
-    logger.warn(`Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
-      `organization and organization runners are enabled. This is not supported. Not scaling up for this event.` +
-    `Not throwing error to prevent re-queueing and just ignoring the event.`);
+    logger.warn(
+      `Repository ${payload.repositoryOwner}/${payload.repositoryName} does not belong to a GitHub` +
+        `organization and organization runners are enabled. This is not supported. Not scaling up for this event.` +
+        `Not throwing error to prevent re-queueing and just ignoring the event.`,
+    );
     return;
   }
 
@@ -350,7 +352,7 @@ async function createStartRunnerConfig(
   }
 }
 
-function isValidRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean) : boolean {
+function isValidRepoOwnerTypeIfOrgLevelEnabled(payload: ActionRequestMessage, enableOrgLevel: boolean): boolean {
   return !(enableOrgLevel && payload.repoOwnerType !== 'Organization');
 }
 
diff --git a/lambdas/functions/webhook/src/webhook/index.test.ts b/lambdas/functions/webhook/src/webhook/index.test.ts
index 1405b0e5c3..85e73e3e9b 100644
--- a/lambdas/functions/webhook/src/webhook/index.test.ts
+++ b/lambdas/functions/webhook/src/webhook/index.test.ts
@@ -450,7 +450,7 @@ describe('handler', () => {
         installationId: 0,
         queueId: 'ubuntu-queue-id',
         queueFifo: false,
-        repoOwnerType: "Organization",
+        repoOwnerType: 'Organization',
       });
     });
     it('Check webhook will accept jobs for latest labels if workflow labels are not specific', async () => {
@@ -493,7 +493,7 @@ describe('handler', () => {
         installationId: 0,
         queueId: 'ubuntu-queue-id',
         queueFifo: false,
-        repoOwnerType: "Organization",
+        repoOwnerType: 'Organization',
       });
     });
   });
@@ -533,7 +533,7 @@ describe('handler', () => {
       installationId: 0,
       queueId: 'ubuntu-queue-id',
       queueFifo: false,
-      repoOwnerType: "Organization",
+      repoOwnerType: 'Organization',
     });
   });