Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EventRules cannot have targets with the same Construct ID. #2377

Closed
Labels
bug This issue is a bug.

Comments

@robwettach
Copy link

Describe the bug
Two Lambda Functions 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)

The issue is that Function's asEventRuleTarget uses node.id as the id field for the EventRuleTargetProps (source). 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). This needs to be globally unique as when calling EventRule.addTarget this value is used as a unique identifier (source).

To Reproduce

class CustomConstruct extends Construct {
  public readonly lambda: Function;

  constructor(scope: Construct, id: string) {
    super(scope, id);

    this.lambda = new Function(this, "Resource", {
      code: new InlineCode("def handler:\n\tprint(\"Hello\")"),
      handler: "index.handler",
      runtime: Runtime.Python36
    });
  }
}

const construct1 = new CustomConstruct(this, "Construct1");
const construct2 = new CustomConstruct(this, "Construct2");

new EventRule(this, "ConflictingRule", {
  description: "This will fail because the targets both have the same Construct ID",
  scheduleExpression: "rate(1 minute)",
  enabled: true,
  targets: [
    construct1.lambda,
    construct2.lambda
  ]
});

Expected behavior
I can name my constructs in any manner I want, they can still be accepted as distinct EventRule targets without collision between two distinct construct instances.

Version:

  • OS: Mac OS
  • Programming Language: TypeScript
  • CDK Version: 0.26.0
@made2591
Copy link
Contributor

made2591 commented Jun 4, 2019

The bug is still present in version 0.33: fixed in #2744

rix0rrr pushed a commit that referenced this issue Jun 17, 2019
)

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
This was referenced Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment