Skip to content

Commit bf79711

Browse files
ulichulich
authored
chore: Refactor runner config to be an array of strings (#1842)
run on branch fix test
1 parent c88a005 commit bf79711

File tree

4 files changed

+68
-35
lines changed

4 files changed

+68
-35
lines changed

‎modules/runners/lambdas/runners/src/aws/runners.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ describe('create runner', () => {
217217
expect(mockSSM.putParameter).toBeCalledWith({
218218
Name: `unit-test-environment-${instance}`,
219219
Type: 'SecureString',
220-
Value: 'bla',
220+
Value: '--token foo --url http://github.com',
221221
});
222222
}
223223
});
@@ -247,7 +247,7 @@ describe('create runner', () => {
247247
await createRunner(createRunnerConfig(defaultRunnerConfig));
248248
expect(mockSSM.putParameter).toBeCalledWith({
249249
Name: `${ENVIRONMENT}-i-1234`,
250-
Value: 'bla',
250+
Value: '--token foo --url http://github.com',
251251
Type: 'SecureString',
252252
});
253253
});
@@ -358,7 +358,7 @@ interface RunnerConfig {
358358

359359
function createRunnerConfig(runnerConfig: RunnerConfig): RunnerInputParameters {
360360
return {
361-
runnerServiceConfig: 'bla',
361+
runnerServiceConfig: ['--token foo', '--url http://github.com'],
362362
environment: ENVIRONMENT,
363363
runnerType: runnerConfig.type,
364364
runnerOwner: REPO_NAME,

‎modules/runners/lambdas/runners/src/aws/runners.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export interface ListRunnerFilters {
2929
}
3030

3131
export interface RunnerInputParameters {
32-
runnerServiceConfig: string;
32+
runnerServiceConfig: string[];
3333
environment: string;
3434
runnerType: 'Org' | 'Repo';
3535
runnerOwner: string;
@@ -207,7 +207,7 @@ export async function createRunner(runnerParameters: RunnerInputParameters): Pro
207207
await ssm
208208
.putParameter({
209209
Name: `${runnerParameters.environment}-${instance}`,
210-
Value: runnerParameters.runnerServiceConfig,
210+
Value: runnerParameters.runnerServiceConfig.join(' '),
211211
Type: 'SecureString',
212212
})
213213
.promise();

‎modules/runners/lambdas/runners/src/scale-runners/scale-up.test.ts‎

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const cleanEnv = process.env;
5353

5454
const EXPECTED_RUNNER_PARAMS: RunnerInputParameters = {
5555
environment: 'unit-test-environment',
56-
runnerServiceConfig: `--url https://github.enterprise.something/${TEST_DATA.repositoryOwner} --token 1234abcd`,
56+
runnerServiceConfig: [`--url https://github.enterprise.something/${TEST_DATA.repositoryOwner}`, '--token 1234abcd'],
5757
runnerType: 'Org',
5858
runnerOwner: TEST_DATA.repositoryOwner,
5959
launchTemplateName: 'lt-1',
@@ -236,8 +236,11 @@ describe('scaleUp with GHES', () => {
236236
process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
237237
process.env.RUNNER_GROUP_NAME = 'TEST_GROUP';
238238
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
239-
expectedRunnerParams.runnerServiceConfig =
240-
expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2 --runnergroup TEST_GROUP`;
239+
expectedRunnerParams.runnerServiceConfig = [
240+
...expectedRunnerParams.runnerServiceConfig,
241+
'--labels label1,label2',
242+
'--runnergroup TEST_GROUP',
243+
];
241244
expect(createRunner).toBeCalledWith(expectedRunnerParams);
242245
});
243246
});
@@ -248,10 +251,10 @@ describe('scaleUp with GHES', () => {
248251
expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS };
249252
expectedRunnerParams.runnerType = 'Repo';
250253
expectedRunnerParams.runnerOwner = `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`;
251-
expectedRunnerParams.runnerServiceConfig =
252-
`--url ` +
253-
`https://github.enterprise.something/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` +
254-
`--token 1234abcd`;
254+
expectedRunnerParams.runnerServiceConfig = [
255+
`--url https://github.enterprise.something/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
256+
`--token 1234abcd`,
257+
];
255258
});
256259

257260
it('gets the current repo level runners', async () => {
@@ -317,15 +320,21 @@ describe('scaleUp with GHES', () => {
317320
it('creates a runner with correct config and labels', async () => {
318321
process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
319322
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
320-
expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
323+
expectedRunnerParams.runnerServiceConfig = [
324+
...expectedRunnerParams.runnerServiceConfig,
325+
`--labels label1,label2`,
326+
];
321327
expect(createRunner).toBeCalledWith(expectedRunnerParams);
322328
});
323329

324330
it('creates a runner and ensure the group argument is ignored', async () => {
325331
process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
326332
process.env.RUNNER_GROUP_NAME = 'TEST_GROUP_IGNORED';
327333
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
328-
expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
334+
expectedRunnerParams.runnerServiceConfig = [
335+
...expectedRunnerParams.runnerServiceConfig,
336+
`--labels label1,label2`,
337+
];
329338
expect(createRunner).toBeCalledWith(expectedRunnerParams);
330339
});
331340

@@ -393,8 +402,10 @@ describe('scaleUp with public GH', () => {
393402
beforeEach(() => {
394403
process.env.ENABLE_ORGANIZATION_RUNNERS = 'true';
395404
expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS };
396-
expectedRunnerParams.runnerServiceConfig =
397-
`--url https://github.com/${TEST_DATA.repositoryOwner} ` + `--token 1234abcd`;
405+
expectedRunnerParams.runnerServiceConfig = [
406+
`--url https://github.com/${TEST_DATA.repositoryOwner}`,
407+
`--token 1234abcd`,
408+
];
398409
});
399410

400411
it('gets the current org level runners', async () => {
@@ -435,8 +446,11 @@ describe('scaleUp with public GH', () => {
435446
process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
436447
process.env.RUNNER_GROUP_NAME = 'TEST_GROUP';
437448
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
438-
expectedRunnerParams.runnerServiceConfig =
439-
expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2 --runnergroup TEST_GROUP`;
449+
expectedRunnerParams.runnerServiceConfig = [
450+
...expectedRunnerParams.runnerServiceConfig,
451+
`--labels label1,label2`,
452+
`--runnergroup TEST_GROUP`,
453+
];
440454
expect(createRunner).toBeCalledWith(expectedRunnerParams);
441455
});
442456
});
@@ -447,8 +461,10 @@ describe('scaleUp with public GH', () => {
447461
expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS };
448462
expectedRunnerParams.runnerType = 'Repo';
449463
expectedRunnerParams.runnerOwner = `${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`;
450-
expectedRunnerParams.runnerServiceConfig =
451-
`--url https://github.com/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName} ` + `--token 1234abcd`;
464+
expectedRunnerParams.runnerServiceConfig = [
465+
`--url https://github.com/${TEST_DATA.repositoryOwner}/${TEST_DATA.repositoryName}`,
466+
`--token 1234abcd`,
467+
];
452468
});
453469

454470
it('gets the current repo level runners', async () => {
@@ -499,15 +515,21 @@ describe('scaleUp with public GH', () => {
499515
it('creates a runner with correct config and labels', async () => {
500516
process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
501517
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
502-
expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
518+
expectedRunnerParams.runnerServiceConfig = [
519+
...expectedRunnerParams.runnerServiceConfig,
520+
`--labels label1,label2`,
521+
];
503522
expect(createRunner).toBeCalledWith(expectedRunnerParams);
504523
});
505524

506525
it('creates a runner and ensure the group argument is ignored', async () => {
507526
process.env.RUNNER_EXTRA_LABELS = 'label1,label2';
508527
process.env.RUNNER_GROUP_NAME = 'TEST_GROUP_IGNORED';
509528
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
510-
expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --labels label1,label2`;
529+
expectedRunnerParams.runnerServiceConfig = [
530+
...expectedRunnerParams.runnerServiceConfig,
531+
`--labels label1,label2`,
532+
];
511533
expect(createRunner).toBeCalledWith(expectedRunnerParams);
512534
});
513535

@@ -524,14 +546,14 @@ describe('scaleUp with public GH', () => {
524546
it('creates a ephemeral runner.', async () => {
525547
process.env.ENABLE_EPHEMERAL_RUNNERS = 'true';
526548
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
527-
expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --ephemeral`;
549+
expectedRunnerParams.runnerServiceConfig = [...expectedRunnerParams.runnerServiceConfig, `--ephemeral`];
528550
expect(createRunner).toBeCalledWith(expectedRunnerParams);
529551
});
530552

531553
it('disable auto update on the runner.', async () => {
532554
process.env.DISABLE_RUNNER_AUTOUPDATE = 'true';
533555
await scaleUpModule.scaleUp('aws:sqs', TEST_DATA);
534-
expectedRunnerParams.runnerServiceConfig = expectedRunnerParams.runnerServiceConfig + ` --disableupdate`;
556+
expectedRunnerParams.runnerServiceConfig = [...expectedRunnerParams.runnerServiceConfig, `--disableupdate`];
535557
expect(createRunner).toBeCalledWith(expectedRunnerParams);
536558
});
537559

‎modules/runners/lambdas/runners/src/scale-runners/scale-up.ts‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,28 @@ interface CreateEC2RunnerConfig {
3535
}
3636

3737
function generateRunnerServiceConfig(githubRunnerConfig: CreateGitHubRunnerConfig, token: string) {
38-
const labelsArgument =
39-
githubRunnerConfig.runnerExtraLabels !== undefined ? `--labels ${githubRunnerConfig.runnerExtraLabels} ` : '';
40-
const runnerGroupArgument =
41-
githubRunnerConfig.runnerGroup !== undefined ? `--runnergroup ${githubRunnerConfig.runnerGroup} ` : '';
42-
const configBaseUrl = githubRunnerConfig.ghesBaseUrl ? githubRunnerConfig.ghesBaseUrl : 'https://github.com';
43-
const ephemeralArgument = githubRunnerConfig.ephemeral ? '--ephemeral ' : '';
44-
const disableUpdateArgument = githubRunnerConfig.disableAutoUpdate ? '--disableupdate ' : '';
45-
const runnerArgs = `--token ${token} ${labelsArgument}${ephemeralArgument}${disableUpdateArgument}`;
46-
return githubRunnerConfig.runnerType === 'Org'
47-
? `--url ${configBaseUrl}/${githubRunnerConfig.runnerOwner} ${runnerArgs}${runnerGroupArgument}`.trim()
48-
: `--url ${configBaseUrl}/${githubRunnerConfig.runnerOwner} ${runnerArgs}`.trim();
38+
const config = [
39+
`--url ${githubRunnerConfig.ghesBaseUrl ?? 'https://github.com'}/${githubRunnerConfig.runnerOwner}`,
40+
`--token ${token}`,
41+
];
42+
43+
if (githubRunnerConfig.runnerExtraLabels !== undefined) {
44+
config.push(`--labels ${githubRunnerConfig.runnerExtraLabels}`);
45+
}
46+
47+
if (githubRunnerConfig.ephemeral) {
48+
config.push(`--ephemeral`);
49+
}
50+
51+
if (githubRunnerConfig.disableAutoUpdate) {
52+
config.push('--disableupdate');
53+
}
54+
55+
if (githubRunnerConfig.runnerType === 'Org' && githubRunnerConfig.runnerGroup !== undefined) {
56+
config.push(`--runnergroup ${githubRunnerConfig.runnerGroup}`);
57+
}
58+
59+
return config;
4960
}
5061

5162
async function getGithubRunnerRegistrationToken(githubRunnerConfig: CreateGitHubRunnerConfig, ghClient: Octokit) {

0 commit comments

Comments
 (0)