chore: Refactor runner config to be an array of strings (philips-labs…

…#1842) run on branch fix test
enverus-cts · Mar 15, 2022 · bf79711 · bf79711
1 parent c88a005
commit bf79711
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 35 deletions.
diff --git a/modules/runners/lambdas/runners/src/aws/runners.test.ts b/modules/runners/lambdas/runners/src/aws/runners.test.ts
@@ -217,7 +217,7 @@ describe('create runner', () => {
       expect(mockSSM.putParameter).toBeCalledWith({
         Name: `unit-test-environment-${instance}`,
         Type: 'SecureString',
-        Value: 'bla',
+        Value: '--token foo --url http://github.com',
       });
     }
   });
@@ -247,7 +247,7 @@ describe('create runner', () => {
     await createRunner(createRunnerConfig(defaultRunnerConfig));
     expect(mockSSM.putParameter).toBeCalledWith({
       Name: `${ENVIRONMENT}-i-1234`,
-      Value: 'bla',
+      Value: '--token foo --url http://github.com',
       Type: 'SecureString',
     });
   });
@@ -358,7 +358,7 @@ interface RunnerConfig {
 
 function createRunnerConfig(runnerConfig: RunnerConfig): RunnerInputParameters {
   return {
-    runnerServiceConfig: 'bla',
+    runnerServiceConfig: ['--token foo', '--url http://github.com'],
     environment: ENVIRONMENT,
     runnerType: runnerConfig.type,
     runnerOwner: REPO_NAME,

diff --git a/modules/runners/lambdas/runners/src/aws/runners.ts b/modules/runners/lambdas/runners/src/aws/runners.ts
@@ -29,7 +29,7 @@ export interface ListRunnerFilters {
 }
 
 export interface RunnerInputParameters {
-  runnerServiceConfig: string;
+  runnerServiceConfig: string[];
   environment: string;
   runnerType: 'Org' | 'Repo';
   runnerOwner: string;
@@ -207,7 +207,7 @@ export async function createRunner(runnerParameters: RunnerInputParameters): Pro
     await ssm
       .putParameter({
         Name: `${runnerParameters.environment}-${instance}`,
-        Value: runnerParameters.runnerServiceConfig,
+        Value: runnerParameters.runnerServiceConfig.join(' '),
         Type: 'SecureString',
       })
       .promise();

diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts
@@ -53,7 +53,7 @@ const cleanEnv = process.env;
 
 const EXPECTED_RUNNER_PARAMS: RunnerInputParameters = {
   environment: 'unit-test-environment',
-  runnerServiceConfig: `--url https://github.enterprise.something/${TEST_DATA.repositoryOwner} --token 1234abcd`,
+  runnerServiceConfig: [`--url https://github.enterprise.something/${TEST_DATA.repositoryOwner}`, '--token 1234abcd'],
   runnerType: 'Org',
   runnerOwner: TEST_DATA.repositoryOwner,
   launchTemplateName: 'lt-1',
@@ -236,8 +236,11 @@ describe('scaleUp with GHES', () => {
       process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
       process.env.RUNNER_GROUP_NAME = 'TEST_GROUP';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig =
-        expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2 --runnergroup TEST_GROUP`;
+      expectedRunnerParams.runnerServiceConfig = [
+        ...expectedRunnerParams.runnerServiceConfig,
+        '--labels label1,label2',
+        '--runnergroup TEST_GROUP',
+      ];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
   });
@@ -248,10 +251,10 @@ describe('scaleUp with GHES', () => {
       expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS };
       expectedRunnerParams.runnerType = 'Repo';
       expectedRunnerParams.runnerOwner = `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`;
-      expectedRunnerParams.runnerServiceConfig =
-        `--url ` +
-        `https://github.enterprise.something/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` +
-        `--token 1234abcd`;
+      expectedRunnerParams.runnerServiceConfig = [
+        `--url https://github.enterprise.something/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
+        `--token 1234abcd`,
+      ];
     });
 
     it('gets the current repo level runners', async () => {
@@ -317,15 +320,21 @@ describe('scaleUp with GHES', () => {
     it('creates a runner with correct config and labels', async () => {
       process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
+      expectedRunnerParams.runnerServiceConfig = [
+        ...expectedRunnerParams.runnerServiceConfig,
+        `--labels label1,label2`,
+      ];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
 
     it('creates a runner and ensure the group argument is ignored', async () => {
       process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
       process.env.RUNNER_GROUP_NAME = 'TEST_GROUP_IGNORED';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
+      expectedRunnerParams.runnerServiceConfig = [
+        ...expectedRunnerParams.runnerServiceConfig,
+        `--labels label1,label2`,
+      ];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
 
@@ -393,8 +402,10 @@ describe('scaleUp with public GH', () => {
     beforeEach(() => {
       process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
       expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS };
-      expectedRunnerParams.runnerServiceConfig =
-        `--url https://github.com/${TEST_DATA.repositoryOwner} ` + `--token 1234abcd`;
+      expectedRunnerParams.runnerServiceConfig = [
+        `--url https://github.com/${TEST_DATA.repositoryOwner}`,
+        `--token 1234abcd`,
+      ];
     });
 
     it('gets the current org level runners', async () => {
@@ -435,8 +446,11 @@ describe('scaleUp with public GH', () => {
       process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
       process.env.RUNNER_GROUP_NAME = 'TEST_GROUP';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig =
-        expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2 --runnergroup TEST_GROUP`;
+      expectedRunnerParams.runnerServiceConfig = [
+        ...expectedRunnerParams.runnerServiceConfig,
+        `--labels label1,label2`,
+        `--runnergroup TEST_GROUP`,
+      ];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
   });
@@ -447,8 +461,10 @@ describe('scaleUp with public GH', () => {
       expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS };
       expectedRunnerParams.runnerType = 'Repo';
       expectedRunnerParams.runnerOwner = `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`;
-      expectedRunnerParams.runnerServiceConfig =
-        `--url https://github.com/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` + `--token 1234abcd`;
+      expectedRunnerParams.runnerServiceConfig = [
+        `--url https://github.com/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
+        `--token 1234abcd`,
+      ];
     });
 
     it('gets the current repo level runners', async () => {
@@ -499,15 +515,21 @@ describe('scaleUp with public GH', () => {
     it('creates a runner with correct config and labels', async () => {
       process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
+      expectedRunnerParams.runnerServiceConfig = [
+        ...expectedRunnerParams.runnerServiceConfig,
+        `--labels label1,label2`,
+      ];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
 
     it('creates a runner and ensure the group argument is ignored', async () => {
       process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
       process.env.RUNNER_GROUP_NAME = 'TEST_GROUP_IGNORED';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
+      expectedRunnerParams.runnerServiceConfig = [
+        ...expectedRunnerParams.runnerServiceConfig,
+        `--labels label1,label2`,
+      ];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
 
@@ -524,14 +546,14 @@ describe('scaleUp with public GH', () => {
     it('creates a ephemeral runner.', async () => {
       process.env.ENABLE_EPHEMERAL_RUNNERS = 'true';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --ephemeral`;
+      expectedRunnerParams.runnerServiceConfig = [...expectedRunnerParams.runnerServiceConfig, `--ephemeral`];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
 
     it('disable auto update on the runner.', async () => {
       process.env.DISABLE_RUNNER_AUTOUPDATE = 'true';
       await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
-      expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --disableupdate`;
+      expectedRunnerParams.runnerServiceConfig = [...expectedRunnerParams.runnerServiceConfig, `--disableupdate`];
       expect(createRunner).toBeCalledWith(expectedRunnerParams);
     });
 

diff --git a/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts b/modules/runners/lambdas/runners/src/scale-runners/scale-up.ts
@@ -35,17 +35,28 @@ interface CreateEC2RunnerConfig {
 }
 
 function generateRunnerServiceConfig(githubRunnerConfig: CreateGitHubRunnerConfig, token: string) {
-  const labelsArgument =
-    githubRunnerConfig.runnerExtraLabels !== undefined ? `--labels ${githubRunnerConfig.runnerExtraLabels} ` : '';
-  const runnerGroupArgument =
-    githubRunnerConfig.runnerGroup !== undefined ? `--runnergroup ${githubRunnerConfig.runnerGroup} ` : '';
-  const configBaseUrl = githubRunnerConfig.ghesBaseUrl ? githubRunnerConfig.ghesBaseUrl : 'https://github.com';
-  const ephemeralArgument = githubRunnerConfig.ephemeral ? '--ephemeral ' : '';
-  const disableUpdateArgument = githubRunnerConfig.disableAutoUpdate ? '--disableupdate ' : '';
-  const runnerArgs = `--token ${token} ${labelsArgument}${ephemeralArgument}${disableUpdateArgument}`;
-  return githubRunnerConfig.runnerType === 'Org'
-    ? `--url ${configBaseUrl}/${githubRunnerConfig.runnerOwner} ${runnerArgs}${runnerGroupArgument}`.trim()
-    : `--url ${configBaseUrl}/${githubRunnerConfig.runnerOwner} ${runnerArgs}`.trim();
+  const config = [
+    `--url ${githubRunnerConfig.ghesBaseUrl ?? 'https://github.com'}/${githubRunnerConfig.runnerOwner}`,
+    `--token ${token}`,
+  ];
+
+  if (githubRunnerConfig.runnerExtraLabels !== undefined) {
+    config.push(`--labels ${githubRunnerConfig.runnerExtraLabels}`);
+  }
+
+  if (githubRunnerConfig.ephemeral) {
+    config.push(`--ephemeral`);
+  }
+
+  if (githubRunnerConfig.disableAutoUpdate) {
+    config.push('--disableupdate');
+  }
+
+  if (githubRunnerConfig.runnerType === 'Org' && githubRunnerConfig.runnerGroup !== undefined) {
+    config.push(`--runnergroup ${githubRunnerConfig.runnerGroup}`);
+  }
+
+  return config;
 }
 
 async function getGithubRunnerRegistrationToken(githubRunnerConfig: CreateGitHubRunnerConfig, ghClient: Octokit) {