Skip to content

Commit

Permalink
chore(core): enable deletion of more than 10 ssm parameters at a time (
Browse files Browse the repository at this point in the history
…aws#27391)

Cross-region-ssm-writer-handler fails when trying to delete more than 10 ssm parameters

This feature is used when exporting resources from a stack in one region to a stack in another region.
It uses the `crossRegionReferences: true` flag in the stack.

The reason for this change is the following error I received on a stack update (slightly modified to hide some names)

```
❌ Deployment failed: Error: The stack named NomCoreStack failed to deploy: UPDATE_ROLLBACK_FAILED (The following resource(s) failed to update: [ExportsWriteruseast2828FA26B86FBEFA7]. ): Received response status [FAILED] from custom resource. Message returned: ValidationException: 1 validation error detected: Value '[/cdk/exports/NomServicesStack/NomCoreStackxohpRefUserPoolIdentityPoolA58D72D6C9223D28, /cdk/exports/NomIntegrationTestsStack/NomCoreStackxohpRefUserPoolUserPoolClient40176907C34D1493, /cdk/exports/NomLeafStack/NomCoreStackxohpRefUserPoolIdentityPoolA58D72D6C9223D28, /cdk/exports/NomLeafStack/NomCoreStackxohpFnGetAttUserPool6D0DFADBArnD191ECDE, /cdk/exports/NomCoreStack/NomCoreStackxohpRefUserPoolUserPoolClient40176907C34D1493, /cdk/exports/NomIntegrationTestsStack/NomCoreStackxohpFnGetAttUserPool6D0DFADBArnD191ECDE, /cdk/exports/NomIntegrationTestsStack/NomCoreStackxohpRefUserPoolIdentityPoolA58D72D6C9223D28, /cdk/exports/NomServicesStack/NomCoreStackxohpFnGetAttUserPool6D0DFADBArnD191ECDE, /cdk/exports/NomLeafStack/NomCoreStackxohpRefUserPoolUserPoolClient40176907C34D1493, /cdk/exports/NomServicesStack/NomCoreStackxohpRefUserPoolUserPoolClient40176907C34D1493, /cdk/exports/NomLeafStack/NomCoreStackxohpRefUserPoolUserPoolDomain9F01E991C941B942]' at 'names' failed to satisfy constraint: Member must have length less than or equal to 10
```

Design for the fix is simple: Break the list of names to delete into chunks of at most 10. Invoke delete as many times as needed to delete all. Throttling is not handled here. This may come into play when there are 50 or more parameters to delete, which may not be a concern worth addressing at this point.


Closes No issue created for this

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
twoquarterrican authored Oct 4, 2023
1 parent ae24fbf commit 08f8cd3
Show file tree
Hide file tree
Showing 8 changed files with 254 additions and 169 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
"S3Bucket": {
"Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1"
},
"S3Key": "1a067234d252533a95ecaaccd4b3e821e6a69df0b03b918b596fc5a40eeb71a1.zip"
"S3Key": "0cd3b0876ff31491e457d92c24b80c95310950e9839c43eb5a86ad9535d15597.zip"
},
"Timeout": 900,
"MemorySize": 128,
Expand Down

Large diffs are not rendered by default.

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,9 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
const removedExports = except(oldExports, exports);
await throwIfAnyInUse(ssm, removedExports);
// if the ones we are removing are not in use then delete them
// skip if no export names are to be deleted
const removedExportsNames = Object.keys(removedExports);
if (removedExportsNames.length > 0) {
await ssm.deleteParameters({
Names: removedExportsNames,
});
}
// this method will skip if no export names are to be deleted
await deleteParameters(ssm, removedExportsNames);

// also throw an error if we are creating a new export that already exists for some reason
await throwIfAnyInUse(ssm, newExports);
Expand All @@ -48,9 +44,7 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
// the stack deletion.
await throwIfAnyInUse(ssm, exports);
// if none are in use then delete all of them
await ssm.deleteParameters({
Names: Object.keys(exports),
});
await deleteParameters(ssm, Object.keys(exports));
return;
default:
return;
Expand All @@ -74,6 +68,27 @@ async function putParameters(ssm: SSM, parameters: CrossRegionExports): Promise<
}));
}

/**
* Delete parameters no longer in use.
* From https://docs.aws.amazon.com/systems-manager/latest/APIReference/API_DeleteParameters.html there
* is a constraint on names. It must have size at least 1 and at most 10.
*/
async function deleteParameters(ssm: SSM, names: string[]) {
// max allowed by DeleteParameters api
const maxSize = 10;
// more testable if we delete in order
names.sort();
for (let chunkStartIdx = 0; chunkStartIdx < names.length; chunkStartIdx += maxSize) {
const chunkOfNames = names.slice(chunkStartIdx, chunkStartIdx + maxSize);
// also observe minimum size constraint: Names parameter must have size at least 1
if (chunkOfNames.length > 0) {
await ssm.deleteParameters({
Names: chunkOfNames,
});
}
}
}

/**
* Query for existing parameters that are in use
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,62 @@ describe('cross-region-ssm-writer entrypoint', () => {
});
});

test('thorws if parameters are in use', async () => {
test('more than 10 parameters are deleted', async () => {
// GIVEN
const event = makeEvent({
RequestType: 'Delete',
ResourceProperties: {
ServiceToken: '<ServiceToken>',
WriterProps: {
region: 'us-east-1',
exports: {
'/cdk/exports/MyStack/RemovedExport01': 'RemovedValue01',
'/cdk/exports/MyStack/RemovedExport02': 'RemovedValue02',
'/cdk/exports/MyStack/RemovedExport03': 'RemovedValue03',
'/cdk/exports/MyStack/RemovedExport04': 'RemovedValue04',
'/cdk/exports/MyStack/RemovedExport05': 'RemovedValue05',
'/cdk/exports/MyStack/RemovedExport06': 'RemovedValue06',
'/cdk/exports/MyStack/RemovedExport07': 'RemovedValue07',
'/cdk/exports/MyStack/RemovedExport08': 'RemovedValue08',
'/cdk/exports/MyStack/RemovedExport09': 'RemovedValue09',
'/cdk/exports/MyStack/RemovedExport10': 'RemovedValue10',
'/cdk/exports/MyStack/RemovedExport11': 'RemovedValue11',
'/cdk/exports/MyStack/RemovedExport12': 'RemovedValue12',
},
},
},
});

// WHEN
await handler(event);

// THEN
expect(mockPutParameter).toHaveBeenCalledTimes(0);
expect(mocklistTagsForResource).toHaveBeenCalledTimes(12);
expect(mockDeleteParameters).toHaveBeenCalledTimes(2);
expect(mockDeleteParameters).toHaveBeenCalledWith({
Names: [
'/cdk/exports/MyStack/RemovedExport01',
'/cdk/exports/MyStack/RemovedExport02',
'/cdk/exports/MyStack/RemovedExport03',
'/cdk/exports/MyStack/RemovedExport04',
'/cdk/exports/MyStack/RemovedExport05',
'/cdk/exports/MyStack/RemovedExport06',
'/cdk/exports/MyStack/RemovedExport07',
'/cdk/exports/MyStack/RemovedExport08',
'/cdk/exports/MyStack/RemovedExport09',
'/cdk/exports/MyStack/RemovedExport10',
],
});
expect(mockDeleteParameters).toHaveBeenCalledWith({
Names: [
'/cdk/exports/MyStack/RemovedExport11',
'/cdk/exports/MyStack/RemovedExport12',
],
});
});

test('throws if parameters are in use', async () => {
// GIVEN
const event = makeEvent({
RequestType: 'Delete',
Expand Down

0 comments on commit 08f8cd3

Please sign in to comment.