fix(events-targets): event targets can have the same construct id (#2744

) Extend the Target for Event Rule Lambda cannot have same construct ID Two supported Event Targets resources with the same Construct ID but in different scopes cannot both be used as targets to the same EventRule: ``` Error: Duplicate event rule target with ID: Resource at EventRule.addTarget (.../node_modules/@aws-cdk/aws-events/lib/rule.js:57:19) at new EventRule (.../node_modules/@aws-cdk/aws-events/lib/rule.js:26:18) ``` Assuming a Lambda Function as a RuleTarget, the issue is that Function's asEventRuleTarget uses `node.id` as the id field for the EventRuleTargetProps ([source](https://github.com/awslabs/aws-cdk/blob/48d536b191fb205869ef6724b718540be1037135/packages/%40aws-cdk/aws-events-targets/lib/lambda.ts#L32)). This isn't unique, as it's just the ID that was passed when creating the Function construct - instead, it should use `node.uniqueId` ([source](https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/cdk/lib/construct.ts#L31-L36)). This needs to be globally unique as when calling EventRule.addTarget this value is used as a unique identifier ([source](https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-events/lib/rule.ts#L124-L126)). I fix the bug as suggested by @robwettach in the package `@aws-cdk/aws-events` by using 64-maximum chars taken from the `uniqueId` - that is unique across the same app. This changes affects every binding in the supported event targets ([source](https://github.com/made2591/aws-cdk/blob/feature/aws-events-targets-sqs/packages/%40aws-cdk/aws-events-targets/lib/)). Close #2377
aws · Jun 17, 2019 · 210dd0f · 210dd0f
1 parent 1a386d8
commit 210dd0f
Show file tree

Hide file tree

Showing 28 changed files with 227 additions and 202 deletions.
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
@@ -679,7 +679,7 @@
                 ]
               ]
             },
-            "Id": "Pipeline",
+            "Id": "PipelineStackPipeline9DB740AF",
             "RoleArn": {
               "Fn::GetAtt": [
                 "PipelineEventsRole46BEEA7C",
@@ -751,7 +751,7 @@
                 ]
               ]
             },
-            "Id": "Pipeline",
+            "Id": "PipelineStackPipeline9DB740AF",
             "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
@@ -444,7 +444,7 @@
                 ]
               ]
             },
-            "Id": "Pipeline",
+            "Id": "awscdkcodepipelinelambdaPipeline87A4B3D3",
             "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": "Pipeline",
+            "Id": "awscdkcodepipelinecodebuildmultipleinputsoutputsPipeline314D3A85",
             "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": "Pipeline",
+            "Id": "awscdkcodepipelinecodecommitPipelineF780CA18",
             "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
@@ -277,7 +277,7 @@
                 ]
               ]
             },
-            "Id": "MyPipeline",
+            "Id": "awscdkcodepipelineecrsourceMyPipeline63CF3194",
             "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
@@ -374,7 +374,7 @@
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic"
+            "Id": "awscdkpipelineeventtargetMyTopic8D32776A"
           }
         ]
       }
@@ -426,7 +426,7 @@
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic"
+            "Id": "awscdkpipelineeventtargetMyTopic8D32776A"
           }
         ]
       }
@@ -473,7 +473,7 @@
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic",
+            "Id": "awscdkpipelineeventtargetMyTopic8D32776A",
             "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": "ComplianceTopic"
+            "Id": "awscdkconfigruleintegComplianceTopic55CAF01A"
           }
         ]
       }

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
@@ -847,7 +847,7 @@
                 "Ref": "ScheduledEc2TaskScheduledTaskDef56328BA4"
               }
             },
-            "Id": "ScheduledTaskDef-on-EcsCluster",
+            "Id": "awsecsintegecsScheduledEc2TaskScheduledTaskDef18FB4348",
             "Input": "{}",
             "RoleArn": {
               "Fn::GetAtt": [

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.scheduled-ecs-task.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.scheduled-ecs-task.ts
@@ -31,7 +31,7 @@ export = {
             TaskCount: 1,
             TaskDefinitionArn: { Ref: "ScheduledEc2TaskScheduledTaskDef56328BA4" }
           },
-          Id: "ScheduledTaskDef-on-EcsCluster",
+          Id: "ScheduledEc2TaskScheduledTaskDef1EA607E3",
           Input: "{}",
           RoleArn: { "Fn::GetAtt": ["ScheduledEc2TaskScheduledTaskDefEventsRole64113C5F", "Arn"] }
         }
@@ -97,7 +97,7 @@ export = {
             TaskCount: 2,
             TaskDefinitionArn: { Ref: "ScheduledEc2TaskScheduledTaskDef56328BA4" }
           },
-          Id: "ScheduledTaskDef-on-EcsCluster",
+          Id: "ScheduledEc2TaskScheduledTaskDef1EA607E3",
           Input: "{}",
           RoleArn: { "Fn::GetAtt": ["ScheduledEc2TaskScheduledTaskDefEventsRole64113C5F", "Arn"] }
         }

diff --git a/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts b/packages/@aws-cdk/aws-events-targets/lib/codebuild.ts
@@ -15,7 +15,7 @@ export class CodeBuildProject implements events.IRuleTarget {
    */
   public bind(_rule: events.IRule): events.RuleTargetConfig {
     return {
-      id: this.project.node.id,
+      id: this.project.node.uniqueId,
       arn: this.project.projectArn,
       role: singletonEventRole(this.project, [new iam.PolicyStatement()
         .addAction('codebuild:StartBuild')

diff --git a/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts b/packages/@aws-cdk/aws-events-targets/lib/codepipeline.ts
@@ -12,7 +12,7 @@ export class CodePipeline implements events.IRuleTarget {
 
   public bind(_rule: events.IRule): events.RuleTargetConfig {
     return {
-      id: this.pipeline.node.id,
+      id: this.pipeline.node.uniqueId,
       arn: this.pipeline.pipelineArn,
       role: singletonEventRole(this.pipeline, [new iam.PolicyStatement()
         .addResource(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
@@ -99,7 +99,7 @@ export class EcsTask implements events.IRuleTarget {
         .addResource(this.taskDefinition.taskRole.roleArn));
     }
 
-    const id = this.taskDefinition.node.id + '-on-' + this.cluster.node.id;
+    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

diff --git a/packages/@aws-cdk/aws-events-targets/lib/lambda.ts b/packages/@aws-cdk/aws-events-targets/lib/lambda.ts
@@ -39,7 +39,7 @@ export class LambdaFunction implements events.IRuleTarget {
     }
 
     return {
-      id: this.handler.node.id,
+      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
@@ -39,7 +39,7 @@ export class SnsTopic implements events.IRuleTarget {
     this.topic.grantPublish(new iam.ServicePrincipal('events.amazonaws.com'));
 
     return {
-      id: this.topic.node.id,
+      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
@@ -59,7 +59,7 @@ export class SqsQueue implements events.IRuleTarget {
     );
 
     const result = {
-      id: this.queue.node.id,
+      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
@@ -29,7 +29,7 @@ export class SfnStateMachine implements events.IRuleTarget {
    */
   public bind(_rule: events.IRule): events.RuleTargetConfig {
     return {
-        id: this.machine.node.id,
+        id: this.machine.node.uniqueId,
         arn: this.machine.stateMachineArn,
         role: singletonEventRole(this.machine, [new iam.PolicyStatement()
             .addAction('states:StartExecution')

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": "MyProject",
+            "Id": "awscdkcodebuildeventsMyProjectEF919B0E",
             "RoleArn": {
               "Fn::GetAtt": [
                 "MyProjectEventsRole5B7D93F5",
@@ -56,7 +56,7 @@
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic",
+            "Id": "awscdkcodebuildeventsMyTopic550011DC",
             "InputTransformer": {
               "InputPathsMap": {
                 "detail-repositoryName": "$.detail.repositoryName",
@@ -68,59 +68,6 @@
         ]
       }
     },
-    "MyProjectEventsRole5B7D93F5": {
-      "Type": "AWS::IAM::Role",
-      "Properties": {
-        "AssumeRolePolicyDocument": {
-          "Statement": [
-            {
-              "Action": "sts:AssumeRole",
-              "Effect": "Allow",
-              "Principal": {
-                "Service": {
-                  "Fn::Join": [
-                    "",
-                    [
-                      "events.",
-                      {
-                        "Ref": "AWS::URLSuffix"
-                      }
-                    ]
-                  ]
-                }
-              }
-            }
-          ],
-          "Version": "2012-10-17"
-        }
-      }
-    },
-    "MyProjectEventsRoleDefaultPolicy397DCBF8": {
-      "Type": "AWS::IAM::Policy",
-      "Properties": {
-        "PolicyDocument": {
-          "Statement": [
-            {
-              "Action": "codebuild:StartBuild",
-              "Effect": "Allow",
-              "Resource": {
-                "Fn::GetAtt": [
-                  "MyProject39F7B0AE",
-                  "Arn"
-                ]
-              }
-            }
-          ],
-          "Version": "2012-10-17"
-        },
-        "PolicyName": "MyProjectEventsRoleDefaultPolicy397DCBF8",
-        "Roles": [
-          {
-            "Ref": "MyProjectEventsRole5B7D93F5"
-          }
-        ]
-      }
-    },
     "MyProjectRole9BBE5233": {
       "Type": "AWS::IAM::Role",
       "Properties": {
@@ -267,24 +214,24 @@
           "source": [
             "aws.codebuild"
           ],
-          "detail-type": [
-            "CodeBuild Build State Change"
-          ],
           "detail": {
             "project-name": [
               {
                 "Ref": "MyProject39F7B0AE"
               }
             ]
-          }
+          },
+          "detail-type": [
+            "CodeBuild Build State Change"
+          ]
         },
         "State": "ENABLED",
         "Targets": [
           {
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic"
+            "Id": "awscdkcodebuildeventsMyTopic550011DC"
           }
         ]
       }
@@ -296,24 +243,24 @@
           "source": [
             "aws.codebuild"
           ],
-          "detail-type": [
-            "CodeBuild Build Phase Change"
-          ],
           "detail": {
             "project-name": [
               {
                 "Ref": "MyProject39F7B0AE"
               }
             ]
-          }
+          },
+          "detail-type": [
+            "CodeBuild Build Phase Change"
+          ]
         },
         "State": "ENABLED",
         "Targets": [
           {
             "Arn": {
               "Ref": "MyTopic86869434"
             },
-            "Id": "MyTopic",
+            "Id": "awscdkcodebuildeventsMyTopic550011DC",
             "InputTransformer": {
               "InputPathsMap": {
                 "detail-completed-phase": "$.detail.completed-phase"
@@ -324,6 +271,59 @@
         ]
       }
     },
+    "MyProjectEventsRole5B7D93F5": {
+      "Type": "AWS::IAM::Role",
+      "Properties": {
+        "AssumeRolePolicyDocument": {
+          "Statement": [
+            {
+              "Action": "sts:AssumeRole",
+              "Effect": "Allow",
+              "Principal": {
+                "Service": {
+                  "Fn::Join": [
+                    "",
+                    [
+                      "events.",
+                      {
+                        "Ref": "AWS::URLSuffix"
+                      }
+                    ]
+                  ]
+                }
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        }
+      }
+    },
+    "MyProjectEventsRoleDefaultPolicy397DCBF8": {
+      "Type": "AWS::IAM::Policy",
+      "Properties": {
+        "PolicyDocument": {
+          "Statement": [
+            {
+              "Action": "codebuild:StartBuild",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::GetAtt": [
+                  "MyProject39F7B0AE",
+                  "Arn"
+                ]
+              }
+            }
+          ],
+          "Version": "2012-10-17"
+        },
+        "PolicyName": "MyProjectEventsRoleDefaultPolicy397DCBF8",
+        "Roles": [
+          {
+            "Ref": "MyProjectEventsRole5B7D93F5"
+          }
+        ]
+      }
+    },
     "MyQueueE6CA6235": {
       "Type": "AWS::SQS::Queue"
     },
@@ -417,4 +417,4 @@
       }
     }
   }
-}
+}