From 6da0fb94b4bc7b64a6d78bc886ecbdd74c346456 Mon Sep 17 00:00:00 2001 From: Timo Ulich Date: Tue, 15 Mar 2022 14:59:19 +0100 Subject: [PATCH] chore: Refactor runner config to be an array of strings (#1842) run on branch fix test --- .../lambdas/runners/src/aws/runners.test.ts | 6 +- .../lambdas/runners/src/aws/runners.ts | 4 +- .../src/scale-runners/scale-up.test.ts | 60 +++++++++++++------ .../runners/src/scale-runners/scale-up.ts | 33 ++++++---- 4 files changed, 68 insertions(+), 35 deletions(-) diff --git a/modules/runners/lambdas/runners/src/aws/runners.test.ts b/modules/runners/lambdas/runners/src/aws/runners.test.ts index e02e4f9194..38ee9e798a 100644 --- 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 index 0252a1fbb4..f5f8e449a6 100644 --- 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 index 09e143ee46..35f9ae1534 100644 --- 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,7 +320,10 @@ 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); }); @@ -325,7 +331,10 @@ describe('scaleUp with GHES', () => { 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,7 +515,10 @@ 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); }); @@ -507,7 +526,10 @@ describe('scaleUp with public GH', () => { 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 index f643c95813..c154a6214f 100644 --- 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) {