diff --git a/lambdas/functions/control-plane/src/lambda.ts b/lambdas/functions/control-plane/src/lambda.ts index 4594a1289e..ea4ddc56fc 100644 --- a/lambdas/functions/control-plane/src/lambda.ts +++ b/lambdas/functions/control-plane/src/lambda.ts @@ -124,7 +124,7 @@ export async function runnerConfigHousekeeper(event: unknown, context: Context): const housekeeper = createRunnerConfigHousekeeper(); try { - await housekeeper.houseKeeper(); + await housekeeper.houseKeeper(() => context.getRemainingTimeInMillis()); } catch (e) { logger.error(`${(e as Error).message}`, { error: e as Error }); } 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 index 1c16607333..e8b8b11c1c 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.test.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.test.ts @@ -2,7 +2,7 @@ import { DeleteParameterCommand, GetParametersByPathCommand, SSMClient } from '@ import { mockClient } from 'aws-sdk-client-mock'; import 'aws-sdk-client-mock-jest/vitest'; import { cleanSSMTokens } from './runner-config-housekeeper'; -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; process.env.AWS_REGION = 'eu-east-1'; @@ -16,6 +16,7 @@ dateOld.setDate(dateOld.getDate() - deleteAmisOlderThenDays - 1); const tokenPath = '/path/to/tokens/'; describe('clean SSM tokens / JIT config', () => { + afterEach(() => vi.unstubAllEnvs()); beforeEach(() => { mockSSMClient.reset(); mockSSMClient.on(GetParametersByPathCommand).resolves({ @@ -53,6 +54,79 @@ describe('clean SSM tokens / JIT config', () => { expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-new-01' }); }); + it.each([undefined, []])('keeps later pages when the first page has no parameters (%s)', async (firstPage) => { + mockSSMClient.reset(); + mockSSMClient + .on(GetParametersByPathCommand) + .resolvesOnce({ Parameters: firstPage, NextToken: 'empty-page' }) + .resolvesOnce({ NextToken: 'last-page' }) + .resolvesOnce({ Parameters: [{ Name: tokenPath + 'i-old-later', LastModifiedDate: dateOld }] }); + + await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); + + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 3); + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { + Path: tokenPath, + NextToken: 'last-page', + }); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-later' }); + }); + + it('keeps deletions from earlier pages when a later listing page fails', async () => { + mockSSMClient + .on(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }) + .rejects(new Error('SSM unavailable')); + + await expect(cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath })).rejects.toThrow('SSM unavailable'); + + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); + }); + + it('starts fresh against remaining parameters after an interrupted invocation', async () => { + let remaining = 60000; + const inventory = [ + { Name: tokenPath + 'first', LastModifiedDate: dateOld }, + { Name: tokenPath + 'second', LastModifiedDate: dateOld }, + ]; + mockSSMClient.reset(); + mockSSMClient.on(GetParametersByPathCommand).callsFake(() => ({ Parameters: [...inventory] })); + mockSSMClient.on(DeleteParameterCommand).callsFake((input) => { + inventory.splice( + inventory.findIndex((item) => item.Name === input.Name), + 1, + ); + remaining = 0; + return {}; + }); + await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }, () => remaining); + expect(inventory).toHaveLength(1); + mockSSMClient.resetHistory(); + await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); + expect(mockSSMClient.commandCalls(GetParametersByPathCommand)[0].args[0].input.NextToken).toBeUndefined(); + expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'first' }); + expect(inventory).toHaveLength(0); + }); + + it('continues past a failed deletion within the same invocation', async () => { + mockSSMClient.on(GetParametersByPathCommand, { Path: tokenPath }).resolves({ + Parameters: [ + { Name: tokenPath + 'failed', LastModifiedDate: dateOld }, + { Name: tokenPath + 'healthy', LastModifiedDate: dateOld }, + ], + }); + mockSSMClient.on(DeleteParameterCommand, { Name: tokenPath + 'failed' }).rejects(new Error('Denied')); + await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'healthy' }); + }); + + it('deletes a page before requesting the next page', async () => { + mockSSMClient.on(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }).callsFake(() => { + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); + return {}; + }); + await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); + }); + it('should not delete when dry run is activated', async () => { await cleanSSMTokens({ dryRun: true, diff --git a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts index 8bd657b22a..b548384b30 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts @@ -1,9 +1,4 @@ -import { - DeleteParameterCommand, - GetParametersByPathCommand, - SSMClient, - type GetParametersByPathCommandOutput, -} from '@aws-sdk/client-ssm'; +import { DeleteParameterCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; import { getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; import type { RunnerConfigHousekeeper } from '../../core'; @@ -21,7 +16,7 @@ export function createAwsSsmRunnerConfigHousekeeper(options?: SSMCleanupOptions) return new AwsSsmRunnerConfigHousekeeper(options ?? loadCleanupOptions()); } -export async function cleanSSMTokens(options: SSMCleanupOptions): Promise { +export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = () => Infinity): Promise { validateOptions(options); logger.info('Cleaning expired runner configurations', { minimumDaysOld: options.minimumDaysOld, @@ -30,63 +25,39 @@ export async function cleanSSMTokens(options: SSMCleanupOptions): Promise }); const client = getTracedAWSV3Client(new SSMClient({ region: process.env.AWS_REGION })); - let parameters: GetParametersByPathCommandOutput; - try { - parameters = await client.send(new GetParametersByPathCommand({ Path: options.tokenPath })); - while (parameters.NextToken) { - const nextParameters = await client.send( - new GetParametersByPathCommand({ Path: options.tokenPath, NextToken: parameters.NextToken }), - ); - parameters.Parameters?.push(...(nextParameters.Parameters ?? [])); - parameters.NextToken = nextParameters.NextToken; - } - } catch (error) { - logger.error('Failed to list runner configurations', { - tokenPath: options.tokenPath, - errorNames: getErrorNames(error), - }); - throw error; - } - logger.info('Found runner configurations', { - tokenPath: options.tokenPath, - parameterCount: parameters.Parameters?.length ?? 0, - }); - + let nextToken: string | undefined; const minimumDate = new Date(); minimumDate.setDate(minimumDate.getDate() - options.minimumDaysOld); - - for (const parameter of parameters.Parameters ?? []) { - if (parameter.LastModifiedDate && new Date(parameter.LastModifiedDate) < minimumDate) { - logger.info('Deleting expired runner configuration', { - parameterName: parameter.Name, - lastModifiedDate: parameter.LastModifiedDate, - dryRun: options.dryRun, - }); + do { + if (remainingTime() < 10000) return; + const page = await client.send(new GetParametersByPathCommand({ Path: options.tokenPath, NextToken: nextToken })); + for (const parameter of page.Parameters ?? []) { + if (remainingTime() < 10000) return; + if (!parameter.Name || !parameter.LastModifiedDate || !(new Date(parameter.LastModifiedDate) < minimumDate)) + continue; + logger.info('Deleting expired runner configuration', { parameterName: parameter.Name, dryRun: options.dryRun }); try { if (!options.dryRun) { await new Promise((resolve) => setTimeout(resolve, 50)); await client.send(new DeleteParameterCommand({ Name: parameter.Name })); } } catch (error) { + // Failed items remain in the inventory for the next complete sweep. logger.warn('Failed to delete expired runner configuration', { parameterName: parameter.Name, errorNames: getErrorNames(error), }); } - } else { - logger.debug('Skipping runner configuration that is not expired', { - parameterName: parameter.Name, - lastModifiedDate: parameter.LastModifiedDate, - }); } - } + nextToken = page.NextToken; + } while (nextToken); } class AwsSsmRunnerConfigHousekeeper implements RunnerConfigHousekeeper { constructor(private readonly options: SSMCleanupOptions) {} - houseKeeper(): Promise { - return cleanSSMTokens(this.options); + houseKeeper(remainingTime?: () => number): Promise { + return cleanSSMTokens(this.options, remainingTime); } } diff --git a/lambdas/libs/storage-providers/core/index.ts b/lambdas/libs/storage-providers/core/index.ts index e6060eae2a..86950eb2c7 100644 --- a/lambdas/libs/storage-providers/core/index.ts +++ b/lambdas/libs/storage-providers/core/index.ts @@ -17,7 +17,7 @@ export interface RunnerConfigStore { } export interface RunnerConfigHousekeeper { - houseKeeper(): Promise; + houseKeeper(remainingTime?: () => number): Promise; } export interface GitHubAppCredential { diff --git a/modules/runner-config/ssm-housekeeper/README.md b/modules/runner-config/ssm-housekeeper/README.md index b8899e3f43..a878605eda 100644 --- a/modules/runner-config/ssm-housekeeper/README.md +++ b/modules/runner-config/ssm-housekeeper/README.md @@ -1,5 +1,7 @@ # SSM housekeeper module +Cleanup is stateless: each invocation lists current parameters and deletes eligible items page by page. It starts deleting before listing the next page, including after empty pages. Individual deletion failures do not block other items, and a later listing failure leaves earlier deletions completed. A deadline guard stops new work with ten seconds remaining. The next scheduled invocation starts a fresh scan; deleted parameters are no longer listed. No scan cursor or completed-item list is stored. Age and dry-run protections remain in place. + > This module is treated as an internal module; breaking changes do not trigger a major release bump. This provider-neutral child module owns the Lambda function, EventBridge schedule, IAM policies, and CloudWatch log group used to remove expired runner registration parameters from Parameter Store.