From e1179cc757a7a8810d5b4702c9e48bf090cb7c8c Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 3 Jul 2019 12:22:15 +0200 Subject: [PATCH] fix(events): allow adding same target to rule multiple times As rule targets can have different input configurations adding the same target multiple times must be possible. Move rule target id generation to `aws-events` where it's easy to simply increment a counter. This id is passed as an argument to `bind()` for targets that need to know their given id (e.g. `EcsTask`). Add `permissionsNode` on `IFunction` to handle permission checks for both functions and singleton functions. Fixes #3173 --- .../integ.codecommit-events.expected.json | 4 +- ...yed-through-codepipeline.lit.expected.json | 4 +- .../test/integ.lambda-pipeline.expected.json | 2 +- ...uild-multiple-inputs-outputs.expected.json | 2 +- .../integ.pipeline-code-commit.expected.json | 2 +- .../integ.pipeline-ecr-source.expected.json | 2 +- .../test/integ.pipeline-events.expected.json | 6 +-- .../test/integ.rule.lit.expected.json | 4 +- ...integ.scheduled-ecs-task.lit.expected.json | 4 +- ...g.scheduled-fargate-task.lit.expected.json | 6 +-- .../aws-events-targets/lib/codebuild.ts | 3 +- .../aws-events-targets/lib/codepipeline.ts | 3 +- .../aws-events-targets/lib/ecs-task.ts | 6 +-- .../@aws-cdk/aws-events-targets/lib/lambda.ts | 5 +- .../@aws-cdk/aws-events-targets/lib/sns.ts | 3 +- .../@aws-cdk/aws-events-targets/lib/sqs.ts | 3 +- .../aws-events-targets/lib/state-machine.ts | 3 +- .../test/codebuild/codebuild.test.ts | 2 +- .../integ.project-events.expected.json | 10 ++-- .../integ.pipeline-event-target.expected.json | 4 +- .../test/codepipeline/pipeline.test.ts | 2 +- .../test/ecs/event-rule-target.test.ts | 6 +-- .../integ.event-ec2-task.lit.expected.json | 4 +- .../integ.event-fargate-task.expected.json | 6 +-- .../test/lambda/integ.events.expected.json | 6 +-- .../test/lambda/lambda.test.ts | 47 ++++++++++++++++++- .../integ.sns-event-rule-target.expected.json | 4 +- .../aws-events-targets/test/sns/sns.test.ts | 2 +- .../integ.sqs-event-rule-target.expected.json | 4 +- .../aws-events-targets/test/sqs/sqs.test.ts | 2 +- packages/@aws-cdk/aws-events/lib/rule.ts | 22 ++------- packages/@aws-cdk/aws-events/lib/target.ts | 10 +--- .../@aws-cdk/aws-lambda/lib/function-base.ts | 14 +++++- packages/@aws-cdk/aws-lambda/lib/function.ts | 4 +- .../aws-lambda/lib/singleton-lambda.ts | 2 + .../test/integ.instance.lit.expected.json | 2 +- .../aws-ses/lib/receipt-rule-action.ts | 2 +- .../test/__snapshots__/synth.test.js.snap | 2 +- 38 files changed, 126 insertions(+), 93 deletions(-) diff --git a/packages/@aws-cdk/aws-codecommit/test/integ.codecommit-events.expected.json b/packages/@aws-cdk/aws-codecommit/test/integ.codecommit-events.expected.json index 51d2d3905571d..bb22681403bf0 100644 --- a/packages/@aws-cdk/aws-codecommit/test/integ.codecommit-events.expected.json +++ b/packages/@aws-cdk/aws-codecommit/test/integ.codecommit-events.expected.json @@ -37,7 +37,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "MyTopic" + "Id": "Target0" } ] } @@ -46,4 +46,4 @@ "Type": "AWS::SNS::Topic" } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json index d94d8873b169a..73d6b0aa3f8b2 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json @@ -1194,7 +1194,7 @@ ] ] }, - "Id": "PipelineStackPipeline9DB740AF", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "PipelineEventsRole46BEEA7C", @@ -1266,7 +1266,7 @@ ] ] }, - "Id": "PipelineStackPipeline9DB740AF", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "PipelineEventsRole46BEEA7C", diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json index d0169c49a5d88..4733d7b5fa301 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json @@ -635,7 +635,7 @@ ] ] }, - "Id": "awscdkcodepipelinelambdaPipeline87A4B3D3", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "PipelineEventsRole46BEEA7C", diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-build-multiple-inputs-outputs.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-build-multiple-inputs-outputs.expected.json index 3ce027cd3cced..b84bae7cf8ada 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-build-multiple-inputs-outputs.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-build-multiple-inputs-outputs.expected.json @@ -61,7 +61,7 @@ ] ] }, - "Id": "awscdkcodepipelinecodebuildmultipleinputsoutputsPipeline314D3A85", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "PipelineEventsRole46BEEA7C", diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json index 360da5b54c718..a800db4b9e522 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json @@ -61,7 +61,7 @@ ] ] }, - "Id": "awscdkcodepipelinecodecommitPipelineF780CA18", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "PipelineEventsRole46BEEA7C", diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-ecr-source.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-ecr-source.expected.json index 68f342ae88c02..5601fb1bdb6cd 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-ecr-source.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-ecr-source.expected.json @@ -421,7 +421,7 @@ ] ] }, - "Id": "awscdkcodepipelineecrsourceMyPipeline63CF3194", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "MyPipelineEventsRoleFAB99F32", diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json index ca5873f6a6ed6..f58c956098bfe 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json @@ -508,7 +508,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdkpipelineeventtargetMyTopic8D32776A" + "Id": "Target0" } ] } @@ -560,7 +560,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdkpipelineeventtargetMyTopic8D32776A" + "Id": "Target0" } ] } @@ -669,7 +669,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdkpipelineeventtargetMyTopic8D32776A", + "Id": "Target0", "InputTransformer": { "InputPathsMap": { "detail-pipeline": "$.detail.pipeline", diff --git a/packages/@aws-cdk/aws-config/test/integ.rule.lit.expected.json b/packages/@aws-cdk/aws-config/test/integ.rule.lit.expected.json index 86cb180ebc9e3..433de9a2406f5 100644 --- a/packages/@aws-cdk/aws-config/test/integ.rule.lit.expected.json +++ b/packages/@aws-cdk/aws-config/test/integ.rule.lit.expected.json @@ -206,7 +206,7 @@ "Arn": { "Ref": "ComplianceTopic0229448B" }, - "Id": "awscdkconfigruleintegComplianceTopic55CAF01A" + "Id": "Target0" } ] } @@ -251,4 +251,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json index d344a7f79e42c..f000022c728a7 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json +++ b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json @@ -849,7 +849,7 @@ "Ref": "ScheduledEc2TaskScheduledTaskDef56328BA4" } }, - "Id": "awsecsintegecsScheduledEc2TaskScheduledTaskDef18FB4348", + "Id": "Target0", "Input": "{}", "RoleArn": { "Fn::GetAtt": [ @@ -868,4 +868,4 @@ "Default": "/aws/service/ecs/optimized-ami/amazon-linux-2/recommended/image_id" } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.scheduled-fargate-task.lit.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.scheduled-fargate-task.lit.expected.json index d8b8dcff9ffe7..f796a7143d42f 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.scheduled-fargate-task.lit.expected.json +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.scheduled-fargate-task.lit.expected.json @@ -212,7 +212,7 @@ "Ref": "ScheduledFargateTaskScheduledTaskDef521FA675" } }, - "Id": "awsfargateintegScheduledFargateTaskScheduledTaskDefB0AD4F70", + "Id": "Target0", "Input": "{}", "RoleArn": { "Fn::GetAtt": [ @@ -683,7 +683,7 @@ "Arn" ] }, - "Id": "awsfargateintegScheduledFargateTaskScheduledTaskDefB0AD4F70", + "Id": "Target0", "EcsParameters": { "TaskDefinitionArn": { "Ref": "ScheduledFargateTaskScheduledTaskDef521FA675" @@ -760,7 +760,7 @@ "Arn" ] }, - "Id": "awsfargateintegScheduledFargateTaskScheduledTaskDefB0AD4F70", + "Id": "Target0", "EcsParameters": { "TaskDefinitionArn": { "Ref": "ScheduledFargateTaskScheduledTaskDef521FA675" diff --git a/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts b/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts index 8b03b0d998eef..a253d729d9c4a 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts @@ -13,9 +13,8 @@ export class CodeBuildProject implements events.IRuleTarget { /** * Allows using build projects as event rule targets. */ - public bind(_rule: events.IRule): events.RuleTargetConfig { + public bind(_rule: events.IRule, _id: string): events.RuleTargetConfig { return { - id: this.project.node.uniqueId, arn: this.project.projectArn, role: singletonEventRole(this.project, [new iam.PolicyStatement({ actions: ['codebuild:StartBuild'], diff --git a/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts b/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts index ef0fa12a97176..9ec4c5bd5088c 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts @@ -10,9 +10,8 @@ export class CodePipeline implements events.IRuleTarget { constructor(private readonly pipeline: codepipeline.IPipeline) { } - public bind(_rule: events.IRule): events.RuleTargetConfig { + public bind(_rule: events.IRule, _id: string): events.RuleTargetConfig { return { - id: this.pipeline.node.uniqueId, arn: this.pipeline.pipelineArn, role: singletonEventRole(this.pipeline, [new iam.PolicyStatement({ resources: [this.pipeline.pipelineArn], diff --git a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts index e3f0942d8e2d0..c591ec9b7e545 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/ecs-task.ts @@ -77,7 +77,7 @@ export class EcsTask implements events.IRuleTarget { /** * Allows using tasks as target of CloudWatch events */ - public bind(rule: events.IRule): events.RuleTargetConfig { + public bind(rule: events.IRule, id: string): events.RuleTargetConfig { const policyStatements = [new iam.PolicyStatement({ actions: ['ecs:RunTask'], resources: [this.taskDefinition.taskDefinitionArn], @@ -103,7 +103,6 @@ export class EcsTask implements events.IRuleTarget { })); } - const id = this.taskDefinition.node.uniqueId; const arn = this.cluster.clusterArn; const role = singletonEventRole(this.taskDefinition, policyStatements); const containerOverrides = this.props.containerOverrides && this.props.containerOverrides @@ -148,7 +147,7 @@ export class EcsTask implements events.IRuleTarget { } ] }, - physicalResourceId: id, + physicalResourceId: this.taskDefinition.node.uniqueId, }, policyStatements: [ // Cannot use automatic policy statements because we need iam:PassRole new iam.PolicyStatement({ @@ -164,7 +163,6 @@ export class EcsTask implements events.IRuleTarget { } return { - id, arn, role, ecsParameters: { diff --git a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts index 7f3cbb1fa3b0e..c601fe69b2a90 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts @@ -28,9 +28,9 @@ export class LambdaFunction implements events.IRuleTarget { * Returns a RuleTarget that can be used to trigger this Lambda as a * result from a CloudWatch event. */ - public bind(rule: events.IRule): events.RuleTargetConfig { + public bind(rule: events.IRule, _id: string): events.RuleTargetConfig { const permissionId = `AllowEventRule${rule.node.uniqueId}`; - if (!this.handler.node.tryFindChild(permissionId)) { + if (!this.handler.permissionsNode.tryFindChild(permissionId)) { this.handler.addPermission(permissionId, { action: 'lambda:InvokeFunction', principal: new iam.ServicePrincipal('events.amazonaws.com'), @@ -39,7 +39,6 @@ export class LambdaFunction implements events.IRuleTarget { } return { - id: this.handler.node.uniqueId, arn: this.handler.functionArn, input: this.props.event, }; diff --git a/packages/@aws-cdk/aws-events-targets/lib/sns.ts b/packages/@aws-cdk/aws-events-targets/lib/sns.ts index 395c1fe2121e2..3d8c937f1cb16 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/sns.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/sns.ts @@ -34,12 +34,11 @@ export class SnsTopic implements events.IRuleTarget { * * @see https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/resource-based-policies-cwe.html#sns-permissions */ - public bind(_rule: events.IRule): events.RuleTargetConfig { + public bind(_rule: events.IRule, _id: string): events.RuleTargetConfig { // deduplicated automatically this.topic.grantPublish(new iam.ServicePrincipal('events.amazonaws.com')); return { - id: this.topic.node.uniqueId, arn: this.topic.topicArn, input: this.props.message, }; diff --git a/packages/@aws-cdk/aws-events-targets/lib/sqs.ts b/packages/@aws-cdk/aws-events-targets/lib/sqs.ts index 0c97cc05b3ed1..bf5c4650c8936 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/sqs.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/sqs.ts @@ -48,7 +48,7 @@ export class SqsQueue implements events.IRuleTarget { * * @see https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/resource-based-policies-cwe.html#sqs-permissions */ - public bind(rule: events.IRule): events.RuleTargetConfig { + public bind(rule: events.IRule, _id: string): events.RuleTargetConfig { // deduplicated automatically this.queue.grantSendMessages(new iam.ServicePrincipal('events.amazonaws.com', { @@ -59,7 +59,6 @@ export class SqsQueue implements events.IRuleTarget { ); const result = { - id: this.queue.node.uniqueId, arn: this.queue.queueArn, input: this.props.message, }; diff --git a/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts b/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts index 21d8040c7111e..dc957bea2f23a 100644 --- a/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts +++ b/packages/@aws-cdk/aws-events-targets/lib/state-machine.ts @@ -27,9 +27,8 @@ export class SfnStateMachine implements events.IRuleTarget { * * @see https://docs.aws.amazon.com/AmazonCloudWatch/latest/events/resource-based-policies-cwe.html#sns-permissions */ - public bind(_rule: events.IRule): events.RuleTargetConfig { + public bind(_rule: events.IRule, _id: string): events.RuleTargetConfig { return { - id: this.machine.node.uniqueId, arn: this.machine.stateMachineArn, role: singletonEventRole(this.machine, [new iam.PolicyStatement({ actions: ['states:StartExecution'], diff --git a/packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts b/packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts index 0d5d370999415..fd9eecc4beff9 100644 --- a/packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/codebuild/codebuild.test.ts @@ -25,7 +25,7 @@ test('use codebuild project as an eventrule target', () => { "Arn" ] }, - Id: "MyProject", + Id: "Target0", RoleArn: { "Fn::GetAtt": [ "MyProjectEventsRole5B7D93F5", diff --git a/packages/@aws-cdk/aws-events-targets/test/codebuild/integ.project-events.expected.json b/packages/@aws-cdk/aws-events-targets/test/codebuild/integ.project-events.expected.json index 7a08a5660234b..3943db5cdaf0d 100644 --- a/packages/@aws-cdk/aws-events-targets/test/codebuild/integ.project-events.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/codebuild/integ.project-events.expected.json @@ -44,7 +44,7 @@ "Arn" ] }, - "Id": "awscdkcodebuildeventsMyProjectEF919B0E", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "MyProjectEventsRole5B7D93F5", @@ -56,7 +56,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdkcodebuildeventsMyTopic550011DC", + "Id": "Target1", "InputTransformer": { "InputPathsMap": { "detail-repositoryName": "$.detail.repositoryName", @@ -231,7 +231,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdkcodebuildeventsMyTopic550011DC" + "Id": "Target0" } ] } @@ -260,7 +260,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdkcodebuildeventsMyTopic550011DC", + "Id": "Target0", "InputTransformer": { "InputPathsMap": { "detail-completed-phase": "$.detail.completed-phase" @@ -417,4 +417,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json b/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json index b993508c46936..2b8e1e33bee36 100644 --- a/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json @@ -461,7 +461,7 @@ ] ] }, - "Id": "pipelineeventspipelinePipeline22F2A91DFBB66895", + "Id": "Target0", "RoleArn": { "Fn::GetAtt": [ "pipelinePipeline22F2A91DEventsRole048D7F59", @@ -473,4 +473,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts b/packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts index f56c513df9618..62baaca9d2114 100644 --- a/packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/codepipeline/pipeline.test.ts @@ -56,7 +56,7 @@ test('use codebuild project as an eventrule target', () => { Targets: [ { Arn: pipelineArn, - Id: "Pipeline", + Id: "Target0", RoleArn: { "Fn::GetAtt": [ "PipelineEventsRole46BEEA7C", "Arn" ] } } ] diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts b/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts index f0feb5a4d83fa..15f9568112823 100644 --- a/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/ecs/event-rule-target.test.ts @@ -51,7 +51,7 @@ test("Can use EC2 taskdef as EventRule target", () => { InputTemplate: "{\"containerOverrides\":[{\"name\":\"TheContainer\",\"command\":[\"echo\",]}]}" }, RoleArn: { "Fn::GetAtt": ["TaskDefEventsRoleFB3B67B8", "Arn"] }, - Id: taskDefinition.node.uniqueId + Id: "Target0" } ] }); @@ -124,7 +124,7 @@ test("Can use Fargate taskdef as EventRule target", () => { "Arn" ] }, - Id: taskDefinition.node.uniqueId, + Id: "Target0", EcsParameters: { TaskDefinitionArn: { Ref: "TaskDef54694570" @@ -180,7 +180,7 @@ test("Can use Fargate taskdef as EventRule target", () => { InputTemplate: "{\"containerOverrides\":[{\"name\":\"TheContainer\",\"command\":[\"echo\",]}]}" }, RoleArn: { "Fn::GetAtt": ["TaskDefEventsRoleFB3B67B8", "Arn"] }, - Id: taskDefinition.node.uniqueId + Id: "Target0" } ] }); diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json index 7816a7c459ca5..9ea9c89311466 100644 --- a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json @@ -1162,7 +1162,7 @@ "Ref": "TaskDef54694570" } }, - "Id": "awsecsintegecsTaskDef8DD0C801", + "Id": "Target0", "Input": "{\"containerOverrides\":[{\"name\":\"TheContainer\",\"environment\":[{\"name\":\"I_WAS_TRIGGERED\",\"value\":\"From CloudWatch Events\"}]}]}", "RoleArn": { "Fn::GetAtt": [ @@ -1197,4 +1197,4 @@ "Description": "Artifact hash for asset \"aws-ecs-integ-ecs/AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62c/Code\"" } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-fargate-task.expected.json b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-fargate-task.expected.json index 654ac00374d93..4e99bb13e1a12 100644 --- a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-fargate-task.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-fargate-task.expected.json @@ -638,7 +638,7 @@ "Arn" ] }, - "Id": "awsecsintegfargateTaskDef8878AF94", + "Id": "Target0", "EcsParameters": { "TaskDefinitionArn": { "Ref": "TaskDef54694570" @@ -715,7 +715,7 @@ "Arn" ] }, - "Id": "awsecsintegfargateTaskDef8878AF94", + "Id": "Target0", "EcsParameters": { "TaskDefinitionArn": { "Ref": "TaskDef54694570" @@ -935,7 +935,7 @@ "Ref": "TaskDef54694570" } }, - "Id": "awsecsintegfargateTaskDef8878AF94", + "Id": "Target0", "Input": "{\"containerOverrides\":[{\"name\":\"TheContainer\",\"environment\":[{\"name\":\"I_WAS_TRIGGERED\",\"value\":\"From CloudWatch Events\"}]}]}", "RoleArn": { "Fn::GetAtt": [ diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json index 3d99700f4739d..e2b00b5ad3537 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/integ.events.expected.json @@ -111,7 +111,7 @@ "Arn" ] }, - "Id": "lambdaeventsMyFunc910E580F" + "Id": "Target0" } ] } @@ -129,10 +129,10 @@ "Arn" ] }, - "Id": "lambdaeventsMyFunc910E580F" + "Id": "Target0" } ] } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts index b2696f39e486d..782322f20c9be 100644 --- a/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/lambda/lambda.test.ts @@ -51,12 +51,57 @@ test('use lambda as an event rule target', () => { Targets: [ { Arn: { "Fn::GetAtt": [lambdaId, "Arn"] }, - Id: "MyLambda" + Id: "Target0" } ] })); }); +test('adding same lambda function as target mutiple times creates permission only once', () => { + // GIVEN + const stack = new cdk.Stack(); + const fn = newTestLambda(stack); + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + }); + + // WHEN + rule.addTarget(new targets.LambdaFunction(fn, { + event: events.RuleTargetInput.fromObject({ key: 'value1' }) + })); + rule.addTarget(new targets.LambdaFunction(fn, { + event: events.RuleTargetInput.fromObject({ key: 'value2' }) + })); + + // THEN + expect(stack).to(countResources('AWS::Lambda::Permission', 1)); +}); + +test('adding same singleton lambda function as target mutiple times creates permission only once', () => { + // GIVEN + const stack = new cdk.Stack(); + const fn = new lambda.SingletonFunction(stack, 'MyLambda', { + code: new lambda.InlineCode('foo'), + handler: 'bar', + runtime: lambda.Runtime.PYTHON_2_7, + uuid: 'uuid' + }); + const rule = new events.Rule(stack, 'Rule', { + schedule: events.Schedule.rate(cdk.Duration.minutes(1)), + }); + + // WHEN + rule.addTarget(new targets.LambdaFunction(fn, { + event: events.RuleTargetInput.fromObject({ key: 'value1' }) + })); + rule.addTarget(new targets.LambdaFunction(fn, { + event: events.RuleTargetInput.fromObject({ key: 'value2' }) + })); + + // THEN + expect(stack).to(countResources('AWS::Lambda::Permission', 1)); +}); + function newTestLambda(scope: cdk.Construct) { return new lambda.Function(scope, 'MyLambda', { code: new lambda.InlineCode('foo'), diff --git a/packages/@aws-cdk/aws-events-targets/test/sns/integ.sns-event-rule-target.expected.json b/packages/@aws-cdk/aws-events-targets/test/sns/integ.sns-event-rule-target.expected.json index ea44e7de6f50d..f1c2724a24a6f 100644 --- a/packages/@aws-cdk/aws-events-targets/test/sns/integ.sns-event-rule-target.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/sns/integ.sns-event-rule-target.expected.json @@ -49,7 +49,7 @@ "Arn": { "Ref": "MyTopic86869434" }, - "Id": "awscdksnseventtargetMyTopicB7575CD8" + "Id": "Target0" } ] } @@ -108,4 +108,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-events-targets/test/sns/sns.test.ts b/packages/@aws-cdk/aws-events-targets/test/sns/sns.test.ts index cadf5eb2eb442..4f6fbc20ceb14 100644 --- a/packages/@aws-cdk/aws-events-targets/test/sns/sns.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/sns/sns.test.ts @@ -38,7 +38,7 @@ test('sns topic as an event rule target', () => { Targets: [ { Arn: { Ref: "MyTopic86869434" }, - Id: "MyTopic" + Id: "Target0" } ] })); diff --git a/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json b/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json index b56bcd5f6bfe0..4fd7303901d20 100644 --- a/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/sqs/integ.sqs-event-rule-target.expected.json @@ -13,7 +13,7 @@ "Arn" ] }, - "Id": "awscdksqseventtargetMyQueueF7EBF3AE" + "Id": "Target0" } ] } @@ -74,4 +74,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts b/packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts index 31dc30cfa4ef0..eb12e2f471b66 100644 --- a/packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts +++ b/packages/@aws-cdk/aws-events-targets/test/sqs/sqs.test.ts @@ -61,7 +61,7 @@ test('sns topic as an event rule target', () => { "Arn" ] }, - Id: "MyQueue" + Id: "Target0" } ] })); diff --git a/packages/@aws-cdk/aws-events/lib/rule.ts b/packages/@aws-cdk/aws-events/lib/rule.ts index 2834dc70998d3..4dc91900738cf 100644 --- a/packages/@aws-cdk/aws-events/lib/rule.ts +++ b/packages/@aws-cdk/aws-events/lib/rule.ts @@ -127,14 +127,11 @@ export class Rule extends Resource implements IRule { public addTarget(target?: IRuleTarget) { if (!target) { return; } - const targetProps = target.bind(this); - const id = sanitizeId(targetProps.id); - const inputProps = targetProps.input && targetProps.input.bind(this); + // Simply increment id for each `addTarget` call. This is guaranteed to be unique. + const id = `Target${this.targets.length}`; - // check if a target with this ID already exists - if (this.targets.find(t => t.id === id)) { - throw new Error('Duplicate event rule target with ID: ' + id); - } + const targetProps = target.bind(this, id); + const inputProps = targetProps.input && targetProps.input.bind(this); const roleArn = targetProps.role ? targetProps.role.roleArn : undefined; @@ -230,14 +227,3 @@ export class Rule extends Resource implements IRule { return out; } } - -/** - * Sanitize whatever is returned to make a valid ID - * - * Result must match regex [\.\-_A-Za-z0-9]+ - */ -function sanitizeId(id: string) { - const _id = id.replace(/[^\.\-_A-Za-z0-9]/g, '-'); - // cut to 64 chars to respect AWS::Events::Rule Target Id field specification - return _id.substring(Math.max(_id.length - 64, 0), _id.length); -} diff --git a/packages/@aws-cdk/aws-events/lib/target.ts b/packages/@aws-cdk/aws-events/lib/target.ts index 4c5fe79e708ab..8f88718d44ce5 100644 --- a/packages/@aws-cdk/aws-events/lib/target.ts +++ b/packages/@aws-cdk/aws-events/lib/target.ts @@ -12,21 +12,15 @@ export interface IRuleTarget { * NOTE: Do not use the various `inputXxx` options. They can be set in a call to `addTarget`. * * @param rule The CloudWatch Event Rule that would trigger this target. + * @param id The id of the target that wil be attached to the rule. */ - bind(rule: IRule): RuleTargetConfig; + bind(rule: IRule, id: string): RuleTargetConfig; } /** * Properties for an event rule target */ export interface RuleTargetConfig { - /** - * A unique, user-defined identifier for the target. Acceptable values - * include alphanumeric characters, periods (.), hyphens (-), and - * underscores (_). - */ - readonly id: string; - /** * The Amazon Resource Name (ARN) of the target. */ diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 4615ecc239b30..623fabb68820a 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -1,7 +1,7 @@ import cloudwatch = require('@aws-cdk/aws-cloudwatch'); import ec2 = require('@aws-cdk/aws-ec2'); import iam = require('@aws-cdk/aws-iam'); -import { IResource, Resource } from '@aws-cdk/core'; +import { ConstructNode, IResource, Resource } from '@aws-cdk/core'; import { IEventSource } from './event-source'; import { EventSourceMapping, EventSourceMappingOptions } from './event-source-mapping'; import { IVersion } from './lambda-version'; @@ -41,6 +41,11 @@ export interface IFunction extends IResource, ec2.IConnectable, iam.IGrantable { */ readonly latestVersion: IVersion; + /** + * The construct node where permissions are attached. + */ + readonly permissionsNode: ConstructNode; + /** * Adds an event source that maps to this AWS Lambda function. * @param id construct ID @@ -140,6 +145,11 @@ export abstract class FunctionBase extends Resource implements IFunction { */ public abstract readonly role?: iam.IRole; + /** + * The construct node where permissions are attached. + */ + public abstract readonly permissionsNode: ConstructNode; + /** * Whether the addPermission() call adds any permissions * @@ -282,6 +292,7 @@ export abstract class FunctionBase extends Resource implements IFunction { export abstract class QualifiedFunctionBase extends FunctionBase { public abstract readonly lambda: IFunction; + public readonly permissionsNode = this.node; public get latestVersion() { return this.lambda.latestVersion; @@ -294,6 +305,7 @@ export abstract class QualifiedFunctionBase extends FunctionBase { class LatestVersion extends FunctionBase implements IVersion { public readonly lambda: IFunction; public readonly version = '$LATEST'; + public readonly permissionsNode = this.node; protected readonly canCreatePermissions = true; diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index f31e3142b9435..0236d4fb54047 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -234,7 +234,6 @@ export interface FunctionProps { * library. */ export class Function extends FunctionBase { - public static fromFunctionArn(scope: Construct, id: string, functionArn: string): IFunction { return Function.fromFunctionAttributes(scope, id, { functionArn }); } @@ -259,6 +258,7 @@ export class Function extends FunctionBase { public readonly functionArn = functionArn; public readonly grantPrincipal: iam.IPrincipal; public readonly role = role; + public readonly permissionsNode = this.node; protected readonly canCreatePermissions = false; @@ -375,6 +375,8 @@ export class Function extends FunctionBase { */ public readonly grantPrincipal: iam.IPrincipal; + public readonly permissionsNode = this.node; + protected readonly canCreatePermissions = true; private readonly layers: ILayerVersion[] = []; diff --git a/packages/@aws-cdk/aws-lambda/lib/singleton-lambda.ts b/packages/@aws-cdk/aws-lambda/lib/singleton-lambda.ts index 936a441f49bc7..a86c5e937270b 100644 --- a/packages/@aws-cdk/aws-lambda/lib/singleton-lambda.ts +++ b/packages/@aws-cdk/aws-lambda/lib/singleton-lambda.ts @@ -42,6 +42,7 @@ export class SingletonFunction extends FunctionBase { public readonly functionName: string; public readonly functionArn: string; public readonly role?: iam.IRole; + public readonly permissionsNode: cdk.ConstructNode; protected readonly canCreatePermissions: boolean; private lambdaFunction: IFunction; @@ -49,6 +50,7 @@ export class SingletonFunction extends FunctionBase { super(scope, id); this.lambdaFunction = this.ensureLambda(props); + this.permissionsNode = this.lambdaFunction.node; this.functionArn = this.lambdaFunction.functionArn; this.functionName = this.lambdaFunction.functionName; diff --git a/packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json b/packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json index 94fc189ac129a..204b222421d16 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json +++ b/packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json @@ -884,7 +884,7 @@ "Arn" ] }, - "Id": "awscdkrdsinstanceFunctionD515EE19" + "Id": "Target0" } ] } diff --git a/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts b/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts index 47034c43f4a90..634212fe60a89 100644 --- a/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts +++ b/packages/@aws-cdk/aws-ses/lib/receipt-rule-action.ts @@ -276,7 +276,7 @@ export class ReceiptRuleLambdaAction implements IReceiptRuleAction { // Allow SES to invoke Lambda function // See https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-permissions.html#receiving-email-permissions-lambda const permissionId = 'AllowSes'; - if (!this.props.function.node.tryFindChild(permissionId)) { + if (!this.props.function.permissionsNode.tryFindChild(permissionId)) { this.props.function.addPermission(permissionId, { action: 'lambda:InvokeFunction', principal: new iam.ServicePrincipal('ses.amazonaws.com'), diff --git a/packages/decdk/test/__snapshots__/synth.test.js.snap b/packages/decdk/test/__snapshots__/synth.test.js.snap index cb21a35759b02..9b1a900e6fd3a 100644 --- a/packages/decdk/test/__snapshots__/synth.test.js.snap +++ b/packages/decdk/test/__snapshots__/synth.test.js.snap @@ -2836,7 +2836,7 @@ Object { ], ], }, - "Id": "pipelinePipeline22F2A91D", + "Id": "Target0", "RoleArn": Object { "Fn::GetAtt": Array [ "PipelineEventsRole46BEEA7C",