feat(scheduler-targets): add support for universal target #32341

sakurai-ryo · 2024-12-01T10:52:04Z

Issue # (if applicable)

Reason for this change

EventBridge Scheduler has a mechanism called Universal Target that calls a wide range of AWS APIs.
Supporting this mechanism in L2 Construct will make it easier to configure EventBridge Scheduler.
https://docs.aws.amazon.com/scheduler/latest/UserGuide/managing-targets-universal.html

Description of changes

Added Universal construct targeting AWS APIs.
Users can execute any AWS API by passing service and action to Props.

According to the following documentation, the service must be lowercase, and the action must be camelCase, so that you can validate it.
arn:aws:scheduler:::aws-sdk:service:apiAction
https://docs.aws.amazon.com/scheduler/latest/UserGuide/managing-targets-universal.html#:~:text=schedule%20to%20target.-,Arn,-%E2%80%93%20The%20complete%20service

Description of how you validated changes

Added unit tests and integration tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

codecov · 2024-12-01T11:04:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.41%. Comparing base (f0e2f2a) to head (c49cc2b).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32341   +/-   ##
=======================================
  Coverage   81.41%   81.41%           
=======================================
  Files         223      223           
  Lines       13721    13721           
  Branches     2416     2416           
=======================================
  Hits        11171    11171           
  Misses       2271     2271           
  Partials      279      279

Flag	Coverage Δ
suite.unit	`81.41% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`80.75% <ø> (ø)`
packages/aws-cdk-lib/core	`82.10% <ø> (ø)`

…heduler-universal-target

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/aws-api.ts

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

…heduler-universal-target

sakurai-ryo · 2024-12-26T03:53:21Z

@go-to-k
Thanks for your review.
I addressed all the points you commented.
Cloud you review it again?

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

…ai-ryo/aws-cdk into scheduler-universal-target

go-to-k

LGTM. Thanks for the changes.

samson-keung · 2025-01-08T00:02:53Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

+   * The API action to call.
+   *
+   * You cannot use read-only API actions such as common GET operations.
+   *
+   * Also, This must be in camelCase.


nit:

Suggested change

* The API action to call.

*

* You cannot use read-only API actions such as common GET operations.

*

* Also, This must be in camelCase.

* The API action to call. Must be camelCase.

*

* You cannot use read-only API actions such as common GET operations. See https://docs.aws.amazon.com/scheduler/latest/UserGuide/managing-targets-universal.html#unsupported-api-actions

Thanks, fixed.
ead143b

samson-keung · 2025-01-08T00:03:23Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

+/**
+ * Properties for a Universal Target
+ */
+export interface UniversalProps extends ScheduleTargetBaseProps {


nit:

Suggested change

export interface UniversalProps extends ScheduleTargetBaseProps {

export interface UniversalTargetProps extends ScheduleTargetBaseProps {

fixed.a2d684b

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

samson-keung · 2025-01-08T00:57:56Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

+  /**
+   * The action for the IAM policy statement that will be added to the scheduler role's policy
+   * to allow the scheduler to make the API call.
+   *
+   * Use this in cases where the IAM action name does not match the
+   * API service/action name, e.g., `lambda:invoke` requires `lambda:InvokeFunction` permission.
+   *
+   * @default - service:action
+   */
+  readonly iamAction?: string;


Many AWS operations requires more than one action. For example, S3 PutObject needs more than the s3:PutObject IAM action in certain bucket configuration: https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-with-s3-policy-actions.html#using-with-s3-policy-actions-related-to-objects

In real world scenario, I think most CDK users will need to add more actions than the default of service:action. (<-- This is a guess.) So instead of letting them rely on the default policy then find out it is not enough after deployment, I believe we should make the role prop required for Universal Target in particular. This way CDK will lead the users to consciously determine the required and minimum permissions they need for the Universal Target to work.

Please let me know what you think.

In the ScheduleTargetBase construct, a secure IAM Role is automatically created on behalf of the user, with conditions attached to the trust policy.
Making the role prop required might increase the user's workload and potentially compromise security.

The current props are based on stepfunctions-tasks, and I think they represent a reasonably reliable implementation.
https://github.com/aws/aws-cdk/blob/v2.175.0/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/aws-sdk/call-aws-service.ts

Thanks @sakurai-ryo for the response. I agree with you that having CDK adding the assume-role-policy with secure conditions here is valuable. I am convinced that we should not require CDK users to pass in a role prop.

I spent some time to study the stepfunctions-tasks.CallAwsService class you linked and its history. It originally did not have the additionalPolicyStatements prop. And with the iamX props, it seems to have created confusions because the service:action is not enough permissions for the application to work [1].

I think the best middle ground here is to have the following props:

export interface UniversalTargetProps extends ScheduleTargetBaseProps { readonly service: string; readonly action: string; /** * The IAM policy statements needed to invoke the target. These statements are attached to the Scheduler's role. * * Note that the default may not be the correct actions as not all AWS services follows the same IAM action pattern, or there may be more actions needed to invoke the target. * * @default - Policy with `service:action` action only. */ readonly policyStatements?: PolicyStatement[]; /* <--- this will **replace** the default policy */ }

[1] List of issues with regards to confusion on default policy:

aws-stepfunctions-tasks: CallAwsService for elasticloadbalancingv2 produces invalid IAM role policies #32417

aws-cdk-lib/aws-stepfunctions-tasks: Issue with incorrectly generated IAM policy. #30862

aws-cdk-lib/aws-stepfunctions-tasks: sesv2 task does not grant ses:SendEmail permission #30745

stepfunctions-tasks: mediapackagevod service generates wrong action in role policy #28774

aws_stepfunctions_tasks : Generates wrong action in role policy #27573

Thank you for discussing this with me to make CDK better. I appreciate it!

@samson-keung
Thank you for your suggestion.
As a result, the API design has become more refined!

I’ve made it so that either the default policy or the policy statements specified in policyStatements prop are added to the IAM Role, but not both. Does this align with your intention?

6bae7c3

Pull request has been modified.

…heduler-universal-target

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

samson-keung · 2025-01-10T21:04:46Z

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

+  /**
+   * The action for the IAM policy statement that will be added to the scheduler role's policy
+   * to allow the scheduler to make the API call.
+   *
+   * Use this in cases where the IAM action name does not match the
+   * API service/action name, e.g., `lambda:invoke` requires `lambda:InvokeFunction` permission.
+   *
+   * @default - service:action
+   */
+  readonly iamAction?: string;


Thanks @sakurai-ryo for the response. I agree with you that having CDK adding the assume-role-policy with secure conditions here is valuable. I am convinced that we should not require CDK users to pass in a role prop.

I spent some time to study the stepfunctions-tasks.CallAwsService class you linked and its history. It originally did not have the additionalPolicyStatements prop. And with the iamX props, it seems to have created confusions because the service:action is not enough permissions for the application to work [1].

I think the best middle ground here is to have the following props:

export interface UniversalTargetProps extends ScheduleTargetBaseProps { readonly service: string; readonly action: string; /** * The IAM policy statements needed to invoke the target. These statements are attached to the Scheduler's role. * * Note that the default may not be the correct actions as not all AWS services follows the same IAM action pattern, or there may be more actions needed to invoke the target. * * @default - Policy with `service:action` action only. */ readonly policyStatements?: PolicyStatement[]; /* <--- this will **replace** the default policy */ }

[1] List of issues with regards to confusion on default policy:

aws-stepfunctions-tasks: CallAwsService for elasticloadbalancingv2 produces invalid IAM role policies #32417

aws-cdk-lib/aws-stepfunctions-tasks: Issue with incorrectly generated IAM policy. #30862

aws-cdk-lib/aws-stepfunctions-tasks: sesv2 task does not grant ses:SendEmail permission #30745

stepfunctions-tasks: mediapackagevod service generates wrong action in role policy #28774

aws_stepfunctions_tasks : Generates wrong action in role policy #27573

Thank you for discussing this with me to make CDK better. I appreciate it!

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts

…hub.com/sakurai-ryo/aws-cdk into scheduler-universal-target

…heduler-universal-target

Pull request has been modified.

samson-keung

Thank you. This is awesome!

mergify · 2025-01-13T17:33:23Z

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 · 2025-01-13T18:05:18Z

AWS CodeBuild CI Report

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

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

mergify · 2025-01-13T18:05:40Z

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).

github-actions · 2025-01-13T18:06:08Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

ryo_sakurai and others added 2 commits November 29, 2024 20:24

add new target

59def06

add test

58dfca3

aws-cdk-automation requested a review from a team December 1, 2024 10:52

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 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Dec 1, 2024

sakurai-ryo added 3 commits December 1, 2024 20:28

export

0071ae2

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into sc…

4c49741

…heduler-universal-target

fix readme

450a7a9

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

go-to-k suggested changes Dec 24, 2024

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 Dec 24, 2024

sakurai-ryo added 10 commits December 25, 2024 16:08

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into sc…

2ec0567

…heduler-universal-target

fix type

7b4a4ec

change construct name

319ae62

rm iamResources and iamAction

3c1c2e3

fix typo

209f6ea

rm unintended file

316aed7

rm link

50c8858

fix comments

2de7da1

update readme

c5bb21a

update universal.ts

7297027

go-to-k reviewed Dec 26, 2024

View reviewed changes

packages/@aws-cdk/aws-scheduler-targets-alpha/lib/universal.ts Show resolved Hide resolved

sakurai-ryo added 4 commits December 26, 2024 17:47

add props

e0469c6

Merge branch 'scheduler-universal-target' of https://github.com/sakur…

120eacb

…ai-ryo/aws-cdk into scheduler-universal-target

fix indent

e211940

update readme

bec0414

update readme

2fe40a6

go-to-k approved these changes Dec 27, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 27, 2024

github-actions bot mentioned this pull request Jan 1, 2025

Monthly PR metrics report #32703

Open

samson-keung self-assigned this Jan 7, 2025

samson-keung added the needs-security-review Related to feature or issues that needs security review label Jan 7, 2025

samson-keung previously requested changes Jan 8, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 8, 2025

sakurai-ryo added 2 commits January 10, 2025 19:15

fix jsdoc

ead143b

change props name

a2d684b

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into sc…

b0af30b

…heduler-universal-target

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 10, 2025

samson-keung previously requested changes Jan 10, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 10, 2025

sakurai-ryo added 3 commits January 13, 2025 20:06

Merge branches 'scheduler-universal-target' and 'main' of https://git…

6e5591a

…hub.com/sakurai-ryo/aws-cdk into scheduler-universal-target

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into sc…

44e9f84

…heduler-universal-target

add policyStatements prop

6bae7c3

fix build error

a6ba36e

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 13, 2025

samson-keung approved these changes Jan 13, 2025

View reviewed changes

Merge branch 'main' into scheduler-universal-target

c49cc2b

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 13, 2025

mergify bot merged commit 021e6d6 into aws:main Jan 13, 2025
21 of 22 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Jan 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(scheduler-targets): add support for universal target #32341

feat(scheduler-targets): add support for universal target #32341

sakurai-ryo commented Dec 1, 2024 •

edited

Loading

codecov bot commented Dec 1, 2024 •

edited

Loading

sakurai-ryo commented Dec 26, 2024

go-to-k left a comment

samson-keung Jan 8, 2025

sakurai-ryo Jan 10, 2025

samson-keung Jan 8, 2025

sakurai-ryo Jan 10, 2025

samson-keung Jan 8, 2025

sakurai-ryo Jan 10, 2025

samson-keung Jan 10, 2025

sakurai-ryo Jan 13, 2025

samson-keung Jan 10, 2025

samson-keung left a comment

mergify bot commented Jan 13, 2025

aws-cdk-automation commented Jan 13, 2025

mergify bot commented Jan 13, 2025

github-actions bot commented Jan 13, 2025

	export interface UniversalProps extends ScheduleTargetBaseProps {
	export interface UniversalTargetProps extends ScheduleTargetBaseProps {

feat(scheduler-targets): add support for universal target #32341

feat(scheduler-targets): add support for universal target #32341

Conversation

sakurai-ryo commented Dec 1, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

codecov bot commented Dec 1, 2024 • edited Loading

Codecov Report

sakurai-ryo commented Dec 26, 2024

go-to-k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samson-keung left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 13, 2025

aws-cdk-automation commented Jan 13, 2025

AWS CodeBuild CI Report

mergify bot commented Jan 13, 2025

github-actions bot commented Jan 13, 2025

sakurai-ryo commented Dec 1, 2024 •

edited

Loading

codecov bot commented Dec 1, 2024 •

edited

Loading