From 3a1992abd73a75c311a4df2f5015ce6d7b76e628 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Tue, 18 Aug 2026 22:24:00 +0200 Subject: [PATCH 1/5] refactor(storage): extract runner config store --- lambdas/functions/control-plane/package.json | 1 + .../functions/control-plane/src/modules.d.ts | 1 - .../src/pool/pool-contract.test.ts | 3 + .../control-plane/src/pool/pool.test.ts | 13 +++ .../functions/control-plane/src/pool/pool.ts | 4 +- .../src/scale-runners/github-runner.ts | 50 ++++++----- .../scale-runners/scale-up-contract.test.ts | 3 + .../src/scale-runners/scale-up.test.ts | 18 +++- .../src/scale-runners/scale-up.ts | 4 +- .../ec2/src/control-plane/runner-creation.ts | 5 +- .../ec2/src/control-plane/scale-up.test.ts | 3 +- lambdas/libs/compute-providers/core/index.ts | 3 +- .../aws/ssm/environment.d.ts | 10 +++ .../aws/ssm/parameter-store-tags.ts | 42 +++++++++ .../aws/ssm/runner-config-store.test.ts | 88 +++++++++++++++++++ .../aws/ssm/runner-config-store.ts | 37 ++++++++ lambdas/libs/storage-providers/core/index.ts | 14 +++ .../libs/storage-providers/environment.d.ts | 9 ++ lambdas/libs/storage-providers/index.ts | 2 + lambdas/libs/storage-providers/package.json | 29 ++++++ .../storage-providers/runner-config.test.ts | 84 ++++++++++++++++++ .../libs/storage-providers/runner-config.ts | 46 ++++++++++ lambdas/libs/storage-providers/tsconfig.json | 5 ++ .../libs/storage-providers/vitest.config.ts | 14 +++ lambdas/yarn.lock | 9 ++ 25 files changed, 465 insertions(+), 32 deletions(-) create mode 100644 lambdas/libs/storage-providers/aws/ssm/environment.d.ts create mode 100644 lambdas/libs/storage-providers/aws/ssm/parameter-store-tags.ts create mode 100644 lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts create mode 100644 lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts create mode 100644 lambdas/libs/storage-providers/core/index.ts create mode 100644 lambdas/libs/storage-providers/environment.d.ts create mode 100644 lambdas/libs/storage-providers/index.ts create mode 100644 lambdas/libs/storage-providers/package.json create mode 100644 lambdas/libs/storage-providers/runner-config.test.ts create mode 100644 lambdas/libs/storage-providers/runner-config.ts create mode 100644 lambdas/libs/storage-providers/tsconfig.json create mode 100644 lambdas/libs/storage-providers/vitest.config.ts diff --git a/lambdas/functions/control-plane/package.json b/lambdas/functions/control-plane/package.json index dc74e770a9..0f443fc849 100644 --- a/lambdas/functions/control-plane/package.json +++ b/lambdas/functions/control-plane/package.json @@ -33,6 +33,7 @@ "@aws-github-runner/aws-powertools-util": "*", "@aws-github-runner/aws-ssm-util": "*", "@aws-github-runner/compute-providers": "*", + "@aws-github-runner/storage-providers": "*", "@aws-lambda-powertools/parameters": "^2.31.0", "@aws-sdk/client-ec2": "^3.1009.0", "@aws-sdk/client-sqs": "^3.1009.0", diff --git a/lambdas/functions/control-plane/src/modules.d.ts b/lambdas/functions/control-plane/src/modules.d.ts index d5157ccb37..84b0d23a02 100644 --- a/lambdas/functions/control-plane/src/modules.d.ts +++ b/lambdas/functions/control-plane/src/modules.d.ts @@ -18,7 +18,6 @@ declare namespace NodeJS { RUNNER_OWNER: string; COMPUTE_PROVIDER_TYPE?: string; SCALE_DOWN_CONFIG: string; - SSM_TOKEN_PATH: string; SSM_CLEANUP_CONFIG: string; SUBNET_IDS: string; INSTANCE_TYPES: string; diff --git a/lambdas/functions/control-plane/src/pool/pool-contract.test.ts b/lambdas/functions/control-plane/src/pool/pool-contract.test.ts index e519e412e4..28e38c6a77 100644 --- a/lambdas/functions/control-plane/src/pool/pool-contract.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool-contract.test.ts @@ -1,5 +1,6 @@ import type { Octokit } from '@octokit/rest'; import type { ComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; +import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, vi } from 'vitest'; import { definePoolContractTests } from '../test/compute-provider-contracts/pool'; @@ -48,6 +49,8 @@ const computeProviders = providerTypes.map((type) => ({ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; + process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; + resetRunnerConfigStore(); mockedAppAuth.mockResolvedValue({ type: 'app', token: 'app-token', appId: 1, expiresAt: 'some-date' }); mockedInstallationAuth.mockResolvedValue({ diff --git a/lambdas/functions/control-plane/src/pool/pool.test.ts b/lambdas/functions/control-plane/src/pool/pool.test.ts index 8372ab6403..f8041c2fe9 100644 --- a/lambdas/functions/control-plane/src/pool/pool.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool.test.ts @@ -1,5 +1,6 @@ import type { Octokit } from '@octokit/rest'; import { defaultComputeProvider } from '@aws-github-runner/compute-providers/provider-types'; +import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import * as ghAuth from '../github/auth'; @@ -90,6 +91,7 @@ const githubRunnersRegistered = [ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; + resetRunnerConfigStore(); process.env.RUNNERS_MAXIMUM_COUNT = '-1'; process.env.ENVIRONMENT = 'unit-test-environment'; process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; @@ -166,6 +168,17 @@ describe('pool adjustment', () => { expect(poolProvider.createRunners).not.toHaveBeenCalled(); }); + + it('rejects an unsupported runner config store before GitHub or runner lookups', async () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'unsupported-provider'; + + await expect(adjust({ poolSize: 10 })).rejects.toThrow( + "Unsupported runner config storage provider 'unsupported-provider'", + ); + + expect(mockedAppAuth).not.toHaveBeenCalled(); + expect(poolProvider.listRunners).not.toHaveBeenCalled(); + }); }); describe('With GHES', () => { diff --git a/lambdas/functions/control-plane/src/pool/pool.ts b/lambdas/functions/control-plane/src/pool/pool.ts index da5d2ee9b1..ab7f5a7c94 100644 --- a/lambdas/functions/control-plane/src/pool/pool.ts +++ b/lambdas/functions/control-plane/src/pool/pool.ts @@ -1,6 +1,7 @@ import { Octokit } from '@octokit/rest'; import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { resolveComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; +import { getRunnerConfigStore } from '@aws-github-runner/storage-providers'; import yn from 'yn'; import { @@ -31,7 +32,6 @@ export async function adjust(event: PoolEvent): Promise { const runnerGroup = process.env.RUNNER_GROUP_NAME || ''; const runnerNamePrefix = process.env.RUNNER_NAME_PREFIX || ''; const environment = process.env.ENVIRONMENT; - const ssmTokenPath = process.env.SSM_TOKEN_PATH; const ssmConfigPath = process.env.SSM_CONFIG_PATH || ''; const ephemeral = yn(process.env.ENABLE_EPHEMERAL_RUNNERS, { default: false }); const enableJitConfig = yn(process.env.ENABLE_JIT_CONFIG, { default: ephemeral }); @@ -41,6 +41,7 @@ export async function adjust(event: PoolEvent): Promise { process.env.SSM_PARAMETER_STORE_TAGS && process.env.SSM_PARAMETER_STORE_TAGS.trim() !== '' ? validateSsmParameterStoreTags(process.env.SSM_PARAMETER_STORE_TAGS) : []; + getRunnerConfigStore(); // -1 disables the maximum check, matching the scale-up lambda's semantics. Defaults to unlimited // when unset so the pool keeps its previous behavior on stacks that do not provide the variable. const maximumRunners = parseInt(process.env.RUNNERS_MAXIMUM_COUNT || '-1'); @@ -103,7 +104,6 @@ export async function adjust(event: PoolEvent): Promise { runnerNamePrefix, runnerType: 'Org', disableAutoUpdate: disableAutoUpdate, - ssmTokenPath, ssmConfigPath, ssmParameterStoreTags, }, diff --git a/lambdas/functions/control-plane/src/scale-runners/github-runner.ts b/lambdas/functions/control-plane/src/scale-runners/github-runner.ts index 745c66770a..79c78608dc 100644 --- a/lambdas/functions/control-plane/src/scale-runners/github-runner.ts +++ b/lambdas/functions/control-plane/src/scale-runners/github-runner.ts @@ -1,5 +1,10 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { getParameter, putParameter } from '@aws-github-runner/aws-ssm-util'; +import { + getRunnerConfigStore, + type RunnerConfigMetadataTag, + type RunnerConfigStore, +} from '@aws-github-runner/storage-providers'; import { Octokit } from '@octokit/rest'; import { getStoredInstallationId } from '../github/auth'; @@ -14,7 +19,7 @@ export interface GitHubRunnerMetadata { } export interface StartRunnerConfigOptions { - getSsmParameterTags?: (runnerId: string) => { Key: string; Value: string }[]; + getRunnerConfigMetadataTags?: (runnerId: string) => RunnerConfigMetadataTag[]; onJitConfigCreated?: (runnerId: string, metadata: GitHubRunnerMetadata) => Promise; } @@ -250,18 +255,20 @@ export async function createStartRunnerConfig( ghClient: Octokit, options: StartRunnerConfigOptions = {}, ): Promise { + const runnerConfigStore = getRunnerConfigStore(); if (githubRunnerConfig.enableJitConfig && githubRunnerConfig.ephemeral) { - return await createJitConfig(githubRunnerConfig, runnerIds, ghClient, options); + return await createJitConfig(githubRunnerConfig, runnerIds, ghClient, runnerConfigStore, options); } else { - return await createRegistrationTokenConfig(githubRunnerConfig, runnerIds, ghClient, options); + return await createRegistrationTokenConfig(githubRunnerConfig, runnerIds, ghClient, runnerConfigStore, options); } } -function addDelay(runnerIds: string[]) { +function addDelay(runnerIds: string[], runnerConfigStore: RunnerConfigStore) { const delay = async (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - const ssmParameterStoreMaxThroughput = 40; - const isDelay = runnerIds.length >= ssmParameterStoreMaxThroughput; - return { isDelay, delay }; + const maxWritesPerSecond = runnerConfigStore.maxWritesPerSecond; + const isDelay = maxWritesPerSecond !== undefined && runnerIds.length >= maxWritesPerSecond; + const delayMilliseconds = maxWritesPerSecond === undefined ? 0 : 1000 / maxWritesPerSecond; + return { isDelay, delay, delayMilliseconds }; } /** @@ -273,9 +280,10 @@ async function createRegistrationTokenConfig( githubRunnerConfig: CreateGitHubRunnerConfig, runnerIds: string[], ghClient: Octokit, + runnerConfigStore: RunnerConfigStore, options: StartRunnerConfigOptions, ): Promise { - const { isDelay, delay } = addDelay(runnerIds); + const { isDelay, delay, delayMilliseconds } = addDelay(runnerIds, runnerConfigStore); const token = await getGithubRunnerRegistrationToken(githubRunnerConfig, ghClient); const runnerServiceConfig = generateRunnerServiceConfig(githubRunnerConfig, token); @@ -284,12 +292,13 @@ async function createRegistrationTokenConfig( }); for (const runnerId of runnerIds) { - await putParameter(`${githubRunnerConfig.ssmTokenPath}/${runnerId}`, runnerServiceConfig.join(' '), true, { - tags: [...(options.getSsmParameterTags?.(runnerId) ?? []), ...githubRunnerConfig.ssmParameterStoreTags], - }); + await runnerConfigStore.create( + { runnerId, value: runnerServiceConfig.join(' ') }, + { metadataTags: options.getRunnerConfigMetadataTags?.(runnerId) }, + ); if (isDelay) { - // Delay to prevent AWS ssm rate limits by being within the max throughput limit - await delay(25); + // Delay to stay within the selected store's maximum write throughput. + await delay(delayMilliseconds); } } @@ -306,10 +315,11 @@ async function createJitConfig( githubRunnerConfig: CreateGitHubRunnerConfig, runnerIds: string[], ghClient: Octokit, + runnerConfigStore: RunnerConfigStore, options: StartRunnerConfigOptions, ): Promise { const runnerGroupId = await getRunnerGroupId(githubRunnerConfig, ghClient); - const { isDelay, delay } = addDelay(runnerIds); + const { isDelay, delay, delayMilliseconds } = addDelay(runnerIds, runnerConfigStore); const runnerLabels = githubRunnerConfig.runnerLabels.split(','); const failedRunnerIds: string[] = []; @@ -347,16 +357,16 @@ async function createJitConfig( runnerLabels, }); - // store jit config in ssm parameter store logger.debug('Runner JIT config for ephemeral runner generated.', { instance: runnerId, }); - await putParameter(`${githubRunnerConfig.ssmTokenPath}/${runnerId}`, runnerConfig.data.encoded_jit_config, true, { - tags: [...(options.getSsmParameterTags?.(runnerId) ?? []), ...githubRunnerConfig.ssmParameterStoreTags], - }); + await runnerConfigStore.create( + { runnerId, value: runnerConfig.data.encoded_jit_config }, + { metadataTags: options.getRunnerConfigMetadataTags?.(runnerId) }, + ); if (isDelay) { - // Delay to prevent AWS ssm rate limits by being within the max throughput limit - await delay(25); + // Delay to stay within the selected store's maximum write throughput. + await delay(delayMilliseconds); } } catch (error) { failedRunnerIds.push(runnerId); diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts index 3c1a0362bb..5d190f3e5f 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts @@ -1,4 +1,5 @@ import type { Octokit } from '@octokit/rest'; +import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, vi } from 'vitest'; import { providerTypes } from '../test/compute-provider-contracts/provider-types'; @@ -56,6 +57,8 @@ const computeProviders = providerTypes.map((type) => ({ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; + process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; + resetRunnerConfigStore(); mockedAppAuth.mockResolvedValue({ type: 'app', token: 'app-token', appId: 1, expiresAt: 'some-date' }); mockedInstallationAuth.mockResolvedValue({ 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 b218fe83c6..dee25440bf 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 @@ -18,6 +18,7 @@ import type { } from './types'; import { defaultComputeProvider } from '@aws-github-runner/compute-providers/provider-types'; import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { Octokit } from '@octokit/rest'; @@ -147,6 +148,7 @@ function setDefaults() { process.env.GITHUB_APP_CLIENT_SECRET = 'TEST_CLIENT_SECRET'; process.env.RUNNERS_MAXIMUM_COUNT = '3'; process.env.ENVIRONMENT = EXPECTED_RUNNER_PARAMS.environment; + process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; } async function createTestProviderRunners(input: CreateScaleUpRunnersInput): Promise { @@ -168,7 +170,7 @@ async function createTestProviderRunners(input: CreateScaleUpRunnersInput [{ Key: 'RunnerId', Value: runnerId }], + getRunnerConfigMetadataTags: (runnerId) => [{ key: 'RunnerId', value: runnerId }], }, ); } catch { @@ -187,6 +189,7 @@ beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); setDefaults(); + resetRunnerConfigStore(); defaultSSMGetParameterMockImpl(); defaultOctokitMockImpl(); @@ -2166,6 +2169,19 @@ describe('compute provider selection', () => { }); }); +describe('runner config store preflight', () => { + it('rejects an unsupported store before resolving compute or GitHub providers', async () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'unsupported-provider'; + + await expect(scaleUpModule.scaleUp(TEST_DATA)).rejects.toThrow( + "Unsupported runner config storage provider 'unsupported-provider'", + ); + + expect(mockedResolveCapability).not.toHaveBeenCalled(); + expect(mockedAppAuth).not.toHaveBeenCalled(); + }); +}); + describe('Multi-app round-robin', () => { const mockedGetAppCount = vi.mocked(ghAuth.getAppCount); const mockedGetStoredInstallationId = vi.mocked(ghAuth.getStoredInstallationId); 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 44d522a1f0..a33a8c9705 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts @@ -1,5 +1,6 @@ import { addPersistentContextToChildLogger, createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { resolveComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; +import { getRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { Octokit } from '@octokit/rest'; import yn from 'yn'; @@ -80,7 +81,6 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise [{ Key: 'InstanceId', Value: instanceId }], - onJitConfigCreated: async (instanceId, metadata) => await tagEc2RunnerMetadata(ec2Operations, instanceId, metadata), + getRunnerConfigMetadataTags: (instanceId) => [{ key: 'InstanceId', value: instanceId }], + onJitConfigCreated: async (instanceId, metadata) => + await tagEc2RunnerMetadata(ec2Operations, instanceId, metadata), }; } diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts index 22785e0268..f69782611b 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts @@ -51,7 +51,6 @@ function runnerConfig(overrides: Partial = {}): Create runnerOwner, runnerType: 'Org', disableAutoUpdate: false, - ssmTokenPath: '/github-action-runners/default/runners/config', ssmConfigPath: '/github-action-runners/default/runners/config', ssmParameterStoreTags: [], ...overrides, @@ -175,7 +174,7 @@ describe('scaleUp with GHES', () => { { Key: 'ghr:runner_labels', Value: 'label1,label2' }, ]); const [, , , options] = mockCreateStartRunnerConfig.mock.calls[0]; - expect(options?.getSsmParameterTags?.('i-12345')).toEqual([{ Key: 'InstanceId', Value: 'i-12345' }]); + expect(options?.getRunnerConfigMetadataTags?.('i-12345')).toEqual([{ key: 'InstanceId', value: 'i-12345' }]); }); it('chunks comma-joined GitHub runner labels by the EC2 tag value max length', async () => { diff --git a/lambdas/libs/compute-providers/core/index.ts b/lambdas/libs/compute-providers/core/index.ts index c5560942fa..edfefb15d2 100644 --- a/lambdas/libs/compute-providers/core/index.ts +++ b/lambdas/libs/compute-providers/core/index.ts @@ -21,7 +21,6 @@ export interface CreateGitHubRunnerConfig { runnerOwner: string; runnerType: RunnerType; disableAutoUpdate: boolean; - ssmTokenPath: string; ssmConfigPath: string; ssmParameterStoreTags: { Key: string; Value: string }[]; } @@ -32,7 +31,7 @@ export interface GitHubRunnerMetadata { } export interface StartRunnerConfigOptions { - getSsmParameterTags?: (runnerId: string) => { Key: string; Value: string }[]; + getRunnerConfigMetadataTags?: (runnerId: string) => { key: string; value: string }[]; onJitConfigCreated?: (runnerId: string, metadata: GitHubRunnerMetadata) => Promise; } diff --git a/lambdas/libs/storage-providers/aws/ssm/environment.d.ts b/lambdas/libs/storage-providers/aws/ssm/environment.d.ts new file mode 100644 index 0000000000..c6dd725742 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/environment.d.ts @@ -0,0 +1,10 @@ +export {}; + +declare global { + namespace NodeJS { + interface ProcessEnv { + SSM_PARAMETER_STORE_TAGS?: string; + SSM_TOKEN_PATH?: string; + } + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/parameter-store-tags.ts b/lambdas/libs/storage-providers/aws/ssm/parameter-store-tags.ts new file mode 100644 index 0000000000..d35150e10a --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/parameter-store-tags.ts @@ -0,0 +1,42 @@ +interface SsmParameterStoreTag { + Key: string; + Value: string; +} + +export function loadSsmParameterStoreTagsFromEnvironment(): SsmParameterStoreTag[] { + return process.env.SSM_PARAMETER_STORE_TAGS && process.env.SSM_PARAMETER_STORE_TAGS.trim() !== '' + ? validateSsmParameterStoreTags(process.env.SSM_PARAMETER_STORE_TAGS) + : []; +} + +function validateSsmParameterStoreTags(tagsJson: string): SsmParameterStoreTag[] { + try { + const tags: unknown = JSON.parse(tagsJson); + + if (!Array.isArray(tags)) { + throw new Error('Tags must be an array'); + } + + if (tags.length === 0) { + return []; + } + + tags.forEach((tag: unknown, index: number) => { + if (typeof tag !== 'object' || tag === null) { + throw new Error(`Tag at index ${index} must be an object`); + } + + const candidate = tag as Record; + if (!candidate.Key || typeof candidate.Key !== 'string' || candidate.Key.trim() === '') { + throw new Error(`Tag at index ${index} has missing or invalid 'Key' property`); + } + if (!Object.prototype.hasOwnProperty.call(candidate, 'Value') || typeof candidate.Value !== 'string') { + throw new Error(`Tag at index ${index} has missing or invalid 'Value' property`); + } + }); + + return tags as SsmParameterStoreTag[]; + } catch (error) { + throw new Error(`Failed to parse SSM_PARAMETER_STORE_TAGS: ${(error as Error).message}`); + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts new file mode 100644 index 0000000000..eaacff2718 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts @@ -0,0 +1,88 @@ +import { putParameter } from '@aws-github-runner/aws-ssm-util'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmRunnerConfigStore } from './runner-config-store'; + +vi.mock('@aws-github-runner/aws-ssm-util', () => ({ + putParameter: vi.fn(), +})); + +const putParameterMock = vi.mocked(putParameter); +const cleanEnv = process.env; + +describe('aws_ssm runner config store', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.SSM_PARAMETER_STORE_TAGS; + process.env.SSM_TOKEN_PATH = '/runner/tokens'; + }); + + it('creates a secure parameter at the legacy path with metadata tags before configured tags', async () => { + process.env.SSM_PARAMETER_STORE_TAGS = JSON.stringify([ + { Key: 'Environment', Value: 'test' }, + { Key: 'Team', Value: 'actions' }, + ]); + const store = createAwsSsmRunnerConfigStore(); + + await store.create( + { runnerId: 'i-123', value: 'encoded-jit-config' }, + { metadataTags: [{ key: 'InstanceId', value: 'i-123' }] }, + ); + + expect(store.maxWritesPerSecond).toBe(40); + expect(putParameterMock).toHaveBeenCalledWith('/runner/tokens/i-123', 'encoded-jit-config', true, { + tags: [ + { Key: 'InstanceId', Value: 'i-123' }, + { Key: 'Environment', Value: 'test' }, + { Key: 'Team', Value: 'actions' }, + ], + }); + }); + + it('uses an empty tag list when no tags are configured', async () => { + const store = createAwsSsmRunnerConfigStore(); + + await store.create({ runnerId: 'runner-1', value: 'registration-config' }); + + expect(putParameterMock).toHaveBeenCalledWith('/runner/tokens/runner-1', 'registration-config', true, { + tags: [], + }); + }); + + it.each([undefined, '', ' '])('rejects missing or blank SSM_TOKEN_PATH %j before writing', (tokenPath) => { + setTokenPath(tokenPath); + + expect(() => createAwsSsmRunnerConfigStore()).toThrow('Environment variable SSM_TOKEN_PATH is not set'); + expect(putParameterMock).not.toHaveBeenCalled(); + }); + + it.each([ + ['{}', 'Tags must be an array'], + ['[null]', 'Tag at index 0 must be an object'], + [JSON.stringify([{ Key: '', Value: 'test' }]), "Tag at index 0 has missing or invalid 'Key' property"], + [JSON.stringify([{ Key: 'Environment' }]), "Tag at index 0 has missing or invalid 'Value' property"], + ])('rejects invalid legacy SSM parameter tags', (tags, reason) => { + process.env.SSM_PARAMETER_STORE_TAGS = tags; + + expect(() => createAwsSsmRunnerConfigStore()).toThrow(`Failed to parse SSM_PARAMETER_STORE_TAGS: ${reason}`); + expect(putParameterMock).not.toHaveBeenCalled(); + }); + + it('treats a blank legacy tag value as no configured tags', async () => { + process.env.SSM_PARAMETER_STORE_TAGS = ' '; + const store = createAwsSsmRunnerConfigStore(); + + await store.create({ runnerId: 'runner-1', value: 'jit-config' }); + + expect(putParameterMock).toHaveBeenCalledWith('/runner/tokens/runner-1', 'jit-config', true, { tags: [] }); + }); +}); + +function setTokenPath(tokenPath: string | undefined): void { + if (tokenPath === undefined) { + delete process.env.SSM_TOKEN_PATH; + } else { + process.env.SSM_TOKEN_PATH = tokenPath; + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts new file mode 100644 index 0000000000..179bcfa87c --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts @@ -0,0 +1,37 @@ +import { putParameter } from '@aws-github-runner/aws-ssm-util'; + +import type { RunnerConfigMetadataTag, RunnerConfigRecord, RunnerConfigStore } from '../../core'; +import type {} from './environment'; +import { loadSsmParameterStoreTagsFromEnvironment } from './parameter-store-tags'; + +interface AwsSsmRunnerConfigStoreConfig { + tokenPath: string; + parameterStoreTags: { Key: string; Value: string }[]; +} + +export function createAwsSsmRunnerConfigStore(): RunnerConfigStore { + const tokenPath = process.env.SSM_TOKEN_PATH; + if (!tokenPath || tokenPath.trim() === '') { + throw new Error('Environment variable SSM_TOKEN_PATH is not set'); + } + + return new AwsSsmRunnerConfigStore({ + tokenPath, + parameterStoreTags: loadSsmParameterStoreTagsFromEnvironment(), + }); +} + +class AwsSsmRunnerConfigStore implements RunnerConfigStore { + readonly maxWritesPerSecond = 40; + + constructor(private readonly config: AwsSsmRunnerConfigStoreConfig) {} + + async create(record: RunnerConfigRecord, options: { metadataTags?: RunnerConfigMetadataTag[] } = {}): Promise { + await putParameter(`${this.config.tokenPath}/${record.runnerId}`, record.value, true, { + tags: [ + ...(options.metadataTags ?? []).map(({ key, value }) => ({ Key: key, Value: value })), + ...this.config.parameterStoreTags, + ], + }); + } +} diff --git a/lambdas/libs/storage-providers/core/index.ts b/lambdas/libs/storage-providers/core/index.ts new file mode 100644 index 0000000000..7f9df413c0 --- /dev/null +++ b/lambdas/libs/storage-providers/core/index.ts @@ -0,0 +1,14 @@ +export interface RunnerConfigMetadataTag { + key: string; + value: string; +} + +export interface RunnerConfigRecord { + runnerId: string; + value: string; +} + +export interface RunnerConfigStore { + readonly maxWritesPerSecond?: number; + create(record: RunnerConfigRecord, options?: { metadataTags?: RunnerConfigMetadataTag[] }): Promise; +} diff --git a/lambdas/libs/storage-providers/environment.d.ts b/lambdas/libs/storage-providers/environment.d.ts new file mode 100644 index 0000000000..0f7ade9095 --- /dev/null +++ b/lambdas/libs/storage-providers/environment.d.ts @@ -0,0 +1,9 @@ +export {}; + +declare global { + namespace NodeJS { + interface ProcessEnv { + RUNNER_CONFIG_STORAGE_PROVIDER?: string; + } + } +} diff --git a/lambdas/libs/storage-providers/index.ts b/lambdas/libs/storage-providers/index.ts new file mode 100644 index 0000000000..862924118b --- /dev/null +++ b/lambdas/libs/storage-providers/index.ts @@ -0,0 +1,2 @@ +export type { RunnerConfigMetadataTag, RunnerConfigRecord, RunnerConfigStore } from './core'; +export { getRunnerConfigStore, resetRunnerConfigStore } from './runner-config'; diff --git a/lambdas/libs/storage-providers/package.json b/lambdas/libs/storage-providers/package.json new file mode 100644 index 0000000000..2b753fc436 --- /dev/null +++ b/lambdas/libs/storage-providers/package.json @@ -0,0 +1,29 @@ +{ + "name": "@aws-github-runner/storage-providers", + "version": "1.0.0", + "main": "index.ts", + "exports": { + ".": "./index.ts" + }, + "type": "module", + "license": "MIT", + "scripts": { + "test": "NODE_ENV=test nx test", + "test:watch": "NODE_ENV=test nx test --watch", + "lint": "eslint .", + "format": "prettier --write \"**/*.ts\"", + "format-check": "prettier --check \"**/*.ts\"", + "all": "yarn format && yarn lint && yarn test" + }, + "dependencies": { + "@aws-github-runner/aws-ssm-util": "*" + }, + "nx": { + "includedScripts": [ + "format", + "format-check", + "lint", + "all" + ] + } +} diff --git a/lambdas/libs/storage-providers/runner-config.test.ts b/lambdas/libs/storage-providers/runner-config.test.ts new file mode 100644 index 0000000000..10467ebdb4 --- /dev/null +++ b/lambdas/libs/storage-providers/runner-config.test.ts @@ -0,0 +1,84 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmRunnerConfigStore } from './aws/ssm/runner-config-store'; +import type { RunnerConfigStore } from './core'; +import { getRunnerConfigStore, resetRunnerConfigStore } from './runner-config'; + +vi.mock('./aws/ssm/runner-config-store', () => ({ + createAwsSsmRunnerConfigStore: vi.fn(), +})); + +const createAwsSsmRunnerConfigStoreMock = vi.mocked(createAwsSsmRunnerConfigStore); +const cleanEnv = process.env; + +describe('runner config store selection', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; + resetRunnerConfigStore(); + }); + + it.each([undefined, '', ' '])('uses aws_ssm for default selector input %j', (provider) => { + setProvider(provider); + const store = stubStore(); + + expect(getRunnerConfigStore()).toBe(store); + expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledOnce(); + }); + + it.each(['aws_ssm', ' AWS_SSM '])('uses aws_ssm for explicit selector input %j', (provider) => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = provider; + const store = stubStore(); + + expect(getRunnerConfigStore()).toBe(store); + expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledOnce(); + }); + + it('rejects an unsupported provider on first use', () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + + expect(createAwsSsmRunnerConfigStoreMock).not.toHaveBeenCalled(); + expect(() => getRunnerConfigStore()).toThrow("Unsupported runner config storage provider 'not-registered'"); + expect(createAwsSsmRunnerConfigStoreMock).not.toHaveBeenCalled(); + }); + + it('selects lazily and caches the created store', () => { + const store = stubStore(); + + expect(createAwsSsmRunnerConfigStoreMock).not.toHaveBeenCalled(); + const first = getRunnerConfigStore(); + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + const second = getRunnerConfigStore(); + + expect(first).toBe(store); + expect(second).toBe(store); + expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledOnce(); + }); + + it('selects again after the test reset', () => { + const firstStore = stubStore(); + expect(getRunnerConfigStore()).toBe(firstStore); + + const secondStore = { create: vi.fn() } satisfies RunnerConfigStore; + createAwsSsmRunnerConfigStoreMock.mockReturnValue(secondStore); + resetRunnerConfigStore(); + + expect(getRunnerConfigStore()).toBe(secondStore); + expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledTimes(2); + }); +}); + +function setProvider(provider: string | undefined): void { + if (provider === undefined) { + delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; + } else { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = provider; + } +} + +function stubStore(): RunnerConfigStore { + const store = { create: vi.fn() } satisfies RunnerConfigStore; + createAwsSsmRunnerConfigStoreMock.mockReturnValue(store); + return store; +} diff --git a/lambdas/libs/storage-providers/runner-config.ts b/lambdas/libs/storage-providers/runner-config.ts new file mode 100644 index 0000000000..dfac2fcfb1 --- /dev/null +++ b/lambdas/libs/storage-providers/runner-config.ts @@ -0,0 +1,46 @@ +import { createAwsSsmRunnerConfigStore } from './aws/ssm/runner-config-store'; +import type { RunnerConfigStore } from './core'; +import type {} from './environment'; + +type RunnerConfigStoreFactory = () => RunnerConfigStore; + +const providerFactories = { + aws_ssm: createAwsSsmRunnerConfigStore, +} as const satisfies Record; + +type RunnerConfigStorageProvider = keyof typeof providerFactories; + +const defaultProvider = 'aws_ssm' satisfies RunnerConfigStorageProvider; + +let runnerConfigStore: RunnerConfigStore | undefined; + +export function getRunnerConfigStore(): RunnerConfigStore { + runnerConfigStore ??= providerFactories[resolveProvider(process.env.RUNNER_CONFIG_STORAGE_PROVIDER)](); + return runnerConfigStore; +} + +// Test-only reset for cases that need to exercise first-use environment selection. +export function resetRunnerConfigStore(): void { + runnerConfigStore = undefined; +} + +function resolveProvider(provider: unknown): RunnerConfigStorageProvider { + if (provider === undefined) { + return defaultProvider; + } + + if (typeof provider !== 'string') { + throw new Error(`Unsupported runner config storage provider '${String(provider)}'`); + } + + const normalizedProvider = provider.trim().toLowerCase(); + if (normalizedProvider === '') { + return defaultProvider; + } + + if (!Object.prototype.hasOwnProperty.call(providerFactories, normalizedProvider)) { + throw new Error(`Unsupported runner config storage provider '${String(provider)}'`); + } + + return normalizedProvider as RunnerConfigStorageProvider; +} diff --git a/lambdas/libs/storage-providers/tsconfig.json b/lambdas/libs/storage-providers/tsconfig.json new file mode 100644 index 0000000000..139069a7cf --- /dev/null +++ b/lambdas/libs/storage-providers/tsconfig.json @@ -0,0 +1,5 @@ +{ + "extends": "../../tsconfig.json", + "include": ["**/*.ts"], + "exclude": ["**/*.test.ts", "vitest.config.ts"] +} diff --git a/lambdas/libs/storage-providers/vitest.config.ts b/lambdas/libs/storage-providers/vitest.config.ts new file mode 100644 index 0000000000..a5812ad13e --- /dev/null +++ b/lambdas/libs/storage-providers/vitest.config.ts @@ -0,0 +1,14 @@ +import { resolve } from 'path'; + +import { mergeConfig } from 'vitest/config'; +import defaultConfig from '../../vitest.base.config'; + +export default mergeConfig(defaultConfig, { + test: { + setupFiles: [resolve(__dirname, '../../aws-vitest-setup.ts')], + coverage: { + include: ['index.ts', 'runner-config.ts', 'core/**/*.ts', 'aws/**/*.ts'], + exclude: ['**/*.test.ts', '**/*.d.ts'], + }, + }, +}); diff --git a/lambdas/yarn.lock b/lambdas/yarn.lock index 811757d346..ce58f39907 100644 --- a/lambdas/yarn.lock +++ b/lambdas/yarn.lock @@ -154,6 +154,7 @@ __metadata: "@aws-github-runner/aws-powertools-util": "npm:*" "@aws-github-runner/aws-ssm-util": "npm:*" "@aws-github-runner/compute-providers": "npm:*" + "@aws-github-runner/storage-providers": "npm:*" "@aws-lambda-powertools/parameters": "npm:^2.31.0" "@aws-sdk/client-ec2": "npm:^3.1009.0" "@aws-sdk/client-sqs": "npm:^3.1009.0" @@ -199,6 +200,14 @@ __metadata: languageName: unknown linkType: soft +"@aws-github-runner/storage-providers@npm:*, @aws-github-runner/storage-providers@workspace:libs/storage-providers": + version: 0.0.0-use.local + resolution: "@aws-github-runner/storage-providers@workspace:libs/storage-providers" + dependencies: + "@aws-github-runner/aws-ssm-util": "npm:*" + languageName: unknown + linkType: soft + "@aws-github-runner/termination-watcher@workspace:functions/termination-watcher": version: 0.0.0-use.local resolution: "@aws-github-runner/termination-watcher@workspace:functions/termination-watcher" From 2bbceef9ff3d092f2eb943a694efb52118151443 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 4 Sep 2026 11:58:42 +0200 Subject: [PATCH 2/5] refactor(storage): make runner config store provider-neutral --- .../src/pool/pool-contract.test.ts | 2 - .../control-plane/src/pool/pool.test.ts | 2 - .../functions/control-plane/src/pool/pool.ts | 2 - .../src/scale-runners/github-runner.ts | 12 +-- .../scale-runners/scale-up-contract.test.ts | 2 - .../src/scale-runners/scale-up.test.ts | 4 +- .../src/scale-runners/scale-up.ts | 2 - .../ec2/src/control-plane/runner-creation.ts | 2 +- .../ec2/src/control-plane/scale-up.test.ts | 2 +- lambdas/libs/compute-providers/core/index.ts | 2 +- .../aws/ssm/runner-config-store.test.ts | 2 +- .../aws/ssm/runner-config-store.ts | 6 +- lambdas/libs/storage-providers/core/index.ts | 4 +- lambdas/libs/storage-providers/index.ts | 4 +- .../storage-providers/runner-config.test.ts | 82 ++----------------- .../libs/storage-providers/runner-config.ts | 43 +--------- 16 files changed, 28 insertions(+), 145 deletions(-) diff --git a/lambdas/functions/control-plane/src/pool/pool-contract.test.ts b/lambdas/functions/control-plane/src/pool/pool-contract.test.ts index 28e38c6a77..1a1e88a7f6 100644 --- a/lambdas/functions/control-plane/src/pool/pool-contract.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool-contract.test.ts @@ -1,6 +1,5 @@ import type { Octokit } from '@octokit/rest'; import type { ComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; -import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, vi } from 'vitest'; import { definePoolContractTests } from '../test/compute-provider-contracts/pool'; @@ -50,7 +49,6 @@ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; - resetRunnerConfigStore(); mockedAppAuth.mockResolvedValue({ type: 'app', token: 'app-token', appId: 1, expiresAt: 'some-date' }); mockedInstallationAuth.mockResolvedValue({ diff --git a/lambdas/functions/control-plane/src/pool/pool.test.ts b/lambdas/functions/control-plane/src/pool/pool.test.ts index f8041c2fe9..60ac89b239 100644 --- a/lambdas/functions/control-plane/src/pool/pool.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool.test.ts @@ -1,6 +1,5 @@ import type { Octokit } from '@octokit/rest'; import { defaultComputeProvider } from '@aws-github-runner/compute-providers/provider-types'; -import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import * as ghAuth from '../github/auth'; @@ -91,7 +90,6 @@ const githubRunnersRegistered = [ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; - resetRunnerConfigStore(); process.env.RUNNERS_MAXIMUM_COUNT = '-1'; process.env.ENVIRONMENT = 'unit-test-environment'; process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; diff --git a/lambdas/functions/control-plane/src/pool/pool.ts b/lambdas/functions/control-plane/src/pool/pool.ts index ab7f5a7c94..029c494bac 100644 --- a/lambdas/functions/control-plane/src/pool/pool.ts +++ b/lambdas/functions/control-plane/src/pool/pool.ts @@ -1,7 +1,6 @@ import { Octokit } from '@octokit/rest'; import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { resolveComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; -import { getRunnerConfigStore } from '@aws-github-runner/storage-providers'; import yn from 'yn'; import { @@ -41,7 +40,6 @@ export async function adjust(event: PoolEvent): Promise { process.env.SSM_PARAMETER_STORE_TAGS && process.env.SSM_PARAMETER_STORE_TAGS.trim() !== '' ? validateSsmParameterStoreTags(process.env.SSM_PARAMETER_STORE_TAGS) : []; - getRunnerConfigStore(); // -1 disables the maximum check, matching the scale-up lambda's semantics. Defaults to unlimited // when unset so the pool keeps its previous behavior on stacks that do not provide the variable. const maximumRunners = parseInt(process.env.RUNNERS_MAXIMUM_COUNT || '-1'); diff --git a/lambdas/functions/control-plane/src/scale-runners/github-runner.ts b/lambdas/functions/control-plane/src/scale-runners/github-runner.ts index 79c78608dc..7e5f96dc1a 100644 --- a/lambdas/functions/control-plane/src/scale-runners/github-runner.ts +++ b/lambdas/functions/control-plane/src/scale-runners/github-runner.ts @@ -1,8 +1,8 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { getParameter, putParameter } from '@aws-github-runner/aws-ssm-util'; import { - getRunnerConfigStore, - type RunnerConfigMetadataTag, + createRunnerConfigStore, + type RunnerConfigMetadata, type RunnerConfigStore, } from '@aws-github-runner/storage-providers'; import { Octokit } from '@octokit/rest'; @@ -19,7 +19,7 @@ export interface GitHubRunnerMetadata { } export interface StartRunnerConfigOptions { - getRunnerConfigMetadataTags?: (runnerId: string) => RunnerConfigMetadataTag[]; + getRunnerConfigMetadata?: (runnerId: string) => RunnerConfigMetadata[]; onJitConfigCreated?: (runnerId: string, metadata: GitHubRunnerMetadata) => Promise; } @@ -255,7 +255,7 @@ export async function createStartRunnerConfig( ghClient: Octokit, options: StartRunnerConfigOptions = {}, ): Promise { - const runnerConfigStore = getRunnerConfigStore(); + const runnerConfigStore = createRunnerConfigStore(); if (githubRunnerConfig.enableJitConfig && githubRunnerConfig.ephemeral) { return await createJitConfig(githubRunnerConfig, runnerIds, ghClient, runnerConfigStore, options); } else { @@ -294,7 +294,7 @@ async function createRegistrationTokenConfig( for (const runnerId of runnerIds) { await runnerConfigStore.create( { runnerId, value: runnerServiceConfig.join(' ') }, - { metadataTags: options.getRunnerConfigMetadataTags?.(runnerId) }, + { metadata: options.getRunnerConfigMetadata?.(runnerId) }, ); if (isDelay) { // Delay to stay within the selected store's maximum write throughput. @@ -362,7 +362,7 @@ async function createJitConfig( }); await runnerConfigStore.create( { runnerId, value: runnerConfig.data.encoded_jit_config }, - { metadataTags: options.getRunnerConfigMetadataTags?.(runnerId) }, + { metadata: options.getRunnerConfigMetadata?.(runnerId) }, ); if (isDelay) { // Delay to stay within the selected store's maximum write throughput. diff --git a/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts index 5d190f3e5f..7c381c1526 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up-contract.test.ts @@ -1,5 +1,4 @@ import type { Octokit } from '@octokit/rest'; -import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, vi } from 'vitest'; import { providerTypes } from '../test/compute-provider-contracts/provider-types'; @@ -58,7 +57,6 @@ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; - resetRunnerConfigStore(); mockedAppAuth.mockResolvedValue({ type: 'app', token: 'app-token', appId: 1, expiresAt: 'some-date' }); mockedInstallationAuth.mockResolvedValue({ 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 dee25440bf..91545a332d 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 @@ -18,7 +18,6 @@ import type { } from './types'; import { defaultComputeProvider } from '@aws-github-runner/compute-providers/provider-types'; import { getParameter } from '@aws-github-runner/aws-ssm-util'; -import { resetRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { Octokit } from '@octokit/rest'; @@ -170,7 +169,7 @@ async function createTestProviderRunners(input: CreateScaleUpRunnersInput [{ key: 'RunnerId', value: runnerId }], + getRunnerConfigMetadata: (runnerId) => [{ key: 'RunnerId', value: runnerId }], }, ); } catch { @@ -189,7 +188,6 @@ beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); setDefaults(); - resetRunnerConfigStore(); defaultSSMGetParameterMockImpl(); defaultOctokitMockImpl(); 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 a33a8c9705..0b00d620b3 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts @@ -1,6 +1,5 @@ import { addPersistentContextToChildLogger, createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { resolveComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; -import { getRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { Octokit } from '@octokit/rest'; import yn from 'yn'; @@ -91,7 +90,6 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise [{ key: 'InstanceId', value: instanceId }], + getRunnerConfigMetadata: (instanceId) => [{ key: 'InstanceId', value: instanceId }], onJitConfigCreated: async (instanceId, metadata) => await tagEc2RunnerMetadata(ec2Operations, instanceId, metadata), }; diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts index f69782611b..98c376e028 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/scale-up.test.ts @@ -174,7 +174,7 @@ describe('scaleUp with GHES', () => { { Key: 'ghr:runner_labels', Value: 'label1,label2' }, ]); const [, , , options] = mockCreateStartRunnerConfig.mock.calls[0]; - expect(options?.getRunnerConfigMetadataTags?.('i-12345')).toEqual([{ key: 'InstanceId', value: 'i-12345' }]); + expect(options?.getRunnerConfigMetadata?.('i-12345')).toEqual([{ key: 'InstanceId', value: 'i-12345' }]); }); it('chunks comma-joined GitHub runner labels by the EC2 tag value max length', async () => { diff --git a/lambdas/libs/compute-providers/core/index.ts b/lambdas/libs/compute-providers/core/index.ts index edfefb15d2..541e8ae5de 100644 --- a/lambdas/libs/compute-providers/core/index.ts +++ b/lambdas/libs/compute-providers/core/index.ts @@ -31,7 +31,7 @@ export interface GitHubRunnerMetadata { } export interface StartRunnerConfigOptions { - getRunnerConfigMetadataTags?: (runnerId: string) => { key: string; value: string }[]; + getRunnerConfigMetadata?: (runnerId: string) => { key: string; value: string }[]; onJitConfigCreated?: (runnerId: string, metadata: GitHubRunnerMetadata) => Promise; } diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts index eaacff2718..a4bb80aa56 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts @@ -27,7 +27,7 @@ describe('aws_ssm runner config store', () => { await store.create( { runnerId: 'i-123', value: 'encoded-jit-config' }, - { metadataTags: [{ key: 'InstanceId', value: 'i-123' }] }, + { metadata: [{ key: 'InstanceId', value: 'i-123' }] }, ); expect(store.maxWritesPerSecond).toBe(40); diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts index 179bcfa87c..928e5f8f3d 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts @@ -1,6 +1,6 @@ import { putParameter } from '@aws-github-runner/aws-ssm-util'; -import type { RunnerConfigMetadataTag, RunnerConfigRecord, RunnerConfigStore } from '../../core'; +import type { RunnerConfigMetadata, RunnerConfigRecord, RunnerConfigStore } from '../../core'; import type {} from './environment'; import { loadSsmParameterStoreTagsFromEnvironment } from './parameter-store-tags'; @@ -26,10 +26,10 @@ class AwsSsmRunnerConfigStore implements RunnerConfigStore { constructor(private readonly config: AwsSsmRunnerConfigStoreConfig) {} - async create(record: RunnerConfigRecord, options: { metadataTags?: RunnerConfigMetadataTag[] } = {}): Promise { + async create(record: RunnerConfigRecord, options: { metadata?: RunnerConfigMetadata[] } = {}): Promise { await putParameter(`${this.config.tokenPath}/${record.runnerId}`, record.value, true, { tags: [ - ...(options.metadataTags ?? []).map(({ key, value }) => ({ Key: key, Value: value })), + ...(options.metadata ?? []).map(({ key, value }) => ({ Key: key, Value: value })), ...this.config.parameterStoreTags, ], }); diff --git a/lambdas/libs/storage-providers/core/index.ts b/lambdas/libs/storage-providers/core/index.ts index 7f9df413c0..26fccbc749 100644 --- a/lambdas/libs/storage-providers/core/index.ts +++ b/lambdas/libs/storage-providers/core/index.ts @@ -1,4 +1,4 @@ -export interface RunnerConfigMetadataTag { +export interface RunnerConfigMetadata { key: string; value: string; } @@ -10,5 +10,5 @@ export interface RunnerConfigRecord { export interface RunnerConfigStore { readonly maxWritesPerSecond?: number; - create(record: RunnerConfigRecord, options?: { metadataTags?: RunnerConfigMetadataTag[] }): Promise; + create(record: RunnerConfigRecord, options?: { metadata?: RunnerConfigMetadata[] }): Promise; } diff --git a/lambdas/libs/storage-providers/index.ts b/lambdas/libs/storage-providers/index.ts index 862924118b..bc59b39411 100644 --- a/lambdas/libs/storage-providers/index.ts +++ b/lambdas/libs/storage-providers/index.ts @@ -1,2 +1,2 @@ -export type { RunnerConfigMetadataTag, RunnerConfigRecord, RunnerConfigStore } from './core'; -export { getRunnerConfigStore, resetRunnerConfigStore } from './runner-config'; +export type { RunnerConfigMetadata, RunnerConfigRecord, RunnerConfigStore } from './core'; +export { createRunnerConfigStore } from './runner-config'; diff --git a/lambdas/libs/storage-providers/runner-config.test.ts b/lambdas/libs/storage-providers/runner-config.test.ts index 10467ebdb4..4ae4bd6351 100644 --- a/lambdas/libs/storage-providers/runner-config.test.ts +++ b/lambdas/libs/storage-providers/runner-config.test.ts @@ -1,84 +1,18 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { createAwsSsmRunnerConfigStore } from './aws/ssm/runner-config-store'; -import type { RunnerConfigStore } from './core'; -import { getRunnerConfigStore, resetRunnerConfigStore } from './runner-config'; +import { createRunnerConfigStore } from './runner-config'; vi.mock('./aws/ssm/runner-config-store', () => ({ createAwsSsmRunnerConfigStore: vi.fn(), })); -const createAwsSsmRunnerConfigStoreMock = vi.mocked(createAwsSsmRunnerConfigStore); -const cleanEnv = process.env; +describe('runner config store factory', () => { + it('creates the SSM implementation while the provider seam is being introduced', () => { + const store = { create: vi.fn() }; + vi.mocked(createAwsSsmRunnerConfigStore).mockReturnValue(store); -describe('runner config store selection', () => { - beforeEach(() => { - vi.clearAllMocks(); - process.env = { ...cleanEnv }; - delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; - resetRunnerConfigStore(); - }); - - it.each([undefined, '', ' '])('uses aws_ssm for default selector input %j', (provider) => { - setProvider(provider); - const store = stubStore(); - - expect(getRunnerConfigStore()).toBe(store); - expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledOnce(); - }); - - it.each(['aws_ssm', ' AWS_SSM '])('uses aws_ssm for explicit selector input %j', (provider) => { - process.env.RUNNER_CONFIG_STORAGE_PROVIDER = provider; - const store = stubStore(); - - expect(getRunnerConfigStore()).toBe(store); - expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledOnce(); - }); - - it('rejects an unsupported provider on first use', () => { - process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; - - expect(createAwsSsmRunnerConfigStoreMock).not.toHaveBeenCalled(); - expect(() => getRunnerConfigStore()).toThrow("Unsupported runner config storage provider 'not-registered'"); - expect(createAwsSsmRunnerConfigStoreMock).not.toHaveBeenCalled(); - }); - - it('selects lazily and caches the created store', () => { - const store = stubStore(); - - expect(createAwsSsmRunnerConfigStoreMock).not.toHaveBeenCalled(); - const first = getRunnerConfigStore(); - process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; - const second = getRunnerConfigStore(); - - expect(first).toBe(store); - expect(second).toBe(store); - expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledOnce(); - }); - - it('selects again after the test reset', () => { - const firstStore = stubStore(); - expect(getRunnerConfigStore()).toBe(firstStore); - - const secondStore = { create: vi.fn() } satisfies RunnerConfigStore; - createAwsSsmRunnerConfigStoreMock.mockReturnValue(secondStore); - resetRunnerConfigStore(); - - expect(getRunnerConfigStore()).toBe(secondStore); - expect(createAwsSsmRunnerConfigStoreMock).toHaveBeenCalledTimes(2); + expect(createRunnerConfigStore()).toBe(store); + expect(createAwsSsmRunnerConfigStore).toHaveBeenCalledOnce(); }); }); - -function setProvider(provider: string | undefined): void { - if (provider === undefined) { - delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; - } else { - process.env.RUNNER_CONFIG_STORAGE_PROVIDER = provider; - } -} - -function stubStore(): RunnerConfigStore { - const store = { create: vi.fn() } satisfies RunnerConfigStore; - createAwsSsmRunnerConfigStoreMock.mockReturnValue(store); - return store; -} diff --git a/lambdas/libs/storage-providers/runner-config.ts b/lambdas/libs/storage-providers/runner-config.ts index dfac2fcfb1..65fe0e1ee3 100644 --- a/lambdas/libs/storage-providers/runner-config.ts +++ b/lambdas/libs/storage-providers/runner-config.ts @@ -2,45 +2,6 @@ import { createAwsSsmRunnerConfigStore } from './aws/ssm/runner-config-store'; import type { RunnerConfigStore } from './core'; import type {} from './environment'; -type RunnerConfigStoreFactory = () => RunnerConfigStore; - -const providerFactories = { - aws_ssm: createAwsSsmRunnerConfigStore, -} as const satisfies Record; - -type RunnerConfigStorageProvider = keyof typeof providerFactories; - -const defaultProvider = 'aws_ssm' satisfies RunnerConfigStorageProvider; - -let runnerConfigStore: RunnerConfigStore | undefined; - -export function getRunnerConfigStore(): RunnerConfigStore { - runnerConfigStore ??= providerFactories[resolveProvider(process.env.RUNNER_CONFIG_STORAGE_PROVIDER)](); - return runnerConfigStore; -} - -// Test-only reset for cases that need to exercise first-use environment selection. -export function resetRunnerConfigStore(): void { - runnerConfigStore = undefined; -} - -function resolveProvider(provider: unknown): RunnerConfigStorageProvider { - if (provider === undefined) { - return defaultProvider; - } - - if (typeof provider !== 'string') { - throw new Error(`Unsupported runner config storage provider '${String(provider)}'`); - } - - const normalizedProvider = provider.trim().toLowerCase(); - if (normalizedProvider === '') { - return defaultProvider; - } - - if (!Object.prototype.hasOwnProperty.call(providerFactories, normalizedProvider)) { - throw new Error(`Unsupported runner config storage provider '${String(provider)}'`); - } - - return normalizedProvider as RunnerConfigStorageProvider; +export function createRunnerConfigStore(): RunnerConfigStore { + return createAwsSsmRunnerConfigStore(); } From 97103013603c17440c3e5ea2f78b68faa168ea63 Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 4 Sep 2026 13:32:42 +0200 Subject: [PATCH 3/5] fix(compute-providers): format runner creation callback --- .../aws/ec2/src/control-plane/runner-creation.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts index 4b7c4645c1..59d01733eb 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-creation.ts @@ -149,8 +149,7 @@ async function terminateFailedInstances( function createEc2StartRunnerConfigOptions(ec2Operations: Ec2RunnerResourceOperations): StartRunnerConfigOptions { return { getRunnerConfigMetadata: (instanceId) => [{ key: 'InstanceId', value: instanceId }], - onJitConfigCreated: async (instanceId, metadata) => - await tagEc2RunnerMetadata(ec2Operations, instanceId, metadata), + onJitConfigCreated: async (instanceId, metadata) => await tagEc2RunnerMetadata(ec2Operations, instanceId, metadata), }; } From 7d22ad91b7039a3f65e08a9beb6d8b1dbc3dcebc Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 4 Sep 2026 13:39:29 +0200 Subject: [PATCH 4/5] fix(control-plane): keep storage selection tests in feature branch --- .../functions/control-plane/src/pool/pool.test.ts | 11 ----------- .../src/scale-runners/scale-up.test.ts | 13 ------------- 2 files changed, 24 deletions(-) diff --git a/lambdas/functions/control-plane/src/pool/pool.test.ts b/lambdas/functions/control-plane/src/pool/pool.test.ts index 60ac89b239..8372ab6403 100644 --- a/lambdas/functions/control-plane/src/pool/pool.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool.test.ts @@ -166,17 +166,6 @@ describe('pool adjustment', () => { expect(poolProvider.createRunners).not.toHaveBeenCalled(); }); - - it('rejects an unsupported runner config store before GitHub or runner lookups', async () => { - process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'unsupported-provider'; - - await expect(adjust({ poolSize: 10 })).rejects.toThrow( - "Unsupported runner config storage provider 'unsupported-provider'", - ); - - expect(mockedAppAuth).not.toHaveBeenCalled(); - expect(poolProvider.listRunners).not.toHaveBeenCalled(); - }); }); describe('With GHES', () => { 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 91545a332d..c7068e5728 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 @@ -2167,19 +2167,6 @@ describe('compute provider selection', () => { }); }); -describe('runner config store preflight', () => { - it('rejects an unsupported store before resolving compute or GitHub providers', async () => { - process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'unsupported-provider'; - - await expect(scaleUpModule.scaleUp(TEST_DATA)).rejects.toThrow( - "Unsupported runner config storage provider 'unsupported-provider'", - ); - - expect(mockedResolveCapability).not.toHaveBeenCalled(); - expect(mockedAppAuth).not.toHaveBeenCalled(); - }); -}); - describe('Multi-app round-robin', () => { const mockedGetAppCount = vi.mocked(ghAuth.getAppCount); const mockedGetStoredInstallationId = vi.mocked(ghAuth.getStoredInstallationId); From 4329d4b8ab188ff8bb0f096f0abf0a0daba0f60b Mon Sep 17 00:00:00 2001 From: edersonbrilhante Date: Fri, 4 Sep 2026 13:46:24 +0200 Subject: [PATCH 5/5] fix(deps): align Lambda workspace lockfile --- lambdas/libs/compute-providers/package.json | 1 + lambdas/libs/storage-providers/package.json | 8 +++++++- lambdas/yarn.lock | 5 +++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lambdas/libs/compute-providers/package.json b/lambdas/libs/compute-providers/package.json index c1806818cc..a6fecab50c 100644 --- a/lambdas/libs/compute-providers/package.json +++ b/lambdas/libs/compute-providers/package.json @@ -26,6 +26,7 @@ "dependencies": { "@aws-github-runner/aws-powertools-util": "*", "@aws-github-runner/aws-ssm-util": "*", + "@aws-github-runner/storage-providers": "*", "@aws-sdk/client-ec2": "^3.1009.0", "@octokit/rest": "22.0.1", "moment": "2.29.4", diff --git a/lambdas/libs/storage-providers/package.json b/lambdas/libs/storage-providers/package.json index 2b753fc436..65e93c2c08 100644 --- a/lambdas/libs/storage-providers/package.json +++ b/lambdas/libs/storage-providers/package.json @@ -16,7 +16,13 @@ "all": "yarn format && yarn lint && yarn test" }, "dependencies": { - "@aws-github-runner/aws-ssm-util": "*" + "@aws-github-runner/aws-powertools-util": "*", + "@aws-github-runner/aws-ssm-util": "*", + "@aws-sdk/client-ssm": "^3.1009.0" + }, + "devDependencies": { + "aws-sdk-client-mock": "^4.1.0", + "aws-sdk-client-mock-jest": "^4.1.0" }, "nx": { "includedScripts": [ diff --git a/lambdas/yarn.lock b/lambdas/yarn.lock index ce58f39907..0e7daca0c7 100644 --- a/lambdas/yarn.lock +++ b/lambdas/yarn.lock @@ -137,6 +137,7 @@ __metadata: dependencies: "@aws-github-runner/aws-powertools-util": "npm:*" "@aws-github-runner/aws-ssm-util": "npm:*" + "@aws-github-runner/storage-providers": "npm:*" "@aws-sdk/client-ec2": "npm:^3.1009.0" "@octokit/rest": "npm:22.0.1" aws-sdk-client-mock: "npm:^4.1.0" @@ -204,7 +205,11 @@ __metadata: version: 0.0.0-use.local resolution: "@aws-github-runner/storage-providers@workspace:libs/storage-providers" dependencies: + "@aws-github-runner/aws-powertools-util": "npm:*" "@aws-github-runner/aws-ssm-util": "npm:*" + "@aws-sdk/client-ssm": "npm:^3.1009.0" + aws-sdk-client-mock: "npm:^4.1.0" + aws-sdk-client-mock-jest: "npm:^4.1.0" languageName: unknown linkType: soft