diff --git a/lambdas/functions/ami-housekeeper/src/ami.test.ts b/lambdas/functions/ami-housekeeper/src/ami.test.ts index 281e97856b..addff90c2c 100644 --- a/lambdas/functions/ami-housekeeper/src/ami.test.ts +++ b/lambdas/functions/ami-housekeeper/src/ami.test.ts @@ -10,7 +10,7 @@ import { import { DescribeParametersCommand, DescribeParametersCommandOutput, - GetParameterCommand, + GetParametersCommand, SSMClient, } from '@aws-sdk/client-ssm'; import { mockClient } from 'aws-sdk-client-mock'; @@ -83,21 +83,22 @@ describe("delete AMI's", () => { mockSSMClient.reset(); mockSSMClient.on(DescribeParametersCommand).resolves(ssmParameters); - mockSSMClient.on(GetParameterCommand, { Name: 'ami-id/ami-ssm0001' }).resolves({ - Parameter: { - Name: 'ami-id/ami-ssm0001', - Type: 'String', - Value: 'ami-ssm0001', - Version: 1, - }, - }); - mockSSMClient.on(GetParameterCommand, { Name: 'ami-id/ami-ssm0002' }).resolves({ - Parameter: { - Name: 'ami-id/ami-ssm0002', - Type: 'String', - Value: 'ami-ssm0002', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand).resolves({ + Parameters: [ + { + Name: 'ami-id/ami-ssm0001', + Type: 'String', + Value: 'ami-ssm0001', + Version: 1, + }, + { + Name: 'ami-id/ami-ssm0002', + Type: 'String', + Value: 'ami-ssm0002', + Version: 1, + }, + ], + InvalidParameters: [], }); mockEC2Client.on(DescribeLaunchTemplatesCommand).resolves({ @@ -143,12 +144,10 @@ describe("delete AMI's", () => { expect(mockEC2Client).toHaveReceivedCommand(DescribeLaunchTemplatesCommand); expect(mockEC2Client).toHaveReceivedCommand(DescribeLaunchTemplateVersionsCommand); expect(mockSSMClient).toHaveReceivedCommand(DescribeParametersCommand); - expect(mockSSMClient).toHaveReceivedCommandTimes(GetParameterCommand, 2); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: 'ami-id/ami-ssm0001', - }); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: 'ami-id/ami-ssm0002', + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['ami-id/ami-ssm0001', 'ami-id/ami-ssm0002'], + WithDecryption: true, }); }); @@ -485,13 +484,16 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/github-runner/config/ami_id' }).resolves({ - Parameter: { - Name: '/github-runner/config/ami_id', - Type: 'String', - Value: 'ami-underscore0001', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand).resolves({ + Parameters: [ + { + Name: '/github-runner/config/ami_id', + Type: 'String', + Value: 'ami-underscore0001', + Version: 1, + }, + ], + InvalidParameters: [], }); await amiCleanup({ @@ -501,8 +503,9 @@ describe("delete AMI's", () => { // AMI should not be deleted because it's referenced in SSM expect(mockEC2Client).not.toHaveReceivedCommand(DeregisterImageCommand); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: '/github-runner/config/ami_id', + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['/github-runner/config/ami_id'], + WithDecryption: true, }); expect(mockSSMClient).not.toHaveReceivedCommand(DescribeParametersCommand); }); @@ -518,13 +521,16 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/github-runner/config/ami-id' }).resolves({ - Parameter: { - Name: '/github-runner/config/ami-id', - Type: 'String', - Value: 'ami-hyphen0001', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand).resolves({ + Parameters: [ + { + Name: '/github-runner/config/ami-id', + Type: 'String', + Value: 'ami-hyphen0001', + Version: 1, + }, + ], + InvalidParameters: [], }); await amiCleanup({ @@ -534,8 +540,9 @@ describe("delete AMI's", () => { // AMI should not be deleted because it's referenced in SSM expect(mockEC2Client).not.toHaveReceivedCommand(DeregisterImageCommand); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: '/github-runner/config/ami-id', + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['/github-runner/config/ami-id'], + WithDecryption: true, }); expect(mockSSMClient).not.toHaveReceivedCommand(DescribeParametersCommand); }); @@ -561,13 +568,16 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/some/path/ami-id' }).resolves({ - Parameter: { - Name: '/some/path/ami-id', - Type: 'String', - Value: 'ami-wildcard0001', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand).resolves({ + Parameters: [ + { + Name: '/some/path/ami-id', + Type: 'String', + Value: 'ami-wildcard0001', + Version: 1, + }, + ], + InvalidParameters: [], }); await amiCleanup({ @@ -580,8 +590,9 @@ describe("delete AMI's", () => { expect(mockSSMClient).toHaveReceivedCommandWith(DescribeParametersCommand, { ParameterFilters: [{ Key: 'Name', Option: 'Contains', Values: ['ami-id'] }], }); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: '/some/path/ami-id', + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['/some/path/ami-id'], + WithDecryption: true, }); }); @@ -606,13 +617,16 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/github-runner/config/ami_id' }).resolves({ - Parameter: { - Name: '/github-runner/config/ami_id', - Type: 'String', - Value: 'ami-wildcard-underscore0001', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand).resolves({ + Parameters: [ + { + Name: '/github-runner/config/ami_id', + Type: 'String', + Value: 'ami-wildcard-underscore0001', + Version: 1, + }, + ], + InvalidParameters: [], }); await amiCleanup({ @@ -625,8 +639,9 @@ describe("delete AMI's", () => { expect(mockSSMClient).toHaveReceivedCommandWith(DescribeParametersCommand, { ParameterFilters: [{ Key: 'Name', Option: 'Contains', Values: ['ami_id'] }], }); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: '/github-runner/config/ami_id', + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['/github-runner/config/ami_id'], + WithDecryption: true, }); }); @@ -649,13 +664,16 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/explicit/ami_id' }).resolves({ - Parameter: { - Name: '/explicit/ami_id', - Type: 'String', - Value: 'ami-explicit0001', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand, { Names: ['/explicit/ami_id'], WithDecryption: true }).resolves({ + Parameters: [ + { + Name: '/explicit/ami_id', + Type: 'String', + Value: 'ami-explicit0001', + Version: 1, + }, + ], + InvalidParameters: [], }); mockSSMClient.on(DescribeParametersCommand).resolves({ @@ -668,13 +686,16 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/discovered/ami-id' }).resolves({ - Parameter: { - Name: '/discovered/ami-id', - Type: 'String', - Value: 'ami-wildcard0001', - Version: 1, - }, + mockSSMClient.on(GetParametersCommand, { Names: ['/discovered/ami-id'], WithDecryption: true }).resolves({ + Parameters: [ + { + Name: '/discovered/ami-id', + Type: 'String', + Value: 'ami-wildcard0001', + Version: 1, + }, + ], + InvalidParameters: [], }); await amiCleanup({ @@ -688,14 +709,16 @@ describe("delete AMI's", () => { ImageId: 'ami-unused0001', }); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: '/explicit/ami_id', + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['/explicit/ami_id'], + WithDecryption: true, }); expect(mockSSMClient).toHaveReceivedCommandWith(DescribeParametersCommand, { ParameterFilters: [{ Key: 'Name', Option: 'Contains', Values: ['ami-id'] }], }); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParameterCommand, { - Name: '/discovered/ami-id', + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersCommand, { + Names: ['/discovered/ami-id'], + WithDecryption: true, }); }); @@ -710,7 +733,7 @@ describe("delete AMI's", () => { ], }); - mockSSMClient.on(GetParameterCommand, { Name: '/nonexistent/ami_id' }).rejects(new Error('ParameterNotFound')); + mockSSMClient.on(GetParametersCommand).rejects(new Error('ParameterNotFound')); // Should not throw and should delete the AMI since SSM reference failed await amiCleanup({ @@ -768,7 +791,7 @@ describe("delete AMI's", () => { ImageId: 'ami-no-ssm0001', }); expect(mockSSMClient).not.toHaveReceivedCommand(DescribeParametersCommand); - expect(mockSSMClient).not.toHaveReceivedCommand(GetParameterCommand); + expect(mockSSMClient).not.toHaveReceivedCommand(GetParametersCommand); }); }); }); diff --git a/lambdas/functions/ami-housekeeper/src/ami.ts b/lambdas/functions/ami-housekeeper/src/ami.ts index f61dea921c..24730d06ad 100644 --- a/lambdas/functions/ami-housekeeper/src/ami.ts +++ b/lambdas/functions/ami-housekeeper/src/ami.ts @@ -8,7 +8,7 @@ import { Filter, Image, } from '@aws-sdk/client-ec2'; -import { GetParameterCommand, SSMClient, DescribeParametersCommand } from '@aws-sdk/client-ssm'; +import { GetParametersCommand, SSMClient, DescribeParametersCommand } from '@aws-sdk/client-ssm'; import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; import { getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; @@ -184,23 +184,56 @@ async function deleteSnapshot(options: AmiCleanupOptions, amiDetails: Image, ec2 } /** - * Resolves the value of an SSM parameter by its name. Doesn't fail on errors, - * but warns instead, as this process is best-effort. + * Resolves the values of multiple SSM parameters by their names in batched API calls. + * Doesn't fail on errors, but warns instead, as this process is best-effort. * - * @param name - The SSM parameter name to resolve + * @param names - Array of SSM parameter names to resolve * @param ssmClient - Configured SSM client for making API calls - * @returns The parameter value if successful, undefined if parameter doesn't exist or access fails + * @returns Array of parameter values (may contain undefined for missing/failed parameters) */ -async function resolveSsmParameterValue(name: string, ssmClient: SSMClient): Promise { - try { - const { Parameter: parameter } = await ssmClient.send(new GetParameterCommand({ Name: name })); +async function resolveSsmParameterValues(names: string[], ssmClient: SSMClient): Promise<(string | undefined)[]> { + if (names.length === 0) { + return []; + } + + const results = new Map(); + + // AWS SSM GetParameters API has a limit of 10 parameters per call + const chunkSize = 10; + for (let i = 0; i < names.length; i += chunkSize) { + const chunk = names.slice(i, i + chunkSize); + try { + const response = await ssmClient.send( + new GetParametersCommand({ + Names: chunk, + WithDecryption: true, + }), + ); - return parameter?.Value; - } catch (error: unknown) { - logger.warn(`Failed to resolve image id from SSM parameter ${name}`, { error }); + for (const param of response.Parameters ?? []) { + if (param.Name) { + results.set(param.Name, param.Value); + } + } - return undefined; + // Log warnings for invalid parameters + for (const invalidParam of response.InvalidParameters ?? []) { + logger.warn( + `Failed to resolve image id from SSM parameter ${invalidParam}: Parameter not found or access denied`, + ); + results.set(invalidParam, undefined); + } + } catch (error: unknown) { + logger.warn(`Failed to resolve image ids from SSM parameters ${chunk.join(', ')}`, { error }); + // Mark all parameters in this chunk as undefined + for (const name of chunk) { + results.set(name, undefined); + } + } } + + // Return values in the same order as input names + return names.map((name) => results.get(name)); } /** @@ -273,11 +306,12 @@ async function getAmisReferedInSSM(options: AmiCleanupOptions): Promise<(string const explicitNames = options.ssmParameterNames.filter((n) => !n.startsWith('*')); const wildcardPatterns = options.ssmParameterNames.filter((n) => n.startsWith('*')); - const explicitValuesPromise = Promise.all(explicitNames.map((name) => resolveSsmParameterValue(name, ssmClient))); + // Batch fetch explicit parameter values in chunks of 10 (AWS API limit) + const explicitValuesPromise = resolveSsmParameterValues(explicitNames, ssmClient); // Handle wildcard patterns by first discovering matching parameters, then // fetching their values - let wildcardValues: Promise<(string | undefined)[]> = Promise.resolve([]); + let wildcardValuesPromise: Promise<(string | undefined)[]> = Promise.resolve([]); if (wildcardPatterns.length > 0) { // Convert wildcard patterns to SSM ParameterFilters using Contains logic // Example: "*ami-id" becomes a filter for parameters containing "ami-id" @@ -287,24 +321,30 @@ async function getAmisReferedInSSM(options: AmiCleanupOptions): Promise<(string Values: [p.replace(/^\*/g, '')], })); - try { - // Discover parameters matching the wildcard patterns - logger.debug('Describing SSM parameter', { filters }); - const ssmParameters = await ssmClient.send(new DescribeParametersCommand({ ParameterFilters: filters })); - - // Fetch the actual values of discovered parameters - wildcardValues = Promise.all( - (ssmParameters.Parameters ?? []).map((param) => resolveSsmParameterValue(param.Name!, ssmClient)), - ); - } catch (e) { - logger.warn('Failed to describe SSM parameters using wildcard patterns', { error: e }); - } + wildcardValuesPromise = (async () => { + try { + // Discover parameters matching the wildcard patterns + logger.debug('Describing SSM parameter', { filters }); + const ssmParameters = await ssmClient.send(new DescribeParametersCommand({ ParameterFilters: filters })); + + // Batch fetch the actual values of discovered parameters + const discoveredNames = (ssmParameters.Parameters ?? []) + .map((param) => param.Name) + .filter((name): name is string => name !== undefined); + + return resolveSsmParameterValues(discoveredNames, ssmClient); + } catch (e) { + logger.warn('Failed to describe SSM parameters using wildcard patterns', { error: e }); + return []; + } + })(); } // Combine results from both explicit and wildcard parameter resolution - const values = await Promise.all([explicitValuesPromise, wildcardValues]); + const [explicitValues, wildcardValues] = await Promise.all([explicitValuesPromise, wildcardValuesPromise]); + const values = [...explicitValues, ...wildcardValues]; logger.debug('Resolved SSM parameter values', { values }); - return values.flat(); + return values; } export { amiCleanup, getAmisNotInUse }; diff --git a/lambdas/functions/control-plane/src/github/auth.test.ts b/lambdas/functions/control-plane/src/github/auth.test.ts index 80b4314182..976f473583 100644 --- a/lambdas/functions/control-plane/src/github/auth.test.ts +++ b/lambdas/functions/control-plane/src/github/auth.test.ts @@ -2,7 +2,7 @@ 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 { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { getParameters } from '@aws-github-runner/aws-ssm-util'; import * as nock from 'nock'; import { createGithubAppAuth, createOctokitClient } from './auth'; @@ -26,7 +26,7 @@ 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 mockedGet = vi.mocked(getParameter); +const mockedGetParameters = vi.mocked(getParameters); beforeEach(() => { vi.resetModules(); @@ -89,7 +89,12 @@ ${decryptedValue}`, const b64PrivateKeyWithLineBreaks = Buffer.from(decryptedValue + '\n' + decryptedValue, 'binary').toString( 'base64', ); - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64PrivateKeyWithLineBreaks); + 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 }); @@ -112,7 +117,12 @@ ${decryptedValue}`, privateKey: decryptedValue, installationId, }; - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); + mockedGetParameters.mockResolvedValueOnce( + new Map([ + [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], + [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], + ]), + ); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); @@ -124,8 +134,7 @@ ${decryptedValue}`, const result = await createGithubAppAuth(installationId); // Assert - expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME); - expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME); + expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); expect(mockedCreatAppAuth).toBeCalledTimes(1); expect(mockedCreatAppAuth).toBeCalledWith({ ...authOptions }); @@ -149,7 +158,12 @@ ${decryptedValue}`, request: mockedRequestInterface.mockImplementation(() => ({ baseUrl: githubServerUrl })), }; - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); + mockedGetParameters.mockResolvedValueOnce( + new Map([ + [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], + [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], + ]), + ); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); // eslint-disable-next-line @typescript-eslint/no-unused-vars @@ -161,8 +175,7 @@ ${decryptedValue}`, const result = await createGithubAppAuth(installationId, githubServerUrl); // Assert - expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME); - expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME); + expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); expect(mockedCreatAppAuth).toBeCalledTimes(1); expect(mockedCreatAppAuth).toBeCalledWith(authOptions); @@ -187,7 +200,12 @@ ${decryptedValue}`, request: mockedRequestInterface.mockImplementation(() => ({ baseUrl: githubServerUrl })), }; - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); + mockedGetParameters.mockResolvedValueOnce( + new Map([ + [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID], + [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64], + ]), + ); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); // Add the required hook method to make it compatible with AuthInterface @@ -198,8 +216,7 @@ ${decryptedValue}`, const result = await createGithubAppAuth(installationId, githubServerUrl); // Assert - expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_ID_NAME); - expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME); + expect(getParameters).toBeCalledWith([PARAMETER_GITHUB_APP_ID_NAME, PARAMETER_GITHUB_APP_KEY_BASE64_NAME]); expect(mockedCreatAppAuth).toBeCalledTimes(1); expect(mockedCreatAppAuth).toBeCalledWith(authOptions); diff --git a/lambdas/functions/control-plane/src/github/auth.ts b/lambdas/functions/control-plane/src/github/auth.ts index d529cc81e8..fa7356ac29 100644 --- a/lambdas/functions/control-plane/src/github/auth.ts +++ b/lambdas/functions/control-plane/src/github/auth.ts @@ -20,7 +20,7 @@ import { request } from '@octokit/request'; import { Octokit } from '@octokit/rest'; import { throttling } from '@octokit/plugin-throttling'; import { createChildLogger } from '@aws-github-runner/aws-powertools-util'; -import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { getParameters } from '@aws-github-runner/aws-ssm-util'; import { EndpointDefaults } from '@octokit/types'; const logger = createChildLogger('gh-auth'); @@ -70,11 +70,25 @@ export async function createGithubInstallationAuth( } async function createAuth(installationId: number | undefined, ghesApiUrl: string): Promise { - const appId = parseInt(await getParameter(process.env.PARAMETER_GITHUB_APP_ID_NAME)); + // Batch fetch both App ID and Private Key in a single SSM API call + const paramNames = [process.env.PARAMETER_GITHUB_APP_ID_NAME, process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME]; + const params = await getParameters(paramNames); + + const appIdValue = params.get(process.env.PARAMETER_GITHUB_APP_ID_NAME); + const privateKeyBase64 = params.get(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME); + + if (!appIdValue) { + throw new Error(`Parameter ${process.env.PARAMETER_GITHUB_APP_ID_NAME} not found`); + } + if (!privateKeyBase64) { + throw new Error(`Parameter ${process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME} not found`); + } + + const appId = parseInt(appIdValue); let authOptions: StrategyOptions = { appId, privateKey: Buffer.from( - await getParameter(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME), + privateKeyBase64, 'base64', // 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 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 2a4c2c1c58..d51c498af9 100644 --- a/lambdas/functions/control-plane/src/scale-runners/scale-up.ts +++ b/lambdas/functions/control-plane/src/scale-runners/scale-up.ts @@ -132,8 +132,6 @@ async function getGithubRunnerRegistrationToken(githubRunnerConfig: CreateGitHub repo: githubRunnerConfig.runnerOwner.split('/')[1], }); - const appId = parseInt(await getParameter(process.env.PARAMETER_GITHUB_APP_ID_NAME)); - logger.info('App id from SSM', { appId: appId }); return registrationToken.data.token; } diff --git a/lambdas/functions/webhook/src/ConfigLoader.test.ts b/lambdas/functions/webhook/src/ConfigLoader.test.ts index 3e15e3308a..d1a4f304b4 100644 --- a/lambdas/functions/webhook/src/ConfigLoader.test.ts +++ b/lambdas/functions/webhook/src/ConfigLoader.test.ts @@ -1,4 +1,4 @@ -import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util'; import { ConfigWebhook, ConfigWebhookEventBridge, ConfigDispatcher } from './ConfigLoader'; import { logger } from '@aws-github-runner/aws-powertools-util'; @@ -183,9 +183,15 @@ describe('ConfigLoader Tests', () => { { 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/matcher/config-1') return partialMatcher1; - if (paramPath === '/path/to/matcher/config-2') return partialMatcher2; if (paramPath === '/path/to/webhook/secret') return 'secret'; return ''; }); @@ -205,15 +211,21 @@ describe('ConfigLoader Tests', () => { 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], + ]), + ); + vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config-1') return partialMatcher1; - if (paramPath === '/path/to/matcher/config-2') return partialMatcher2; if (paramPath === '/path/to/webhook/secret') return 'secret'; return ''; }); await expect(ConfigWebhook.load()).rejects.toThrow( - "Failed to load config: Failed to parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196", // eslint-disable-line max-len + "Failed to load config: Failed to load/parse combined matcher config: Expected ',' or ']' after array element in JSON at position 196", // eslint-disable-line max-len ); }); }); @@ -291,11 +303,13 @@ describe('ConfigLoader Tests', () => { { id: '2', arn: 'arn:aws:sqs:queue2', matcherConfig: { labelMatchers: [['y']], exactMatch: true } }, ]; - vi.mocked(getParameter).mockImplementation(async (paramPath: string) => { - if (paramPath === '/path/to/matcher/config-1') return partial1; - if (paramPath === '/path/to/matcher/config-2') return partial2; - return ''; - }); + // 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], + ]), + ); const config: ConfigDispatcher = await ConfigDispatcher.load(); diff --git a/lambdas/functions/webhook/src/ConfigLoader.ts b/lambdas/functions/webhook/src/ConfigLoader.ts index 4af58022a4..e77a92b16e 100644 --- a/lambdas/functions/webhook/src/ConfigLoader.ts +++ b/lambdas/functions/webhook/src/ConfigLoader.ts @@ -1,4 +1,4 @@ -import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { getParameter, getParameters } from '@aws-github-runner/aws-ssm-util'; import { RunnerMatcherConfig } from './sqs'; import { logger } from '@aws-github-runner/aws-powertools-util'; @@ -101,16 +101,27 @@ abstract class MatcherAwareConfig extends BaseConfig { .split(':') .map((p) => p.trim()) .filter(Boolean); - let combinedString = ''; - for (const path of paths) { - await this.loadParameter(path, 'matcherConfig'); - combinedString += this.matcherConfig; - } + // Batch fetch all matcher config paths in a single SSM API call try { - this.matcherConfig = JSON.parse(combinedString); + 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); + } } catch (error) { - this.configLoadingErrors.push(`Failed to parse combined matcher config: ${(error as Error).message}`); + this.configLoadingErrors.push(`Failed to load/parse combined matcher config: ${(error as Error).message}`); } } } diff --git a/lambdas/libs/aws-ssm-util/src/index.ts b/lambdas/libs/aws-ssm-util/src/index.ts index 0b4925c17d..ee2a3943f4 100644 --- a/lambdas/libs/aws-ssm-util/src/index.ts +++ b/lambdas/libs/aws-ssm-util/src/index.ts @@ -1,4 +1,4 @@ -import { PutParameterCommand, SSMClient, Tag } from '@aws-sdk/client-ssm'; +import { GetParametersCommand, PutParameterCommand, SSMClient, Tag } from '@aws-sdk/client-ssm'; import { getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; import { SSMProvider } from '@aws-lambda-powertools/parameters/ssm'; @@ -17,6 +17,35 @@ export async function getParameter(parameter_name: string): Promise { return result; } +export async function getParameters(parameter_names: string[]): Promise> { + if (parameter_names.length === 0) { + return new Map(); + } + + const ssmClient = getTracedAWSV3Client(new SSMClient({ region: process.env.AWS_REGION })); + const result = new Map(); + + // AWS SSM GetParameters API has a limit of 10 parameters per call + const chunkSize = 10; + for (let i = 0; i < parameter_names.length; i += chunkSize) { + const chunk = parameter_names.slice(i, i + chunkSize); + const response = await ssmClient.send( + new GetParametersCommand({ + Names: chunk, + WithDecryption: true, + }), + ); + + for (const param of response.Parameters ?? []) { + if (param.Name && param.Value) { + result.set(param.Name, param.Value); + } + } + } + + return result; +} + export const SSM_ADVANCED_TIER_THRESHOLD = 4000; export async function putParameter(