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/dispatch.test.ts b/lambdas/functions/webhook/src/runners/dispatch.test.ts index bb2cdc7cce..b3140d6e96 100644 --- a/lambdas/functions/webhook/src/runners/dispatch.test.ts +++ b/lambdas/functions/webhook/src/runners/dispatch.test.ts @@ -1,5 +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'; @@ -14,12 +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; @@ -37,7 +39,6 @@ describe('Dispatcher', () => { vi.clearAllMocks(); vi.resetAllMocks(); - mockSSMResponse(); config = await createConfig(undefined, runnerConfig); }); @@ -242,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); }); }); @@ -394,16 +395,9 @@ describe('Dispatcher', () => { }); }); -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 { @@ -411,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/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/storage-providers/aws/ssm/environment.d.ts b/lambdas/libs/storage-providers/aws/ssm/environment.d.ts index 2604ce057e..ab8b54e88a 100644 --- a/lambdas/libs/storage-providers/aws/ssm/environment.d.ts +++ b/lambdas/libs/storage-providers/aws/ssm/environment.d.ts @@ -3,6 +3,8 @@ export {}; declare global { namespace NodeJS { interface ProcessEnv { + PARAMETER_GITHUB_APP_WEBHOOK_SECRET?: string; + PARAMETER_RUNNER_MATCHER_CONFIG_PATH?: string; SSM_PARAMETER_STORE_TAGS?: string; SSM_CONFIG_PATH?: string; SSM_TOKEN_PATH?: string; 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/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 index 27339a2159..e6060eae2a 100644 --- a/lambdas/libs/storage-providers/core/index.ts +++ b/lambdas/libs/storage-providers/core/index.ts @@ -1,3 +1,6 @@ +export interface GitHubWebhookSecretStore { + get(): Promise; +} export interface RunnerConfigMetadata { key: string; value: string; @@ -58,3 +61,7 @@ export interface RunnerGroupCacheStore { get(runnerGroupName: string): Promise; create(record: RunnerGroupCacheRecord): Promise; } + +export interface RunnerMatcherConfigStore { + get(): Promise; +} 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 index 4b5f14f1c6..fcd4bd9338 100644 --- a/lambdas/libs/storage-providers/index.ts +++ b/lambdas/libs/storage-providers/index.ts @@ -1,6 +1,7 @@ export type { GitHubAppCredential, GitHubAppCredentialsStore, + GitHubWebhookSecretStore, RunnerConfigConsumer, RunnerConfigConsumeOptions, RunnerConfigHousekeeper, @@ -9,6 +10,7 @@ export type { RunnerConfigStore, RunnerGroupCacheRecord, RunnerGroupCacheStore, + RunnerMatcherConfigStore, } from './core'; export { createRunnerConfigHousekeeper } from './runner-config-housekeeper'; export { createRunnerConfigConsumer, type RunnerConfigConsumerConfig } from './runner-config-consumer'; @@ -20,3 +22,5 @@ export { export type { RunnerConfigStorageProvider } from './provider'; export { createCommonStorage, createStorageProviders } from './storage-providers'; export type { StorageProviders, RunnerConfigStorage, CommonStorage } from './core'; +export { getGitHubWebhookSecretStore, resetGitHubWebhookSecretStore } from './github-webhook-secret'; +export { getRunnerMatcherConfigStore, resetRunnerMatcherConfigStore } from './runner-matcher-config'; 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/vitest.config.ts b/lambdas/libs/storage-providers/vitest.config.ts index 20c739f253..a009dab1bf 100644 --- a/lambdas/libs/storage-providers/vitest.config.ts +++ b/lambdas/libs/storage-providers/vitest.config.ts @@ -13,6 +13,8 @@ export default mergeConfig(defaultConfig, { 'runner-config-consumer.ts', 'storage-providers.ts', 'provider.ts', + 'github-webhook-secret.ts', + 'runner-matcher-config.ts', 'core/**/*.ts', 'aws/**/*.ts', ], diff --git a/lambdas/yarn.lock b/lambdas/yarn.lock index 0e7daca0c7..17e99e638d 100644 --- a/lambdas/yarn.lock +++ b/lambdas/yarn.lock @@ -241,8 +241,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"