From 0d2b5291a10a318bed8d77166eae2bd317dee62e Mon Sep 17 00:00:00 2001 From: watany <76135106+watany-dev@users.noreply.github.com> Date: Wed, 19 Oct 2022 20:28:10 +0900 Subject: [PATCH] fix(config): Creating multiple rules from the same lambda (#21594) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes #17582 because the id of ".addPermission" is set to a fixed value of ″permission″, which means that only one can be set in the stack. 1. and add a unique suffix to the id. This will allow multiple custom rules to be handled in one stack. 2. Do the id check before addPermission. This will allow only one permission to be granted to a custom rule from the config service. Addendum:. I have created a hash from FunctionName, AccountID, and Region to make the suffix unique. Therefore, the omitted parts in the test code have been modified to fix the result. ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-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* --- packages/@aws-cdk/aws-config/lib/rule.ts | 22 ++++++--- .../aws-config/test/integ.rule.lit.ts | 2 +- .../aws-config/test/integ.scoped-rule.ts | 2 +- .../aws-cdk-config-rule-integ.assets.json | 4 +- .../aws-cdk-config-rule-integ.template.json | 4 +- .../rule.lit.integ.snapshot/manifest.json | 15 ++++-- .../test/rule.lit.integ.snapshot/tree.json | 6 +-- .../@aws-cdk/aws-config/test/rule.test.ts | 48 +++++++++++++++++-- ...s-cdk-config-rule-scoped-integ.assets.json | 4 +- ...cdk-config-rule-scoped-integ.template.json | 4 +- .../scoped-rule.integ.snapshot/manifest.json | 15 ++++-- .../test/scoped-rule.integ.snapshot/tree.json | 6 +-- 12 files changed, 99 insertions(+), 33 deletions(-) diff --git a/packages/@aws-cdk/aws-config/lib/rule.ts b/packages/@aws-cdk/aws-config/lib/rule.ts index 78c3481db084d..fc1d1e6c83eb3 100644 --- a/packages/@aws-cdk/aws-config/lib/rule.ts +++ b/packages/@aws-cdk/aws-config/lib/rule.ts @@ -1,7 +1,8 @@ +import { createHash } from 'crypto'; import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; import * as lambda from '@aws-cdk/aws-lambda'; -import { IResource, Lazy, Resource } from '@aws-cdk/core'; +import { IResource, Lazy, Resource, Stack } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { CfnConfigRule } from './config.generated'; @@ -408,11 +409,20 @@ export class CustomRule extends RuleNew { messageType: MessageType.SCHEDULED_NOTIFICATION, }); } - - props.lambdaFunction.addPermission('Permission', { - principal: new iam.ServicePrincipal('config.amazonaws.com'), - sourceAccount: this.env.account, - }); + const hash = createHash('sha256') + .update(JSON.stringify({ + fnName: props.lambdaFunction.functionName.toString, + accountId: Stack.of(this).resolve(this.env.account), + region: Stack.of(this).resolve(this.env.region), + }), 'utf8') + .digest('base64'); + const customRulePermissionId: string = `CustomRulePermission${hash}`; + if (!props.lambdaFunction.permissionsNode.tryFindChild(customRulePermissionId)) { + props.lambdaFunction.addPermission(customRulePermissionId, { + principal: new iam.ServicePrincipal('config.amazonaws.com'), + sourceAccount: this.env.account, + }); + }; if (props.lambdaFunction.role) { props.lambdaFunction.role.addManagedPolicy( diff --git a/packages/@aws-cdk/aws-config/test/integ.rule.lit.ts b/packages/@aws-cdk/aws-config/test/integ.rule.lit.ts index a5d9c646d1d10..08e2c2ee8a584 100644 --- a/packages/@aws-cdk/aws-config/test/integ.rule.lit.ts +++ b/packages/@aws-cdk/aws-config/test/integ.rule.lit.ts @@ -38,5 +38,5 @@ class ConfigStack extends cdk.Stack { } } -new ConfigStack(app, 'aws-cdk-config-rule-integ'); +new ConfigStack(app, 'aws-cdk-config-rule-integ', {}); app.synth(); diff --git a/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts b/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts index 833794f336ad8..0eccad268e525 100644 --- a/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts +++ b/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts @@ -4,7 +4,7 @@ import * as config from '../lib'; const app = new cdk.App(); -const stack = new cdk.Stack(app, 'aws-cdk-config-rule-scoped-integ'); +const stack = new cdk.Stack(app, 'aws-cdk-config-rule-scoped-integ', {}); const fn = new lambda.Function(stack, 'CustomFunction', { code: lambda.AssetCode.fromInline('exports.handler = (event) => console.log(event);'), diff --git a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.assets.json b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.assets.json index 7f96ffaf0785c..9e29cd6c1ff13 100644 --- a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.assets.json +++ b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.assets.json @@ -1,7 +1,7 @@ { "version": "21.0.0", "files": { - "9c0ec14ff7954b877625fb363a75213d58cb40e40acfcb23727388ddf0c52fec": { + "99b272ad5d23fb805d1e06b58a04179d8720a36f6aa8cf035eff419db2e87432": { "source": { "path": "aws-cdk-config-rule-integ.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "9c0ec14ff7954b877625fb363a75213d58cb40e40acfcb23727388ddf0c52fec.json", + "objectKey": "99b272ad5d23fb805d1e06b58a04179d8720a36f6aa8cf035eff419db2e87432.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.template.json b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.template.json index 68c6f438dafbd..453a6df4c44cb 100644 --- a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.template.json +++ b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/aws-cdk-config-rule-integ.template.json @@ -62,7 +62,7 @@ "CustomFunctionServiceRoleD3F73B79" ] }, - "CustomFunctionPermission41887A5E": { + "CustomFunctionCustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5QED54A3F8": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -107,7 +107,7 @@ } }, "DependsOn": [ - "CustomFunctionPermission41887A5E", + "CustomFunctionCustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5QED54A3F8", "CustomFunctionBADD59E7", "CustomFunctionServiceRoleD3F73B79" ] diff --git a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/manifest.json index 231c02b46bba8..7c1a82eb94b9e 100644 --- a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/manifest.json +++ b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/manifest.json @@ -23,7 +23,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/9c0ec14ff7954b877625fb363a75213d58cb40e40acfcb23727388ddf0c52fec.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/99b272ad5d23fb805d1e06b58a04179d8720a36f6aa8cf035eff419db2e87432.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -51,10 +51,10 @@ "data": "CustomFunctionBADD59E7" } ], - "/aws-cdk-config-rule-integ/CustomFunction/Permission": [ + "/aws-cdk-config-rule-integ/CustomFunction/CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=": [ { "type": "aws:cdk:logicalId", - "data": "CustomFunctionPermission41887A5E" + "data": "CustomFunctionCustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5QED54A3F8" } ], "/aws-cdk-config-rule-integ/Custom/Resource": [ @@ -104,6 +104,15 @@ "type": "aws:cdk:logicalId", "data": "CheckBootstrapVersion" } + ], + "CustomFunctionCustomRulePermissionXogMcOcBfKkfAgTC3zxpecyWNuSNTUwy6QrCZdRtCdwF5AB15B7": [ + { + "type": "aws:cdk:logicalId", + "data": "CustomFunctionCustomRulePermissionXogMcOcBfKkfAgTC3zxpecyWNuSNTUwy6QrCZdRtCdwF5AB15B7", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] + } ] }, "displayName": "aws-cdk-config-rule-integ" diff --git a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/tree.json b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/tree.json index 302082fb96f44..f2b1dc364816f 100644 --- a/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/tree.json +++ b/packages/@aws-cdk/aws-config/test/rule.lit.integ.snapshot/tree.json @@ -105,9 +105,9 @@ "version": "0.0.0" } }, - "Permission": { - "id": "Permission", - "path": "aws-cdk-config-rule-integ/CustomFunction/Permission", + "CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=": { + "id": "CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=", + "path": "aws-cdk-config-rule-integ/CustomFunction/CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=", "attributes": { "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", "aws:cdk:cloudformation:props": { diff --git a/packages/@aws-cdk/aws-config/test/rule.test.ts b/packages/@aws-cdk/aws-config/test/rule.test.ts index 740ebf9220264..ae037c19df4ce 100644 --- a/packages/@aws-cdk/aws-config/test/rule.test.ts +++ b/packages/@aws-cdk/aws-config/test/rule.test.ts @@ -91,11 +91,6 @@ describe('rule', () => { }, MaximumExecutionFrequency: 'Six_Hours', }, - DependsOn: [ - 'FunctionPermissionEC8FE997', - 'Function76856677', - 'FunctionServiceRole675BB04A', - ], }); Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { @@ -460,6 +455,49 @@ describe('rule', () => { }); }); + test('create two custom rules and one function', () => { + // GIVEN + const stack = new cdk.Stack(); + const fn = new lambda.Function(stack, 'Function', { + code: lambda.AssetCode.fromInline('foo'), + handler: 'index.handler', + runtime: lambda.Runtime.NODEJS_14_X, + }); + + // WHEN + new config.CustomRule(stack, 'Rule1', { + configurationChanges: true, + description: 'really cool rule', + lambdaFunction: fn, + maximumExecutionFrequency: config.MaximumExecutionFrequency.SIX_HOURS, + configRuleName: 'cool rule 1', + periodic: true, + }); + new config.CustomRule(stack, 'Rule2', { + configurationChanges: true, + description: 'really cool rule', + lambdaFunction: fn, + configRuleName: 'cool rule 2', + }); + + // THEN + Template.fromStack(stack).resourceCountIs('AWS::Config::ConfigRule', 2); + Template.fromStack(stack).resourceCountIs('AWS::Lambda::Permission', 1); + + Template.fromStack(stack).hasResourceProperties('AWS::Lambda::Permission', { + Action: 'lambda:InvokeFunction', + FunctionName: { + 'Fn::GetAtt': [ + 'Function76856677', + 'Arn', + ], + }, + Principal: 'config.amazonaws.com', + SourceAccount: { + Ref: 'AWS::AccountId', + }, + }); + }); test('create a 0 charactor policy', () => { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.assets.json b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.assets.json index fef322c5aba81..300c3d5fa29f2 100644 --- a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.assets.json +++ b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.assets.json @@ -1,7 +1,7 @@ { "version": "21.0.0", "files": { - "334d65f391737c79c5dd4a7f1fd9b8b58c86d362835cfcfd1a3873245cb214e0": { + "ce24448515abcdc66d5b46f4e7b5a3a4bad2eda8fa9f00dde24710cbc9860c87": { "source": { "path": "aws-cdk-config-rule-scoped-integ.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "334d65f391737c79c5dd4a7f1fd9b8b58c86d362835cfcfd1a3873245cb214e0.json", + "objectKey": "ce24448515abcdc66d5b46f4e7b5a3a4bad2eda8fa9f00dde24710cbc9860c87.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.template.json b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.template.json index bbbf2466ae4a5..e004ce28e7a8c 100644 --- a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.template.json +++ b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/aws-cdk-config-rule-scoped-integ.template.json @@ -62,7 +62,7 @@ "CustomFunctionServiceRoleD3F73B79" ] }, - "CustomFunctionPermission41887A5E": { + "CustomFunctionCustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5QED54A3F8": { "Type": "AWS::Lambda::Permission", "Properties": { "Action": "lambda:InvokeFunction", @@ -103,7 +103,7 @@ } }, "DependsOn": [ - "CustomFunctionPermission41887A5E", + "CustomFunctionCustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5QED54A3F8", "CustomFunctionBADD59E7", "CustomFunctionServiceRoleD3F73B79" ] diff --git a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/manifest.json b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/manifest.json index 5df879d5b8fd0..6806b5a47e4c5 100644 --- a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/manifest.json +++ b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/manifest.json @@ -23,7 +23,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/334d65f391737c79c5dd4a7f1fd9b8b58c86d362835cfcfd1a3873245cb214e0.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/ce24448515abcdc66d5b46f4e7b5a3a4bad2eda8fa9f00dde24710cbc9860c87.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -51,10 +51,10 @@ "data": "CustomFunctionBADD59E7" } ], - "/aws-cdk-config-rule-scoped-integ/CustomFunction/Permission": [ + "/aws-cdk-config-rule-scoped-integ/CustomFunction/CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=": [ { "type": "aws:cdk:logicalId", - "data": "CustomFunctionPermission41887A5E" + "data": "CustomFunctionCustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5QED54A3F8" } ], "/aws-cdk-config-rule-scoped-integ/Custom/Resource": [ @@ -74,6 +74,15 @@ "type": "aws:cdk:logicalId", "data": "CheckBootstrapVersion" } + ], + "CustomFunctionPermission41887A5E": [ + { + "type": "aws:cdk:logicalId", + "data": "CustomFunctionPermission41887A5E", + "trace": [ + "!!DESTRUCTIVE_CHANGES: WILL_DESTROY" + ] + } ] }, "displayName": "aws-cdk-config-rule-scoped-integ" diff --git a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/tree.json b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/tree.json index f1136029cb247..5b58c8e4a216f 100644 --- a/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/tree.json +++ b/packages/@aws-cdk/aws-config/test/scoped-rule.integ.snapshot/tree.json @@ -105,9 +105,9 @@ "version": "0.0.0" } }, - "Permission": { - "id": "Permission", - "path": "aws-cdk-config-rule-scoped-integ/CustomFunction/Permission", + "CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=": { + "id": "CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=", + "path": "aws-cdk-config-rule-scoped-integ/CustomFunction/CustomRulePermissionbM1jVaicvRO9SDCiAbsQcYrOlESEtMwrrF9ZQQRvd5Q=", "attributes": { "aws:cdk:cloudformation:type": "AWS::Lambda::Permission", "aws:cdk:cloudformation:props": {