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 aws#3173
jogold · Jul 3, 2019 · e1179cc · e1179cc
1 parent 4667b60
commit e1179cc
Show file tree

Hide file tree

Showing 38 changed files with 126 additions and 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
@@ -37,7 +37,7 @@
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic"
+            "Id": "Target0"
           }
         ]
       }
@@ -46,4 +46,4 @@
       "Type": "AWS::SNS::Topic"
     }
   }
-}
+}
diff --git a/...ws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json b/...ws-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
@@ -635,7 +635,7 @@
                 ]
               ]
             },
-            "Id": "awscdkcodepipelinelambdaPipeline87A4B3D3",
+            "Id": "Target0",
             "RoleArn": {
               "Fn::GetAtt": [
                 "PipelineEventsRole46BEEA7C",

diff --git a/...codepipeline-actions/test/integ.pipeline-code-build-multiple-inputs-outputs.expected.json b/...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
@@ -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
@@ -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
@@ -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
@@ -206,7 +206,7 @@
             "Arn": {
               "Ref": "ComplianceTopic0229448B"
             },
-            "Id": "awscdkconfigruleintegComplianceTopic55CAF01A"
+            "Id": "Target0"
           }
         ]
       }
@@ -251,4 +251,4 @@
       }
     }
   }
-}
+}
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
@@ -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"
     }
   }
-}
+}
diff --git a/...ges/@aws-cdk/aws-ecs-patterns/test/fargate/integ.scheduled-fargate-task.lit.expected.json b/...ges/@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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 @@
       }
     }
   }
-}
+}
diff --git a/...s/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json b/...s/@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 @@
       }
     }
   }
-}
+}
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
@@ -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
@@ -51,7 +51,7 @@ test("Can use EC2 taskdef as EventRule target", () => {
           InputTemplate: "{\"containerOverrides\":[{\"name\":\"TheContainer\",\"command\":[\"echo\",<detail-event>]}]}"
         },
         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\",<detail-event>]}]}"
         },
         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
@@ -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\""
     }
   }
-}
+}