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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permission check in aws-events-targets does not work with singleton functions #3173

Closed
1 task done
jogold opened this issue Jul 2, 2019 · 0 comments 路 Fixed by #3187 or #3353
Closed
1 task done

Permission check in aws-events-targets does not work with singleton functions #3173

jogold opened this issue Jul 2, 2019 · 0 comments 路 Fixed by #3187 or #3353
Labels
needs-triage This issue or PR still needs to be triaged.

Comments

@jogold
Copy link
Contributor

jogold commented Jul 2, 2019

  • I'm submitting a ...

    • 馃 bug report
  • What is the current behavior?

When working with singleton functions as rule targets, the following check (L33) will always return true (this.handler.node has no children for singleton functions):
https://github.com/awslabs/aws-cdk/blob/cd1b16f42abca205f3ab6ab1fafd92a4d043a3e3/packages/%40aws-cdk/aws-events-targets/lib/lambda.ts#L27-L39

This is because singleton functions create a new function attached on the stack and add permissions to this created function:
https://github.com/awslabs/aws-cdk/blob/cd1b16f42abca205f3ab6ab1fafd92a4d043a3e3/packages/%40aws-cdk/aws-lambda/lib/singleton-lambda.ts#L61-L74

This results in There is already a Construct with name ... error messages.

  • What is the expected behavior (or behavior of feature suggested)?
    Permission constructs with same ids should not be created.

  • What is the motivation / use case for changing the behavior or adding this feature?
    It's a common case to have singleton functions as targets for events

  • Please tell us about your environment:

    • CDK CLI Version: 0.36.0
    • Module Version: 0.36.0
    • OS: all
    • Language: all
@jogold jogold added the needs-triage This issue or PR still needs to be triaged. label Jul 2, 2019
jogold added a commit to jogold/aws-cdk that referenced this issue Jul 3, 2019
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 added a commit to jogold/aws-cdk that referenced this issue Jul 3, 2019
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 added a commit to jogold/aws-cdk that referenced this issue Jul 3, 2019
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 added a commit to jogold/aws-cdk that referenced this issue Jul 3, 2019
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
rix0rrr pushed a commit that referenced this issue Jul 19, 2019
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
skinny85 pushed a commit that referenced this issue Jul 19, 2019
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
skinny85 pushed a commit that referenced this issue Jul 19, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage This issue or PR still needs to be triaged.
Projects
None yet
1 participant