feat(scheduler-targets): step function start execution target #27424

WtfJoke · 2023-10-05T23:21:43Z

A StepFunctionStartExecution ScheduleTarget was implemented similar to the already existing LambdaInvoke target.

I've added an integration test, which will trigger an eventbridge schedule once. This schedule has a step function as a target. The step function creates a parameter with a given value, which will get verified by the test

Closes #27377

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

filletofish

Thank you for the commit! It motivated me to add more issues for Scheduler Targets to get help from the community to build all targets for L2 Scheduler constructs.

filletofish · 2023-10-09T08:29:05Z

packages/cdk-cli-wrapper/.warnings.jsii.js

@@ -0,0 +1,73 @@
+function cdk_cli_wrapper_ICdk(p) {


Looks like cdk-cli-wrapper folder was committed by mistake?

Yeah, I commited this by mistake. Good spot! Thanks

filletofish · 2023-10-09T08:34:49Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/stepfunctions-start-execution.ts

+/**
+ * Use an AWS Step function as a target for AWS EventBridge Scheduler.
+ */
+export class StepFunctionsStartExecution extends ScheduleTargetBase implements IScheduleTarget {


I would recommend to add unit tests for this target too. See https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-scheduler-targets-alpha/test/target.test.ts#L9. We probably need to rename that test file and create one file with unit tests per each target.

I created a new unit test which contains almost identical unit tests for step functions.

I also renamed the existing file target.test.ts into lambda-invoke.test.ts.

WtfJoke · 2023-10-09T21:59:18Z

@filletofish thanks for the review! I'm glad you created more issues for it, I thought about contributing more targets, but have chosen the one I needed the most :)

I've addressed your comments, please have a look on it again.
Thank you!

WtfJoke · 2023-10-09T22:03:59Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/stepfunctions-start-execution.ts

+    const stateMachineEnv = this.stateMachine.env;
+    if (!sameEnvDimension(stateMachineEnv.region, schedule.env.region)) {
+      throw new Error(`Cannot assign stateMachine in region ${stateMachineEnv.region} to the schedule ${Names.nodeUniqueId(schedule.node)} in region ${schedule.env.region}. Both the schedule and the stateMachine must be in the same region.`);
+    }
+
+    if (!sameEnvDimension(stateMachineEnv.account, schedule.env.account)) {
+      throw new Error(`Cannot assign stateMachine in account ${stateMachineEnv.account} to the schedule ${Names.nodeUniqueId(schedule.node)} in account ${schedule.env.region}. Both the schedule and the stateMachine must be in the same account.`);
+    }
+
+    if (this.props.role && !sameEnvDimension(this.props.role.env.account, stateMachineEnv.account)) {
+      throw new Error(`Cannot grant permission to execution role in account ${this.props.role.env.account} to invoke target ${Names.nodeUniqueId(this.stateMachine.node)} in account ${stateMachineEnv.account}. Both the target and the execution role must be in the same account.`);
+    }


It might make sense to extract this common logic block somewhere (since its the same for step functions and lambda and probably some targets more, except the message), do you have a favorite spot for it? Should it go to ScheduleTargetBase, in a new class/function or stay "duplicated"?

@kaizencc do you have any opinion on this? I personally do not mind the duplication of the code

I don't mind it for now, but would be happy with less duplication :). At best we can create an issue for it and be able to point to it in the future if this gets out of hand

packages/@aws-cdk/aws-scheduler-targets-alpha/README.md

kaizencc · 2023-10-11T12:59:03Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/stepfunctions-start-execution.ts

+    const stateMachineEnv = this.stateMachine.env;
+    if (!sameEnvDimension(stateMachineEnv.region, schedule.env.region)) {
+      throw new Error(`Cannot assign stateMachine in region ${stateMachineEnv.region} to the schedule ${Names.nodeUniqueId(schedule.node)} in region ${schedule.env.region}. Both the schedule and the stateMachine must be in the same region.`);
+    }
+
+    if (!sameEnvDimension(stateMachineEnv.account, schedule.env.account)) {
+      throw new Error(`Cannot assign stateMachine in account ${stateMachineEnv.account} to the schedule ${Names.nodeUniqueId(schedule.node)} in account ${schedule.env.region}. Both the schedule and the stateMachine must be in the same account.`);
+    }
+
+    if (this.props.role && !sameEnvDimension(this.props.role.env.account, stateMachineEnv.account)) {
+      throw new Error(`Cannot grant permission to execution role in account ${this.props.role.env.account} to invoke target ${Names.nodeUniqueId(this.stateMachine.node)} in account ${stateMachineEnv.account}. Both the target and the execution role must be in the same account.`);
+    }


I don't mind it for now, but would be happy with less duplication :). At best we can create an issue for it and be able to point to it in the future if this gets out of hand

kaizencc

Thanks for the submission @WtfJoke!

aws-cdk-automation · 2023-10-11T13:33:09Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: d3302f0
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2023-10-11T13:33:37Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation requested a review from a team October 5, 2023 23:21

github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Oct 5, 2023

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 6, 2023

filletofish suggested changes Oct 9, 2023

View reviewed changes

WtfJoke mentioned this pull request Oct 9, 2023

(events): L2 for EventBridge Scheduler #23394

Closed

2 tasks

WtfJoke added 2 commits October 9, 2023 23:53

chore: rename test from target.test.ts to lambda-invoke.test.ts

24416f9

feat(scheduler-targets): Add step function start execution target

ae2e8ae

WtfJoke commented Oct 9, 2023

View reviewed changes

filletofish approved these changes Oct 10, 2023

View reviewed changes

kaizencc reviewed Oct 11, 2023

View reviewed changes

kaizencc added 2 commits October 11, 2023 09:03

Update packages/@aws-cdk/aws-scheduler-targets-alpha/README.md

6062a8d

Merge branch 'main' into scheduler-targets-add-step-functions_27377

d3302f0

kaizencc changed the title ~~feat(scheduler-targets): Add step function start execution target~~ feat(scheduler-targets): step function start execution target Oct 11, 2023

kaizencc approved these changes Oct 11, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 11, 2023

mergify bot merged commit 3a87141 into aws:main Oct 11, 2023
17 checks passed

WtfJoke deleted the scheduler-targets-add-step-functions_27377 branch October 11, 2023 20:04

github-actions bot mentioned this pull request Nov 1, 2023

Monthly PR metrics report - October 2023 #27786

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(scheduler-targets): step function start execution target #27424

feat(scheduler-targets): step function start execution target #27424

WtfJoke commented Oct 5, 2023 •

edited by kaizencc

Loading

filletofish left a comment

filletofish Oct 9, 2023

WtfJoke Oct 9, 2023

filletofish Oct 9, 2023

WtfJoke Oct 9, 2023

WtfJoke commented Oct 9, 2023 •

edited

Loading

WtfJoke Oct 9, 2023 •

edited

Loading

filletofish Oct 10, 2023

kaizencc Oct 11, 2023

kaizencc Oct 11, 2023

kaizencc left a comment

aws-cdk-automation commented Oct 11, 2023

mergify bot commented Oct 11, 2023

feat(scheduler-targets): step function start execution target #27424

feat(scheduler-targets): step function start execution target #27424

Conversation

WtfJoke commented Oct 5, 2023 • edited by kaizencc Loading

filletofish left a comment

Choose a reason for hiding this comment

filletofish Oct 9, 2023

Choose a reason for hiding this comment

WtfJoke Oct 9, 2023

Choose a reason for hiding this comment

filletofish Oct 9, 2023

Choose a reason for hiding this comment

WtfJoke Oct 9, 2023

Choose a reason for hiding this comment

WtfJoke commented Oct 9, 2023 • edited Loading

WtfJoke Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

filletofish Oct 10, 2023

Choose a reason for hiding this comment

kaizencc Oct 11, 2023

Choose a reason for hiding this comment

kaizencc Oct 11, 2023

Choose a reason for hiding this comment

kaizencc left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Oct 11, 2023

AWS CodeBuild CI Report

mergify bot commented Oct 11, 2023

WtfJoke commented Oct 5, 2023 •

edited by kaizencc

Loading

WtfJoke commented Oct 9, 2023 •

edited

Loading

WtfJoke Oct 9, 2023 •

edited

Loading