From 841060d6adde4ea6d58e008f85cc155b8c3a3768 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Mon, 6 Jul 2020 19:51:32 +0200 Subject: [PATCH] fix(config): cannot scope a custom rule without configurationChanges on (#8738) While CloudFormation allows to specify the scope for a custom ConfigRule without necessarily specifying `configurationChanges: true`, this was not allowed by the corresponding construct. This removed the offending guard, and replaced the test that verified the throwing behavior with a regression test that validates this configuration is allowed. ---- *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 | 20 +--- .../test/integ.scoped-rule.expected.json | 109 ++++++++++++++++++ .../aws-config/test/integ.scoped-rule.ts | 22 ++++ .../@aws-cdk/aws-config/test/test.rule.ts | 4 +- 4 files changed, 139 insertions(+), 16 deletions(-) create mode 100644 packages/@aws-cdk/aws-config/test/integ.scoped-rule.expected.json create mode 100644 packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts diff --git a/packages/@aws-cdk/aws-config/lib/rule.ts b/packages/@aws-cdk/aws-config/lib/rule.ts index 0659fefa8089b..12dd86b8ab1bd 100644 --- a/packages/@aws-cdk/aws-config/lib/rule.ts +++ b/packages/@aws-cdk/aws-config/lib/rule.ts @@ -122,10 +122,10 @@ abstract class RuleNew extends RuleBase { * @param identifier the resource identifier */ public scopeToResource(type: string, identifier?: string) { - this.scopeTo({ + this.scope = { complianceResourceId: identifier, complianceResourceTypes: [type], - }); + }; } /** @@ -136,9 +136,9 @@ abstract class RuleNew extends RuleBase { * @param types resource types */ public scopeToResources(...types: string[]) { - this.scopeTo({ + this.scope = { complianceResourceTypes: types, - }); + }; } /** @@ -148,18 +148,10 @@ abstract class RuleNew extends RuleBase { * @param value the tag value */ public scopeToTag(key: string, value?: string) { - this.scopeTo({ + this.scope = { tagKey: key, tagValue: value, - }); - } - - private scopeTo(scope: CfnConfigRule.ScopeProperty) { - if (!this.isManaged && !this.isCustomWithChanges) { - throw new Error('Cannot scope rule when `configurationChanges` is set to false.'); - } - - this.scope = scope; + }; } } diff --git a/packages/@aws-cdk/aws-config/test/integ.scoped-rule.expected.json b/packages/@aws-cdk/aws-config/test/integ.scoped-rule.expected.json new file mode 100644 index 0000000000000..99d314d0c45af --- /dev/null +++ b/packages/@aws-cdk/aws-config/test/integ.scoped-rule.expected.json @@ -0,0 +1,109 @@ +{ + "Resources": { + "CustomFunctionServiceRoleD3F73B79": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] + ] + }, + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSConfigRulesExecutionRole" + ] + ] + } + ] + } + }, + "CustomFunctionBADD59E7": { + "Type": "AWS::Lambda::Function", + "Properties": { + "Code": { + "ZipFile": "exports.handler = (event) => console.log(event);" + }, + "Handler": "index.handler", + "Role": { + "Fn::GetAtt": [ + "CustomFunctionServiceRoleD3F73B79", + "Arn" + ] + }, + "Runtime": "nodejs10.x" + }, + "DependsOn": [ + "CustomFunctionServiceRoleD3F73B79" + ] + }, + "CustomFunctionPermission41887A5E": { + "Type": "AWS::Lambda::Permission", + "Properties": { + "Action": "lambda:InvokeFunction", + "FunctionName": { + "Fn::GetAtt": [ + "CustomFunctionBADD59E7", + "Arn" + ] + }, + "Principal": "config.amazonaws.com" + } + }, + "Custom8166710A": { + "Type": "AWS::Config::ConfigRule", + "Properties": { + "Source": { + "Owner": "CUSTOM_LAMBDA", + "SourceDetails": [ + { + "EventSource": "aws.config", + "MessageType": "ScheduledNotification" + } + ], + "SourceIdentifier": { + "Fn::GetAtt": [ + "CustomFunctionBADD59E7", + "Arn" + ] + } + }, + "Scope": { + "ComplianceResourceTypes": [ + "AWS::EC2::Instance" + ] + } + }, + "DependsOn": [ + "CustomFunctionPermission41887A5E", + "CustomFunctionBADD59E7", + "CustomFunctionServiceRoleD3F73B79" + ] + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts b/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts new file mode 100644 index 0000000000000..aee8392f402f0 --- /dev/null +++ b/packages/@aws-cdk/aws-config/test/integ.scoped-rule.ts @@ -0,0 +1,22 @@ +import * as lambda from '@aws-cdk/aws-lambda'; +import * as cdk from '@aws-cdk/core'; +import * as config from '../lib'; + +const app = new cdk.App(); + +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);'), + handler: 'index.handler', + runtime: lambda.Runtime.NODEJS_10_X, +}); + +const customRule = new config.CustomRule(stack, 'Custom', { + lambdaFunction: fn, + periodic: true, +}); + +customRule.scopeToResource('AWS::EC2::Instance'); + +app.synth(); diff --git a/packages/@aws-cdk/aws-config/test/test.rule.ts b/packages/@aws-cdk/aws-config/test/test.rule.ts index 0f19a826de6d2..c13dacf2c8a3f 100644 --- a/packages/@aws-cdk/aws-config/test/test.rule.ts +++ b/packages/@aws-cdk/aws-config/test/test.rule.ts @@ -204,7 +204,7 @@ export = { test.done(); }, - 'throws when scoping a custom rule without configuration changes'(test: Test) { + 'allows scoping a custom rule without configurationChanges enabled'(test: Test) { // GIVEN const stack = new cdk.Stack(); const fn = new lambda.Function(stack, 'Function', { @@ -220,7 +220,7 @@ export = { }); // THEN - test.throws(() => rule.scopeToResource('resource'), /`configurationChanges`/); + test.doesNotThrow(() => rule.scopeToResource('resource')); test.done(); },