From de01c259e8eb385f98f535d2586284608d8d7a9a Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Wed, 23 Sep 2026 16:59:11 +0200 Subject: [PATCH 1/4] fix(ssm): batch housekeeper parameter deletions --- docs/configuration.md | 4 + .../aws/ssm/runner-config-housekeeper.test.ts | 342 ++++++++++-------- .../aws/ssm/runner-config-housekeeper.ts | 28 +- .../runner-config/ssm-housekeeper/README.md | 4 + .../ssm-housekeeper/iam-policies.tf | 2 +- .../policies/lambda-ssm-housekeeper.json | 2 +- 6 files changed, 218 insertions(+), 164 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 81ae07d5f8..b1b838d109 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,6 +21,10 @@ 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 requests pages of up to 10 parameters and deletes eligible names from each page in one `DeleteParameters` request before fetching the next page, waiting 350 ms before each batch. It stops starting new work 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. + +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..f92c817624 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,226 @@ -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'; + +const { warn } = vi.hoisted(() => ({ warn: vi.fn() })); +vi.mock('./logger', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + createAwsSsmStorageLogger: () => ({ info: vi.fn(), debug: vi.fn(), warn, error: vi.fn() }), + }; +}); -process.env.AWS_REGION = 'eu-east-1'; +import { cleanSSMTokens } from './runner-config-housekeeper'; const mockSSMClient = mockClient(SSMClient); - -const deleteAmisOlderThenDays = 1; -const now = new Date(); -const dateOld = new Date(); -dateOld.setDate(dateOld.getDate() - deleteAmisOlderThenDays - 1); - 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 })); + +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, + }; + }); +} + +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(); 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).resolves({}); }); - - it('should delete parameters older then minimumDaysOld', async () => { - await cleanSSMTokens({ - dryRun: false, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, - }); - - expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-new-01' }); + 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)); + for (const call of mockSSMClient.commandCalls(GetParametersByPathCommand)) { + expect(call.args[0].input.MaxResults).toBe(10); + } }); - it.each([undefined, []])('keeps later pages when the first page has no parameters (%s)', async (firstPage) => { - mockSSMClient.reset(); + it('filters young, boundary-age and incomplete parameters across listing pages', async () => { 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' }); + .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, 2); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-0`] }); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: ['old-on-second-page'] }); }); - 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'); + it('does not delete in dry-run mode', async () => { + await clean({ dryRun: true }); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + expect(vi.getTimerCount()).toBe(0); + }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); + 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('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 {}; + 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'], }); - 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); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-10`] }); }); - 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('reports invalid names in a successful response and continues cleanup', async () => { + mockPages(11); + mockSSMClient + .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`], }); - mockSSMClient.on(DeleteParameterCommand, { Name: tokenPath + 'failed' }).rejects(new Error('Denied')); - await cleanSSMTokens({ dryRun: false, minimumDaysOld: 1, tokenPath }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'healthy' }); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 2); }); - 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.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('should not delete when dry run is activated', async () => { - await cleanSSMTokens({ - dryRun: true, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, - }); + it('deletes the current page before fetching the next page, even if that listing fails', async () => { + mockSSMClient + .on(GetParametersByPathCommand) + .resolvesOnce({ Parameters: staleParameters(1), NextToken: 'next' }) + .callsFake(() => { + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-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(GetParametersByPathCommand, { Path: tokenPath }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-old-01' }); - expect(mockSSMClient).not.toHaveReceivedCommandWith(DeleteParameterCommand, { Name: tokenPath + 'i-new-01' }); + it('does not start listing with less than ten seconds remaining', async () => { + await clean({}, () => 9999); + expect(mockSSMClient.calls()).toHaveLength(0); }); - 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('does not delete when listing consumes the remaining time', async () => { + let remaining = 60000; + mockSSMClient.on(GetParametersByPathCommand).callsFake(() => { + remaining = 9999; + return { Parameters: staleParameters(1), NextToken: 'next' }; + }); + await clean({}, () => remaining); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); }); - it('should not error on delete failure.', async () => { - mockSSMClient.on(DeleteParameterCommand).rejects(new Error('ParameterNotFound')); + it('checks the remaining time again after the pacing delay', async () => { + const deadline = now.getTime() + 10300; + await clean({}, () => deadline - Date.now()); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); + }); - await expect( - cleanSSMTokens({ - dryRun: false, - minimumDaysOld: deleteAmisOlderThenDays, - tokenPath: tokenPath, - }), - ).resolves.not.toThrow(); + 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); }); - 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..4625909ab7 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,13 @@ -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; export interface SSMCleanupOptions { dryRun: boolean; @@ -30,21 +33,30 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = minimumDate.setDate(minimumDate.getDate() - options.minimumDaysOld); do { if (remainingTime() < 10000) return; - const page = await client.send(new GetParametersByPathCommand({ Path: options.tokenPath, NextToken: nextToken })); + const page = await client.send( + new GetParametersByPathCommand({ Path: options.tokenPath, NextToken: nextToken, MaxResults: DELETE_BATCH_SIZE }), + ); + const names: string[] = []; 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 }); + names.push(parameter.Name); + } + if (!options.dryRun && names.length) { + if (remainingTime() < 10000) return; + await new Promise((resolve) => setTimeout(resolve, DELETE_BATCH_DELAY_MS)); + if (remainingTime() < 10000) return; try { - if (!options.dryRun) { - await new Promise((resolve) => setTimeout(resolve, 50)); - await client.send(new DeleteParameterCommand({ Name: parameter.Name })); + // SDK retries handle retryable failures; exhausted batches remain for the next sweep. + const result = await client.send(new DeleteParametersCommand({ Names: names })); + if (result.InvalidParameters?.length) { + logger.warn('Runner configurations were not deleted', { parameterNames: result.InvalidParameters }); } } catch (error) { - // Failed items remain in the inventory for the next complete sweep. - logger.warn('Failed to delete expired runner configuration', { - parameterName: parameter.Name, + logger.warn('Failed to delete expired runner configuration batch', { + parameterNames: names, errorNames: getErrorNames(error), }); } diff --git a/modules/runner-config/ssm-housekeeper/README.md b/modules/runner-config/ssm-housekeeper/README.md index a878605eda..3b17d5b15e 100644 --- a/modules/runner-config/ssm-housekeeper/README.md +++ b/modules/runner-config/ssm-housekeeper/README.md @@ -8,6 +8,10 @@ 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 requests pages of up to 10 parameters and batch-deletes names older than the configured minimum age before fetching the next page, with a 350 ms delay before each batch. 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. + +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}*" From 9db3803176c089f693ebd34d52d5e61b39f5e9dd Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Fri, 25 Sep 2026 12:30:52 +0200 Subject: [PATCH 2/4] fix(ssm): combine cleanup batches across listing pages --- docs/configuration.md | 2 +- .../aws/ssm/runner-config-housekeeper.test.ts | 71 ++++++++++++++++--- .../aws/ssm/runner-config-housekeeper.ts | 68 ++++++++++-------- .../runner-config/ssm-housekeeper/README.md | 2 +- 4 files changed, 105 insertions(+), 38 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index b1b838d109..bb722ebe00 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,7 +21,7 @@ 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 requests pages of up to 10 parameters and deletes eligible names from each page in one `DeleteParameters` request before fetching the next page, waiting 350 ms before each batch. It stops starting new work 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. +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 starting new work 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. 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. 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 f92c817624..fcb7fd4b92 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 @@ -59,9 +59,48 @@ describe('clean SSM tokens / JIT config', () => { Array.from({ length: Math.ceil(count / 10) }, (_, i) => Math.min(10, count - i * 10)), ); expect(batches.flat()).toEqual(parameters.map((parameter) => parameter.Name)); - for (const call of mockSSMClient.commandCalls(GetParametersByPathCommand)) { - expect(call.args[0].input.MaxResults).toBe(10); - } + }); + + 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: 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), + }); }); it('filters young, boundary-age and incomplete parameters across listing pages', async () => { @@ -80,9 +119,10 @@ describe('clean SSM tokens / JIT config', () => { .resolvesOnce({ Parameters: [{ Name: 'old-on-second-page', LastModifiedDate: old }] }); await clean(); expect(mockSSMClient).toHaveReceivedCommandWith(GetParametersByPathCommand, { Path: tokenPath, NextToken: 'next' }); - expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 2); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-0`] }); - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: ['old-on-second-page'] }); + expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 1); + expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { + Names: [`${tokenPath}i-0`, 'old-on-second-page'], + }); }); it('does not delete in dry-run mode', async () => { @@ -153,17 +193,18 @@ describe('clean SSM tokens / JIT config', () => { expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-0`] }); }); - it('deletes the current page before fetching the next page, even if that listing fails', async () => { + it('flushes buffered names before propagating a later listing failure', async () => { mockSSMClient .on(GetParametersByPathCommand) .resolvesOnce({ Parameters: staleParameters(1), NextToken: 'next' }) .callsFake(() => { - expect(mockSSMClient).toHaveReceivedCommandWith(DeleteParametersCommand, { Names: [`${tokenPath}i-0`] }); + 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`] }); }); it('does not start listing with less than ten seconds remaining', async () => { @@ -182,6 +223,20 @@ describe('clean SSM tokens / JIT config', () => { expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); }); + it('leaves a buffered partial batch for the next run when a later page exhausts runtime', 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('checks the remaining time again after the pacing delay', async () => { const deadline = now.getTime() + 10300; await clean({}, () => deadline - Date.now()); 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 4625909ab7..c07d8456e5 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts @@ -31,38 +31,50 @@ 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, MaxResults: DELETE_BATCH_SIZE }), - ); - const names: string[] = []; - 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 }); - names.push(parameter.Name); + const pendingNames: string[] = []; + + async function flushPendingNames(): Promise { + if (!pendingNames.length) return true; + if (remainingTime() < 10000) return false; + await new Promise((resolve) => setTimeout(resolve, DELETE_BATCH_DELAY_MS)); + if (remainingTime() < 10000) return false; + const names = pendingNames.splice(0, DELETE_BATCH_SIZE); + try { + // SDK retries handle retryable failures; exhausted batches remain for the next sweep. + const result = await client.send(new DeleteParametersCommand({ Names: names })); + if (result.InvalidParameters?.length) { + logger.warn('Runner configurations were not deleted', { parameterNames: result.InvalidParameters }); + } + } catch (error) { + logger.warn('Failed to delete expired runner configuration batch', { + parameterNames: names, + errorNames: getErrorNames(error), + }); } - if (!options.dryRun && names.length) { - if (remainingTime() < 10000) return; - await new Promise((resolve) => setTimeout(resolve, DELETE_BATCH_DELAY_MS)); + return true; + } + + try { + do { if (remainingTime() < 10000) return; - try { - // SDK retries handle retryable failures; exhausted batches remain for the next sweep. - const result = await client.send(new DeleteParametersCommand({ Names: names })); - if (result.InvalidParameters?.length) { - logger.warn('Runner configurations were not deleted', { parameterNames: result.InvalidParameters }); + 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 }); + if (!options.dryRun) { + pendingNames.push(parameter.Name); + if (pendingNames.length === DELETE_BATCH_SIZE && !(await flushPendingNames())) return; } - } catch (error) { - logger.warn('Failed to delete expired runner configuration batch', { - parameterNames: names, - errorNames: getErrorNames(error), - }); } - } - nextToken = page.NextToken; - } while (nextToken); + nextToken = page.NextToken; + } while (nextToken); + } 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. + await flushPendingNames(); + } } class AwsSsmRunnerConfigHousekeeper implements RunnerConfigHousekeeper { diff --git a/modules/runner-config/ssm-housekeeper/README.md b/modules/runner-config/ssm-housekeeper/README.md index 3b17d5b15e..0ff31bebf5 100644 --- a/modules/runner-config/ssm-housekeeper/README.md +++ b/modules/runner-config/ssm-housekeeper/README.md @@ -8,7 +8,7 @@ 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 requests pages of up to 10 parameters and batch-deletes names older than the configured minimum age before fetching the next page, with a 350 ms delay before each batch. 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. +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. Deploy the Lambda update together with the Terraform IAM policy update: batch deletion requires `ssm:DeleteParameters` on the configured token path. From e0c2c8b00cd0911734a9a8e6c0226987dfb5669f Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Fri, 25 Sep 2026 13:37:48 +0200 Subject: [PATCH 3/4] feat(ssm): log confirmed cleanup deletions and run summaries --- docs/configuration.md | 2 + .../aws/ssm/runner-config-housekeeper.test.ts | 58 ++++++++++++++++++- .../aws/ssm/runner-config-housekeeper.ts | 28 ++++++++- .../runner-config/ssm-housekeeper/README.md | 2 + 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index bb722ebe00..e09a3b65fe 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -23,6 +23,8 @@ For the experimental multi-runner configuration, set `multi_runner_config. 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 starting new work 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: 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 fcb7fd4b92..61051020ae 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 @@ -3,12 +3,12 @@ import { mockClient } from 'aws-sdk-client-mock'; import 'aws-sdk-client-mock-jest/vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { warn } = vi.hoisted(() => ({ warn: vi.fn() })); +const { info, warn } = vi.hoisted(() => ({ info: vi.fn(), warn: vi.fn() })); vi.mock('./logger', async (importOriginal) => { const original = await importOriginal(); return { ...original, - createAwsSsmStorageLogger: () => ({ info: vi.fn(), debug: vi.fn(), warn, error: vi.fn() }), + createAwsSsmStorageLogger: () => ({ info, debug: vi.fn(), warn, error: vi.fn() }), }; }); @@ -44,9 +44,10 @@ describe('clean SSM tokens / JIT config', () => { vi.useFakeTimers(); vi.setSystemTime(now); warn.mockClear(); + info.mockClear(); mockSSMClient.reset(); mockSSMClient.on(GetParametersByPathCommand).resolves({ Parameters: staleParameters(1) }); - mockSSMClient.on(DeleteParametersCommand).resolves({}); + mockSSMClient.on(DeleteParametersCommand).callsFake((input) => ({ DeletedParameters: input.Names })); }); afterEach(() => vi.useRealTimers()); @@ -61,6 +62,44 @@ describe('clean SSM tokens / JIT config', () => { expect(batches.flat()).toEqual(parameters.map((parameter) => parameter.Name)); }); + 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, + tokenPath, + status: 'completed', + }); + }); + + 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('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) => @@ -129,6 +168,11 @@ describe('clean SSM tokens / JIT config', () => { 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 }]])( @@ -205,11 +249,19 @@ describe('clean SSM tokens / JIT config', () => { 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 }), + ); }); it('does not start listing with less than ten seconds remaining', async () => { await clean({}, () => 9999); expect(mockSSMClient.calls()).toHaveLength(0); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ status: 'runtime-limit', attempted: 0, deleted: 0 }), + ); }); it('does not delete when listing consumes the remaining time', async () => { 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 c07d8456e5..591b61297b 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts @@ -32,6 +32,8 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = const minimumDate = new Date(); minimumDate.setDate(minimumDate.getDate() - options.minimumDaysOld); 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; @@ -39,13 +41,22 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = await new Promise((resolve) => setTimeout(resolve, DELETE_BATCH_DELAY_MS)); if (remainingTime() < 10000) 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), @@ -60,8 +71,10 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = 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)) + 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); @@ -70,10 +83,21 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = } 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. - await flushPendingNames(); + 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, + }); } } diff --git a/modules/runner-config/ssm-housekeeper/README.md b/modules/runner-config/ssm-housekeeper/README.md index 0ff31bebf5..c86d44b752 100644 --- a/modules/runner-config/ssm-housekeeper/README.md +++ b/modules/runner-config/ssm-housekeeper/README.md @@ -10,6 +10,8 @@ The module is an implementation detail of the experimental runner configuration. 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. From 58bb2ab26c1dfc7bcf81ddc3849c8a5e82fde03d Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Wed, 7 Oct 2026 11:31:35 +0200 Subject: [PATCH 4/4] fix(ssm): reserve runtime to flush partial cleanup batches --- docs/configuration.md | 2 +- .../aws/ssm/runner-config-housekeeper.test.ts | 47 +++++++++++++++++-- .../aws/ssm/runner-config-housekeeper.ts | 12 +++-- .../runner-config/ssm-housekeeper/README.md | 2 +- 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index e09a3b65fe..d207f23c70 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -21,7 +21,7 @@ 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 starting new work 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. +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. 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 61051020ae..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 @@ -255,8 +255,8 @@ describe('clean SSM tokens / JIT config', () => { ); }); - it('does not start listing with less than ten seconds remaining', async () => { - await clean({}, () => 9999); + 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', @@ -275,7 +275,7 @@ describe('clean SSM tokens / JIT config', () => { expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); }); - it('leaves a buffered partial batch for the next run when a later page exhausts runtime', async () => { + 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) @@ -290,11 +290,50 @@ describe('clean SSM tokens / JIT config', () => { }); it('checks the remaining time again after the pacing delay', async () => { - const deadline = now.getTime() + 10300; + let deadline = Infinity; + mockSSMClient.on(GetParametersByPathCommand).callsFake(() => { + deadline = Date.now() + 10300; + return { Parameters: staleParameters(1) }; + }); await clean({}, () => deadline - Date.now()); + expect(mockSSMClient).toHaveReceivedCommandTimes(GetParametersByPathCommand, 1); expect(mockSSMClient).toHaveReceivedCommandTimes(DeleteParametersCommand, 0); }); + 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), + }); + expect(info).toHaveBeenCalledWith( + 'Runner configuration cleanup summary', + expect.objectContaining({ status: 'runtime-limit', attempted: 6, deleted: 6, pending: 0 }), + ); + }); + it('starts a fresh scan of remaining parameters after stopping between pages', async () => { let remaining = 60000; let inventory = staleParameters(11); 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 591b61297b..ec17087c7c 100644 --- a/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts +++ b/lambdas/libs/storage-providers/aws/ssm/runner-config-housekeeper.ts @@ -8,6 +8,10 @@ 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; @@ -37,9 +41,9 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = async function flushPendingNames(): Promise { if (!pendingNames.length) return true; - if (remainingTime() < 10000) return false; + if (remainingTime() < DELETE_RUNTIME_GUARD_MS) return false; await new Promise((resolve) => setTimeout(resolve, DELETE_BATCH_DELAY_MS)); - if (remainingTime() < 10000) return false; + if (remainingTime() < DELETE_RUNTIME_GUARD_MS) return false; const names = pendingNames.splice(0, DELETE_BATCH_SIZE); summary.attempted += names.length; try { @@ -67,10 +71,10 @@ export async function cleanSSMTokens(options: SSMCleanupOptions, remainingTime = try { do { - if (remainingTime() < 10000) return; + 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() < 10000) return; + if (remainingTime() < DELETE_RUNTIME_GUARD_MS) return; if (!parameter.Name || !parameter.LastModifiedDate || !(new Date(parameter.LastModifiedDate) < minimumDate)) { summary.skipped++; continue; diff --git a/modules/runner-config/ssm-housekeeper/README.md b/modules/runner-config/ssm-housekeeper/README.md index c86d44b752..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.