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

(aws-scheduler-targets-alpha): Add EventBridgePutEvents Target #27454

Closed
1 of 2 tasks
filletofish opened this issue Oct 9, 2023 · 6 comments · Fixed by #27629
Closed
1 of 2 tasks

(aws-scheduler-targets-alpha): Add EventBridgePutEvents Target #27454

filletofish opened this issue Oct 9, 2023 · 6 comments · Fixed by #27629
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@filletofish
Copy link
Contributor

Describe the feature

Work to support L2 constructs for AWS Scheduler is in progress (#23394). See the approved RFC. RFC planned to add 12 templates targets, but only Lambda Invoke is currently implemented (#26575).

This issue tracks implementation of EventBridgePutEvents target to put an event to AWS Event Bridge Event Bus.

Use Case

Customers would like to use templated target EventBridgePutEvents to be able to put events to Event Bus on schedule. L2 target construct should grant required permissions to the AWS Scheduler to put events to a Event Bus.

Proposed Solution

The proposed solution needs to be adopted to the recent examples of LambdaInvoke (https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-scheduler-targets-alpha/lib/lambda-invoke.ts).

Solution should also include unit and integration tests.

Class EventBridgePutEvents should:

  1. Grant Scheduler Execution Role permissions to Put Events via addTargetActionToRole
  2. Override bindBaseTargetConfig to return eventBridgeParameters as part of ScheduleTargetConfig.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.99.1

Environment details (OS name and version, etc.)

MacOS

@filletofish filletofish added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Oct 9, 2023
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Oct 9, 2023
@pahud
Copy link
Contributor

pahud commented Oct 9, 2023

Thank you for all those feature requests and PRs!

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 9, 2023
@WtfJoke
Copy link
Contributor

WtfJoke commented Oct 19, 2023

Is it fine for you if I pick that?

@WtfJoke
Copy link
Contributor

WtfJoke commented Oct 20, 2023

I hope you dont mind, that I've opened #27629. I couldnt resist 🙈
Would appreciate feedback once you have time :)

@filletofish
Copy link
Contributor Author

@WtfJoke of course. Thank you!

Please, I will make it clear that people should feel free to pick up the issues and create pull requests!

I will try to review the PR soon

@WtfJoke
Copy link
Contributor

WtfJoke commented Oct 22, 2023

@WtfJoke of course. Thank you!

Please, I will make it clear that people should feel free to pick up the issues and create pull requests!

I will try to review the PR soon

Awesome, thank you

@mergify mergify bot closed this as completed in #27629 Nov 29, 2023
mergify bot pushed a commit that referenced this issue Nov 29, 2023
An eventBridgePutEvents target was implemented similar to the already existing LambdaInvoke/StepFunctionStartExecution target.

I needed to change some properties and methods from Target.ts from private to protected so that the logic could be reused (hope that is ok).

Some design choices to outline (let me know if you disagree or have improvements I could take :) ):
1. PutEvents would accept multiple Entries (eg. an array), but I decided to support just one single event, because how Target is currently designed (to support only one target arn). It also aligns with the templated integration in the aws management console.
2. It throws an error in the constructor if the base prop `input` is used. All the props should be delivered by the new `EventBridgePutEventsEntry`. It felt not right for the developer experience to split the object (detail to `input` and `source`, `detailType` to `EventBridgePutEventsEntry` ).


Closes #27454.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

chenjane-dev pushed a commit to chenjane-dev/aws-cdk that referenced this issue Dec 5, 2023
An eventBridgePutEvents target was implemented similar to the already existing LambdaInvoke/StepFunctionStartExecution target.

I needed to change some properties and methods from Target.ts from private to protected so that the logic could be reused (hope that is ok).

Some design choices to outline (let me know if you disagree or have improvements I could take :) ):
1. PutEvents would accept multiple Entries (eg. an array), but I decided to support just one single event, because how Target is currently designed (to support only one target arn). It also aligns with the templated integration in the aws management console.
2. It throws an error in the constructor if the base prop `input` is used. All the props should be delivered by the new `EventBridgePutEventsEntry`. It felt not right for the developer experience to split the object (detail to `input` and `source`, `detailType` to `EventBridgePutEventsEntry` ).


Closes aws#27454.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants