diff --git a/lambdas/functions/control-plane/package.json b/lambdas/functions/control-plane/package.json index dc74e770a9..ee3aedd210 100644 --- a/lambdas/functions/control-plane/package.json +++ b/lambdas/functions/control-plane/package.json @@ -31,8 +31,8 @@ }, "dependencies": { "@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/github/auth.test.ts b/lambdas/functions/control-plane/src/github/auth.test.ts index dd2cf3b8c2..f87053819e 100644 --- a/lambdas/functions/control-plane/src/github/auth.test.ts +++ b/lambdas/functions/control-plane/src/github/auth.test.ts @@ -2,13 +2,19 @@ import { createAppAuth } from '@octokit/auth-app'; import { StrategyOptions } from '@octokit/auth-app/dist-types/types'; import { request } from '@octokit/request'; import { RequestInterface, RequestParameters } from '@octokit/types'; -import { getParameters } from '@aws-github-runner/aws-ssm-util'; +import { + getGitHubAppCredentialsStore, + type GitHubAppCredential, + type GitHubAppCredentialsStore, +} from '@aws-github-runner/storage-providers'; import { generateKeyPairSync } from 'node:crypto'; import * as nock from 'nock'; import { createGithubAppAuth, createOctokitClient, + getAppCount, + getAppId, getStoredInstallationId, onRateLimit, onSecondaryRateLimit, @@ -25,24 +31,27 @@ type MockProxy = T & { // eslint-disable-next-line @typescript-eslint/no-explicit-any const mock = (implementation?: any): MockProxy => vi.fn(implementation) as any; -vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/storage-providers', () => ({ + getGitHubAppCredentialsStore: vi.fn(), +})); vi.mock('@octokit/auth-app'); const cleanEnv = process.env; -const ENVIRONMENT = 'dev'; -const GITHUB_APP_ID = '1'; -const PARAMETER_GITHUB_APP_ID_NAME = `/actions-runner/${ENVIRONMENT}/github_app_id`; -const PARAMETER_GITHUB_APP_KEY_BASE64_NAME = `/actions-runner/${ENVIRONMENT}/github_app_key_base64`; +const GITHUB_APP_ID = 1; -const mockedGetParameters = vi.mocked(getParameters); +const mockedGetGitHubAppCredentialsStore = vi.mocked(getGitHubAppCredentialsStore); +const mockCredentialsGet = vi.fn(); +const credentialsStore = { + get: mockCredentialsGet, +} satisfies GitHubAppCredentialsStore; beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); + mockCredentialsGet.mockReset(); resetAppCredentialsCache(); process.env = { ...cleanEnv }; - process.env.PARAMETER_GITHUB_APP_ID_NAME = PARAMETER_GITHUB_APP_ID_NAME; - process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME = PARAMETER_GITHUB_APP_KEY_BASE64_NAME; + mockedGetGitHubAppCredentialsStore.mockReturnValue(credentialsStore); nock.disableNetConnect(); }); @@ -80,38 +89,18 @@ describe('Test createGithubAppAuth', () => { const authType = 'app'; const token = '123456'; const decryptedValue = 'decryptedValue'; - const b64 = Buffer.from(decryptedValue, 'binary').toString('base64'); - - beforeEach(() => { - process.env.ENVIRONMENT = ENVIRONMENT; - }); - it('Throws early when PARAMETER_GITHUB_APP_ID_NAME is not set', async () => { - delete process.env.PARAMETER_GITHUB_APP_ID_NAME; + it('Propagates errors from the credential store', async () => { + const error = new Error('Unable to load GitHub App credentials'); + mockCredentialsGet.mockRejectedValueOnce(error); - await expect(createGithubAppAuth(installationId)).rejects.toThrow( - 'Environment variable PARAMETER_GITHUB_APP_ID_NAME is not set', - ); - expect(mockedGetParameters).not.toHaveBeenCalled(); - }); - - it('Throws early when PARAMETER_GITHUB_APP_KEY_BASE64_NAME is not set', async () => { - delete process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME; - - await expect(createGithubAppAuth(installationId)).rejects.toThrow( - 'Environment variable PARAMETER_GITHUB_APP_KEY_BASE64_NAME is not set', - ); - expect(mockedGetParameters).not.toHaveBeenCalled(); + await expect(createGithubAppAuth(installationId)).rejects.toBe(error); + expect(mockCredentialsGet).toHaveBeenCalledOnce(); }); it('Creates auth object with createJwt callback including jti claim', async () => { // Arrange - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: decryptedValue }]); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); @@ -124,7 +113,7 @@ describe('Test createGithubAppAuth', () => { // Assert expect(mockedCreatAppAuth).toBeCalledTimes(1); const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; - expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.appId).toBe(GITHUB_APP_ID); expect(callArgs.createJwt).toBeTypeOf('function'); expect(callArgs).not.toHaveProperty('privateKey'); expect(callArgs.installationId).toBe(installationId); @@ -137,14 +126,7 @@ describe('Test createGithubAppAuth', () => { privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, publicKeyEncoding: { type: 'spki', format: 'pem' }, }); - const b64Key = Buffer.from(privateKey as string).toString('base64'); - - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64Key], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: privateKey as string }]); let capturedCreateJwt: (appId: string | number, timeDifference?: number) => Promise<{ jwt: string }>; mockedCreatAppAuth.mockImplementation((opts: StrategyOptions) => { @@ -173,41 +155,9 @@ describe('Test createGithubAppAuth', () => { expect(payload).toHaveProperty('iss'); }); - it('Creates auth object with line breaks in SSH key.', async () => { - // Arrange - const b64PrivateKeyWithLineBreaks = Buffer.from(decryptedValue + '\n' + decryptedValue, 'binary').toString( - 'base64', - ); - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64PrivateKeyWithLineBreaks], - ]), - ); - - const mockedAuth = vi.fn(); - mockedAuth.mockResolvedValue({ token }); - const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() }); - mockedCreatAppAuth.mockReturnValue(mockWithHook); - - // Act - const result = await createGithubAppAuth(installationId); - - // Assert - expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); - expect(mockedCreatAppAuth).toBeCalledTimes(1); - expect(mockedAuth).toBeCalledWith({ type: authType }); - expect(result.token).toBe(token); - }); - it('Creates auth object for public GitHub', async () => { // Arrange - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: decryptedValue }]); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); @@ -218,11 +168,9 @@ describe('Test createGithubAppAuth', () => { const result = await createGithubAppAuth(installationId); // Assert - expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); - expect(mockedCreatAppAuth).toBeCalledTimes(1); const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; - expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.appId).toBe(GITHUB_APP_ID); expect(callArgs.createJwt).toBeTypeOf('function'); expect(callArgs.installationId).toBe(installationId); expect(mockedAuth).toBeCalledWith({ type: authType }); @@ -238,12 +186,7 @@ describe('Test createGithubAppAuth', () => { () => mockedRequestInterface as RequestInterface, ); - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: decryptedValue }]); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -255,11 +198,9 @@ describe('Test createGithubAppAuth', () => { const result = await createGithubAppAuth(installationId, githubServerUrl); // Assert - expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); - expect(mockedCreatAppAuth).toBeCalledTimes(1); const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; - expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.appId).toBe(GITHUB_APP_ID); expect(callArgs.createJwt).toBeTypeOf('function'); expect(callArgs.installationId).toBe(installationId); expect(callArgs.request).toBeDefined(); @@ -278,12 +219,7 @@ describe('Test createGithubAppAuth', () => { const installationId = undefined; - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: decryptedValue }]); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() }); @@ -293,11 +229,9 @@ describe('Test createGithubAppAuth', () => { const result = await createGithubAppAuth(installationId, githubServerUrl); // Assert - expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); - expect(mockedCreatAppAuth).toBeCalledTimes(1); const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; - expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.appId).toBe(GITHUB_APP_ID); expect(callArgs.createJwt).toBeTypeOf('function'); expect(callArgs).not.toHaveProperty('installationId'); expect(callArgs.request).toBeDefined(); @@ -330,98 +264,48 @@ describe('Test throttling retry caps', () => { }); }); -describe('Test getStoredInstallationId', () => { - const decryptedValue = 'decryptedValue'; - const b64 = Buffer.from(decryptedValue, 'binary').toString('base64'); - - beforeEach(() => { - const mockedAuth = vi.fn(); - mockedAuth.mockResolvedValue({ token: 'token' }); - const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() }); - vi.mocked(createAppAuth).mockReturnValue(mockWithHook); - }); - +describe('Test GitHub App credential accessors', () => { it('returns stored installation ID when configured', async () => { - const installationIdParam = `/actions-runner/${ENVIRONMENT}/github_app_installation_id`; - process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME = installationIdParam; - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - [installationIdParam, '12345'], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([ + { appId: GITHUB_APP_ID, privateKey: 'private-key', installationId: 12345 }, + ]); const result = await getStoredInstallationId(0); expect(result).toBe(12345); }); - it('returns undefined when installation ID param is empty', async () => { - process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME = ''; - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); - - const result = await getStoredInstallationId(0); - expect(result).toBeUndefined(); - }); - - it('returns undefined when env var is not set', async () => { - delete process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME; - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); + it('returns undefined when the credential has no installation ID', async () => { + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: 'private-key' }]); const result = await getStoredInstallationId(0); expect(result).toBeUndefined(); }); it('returns undefined for out-of-bounds appIndex', async () => { - process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME = ''; - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], - [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], - ]), - ); + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: 'private-key' }]); const result = await getStoredInstallationId(99); expect(result).toBeUndefined(); }); - it('loads installation IDs for multi-app setup', async () => { - const app1IdParam = `/actions-runner/${ENVIRONMENT}/github_app_id`; - const app2IdParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_id`; - const app1KeyParam = `/actions-runner/${ENVIRONMENT}/github_app_key_base64`; - const app2KeyParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_key_base64`; - const app2InstallParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_installation_id`; - - process.env.PARAMETER_GITHUB_APP_ID_NAME = `${app1IdParam}:${app2IdParam}`; - process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME = `${app1KeyParam}:${app2KeyParam}`; - process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME = `:${app2InstallParam}`; - - mockedGetParameters.mockResolvedValueOnce( - new Map([ - [app1IdParam, '1'], - [app1KeyParam, b64], - [app2IdParam, '2'], - [app2KeyParam, b64], - [app2InstallParam, '67890'], - ]), - ); + it('loads multi-app credentials once and exposes values by index', async () => { + const credentials: GitHubAppCredential[] = [ + { appId: 1, privateKey: 'private-key-1' }, + { appId: 2, privateKey: 'private-key-2', installationId: 67890 }, + ]; + mockCredentialsGet.mockResolvedValueOnce(credentials); + + await expect(getAppCount()).resolves.toBe(2); + await expect(getAppId()).resolves.toBe('1'); + await expect(getAppId(1)).resolves.toBe('2'); + await expect(getStoredInstallationId(0)).resolves.toBeUndefined(); + await expect(getStoredInstallationId(1)).resolves.toBe(67890); + expect(mockCredentialsGet).toHaveBeenCalledOnce(); + }); - // Primary app (index 0) has no stored installation ID - const result0 = await getStoredInstallationId(0); - expect(result0).toBeUndefined(); + it('throws a clear error for an out-of-bounds app ID index', async () => { + mockCredentialsGet.mockResolvedValueOnce([{ appId: GITHUB_APP_ID, privateKey: 'private-key' }]); - // Additional app (index 1) has stored installation ID - const result1 = await getStoredInstallationId(1); - expect(result1).toBe(67890); + await expect(getAppId(99)).rejects.toThrow('GitHub App credential at index 99 not found'); }); }); diff --git a/lambdas/functions/control-plane/src/github/auth.ts b/lambdas/functions/control-plane/src/github/auth.ts index f64ac00b30..e4ade0b38a 100644 --- a/lambdas/functions/control-plane/src/github/auth.ts +++ b/lambdas/functions/control-plane/src/github/auth.ts @@ -22,7 +22,7 @@ import { Octokit } from '@octokit/rest'; import { retry } from '@octokit/plugin-retry'; import { throttling } from '@octokit/plugin-throttling'; import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; -import { getParameters } from '@aws-github-runner/aws-ssm-util'; +import { getGitHubAppCredentialsStore, type GitHubAppCredential } from '@aws-github-runner/storage-providers'; import { EndpointDefaults } from '@octokit/types'; const logger = createChildLogger('gh-auth'); @@ -69,52 +69,10 @@ export function onSecondaryRateLimit( return retryCount < MAX_SECONDARY_RATE_LIMIT_RETRIES; } -interface GitHubAppCredential { - appId: number; - privateKey: string; - installationId?: number; -} - let appCredentialsPromise: Promise | null = null; async function loadAppCredentials(): Promise { - if (!process.env.PARAMETER_GITHUB_APP_ID_NAME) { - throw new Error('Environment variable PARAMETER_GITHUB_APP_ID_NAME is not set'); - } - if (!process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME) { - throw new Error('Environment variable PARAMETER_GITHUB_APP_KEY_BASE64_NAME is not set'); - } - const idParams = process.env.PARAMETER_GITHUB_APP_ID_NAME.split(':').filter(Boolean); - const keyParams = process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME.split(':').filter(Boolean); - const installationIdParams = (process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME || '').split(':'); - if (idParams.length !== keyParams.length) { - throw new Error(`GitHub App parameter count mismatch: ${idParams.length} IDs vs ${keyParams.length} keys`); - } - // Batch fetch all SSM parameters in a single call to reduce API calls - const allParamNames = [...idParams, ...keyParams, ...installationIdParams.filter((p) => p.length > 0)]; - const params = await getParameters(allParamNames); - - const credentials: GitHubAppCredential[] = []; - for (let i = 0; i < idParams.length; i++) { - const appIdValue = params.get(idParams[i]); - if (!appIdValue) { - throw new Error(`Parameter ${idParams[i]} not found`); - } - const appId = parseInt(appIdValue, 10); - const privateKeyBase64 = params.get(keyParams[i]); - if (!privateKeyBase64) { - throw new Error(`Parameter ${keyParams[i]} not found`); - } - // replace literal \n characters with new lines to allow the key to be stored as a - // single line variable. This logic should match how the GitHub Terraform provider - // processes private keys to retain compatibility between the projects - const privateKey = Buffer.from(privateKeyBase64, 'base64').toString().replace(/\\n/g, '\n'); - const installationIdParam = installationIdParams[i]; - const installationIdValue = - installationIdParam && installationIdParam.length > 0 ? params.get(installationIdParam) : undefined; - const installationId = installationIdValue ? parseInt(installationIdValue, 10) : undefined; - credentials.push({ appId, privateKey, installationId }); - } + const credentials = await getGitHubAppCredentialsStore().get(); logger.info(`Loaded ${credentials.length} GitHub App credential(s)`); return credentials; } @@ -137,6 +95,14 @@ export async function getStoredInstallationId(appIndex: number): Promise { + const credential = (await getAppCredentials())[appIndex]; + if (!credential) { + throw new Error(`GitHub App credential at index ${appIndex} not found`); + } + return credential.appId.toString(); +} + export async function createOctokitClient(token: string, ghesApiUrl = ''): Promise { const CustomOctokit = Octokit.plugin(retry, throttling); const ocktokitOptions: OctokitOptions = { diff --git a/lambdas/functions/control-plane/src/github/rate-limit.test.ts b/lambdas/functions/control-plane/src/github/rate-limit.test.ts index d9d18c5921..93e6d24ba0 100644 --- a/lambdas/functions/control-plane/src/github/rate-limit.test.ts +++ b/lambdas/functions/control-plane/src/github/rate-limit.test.ts @@ -1,50 +1,37 @@ -import { ResponseHeaders } from '@octokit/types'; -import { createSingleMetric } from '@aws-github-runner/aws-powertools-util'; import { MetricUnit } from '@aws-lambda-powertools/metrics'; +import { createSingleMetric } from '@aws-github-runner/aws-powertools-util'; +import type { ResponseHeaders } from '@octokit/types'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { getAppId } from './auth'; import { metricGitHubAppRateLimit } from './rate-limit'; -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { getParameter } from '@aws-github-runner/aws-ssm-util'; - -process.env.PARAMETER_GITHUB_APP_ID_NAME = 'test'; -vi.mock('@aws-github-runner/aws-ssm-util', async () => { - // Return only what we need without spreading actual - return { - getParameter: vi.fn((name: string) => { - if (name === process.env.PARAMETER_GITHUB_APP_ID_NAME) { - return '1234'; - } else { - return ''; - } - }), - }; -}); -vi.mock('@aws-github-runner/aws-powertools-util', async () => { - // Provide only what's needed without spreading actual - return { - // Mock the logger - logger: { - debug: vi.fn(), - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - }, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - createSingleMetric: vi.fn((name: string, unit: string, value: number, dimensions?: Record) => { - return { - addMetadata: vi.fn(), - }; - }), - }; +vi.mock('./auth', () => ({ + getAppId: vi.fn(), +})); + +vi.mock('@aws-github-runner/aws-powertools-util', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, + createSingleMetric: vi.fn(() => ({ addMetadata: vi.fn() })), +})); + +const cleanEnv = process.env; +const mockedGetAppId = vi.mocked(getAppId); + +beforeEach(() => { + vi.clearAllMocks(); + mockedGetAppId.mockReset(); + mockedGetAppId.mockResolvedValue('1234'); + process.env = { ...cleanEnv }; }); describe('metricGitHubAppRateLimit', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it('should update rate limit metric', async () => { - // set process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT to true + it('updates the rate limit metric', async () => { process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT = 'true'; const headers: ResponseHeaders = { 'x-ratelimit-remaining': '10', @@ -53,13 +40,13 @@ describe('metricGitHubAppRateLimit', () => { await metricGitHubAppRateLimit(headers); + expect(mockedGetAppId).toHaveBeenCalledWith(undefined); expect(createSingleMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 10, { AppId: '1234', }); }); - it('should not update rate limit metric', async () => { - // set process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT to false + it('does not update the rate limit metric when disabled', async () => { process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT = 'false'; const headers: ResponseHeaders = { 'x-ratelimit-remaining': '10', @@ -68,107 +55,85 @@ describe('metricGitHubAppRateLimit', () => { await metricGitHubAppRateLimit(headers); + expect(mockedGetAppId).not.toHaveBeenCalled(); expect(createSingleMetric).not.toHaveBeenCalled(); }); - it('should not update rate limit metric if headers are undefined', async () => { - // set process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT to true + it('does not update the rate limit metric if headers are undefined', async () => { process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT = 'true'; await metricGitHubAppRateLimit(undefined as unknown as ResponseHeaders); + expect(mockedGetAppId).not.toHaveBeenCalled(); expect(createSingleMetric).not.toHaveBeenCalled(); }); - it('should cache GitHub App ID and only call getParameter once', async () => { - // Reset modules to clear the appIdPromises Map cache - vi.resetModules(); - const { metricGitHubAppRateLimit: freshMetricFunction } = await import('./rate-limit'); - + it('does not update the metric when the app ID lookup fails', async () => { process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT = 'true'; + mockedGetAppId.mockRejectedValueOnce(new Error('credential store unavailable')); const headers: ResponseHeaders = { 'x-ratelimit-remaining': '10', 'x-ratelimit-limit': '60', }; - const mockGetParameter = vi.mocked(getParameter); - mockGetParameter.mockClear(); + await expect(metricGitHubAppRateLimit(headers)).resolves.not.toThrow(); - await freshMetricFunction(headers); - await freshMetricFunction(headers); - await freshMetricFunction(headers); - - // getParameter should only be called once due to caching (index 0 cached after first call) - expect(mockGetParameter).toHaveBeenCalledTimes(1); - // split(':')[0] of 'test' is still 'test' - expect(mockGetParameter).toHaveBeenCalledWith(process.env.PARAMETER_GITHUB_APP_ID_NAME); + expect(createSingleMetric).not.toHaveBeenCalled(); }); }); describe('metricGitHubAppRateLimit multi-app', () => { - let freshMetricFunction: typeof metricGitHubAppRateLimit; - let mockGetParam: ReturnType; - - beforeEach(async () => { - // Reset modules to get a clean appIdPromises Map for each test - vi.resetModules(); - - process.env.PARAMETER_GITHUB_APP_ID_NAME = 'app0:app1'; + beforeEach(() => { process.env.ENABLE_METRIC_GITHUB_APP_RATE_LIMIT = 'true'; - - mockGetParam = vi.fn((name: string) => { - if (name === 'app0') return Promise.resolve('1234'); - if (name === 'app1') return Promise.resolve('5678'); - return Promise.resolve(''); + mockedGetAppId.mockImplementation(async (appIndex = 0) => { + if (appIndex === 0) return '1234'; + if (appIndex === 1) return '5678'; + throw new Error(`GitHub App credential at index ${appIndex} not found`); }); - - vi.doMock('@aws-github-runner/aws-ssm-util', () => ({ getParameter: mockGetParam })); - vi.doMock('@aws-github-runner/aws-powertools-util', () => ({ - logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - createSingleMetric: vi.fn(() => ({ addMetadata: vi.fn() })), - })); - - const mod = await import('./rate-limit'); - freshMetricFunction = mod.metricGitHubAppRateLimit; - }); - - afterEach(() => { - vi.resetModules(); - process.env.PARAMETER_GITHUB_APP_ID_NAME = 'test'; }); - it('should label metric with correct appId for index 0 (primary app)', async () => { - const { createSingleMetric: mockMetric } = await import('@aws-github-runner/aws-powertools-util'); + it('labels the metric with the primary app ID', async () => { const headers: ResponseHeaders = { 'x-ratelimit-remaining': '50', 'x-ratelimit-limit': '5000' }; - await freshMetricFunction(headers, 0); - expect(mockMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 50, { AppId: '1234' }); + + await metricGitHubAppRateLimit(headers, 0); + + expect(mockedGetAppId).toHaveBeenCalledWith(0); + expect(createSingleMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 50, { + AppId: '1234', + }); }); - it('should label metric with correct appId for index 1 (additional app)', async () => { - const { createSingleMetric: mockMetric } = await import('@aws-github-runner/aws-powertools-util'); + it('labels the metric with an additional app ID', async () => { const headers: ResponseHeaders = { 'x-ratelimit-remaining': '100', 'x-ratelimit-limit': '5000' }; - await freshMetricFunction(headers, 1); - expect(mockMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 100, { AppId: '5678' }); + + await metricGitHubAppRateLimit(headers, 1); + + expect(mockedGetAppId).toHaveBeenCalledWith(1); + expect(createSingleMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 100, { + AppId: '5678', + }); }); - it('should default to index 0 when no appIndex is passed', async () => { - const { createSingleMetric: mockMetric } = await import('@aws-github-runner/aws-powertools-util'); + it('defaults to the primary app when no app index is passed', async () => { const headers: ResponseHeaders = { 'x-ratelimit-remaining': '75', 'x-ratelimit-limit': '5000' }; - await freshMetricFunction(headers); - expect(mockMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 75, { AppId: '1234' }); + + await metricGitHubAppRateLimit(headers); + + expect(mockedGetAppId).toHaveBeenCalledWith(undefined); + expect(createSingleMetric).toHaveBeenCalledWith('GitHubAppRateLimitRemaining', MetricUnit.Count, 75, { + AppId: '1234', + }); }); - it('should cache per index and call getParameter separately for each index', async () => { + it('forwards each app index to the shared credential accessor', async () => { const headers: ResponseHeaders = { 'x-ratelimit-remaining': '10', 'x-ratelimit-limit': '5000' }; - // Two calls with index 1, then one with index 0 - await freshMetricFunction(headers, 1); - await freshMetricFunction(headers, 1); - await freshMetricFunction(headers, 0); + await metricGitHubAppRateLimit(headers, 1); + await metricGitHubAppRateLimit(headers, 1); + await metricGitHubAppRateLimit(headers, 0); - // getParameter should be called exactly once per distinct index - expect(mockGetParam).toHaveBeenCalledTimes(2); - expect(mockGetParam).toHaveBeenCalledWith('app1'); - expect(mockGetParam).toHaveBeenCalledWith('app0'); + expect(mockedGetAppId).toHaveBeenNthCalledWith(1, 1); + expect(mockedGetAppId).toHaveBeenNthCalledWith(2, 1); + expect(mockedGetAppId).toHaveBeenNthCalledWith(3, 0); }); }); diff --git a/lambdas/functions/control-plane/src/github/rate-limit.ts b/lambdas/functions/control-plane/src/github/rate-limit.ts index df2372a255..b5559a5d82 100644 --- a/lambdas/functions/control-plane/src/github/rate-limit.ts +++ b/lambdas/functions/control-plane/src/github/rate-limit.ts @@ -2,22 +2,8 @@ import { ResponseHeaders } from '@octokit/types'; import { createSingleMetric, logger } from '@aws-github-runner/aws-powertools-util'; import { MetricUnit } from '@aws-lambda-powertools/metrics'; import yn from 'yn'; -import { getParameter } from '@aws-github-runner/aws-ssm-util'; -// Cache the app ID per app index to avoid repeated SSM calls across Lambda invocations. -// In multi-app mode PARAMETER_GITHUB_APP_ID_NAME is a ':'-joined list of SSM param names, -// one per app in app-index order; index 0 is the primary app. -const appIdPromises = new Map>(); - -async function getAppId(appIndex = 0): Promise { - let cached = appIdPromises.get(appIndex); - if (!cached) { - const paramName = process.env.PARAMETER_GITHUB_APP_ID_NAME.split(':')[appIndex]; - cached = getParameter(paramName); - appIdPromises.set(appIndex, cached); - } - return cached; -} +import { getAppId } from './auth'; export async function metricGitHubAppRateLimit(headers: ResponseHeaders, appIndex?: number): Promise { try { diff --git a/lambdas/functions/control-plane/src/lambda.test.ts b/lambdas/functions/control-plane/src/lambda.test.ts index 26b130ffe1..4c61f2c585 100644 --- a/lambdas/functions/control-plane/src/lambda.test.ts +++ b/lambdas/functions/control-plane/src/lambda.test.ts @@ -1,4 +1,5 @@ import { captureLambdaHandler, logger } from '@aws-github-runner/aws-powertools-util'; +import { getRunnerConfigStore, type RunnerConfigStore } from '@aws-github-runner/storage-providers'; import { Context, SQSEvent, SQSRecord } from 'aws-lambda'; import { addMiddleware, adjustPool, scaleDownHandler, scaleUpHandler, ssmHousekeeper, jobRetryCheck } from './lambda'; @@ -6,7 +7,6 @@ import { adjust } from './pool/pool'; import { scaleDown } from './scale-runners/scale-down'; import { scaleUp } from './scale-runners/scale-up'; import type { ActionRequestMessage } from './scale-runners/types'; -import { cleanSSMTokens } from './scale-runners/ssm-housekeeper'; import { checkAndRetryJob } from './scale-runners/job-retry'; import { describe, it, expect, vi, MockedFunction, beforeEach } from 'vitest'; @@ -64,10 +64,16 @@ const context: Context = { vi.mock('./pool/pool'); vi.mock('./scale-runners/scale-down'); vi.mock('./scale-runners/scale-up'); -vi.mock('./scale-runners/ssm-housekeeper'); vi.mock('./scale-runners/job-retry'); vi.mock('@aws-github-runner/aws-powertools-util'); -vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/storage-providers', () => ({ + getRunnerConfigStore: vi.fn(), +})); + +const runnerConfigStore = { + create: vi.fn(), + houseKeeper: vi.fn(), +} satisfies RunnerConfigStore; describe('Test scale up lambda wrapper.', () => { it('Do not handle empty record sets.', async () => { @@ -297,22 +303,31 @@ describe('Test middleware', () => { }); describe('Test ssm housekeeper lambda wrapper.', () => { - it('Invoke without errors.', async () => { - vi.mocked(cleanSSMTokens).mockResolvedValue(); + beforeEach(() => { + vi.mocked(getRunnerConfigStore).mockReturnValue(runnerConfigStore); + }); - process.env.SSM_CLEANUP_CONFIG = JSON.stringify({ - dryRun: false, - minimumDaysOld: 1, - tokenPath: '/path/to/tokens/', - }); + it('Invoke without errors.', async () => { + runnerConfigStore.houseKeeper.mockResolvedValue(); await expect(ssmHousekeeper({}, context)).resolves.not.toThrow(); + expect(getRunnerConfigStore).toHaveBeenCalledOnce(); + expect(runnerConfigStore.houseKeeper).toHaveBeenCalledOnce(); }); it('Errors not throws.', async () => { - vi.mocked(cleanSSMTokens).mockRejectedValue(new Error()); + runnerConfigStore.houseKeeper.mockRejectedValue(new Error()); await expect(ssmHousekeeper({}, context)).resolves.not.toThrow(); }); + + it('does not catch provider construction errors', async () => { + const error = new Error('Invalid provider configuration'); + vi.mocked(getRunnerConfigStore).mockImplementation(() => { + throw error; + }); + + await expect(ssmHousekeeper({}, context)).rejects.toBe(error); + }); }); describe('Test job retry check wrapper', () => { diff --git a/lambdas/functions/control-plane/src/lambda.ts b/lambdas/functions/control-plane/src/lambda.ts index d229a0350e..270980c3bc 100644 --- a/lambdas/functions/control-plane/src/lambda.ts +++ b/lambdas/functions/control-plane/src/lambda.ts @@ -1,13 +1,13 @@ import middy from '@middy/core'; import { logger, setContext } from '@aws-github-runner/aws-powertools-util'; import { captureLambdaHandler, tracer } from '@aws-github-runner/aws-powertools-util'; +import { getRunnerConfigStore } from '@aws-github-runner/storage-providers'; import { Context, type SQSBatchItemFailure, type SQSBatchResponse, SQSEvent } from 'aws-lambda'; import { PoolEvent, adjust } from './pool/pool'; import { scaleDown } from './scale-runners/scale-down'; import { scaleUp } from './scale-runners/scale-up'; import type { ActionRequestMessage, ActionRequestMessageSQS } from './scale-runners/types'; -import { SSMCleanupOptions, cleanSSMTokens } from './scale-runners/ssm-housekeeper'; import { checkAndRetryJob } from './scale-runners/job-retry'; export async function scaleUpHandler(event: SQSEvent, context: Context): Promise { @@ -121,10 +121,10 @@ addMiddleware(); export async function ssmHousekeeper(event: unknown, context: Context): Promise { setContext(context, 'lambda.ts'); logger.logEventIfEnabled(event); - const config = JSON.parse(process.env.SSM_CLEANUP_CONFIG) as SSMCleanupOptions; + const runnerConfigStore = getRunnerConfigStore(); try { - await cleanSSMTokens(config); + await runnerConfigStore.houseKeeper(); } catch (e) { logger.error(`${(e as Error).message}`, { error: e as Error }); } diff --git a/lambdas/functions/control-plane/src/modules.d.ts b/lambdas/functions/control-plane/src/modules.d.ts index d5157ccb37..af537afcba 100644 --- a/lambdas/functions/control-plane/src/modules.d.ts +++ b/lambdas/functions/control-plane/src/modules.d.ts @@ -13,13 +13,9 @@ declare namespace NodeJS { MINIMUM_RUNNING_TIME_IN_MINUTES: string; PARAMETER_GITHUB_APP_CLIENT_ID_NAME: string; PARAMETER_GITHUB_APP_CLIENT_SECRET_NAME: string; - PARAMETER_GITHUB_APP_ID_NAME: string; - PARAMETER_GITHUB_APP_KEY_BASE64_NAME: string; 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; INSTANCE_TARGET_CAPACITY_TYPE: 'on-demand' | 'spot'; 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..a949afae39 100644 --- a/lambdas/functions/control-plane/src/pool/pool-contract.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool-contract.test.ts @@ -20,7 +20,6 @@ vi.mock('../github/auth', () => ({ vi.mock('../scale-runners/github-runner', () => ({ createStartRunnerConfig: vi.fn(), getGitHubEnterpriseApiUrl: vi.fn(), - validateSsmParameterStoreTags: vi.fn(), })); const mockedAppAuth = vi.mocked(ghAuth.createGithubAppAuth); @@ -48,7 +47,6 @@ const computeProviders = providerTypes.map((type) => ({ beforeEach(() => { vi.clearAllMocks(); process.env = { ...cleanEnv }; - mockedAppAuth.mockResolvedValue({ type: 'app', token: 'app-token', appId: 1, expiresAt: 'some-date' }); mockedInstallationAuth.mockResolvedValue({ type: 'token', @@ -62,7 +60,6 @@ beforeEach(() => { }); mockedCreateClient.mockResolvedValue(githubClient); vi.mocked(githubRunner.getGitHubEnterpriseApiUrl).mockReturnValue({ ghesApiUrl: '', ghesBaseUrl: '' }); - vi.mocked(githubRunner.validateSsmParameterStoreTags).mockReturnValue([]); vi.mocked(githubClient.apps.getOrgInstallation).mockResolvedValue({ data: { id: 2 } } as never); vi.mocked(githubClient.paginate).mockResolvedValue([]); }); diff --git a/lambdas/functions/control-plane/src/pool/pool.test.ts b/lambdas/functions/control-plane/src/pool/pool.test.ts index 568403c3be..a4bdca3000 100644 --- a/lambdas/functions/control-plane/src/pool/pool.test.ts +++ b/lambdas/functions/control-plane/src/pool/pool.test.ts @@ -51,7 +51,6 @@ vi.mock('../scale-runners/github-runner', async () => ({ ghesApiUrl: '', ghesBaseUrl: '', }), - validateSsmParameterStoreTags: vi.fn().mockReturnValue([]), })); const mocktokit = Octokit as MockedClass; @@ -143,7 +142,6 @@ beforeEach(() => { process.env.ENABLE_ORGANIZATION_RUNNERS = 'true'; process.env.LAUNCH_TEMPLATE_NAME = 'lt-1'; process.env.SUBNET_IDS = 'subnet-123'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/tokens'; process.env.INSTANCE_TYPES = 'm5.large'; process.env.INSTANCE_TARGET_CAPACITY_TYPE = 'spot'; process.env.RUNNER_OWNER = ORG; @@ -247,8 +245,8 @@ describe('Test simple pool.', () => { }); it('Rejects unsupported pool provider types.', async () => { - await expect(adjust({ poolSize: 10, type: 'microvm' })).rejects.toThrow( - "Unsupported compute provider type 'microvm'", + await expect(adjust({ poolSize: 10, type: 'unsupported-provider' })).rejects.toThrow( + "Unsupported compute provider type 'unsupported-provider'", ); expect(mockListRunners).not.toHaveBeenCalled(); }); diff --git a/lambdas/functions/control-plane/src/pool/pool.ts b/lambdas/functions/control-plane/src/pool/pool.ts index da5d2ee9b1..21e91adebc 100644 --- a/lambdas/functions/control-plane/src/pool/pool.ts +++ b/lambdas/functions/control-plane/src/pool/pool.ts @@ -10,7 +10,7 @@ import { getStoredInstallationId, } from '../github/auth'; import { controlPlaneProviderRegistry } from '../control-plane-providers'; -import { getGitHubEnterpriseApiUrl, validateSsmParameterStoreTags } from '../scale-runners/github-runner'; +import { getGitHubEnterpriseApiUrl } from '../scale-runners/github-runner'; import type { RunnerStatus } from './pool-provider'; const logger = createChildLogger('pool'); @@ -31,16 +31,10 @@ 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 }); const disableAutoUpdate = yn(process.env.DISABLE_RUNNER_AUTOUPDATE, { default: false }); const runnerOwner = process.env.RUNNER_OWNER; - const ssmParameterStoreTags: { Key: string; Value: string }[] = - process.env.SSM_PARAMETER_STORE_TAGS && process.env.SSM_PARAMETER_STORE_TAGS.trim() !== '' - ? validateSsmParameterStoreTags(process.env.SSM_PARAMETER_STORE_TAGS) - : []; // -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,9 +97,6 @@ export async function adjust(event: PoolEvent): Promise { runnerNamePrefix, runnerType: 'Org', disableAutoUpdate: disableAutoUpdate, - ssmTokenPath, - ssmConfigPath, - ssmParameterStoreTags, }, numberOfRunners: topUp, githubInstallationClient, 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..ec78a2a2b5 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 { + getRunnerGroupCacheStore, + getRunnerConfigStore, + type RunnerConfigMetadata, + 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 }[]; + getRunnerConfigMetadata?: (runnerId: string) => RunnerConfigMetadata[]; onJitConfigCreated?: (runnerId: string, metadata: GitHubRunnerMetadata) => Promise; } @@ -51,37 +56,6 @@ function quoteShellArg(value: string): string { return `'${value.replace(/'/g, `'\\''`)}'`; } -export function validateSsmParameterStoreTags(tagsJson: string): { Key: string; Value: string }[] { - try { - const tags = JSON.parse(tagsJson); - - if (!Array.isArray(tags)) { - throw new Error('Tags must be an array'); - } - - if (tags.length === 0) { - return []; - } - - tags.forEach((tag, index) => { - if (typeof tag !== 'object' || tag === null) { - throw new Error(`Tag at index ${index} must be an object`); - } - if (!tag.Key || typeof tag.Key !== 'string' || tag.Key.trim() === '') { - throw new Error(`Tag at index ${index} has missing or invalid 'Key' property`); - } - if (!Object.prototype.hasOwnProperty.call(tag, 'Value') || typeof tag.Value !== 'string') { - throw new Error(`Tag at index ${index} has missing or invalid 'Value' property`); - } - }); - - return tags; - } catch (err) { - logger.error('Invalid SSM_PARAMETER_STORE_TAGS format', { error: err }); - throw new Error(`Failed to parse SSM_PARAMETER_STORE_TAGS: ${(err as Error).message}`); - } -} - async function getGithubRunnerRegistrationToken(githubRunnerConfig: CreateGitHubRunnerConfig, ghClient: Octokit) { const registrationToken = githubRunnerConfig.runnerType === 'Org' @@ -186,39 +160,30 @@ export async function getRunnerGroupId( // if the runnerType is Repo, then runnerGroupId is default to 1 let runnerGroupId: number | undefined = 1; if (githubRunnerConfig.runnerType === 'Org' && githubRunnerConfig.runnerGroup !== undefined) { - let runnerGroup: string | undefined; - // check if runner group id is already stored in SSM Parameter Store and - // use it if it exists to avoid API call to GitHub + const runnerGroupCacheStore = getRunnerGroupCacheStore(); + let cachedRunnerGroupId: number | undefined; + // Use a cached runner group id when available to avoid an API call to GitHub. try { - runnerGroup = await getParameter( - `${githubRunnerConfig.ssmConfigPath}/runner-group/${githubRunnerConfig.runnerGroup}`, - ); + cachedRunnerGroupId = await runnerGroupCacheStore.get(githubRunnerConfig.runnerGroup); } catch (err) { logger.debug('Handling error:', err as Error); - logger.warn( - `SSM Parameter "${githubRunnerConfig.ssmConfigPath}/runner-group/${githubRunnerConfig.runnerGroup}" - for Runner group ${githubRunnerConfig.runnerGroup} does not exist`, - ); + logger.warn(`Cached id for runner group ${githubRunnerConfig.runnerGroup} does not exist`); } - if (runnerGroup === undefined) { + if (cachedRunnerGroupId === undefined) { // get runner group id from GitHub runnerGroupId = await getRunnerGroupByName(ghClient, githubRunnerConfig); - // store runner group id in SSM + // cache the runner group id try { - await putParameter( - `${githubRunnerConfig.ssmConfigPath}/runner-group/${githubRunnerConfig.runnerGroup}`, - runnerGroupId.toString(), - false, - { - tags: githubRunnerConfig.ssmParameterStoreTags, - }, - ); + await runnerGroupCacheStore.create({ + runnerGroupName: githubRunnerConfig.runnerGroup, + runnerGroupId, + }); } catch (err) { - logger.debug('Error storing runner group id in SSM Parameter Store', err as Error); + logger.debug('Error storing runner group id in cache', err as Error); throw err; } } else { - runnerGroupId = parseInt(runnerGroup); + runnerGroupId = cachedRunnerGroupId; } } return runnerGroupId; @@ -250,18 +215,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 +240,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 +252,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(' ') }, + { metadata: options.getRunnerConfigMetadata?.(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 +275,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 +317,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 }, + { metadata: options.getRunnerConfigMetadata?.(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.test.ts b/lambdas/functions/control-plane/src/scale-runners/scale-up.test.ts index eb6899ff79..4ab47c0bba 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 @@ -1,9 +1,12 @@ -import { PutParameterCommand, SSMClient } from '@aws-sdk/client-ssm'; -import { mockClient } from 'aws-sdk-client-mock'; -import 'aws-sdk-client-mock-jest/vitest'; -// Using vi.mocked instead of jest-mock +import { + getRunnerConfigStore, + getRunnerGroupCacheStore, + type RunnerConfigStore, + type RunnerGroupCacheStore, +} from '@aws-github-runner/storage-providers'; +import type { Octokit } from '@octokit/rest'; import nock from 'nock'; -import { performance } from 'perf_hooks'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { controlPlaneProviderRegistry } from '../control-plane-providers'; import * as ghAuth from '../github/auth'; @@ -16,9 +19,6 @@ import type { CreateScaleUpRunnersInput, ScaleUpComputeProvider, } from './types'; -import { getParameter } from '@aws-github-runner/aws-ssm-util'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Octokit } from '@octokit/rest'; const mockOctokit = { paginate: vi.fn(), @@ -53,8 +53,21 @@ const createRunner = vi.fn<(input: TestRunnerCreationInput) => Promise Promise>(); const mockCreateRunner = vi.mocked(createRunner); const mockListRunners = vi.mocked(listRunners); -const mockSSMClient = mockClient(SSMClient); -const mockSSMgetParameter = vi.mocked(getParameter); +const mockGetRunnerConfigStore = vi.mocked(getRunnerConfigStore); +const mockGetRunnerGroupCacheStore = vi.mocked(getRunnerGroupCacheStore); +const mockRunnerConfigCreate = vi.fn(); +const mockRunnerConfigHouseKeeper = vi.fn(); +const mockRunnerGroupCacheGet = vi.fn(); +const mockRunnerGroupCacheCreate = vi.fn(); +const mockRunnerConfigStore: RunnerConfigStore = { + maxWritesPerSecond: 40, + create: mockRunnerConfigCreate, + houseKeeper: mockRunnerConfigHouseKeeper, +}; +const mockRunnerGroupCacheStore: RunnerGroupCacheStore = { + get: mockRunnerGroupCacheGet, + create: mockRunnerGroupCacheCreate, +}; const mockPublishRetryMessage = vi.mocked(publishRetryMessage); const testProviderState = { provider: 'test' }; const mockComputeProvider: ScaleUpComputeProvider = { @@ -86,16 +99,10 @@ vi.mock('../github/auth', async () => ({ getStoredInstallationId: vi.fn().mockResolvedValue(undefined), })); -vi.mock('@aws-github-runner/aws-ssm-util', async () => { - const actual = (await vi.importActual( - '@aws-github-runner/aws-ssm-util', - )) as typeof import('@aws-github-runner/aws-ssm-util'); - - return { - ...actual, - getParameter: vi.fn(), - }; -}); +vi.mock('@aws-github-runner/storage-providers', () => ({ + getRunnerConfigStore: vi.fn(), + getRunnerGroupCacheStore: vi.fn(), +})); vi.mock('./job-retry', () => ({ publishRetryMessage: vi.fn(), @@ -140,7 +147,6 @@ let expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; function setDefaults() { process.env = { ...cleanEnv }; - process.env.PARAMETER_GITHUB_APP_ID_NAME = 'github-app-id'; process.env.GITHUB_APP_KEY_BASE64 = 'TEST_CERTIFICATE_DATA'; process.env.GITHUB_APP_ID = '1337'; process.env.GITHUB_APP_CLIENT_ID = 'TEST_CLIENT_ID'; @@ -168,7 +174,7 @@ async function createTestProviderRunners(input: CreateScaleUpRunnersInput [{ Key: 'RunnerId', Value: runnerId }], + getRunnerConfigMetadata: (runnerId) => [{ key: 'RunnerId', value: runnerId }], }, ); } catch { @@ -187,8 +193,12 @@ beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); setDefaults(); - - defaultSSMGetParameterMockImpl(); + mockGetRunnerConfigStore.mockReturnValue(mockRunnerConfigStore); + mockGetRunnerGroupCacheStore.mockReturnValue(mockRunnerGroupCacheStore); + mockRunnerConfigCreate.mockResolvedValue(); + mockRunnerConfigHouseKeeper.mockResolvedValue(); + mockRunnerGroupCacheGet.mockResolvedValue(1); + mockRunnerGroupCacheCreate.mockResolvedValue(); defaultOctokitMockImpl(); mockedResolveCapability.mockReturnValue(() => mockComputeProvider); @@ -268,12 +278,9 @@ describe('scaleUp with GHES', () => { process.env.ENABLE_EPHEMERAL_RUNNERS = 'true'; process.env.RUNNER_NAME_PREFIX = 'unit-test-'; process.env.RUNNER_GROUP_NAME = 'Default'; - process.env.SSM_CONFIG_PATH = '/github-action-runners/default/runners/config'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; process.env.RUNNER_LABELS = 'label1,label2'; expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; - mockSSMClient.reset(); }); it('does not create a token when maximum runners has been reached', async () => { @@ -328,9 +335,7 @@ describe('scaleUp with GHES', () => { it('returns a retryable failure if runner group lookup fails for ephemeral runners', async () => { process.env.RUNNER_GROUP_NAME = 'test-runner-group'; - mockSSMgetParameter.mockImplementation(async () => { - throw new Error('ParameterNotFound'); - }); + mockRunnerGroupCacheGet.mockRejectedValue(new Error('Cache entry not found')); await expect(scaleUpModule.scaleUp(TEST_DATA)).resolves.toEqual(['foobar']); @@ -345,24 +350,27 @@ describe('scaleUp with GHES', () => { expect(createRunner).not.toHaveBeenCalled(); }); - it('create SSM parameter for runner group id if it does not exist', async () => { - mockSSMgetParameter.mockImplementation(async () => { - throw new Error('ParameterNotFound'); - }); + it('caches the runner group id if it does not exist', async () => { + mockRunnerGroupCacheGet.mockResolvedValue(undefined); + await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerGroupCacheGet).toHaveBeenCalledWith('Default'); expect(mockOctokit.paginate).toHaveBeenCalledTimes(1); - expect(mockSSMClient).toHaveReceivedCommandTimes(PutParameterCommand, 2); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: `${process.env.SSM_CONFIG_PATH}/runner-group/${process.env.RUNNER_GROUP_NAME}`, - Value: '1', - Type: 'String', + expect(mockRunnerGroupCacheCreate).toHaveBeenCalledWith({ + runnerGroupName: 'Default', + runnerGroupId: 1, }); + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(1); }); - it('Does not create SSM parameter for runner group id if it exists', async () => { + it('reuses a cached runner group id', async () => { await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerGroupCacheGet).toHaveBeenCalledWith('Default'); expect(mockOctokit.paginate).toHaveBeenCalledTimes(0); - expect(mockSSMClient).toHaveReceivedCommandTimes(PutParameterCommand, 1); + expect(mockRunnerGroupCacheCreate).not.toHaveBeenCalled(); + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(1); }); it('create start runner config for ephemeral runners ', async () => { @@ -375,17 +383,10 @@ describe('scaleUp with GHES', () => { runner_group_id: 1, labels: ['label1', 'label2'], }); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: 'TEST_JIT_CONFIG_ORG', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-12345', value: 'TEST_JIT_CONFIG_ORG' }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); }); it('create start runner config for non-ephemeral runners ', async () => { @@ -394,19 +395,15 @@ describe('scaleUp with GHES', () => { await scaleUpModule.scaleUp(TEST_DATA); expect(mockOctokit.actions.generateRunnerJitconfigForOrg).not.toBeCalled(); expect(mockOctokit.actions.createRegistrationTokenForOrg).toBeCalled(); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: - '--url https://github.enterprise.something/Codertocat --token 1234abcd ' + - '--labels label1,label2 --runnergroup Default', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { + runnerId: 'i-12345', + value: + '--url https://github.enterprise.something/Codertocat --token 1234abcd ' + + '--labels label1,label2 --runnergroup Default', + }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); }); it('quotes runner labels with semicolon separators in non-ephemeral runner config', async () => { @@ -421,19 +418,15 @@ describe('scaleUp with GHES', () => { }, ]); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: - '--url https://github.enterprise.something/Codertocat --token 1234abcd ' + - "--labels 'label1,label2,ghr-provider-capability:intel;amd' --runnergroup Default", - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { + runnerId: 'i-12345', + value: + '--url https://github.enterprise.something/Codertocat --token 1234abcd ' + + "--labels 'label1,label2,ghr-provider-capability:intel;amd' --runnergroup Default", + }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); }); it('should create JIT config for all remaining instances even when GitHub API fails for one instance', async () => { @@ -493,23 +486,18 @@ describe('scaleUp with GHES', () => { labels: ['label1', 'label2'], }); - expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-1', - Value: 'TEST_JIT_CONFIG_unit-test-i-instance-1', - Type: 'SecureString', - Tags: [{ Key: 'RunnerId', Value: 'i-instance-1' }], - }); - - expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-3', - Value: 'TEST_JIT_CONFIG_unit-test-i-instance-3', - Type: 'SecureString', - Tags: [{ Key: 'RunnerId', Value: 'i-instance-3' }], - }); - - expect(mockSSMClient).not.toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-2', - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-instance-1', value: 'TEST_JIT_CONFIG_unit-test-i-instance-1' }, + { metadata: [{ key: 'RunnerId', value: 'i-instance-1' }] }, + ); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-instance-3', value: 'TEST_JIT_CONFIG_unit-test-i-instance-3' }, + { metadata: [{ key: 'RunnerId', value: 'i-instance-3' }] }, + ); + expect(mockRunnerConfigCreate).not.toHaveBeenCalledWith( + expect.objectContaining({ runnerId: 'i-instance-2' }), + expect.anything(), + ); }); it('should handle retryable errors with error handling logic', async () => { @@ -545,16 +533,14 @@ describe('scaleUp with GHES', () => { await scaleUpModule.scaleUp(TEST_DATA); - expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-2', - Value: 'TEST_JIT_CONFIG_unit-test-i-instance-2', - Type: 'SecureString', - Tags: [{ Key: 'RunnerId', Value: 'i-instance-2' }], - }); - - expect(mockSSMClient).not.toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-1', - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-instance-2', value: 'TEST_JIT_CONFIG_unit-test-i-instance-2' }, + { metadata: [{ key: 'RunnerId', value: 'i-instance-2' }] }, + ); + expect(mockRunnerConfigCreate).not.toHaveBeenCalledWith( + expect.objectContaining({ runnerId: 'i-instance-1' }), + expect.anything(), + ); }); it('should handle non-retryable 4xx errors gracefully', async () => { @@ -591,79 +577,62 @@ describe('scaleUp with GHES', () => { await scaleUpModule.scaleUp(TEST_DATA); - expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-2', - Value: 'TEST_JIT_CONFIG_unit-test-i-instance-2', - Type: 'SecureString', - Tags: [{ Key: 'RunnerId', Value: 'i-instance-2' }], - }); - - expect(mockSSMClient).not.toHaveReceivedCommandWith(PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-instance-1', - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-instance-2', value: 'TEST_JIT_CONFIG_unit-test-i-instance-2' }, + { metadata: [{ key: 'RunnerId', value: 'i-instance-2' }] }, + ); + expect(mockRunnerConfigCreate).not.toHaveBeenCalledWith( + expect.objectContaining({ runnerId: 'i-instance-1' }), + expect.anything(), + ); }); it.each(RUNNER_TYPES)( - 'calls create start runner config of 40' + ' instances (ssm rate limit condition) to test time delay ', + 'paces 40 runner-config writes at the store throughput limit for %s runners', async (type: RunnerLifecycle) => { process.env.ENABLE_EPHEMERAL_RUNNERS = type === 'ephemeral' ? 'true' : 'false'; process.env.RUNNERS_MAXIMUM_COUNT = '40'; + const instances = Array.from({ length: 40 }, (_, index) => `i-${index + 1}`); mockCreateRunner.mockImplementation(async () => { return createRunnerResult(instances); }); mockListRunners.mockImplementation(async () => { return []; }); - const startTime = performance.now(); - const instances = [ - 'i-1234', - 'i-5678', - 'i-5567', - 'i-5569', - 'i-5561', - 'i-5560', - 'i-5566', - 'i-5536', - 'i-5526', - 'i-5516', - 'i-122', - 'i-123', - 'i-124', - 'i-125', - 'i-126', - 'i-127', - 'i-128', - 'i-129', - 'i-130', - 'i-131', - 'i-132', - 'i-133', - 'i-134', - 'i-135', - 'i-136', - 'i-137', - 'i-138', - 'i-139', - 'i-140', - 'i-141', - 'i-142', - 'i-143', - 'i-144', - 'i-145', - 'i-146', - 'i-147', - 'i-148', - 'i-149', - 'i-150', - 'i-151', - ]; - await scaleUpModule.scaleUp(TEST_DATA); - const endTime = performance.now(); - expect(endTime - startTime).toBeGreaterThan(1000); - expect(mockSSMClient).toHaveReceivedCommandTimes(PutParameterCommand, 40); + + const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout').mockImplementation((callback) => { + callback(); + return 0 as unknown as NodeJS.Timeout; + }); + try { + await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(40); + expect(setTimeoutSpy).toHaveBeenCalledTimes(40); + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 25); + } finally { + setTimeoutSpy.mockRestore(); + } }, - 10000, ); + + it('does not pace 39 runner-config writes below the store throughput limit', async () => { + process.env.ENABLE_EPHEMERAL_RUNNERS = 'false'; + process.env.RUNNERS_MAXIMUM_COUNT = '39'; + const instances = Array.from({ length: 39 }, (_, index) => `i-${index + 1}`); + mockCreateRunner.mockResolvedValue(createRunnerResult(instances)); + mockListRunners.mockResolvedValue([]); + const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout'); + + try { + await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(39); + expect(setTimeoutSpy).not.toHaveBeenCalled(); + } finally { + setTimeoutSpy.mockRestore(); + } + }); }); describe('dynamic label groups', () => { @@ -674,7 +643,6 @@ describe('scaleUp with GHES', () => { process.env.RUNNER_LABELS = 'base-label'; process.env.RUNNER_NAME_PREFIX = 'unit-test'; expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; - mockSSMClient.reset(); mockResolveLabelsForRunners.mockImplementation(async (labels) => ({ runnerLabels: labels.filter((label) => label.startsWith('ghr-')), @@ -1150,8 +1118,6 @@ describe('scaleUp with public GH', () => { describe('on repo level', () => { beforeEach(() => { - mockSSMClient.reset(); - process.env.ENABLE_ORGANIZATION_RUNNERS = 'false'; process.env.RUNNER_NAME_PREFIX = 'unit-test'; expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; @@ -1195,44 +1161,32 @@ describe('scaleUp with public GH', () => { it('creates a ephemeral runner with JIT config.', async () => { process.env.ENABLE_EPHEMERAL_RUNNERS = 'true'; process.env.ENABLE_JOB_QUEUED_CHECK = 'false'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; await scaleUpModule.scaleUp(TEST_DATA); expect(mockOctokit.actions.getJobForWorkflowRun).not.toBeCalled(); expect(createRunner).toBeCalledWith(expectedRunnerParams); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: 'TEST_JIT_CONFIG_REPO', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-12345', value: 'TEST_JIT_CONFIG_REPO' }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); + expect(mockGetRunnerGroupCacheStore).not.toHaveBeenCalled(); }); it('creates a ephemeral runner with registration token.', async () => { process.env.ENABLE_EPHEMERAL_RUNNERS = 'true'; process.env.ENABLE_JIT_CONFIG = 'false'; process.env.ENABLE_JOB_QUEUED_CHECK = 'false'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; await scaleUpModule.scaleUp(TEST_DATA); expect(mockOctokit.actions.getJobForWorkflowRun).not.toBeCalled(); expect(createRunner).toBeCalledWith(expectedRunnerParams); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --ephemeral', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { + runnerId: 'i-12345', + value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --ephemeral', + }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); }); it('JIT config is ignored for non-ephemeral runners.', async () => { @@ -1240,22 +1194,18 @@ describe('scaleUp with public GH', () => { process.env.ENABLE_JIT_CONFIG = 'true'; process.env.ENABLE_JOB_QUEUED_CHECK = 'false'; process.env.RUNNER_LABELS = 'jit'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; await scaleUpModule.scaleUp(TEST_DATA); expect(mockOctokit.actions.getJobForWorkflowRun).not.toBeCalled(); expect(createRunner).toBeCalledWith(expectedRunnerParams); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --labels jit', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { + runnerId: 'i-12345', + value: '--url https://github.com/Codertocat/hello-world --token 1234abcd --labels jit', + }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); + expect(mockGetRunnerGroupCacheStore).not.toHaveBeenCalled(); }); it('creates a ephemeral runner after checking job is queued.', async () => { @@ -1534,12 +1484,9 @@ describe('scaleUp with Github Data Residency', () => { process.env.ENABLE_EPHEMERAL_RUNNERS = 'true'; process.env.RUNNER_NAME_PREFIX = 'unit-test-'; process.env.RUNNER_GROUP_NAME = 'Default'; - process.env.SSM_CONFIG_PATH = '/github-action-runners/default/runners/config'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; process.env.RUNNER_LABELS = 'label1,label2'; expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; - mockSSMClient.reset(); }); it('does not create a token when maximum runners has been reached', async () => { @@ -1582,24 +1529,27 @@ describe('scaleUp with Github Data Residency', () => { expect(createRunner).not.toHaveBeenCalled(); }); - it('create SSM parameter for runner group id if it does not exist', async () => { - mockSSMgetParameter.mockImplementation(async () => { - throw new Error('ParameterNotFound'); - }); + it('caches the runner group id if it does not exist', async () => { + mockRunnerGroupCacheGet.mockResolvedValue(undefined); + await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerGroupCacheGet).toHaveBeenCalledWith('Default'); expect(mockOctokit.paginate).toHaveBeenCalledTimes(1); - expect(mockSSMClient).toHaveReceivedCommandTimes(PutParameterCommand, 2); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: `${process.env.SSM_CONFIG_PATH}/runner-group/${process.env.RUNNER_GROUP_NAME}`, - Value: '1', - Type: 'String', + expect(mockRunnerGroupCacheCreate).toHaveBeenCalledWith({ + runnerGroupName: 'Default', + runnerGroupId: 1, }); + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(1); }); - it('Does not create SSM parameter for runner group id if it exists', async () => { + it('reuses a cached runner group id', async () => { await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerGroupCacheGet).toHaveBeenCalledWith('Default'); expect(mockOctokit.paginate).toHaveBeenCalledTimes(0); - expect(mockSSMClient).toHaveReceivedCommandTimes(PutParameterCommand, 1); + expect(mockRunnerGroupCacheCreate).not.toHaveBeenCalled(); + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(1); }); it('create start runner config for ephemeral runners ', async () => { @@ -1612,17 +1562,10 @@ describe('scaleUp with Github Data Residency', () => { runner_group_id: 1, labels: ['label1', 'label2'], }); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: 'TEST_JIT_CONFIG_ORG', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { runnerId: 'i-12345', value: 'TEST_JIT_CONFIG_ORG' }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); }); it('create start runner config for non-ephemeral runners ', async () => { @@ -1631,80 +1574,43 @@ describe('scaleUp with Github Data Residency', () => { await scaleUpModule.scaleUp(TEST_DATA); expect(mockOctokit.actions.generateRunnerJitconfigForOrg).not.toBeCalled(); expect(mockOctokit.actions.createRegistrationTokenForOrg).toBeCalled(); - expect(mockSSMClient).toHaveReceivedNthSpecificCommandWith(1, PutParameterCommand, { - Name: '/github-action-runners/default/runners/config/i-12345', - Value: - '--url https://companyname.ghe.com/Codertocat --token 1234abcd ' + - '--labels label1,label2 --runnergroup Default', - Type: 'SecureString', - Tags: [ - { - Key: 'RunnerId', - Value: 'i-12345', - }, - ], - }); + expect(mockRunnerConfigCreate).toHaveBeenCalledWith( + { + runnerId: 'i-12345', + value: + '--url https://companyname.ghe.com/Codertocat --token 1234abcd ' + + '--labels label1,label2 --runnergroup Default', + }, + { metadata: [{ key: 'RunnerId', value: 'i-12345' }] }, + ); }); it.each(RUNNER_TYPES)( - 'calls create start runner config of 40' + ' instances (ssm rate limit condition) to test time delay ', + 'paces 40 runner-config writes at the store throughput limit for %s runners', async (type: RunnerLifecycle) => { process.env.ENABLE_EPHEMERAL_RUNNERS = type === 'ephemeral' ? 'true' : 'false'; process.env.RUNNERS_MAXIMUM_COUNT = '40'; + const instances = Array.from({ length: 40 }, (_, index) => `i-${index + 1}`); mockCreateRunner.mockImplementation(async () => { return createRunnerResult(instances); }); mockListRunners.mockImplementation(async () => { return []; }); - const startTime = performance.now(); - const instances = [ - 'i-1234', - 'i-5678', - 'i-5567', - 'i-5569', - 'i-5561', - 'i-5560', - 'i-5566', - 'i-5536', - 'i-5526', - 'i-5516', - 'i-122', - 'i-123', - 'i-124', - 'i-125', - 'i-126', - 'i-127', - 'i-128', - 'i-129', - 'i-130', - 'i-131', - 'i-132', - 'i-133', - 'i-134', - 'i-135', - 'i-136', - 'i-137', - 'i-138', - 'i-139', - 'i-140', - 'i-141', - 'i-142', - 'i-143', - 'i-144', - 'i-145', - 'i-146', - 'i-147', - 'i-148', - 'i-149', - 'i-150', - 'i-151', - ]; - await scaleUpModule.scaleUp(TEST_DATA); - const endTime = performance.now(); - expect(endTime - startTime).toBeGreaterThan(1000); - expect(mockSSMClient).toHaveReceivedCommandTimes(PutParameterCommand, 40); + + const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout').mockImplementation((callback) => { + callback(); + return 0 as unknown as NodeJS.Timeout; + }); + try { + await scaleUpModule.scaleUp(TEST_DATA); + + expect(mockRunnerConfigCreate).toHaveBeenCalledTimes(40); + expect(setTimeoutSpy).toHaveBeenCalledTimes(40); + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 25); + } finally { + setTimeoutSpy.mockRestore(); + } }, - 10000, ); }); describe('on repo level', () => { @@ -1989,7 +1895,6 @@ describe('Retry mechanism tests', () => { process.env.ENABLE_JOB_QUEUED_CHECK = 'true'; process.env.RUNNERS_MAXIMUM_COUNT = '10'; expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; - mockSSMClient.reset(); }); const createTestMessages = ( @@ -2157,9 +2062,11 @@ describe('compute provider selection', () => { }); it('rejects unsupported scale-up provider types', async () => { - process.env.COMPUTE_PROVIDER_TYPE = 'microvm'; + process.env.COMPUTE_PROVIDER_TYPE = 'unsupported-provider'; - await expect(scaleUpModule.scaleUp(TEST_DATA)).rejects.toThrow("Unsupported compute provider type 'microvm'"); + await expect(scaleUpModule.scaleUp(TEST_DATA)).rejects.toThrow( + "Unsupported compute provider type 'unsupported-provider'", + ); expect(mockedAppAuth).not.toHaveBeenCalled(); }); }); @@ -2175,11 +2082,8 @@ describe('Multi-app round-robin', () => { process.env.RUNNERS_MAXIMUM_COUNT = '10'; process.env.RUNNER_NAME_PREFIX = 'unit-test-'; process.env.RUNNER_GROUP_NAME = 'Default'; - process.env.SSM_CONFIG_PATH = '/github-action-runners/default/runners/config'; - process.env.SSM_TOKEN_PATH = '/github-action-runners/default/runners/config'; process.env.RUNNER_LABELS = 'label1,label2'; expectedRunnerParams = { ...EXPECTED_RUNNER_PARAMS }; - mockSSMClient.reset(); }); it('passes the same appIndex to createGithubInstallationAuth when multi-app is active', async () => { @@ -2263,7 +2167,7 @@ describe('Multi-app round-robin', () => { }); it('stored installationId takes precedence over webhook payload for additional app', async () => { - // Additional app (index 1) with a pre-configured installation id stored in SSM + // Additional app (index 1) with a pre-configured installation id mockedGetAppCount.mockResolvedValue(2); mockedGetStoredInstallationId.mockResolvedValue(77); mockedAppAuth.mockResolvedValue({ @@ -2333,15 +2237,3 @@ function defaultOctokitMockImpl() { mockOctokit.apps.getOrgInstallation.mockImplementation(() => mockInstallationIdReturnValueOrgs); mockOctokit.apps.getRepoInstallation.mockImplementation(() => mockInstallationIdReturnValueRepos); } - -function defaultSSMGetParameterMockImpl() { - mockSSMgetParameter.mockImplementation(async (name: string) => { - if (name === `${process.env.SSM_CONFIG_PATH}/runner-group/${process.env.RUNNER_GROUP_NAME}`) { - return '1'; - } else if (name === `${process.env.PARAMETER_GITHUB_APP_ID_NAME}`) { - return `${process.env.GITHUB_APP_ID}`; - } else { - throw new Error(`ParameterNotFound: ${name}`); - } - }); -} 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..48733c13c9 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts @@ -11,7 +11,6 @@ import { resolveInstallationId, isJobQueued, UnsupportedEventError, - validateSsmParameterStoreTags, } from './github-runner'; import { publishRetryMessage } from './job-retry'; import type { @@ -80,17 +79,11 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise { - beforeEach(() => { - mockSSMClient.reset(); - mockSSMClient.on(GetParametersByPathCommand).resolves({ - Parameters: undefined, - }); - mockSSMClient.on(GetParametersByPathCommand, { Path: tokenPath }).resolves({ - Parameters: [ - { - Name: tokenPath + 'i-old-01', - LastModifiedDate: dateOld, - }, - ], - NextToken: 'next', - }); - mockSSMClient.on(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }).resolves({ - Parameters: [ - { - Name: tokenPath + 'i-new-01', - LastModifiedDate: now, - }, - ], - NextToken: undefined, - }); - }); - - it('should delete parameters older then minimumDaysOld', async () => { - await cleanSSMTokens({ - dryRun: false, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, - }); - - expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-new-01' }); - }); - - it('should not delete when dry run is activated', async () => { - await cleanSSMTokens({ - dryRun: true, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, - }); - - expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-new-01' }); - }); - - it('should not call delete when no parameters are found.', async () => { - await expect( - cleanSSMTokens({ - dryRun: false, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: 'no-exist', - }), - ).resolves.not.toThrow(); - - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-new-01' }); - }); - - it('should not error on delete failure.', async () => { - mockSSMClient.on(DeleteParameterCommand).rejects(new Error('ParameterNotFound')); - - await expect( - cleanSSMTokens({ - dryRun: false, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, - }), - ).resolves.not.toThrow(); - }); - - it('should only accept valid options.', async () => { - await expect( - cleanSSMTokens({ - dryRun: false, - minimumDaysOld: undefined as unknown as number, - tokenPath: tokenPath, - }), - ).rejects.toBeInstanceOf(Error); - - await expect( - cleanSSMTokens({ - dryRun: false, - minimumDaysOld: 0, - tokenPath: tokenPath, - }), - ).rejects.toBeInstanceOf(Error); - - await expect( - cleanSSMTokens({ - dryRun: false, - minimumDaysOld: 1, - tokenPath: undefined as unknown as string, - }), - ).rejects.toBeInstanceOf(Error); - }); -}); diff --git a/lambdas/functions/webhook/package.json b/lambdas/functions/webhook/package.json index 34f4ef3de9..c596db3493 100644 --- a/lambdas/functions/webhook/package.json +++ b/lambdas/functions/webhook/package.json @@ -29,8 +29,8 @@ }, "dependencies": { "@aws-github-runner/aws-powertools-util": "*", - "@aws-github-runner/aws-ssm-util": "*", "@aws-github-runner/compute-providers": "*", + "@aws-github-runner/storage-providers": "*", "@aws-sdk/client-sqs": "^3.1009.0", "@middy/core": "^6.4.5", "@octokit/rest": "22.0.1", diff --git a/lambdas/functions/webhook/src/ConfigLoader.test.ts b/lambdas/functions/webhook/src/ConfigLoader.test.ts index 9f4e5e5864..41bc66f13b 100644 --- a/lambdas/functions/webhook/src/ConfigLoader.test.ts +++ b/lambdas/functions/webhook/src/ConfigLoader.test.ts @@ -1,11 +1,23 @@ -import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util'; +import { + getGitHubWebhookSecretStore, + getRunnerMatcherConfigStore, + type GitHubWebhookSecretStore, + type RunnerMatcherConfigStore, +} from '@aws-github-runner/storage-providers'; import { ConfigWebhook, ConfigWebhookEventBridge, ConfigDispatcher } from './ConfigLoader'; import { logger } from '@aws-github-runner/aws-powertools-util'; import { RunnerMatcherConfig } from './sqs'; import { describe, it, expect, beforeEach, vi } from 'vitest'; -vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/storage-providers'); + +const githubWebhookSecretStore = { + get: vi.fn(), +} satisfies GitHubWebhookSecretStore; +const runnerMatcherConfigStore = { + get: vi.fn(), +} satisfies RunnerMatcherConfigStore; describe('ConfigLoader Tests', () => { beforeEach(() => { @@ -14,6 +26,8 @@ describe('ConfigLoader Tests', () => { ConfigWebhookEventBridge.reset(); ConfigDispatcher.reset(); logger.setLogLevel('DEBUG'); + vi.mocked(getGitHubWebhookSecretStore).mockReturnValue(githubWebhookSecretStore); + vi.mocked(getRunnerMatcherConfigStore).mockReturnValue(runnerMatcherConfigStore); // clear process.env for (const key of Object.keys(process.env)) { @@ -24,8 +38,6 @@ describe('ConfigLoader Tests', () => { describe('Check base object', () => { function setupConfiguration(): void { process.env.EVENT_BUS_NAME = 'event-bus'; - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; const matcherConfig = [ { id: '1', @@ -36,15 +48,8 @@ describe('ConfigLoader Tests', () => { }, }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(matcherConfig); - } - if (paramPath === '/path/to/webhook/secret') { - return 'secret'; - } - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig)); + githubWebhookSecretStore.get.mockResolvedValue('secret'); } it('should return the same instance of ConfigWebhook (singleton)', async () => { @@ -53,7 +58,8 @@ describe('ConfigLoader Tests', () => { const config2 = await ConfigWebhook.load(); expect(config1).toBe(config2); - expect(getParameter).toHaveBeenCalledTimes(2); + expect(githubWebhookSecretStore.get).toHaveBeenCalledOnce(); + expect(runnerMatcherConfigStore.get).toHaveBeenCalledOnce(); }); it('should return the same instance of ConfigWebhookEventBridge (singleton)', async () => { @@ -62,7 +68,8 @@ describe('ConfigLoader Tests', () => { const config2 = await ConfigWebhookEventBridge.load(); expect(config1).toBe(config2); - expect(getParameter).toHaveBeenCalledTimes(1); + expect(githubWebhookSecretStore.get).toHaveBeenCalledOnce(); + expect(runnerMatcherConfigStore.get).not.toHaveBeenCalled(); }); it('should return the same instance of ConfigDispatcher (singleton)', async () => { @@ -71,7 +78,8 @@ describe('ConfigLoader Tests', () => { const config2 = await ConfigDispatcher.load(); expect(config1).toBe(config2); - expect(getParameter).toHaveBeenCalledTimes(1); + expect(githubWebhookSecretStore.get).not.toHaveBeenCalled(); + expect(runnerMatcherConfigStore.get).toHaveBeenCalledOnce(); }); it('should filter secrets from being logged', async () => { @@ -94,8 +102,6 @@ describe('ConfigLoader Tests', () => { describe('ConfigWebhook', () => { it('should load config successfully', async () => { process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; const matcherConfig = [ { id: '1', @@ -106,15 +112,8 @@ describe('ConfigLoader Tests', () => { }, }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(matcherConfig); - } - if (paramPath === '/path/to/webhook/secret') { - return 'secret'; - } - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig)); + githubWebhookSecretStore.get.mockResolvedValue('secret'); const config: ConfigWebhook = await ConfigWebhook.load(); @@ -124,8 +123,6 @@ describe('ConfigLoader Tests', () => { }); it('should load config successfully', async () => { - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; const matcherConfig = [ { id: '1', @@ -136,15 +133,8 @@ describe('ConfigLoader Tests', () => { }, }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(matcherConfig); - } - if (paramPath === '/path/to/webhook/secret') { - return 'secret'; - } - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig)); + githubWebhookSecretStore.get.mockResolvedValue('secret'); const config: ConfigWebhook = await ConfigWebhook.load(); @@ -155,46 +145,25 @@ describe('ConfigLoader Tests', () => { }); it('should throw error if config loading fails', async () => { - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; - - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - throw new Error('Failed to load matcher config'); - } - return ''; - }); + runnerMatcherConfigStore.get.mockRejectedValue( + new Error( + 'Failed to load parameter for matcherConfig from path /path/to/matcher/config: Failed to load matcher config', + ), + ); + githubWebhookSecretStore.get.mockResolvedValue(''); await expect(ConfigWebhook.load()).rejects.toThrow( 'Failed to load config: Failed to load parameter for matcherConfig from path /path/to/matcher/config: Failed to load matcher config', ); }); - it('should load config successfully from multiple paths', async () => { - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config-1:/path/to/matcher/config-2'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; - - const partialMatcher1 = - '[{"id":"1","arn":"arn:aws:sqs:queue1","matcherConfig":{"labelMatchers":[["a"]],"exactMatch":true}}'; - const partialMatcher2 = - ',{"id":"2","arn":"arn:aws:sqs:queue2","matcherConfig":{"labelMatchers":[["b"]],"exactMatch":true}}]'; - + it('should load combined matcher config returned by the store', async () => { const combinedMatcherConfig = [ { id: '1', arn: 'arn:aws:sqs:queue1', matcherConfig: { labelMatchers: [['a']], exactMatch: true } }, { id: '2', arn: 'arn:aws:sqs:queue2', matcherConfig: { labelMatchers: [['b']], exactMatch: true } }, ]; - - // Mock getParameters for batch fetching multiple paths - vi.mocked(getParameters).mockResolvedValue( - new Map([ - ['/path/to/matcher/config-1', partialMatcher1], - ['/path/to/matcher/config-2', partialMatcher2], - ]), - ); - - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/webhook/secret') return 'secret'; - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(combinedMatcherConfig)); + githubWebhookSecretStore.get.mockResolvedValue('secret'); const config: ConfigWebhook = await ConfigWebhook.load(); @@ -202,27 +171,13 @@ describe('ConfigLoader Tests', () => { expect(config.webhookSecret).toBe('secret'); }); - it('should throw error if config loading fails from multiple paths', async () => { - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config-1:/path/to/matcher/config-2'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; - - const partialMatcher1 = - '[{"id":"1","arn":"arn:aws:sqs:queue1","matcherConfig":{"labelMatchers":[["a"]],"exactMatch":true}}'; - const partialMatcher2 = - ',{"id":"2","arn":"arn:aws:sqs:queue2","matcherConfig":{"labelMatchers":[["b"]],"exactMatch":true}}'; - - // Mock getParameters for batch fetching - returns incomplete JSON that will fail to parse - vi.mocked(getParameters).mockResolvedValue( - new Map([ - ['/path/to/matcher/config-1', partialMatcher1], - ['/path/to/matcher/config-2', partialMatcher2], - ]), + it('should propagate an error from the matcher config store', async () => { + runnerMatcherConfigStore.get.mockRejectedValue( + new Error( + "Failed to load/parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196", + ), ); - - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/webhook/secret') return 'secret'; - return ''; - }); + githubWebhookSecretStore.get.mockResolvedValue('secret'); await expect(ConfigWebhook.load()).rejects.toThrow( "Failed to load config: Failed to load/parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196", @@ -234,37 +189,40 @@ describe('ConfigLoader Tests', () => { it('should load config successfully', async () => { process.env.ACCEPT_EVENTS = '["push", "pull_request"]'; process.env.EVENT_BUS_NAME = 'event-bus'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; - - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/webhook/secret') { - return 'secret'; - } - return ''; - }); + githubWebhookSecretStore.get.mockResolvedValue('secret'); const config: ConfigWebhookEventBridge = await ConfigWebhookEventBridge.load(); expect(config.allowedEvents).toEqual(['push', 'pull_request']); expect(config.eventBusName).toBe('event-bus'); expect(config.webhookSecret).toBe('secret'); + expect(runnerMatcherConfigStore.get).not.toHaveBeenCalled(); }); it('should throw error if config loading fails', async () => { - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - throw new Error(`Parameter ${paramPath} not found`); + githubWebhookSecretStore.get.mockRejectedValue(new Error('Webhook secret store is unavailable')); + + await expect(ConfigWebhookEventBridge.load()).rejects.toThrow( + 'Failed to load config: Environment variable for eventBusName is not set and no default value provided., Webhook secret store is unavailable', + ); + }); + + it('should report an error selecting the webhook secret store', async () => { + process.env.EVENT_BUS_NAME = 'event-bus'; + vi.mocked(getGitHubWebhookSecretStore).mockImplementationOnce(() => { + throw new Error("Unsupported runner config storage provider 'not-registered'"); }); await expect(ConfigWebhookEventBridge.load()).rejects.toThrow( - 'Failed to load config: Environment variable for eventBusName is not set and no default value provided., Failed to load parameter for webhookSecret from path undefined: Parameter undefined not found', + "Failed to load config: Unsupported runner config storage provider 'not-registered'", ); + expect(githubWebhookSecretStore.get).not.toHaveBeenCalled(); }); }); describe('ConfigDispatcher', () => { it('should load config successfully', async () => { process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]'; - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; const matcherConfig: RunnerMatcherConfig[] = [ { @@ -276,12 +234,7 @@ describe('ConfigLoader Tests', () => { }, }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(matcherConfig); - } - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig)); const config: ConfigDispatcher = await ConfigDispatcher.load(); @@ -289,27 +242,14 @@ describe('ConfigLoader Tests', () => { expect(config.matcherConfig).toEqual(matcherConfig); }); - it('should load config successfully from multiple paths with repo allow list', async () => { + it('should load combined matcher config returned by the store with repo allow list', async () => { process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]'; - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config-1:/path/to/matcher/config-2'; - - const partial1 = - '[{"id":"1","arn":"arn:aws:sqs:queue1","matcherConfig":{"labelMatchers":[["x"]],"exactMatch":true}}'; - const partial2 = - ',{"id":"2","arn":"arn:aws:sqs:queue2","matcherConfig":{"labelMatchers":[["y"]],"exactMatch":true}}]'; const combined: RunnerMatcherConfig[] = [ { id: '1', arn: 'arn:aws:sqs:queue1', matcherConfig: { labelMatchers: [['x']], exactMatch: true } }, { id: '2', arn: 'arn:aws:sqs:queue2', matcherConfig: { labelMatchers: [['y']], exactMatch: true } }, ]; - - // Mock getParameters for batch fetching multiple paths - vi.mocked(getParameters).mockResolvedValue( - new Map([ - ['/path/to/matcher/config-1', partial1], - ['/path/to/matcher/config-2', partial2], - ]), - ); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(combined)); const config: ConfigDispatcher = await ConfigDispatcher.load(); @@ -318,18 +258,15 @@ describe('ConfigLoader Tests', () => { }); it('should throw error if config loading fails', async () => { - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - throw new Error(`Parameter ${paramPath} not found`); - }); + runnerMatcherConfigStore.get.mockRejectedValue(new Error('Matcher config store is unavailable')); await expect(ConfigDispatcher.load()).rejects.toThrow( - 'Failed to load config: Failed to load parameter for matcherConfig from path undefined: Parameter undefined not found', + 'Failed to load config: Matcher config store is unavailable', ); }); it('should rely on default when optionals are not set.', async () => { process.env.ACCEPT_EVENTS = 'null'; - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; const matcherConfig: RunnerMatcherConfig[] = [ { arn: 'arn:aws:sqs:eu-central-1:123456:npalm-default-queued-builds', @@ -340,12 +277,7 @@ describe('ConfigLoader Tests', () => { }, }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(matcherConfig); - } - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig)); const config: ConfigDispatcher = await ConfigDispatcher.load(); @@ -355,14 +287,7 @@ describe('ConfigLoader Tests', () => { it('should throw an error if runner matcher config is empty.', async () => { process.env.REPOSITORY_ALLOW_LIST = '["repo1", "repo2"]'; - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; - - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(''); - } - return ''; - }); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify('')); await expect(ConfigDispatcher.load()).rejects.toThrow('Failed to load config: Matcher config is empty'); }); diff --git a/lambdas/functions/webhook/src/ConfigLoader.ts b/lambdas/functions/webhook/src/ConfigLoader.ts index d9d9da2590..e6d1d65004 100644 --- a/lambdas/functions/webhook/src/ConfigLoader.ts +++ b/lambdas/functions/webhook/src/ConfigLoader.ts @@ -1,9 +1,9 @@ -import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util'; +import { getGitHubWebhookSecretStore, getRunnerMatcherConfigStore } from '@aws-github-runner/storage-providers'; import { RunnerMatcherConfig } from './sqs'; import { logger } from '@aws-github-runner/aws-powertools-util'; /** - * Base class for loading configuration from environment variables and SSM parameters. + * Base class for loading configuration from environment variables and configuration stores. * * @remarks * To avoid usages or checking values can be undefined we assume that configuration is @@ -54,19 +54,15 @@ abstract class BaseConfig { } } - protected async loadParameter(paramPath: string, propertyName: keyof this): Promise { - logger.debug(`Loading parameter for ${String(propertyName)} from path ${paramPath}`); - await getParameter(paramPath) - .then((value) => { - this.loadProperty(propertyName, value); - }) - .catch((error) => { - const errorMessage = `Failed to load parameter for ${String(propertyName)} from path ${paramPath}: ${(error as Error).message}`; - this.configLoadingErrors.push(errorMessage); - }); + protected async loadStoredProperty(propertyName: keyof this, getValue: () => Promise): Promise { + try { + this.loadProperty(propertyName, await getValue()); + } catch (error) { + this.configLoadingErrors.push((error as Error).message); + } } - private loadProperty(propertyName: keyof this, value: string) { + protected loadProperty(propertyName: keyof this, value: string) { try { this[propertyName] = JSON.parse(value) as unknown as this[keyof this]; } catch { @@ -96,38 +92,11 @@ abstract class MatcherAwareConfig extends BaseConfig { // across the matching queues to avoid concentrating load on a single one. queueSelectionStrategy: QueueSelectionStrategy = 'first'; - protected async loadMatcherConfig(paramPathsEnv: string) { - if (!paramPathsEnv || paramPathsEnv === 'undefined' || paramPathsEnv === 'null' || !paramPathsEnv.includes(':')) { - // Single path or invalid string → load directly - await this.loadParameter(paramPathsEnv, 'matcherConfig'); - return; - } - - const paths = paramPathsEnv - .split(':') - .map((p) => p.trim()) - .filter(Boolean); - - // Batch fetch all matcher config paths in a single SSM API call + protected async loadMatcherConfig() { try { - const params = await getParameters(paths); - let combinedString = ''; - for (const path of paths) { - const value = params.get(path); - if (value) { - combinedString += value; - } else { - this.configLoadingErrors.push( - `Failed to load parameter for matcherConfig from path ${path}: Parameter not found`, - ); - } - } - - if (combinedString) { - this.matcherConfig = JSON.parse(combinedString); - } + this.loadProperty('matcherConfig', await getRunnerMatcherConfigStore().get()); } catch (error) { - this.configLoadingErrors.push(`Failed to load/parse combined matcher config: ${(error as Error).message}`); + this.configLoadingErrors.push((error as Error).message); } } } @@ -142,8 +111,8 @@ export class ConfigWebhook extends MatcherAwareConfig { this.loadEnvVar(process.env.QUEUE_SELECTION_STRATEGY, 'queueSelectionStrategy', 'first'); await Promise.all([ - this.loadMatcherConfig(process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH), - this.loadParameter(process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET, 'webhookSecret'), + this.loadMatcherConfig(), + this.loadStoredProperty('webhookSecret', () => getGitHubWebhookSecretStore().get()), ]); validateWebhookSecret(this); @@ -160,7 +129,7 @@ export class ConfigWebhookEventBridge extends BaseConfig { async loadConfig(): Promise { this.loadEnvVar(process.env.ACCEPT_EVENTS, 'allowedEvents', []); this.loadEnvVar(process.env.EVENT_BUS_NAME, 'eventBusName'); - await this.loadParameter(process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET, 'webhookSecret'); + await this.loadStoredProperty('webhookSecret', () => getGitHubWebhookSecretStore().get()); validateEventBusName(this); validateWebhookSecret(this); @@ -174,7 +143,7 @@ export class ConfigDispatcher extends MatcherAwareConfig { async loadConfig(): Promise { this.loadEnvVar(process.env.REPOSITORY_ALLOW_LIST, 'repositoryAllowList', []); this.loadEnvVar(process.env.QUEUE_SELECTION_STRATEGY, 'queueSelectionStrategy', 'first'); - await this.loadMatcherConfig(process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH); + await this.loadMatcherConfig(); validateRunnerMatcherConfig(this); validateQueueSelectionStrategy(this); diff --git a/lambdas/functions/webhook/src/lambda.test.ts b/lambdas/functions/webhook/src/lambda.test.ts index d65b8371c4..b325c002f4 100644 --- a/lambdas/functions/webhook/src/lambda.test.ts +++ b/lambdas/functions/webhook/src/lambda.test.ts @@ -6,7 +6,12 @@ import { WorkflowJobEvent } from '@octokit/webhooks-types'; import { dispatchToRunners, eventBridgeWebhook, directWebhook } from './lambda'; import { publishForRunners, publishOnEventBridge } from './webhook'; import ValidationError from './ValidationError'; -import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { + getGitHubWebhookSecretStore, + getRunnerMatcherConfigStore, + type GitHubWebhookSecretStore, + type RunnerMatcherConfigStore, +} from '@aws-github-runner/storage-providers'; import { dispatch } from './runners/dispatch'; import { EventWrapper } from './types'; import { describe, it, expect, beforeEach, vi } from 'vitest'; @@ -79,15 +84,24 @@ const context: Context = { vi.mock('./runners/dispatch'); vi.mock('./webhook'); -vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/storage-providers'); + +const githubWebhookSecretStore = { + get: vi.fn(), +} satisfies GitHubWebhookSecretStore; +const runnerMatcherConfigStore = { + get: vi.fn(), +} satisfies RunnerMatcherConfigStore; describe('Test webhook lambda wrapper.', () => { beforeEach(() => { - // We mock all SSM request to resolve to a non empty array. Since we mock all implemeantions - // relying on the config object that is enough to test the handlers. - const mockedGet = vi.mocked(getParameter); - mockedGet.mockResolvedValue('["abc"]'); vi.clearAllMocks(); + // The handlers only need non-empty config values because their downstream + // implementations are mocked in this wrapper test. + vi.mocked(getGitHubWebhookSecretStore).mockReturnValue(githubWebhookSecretStore); + githubWebhookSecretStore.get.mockResolvedValue('["abc"]'); + vi.mocked(getRunnerMatcherConfigStore).mockReturnValue(runnerMatcherConfigStore); + runnerMatcherConfigStore.get.mockResolvedValue('["abc"]'); }); describe('Test webhook lambda wrapper.', () => { diff --git a/lambdas/functions/webhook/src/modules.d.ts b/lambdas/functions/webhook/src/modules.d.ts index 9110746709..05a81a12ab 100644 --- a/lambdas/functions/webhook/src/modules.d.ts +++ b/lambdas/functions/webhook/src/modules.d.ts @@ -2,8 +2,6 @@ declare namespace NodeJS { export interface ProcessEnv { ENVIRONMENT: string; EVENT_BUS_NAME: string; - PARAMETER_GITHUB_APP_WEBHOOK_SECRET: string; - PARAMETER_RUNNER_MATCHER_CONFIG_PATH: string; QUEUE_SELECTION_STRATEGY: string; REPOSITORY_ALLOW_LIST: string; RUNNER_LABELS: string; diff --git a/lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts b/lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts deleted file mode 100644 index 98bba55b30..0000000000 --- a/lambdas/functions/webhook/src/runners/aws-dynamic-labels-policy.ts +++ /dev/null @@ -1 +0,0 @@ -export type { AwsDynamicLabelsPolicy, AwsDynamicLabelsValueRule } from '@aws-github-runner/compute-providers'; diff --git a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.test.ts b/lambdas/functions/webhook/src/runners/aws-dynamic-labels.test.ts deleted file mode 100644 index 790d4c2989..0000000000 --- a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import type { ComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; -import { describe, expect, it } from 'vitest'; - -import type { RunnerMatcherConfig } from '../sqs'; -import { selectAwsDynamicLabelQueue } from './aws-dynamic-labels'; - -describe('selectAwsDynamicLabelQueue', () => { - it('defaults queues without a provider to EC2 dynamic label handling', () => { - const queue = runnerQueue('default-ec2'); - - expect(selectAwsDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); - - it('normalizes compute provider casing and surrounding whitespace', () => { - const queue = runnerQueue('normalized-ec2'); - (queue as unknown as { computeProvider: string }).computeProvider = ' EC2 '; - - expect(selectAwsDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); - - it('skips an unsupported provider strategy and selects the next supported queue', () => { - const unsupportedQueue = runnerQueue('unsupported-provider'); - (unsupportedQueue as unknown as { computeProvider: string }).computeProvider = 'unsupported'; - const ec2Queue = runnerQueue('ec2'); - - expect( - selectAwsDynamicLabelQueue( - [unsupportedQueue, ec2Queue], - ['self-hosted', 'linux'], - ['ghr-ec2-instance-type:t3.large'], - ), - ).toEqual({ - queue: ec2Queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); - }); - - it('rejects a malformed non-string compute provider without throwing', () => { - const queue = runnerQueue('malformed-provider'); - (queue as unknown as { computeProvider: number }).computeProvider = 42; - - expect( - selectAwsDynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); - }); -}); - -function runnerQueue(id: string, computeProvider?: ComputeProviderType): RunnerMatcherConfig { - return { - id, - arn: `arn:${id}`, - computeProvider, - matcherConfig: { - labelMatchers: [['self-hosted', 'linux']], - exactMatch: true, - enableDynamicLabels: true, - }, - }; -} diff --git a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts b/lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts deleted file mode 100644 index 418697398a..0000000000 --- a/lambdas/functions/webhook/src/runners/aws-dynamic-labels.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; -import type { DynamicLabelDispatchTarget } from '@aws-github-runner/compute-providers'; -import { normalizeComputeProviderType } from '@aws-github-runner/compute-providers/provider-types'; -import { webhookProviderRegistry } from '@aws-github-runner/compute-providers/webhook'; - -import type { RunnerMatcherConfig } from '../sqs'; - -const logger = createChildLogger('handler'); - -export function selectAwsDynamicLabelQueue( - matches: RunnerMatcherConfig[], - nonGhrLabels: string[], - sanitizedGhrLabels: string[], -): DynamicLabelDispatchTarget | undefined { - for (const queue of matches) { - const provider = normalizeComputeProviderType(queue.computeProvider); - const dynamicLabels = provider ? webhookProviderRegistry.capability(provider, 'dynamicLabels') : undefined; - - if (!dynamicLabels) { - logger.warn(`Queue ${queue.id} has unsupported compute provider '${provider ?? String(queue.computeProvider)}'`); - continue; - } - - const target = dynamicLabels.selectQueue({ queue, nonGhrLabels, sanitizedGhrLabels }); - if (target) return target; - } - - return undefined; -} diff --git a/lambdas/functions/webhook/src/runners/dispatch.test.ts b/lambdas/functions/webhook/src/runners/dispatch.test.ts index ae571da9d8..b3140d6e96 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.test.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.test.ts @@ -1,4 +1,5 @@ -import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { selectDynamicLabelQueue } from '@aws-github-runner/compute-providers/webhook'; +import { getRunnerMatcherConfigStore, type RunnerMatcherConfigStore } from '@aws-github-runner/storage-providers'; import nock from 'nock'; import { WorkflowJobEvent } from '@octokit/webhooks-types'; @@ -13,9 +14,14 @@ import { logger } from '@aws-github-runner/aws-powertools-util'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; vi.mock('../sqs'); -vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/storage-providers'); +vi.mock('@aws-github-runner/compute-providers/webhook', () => ({ + selectDynamicLabelQueue: vi.fn(), +})); -const GITHUB_APP_WEBHOOK_SECRET = 'TEST_SECRET'; +const runnerMatcherConfigStore = { + get: vi.fn(), +} satisfies RunnerMatcherConfigStore; const cleanEnv = process.env; @@ -33,7 +39,6 @@ describe('Dispatcher', () => { vi.clearAllMocks(); vi.resetAllMocks(); - mockSSMResponse(); config = await createConfig(undefined, runnerConfig); }); @@ -238,7 +243,7 @@ describe('Dispatcher', () => { it('rejects an invalid strategy at config load', async () => { process.env.QUEUE_SELECTION_STRATEGY = 'bogus'; ConfigDispatcher.reset(); - mockSSMResponse(twoExactMatches); + mockMatcherConfigResponse(twoExactMatches); await expect(ConfigDispatcher.load()).rejects.toThrow(/queue selection strategy/i); }); }); @@ -246,7 +251,14 @@ describe('Dispatcher', () => { describe('per-matcher dynamic labels handling', () => { const baseRunner = runnerConfig[0]; - it('strips invalid ghr- labels (too long, bad chars) before policy and dispatch', async () => { + beforeEach(() => { + vi.mocked(selectDynamicLabelQueue).mockImplementation((matches, nonGhrLabels, sanitizedGhrLabels) => ({ + queue: matches[0], + labels: [...nonGhrLabels, ...sanitizedGhrLabels], + })); + }); + + it('strips invalid ghr- labels before provider selection and dispatch', async () => { const longLabel = 'ghr-' + 'a'.repeat(125); // 129 chars config = await createConfig(undefined, [ { @@ -276,19 +288,25 @@ describe('Dispatcher', () => { } as unknown as WorkflowJobEvent; const resp = await dispatch(event, 'workflow_job', config); expect(resp.statusCode).toBe(201); + expect(selectDynamicLabelQueue).toHaveBeenCalledWith( + [expect.objectContaining({ id: baseRunner.id })], + ['self-hosted', 'linux'], + ['ghr-valid:value', 'ghr-list:value;another'], + ); expect(sendActionRequest).toHaveBeenCalledWith( expect.objectContaining({ labels: ['self-hosted', 'linux', 'ghr-valid:value', 'ghr-list:value;another'] }), ); }); - it('rejects the job (202) when the only matching runner has enableDynamicLabels=false', async () => { + it('rejects the job when no provider accepts the dynamic labels', async () => { + vi.mocked(selectDynamicLabelQueue).mockReturnValue(undefined); config = await createConfig(undefined, [ { ...baseRunner, matcherConfig: { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, - enableDynamicLabels: false, + enableDynamicLabels: true, }, }, ]); @@ -296,7 +314,7 @@ describe('Dispatcher', () => { ...workFlowJobEvent, workflow_job: { ...workFlowJobEvent.workflow_job, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + labels: ['self-hosted', 'linux', 'ghr-provider-setting:value'], }, } as unknown as WorkflowJobEvent; const resp = await dispatch(event, 'workflow_job', config); @@ -304,50 +322,20 @@ describe('Dispatcher', () => { expect(sendActionRequest).not.toHaveBeenCalled(); }); - it('keeps dynamic labels when the matched runner enables them and has no policy', async () => { - config = await createConfig(undefined, [ - { - ...baseRunner, - matcherConfig: { - labelMatchers: [['self-hosted', 'linux']], - exactMatch: true, - enableDynamicLabels: true, - }, - }, - ]); - const event = { - ...workFlowJobEvent, - workflow_job: { - ...workFlowJobEvent.workflow_job, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }, - } as unknown as WorkflowJobEvent; - const resp = await dispatch(event, 'workflow_job', config); - expect(resp.statusCode).toBe(201); - expect(sendActionRequest).toHaveBeenCalledWith( - expect.objectContaining({ labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'] }), - ); - }); - - it('skips a matching runner whose policy rejects the dynamic labels and uses the next compliant one', async () => { + it('dispatches to the queue and labels returned by the provider selector', async () => { config = await createConfig(undefined, [ { ...baseRunner, - id: 'strict', + id: 'first', matcherConfig: { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, enableDynamicLabels: true, - awsDynamicLabelsPolicy: { - restricted_keys: { - 'instance-type': { allowed: ['m5.*'] }, - }, - }, }, }, { ...baseRunner, - id: 'permissive', + id: 'selected', matcherConfig: { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, @@ -355,61 +343,29 @@ describe('Dispatcher', () => { }, }, ]); + + vi.mocked(selectDynamicLabelQueue).mockImplementation((matches) => ({ + queue: matches[1], + labels: ['self-hosted', 'linux', 'ghr-provider-setting:normalized'], + })); + const event = { ...workFlowJobEvent, workflow_job: { ...workFlowJobEvent.workflow_job, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + labels: ['self-hosted', 'linux', 'ghr-provider-setting:requested'], }, } as unknown as WorkflowJobEvent; const resp = await dispatch(event, 'workflow_job', config); expect(resp.statusCode).toBe(201); expect(sendActionRequest).toHaveBeenCalledWith( expect.objectContaining({ - queueId: 'permissive', - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], + queueId: 'selected', + labels: ['self-hosted', 'linux', 'ghr-provider-setting:normalized'], }), ); }); - it('rejects the job (202) when no runner accepts the policy', async () => { - config = await createConfig(undefined, [ - { - ...baseRunner, - id: 'first', - matcherConfig: { - labelMatchers: [['self-hosted', 'linux']], - exactMatch: true, - enableDynamicLabels: true, - awsDynamicLabelsPolicy: { - restricted_keys: { - 'instance-type': { allowed: ['m5.*'] }, - }, - }, - }, - }, - { - ...baseRunner, - id: 'second', - matcherConfig: { - labelMatchers: [['self-hosted', 'linux']], - exactMatch: true, - enableDynamicLabels: false, - }, - }, - ]); - const event = { - ...workFlowJobEvent, - workflow_job: { - ...workFlowJobEvent.workflow_job, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }, - } as unknown as WorkflowJobEvent; - const resp = await dispatch(event, 'workflow_job', config); - expect(resp.statusCode).toBe(202); - expect(sendActionRequest).not.toHaveBeenCalled(); - }); - it('forwards non-dynamic jobs as-is to the first match', async () => { config = await createConfig(undefined, [ { @@ -419,7 +375,6 @@ describe('Dispatcher', () => { labelMatchers: [['self-hosted', 'linux']], exactMatch: true, enableDynamicLabels: true, - awsDynamicLabelsPolicy: {}, }, }, ]); @@ -435,20 +390,14 @@ describe('Dispatcher', () => { expect(sendActionRequest).toHaveBeenCalledWith( expect.objectContaining({ queueId: 'first', labels: ['self-hosted', 'linux'] }), ); + expect(selectDynamicLabelQueue).not.toHaveBeenCalled(); }); }); }); -function mockSSMResponse(runnerConfigInput?: RunnerConfig) { - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/github-runner/runner-matcher-config'; - const mockedGet = vi.mocked(getParameter); - mockedGet.mockImplementation((parameter_name) => { - const value = - parameter_name == '/github-runner/runner-matcher-config' - ? JSON.stringify(runnerConfigInput ?? runnerConfig) - : GITHUB_APP_WEBHOOK_SECRET; - return Promise.resolve(value); - }); +function mockMatcherConfigResponse(runnerConfigInput?: RunnerConfig) { + vi.mocked(getRunnerMatcherConfigStore).mockReturnValue(runnerMatcherConfigStore); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(runnerConfigInput ?? runnerConfig)); } async function createConfig(repositoryAllowList?: string[], runnerConfig?: RunnerConfig): Promise { @@ -456,6 +405,6 @@ async function createConfig(repositoryAllowList?: string[], runnerConfig?: Runne process.env.REPOSITORY_ALLOW_LIST = JSON.stringify(repositoryAllowList); } ConfigDispatcher.reset(); - mockSSMResponse(runnerConfig); + mockMatcherConfigResponse(runnerConfig); return await ConfigDispatcher.load(); } diff --git a/lambdas/functions/webhook/src/runners/dispatch.ts b/lambdas/functions/webhook/src/runners/dispatch.ts index 47c1f1bfc0..da6dc01221 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.ts @@ -1,11 +1,11 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; +import { selectDynamicLabelQueue } from '@aws-github-runner/compute-providers/webhook'; import { WorkflowJobEvent } from '@octokit/webhooks-types'; import { Response } from '../lambda'; import { RunnerMatcherConfig, sendActionRequest } from '../sqs'; import ValidationError from '../ValidationError'; import { ConfigDispatcher, ConfigWebhook, QueueSelectionStrategy } from '../ConfigLoader'; -import { selectAwsDynamicLabelQueue } from './aws-dynamic-labels'; import { canRunJob, splitWorkflowJobLabels } from './labels'; const logger = createChildLogger('handler'); @@ -84,7 +84,7 @@ async function handleWorkflowJob( // Dynamic labels present: prefer the first provider-compliant queue. The // queue selection strategy applies to standard jobs only; dynamic-label jobs // always use the first compliant queue. - const dynamicTarget = selectAwsDynamicLabelQueue(matches, nonGhrLabels, sanitizedGhrLabels); + const dynamicTarget = selectDynamicLabelQueue(matches, nonGhrLabels, sanitizedGhrLabels); if (dynamicTarget) { targets = [dynamicTarget.queue]; diff --git a/lambdas/functions/webhook/src/webhook/index.test.ts b/lambdas/functions/webhook/src/webhook/index.test.ts index aa4fbbc506..6d7a272309 100644 --- a/lambdas/functions/webhook/src/webhook/index.test.ts +++ b/lambdas/functions/webhook/src/webhook/index.test.ts @@ -1,5 +1,10 @@ import { Webhooks } from '@octokit/webhooks'; -import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { + getGitHubWebhookSecretStore, + getRunnerMatcherConfigStore, + type GitHubWebhookSecretStore, + type RunnerMatcherConfigStore, +} from '@aws-github-runner/storage-providers'; import nock from 'nock'; @@ -15,9 +20,15 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; vi.mock('../sqs'); vi.mock('../eventbridge'); vi.mock('../runners/dispatch'); -vi.mock('@aws-github-runner/aws-ssm-util'); +vi.mock('@aws-github-runner/storage-providers'); const GITHUB_APP_WEBHOOK_SECRET = 'TEST_SECRET'; +const githubWebhookSecretStore = { + get: vi.fn(), +} satisfies GitHubWebhookSecretStore; +const runnerMatcherConfigStore = { + get: vi.fn(), +} satisfies RunnerMatcherConfigStore; const cleanEnv = process.env; @@ -32,7 +43,7 @@ describe('handle GitHub webhook events', () => { nock.disableNetConnect(); vi.clearAllMocks(); - mockSSMResponse(); + mockConfigResponse(); }); describe('handle and dispatch webhook events to build queues', () => { @@ -284,9 +295,7 @@ describe('Check message size (checkBodySize)', () => { }); }); -function mockSSMResponse() { - process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/path/to/matcher/config'; - process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = '/path/to/webhook/secret'; +function mockConfigResponse() { const matcherConfig = [ { id: '1', @@ -297,13 +306,8 @@ function mockSSMResponse() { }, }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config') { - return JSON.stringify(matcherConfig); - } - if (paramPath === '/path/to/webhook/secret') { - return GITHUB_APP_WEBHOOK_SECRET; - } - throw new Error('Parameter not found'); - }); + vi.mocked(getGitHubWebhookSecretStore).mockReturnValue(githubWebhookSecretStore); + vi.mocked(getRunnerMatcherConfigStore).mockReturnValue(runnerMatcherConfigStore); + githubWebhookSecretStore.get.mockResolvedValue(GITHUB_APP_WEBHOOK_SECRET); + runnerMatcherConfigStore.get.mockResolvedValue(JSON.stringify(matcherConfig)); } diff --git a/lambdas/libs/aws-ssm-util/src/index.test.ts b/lambdas/libs/aws-ssm-util/src/index.test.ts index 8a1d8d3864..f027b16293 100644 --- a/lambdas/libs/aws-ssm-util/src/index.test.ts +++ b/lambdas/libs/aws-ssm-util/src/index.test.ts @@ -127,6 +127,27 @@ describe('Test getParameter and putParameter', () => { }); }); + it('passes tags to the PutParameter command', async () => { + const parameterValue = 'test'; + const parameterName = 'testParam'; + const tags = [{ Key: 'InstanceId', Value: 'i-123' }]; + const output: PutParameterCommandOutput = { + $metadata: { + httpStatusCode: 200, + }, + }; + mockSSMClient.on(PutParameterCommand).resolves(output); + + await putParameter(parameterName, parameterValue, true, { tags }); + + expect(mockSSMClient).toHaveReceivedCommandWith(PutParameterCommand, { + Name: parameterName, + Value: parameterValue, + Type: 'SecureString', + Tags: tags, + }); + }); + it('Gets invalid parameters and returns string', async () => { // Arrange const parameterName = 'invalid'; diff --git a/lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts b/lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts new file mode 100644 index 0000000000..64b7507add --- /dev/null +++ b/lambdas/libs/compute-providers/aws/dynamic-labels-policy.ts @@ -0,0 +1,61 @@ +import type { AwsDynamicLabelsPolicy } from '../contracts'; + +function globToRegExp(glob: string): RegExp { + const escaped = glob.replace(/[.+^${}()|[\]\\]/g, '\\$&'); + const pattern = escaped.replace(/\*/g, '.*').replace(/\?/g, '.'); + return new RegExp(`^${pattern}$`); +} + +function matchesAny(value: string, patterns: string[] | undefined): boolean { + if (!patterns || patterns.length === 0) return false; + return patterns.some((pattern) => globToRegExp(pattern).test(value)); +} + +function evaluateLabel(label: string, policy: AwsDynamicLabelsPolicy, labelPrefix: string): string | null { + const stripped = label.slice(labelPrefix.length); + const colonIndex = stripped.indexOf(':'); + const key = colonIndex === -1 ? stripped : stripped.slice(0, colonIndex); + const value = colonIndex === -1 ? undefined : stripped.slice(colonIndex + 1); + + if (policy.blocked_keys?.includes(key)) { + return `key '${key}' is in blocked_keys`; + } + + const rule = policy.restricted_keys?.[key]; + if (!rule || value === undefined) return null; + + if (rule.allowed && rule.allowed.length > 0 && !matchesAny(value, rule.allowed)) { + return `value '${value}' not in allowed list`; + } + if (rule.denied && matchesAny(value, rule.denied)) { + return `value '${value}' in denied list`; + } + if (rule.max !== undefined && rule.max !== null) { + const valueNumber = Number(value); + const maximum = Number(rule.max); + if (!Number.isFinite(valueNumber) || !Number.isFinite(maximum)) { + return `max set but value '${value}' or max '${rule.max}' is not numeric`; + } + if (valueNumber > maximum) { + return `value '${value}' exceeds max '${rule.max}'`; + } + } + + return null; +} + +export function violationsAgainstAwsDynamicLabelsPolicy( + labels: string[], + policy: AwsDynamicLabelsPolicy | null | undefined, + labelPrefix: string, +): { label: string; reason: string }[] { + if (!policy) return []; + + const violations: { label: string; reason: string }[] = []; + for (const label of labels) { + if (!label.startsWith(labelPrefix)) continue; + const reason = evaluateLabel(label, policy, labelPrefix); + if (reason) violations.push({ label, reason }); + } + return violations; +} diff --git a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-config.ts b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-config.ts index bdc46b8c44..6d195d958b 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-config.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/control-plane/runner-config.ts @@ -141,7 +141,7 @@ async function terminateFailedInstances(instanceIds: string[]): Promise { function createEc2StartRunnerConfigOptions(): StartRunnerConfigOptions { return { - getSsmParameterTags: (instanceId) => [{ Key: 'InstanceId', Value: instanceId }], + getRunnerConfigMetadata: (instanceId) => [{ key: 'InstanceId', value: instanceId }], onJitConfigCreated: async (instanceId, metadata) => await tagEc2RunnerMetadata(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 0c6bac69f4..9018e08ff9 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,9 +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, }; } @@ -182,7 +179,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?.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/aws/ec2/src/webhook/dynamic-labels-policy.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts index a9b919c7bd..8babbadd55 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels-policy.ts @@ -1,4 +1,5 @@ import type { AwsDynamicLabelsPolicy, AwsDynamicLabelsValueRule } from '../../../../contracts'; +import { violationsAgainstAwsDynamicLabelsPolicy } from '../../../dynamic-labels-policy'; export type Ec2DynamicLabelsValueRule = AwsDynamicLabelsValueRule; @@ -10,50 +11,6 @@ export type Ec2DynamicLabelsValueRule = AwsDynamicLabelsValueRule; */ export type Ec2DynamicLabelsPolicy = AwsDynamicLabelsPolicy; -function globToRegExp(glob: string): RegExp { - const escaped = glob.replace(/[.+^${}()|[\]\\]/g, '\\$&'); - const pattern = escaped.replace(/\*/g, '.*').replace(/\?/g, '.'); - return new RegExp(`^${pattern}$`); -} - -function matchesAny(value: string, patterns: string[] | undefined): boolean { - if (!patterns || patterns.length === 0) return false; - return patterns.some((p) => globToRegExp(p).test(value)); -} - -function evaluateLabel(label: string, policy: Ec2DynamicLabelsPolicy): string | null { - const stripped = label.replace(/^ghr-ec2-/, ''); - const colonIdx = stripped.indexOf(':'); - const key = colonIdx === -1 ? stripped : stripped.slice(0, colonIdx); - const value = colonIdx === -1 ? undefined : stripped.slice(colonIdx + 1); - - if (policy.blocked_keys?.includes(key)) { - return `key '${key}' is in blocked_keys`; - } - - const rule = policy.restricted_keys?.[key]; - if (!rule) return null; - if (value === undefined) return null; - - if (rule.allowed && rule.allowed.length > 0 && !matchesAny(value, rule.allowed)) { - return `value '${value}' not in allowed list`; - } - if (rule.denied && matchesAny(value, rule.denied)) { - return `value '${value}' in denied list`; - } - if (rule.max !== undefined && rule.max !== null) { - const valueNum = Number(value); - const maxNum = Number(rule.max); - if (!Number.isFinite(valueNum) || !Number.isFinite(maxNum)) { - return `max set but value '${value}' or max '${rule.max}' is not numeric`; - } - if (valueNum > maxNum) { - return `value '${value}' exceeds max '${rule.max}'`; - } - } - return null; -} - /** * Inspects the labels and returns the rejection reasons for any `ghr-ec2-*` * label that violates the policy. Non-`ghr-ec2-*` labels are ignored. @@ -62,12 +19,5 @@ export function violationsAgainstPolicy( labels: string[], policy: Ec2DynamicLabelsPolicy | null | undefined, ): { label: string; reason: string }[] { - if (!policy) return []; - const violations: { label: string; reason: string }[] = []; - for (const label of labels) { - if (!label.startsWith('ghr-ec2-')) continue; - const reason = evaluateLabel(label, policy); - if (reason) violations.push({ label, reason }); - } - return violations; + return violationsAgainstAwsDynamicLabelsPolicy(labels, policy, 'ghr-ec2-'); } diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts index 400807554f..99c1844a5f 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.test.ts @@ -1,18 +1,38 @@ import { describe, expect, it } from 'vitest'; import type { RunnerMatcherConfig } from '../../../../contracts'; -import { selectEc2DynamicLabelQueue } from './dynamic-labels'; +import { ec2DynamicLabelProvider } from './dynamic-labels'; + +describe('ec2DynamicLabelProvider', () => { + it('returns no violations when the queue has no policy', () => { + const queue = runnerQueue('no-policy'); + + expect(getViolations(queue)).toEqual([]); + }); + + it('returns violations for labels rejected by the policy', () => { + const strictQueue = runnerQueue('strict'); + strictQueue.matcherConfig.awsDynamicLabelsPolicy = { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + + expect(getViolations(strictQueue)).toEqual([ + { + label: 'ghr-ec2-instance-type:t3.large', + reason: "value 't3.large' not in allowed list", + }, + ]); + }); -describe('selectEc2DynamicLabelQueue', () => { it('enforces a legacy EC2 dynamic labels policy when the new key is absent', () => { const queue = runnerQueue('legacy-ec2-policy'); queue.matcherConfig.ec2DynamicLabelsPolicy = { blocked_keys: ['instance-type'], }; - expect( - selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); + expect(getViolations(queue)).toHaveLength(1); }); it('falls back to the legacy EC2 dynamic labels policy when the new policy is null', () => { @@ -22,9 +42,7 @@ describe('selectEc2DynamicLabelQueue', () => { }; queue.matcherConfig.awsDynamicLabelsPolicy = null; - expect( - selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large']), - ).toBeUndefined(); + expect(getViolations(queue)).toHaveLength(1); }); it('prefers a configured AWS dynamic labels policy over the legacy policy', () => { @@ -36,13 +54,17 @@ describe('selectEc2DynamicLabelQueue', () => { blocked_keys: [], }; - expect(selectEc2DynamicLabelQueue([queue], ['self-hosted', 'linux'], ['ghr-ec2-instance-type:t3.large'])).toEqual({ - queue, - labels: ['self-hosted', 'linux', 'ghr-ec2-instance-type:t3.large'], - }); + expect(getViolations(queue)).toEqual([]); }); }); +function getViolations(queue: RunnerMatcherConfig) { + return ec2DynamicLabelProvider.getViolations({ + queue, + labels: ['ghr-ec2-instance-type:t3.large'], + }); +} + function runnerQueue(id: string): RunnerMatcherConfig { return { id, diff --git a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts index 6ddf5b8fbb..5e671da189 100644 --- a/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts +++ b/lambdas/libs/compute-providers/aws/ec2/src/webhook/dynamic-labels.ts @@ -1,12 +1,10 @@ import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; -import type { DynamicLabelDispatchTarget, DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts'; +import type { DynamicLabelProvider, RunnerMatcherConfig } from '../../../../contracts'; import { violationsAgainstPolicy } from './dynamic-labels-policy'; const logger = createChildLogger('handler'); -export type Ec2DynamicLabelDispatchTarget = DynamicLabelDispatchTarget; - function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) { const hasLegacyEc2DynamicLabelsPolicy = Object.prototype.hasOwnProperty.call( queue.matcherConfig, @@ -23,36 +21,6 @@ function resolveEc2DynamicLabelsPolicy(queue: RunnerMatcherConfig) { return queue.matcherConfig.awsDynamicLabelsPolicy; } -export function selectEc2DynamicLabelQueue( - matches: RunnerMatcherConfig[], - nonGhrLabels: string[], - sanitizedGhrLabels: string[], -): Ec2DynamicLabelDispatchTarget | undefined { - for (const queue of matches) { - if (!queue.matcherConfig.enableDynamicLabels) { - logger.warn(`Queue ${queue.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`); - continue; - } - - const violations = violationsAgainstPolicy(sanitizedGhrLabels, resolveEc2DynamicLabelsPolicy(queue)); - if (violations.length === 0) { - return { - queue, - labels: [...nonGhrLabels, ...sanitizedGhrLabels], - }; - } - - for (const violation of violations) { - logger.warn( - `Queue ${queue.id}: dynamic label '${violation.label}' does not match policy (${violation.reason}); trying next match`, - ); - } - } - - return undefined; -} - export const ec2DynamicLabelProvider: DynamicLabelProvider = { - selectQueue: ({ queue, nonGhrLabels, sanitizedGhrLabels }) => - selectEc2DynamicLabelQueue([queue], nonGhrLabels, sanitizedGhrLabels), + getViolations: ({ queue, labels }) => violationsAgainstPolicy(labels, resolveEc2DynamicLabelsPolicy(queue)), }; diff --git a/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts new file mode 100644 index 0000000000..755831fb91 --- /dev/null +++ b/lambdas/libs/compute-providers/aws/ec2/webhook.test.ts @@ -0,0 +1,27 @@ +import { defineWebhookProviderContractTests } from '../../test/webhook-provider-contract'; +import { provider } from './webhook'; + +defineWebhookProviderContractTests({ + provider, + acceptedDynamicLabels: ['ghr-ec2-instance-type:t3.large'], + rejectingPolicies: [ + { + name: 'blocked keys', + apply: (queue) => { + queue.matcherConfig.awsDynamicLabelsPolicy = { + blocked_keys: ['instance-type'], + }; + }, + }, + { + name: 'restricted keys', + apply: (queue) => { + queue.matcherConfig.awsDynamicLabelsPolicy = { + restricted_keys: { + 'instance-type': { allowed: ['m5.*'] }, + }, + }; + }, + }, + ], +}); diff --git a/lambdas/libs/compute-providers/contracts.ts b/lambdas/libs/compute-providers/contracts.ts index 617789ec10..85e99f3949 100644 --- a/lambdas/libs/compute-providers/contracts.ts +++ b/lambdas/libs/compute-providers/contracts.ts @@ -43,12 +43,13 @@ export interface DynamicLabelDispatchTarget { labels: string[]; } +export interface DynamicLabelViolation { + label: string; + reason: string; +} + export interface DynamicLabelProvider { - selectQueue(input: { - queue: RunnerMatcherConfig; - nonGhrLabels: string[]; - sanitizedGhrLabels: string[]; - }): DynamicLabelDispatchTarget | undefined; + getViolations(input: { queue: RunnerMatcherConfig; labels: string[] }): DynamicLabelViolation[]; } export interface ControlPlaneProviderCapabilities { diff --git a/lambdas/libs/compute-providers/core/index.ts b/lambdas/libs/compute-providers/core/index.ts index 2b5f937f36..908b67fb54 100644 --- a/lambdas/libs/compute-providers/core/index.ts +++ b/lambdas/libs/compute-providers/core/index.ts @@ -21,9 +21,6 @@ export interface CreateGitHubRunnerConfig { runnerOwner: string; runnerType: RunnerType; disableAutoUpdate: boolean; - ssmTokenPath: string; - ssmConfigPath: string; - ssmParameterStoreTags: { Key: string; Value: string }[]; } export interface GitHubRunnerMetadata { @@ -32,7 +29,7 @@ export interface GitHubRunnerMetadata { } export interface StartRunnerConfigOptions { - getSsmParameterTags?: (runnerId: string) => { Key: string; Value: string }[]; + getRunnerConfigMetadata?: (runnerId: string) => { key: string; value: string }[]; onJitConfigCreated?: (runnerId: string, metadata: GitHubRunnerMetadata) => Promise; } diff --git a/lambdas/libs/compute-providers/dynamic-labels.test.ts b/lambdas/libs/compute-providers/dynamic-labels.test.ts new file mode 100644 index 0000000000..88aa39689c --- /dev/null +++ b/lambdas/libs/compute-providers/dynamic-labels.test.ts @@ -0,0 +1,13 @@ +import { expect, it } from 'vitest'; + +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; + +const providerTypes = ['alpha', 'beta'] as const; + +it.each(providerTypes)('returns labels belonging to providers other than %s', (provider) => { + const providerLabels = providerTypes.map((type) => `ghr-${type}-size:large`); + + expect(dynamicLabelsForOtherProvider(providerLabels, provider, providerTypes)).toEqual( + providerLabels.filter((label) => !label.startsWith(`ghr-${provider}-`)), + ); +}); diff --git a/lambdas/libs/compute-providers/dynamic-labels.ts b/lambdas/libs/compute-providers/dynamic-labels.ts new file mode 100644 index 0000000000..3c72d77966 --- /dev/null +++ b/lambdas/libs/compute-providers/dynamic-labels.ts @@ -0,0 +1,11 @@ +import { computeProviderTypes } from './provider-types'; + +export function dynamicLabelsForOtherProvider( + labels: string[], + provider: string, + providerTypes: readonly string[] = computeProviderTypes, +): string[] { + return labels.filter((label) => + providerTypes.some((candidate) => candidate !== provider && label.startsWith(`ghr-${candidate}-`)), + ); +} diff --git a/lambdas/libs/compute-providers/provider-types.test.ts b/lambdas/libs/compute-providers/provider-types.test.ts index 76111897ab..9f6f4a981e 100644 --- a/lambdas/libs/compute-providers/provider-types.test.ts +++ b/lambdas/libs/compute-providers/provider-types.test.ts @@ -1,11 +1,15 @@ import { describe, expect, it } from 'vitest'; -import { - defaultComputeProvider, - normalizeComputeProviderType, - resolveComputeProviderType, - computeProviderTypes, -} from './provider-types'; +import { computeProviderTypes, defaultComputeProvider, resolveComputeProviderType } from './provider-types'; + +const defaultProviderInputs = [undefined, '', ' '] as const; +const supportedProviderCases = computeProviderTypes.flatMap( + (provider) => + [ + [provider, provider], + [` ${provider.toUpperCase()} `, provider], + ] as const, +); describe('compute provider configuration', () => { it('defines an explicit default provider', () => { @@ -13,32 +17,16 @@ describe('compute provider configuration', () => { }); }); -describe('compute provider normalization', () => { - it.each([ - [undefined, 'ec2'], - ['', 'ec2'], - [' ', 'ec2'], - [' EC2 ', 'ec2'], - ])('normalizes provider type %j to %j', (type, expected) => { - expect(normalizeComputeProviderType(type)).toBe(expected); - }); - - it.each([[' Unknown '], ['microvm'], [null], [1]])('returns undefined for unsupported provider type %j', (type) => { - expect(normalizeComputeProviderType(type)).toBeUndefined(); +describe('compute provider resolution', () => { + it.each(defaultProviderInputs)('resolves default provider input %j', (type) => { + expect(resolveComputeProviderType(type)).toBe(defaultComputeProvider); }); -}); -describe('compute provider resolution', () => { - it.each([ - [undefined, 'ec2'], - ['', 'ec2'], - [' ', 'ec2'], - [' EC2 ', 'ec2'], - ])('resolves provider type %j to %j', (type, expected) => { + it.each(supportedProviderCases)('resolves provider type %j to %j', (type, expected) => { expect(resolveComputeProviderType(type)).toBe(expected); }); - it.each([[' Unknown '], ['microvm'], [null], [1]])('rejects unsupported provider type %j', (type) => { + it.each([[' Unknown '], ['unsupported-provider'], [null], [1]])('rejects unsupported provider type %j', (type) => { expect(() => resolveComputeProviderType(type)).toThrow(`Unsupported compute provider type '${String(type)}'`); }); }); diff --git a/lambdas/libs/compute-providers/provider-types.ts b/lambdas/libs/compute-providers/provider-types.ts index dcac6c5769..64d7be8e5f 100644 --- a/lambdas/libs/compute-providers/provider-types.ts +++ b/lambdas/libs/compute-providers/provider-types.ts @@ -4,21 +4,19 @@ export type ComputeProviderType = (typeof computeProviderTypes)[number]; export const defaultComputeProvider = 'ec2' satisfies ComputeProviderType; -export function normalizeComputeProviderType(type: unknown): ComputeProviderType | undefined { +export function resolveComputeProviderType(type: unknown): ComputeProviderType { if (type === undefined) return defaultComputeProvider; - if (typeof type !== 'string') return undefined; + if (typeof type !== 'string') { + throw new Error(`Unsupported compute provider type '${String(type)}'`); + } const normalizedType = type.trim().toLowerCase(); if (!normalizedType) return defaultComputeProvider; - return computeProviderTypes.find((computeProviderType) => computeProviderType === normalizedType); -} - -export function resolveComputeProviderType(type: unknown): ComputeProviderType { - const normalizedType = normalizeComputeProviderType(type); - if (!normalizedType) { + const computeProviderType = computeProviderTypes.find((provider) => provider === normalizedType); + if (!computeProviderType) { throw new Error(`Unsupported compute provider type '${String(type)}'`); } - return normalizedType; + return computeProviderType; } diff --git a/lambdas/libs/compute-providers/registry.test.ts b/lambdas/libs/compute-providers/registry.test.ts index 3c95dcaca4..93227831cd 100644 --- a/lambdas/libs/compute-providers/registry.test.ts +++ b/lambdas/libs/compute-providers/registry.test.ts @@ -33,6 +33,6 @@ it('exposes every configured provider through both capability registries', () => unmarkOrphan: expect.any(Function), terminate: expect.any(Function), }); - expect(webhookProviderRegistry.capability(type, 'dynamicLabels').selectQueue).toEqual(expect.any(Function)); + expect(webhookProviderRegistry.capability(type, 'dynamicLabels').getViolations).toEqual(expect.any(Function)); } }); diff --git a/lambdas/libs/compute-providers/templates/provider/provider.test.ts b/lambdas/libs/compute-providers/templates/provider/provider.test.ts index 816b2f9cfc..2644fc4f2a 100644 --- a/lambdas/libs/compute-providers/templates/provider/provider.test.ts +++ b/lambdas/libs/compute-providers/templates/provider/provider.test.ts @@ -29,5 +29,5 @@ it('exposes every compute provider capability from its compute-provider entry po terminate: expect.any(Function), }); expect(webhookPlugin.type).toBe(webhookProvider.type); - expect(webhookPlugin.capabilities.dynamicLabels.selectQueue).toEqual(expect.any(Function)); + expect(webhookPlugin.capabilities.dynamicLabels.getViolations).toEqual(expect.any(Function)); }); diff --git a/lambdas/libs/compute-providers/templates/provider/webhook.ts b/lambdas/libs/compute-providers/templates/provider/webhook.ts index 86e59da7b3..31c522c588 100644 --- a/lambdas/libs/compute-providers/templates/provider/webhook.ts +++ b/lambdas/libs/compute-providers/templates/provider/webhook.ts @@ -3,10 +3,10 @@ import type { ComputeProviderPlugin } from '../../core'; import type { DynamicLabelProvider, WebhookProviderCapabilities, WebhookProviderModule } from '../../contracts'; export const templateDynamicLabelProvider: DynamicLabelProvider = { - selectQueue: (input) => { + getViolations: (input) => { void input; - // Return a dispatch target when this provider accepts the requested dynamic labels. - return undefined; + // Return violations for dynamic labels this provider does not accept. + return []; }, }; diff --git a/lambdas/libs/compute-providers/test/webhook-provider-contract.ts b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts new file mode 100644 index 0000000000..dd4e3097b0 --- /dev/null +++ b/lambdas/libs/compute-providers/test/webhook-provider-contract.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from 'vitest'; + +import type { RunnerMatcherConfig, WebhookProviderModule } from '../contracts'; +import { defaultComputeProvider } from '../provider-types'; +import type { ComputeProviderType } from '../provider-types'; +import { selectDynamicLabelQueue } from '../webhook'; + +interface RejectingPolicyCase { + name: string; + apply(queue: RunnerMatcherConfig): void; +} + +interface WebhookProviderContractOptions { + provider: WebhookProviderModule; + acceptedDynamicLabels: readonly [string, ...string[]]; + rejectingPolicies: readonly [RejectingPolicyCase, ...RejectingPolicyCase[]]; +} + +export function defineWebhookProviderContractTests({ + provider, + acceptedDynamicLabels, + rejectingPolicies, +}: WebhookProviderContractOptions): void { + const nonGhrLabels = ['self-hosted', 'linux']; + const dynamicLabels = [...acceptedDynamicLabels]; + + function expectProviderSelected(queue: RunnerMatcherConfig) { + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toEqual({ + queue, + labels: [...nonGhrLabels, ...dynamicLabels], + }); + } + + describe(`${provider.type} webhook provider contract`, () => { + it('selects an explicitly configured provider through the production registry', () => { + expectProviderSelected(runnerQueue(`${provider.type}-configured`, provider.type)); + }); + + it('skips the provider when dynamic labels are disabled', () => { + const queue = runnerQueue(`${provider.type}-disabled`, provider.type); + queue.matcherConfig.enableDynamicLabels = false; + + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toBeUndefined(); + }); + + for (const policy of rejectingPolicies) { + it(`skips the provider when its ${policy.name} policy rejects the labels`, () => { + const queue = runnerQueue(`${provider.type}-policy-rejected`, provider.type); + policy.apply(queue); + + expect(selectDynamicLabelQueue([queue], nonGhrLabels, dynamicLabels)).toBeUndefined(); + }); + } + + it('normalizes provider configuration before registry selection', () => { + const queue = runnerQueue(`${provider.type}-normalized`); + (queue as unknown as { computeProvider: string }).computeProvider = ` ${provider.type.toUpperCase()} `; + + expectProviderSelected(queue); + }); + + if (provider.type === defaultComputeProvider) { + it('selects the default provider when the queue omits provider configuration', () => { + expectProviderSelected(runnerQueue(`${provider.type}-default`)); + }); + } + }); +} + +function runnerQueue(id: string, computeProvider?: ComputeProviderType): RunnerMatcherConfig { + return { + id, + arn: `arn:${id}`, + computeProvider, + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + }, + }; +} diff --git a/lambdas/libs/compute-providers/tsconfig.json b/lambdas/libs/compute-providers/tsconfig.json index 52d55867fe..51beb73b87 100644 --- a/lambdas/libs/compute-providers/tsconfig.json +++ b/lambdas/libs/compute-providers/tsconfig.json @@ -1,5 +1,5 @@ { "extends": "../../tsconfig.json", - "include": ["*.ts", "core/**/*", "aws/**/*", "templates/**/*"], + "include": ["*.ts", "core/**/*", "aws/**/*", "templates/**/*", "test/**/*"], "exclude": ["aws/**/*.test.ts"] } diff --git a/lambdas/libs/compute-providers/webhook.test.ts b/lambdas/libs/compute-providers/webhook.test.ts new file mode 100644 index 0000000000..7ec7343f97 --- /dev/null +++ b/lambdas/libs/compute-providers/webhook.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it, vi } from 'vitest'; + +import type { DynamicLabelProvider, DynamicLabelViolation, RunnerMatcherConfig } from './contracts'; +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; +import { createDynamicLabelQueueSelector, selectDynamicLabelQueue } from './webhook'; + +const testProviderTypes = ['alpha', 'beta'] as const; +type TestProviderType = (typeof testProviderTypes)[number]; + +describe('selectDynamicLabelQueue', () => { + it.each([ + ['unsupported string', 'unsupported-provider'], + ['non-string', 42], + ])('strictly rejects an %s compute provider', (_description, computeProvider) => { + const invalidQueue = runnerQueue('invalid-provider'); + (invalidQueue as unknown as { computeProvider: unknown }).computeProvider = computeProvider; + + expect(() => selectDynamicLabelQueue([invalidQueue], [], [])).toThrow( + `Unsupported compute provider type '${String(computeProvider)}'`, + ); + }); +}); + +describe('createDynamicLabelQueueSelector', () => { + it('returns the first queue accepted by its provider', () => { + const queue = runnerQueue('accepted'); + const { selectQueue } = selector(); + + expect(selectQueue([queue], ['self-hosted', 'linux'], ['ghr-test-size:large'])).toEqual({ + queue, + labels: ['self-hosted', 'linux', 'ghr-test-size:large'], + }); + }); + + it('skips queues that disable dynamic labels', () => { + const disabledQueue = runnerQueue('disabled'); + disabledQueue.matcherConfig.enableDynamicLabels = false; + const enabledQueue = runnerQueue('enabled'); + const { getViolations, selectQueue } = selector(); + + expect(selectQueue([disabledQueue, enabledQueue], ['self-hosted'], ['ghr-test-size:large'])).toEqual({ + queue: enabledQueue, + labels: ['self-hosted', 'ghr-test-size:large'], + }); + expect(getViolations).toHaveBeenCalledOnce(); + expect(getViolations).toHaveBeenCalledWith({ queue: enabledQueue, labels: ['ghr-test-size:large'] }); + }); + + it('skips queues whose provider reports violations', () => { + const rejectedQueue = runnerQueue('rejected'); + const acceptedQueue = runnerQueue('accepted'); + const { selectQueue } = selector({ + violationsByQueue: { + rejected: [{ label: 'ghr-test-size:large', reason: 'size is unavailable' }], + }, + }); + + expect(selectQueue([rejectedQueue, acceptedQueue], ['self-hosted'], ['ghr-test-size:large'])).toEqual({ + queue: acceptedQueue, + labels: ['self-hosted', 'ghr-test-size:large'], + }); + }); + + it('returns undefined when every provider reports violations', () => { + const queue = runnerQueue('rejected'); + const { selectQueue } = selector({ + violationsByQueue: { + rejected: [{ label: 'ghr-test-size:large', reason: 'size is unavailable' }], + }, + }); + + expect(selectQueue([queue], ['self-hosted'], ['ghr-test-size:large'])).toBeUndefined(); + }); + + it('selects the queue targeted by provider-specific labels', () => { + const alphaQueue = runnerQueue('alpha'); + const betaQueue = runnerQueue('beta'); + const betaLabel = 'ghr-beta-size:large'; + const { getViolations, selectQueue } = selector({ + providerByQueue: { alpha: 'alpha', beta: 'beta' }, + }); + + expect(selectQueue([alphaQueue, betaQueue], ['self-hosted', 'linux'], [betaLabel])).toEqual({ + queue: betaQueue, + labels: ['self-hosted', 'linux', betaLabel], + }); + expect(getViolations).toHaveBeenCalledOnce(); + expect(getViolations).toHaveBeenCalledWith({ queue: betaQueue, labels: [betaLabel] }); + }); +}); + +function selector(options?: { + providerByQueue?: Record; + violationsByQueue?: Record; +}) { + const getViolations = vi.fn(({ queue }) => { + return options?.violationsByQueue?.[queue.id] ?? []; + }); + + return { + getViolations, + selectQueue: createDynamicLabelQueueSelector({ + resolveProvider: (queue) => ({ + type: options?.providerByQueue?.[queue.id] ?? 'alpha', + dynamicLabels: { getViolations }, + }), + dynamicLabelsForOtherProvider: (labels, provider) => + dynamicLabelsForOtherProvider(labels, provider, testProviderTypes), + }), + }; +} + +function runnerQueue(id: string): RunnerMatcherConfig { + return { + id, + arn: `arn:${id}`, + matcherConfig: { + labelMatchers: [['self-hosted', 'linux']], + exactMatch: true, + enableDynamicLabels: true, + }, + }; +} diff --git a/lambdas/libs/compute-providers/webhook.ts b/lambdas/libs/compute-providers/webhook.ts index ee80a54203..1aa0a7b5e6 100644 --- a/lambdas/libs/compute-providers/webhook.ts +++ b/lambdas/libs/compute-providers/webhook.ts @@ -1,8 +1,70 @@ +import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; + import { createComputeProviderRegistry } from './core'; -import type { WebhookProviderCapabilities } from './contracts'; +import type { + DynamicLabelDispatchTarget, + DynamicLabelProvider, + RunnerMatcherConfig, + WebhookProviderCapabilities, +} from './contracts'; +import { dynamicLabelsForOtherProvider } from './dynamic-labels'; +import { resolveComputeProviderType } from './provider-types'; import { enabledWebhookProviders } from './providers.config.webhook'; +const logger = createChildLogger('handler'); + export const webhookProviderRegistry = createComputeProviderRegistry( enabledWebhookProviders.map((provider) => provider.createPlugin()), ); + +export function createDynamicLabelQueueSelector(dependencies: { + resolveProvider(queue: RunnerMatcherConfig): { type: TProvider; dynamicLabels: DynamicLabelProvider }; + dynamicLabelsForOtherProvider(labels: string[], provider: TProvider): string[]; +}) { + return ( + matches: RunnerMatcherConfig[], + nonGhrLabels: string[], + sanitizedGhrLabels: string[], + ): DynamicLabelDispatchTarget | undefined => { + for (const queue of matches) { + const { type: provider, dynamicLabels } = dependencies.resolveProvider(queue); + + if (!queue.matcherConfig.enableDynamicLabels) { + logger.warn( + `Queue ${queue.id} matches non-dynamic labels but does not allow dynamic labels; trying next match`, + ); + continue; + } + + const labelsForOtherProvider = dependencies.dynamicLabelsForOtherProvider(sanitizedGhrLabels, provider); + if (labelsForOtherProvider.length > 0) { + logger.warn(`Queue ${queue.id}: dynamic labels target another compute provider; trying next match`, { + dynamicLabels: labelsForOtherProvider, + }); + continue; + } + + const violations = dynamicLabels.getViolations({ queue, labels: sanitizedGhrLabels }); + if (violations.length === 0) { + return { queue, labels: [...nonGhrLabels, ...sanitizedGhrLabels] }; + } + + for (const violation of violations) { + logger.warn( + `Queue ${queue.id}: dynamic label '${violation.label}' is not accepted (${violation.reason}); trying next match`, + ); + } + } + + return undefined; + }; +} + +export const selectDynamicLabelQueue = createDynamicLabelQueueSelector({ + resolveProvider: (queue) => { + const type = resolveComputeProviderType(queue.computeProvider); + return { type, dynamicLabels: webhookProviderRegistry.capability(type, 'dynamicLabels') }; + }, + dynamicLabelsForOtherProvider, +}); 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..a1af90d6e7 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/environment.d.ts @@ -0,0 +1,17 @@ +export {}; + +declare global { + namespace NodeJS { + interface ProcessEnv { + PARAMETER_GITHUB_APP_ID_NAME?: string; + PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME?: string; + PARAMETER_GITHUB_APP_KEY_BASE64_NAME?: string; + PARAMETER_GITHUB_APP_WEBHOOK_SECRET?: string; + PARAMETER_RUNNER_MATCHER_CONFIG_PATH?: string; + SSM_CONFIG_PATH?: string; + SSM_CLEANUP_CONFIG?: string; + SSM_PARAMETER_STORE_TAGS?: string; + SSM_TOKEN_PATH?: string; + } + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/github-app-credentials-store.test.ts b/lambdas/libs/storage-providers/aws/ssm/github-app-credentials-store.test.ts new file mode 100644 index 0000000000..4ca2e93914 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/github-app-credentials-store.test.ts @@ -0,0 +1,154 @@ +import { getParameters } from '@aws-github-runner/aws-ssm-util'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmGitHubAppCredentialsStore } from './github-app-credentials-store'; + +vi.mock('@aws-github-runner/aws-ssm-util', () => ({ + getParameters: vi.fn(), +})); + +const getParametersMock = vi.mocked(getParameters); +const cleanEnv = process.env; +const primaryIdParameter = '/actions-runner/test/github_app_id'; +const primaryKeyParameter = '/actions-runner/test/github_app_key_base64'; + +describe('aws_ssm GitHub App credentials store', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + process.env.PARAMETER_GITHUB_APP_ID_NAME = primaryIdParameter; + process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME = primaryKeyParameter; + delete process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME; + }); + + it('batch reads and maps the primary GitHub App credential', async () => { + const privateKey = 'fake-private-key'; + getParametersMock.mockResolvedValue( + new Map([ + [primaryIdParameter, '123'], + [primaryKeyParameter, Buffer.from(privateKey).toString('base64')], + ]), + ); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).resolves.toEqual([{ appId: 123, privateKey, installationId: undefined }]); + expect(getParametersMock).toHaveBeenCalledOnce(); + expect(getParametersMock).toHaveBeenCalledWith([primaryIdParameter, primaryKeyParameter]); + }); + + it('preserves multi-app order and optional installation-id slots', async () => { + const additionalIdParameter = '/actions-runner/test/additional_github_app_0_id'; + const additionalKeyParameter = '/actions-runner/test/additional_github_app_0_key_base64'; + const additionalInstallationIdParameter = '/actions-runner/test/additional_github_app_0_installation_id'; + process.env.PARAMETER_GITHUB_APP_ID_NAME = `${primaryIdParameter}:${additionalIdParameter}`; + process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME = `${primaryKeyParameter}:${additionalKeyParameter}`; + process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME = `:${additionalInstallationIdParameter}`; + getParametersMock.mockResolvedValue( + new Map([ + [primaryIdParameter, '123'], + [primaryKeyParameter, Buffer.from('primary-key').toString('base64')], + [additionalIdParameter, '456'], + [additionalKeyParameter, Buffer.from('additional-key').toString('base64')], + [additionalInstallationIdParameter, '789'], + ]), + ); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).resolves.toEqual([ + { appId: 123, privateKey: 'primary-key', installationId: undefined }, + { appId: 456, privateKey: 'additional-key', installationId: 789 }, + ]); + expect(getParametersMock).toHaveBeenCalledWith([ + primaryIdParameter, + additionalIdParameter, + primaryKeyParameter, + additionalKeyParameter, + additionalInstallationIdParameter, + ]); + }); + + it('decodes literal newline escapes in a base64 private key', async () => { + getParametersMock.mockResolvedValue( + new Map([ + [primaryIdParameter, '123'], + [primaryKeyParameter, Buffer.from('first-line\\nsecond-line').toString('base64')], + ]), + ); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).resolves.toEqual([ + { appId: 123, privateKey: 'first-line\nsecond-line', installationId: undefined }, + ]); + }); + + it('preserves parseInt behavior for stored numeric values', async () => { + const installationIdParameter = '/actions-runner/test/github_app_installation_id'; + process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME = installationIdParameter; + getParametersMock.mockResolvedValue( + new Map([ + [primaryIdParameter, '123app'], + [primaryKeyParameter, Buffer.from('fake-private-key').toString('base64')], + [installationIdParameter, '789installation'], + ]), + ); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).resolves.toEqual([{ appId: 123, privateKey: 'fake-private-key', installationId: 789 }]); + }); + + it.each([ + ['PARAMETER_GITHUB_APP_ID_NAME', undefined], + ['PARAMETER_GITHUB_APP_ID_NAME', ''], + ['PARAMETER_GITHUB_APP_KEY_BASE64_NAME', undefined], + ['PARAMETER_GITHUB_APP_KEY_BASE64_NAME', ''], + ] as const)('rejects missing environment value %s=%j before reading', async (name, value) => { + setEnvironmentValue(name, value); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).rejects.toThrow(`Environment variable ${name} is not set`); + expect(getParametersMock).not.toHaveBeenCalled(); + }); + + it('rejects mismatched GitHub App id and key parameter counts before reading', async () => { + process.env.PARAMETER_GITHUB_APP_ID_NAME = `${primaryIdParameter}:/additional/id`; + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).rejects.toThrow('GitHub App parameter count mismatch: 2 IDs vs 1 keys'); + expect(getParametersMock).not.toHaveBeenCalled(); + }); + + it('rejects a missing GitHub App id parameter', async () => { + getParametersMock.mockResolvedValue( + new Map([[primaryKeyParameter, Buffer.from('fake-private-key').toString('base64')]]), + ); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).rejects.toThrow(`Parameter ${primaryIdParameter} not found`); + }); + + it('rejects a missing GitHub App private-key parameter', async () => { + getParametersMock.mockResolvedValue(new Map([[primaryIdParameter, '123']])); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).rejects.toThrow(`Parameter ${primaryKeyParameter} not found`); + }); + + it('propagates parameter-store read errors', async () => { + const error = new Error('access denied'); + getParametersMock.mockRejectedValue(error); + const store = createAwsSsmGitHubAppCredentialsStore(); + + await expect(store.get()).rejects.toBe(error); + }); +}); + +function setEnvironmentValue( + name: 'PARAMETER_GITHUB_APP_ID_NAME' | 'PARAMETER_GITHUB_APP_KEY_BASE64_NAME', + value: string | undefined, +): void { + if (value === undefined) { + delete process.env[name]; + } else { + process.env[name] = value; + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/github-app-credentials-store.ts b/lambdas/libs/storage-providers/aws/ssm/github-app-credentials-store.ts new file mode 100644 index 0000000000..5e5ca2e501 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/github-app-credentials-store.ts @@ -0,0 +1,61 @@ +import { getParameters } from '@aws-github-runner/aws-ssm-util'; + +import type { GitHubAppCredential, GitHubAppCredentialsStore } from '../../core'; +import type {} from './environment'; + +export function createAwsSsmGitHubAppCredentialsStore(): GitHubAppCredentialsStore { + return new AwsSsmGitHubAppCredentialsStore(); +} + +class AwsSsmGitHubAppCredentialsStore implements GitHubAppCredentialsStore { + async get(): Promise { + if (!process.env.PARAMETER_GITHUB_APP_ID_NAME) { + throw new Error('Environment variable PARAMETER_GITHUB_APP_ID_NAME is not set'); + } + if (!process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME) { + throw new Error('Environment variable PARAMETER_GITHUB_APP_KEY_BASE64_NAME is not set'); + } + + const idParameters = process.env.PARAMETER_GITHUB_APP_ID_NAME.split(':').filter(Boolean); + const keyParameters = process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME.split(':').filter(Boolean); + const installationIdParameters = (process.env.PARAMETER_GITHUB_APP_INSTALLATION_ID_NAME || '').split(':'); + if (idParameters.length !== keyParameters.length) { + throw new Error( + `GitHub App parameter count mismatch: ${idParameters.length} IDs vs ${keyParameters.length} keys`, + ); + } + + const parameterNames = [ + ...idParameters, + ...keyParameters, + ...installationIdParameters.filter((parameter) => parameter.length > 0), + ]; + const parameters = await getParameters(parameterNames); + + const credentials: GitHubAppCredential[] = []; + for (let index = 0; index < idParameters.length; index++) { + const appIdValue = parameters.get(idParameters[index]); + if (!appIdValue) { + throw new Error(`Parameter ${idParameters[index]} not found`); + } + + const privateKeyBase64 = parameters.get(keyParameters[index]); + if (!privateKeyBase64) { + throw new Error(`Parameter ${keyParameters[index]} not found`); + } + + const installationIdParameter = installationIdParameters[index]; + const installationIdValue = installationIdParameter ? parameters.get(installationIdParameter) : undefined; + + credentials.push({ + appId: parseInt(appIdValue, 10), + // Match the GitHub Terraform provider's handling of keys stored as a + // single-line base64 value containing literal newline escapes. + privateKey: Buffer.from(privateKeyBase64, 'base64').toString().replace(/\\n/g, '\n'), + installationId: installationIdValue ? parseInt(installationIdValue, 10) : undefined, + }); + } + + return credentials; + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/github-webhook-secret-store.test.ts b/lambdas/libs/storage-providers/aws/ssm/github-webhook-secret-store.test.ts new file mode 100644 index 0000000000..7384c769bc --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/github-webhook-secret-store.test.ts @@ -0,0 +1,51 @@ +import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmGitHubWebhookSecretStore } from './github-webhook-secret-store'; + +vi.mock('@aws-github-runner/aws-ssm-util', () => ({ + getParameter: vi.fn(), +})); + +const getParameterMock = vi.mocked(getParameter); +const cleanEnv = process.env; +const webhookSecretParameter = '/actions-runner/test/webhook_secret'; + +describe('aws_ssm GitHub webhook secret store', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = webhookSecretParameter; + }); + + it('loads the webhook secret parameter', async () => { + getParameterMock.mockResolvedValue('fake-webhook-secret'); + const store = createAwsSsmGitHubWebhookSecretStore(); + + await expect(store.get()).resolves.toBe('fake-webhook-secret'); + expect(getParameterMock).toHaveBeenCalledOnce(); + expect(getParameterMock).toHaveBeenCalledWith(webhookSecretParameter); + }); + + it('wraps a parameter read failure with the legacy error message', async () => { + getParameterMock.mockRejectedValue(new Error('access denied')); + const store = createAwsSsmGitHubWebhookSecretStore(); + + await expect(store.get()).rejects.toThrow( + `Failed to load parameter for webhookSecret from path ${webhookSecretParameter}: access denied`, + ); + }); + + it.each([undefined, '', ' '])('requires a webhook secret parameter path for input %j', (parameterPath) => { + if (parameterPath === undefined) { + delete process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET; + } else { + process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET = parameterPath; + } + + expect(() => createAwsSsmGitHubWebhookSecretStore()).toThrow( + 'Environment variable PARAMETER_GITHUB_APP_WEBHOOK_SECRET is not set', + ); + expect(getParameterMock).not.toHaveBeenCalled(); + }); +}); diff --git a/lambdas/libs/storage-providers/aws/ssm/github-webhook-secret-store.ts b/lambdas/libs/storage-providers/aws/ssm/github-webhook-secret-store.ts new file mode 100644 index 0000000000..ce35e1f532 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/github-webhook-secret-store.ts @@ -0,0 +1,27 @@ +import { getParameter } from '@aws-github-runner/aws-ssm-util'; + +import type { GitHubWebhookSecretStore } from '../../core'; +import type {} from './environment'; + +export function createAwsSsmGitHubWebhookSecretStore(): GitHubWebhookSecretStore { + const parameterPath = process.env.PARAMETER_GITHUB_APP_WEBHOOK_SECRET; + if (!parameterPath || parameterPath.trim() === '') { + throw new Error('Environment variable PARAMETER_GITHUB_APP_WEBHOOK_SECRET is not set'); + } + + return new AwsSsmGitHubWebhookSecretStore(parameterPath); +} + +class AwsSsmGitHubWebhookSecretStore implements GitHubWebhookSecretStore { + constructor(private readonly parameterPath: string) {} + + async get(): Promise { + try { + return await getParameter(this.parameterPath); + } catch (error) { + throw new Error( + `Failed to load parameter for webhookSecret from path ${this.parameterPath}: ${(error as Error).message}`, + ); + } + } +} diff --git a/lambdas/functions/control-plane/src/local-ssm-housekeeper.ts b/lambdas/libs/storage-providers/aws/ssm/local-runner-config-housekeeper.ts similarity index 50% rename from lambdas/functions/control-plane/src/local-ssm-housekeeper.ts rename to lambdas/libs/storage-providers/aws/ssm/local-runner-config-housekeeper.ts index ec635b13ad..79518a8157 100644 --- a/lambdas/functions/control-plane/src/local-ssm-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/local-runner-config-housekeeper.ts @@ -1,11 +1,14 @@ -import { cleanSSMTokens } from './scale-runners/ssm-housekeeper'; +import { createAwsSsmRunnerConfigStore } from './runner-config-store'; export function run(): void { - cleanSSMTokens({ + process.env.SSM_CLEANUP_CONFIG = JSON.stringify({ dryRun: true, minimumDaysOld: 3, tokenPath: '/ghr/my-env/runners/tokens', - }) + }); + + createAwsSsmRunnerConfigStore() + .houseKeeper() .then() .catch((e) => { console.log(e); 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-housekeeper.test.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.test.ts new file mode 100644 index 0000000000..9c837c7fb7 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.test.ts @@ -0,0 +1,97 @@ +import { DeleteParameterCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; +import { mockClient } from 'aws-sdk-client-mock'; +import 'aws-sdk-client-mock-jest/vitest'; +import { beforeEach, describe, expect, it } from 'vitest'; + +import { createAwsSsmRunnerConfigStore } from './runner-config-store'; + +const mockSSMClient = mockClient(SSMClient); +const cleanEnv = process.env; +const minimumDaysOld = 1; +const now = new Date(); +const oldDate = new Date(); +oldDate.setDate(oldDate.getDate() - minimumDaysOld - 1); +const tokenPath = '/path/to/tokens/'; + +describe('aws_ssm runner config housekeeper', () => { + beforeEach(() => { + mockSSMClient.reset(); + process.env = { ...cleanEnv }; + delete process.env.SSM_TOKEN_PATH; + process.env.AWS_REGION = 'eu-east-1'; + setCleanupOptions({ dryRun: false, minimumDaysOld, tokenPath }); + + mockSSMClient.on(GetParametersByPathCommand).resolves({ + Parameters: undefined, + }); + mockSSMClient.on(GetParametersByPathCommand, { Path: tokenPath }).resolves({ + Parameters: [ + { + Name: `${tokenPath}i-old-01`, + LastModifiedDate: oldDate, + }, + ], + NextToken: 'next', + }); + mockSSMClient.on(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }).resolves({ + Parameters: [ + { + Name: `${tokenPath}i-new-01`, + LastModifiedDate: now, + }, + ], + NextToken: undefined, + }); + }); + + it('constructs without writer configuration and deletes expired records across pages', async () => { + const store = createAwsSsmRunnerConfigStore(); + + await store.houseKeeper(); + + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath }); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: `${tokenPath}i-old-01` }); + expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: `${tokenPath}i-new-01` }); + }); + + it('does not delete records during a dry run', async () => { + setCleanupOptions({ dryRun: true, minimumDaysOld, tokenPath }); + const store = createAwsSsmRunnerConfigStore(); + + await store.houseKeeper(); + + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath }); + expect(mockSSMClient).not.toHaveReceivedCommand(DeleteParameterCommand); + }); + + it('does not delete when no records are found', async () => { + setCleanupOptions({ dryRun: false, minimumDaysOld, tokenPath: 'does-not-exist' }); + const store = createAwsSsmRunnerConfigStore(); + + await expect(store.houseKeeper()).resolves.not.toThrow(); + + expect(mockSSMClient).not.toHaveReceivedCommand(DeleteParameterCommand); + }); + + it('continues when deleting an expired record fails', async () => { + mockSSMClient.on(DeleteParameterCommand).rejects(new Error('ParameterNotFound')); + const store = createAwsSsmRunnerConfigStore(); + + await expect(store.houseKeeper()).resolves.not.toThrow(); + }); + + it.each([ + { dryRun: false, minimumDaysOld: undefined as unknown as number, tokenPath }, + { dryRun: false, minimumDaysOld: 0, tokenPath }, + { dryRun: false, minimumDaysOld, tokenPath: undefined as unknown as string }, + ])('rejects invalid cleanup options %#', async (options) => { + setCleanupOptions(options); + const store = createAwsSsmRunnerConfigStore(); + + await expect(store.houseKeeper()).rejects.toBeInstanceOf(Error); + }); +}); + +function setCleanupOptions(options: { dryRun: boolean; minimumDaysOld: number; tokenPath: string }): void { + process.env.SSM_CLEANUP_CONFIG = JSON.stringify(options); +} diff --git a/lambdas/functions/control-plane/src/scale-runners/ssm-housekeeper.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts similarity index 91% rename from lambdas/functions/control-plane/src/scale-runners/ssm-housekeeper.ts rename to lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts index 857b974a9d..30bc1d20ca 100644 --- a/lambdas/functions/control-plane/src/scale-runners/ssm-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts @@ -1,6 +1,5 @@ import { DeleteParameterCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; -import { logger } from '@aws-github-runner/aws-powertools-util'; -import { getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; +import { getTracedAWSV3Client, logger } from '@aws-github-runner/aws-powertools-util'; export interface SSMCleanupOptions { dryRun: boolean; @@ -36,7 +35,6 @@ export async function cleanSSMTokens(options: SSMCleanupOptions): Promise parameters.NextToken = nextParameters.NextToken; } logger.info(`Found #${parameters.Parameters?.length} parameters in path ${options.tokenPath}`); - logger.debug('Found parameters', { parameters }); // minimumDate = today - minimumDaysOld const minimumDate = new Date(); @@ -47,7 +45,7 @@ export async function cleanSSMTokens(options: SSMCleanupOptions): Promise logger.info(`Deleting parameter ${parameter.Name} with last modified date ${parameter.LastModifiedDate}`); try { if (!options.dryRun) { - // sleep 50ms to avoid rait limit + // sleep 50ms to avoid rate limit await new Promise((resolve) => setTimeout(resolve, 50)); await client.send(new DeleteParameterCommand({ Name: parameter.Name })); } 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..04ecdda05d --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.test.ts @@ -0,0 +1,95 @@ +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_CLEANUP_CONFIG; + delete process.env.SSM_PARAMETER_STORE_TAGS; + process.env.SSM_TOKEN_PATH = '/runner/tokens'; + }); + + it('maps metadata to tags before configured SSM 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' }, + { metadata: [{ 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: [] }); + }); + + it.each(['', '{invalid-json'])('parses cleanup configuration %j during provider construction', (config) => { + process.env.SSM_CLEANUP_CONFIG = config; + + expect(() => createAwsSsmRunnerConfigStore()).toThrow(); + }); +}); + +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..1959a6192a --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-store.ts @@ -0,0 +1,58 @@ +import { putParameter } from '@aws-github-runner/aws-ssm-util'; + +import type { RunnerConfigMetadata, RunnerConfigRecord, RunnerConfigStore } from '../../core'; +import type {} from './environment'; +import { loadSsmParameterStoreTagsFromEnvironment } from './parameter-store-tags'; +import { cleanSSMTokens, type SSMCleanupOptions } from './runner-config-housekeeper'; + +interface AwsSsmRunnerConfigStoreConfig { + tokenPath?: string; + parameterStoreTags: { Key: string; Value: string }[]; + cleanupOptions?: SSMCleanupOptions; +} + +export function createAwsSsmRunnerConfigStore(): RunnerConfigStore { + const tokenPath = process.env.SSM_TOKEN_PATH; + const cleanupOptions = + process.env.SSM_CLEANUP_CONFIG !== undefined + ? (JSON.parse(process.env.SSM_CLEANUP_CONFIG) as SSMCleanupOptions) + : undefined; + const hasWriterConfig = tokenPath !== undefined && tokenPath.trim() !== ''; + + if (!hasWriterConfig && cleanupOptions === undefined) { + throw new Error('Environment variable SSM_TOKEN_PATH is not set'); + } + + return new AwsSsmRunnerConfigStore({ + tokenPath: hasWriterConfig ? tokenPath : undefined, + parameterStoreTags: hasWriterConfig ? loadSsmParameterStoreTagsFromEnvironment() : [], + cleanupOptions, + }); +} + +class AwsSsmRunnerConfigStore implements RunnerConfigStore { + readonly maxWritesPerSecond = 40; + + constructor(private readonly config: AwsSsmRunnerConfigStoreConfig) {} + + async create(record: RunnerConfigRecord, options: { metadata?: RunnerConfigMetadata[] } = {}): Promise { + if (!this.config.tokenPath) { + throw new Error('Environment variable SSM_TOKEN_PATH is not set'); + } + + await putParameter(`${this.config.tokenPath}/${record.runnerId}`, record.value, true, { + tags: [ + ...(options.metadata ?? []).map(({ key, value }) => ({ Key: key, Value: value })), + ...this.config.parameterStoreTags, + ], + }); + } + + async houseKeeper(): Promise { + if (!this.config.cleanupOptions) { + throw new Error('Environment variable SSM_CLEANUP_CONFIG is not set'); + } + + await cleanSSMTokens(this.config.cleanupOptions); + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.test.ts b/lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.test.ts new file mode 100644 index 0000000000..3943d5a401 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.test.ts @@ -0,0 +1,72 @@ +import { getParameter, putParameter } from '@aws-github-runner/aws-ssm-util'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmRunnerGroupCacheStore } from './runner-group-cache-store'; + +vi.mock('@aws-github-runner/aws-ssm-util', () => ({ + getParameter: vi.fn(), + putParameter: vi.fn(), +})); + +const getParameterMock = vi.mocked(getParameter); +const putParameterMock = vi.mocked(putParameter); +const cleanEnv = process.env; + +describe('aws_ssm runner group cache store', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.SSM_PARAMETER_STORE_TAGS; + process.env.SSM_CONFIG_PATH = '/runner/config'; + }); + + it('gets and parses a runner group id from the legacy path', async () => { + getParameterMock.mockResolvedValue('42'); + const store = createAwsSsmRunnerGroupCacheStore(); + + await expect(store.get('Default')).resolves.toBe(42); + expect(getParameterMock).toHaveBeenCalledWith('/runner/config/runner-group/Default'); + }); + + it('preserves the previous parseInt behavior for cached values', async () => { + getParameterMock.mockResolvedValue('42cached'); + const store = createAwsSsmRunnerGroupCacheStore(); + + await expect(store.get('Default')).resolves.toBe(42); + }); + + it('propagates cache read errors', async () => { + const error = new Error('not found'); + getParameterMock.mockRejectedValue(error); + const store = createAwsSsmRunnerGroupCacheStore(); + + await expect(store.get('Default')).rejects.toBe(error); + }); + + it('creates a plaintext parameter at the legacy path with configured tags', async () => { + process.env.SSM_PARAMETER_STORE_TAGS = JSON.stringify([{ Key: 'Environment', Value: 'test' }]); + const store = createAwsSsmRunnerGroupCacheStore(); + + await store.create({ runnerGroupName: 'Default', runnerGroupId: 42 }); + + expect(putParameterMock).toHaveBeenCalledWith('/runner/config/runner-group/Default', '42', false, { + tags: [{ Key: 'Environment', Value: 'test' }], + }); + }); + + it.each([undefined, '', ' '])('rejects missing or blank SSM_CONFIG_PATH %j', (configPath) => { + setConfigPath(configPath); + + expect(() => createAwsSsmRunnerGroupCacheStore()).toThrow('Environment variable SSM_CONFIG_PATH is not set'); + expect(getParameterMock).not.toHaveBeenCalled(); + expect(putParameterMock).not.toHaveBeenCalled(); + }); +}); + +function setConfigPath(configPath: string | undefined): void { + if (configPath === undefined) { + delete process.env.SSM_CONFIG_PATH; + } else { + process.env.SSM_CONFIG_PATH = configPath; + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.ts b/lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.ts new file mode 100644 index 0000000000..f79ee1b9ed --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-group-cache-store.ts @@ -0,0 +1,41 @@ +import { getParameter, putParameter } from '@aws-github-runner/aws-ssm-util'; + +import type { RunnerGroupCacheRecord, RunnerGroupCacheStore } from '../../core'; +import type {} from './environment'; +import { loadSsmParameterStoreTagsFromEnvironment } from './parameter-store-tags'; + +interface AwsSsmRunnerGroupCacheStoreConfig { + configPath: string; + parameterStoreTags: { Key: string; Value: string }[]; +} + +export function createAwsSsmRunnerGroupCacheStore(): RunnerGroupCacheStore { + const configPath = process.env.SSM_CONFIG_PATH; + if (!configPath || configPath.trim() === '') { + throw new Error('Environment variable SSM_CONFIG_PATH is not set'); + } + + return new AwsSsmRunnerGroupCacheStore({ + configPath, + parameterStoreTags: loadSsmParameterStoreTagsFromEnvironment(), + }); +} + +class AwsSsmRunnerGroupCacheStore implements RunnerGroupCacheStore { + constructor(private readonly config: AwsSsmRunnerGroupCacheStoreConfig) {} + + async get(runnerGroupName: string): Promise { + const runnerGroupId = await getParameter(this.parameterName(runnerGroupName)); + return parseInt(runnerGroupId); + } + + async create(record: RunnerGroupCacheRecord): Promise { + await putParameter(this.parameterName(record.runnerGroupName), record.runnerGroupId.toString(), false, { + tags: this.config.parameterStoreTags, + }); + } + + private parameterName(runnerGroupName: string): string { + return `${this.config.configPath}/runner-group/${runnerGroupName}`; + } +} diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-matcher-config-store.test.ts b/lambdas/libs/storage-providers/aws/ssm/runner-matcher-config-store.test.ts new file mode 100644 index 0000000000..4b9d2f3379 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-matcher-config-store.test.ts @@ -0,0 +1,103 @@ +import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmRunnerMatcherConfigStore } from './runner-matcher-config-store'; + +vi.mock('@aws-github-runner/aws-ssm-util', () => ({ + getParameter: vi.fn(), + getParameters: vi.fn(), +})); + +const getParameterMock = vi.mocked(getParameter); +const getParametersMock = vi.mocked(getParameters); +const cleanEnv = process.env; + +describe('aws_ssm runner matcher config store', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH; + }); + + it('loads a single matcher config parameter', async () => { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/runner/matcher/config'; + getParameterMock.mockResolvedValue('[{"id":"runner"}]'); + + await expect(createAwsSsmRunnerMatcherConfigStore().get()).resolves.toBe('[{"id":"runner"}]'); + + expect(getParameterMock).toHaveBeenCalledWith('/runner/matcher/config'); + expect(getParametersMock).not.toHaveBeenCalled(); + }); + + it('loads and concatenates matcher config chunks in configured order', async () => { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = ' /runner/matcher/1 : : /runner/matcher/2 '; + getParametersMock.mockResolvedValue( + new Map([ + ['/runner/matcher/2', ',{"id":"runner-2"}]'], + ['/runner/matcher/1', '[{"id":"runner-1"}'], + ]), + ); + + await expect(createAwsSsmRunnerMatcherConfigStore().get()).resolves.toBe('[{"id":"runner-1"},{"id":"runner-2"}]'); + + expect(getParametersMock).toHaveBeenCalledWith(['/runner/matcher/1', '/runner/matcher/2']); + expect(getParameterMock).not.toHaveBeenCalled(); + }); + + it('rejects a missing matcher config chunk', async () => { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/runner/matcher/1:/runner/matcher/2'; + getParametersMock.mockResolvedValue(new Map([['/runner/matcher/1', '[{"id":"runner-1"}']])); + + await expect(createAwsSsmRunnerMatcherConfigStore().get()).rejects.toThrow( + 'Failed to load parameter for matcherConfig from path /runner/matcher/2: Parameter not found', + ); + }); + + it('rejects malformed combined matcher config', async () => { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/runner/matcher/1:/runner/matcher/2'; + getParametersMock.mockResolvedValue( + new Map([ + ['/runner/matcher/1', '[{"id":"runner-1"}'], + ['/runner/matcher/2', ',{"id":"runner-2"}'], + ]), + ); + + await expect(createAwsSsmRunnerMatcherConfigStore().get()).rejects.toThrow( + "Failed to load/parse combined matcher config: Expected ',' or ']' after array element", + ); + }); + + it('propagates a single parameter read failure', async () => { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/runner/matcher/config'; + const error = new Error('read failed'); + getParameterMock.mockRejectedValue(error); + + await expect(createAwsSsmRunnerMatcherConfigStore().get()).rejects.toThrow( + 'Failed to load parameter for matcherConfig from path /runner/matcher/config: read failed', + ); + }); + + it('propagates a batch parameter read failure', async () => { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = '/runner/matcher/1:/runner/matcher/2'; + const error = new Error('read failed'); + getParametersMock.mockRejectedValue(error); + + await expect(createAwsSsmRunnerMatcherConfigStore().get()).rejects.toThrow( + 'Failed to load/parse combined matcher config: read failed', + ); + }); + + it.each([undefined, '', ' '])('requires matcher config parameter paths for input %j', (parameterPaths) => { + if (parameterPaths === undefined) { + delete process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH; + } else { + process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH = parameterPaths; + } + + expect(() => createAwsSsmRunnerMatcherConfigStore()).toThrow( + 'Environment variable PARAMETER_RUNNER_MATCHER_CONFIG_PATH is not set', + ); + expect(getParameterMock).not.toHaveBeenCalled(); + expect(getParametersMock).not.toHaveBeenCalled(); + }); +}); diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-matcher-config-store.ts b/lambdas/libs/storage-providers/aws/ssm/runner-matcher-config-store.ts new file mode 100644 index 0000000000..166151b850 --- /dev/null +++ b/lambdas/libs/storage-providers/aws/ssm/runner-matcher-config-store.ts @@ -0,0 +1,69 @@ +import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util'; + +import type { RunnerMatcherConfigStore } from '../../core'; +import type {} from './environment'; + +export function createAwsSsmRunnerMatcherConfigStore(): RunnerMatcherConfigStore { + const parameterPaths = process.env.PARAMETER_RUNNER_MATCHER_CONFIG_PATH; + if (!parameterPaths || parameterPaths.trim() === '') { + throw new Error('Environment variable PARAMETER_RUNNER_MATCHER_CONFIG_PATH is not set'); + } + + const paths = parameterPaths + .split(':') + .map((path) => path.trim()) + .filter(Boolean); + + if (paths.length === 0) { + throw new Error('Environment variable PARAMETER_RUNNER_MATCHER_CONFIG_PATH is not set'); + } + + return new AwsSsmRunnerMatcherConfigStore(paths); +} + +class AwsSsmRunnerMatcherConfigStore implements RunnerMatcherConfigStore { + constructor(private readonly parameterPaths: string[]) {} + + async get(): Promise { + if (this.parameterPaths.length === 1) { + const path = this.parameterPaths[0]; + try { + return await getParameter(path); + } catch (error) { + throw new Error(`Failed to load parameter for matcherConfig from path ${path}: ${(error as Error).message}`); + } + } + + let parameters: Map; + try { + parameters = await getParameters(this.parameterPaths); + } catch (error) { + throw new Error(`Failed to load/parse combined matcher config: ${(error as Error).message}`); + } + + let combined = ''; + const errors: string[] = []; + for (const path of this.parameterPaths) { + const value = parameters.get(path); + if (value) { + combined += value; + } else { + errors.push(`Failed to load parameter for matcherConfig from path ${path}: Parameter not found`); + } + } + + if (combined) { + try { + JSON.parse(combined); + } catch (error) { + errors.push(`Failed to load/parse combined matcher config: ${(error as Error).message}`); + } + } + + if (errors.length > 0) { + throw new Error(errors.join(', ')); + } + + return combined; + } +} diff --git a/lambdas/libs/storage-providers/core/index.ts b/lambdas/libs/storage-providers/core/index.ts new file mode 100644 index 0000000000..46117ee622 --- /dev/null +++ b/lambdas/libs/storage-providers/core/index.ts @@ -0,0 +1,43 @@ +export interface GitHubAppCredential { + appId: number; + privateKey: string; + installationId?: number; +} + +export interface GitHubAppCredentialsStore { + get(): Promise; +} + +export interface GitHubWebhookSecretStore { + get(): Promise; +} + +export interface RunnerConfigMetadata { + key: string; + value: string; +} + +export interface RunnerConfigRecord { + runnerId: string; + value: string; +} + +export interface RunnerConfigStore { + readonly maxWritesPerSecond?: number; + create(record: RunnerConfigRecord, options?: { metadata?: RunnerConfigMetadata[] }): Promise; + houseKeeper(): Promise; +} + +export interface RunnerGroupCacheRecord { + runnerGroupName: string; + runnerGroupId: number; +} + +export interface RunnerGroupCacheStore { + get(runnerGroupName: string): Promise; + create(record: RunnerGroupCacheRecord): Promise; +} + +export interface RunnerMatcherConfigStore { + get(): 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/github-app-credentials.test.ts b/lambdas/libs/storage-providers/github-app-credentials.test.ts new file mode 100644 index 0000000000..fe1870e387 --- /dev/null +++ b/lambdas/libs/storage-providers/github-app-credentials.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmGitHubAppCredentialsStore } from './aws/ssm/github-app-credentials-store'; +import type { GitHubAppCredentialsStore } from './core'; +import { getGitHubAppCredentialsStore, resetGitHubAppCredentialsStore } from './github-app-credentials'; + +vi.mock('./aws/ssm/github-app-credentials-store', () => ({ + createAwsSsmGitHubAppCredentialsStore: vi.fn(), +})); + +const createAwsSsmGitHubAppCredentialsStoreMock = vi.mocked(createAwsSsmGitHubAppCredentialsStore); +const cleanEnv = process.env; + +describe('GitHub App credentials store selection', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; + resetGitHubAppCredentialsStore(); + }); + + it.each([undefined, '', ' '])('uses aws_ssm for default selector input %j', (provider) => { + setProvider(provider); + const store = stubStore(); + + expect(getGitHubAppCredentialsStore()).toBe(store); + expect(createAwsSsmGitHubAppCredentialsStoreMock).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(getGitHubAppCredentialsStore()).toBe(store); + expect(createAwsSsmGitHubAppCredentialsStoreMock).toHaveBeenCalledOnce(); + }); + + it('rejects an unsupported provider on first use', () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + + expect(() => getGitHubAppCredentialsStore()).toThrow("Unsupported runner config storage provider 'not-registered'"); + expect(createAwsSsmGitHubAppCredentialsStoreMock).not.toHaveBeenCalled(); + }); + + it('selects lazily and caches the created store', () => { + const store = stubStore(); + + expect(createAwsSsmGitHubAppCredentialsStoreMock).not.toHaveBeenCalled(); + const first = getGitHubAppCredentialsStore(); + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + const second = getGitHubAppCredentialsStore(); + + expect(first).toBe(store); + expect(second).toBe(store); + expect(createAwsSsmGitHubAppCredentialsStoreMock).toHaveBeenCalledOnce(); + }); + + it('selects again after the test reset', () => { + const firstStore = stubStore(); + expect(getGitHubAppCredentialsStore()).toBe(firstStore); + + const secondStore = { get: vi.fn() } satisfies GitHubAppCredentialsStore; + createAwsSsmGitHubAppCredentialsStoreMock.mockReturnValue(secondStore); + resetGitHubAppCredentialsStore(); + + expect(getGitHubAppCredentialsStore()).toBe(secondStore); + expect(createAwsSsmGitHubAppCredentialsStoreMock).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(): GitHubAppCredentialsStore { + const store = { get: vi.fn() } satisfies GitHubAppCredentialsStore; + createAwsSsmGitHubAppCredentialsStoreMock.mockReturnValue(store); + return store; +} diff --git a/lambdas/libs/storage-providers/github-app-credentials.ts b/lambdas/libs/storage-providers/github-app-credentials.ts new file mode 100644 index 0000000000..683ab6bb3e --- /dev/null +++ b/lambdas/libs/storage-providers/github-app-credentials.ts @@ -0,0 +1,23 @@ +import { createAwsSsmGitHubAppCredentialsStore } from './aws/ssm/github-app-credentials-store'; +import type { GitHubAppCredentialsStore } from './core'; +import type {} from './environment'; +import { resolveRunnerConfigStorageProvider, type RunnerConfigStorageProvider } from './provider'; + +type GitHubAppCredentialsStoreFactory = () => GitHubAppCredentialsStore; + +const providerFactories = { + aws_ssm: createAwsSsmGitHubAppCredentialsStore, +} as const satisfies Record; + +let githubAppCredentialsStore: GitHubAppCredentialsStore | undefined; + +export function getGitHubAppCredentialsStore(): GitHubAppCredentialsStore { + githubAppCredentialsStore ??= + providerFactories[resolveRunnerConfigStorageProvider(process.env.RUNNER_CONFIG_STORAGE_PROVIDER)](); + return githubAppCredentialsStore; +} + +// Test-only reset for cases that need to exercise first-use environment selection. +export function resetGitHubAppCredentialsStore(): void { + githubAppCredentialsStore = undefined; +} diff --git a/lambdas/libs/storage-providers/github-webhook-secret.test.ts b/lambdas/libs/storage-providers/github-webhook-secret.test.ts new file mode 100644 index 0000000000..5888e735b0 --- /dev/null +++ b/lambdas/libs/storage-providers/github-webhook-secret.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmGitHubWebhookSecretStore } from './aws/ssm/github-webhook-secret-store'; +import type { GitHubWebhookSecretStore } from './core'; +import { getGitHubWebhookSecretStore, resetGitHubWebhookSecretStore } from './github-webhook-secret'; + +vi.mock('./aws/ssm/github-webhook-secret-store', () => ({ + createAwsSsmGitHubWebhookSecretStore: vi.fn(), +})); + +const createAwsSsmGitHubWebhookSecretStoreMock = vi.mocked(createAwsSsmGitHubWebhookSecretStore); +const cleanEnv = process.env; + +describe('GitHub webhook secret store selection', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; + resetGitHubWebhookSecretStore(); + }); + + it.each([undefined, '', ' '])('uses aws_ssm for default selector input %j', (provider) => { + setProvider(provider); + const store = stubStore(); + + expect(getGitHubWebhookSecretStore()).toBe(store); + expect(createAwsSsmGitHubWebhookSecretStoreMock).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(getGitHubWebhookSecretStore()).toBe(store); + expect(createAwsSsmGitHubWebhookSecretStoreMock).toHaveBeenCalledOnce(); + }); + + it('rejects an unsupported provider on first use', () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + + expect(() => getGitHubWebhookSecretStore()).toThrow("Unsupported runner config storage provider 'not-registered'"); + expect(createAwsSsmGitHubWebhookSecretStoreMock).not.toHaveBeenCalled(); + }); + + it('selects lazily and caches the created store', () => { + const store = stubStore(); + + expect(createAwsSsmGitHubWebhookSecretStoreMock).not.toHaveBeenCalled(); + const first = getGitHubWebhookSecretStore(); + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + const second = getGitHubWebhookSecretStore(); + + expect(first).toBe(store); + expect(second).toBe(store); + expect(createAwsSsmGitHubWebhookSecretStoreMock).toHaveBeenCalledOnce(); + }); + + it('selects again after the test reset', () => { + const firstStore = stubStore(); + expect(getGitHubWebhookSecretStore()).toBe(firstStore); + + const secondStore = { get: vi.fn() } satisfies GitHubWebhookSecretStore; + createAwsSsmGitHubWebhookSecretStoreMock.mockReturnValue(secondStore); + resetGitHubWebhookSecretStore(); + + expect(getGitHubWebhookSecretStore()).toBe(secondStore); + expect(createAwsSsmGitHubWebhookSecretStoreMock).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(): GitHubWebhookSecretStore { + const store = { get: vi.fn() } satisfies GitHubWebhookSecretStore; + createAwsSsmGitHubWebhookSecretStoreMock.mockReturnValue(store); + return store; +} diff --git a/lambdas/libs/storage-providers/github-webhook-secret.ts b/lambdas/libs/storage-providers/github-webhook-secret.ts new file mode 100644 index 0000000000..f13df08718 --- /dev/null +++ b/lambdas/libs/storage-providers/github-webhook-secret.ts @@ -0,0 +1,23 @@ +import { createAwsSsmGitHubWebhookSecretStore } from './aws/ssm/github-webhook-secret-store'; +import type { GitHubWebhookSecretStore } from './core'; +import type {} from './environment'; +import { resolveRunnerConfigStorageProvider, type RunnerConfigStorageProvider } from './provider'; + +type GitHubWebhookSecretStoreFactory = () => GitHubWebhookSecretStore; + +const providerFactories = { + aws_ssm: createAwsSsmGitHubWebhookSecretStore, +} as const satisfies Record; + +let githubWebhookSecretStore: GitHubWebhookSecretStore | undefined; + +export function getGitHubWebhookSecretStore(): GitHubWebhookSecretStore { + githubWebhookSecretStore ??= + providerFactories[resolveRunnerConfigStorageProvider(process.env.RUNNER_CONFIG_STORAGE_PROVIDER)](); + return githubWebhookSecretStore; +} + +// Test-only reset for cases that need to exercise first-use environment selection. +export function resetGitHubWebhookSecretStore(): void { + githubWebhookSecretStore = undefined; +} diff --git a/lambdas/libs/storage-providers/index.ts b/lambdas/libs/storage-providers/index.ts new file mode 100644 index 0000000000..49e9d11c18 --- /dev/null +++ b/lambdas/libs/storage-providers/index.ts @@ -0,0 +1,16 @@ +export type { + GitHubAppCredential, + GitHubAppCredentialsStore, + GitHubWebhookSecretStore, + RunnerConfigMetadata, + RunnerConfigRecord, + RunnerConfigStore, + RunnerGroupCacheRecord, + RunnerGroupCacheStore, + RunnerMatcherConfigStore, +} from './core'; +export { getGitHubAppCredentialsStore, resetGitHubAppCredentialsStore } from './github-app-credentials'; +export { getGitHubWebhookSecretStore, resetGitHubWebhookSecretStore } from './github-webhook-secret'; +export { getRunnerConfigStore, resetRunnerConfigStore } from './runner-config'; +export { getRunnerGroupCacheStore, resetRunnerGroupCacheStore } from './runner-group-cache'; +export { getRunnerMatcherConfigStore, resetRunnerMatcherConfigStore } from './runner-matcher-config'; diff --git a/lambdas/libs/storage-providers/package.json b/lambdas/libs/storage-providers/package.json new file mode 100644 index 0000000000..745d026851 --- /dev/null +++ b/lambdas/libs/storage-providers/package.json @@ -0,0 +1,35 @@ +{ + "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" + }, + "devDependencies": { + "aws-sdk-client-mock": "^4.1.0", + "aws-sdk-client-mock-jest": "^4.1.0" + }, + "dependencies": { + "@aws-github-runner/aws-powertools-util": "*", + "@aws-github-runner/aws-ssm-util": "*", + "@aws-sdk/client-ssm": "^3.1009.0" + }, + "nx": { + "includedScripts": [ + "format", + "format-check", + "lint", + "all" + ] + } +} diff --git a/lambdas/libs/storage-providers/provider.ts b/lambdas/libs/storage-providers/provider.ts new file mode 100644 index 0000000000..82b2c7895b --- /dev/null +++ b/lambdas/libs/storage-providers/provider.ts @@ -0,0 +1,26 @@ +export const runnerConfigStorageProviders = ['aws_ssm'] as const; + +export type RunnerConfigStorageProvider = (typeof runnerConfigStorageProviders)[number]; + +const defaultProvider = 'aws_ssm' satisfies RunnerConfigStorageProvider; + +export function resolveRunnerConfigStorageProvider(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 (!runnerConfigStorageProviders.includes(normalizedProvider as RunnerConfigStorageProvider)) { + throw new Error(`Unsupported runner config storage provider '${String(provider)}'`); + } + + return normalizedProvider as RunnerConfigStorageProvider; +} 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..95ba3787dd --- /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(), houseKeeper: 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(), houseKeeper: 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..180c808d78 --- /dev/null +++ b/lambdas/libs/storage-providers/runner-config.ts @@ -0,0 +1,23 @@ +import { createAwsSsmRunnerConfigStore } from './aws/ssm/runner-config-store'; +import type { RunnerConfigStore } from './core'; +import type {} from './environment'; +import { resolveRunnerConfigStorageProvider, type RunnerConfigStorageProvider } from './provider'; + +type RunnerConfigStoreFactory = () => RunnerConfigStore; + +const providerFactories = { + aws_ssm: createAwsSsmRunnerConfigStore, +} as const satisfies Record; + +let runnerConfigStore: RunnerConfigStore | undefined; + +export function getRunnerConfigStore(): RunnerConfigStore { + runnerConfigStore ??= + providerFactories[resolveRunnerConfigStorageProvider(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; +} diff --git a/lambdas/libs/storage-providers/runner-group-cache.test.ts b/lambdas/libs/storage-providers/runner-group-cache.test.ts new file mode 100644 index 0000000000..e67e307905 --- /dev/null +++ b/lambdas/libs/storage-providers/runner-group-cache.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmRunnerGroupCacheStore } from './aws/ssm/runner-group-cache-store'; +import type { RunnerGroupCacheStore } from './core'; +import { getRunnerGroupCacheStore, resetRunnerGroupCacheStore } from './runner-group-cache'; + +vi.mock('./aws/ssm/runner-group-cache-store', () => ({ + createAwsSsmRunnerGroupCacheStore: vi.fn(), +})); + +const createAwsSsmRunnerGroupCacheStoreMock = vi.mocked(createAwsSsmRunnerGroupCacheStore); +const cleanEnv = process.env; + +describe('runner group cache store selection', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; + resetRunnerGroupCacheStore(); + }); + + it.each([undefined, '', ' '])('uses aws_ssm for default selector input %j', (provider) => { + setProvider(provider); + const store = stubStore(); + + expect(getRunnerGroupCacheStore()).toBe(store); + expect(createAwsSsmRunnerGroupCacheStoreMock).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(getRunnerGroupCacheStore()).toBe(store); + expect(createAwsSsmRunnerGroupCacheStoreMock).toHaveBeenCalledOnce(); + }); + + it('rejects an unsupported provider on first use', () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + + expect(() => getRunnerGroupCacheStore()).toThrow("Unsupported runner config storage provider 'not-registered'"); + expect(createAwsSsmRunnerGroupCacheStoreMock).not.toHaveBeenCalled(); + }); + + it('selects lazily and caches the created store', () => { + const store = stubStore(); + + expect(createAwsSsmRunnerGroupCacheStoreMock).not.toHaveBeenCalled(); + const first = getRunnerGroupCacheStore(); + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + const second = getRunnerGroupCacheStore(); + + expect(first).toBe(store); + expect(second).toBe(store); + expect(createAwsSsmRunnerGroupCacheStoreMock).toHaveBeenCalledOnce(); + }); + + it('selects again after the test reset', () => { + const firstStore = stubStore(); + expect(getRunnerGroupCacheStore()).toBe(firstStore); + + const secondStore = { get: vi.fn(), create: vi.fn() } satisfies RunnerGroupCacheStore; + createAwsSsmRunnerGroupCacheStoreMock.mockReturnValue(secondStore); + resetRunnerGroupCacheStore(); + + expect(getRunnerGroupCacheStore()).toBe(secondStore); + expect(createAwsSsmRunnerGroupCacheStoreMock).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(): RunnerGroupCacheStore { + const store = { get: vi.fn(), create: vi.fn() } satisfies RunnerGroupCacheStore; + createAwsSsmRunnerGroupCacheStoreMock.mockReturnValue(store); + return store; +} diff --git a/lambdas/libs/storage-providers/runner-group-cache.ts b/lambdas/libs/storage-providers/runner-group-cache.ts new file mode 100644 index 0000000000..a28b00f48e --- /dev/null +++ b/lambdas/libs/storage-providers/runner-group-cache.ts @@ -0,0 +1,23 @@ +import { createAwsSsmRunnerGroupCacheStore } from './aws/ssm/runner-group-cache-store'; +import type { RunnerGroupCacheStore } from './core'; +import type {} from './environment'; +import { resolveRunnerConfigStorageProvider, type RunnerConfigStorageProvider } from './provider'; + +type RunnerGroupCacheStoreFactory = () => RunnerGroupCacheStore; + +const providerFactories = { + aws_ssm: createAwsSsmRunnerGroupCacheStore, +} as const satisfies Record; + +let runnerGroupCacheStore: RunnerGroupCacheStore | undefined; + +export function getRunnerGroupCacheStore(): RunnerGroupCacheStore { + runnerGroupCacheStore ??= + providerFactories[resolveRunnerConfigStorageProvider(process.env.RUNNER_CONFIG_STORAGE_PROVIDER)](); + return runnerGroupCacheStore; +} + +// Test-only reset for cases that need to exercise first-use environment selection. +export function resetRunnerGroupCacheStore(): void { + runnerGroupCacheStore = undefined; +} diff --git a/lambdas/libs/storage-providers/runner-matcher-config.test.ts b/lambdas/libs/storage-providers/runner-matcher-config.test.ts new file mode 100644 index 0000000000..0dd7f42ad3 --- /dev/null +++ b/lambdas/libs/storage-providers/runner-matcher-config.test.ts @@ -0,0 +1,83 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { createAwsSsmRunnerMatcherConfigStore } from './aws/ssm/runner-matcher-config-store'; +import type { RunnerMatcherConfigStore } from './core'; +import { getRunnerMatcherConfigStore, resetRunnerMatcherConfigStore } from './runner-matcher-config'; + +vi.mock('./aws/ssm/runner-matcher-config-store', () => ({ + createAwsSsmRunnerMatcherConfigStore: vi.fn(), +})); + +const createAwsSsmRunnerMatcherConfigStoreMock = vi.mocked(createAwsSsmRunnerMatcherConfigStore); +const cleanEnv = process.env; + +describe('runner matcher config store selection', () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env = { ...cleanEnv }; + delete process.env.RUNNER_CONFIG_STORAGE_PROVIDER; + resetRunnerMatcherConfigStore(); + }); + + it.each([undefined, '', ' '])('uses aws_ssm for default selector input %j', (provider) => { + setProvider(provider); + const store = stubStore(); + + expect(getRunnerMatcherConfigStore()).toBe(store); + expect(createAwsSsmRunnerMatcherConfigStoreMock).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(getRunnerMatcherConfigStore()).toBe(store); + expect(createAwsSsmRunnerMatcherConfigStoreMock).toHaveBeenCalledOnce(); + }); + + it('rejects an unsupported provider on first use', () => { + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + + expect(() => getRunnerMatcherConfigStore()).toThrow("Unsupported runner config storage provider 'not-registered'"); + expect(createAwsSsmRunnerMatcherConfigStoreMock).not.toHaveBeenCalled(); + }); + + it('selects lazily and caches the created store', () => { + const store = stubStore(); + + expect(createAwsSsmRunnerMatcherConfigStoreMock).not.toHaveBeenCalled(); + const first = getRunnerMatcherConfigStore(); + process.env.RUNNER_CONFIG_STORAGE_PROVIDER = 'not-registered'; + const second = getRunnerMatcherConfigStore(); + + expect(first).toBe(store); + expect(second).toBe(store); + expect(createAwsSsmRunnerMatcherConfigStoreMock).toHaveBeenCalledOnce(); + }); + + it('selects again after the test reset', () => { + const firstStore = stubStore(); + expect(getRunnerMatcherConfigStore()).toBe(firstStore); + + const secondStore = { get: vi.fn() } satisfies RunnerMatcherConfigStore; + createAwsSsmRunnerMatcherConfigStoreMock.mockReturnValue(secondStore); + resetRunnerMatcherConfigStore(); + + expect(getRunnerMatcherConfigStore()).toBe(secondStore); + expect(createAwsSsmRunnerMatcherConfigStoreMock).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(): RunnerMatcherConfigStore { + const store = { get: vi.fn() } satisfies RunnerMatcherConfigStore; + createAwsSsmRunnerMatcherConfigStoreMock.mockReturnValue(store); + return store; +} diff --git a/lambdas/libs/storage-providers/runner-matcher-config.ts b/lambdas/libs/storage-providers/runner-matcher-config.ts new file mode 100644 index 0000000000..6d56d49754 --- /dev/null +++ b/lambdas/libs/storage-providers/runner-matcher-config.ts @@ -0,0 +1,23 @@ +import { createAwsSsmRunnerMatcherConfigStore } from './aws/ssm/runner-matcher-config-store'; +import type { RunnerMatcherConfigStore } from './core'; +import type {} from './environment'; +import { resolveRunnerConfigStorageProvider, type RunnerConfigStorageProvider } from './provider'; + +type RunnerMatcherConfigStoreFactory = () => RunnerMatcherConfigStore; + +const providerFactories = { + aws_ssm: createAwsSsmRunnerMatcherConfigStore, +} as const satisfies Record; + +let runnerMatcherConfigStore: RunnerMatcherConfigStore | undefined; + +export function getRunnerMatcherConfigStore(): RunnerMatcherConfigStore { + runnerMatcherConfigStore ??= + providerFactories[resolveRunnerConfigStorageProvider(process.env.RUNNER_CONFIG_STORAGE_PROVIDER)](); + return runnerMatcherConfigStore; +} + +// Test-only reset for cases that need to exercise first-use environment selection. +export function resetRunnerMatcherConfigStore(): void { + runnerMatcherConfigStore = undefined; +} 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..0b0b3356e8 --- /dev/null +++ b/lambdas/libs/storage-providers/vitest.config.ts @@ -0,0 +1,24 @@ +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', + 'provider.ts', + 'github-app-credentials.ts', + 'github-webhook-secret.ts', + 'runner-config.ts', + 'runner-group-cache.ts', + 'runner-matcher-config.ts', + 'core/**/*.ts', + 'aws/**/*.ts', + ], + exclude: ['**/*.test.ts', '**/*.d.ts'], + }, + }, +}); diff --git a/lambdas/yarn.lock b/lambdas/yarn.lock index 10175a11aa..1322072419 100644 --- a/lambdas/yarn.lock +++ b/lambdas/yarn.lock @@ -162,8 +162,8 @@ __metadata: resolution: "@aws-github-runner/control-plane@workspace:functions/control-plane" dependencies: "@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" @@ -209,6 +209,18 @@ __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-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 + "@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" @@ -237,8 +249,8 @@ __metadata: resolution: "@aws-github-runner/webhook@workspace:functions/webhook" dependencies: "@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-sdk/client-eventbridge": "npm:^3.1009.0" "@aws-sdk/client-sqs": "npm:^3.1009.0" "@middy/core": "npm:^6.4.5"