diff --git a/docs/configuration.md b/docs/configuration.md index 81ae07d5f8..d207f23c70 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,6 +21,12 @@ The module uses the AWS System Manager Parameter Store to store configuration fo For the experimental multi-runner configuration, set `multi_runner_config..storage_provider.aws.ssm.ttl_seconds.tokens` to configure the token TTL. Stable configurations use `ssm_ttl_seconds.tokens` (under `runner_config` for multi-runner lanes); it is translated to the same nested setting. An omitted TTL leaves native expiration disabled. +The SSM housekeeper collects eligible names across listing pages and sends a `DeleteParameters` request whenever it has 10 names, waiting 350 ms before each batch. Any partial batch is flushed when listing finishes or fails, provided enough runtime remains. It stops listing new pages with less than twenty seconds remaining, reserving time to flush a partial batch, and stops sending delete requests with less than ten seconds remaining, including a time check after the delay. This reduces API calls while pacing each invocation below the default three delete requests per second. The quota is shared across the AWS account and Region, so concurrent housekeepers and other clients can still cause throttling. The AWS SDK retries retryable failures; if a batch still fails, the housekeeper logs it and continues with later batches. Parameters left behind remain eligible for a later scheduled run. Names returned in `InvalidParameters` are logged separately. The configured minimum age still applies, and dry-run mode sends no delete requests. + +Each batch logs `Successfully deleted expired runner configuration batch` only for names acknowledged in AWS's `DeletedParameters` response, with `deletedCount`. Sum `deletedCount` to measure confirmed deletions; counting these log entries measures successful batches. The `Runner configuration cleanup summary` log reports parameter counts: `attempted` (submitted names, excluding SDK retries), `deleted` (AWS-confirmed names), `failed` (invalid names or names in failed requests), `skipped` (ineligible entries), and `pending` (buffered names not submitted). It includes `dryRun` and a `status` of `completed`, `runtime-limit`, or `listing-failed`; completed means the scan finished, not that every deletion succeeded. Dry runs report no deletion attempts or successes. Summaries are emitted on normal completion, guarded runtime exits, and listing failures, but cannot be guaranteed after a hard Lambda timeout. + +When upgrading the housekeeper Lambda, also apply the Terraform IAM changes granting `ssm:DeleteParameters`; the singular `ssm:DeleteParameter` permission does not authorize batch deletion. Custom IAM policies must grant the batch action for the runner token path as well. + Furthermore, to accommodate larger JIT configurations or other stored values, the module implements automatic tier selection for SSM parameters: - **Parameter Tiering**: If the size of a parameter's value exceeds 4KB (specifically, 4000 bytes), the module will automatically use the 'Advanced' tier for that SSM parameter. Values smaller than this threshold will use the 'Standard' tier. 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 e8b8b11c1c..d5b2396fc3 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 @@ -1,192 +1,372 @@ -import { DeleteParameterCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; +import { DeleteParametersCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; 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, afterEach, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -process.env.AWS_REGION = 'eu-east-1'; +const { info, warn } = vi.hoisted(() => ({ info: vi.fn(), warn: vi.fn() })); +vi.mock('./logger', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + createAwsSsmStorageLogger: () => ({ info, debug: vi.fn(), warn, error: vi.fn() }), + }; +}); + +import { cleanSSMTokens } from './runner-config-housekeeper'; const mockSSMClient = mockClient(SSMClient); +const tokenPath = '/path/to/tokens/'; +const now = new Date('2026-09-23T12:00:00Z'); +const old = new Date('2026-09-21T12:00:00Z'); +const options = { dryRun: false, minimumDaysOld: 1, tokenPath }; +const staleParameters = (count: number) => + Array.from({ length: count }, (_, i) => ({ Name: `${tokenPath}i-${i}`, LastModifiedDate: old })); -const deleteAmisOlderThenDays = 1; -const now = new Date(); -const dateOld = new Date(); -dateOld.setDate(dateOld.getDate() - deleteAmisOlderThenDays - 1); +function mockPages(count: number) { + const parameters = staleParameters(count); + mockSSMClient.on(GetParametersByPathCommand).callsFake((input) => { + const offset = Number(input.NextToken ?? 0); + return { + Parameters: parameters.slice(offset, offset + 10), + NextToken: offset + 10 < count ? String(offset + 10) : undefined, + }; + }); +} -const tokenPath = '/path/to/tokens/'; +async function clean(overrides = {}, remainingTime?: () => number) { + const cleanup = cleanSSMTokens({ ...options, ...overrides }, remainingTime); + await vi.runAllTimersAsync(); + await cleanup; +} describe('clean SSM tokens / JIT config', () => { - afterEach(() => vi.unstubAllEnvs()); beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(now); + warn.mockClear(); + info.mockClear(); 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, - }); + mockSSMClient.on(GetParametersByPathCommand).resolves({ Parameters: staleParameters(1) }); + mockSSMClient.on(DeleteParametersCommand).callsFake((input) => ({ DeletedParameters: input.Names })); + }); + afterEach(() => vi.useRealTimers()); + + it.each([1, 10, 11, 25])('deletes %i stale parameters in batches of at most ten', async (count) => { + const parameters = staleParameters(count); + mockPages(count); + await clean(); + const batches = mockSSMClient.commandCalls(DeleteParametersCommand).map((call) => call.args[0].input.Names!); + expect(batches.map((batch) => batch.length)).toEqual( + Array.from({ length: Math.ceil(count / 10) }, (_, i) => Math.min(10, count - i * 10)), + ); + expect(batches.flat()).toEqual(parameters.map((parameter) => parameter.Name)); }); - it('should delete parameters older then minimumDaysOld', async () => { - await cleanSSMTokens({ + it('logs only AWS-confirmed deletions and summarizes mixed batch outcomes', async () => { + mockPages(25); + mockSSMClient + .on(DeleteParametersCommand) + .resolvesOnce({ + DeletedParameters: staleParameters(9).map((p) => p.Name), + InvalidParameters: [`${tokenPath}i-9`], + }) + .rejectsOnce(new Error('Rate exceeded')) + .callsFake((input) => ({ DeletedParameters: input.Names })); + await clean(); + const successes = info.mock.calls.filter( + ([message]) => message === 'Successfully deleted expired runner configuration batch', + ); + expect(successes).toHaveLength(2); + expect(successes.map(([, fields]) => fields)).toEqual([{ deletedCount: 9 }, { deletedCount: 5 }]); + expect(info).toHaveBeenCalledWith('Runner configuration cleanup summary', { + attempted: 25, + deleted: 14, + failed: 11, + skipped: 0, + pending: 0, dryRun: false, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, + tokenPath, + status: 'completed', }); + }); - 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 infer successful deletions from an empty AWS response', async () => { + mockSSMClient.on(DeleteParametersCommand).resolves({}); + await clean(); + expect(info).not.toHaveBeenCalledWith('Successfully deleted expired runner configuration batch', expect.anything()); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ attempted: 1, deleted: 0 }), + ); }); - it.each([undefined, []])('keeps later pages when the first page has no parameters (%s)', async (firstPage) => { - mockSSMClient.reset(); + it('combines 46 eligible names across 100 mixed parameters into four full batches and a final six', async () => { + const eligibleCounts = [3, 6, 4, 7, 2, 5, 3, 6, 4, 6]; + const parameters = eligibleCounts.flatMap((count, page) => + Array.from({ length: 10 }, (_, i) => ({ + Name: `${tokenPath}i-${page * 10 + i}`, + LastModifiedDate: (i * 3) % 10 < count ? old : now, + })), + ); + const eligibleNames = parameters.filter((parameter) => parameter.LastModifiedDate === old).map((p) => p.Name); + mockSSMClient.on(GetParametersByPathCommand).callsFake((input) => { + const offset = Number(input.NextToken ?? 0); + // Full batches must be deleted before listing continues. + const seenEligible = eligibleCounts.slice(0, offset / 10).reduce((sum, count) => sum + count, 0); + expect(mockSSMClient.commandCalls(DeleteParametersCommand)).toHaveLength(Math.floor(seenEligible / 10)); + return { + Parameters: parameters.slice(offset, offset + 10), + NextToken: offset + 10 < parameters.length ? String(offset + 10) : undefined, + }; + }); + await clean(); + const batches = mockSSMClient.commandCalls(DeleteParametersCommand).map((call) => call.args[0].input.Names!); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 10); + expect(batches.map((batch) => batch.length)).toEqual([10, 10, 10, 10, 6]); + expect(batches.flat()).toEqual(eligibleNames); + }); + + it('flushes a partial batch when an empty final page has no next token', async () => { mockSSMClient .on(GetParametersByPathCommand) - .resolvesOnce({ Parameters: firstPage, NextToken: 'empty-page' }) - .resolvesOnce({ NextToken: 'last-page' }) - .resolvesOnce({ Parameters: [{ Name: tokenPath + 'i-old-later', LastModifiedDate: dateOld }] }); + .resolvesOnce({ Parameters: staleParameters(6), NextToken: 'last' }) + .callsFake(() => { + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + return {}; + }); + await clean(); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 2); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { + Names: staleParameters(6).map((parameter) => parameter.Name), + }); + }); - await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); + it('filters young, boundary-age and incomplete parameters across listing pages', async () => { + mockSSMClient + .on(GetParametersByPathCommand) + .resolvesOnce({ + Parameters: [ + ...staleParameters(1), + { Name: 'young', LastModifiedDate: now }, + { Name: 'boundary', LastModifiedDate: new Date('2026-09-22T12:00:00Z') }, + { Name: 'missing-date' }, + { LastModifiedDate: old }, + ], + NextToken: 'next', + }) + .resolvesOnce({ Parameters: [{ Name: 'old-on-second-page', LastModifiedDate: old }] }); + await clean(); + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { + Names: [`${tokenPath}i-0`, 'old-on-second-page'], + }); + }); - expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 3); - expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { - Path: tokenPath, - NextToken: 'last-page', + it('does not delete in dry-run mode', async () => { + await clean({ dryRun: true }); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + expect(vi.getTimerCount()).toBe(0); + expect(info).not.toHaveBeenCalledWith('Successfully deleted expired runner configuration batch', expect.anything()); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ attempted: 0, deleted: 0, failed: 0, dryRun: true }), + ); + }); + + it.each([undefined, [], [{ Name: 'young', LastModifiedDate: now }]])( + 'does not send an empty delete request (%j)', + async (Parameters) => { + mockSSMClient.on(GetParametersByPathCommand).resolves({ Parameters }); + await clean(); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + }, + ); + + it('paces successive batch requests', async () => { + mockPages(11); + const cleanup = cleanSSMTokens(options); + await vi.advanceTimersByTimeAsync(349); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + await vi.advanceTimersByTimeAsync(1); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 1); + await vi.advanceTimersByTimeAsync(349); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 1); + await vi.advanceTimersByTimeAsync(1); + await cleanup; + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 2); + }); + + it('logs an exhausted batch failure and continues to later batches', async () => { + mockPages(11); + mockSSMClient.on(DeleteParametersCommand).rejectsOnce(new Error('Rate exceeded')).resolves({}); + await clean(); + expect(warn).toHaveBeenCalledWith('Failed to delete expired runner configuration batch', { + parameterNames: staleParameters(10).map((parameter) => parameter.Name), + errorNames: ['Error'], }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-later' }); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-10`] }); }); - it('keeps deletions from earlier pages when a later listing page fails', async () => { + it('reports invalid names in a successful response and continues cleanup', async () => { + mockPages(11); mockSSMClient - .on(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }) - .rejects(new Error('SSM unavailable')); + .on(DeleteParametersCommand) + .resolvesOnce({ + DeletedParameters: staleParameters(9).map((parameter) => parameter.Name), + InvalidParameters: [`${tokenPath}i-9`], + }) + .resolves({}); + await clean(); + expect(warn).toHaveBeenCalledWith('Runner configurations were not deleted', { + parameterNames: [`${tokenPath}i-9`], + }); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 2); + }); - await expect(cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath })).rejects.toThrow('SSM unavailable'); + it.each([undefined, []])('follows tokens through empty pages (%j)', async (Parameters) => { + mockSSMClient + .on(GetParametersByPathCommand) + .resolvesOnce({ Parameters, NextToken: 'empty' }) + .resolvesOnce({ NextToken: 'last' }) + .resolvesOnce({ Parameters: staleParameters(1) }); + await clean(); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 3); + expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { NextToken: 'last' }); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-0`] }); + }); + + it('flushes buffered names before propagating a later listing failure', async () => { + mockSSMClient + .on(GetParametersByPathCommand) + .resolvesOnce({ Parameters: staleParameters(1), NextToken: 'next' }) + .callsFake(() => { + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + throw new Error('Later listing failed'); + }); + const result = expect(cleanSSMTokens(options)).rejects.toThrow('Later listing failed'); + await vi.runAllTimersAsync(); + await result; + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-0`] }); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ status: 'listing-failed', attempted: 1, deleted: 1 }), + ); + }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); + it('does not start listing with less than twenty seconds remaining', async () => { + await clean({}, () => 19999); + expect(mockSSMClient.calls()).toHaveLength(0); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ status: 'runtime-limit', attempted: 0, deleted: 0 }), + ); }); - it('starts fresh against remaining parameters after an interrupted invocation', async () => { + it('does not delete when listing consumes the remaining time', 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 {}; + mockSSMClient.on(GetParametersByPathCommand).callsFake(() => { + remaining = 9999; + return { Parameters: staleParameters(1), NextToken: 'next' }; }); - 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); + await clean({}, () => remaining); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + }); + + it('leaves a buffered partial batch for the next run when one listing call exhausts the flush reserve', async () => { + let remaining = 60000; + mockSSMClient + .on(GetParametersByPathCommand) + .resolvesOnce({ Parameters: staleParameters(6), NextToken: 'last' }) + .callsFake(() => { + remaining = 9999; + return {}; + }); + await clean({}, () => remaining); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 2); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 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 }, - ], + it('checks the remaining time again after the pacing delay', async () => { + let deadline = Infinity; + mockSSMClient.on(GetParametersByPathCommand).callsFake(() => { + deadline = Date.now() + 10300; + return { Parameters: staleParameters(1) }; }); - mockSSMClient.on(DeleteParameterCommand, { Name: tokenPath + 'failed' }).rejects(new Error('Denied')); - await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'healthy' }); + await clean({}, () => deadline - Date.now()); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); }); - 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 {}; + it('flushes a partial batch before slow listing exhausts runtime on repeated scans', async () => { + // 20 pages: six stale names on the first page, young parameters on the rest. + let inventory = [ + ...staleParameters(6), + ...Array.from({ length: 194 }, (_, i) => ({ Name: `${tokenPath}young-${i}`, LastModifiedDate: now })), + ]; + mockSSMClient.on(GetParametersByPathCommand).callsFake((input) => { + vi.setSystemTime(Date.now() + 5000); // each listing call takes five seconds + const offset = Number(input.NextToken ?? 0); + return { + Parameters: inventory.slice(offset, offset + 10), + NextToken: offset + 10 < inventory.length ? String(offset + 10) : undefined, + }; + }); + mockSSMClient.on(DeleteParametersCommand).callsFake((input) => { + inventory = inventory.filter((parameter) => !input.Names.includes(parameter.Name)); + return { DeletedParameters: input.Names }; + }); + + for (let scan = 0; scan < 3; scan++) { + const deadline = Date.now() + 60000; + await clean({}, () => deadline - Date.now()); + expect(inventory.filter((parameter) => parameter.LastModifiedDate === old)).toEqual([]); + } + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { + Names: staleParameters(6).map((parameter) => parameter.Name), }); - await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ status: 'runtime-limit', attempted: 6, deleted: 6, pending: 0 }), + ); }); - it('should not delete when dry run is activated', async () => { - await cleanSSMTokens({ - dryRun: true, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, + it('starts a fresh scan of remaining parameters after stopping between pages', async () => { + let remaining = 60000; + let inventory = staleParameters(11); + mockSSMClient.on(GetParametersByPathCommand).callsFake(() => ({ + Parameters: inventory.slice(0, 10), + NextToken: inventory.length > 10 ? 'next' : undefined, + })); + mockSSMClient.on(DeleteParametersCommand).callsFake((input) => { + inventory = inventory.filter((parameter) => !input.Names.includes(parameter.Name)); + remaining = 0; + return {}; }); + await clean({}, () => remaining); + expect(inventory.map((parameter) => parameter.Name)).toEqual([`${tokenPath}i-10`]); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 1); + mockSSMClient.resetHistory(); + await clean(); + expect(mockSSMClient.commandCalls(GetParametersByPathCommand)[0].args[0].input.NextToken).toBeUndefined(); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-10`] }); + expect(inventory).toHaveLength(0); + }); - 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); + it('propagates listing failures without deleting', async () => { + mockSSMClient.on(GetParametersByPathCommand).rejects(new Error('Listing failed')); + await expect(cleanSSMTokens(options)).rejects.toThrow('Listing failed'); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); }); + + it.each([{ minimumDaysOld: 0 }, { minimumDaysOld: undefined }, { tokenPath: undefined }])( + 'rejects invalid cleanup options (%j)', + async (invalid) => { + await expect(cleanSSMTokens({ ...options, ...invalid } as typeof options)).rejects.toThrow(); + expect(mockSSMClient.calls()).toHaveLength(0); + }, + ); }); 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 b548384b30..ec17087c7c 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts @@ -1,10 +1,17 @@ -import { DeleteParameterCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; +import { DeleteParametersCommand, GetParametersByPathCommand, SSMClient } from '@aws-sdk/client-ssm'; import { getTracedAWSV3Client } from '@aws-github-runner/aws-powertools-util'; import type { RunnerConfigHousekeeper } from '../../core'; import { createAwsSsmStorageLogger, getErrorNames } from './logger'; const logger = createAwsSsmStorageLogger('runner-config-housekeeper'); +const DELETE_BATCH_SIZE = 10; +// Pacing is per invocation; other housekeepers share the account/Region quota. +const DELETE_BATCH_DELAY_MS = 350; +// No delete request starts with less than this remaining. +const DELETE_RUNTIME_GUARD_MS = 10000; +// Listing stops earlier, reserving time to flush a buffered partial batch. +const LISTING_RUNTIME_GUARD_MS = 20000; export interface SSMCleanupOptions { dryRun: boolean; @@ -28,29 +35,74 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = let nextToken: string | undefined; const minimumDate = new Date(); minimumDate.setDate(minimumDate.getDate() - options.minimumDaysOld); - 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), + const pendingNames: string[] = []; + const summary = { attempted: 0, deleted: 0, failed: 0, skipped: 0 }; + let status = 'runtime-limit'; + + async function flushPendingNames(): Promise { + if (!pendingNames.length) return true; + if (remainingTime() < DELETE_RUNTIME_GUARD_MS) return false; + await new Promise((resolve) => setTimeout(resolve, DELETE_BATCH_DELAY_MS)); + if (remainingTime() < DELETE_RUNTIME_GUARD_MS) return false; + const names = pendingNames.splice(0, DELETE_BATCH_SIZE); + summary.attempted += names.length; + try { + // SDK retries handle retryable failures; exhausted batches remain for the next sweep. + const result = await client.send(new DeleteParametersCommand({ Names: names })); + if (result.DeletedParameters?.length) { + summary.deleted += result.DeletedParameters.length; + logger.info('Successfully deleted expired runner configuration batch', { + deletedCount: result.DeletedParameters.length, }); } + if (result.InvalidParameters?.length) { + summary.failed += result.InvalidParameters.length; + logger.warn('Runner configurations were not deleted', { parameterNames: result.InvalidParameters }); + } + } catch (error) { + summary.failed += names.length; + logger.warn('Failed to delete expired runner configuration batch', { + parameterNames: names, + errorNames: getErrorNames(error), + }); } - nextToken = page.NextToken; - } while (nextToken); + return true; + } + + try { + do { + if (remainingTime() < LISTING_RUNTIME_GUARD_MS) return; + const page = await client.send(new GetParametersByPathCommand({ Path: options.tokenPath, NextToken: nextToken })); + for (const parameter of page.Parameters ?? []) { + if (remainingTime() < DELETE_RUNTIME_GUARD_MS) return; + if (!parameter.Name || !parameter.LastModifiedDate || !(new Date(parameter.LastModifiedDate) < minimumDate)) { + summary.skipped++; + continue; + } + logger.info('Deleting expired runner configuration', { parameterName: parameter.Name, dryRun: options.dryRun }); + if (!options.dryRun) { + pendingNames.push(parameter.Name); + if (pendingNames.length === DELETE_BATCH_SIZE && !(await flushPendingNames())) return; + } + } + nextToken = page.NextToken; + } while (nextToken); + status = 'completed'; + } catch (error) { + status = 'listing-failed'; + throw error; + } finally { + // Flush a partial batch at the end or on a listing error, if runtime permits. + // Otherwise the remaining parameters will be rediscovered by the next invocation. + if (!(await flushPendingNames()) && status === 'completed') status = 'runtime-limit'; + logger.info('Runner configuration cleanup summary', { + ...summary, + pending: pendingNames.length, + dryRun: options.dryRun, + tokenPath: options.tokenPath, + status, + }); + } } class AwsSsmRunnerConfigHousekeeper implements RunnerConfigHousekeeper { diff --git a/modules/runner-config/ssm-housekeeper/README.md b/modules/runner-config/ssm-housekeeper/README.md index a878605eda..642e916aa1 100644 --- a/modules/runner-config/ssm-housekeeper/README.md +++ b/modules/runner-config/ssm-housekeeper/README.md @@ -1,6 +1,6 @@ # 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. +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 listing with twenty seconds remaining, reserving time to flush buffered names, and stops deleting 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. @@ -8,6 +8,12 @@ This provider-neutral child module owns the Lambda function, EventBridge schedul The module is an implementation detail of the experimental runner configuration. It is composed by `runner-config` and is not intended to be called directly. +Cleanup collects names older than the configured minimum age across listing pages, deleting batches of 10 with a 350 ms delay before each batch. Any partial batch is flushed when listing finishes or fails, provided enough runtime remains. It checks the remaining runtime before and after the delay. Dry-run mode only reports candidates. SDK retries handle retryable failures; exhausted batch failures and invalid parameter names are logged, and cleanup continues with later batches. Remaining parameters can be attempted on a later scheduled run. Pacing is per invocation, while AWS delete quotas are shared across the account and Region. + +Each batch logs `Successfully deleted expired runner configuration batch` only for names acknowledged in AWS's `DeletedParameters` response, with `deletedCount`. Sum `deletedCount` to measure confirmed deletions; counting these log entries measures successful batches. The `Runner configuration cleanup summary` log reports parameter counts: `attempted` (submitted names, excluding SDK retries), `deleted` (AWS-confirmed names), `failed` (invalid names or names in failed requests), `skipped` (ineligible entries), and `pending` (buffered names not submitted). It includes `dryRun` and a `status` of `completed`, `runtime-limit`, or `listing-failed`; completed means the scan finished, not that every deletion succeeded. Dry runs report no deletion attempts or successes. Summaries are emitted on normal completion, guarded runtime exits, and listing failures, but cannot be guaranteed after a hard Lambda timeout. + +Deploy the Lambda update together with the Terraform IAM policy update: batch deletion requires `ssm:DeleteParameters` on the configured token path. + ## Requirements diff --git a/modules/runner-config/ssm-housekeeper/iam-policies.tf b/modules/runner-config/ssm-housekeeper/iam-policies.tf index 8d3bab2865..1c21e7680e 100644 --- a/modules/runner-config/ssm-housekeeper/iam-policies.tf +++ b/modules/runner-config/ssm-housekeeper/iam-policies.tf @@ -39,7 +39,7 @@ data "aws_iam_policy_document" "ssm_housekeeper" { statement { effect = "Allow" actions = [ - "ssm:DeleteParameter", + "ssm:DeleteParameters", "ssm:GetParametersByPath", ] resources = [var.config.cleanup.parameter_path_arn] diff --git a/modules/runners/policies/lambda-ssm-housekeeper.json b/modules/runners/policies/lambda-ssm-housekeeper.json index 5e49baafaa..66ff4024df 100644 --- a/modules/runners/policies/lambda-ssm-housekeeper.json +++ b/modules/runners/policies/lambda-ssm-housekeeper.json @@ -4,7 +4,7 @@ { "Effect": "Allow", "Action": [ - "ssm:DeleteParameter", + "ssm:DeleteParameters", "ssm:GetParametersByPath" ], "Resource": "${ssm_token_path}*"