From 26f522a6ff27adc3a00039115ff7e4523b7ed1a0 Mon Sep 17 00:00:00 2001 From: Hoon Oh <2078254+hoonoh@users.noreply.github.com> Date: Tue, 20 Dec 2022 05:40:49 +0900 Subject: [PATCH] fix(core): cross region ssm writer update (#23356) Hello, I have come across many errors with cross region export feature when updating the export values. Without this fix when updating cross region export values and there are no ssm parameters to be deleted, the Lambda function would throw with error: ``` ERROR Error processing event: AccessDeniedException: User: arn:aws:sts::xxx:assumed-role/xxx-CustomCrossRegionExportWrite-xxx/xxx-CustomCrossRegionExportWrite-xxx is not authorized to perform: ssm:DeleteParameters on resource: arn:aws:ssm:us-east-1:xxx:* because no identity-based policy allows the ssm:DeleteParameters action at Request.extractError (/var/runtime/node_modules/aws-sdk/lib/protocol/json.js:52:27) at Request.callListeners (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:106:20) at Request.emit (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:78:10) at Request.emit (/var/runtime/node_modules/aws-sdk/lib/request.js:686:14) at Request.transition (/var/runtime/node_modules/aws-sdk/lib/request.js:22:10) at AcceptorStateMachine.runTo (/var/runtime/node_modules/aws-sdk/lib/state_machine.js:14:12) at /var/runtime/node_modules/aws-sdk/lib/state_machine.js:26:10 at Request. (/var/runtime/node_modules/aws-sdk/lib/request.js:38:9) at Request. (/var/runtime/node_modules/aws-sdk/lib/request.js:688:12) at Request.callListeners (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:116:18) { code: 'AccessDeniedException', time: 2022-12-15T10:13:59.977Z, requestId: 'xxx', statusCode: 400, retryable: false, retryDelay: 10.941837950279254 }} ``` This is because `ssm.deleteParameters` would be called with empty array for `Names` parameter as: ```js await ssm.deleteParameters({ Names: [], }).promise(); ``` ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cross-region-ssm-writer-handler/index.ts | 10 +++++++--- .../cross-region-ssm-writer-handler.test.ts | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/core/lib/custom-resource-provider/cross-region-export-providers/cross-region-ssm-writer-handler/index.ts b/packages/@aws-cdk/core/lib/custom-resource-provider/cross-region-export-providers/cross-region-ssm-writer-handler/index.ts index 84d0e4fe679b1..9f1f2d62d7b99 100644 --- a/packages/@aws-cdk/core/lib/custom-resource-provider/cross-region-export-providers/cross-region-ssm-writer-handler/index.ts +++ b/packages/@aws-cdk/core/lib/custom-resource-provider/cross-region-export-providers/cross-region-ssm-writer-handler/index.ts @@ -30,9 +30,13 @@ 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 - await ssm.deleteParameters({ - Names: Object.keys(removedExports), - }).promise(); + // skip if no export names are to be deleted + const removedExportsNames = Object.keys(removedExports); + if (removedExportsNames.length > 0) { + await ssm.deleteParameters({ + Names: removedExportsNames, + }).promise(); + } // also throw an error if we are creating a new export that already exists for some reason await throwIfAnyInUse(ssm, newExports); diff --git a/packages/@aws-cdk/core/test/custom-resource-provider/cross-region-ssm-writer-handler.test.ts b/packages/@aws-cdk/core/test/custom-resource-provider/cross-region-ssm-writer-handler.test.ts index 17ee6b3c26a31..e5079668b3b8c 100644 --- a/packages/@aws-cdk/core/test/custom-resource-provider/cross-region-ssm-writer-handler.test.ts +++ b/packages/@aws-cdk/core/test/custom-resource-provider/cross-region-ssm-writer-handler.test.ts @@ -177,6 +177,7 @@ describe('cross-region-ssm-writer entrypoint', () => { }); expect(mockPutParameter).toHaveBeenCalledTimes(1); expect(mocklistTagsForResource).toHaveBeenCalledTimes(1); + expect(mockDeleteParameters).toHaveBeenCalledTimes(0); }); test('removed exports are deleted', async () => {