fix(scheduler-alpha): scheduler input always get transformed to string with extra double quotes #31894

samson-keung · 2024-10-24T20:27:19Z

Issue # (if applicable)

Tracking #31785.

Reason for this change

When using ScheduleTargetInput.fromText("some-string"), the string is always wrapped with an extra double quotes in the final template:

"Input": "\"some-string\"",

Customers cannot get rid of the extra escaped double quotes other than using escape hatches.

Description of changes

Skip converting the text to JSON string, which is the reason causing the extra double quotes.

Description of how you validated changes

Unit test and integ test.

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

BREAKING CHANGE: Got rid of extra double quotes on Schedule input which blocks customer intention of using no double quotes in the input text

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

samson-keung · 2024-10-26T04:21:27Z

packages/@aws-cdk/aws-scheduler-alpha/lib/input.ts

@@ -41,7 +41,7 @@ export abstract class ScheduleTargetInput {
 }

 class FieldAwareEventInput extends ScheduleTargetInput {
-  constructor(private readonly input: any) {
+  constructor(private readonly input: any, private readonly toJsonString: boolean) {


There is probably a better implementation approach but since this is only used internal/private in the class, I will fix the behaviour for now, then improve the design for easier readability and maintainability in subsequent PRs.

I think its fine honestly

I see a similar class in aws-events where the second parameter is the inputType and the bind method handles the input accordingly. Could we do something similar here?

kaizencc · 2024-10-28T17:07:31Z

packages/@aws-cdk/aws-scheduler-alpha/lib/input.ts

@@ -41,7 +41,7 @@ export abstract class ScheduleTargetInput {
 }

 class FieldAwareEventInput extends ScheduleTargetInput {
-  constructor(private readonly input: any) {
+  constructor(private readonly input: any, private readonly toJsonString: boolean) {


I think its fine honestly

mergify · 2024-10-28T17:08:10Z

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

mergify · 2024-10-28T17:12: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).

gracelu0 · 2024-10-28T17:17:09Z

packages/@aws-cdk/aws-scheduler-alpha/lib/input.ts

@@ -41,7 +41,7 @@ export abstract class ScheduleTargetInput {
 }

 class FieldAwareEventInput extends ScheduleTargetInput {
-  constructor(private readonly input: any) {
+  constructor(private readonly input: any, private readonly toJsonString: boolean) {


I see a similar class in aws-events where the second parameter is the inputType and the bind method handles the input accordingly. Could we do something similar here?

mergify · 2024-10-28T17:48:32Z

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 · 2024-10-28T18:20:05Z

AWS CodeBuild CI Report

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

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

mergify · 2024-10-28T18:20:34Z

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 · 2024-10-28T18:20:49Z

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.

samson-keung added 2 commits October 24, 2024 12:31

reduce key permissions. pending test changes

ff1e462

update integ test to have less KMS permissions

9025add

aws-cdk-automation requested a review from a team October 24, 2024 20:27

github-actions bot added the p2 label Oct 24, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label Oct 24, 2024

aws-cdk-automation previously requested changes Oct 24, 2024

View reviewed changes

samson-keung force-pushed the scheduler-reduce-key-permissions branch 2 times, most recently from 7c611ac to 01b387a Compare October 24, 2024 21:47

revise schedule integ test to have assertion

fb485c7

samson-keung force-pushed the scheduler-reduce-key-permissions branch from 01b387a to fb485c7 Compare October 24, 2024 22:05

samson-keung added 4 commits October 24, 2024 15:10

Integ assertion is updated to poll for max 10 messages from SQS

d6d4a7d

Add missing permission to Schedule to send messages to queue

6f0f045

make assertion wait time less. 5 minutes

88dad4f

update schedule time so that they do not get invoked

1447087

samson-keung force-pushed the scheduler-reduce-key-permissions branch from d55f6c2 to 475fd3f Compare October 25, 2024 05:41

fix input fromText has extra quotations

5b81a3e

samson-keung force-pushed the scheduler-reduce-key-permissions branch from 475fd3f to 5b81a3e Compare October 25, 2024 15:43

samson-keung added 12 commits October 25, 2024 16:25

update snapshot

37091ff

update integ test

cc45044

update SNS target integ test

4e980a2

update test with input logic update

3001fb0

update sqs target integ test

c8ef60d

update LambdaInvoke integ test

6d0c9c3

revert reducing key permissions

9fc50c5

revert integ.schedule.ts change

d8992ee

fix integ.schedule test

3f3e132

fix integ.schedule test

fabef86

fix integ.schedule test

7c67ac0

update integ

455a092

add unit test for input

550cc14

samson-keung changed the title ~~fix(scheduler-targets-alpha): unnecessary KMS permissions granted to Schedule role~~ fix(scheduler-alpha): scheduler input always get transformed to string with extra double quotes Oct 26, 2024

samson-keung marked this pull request as ready for review October 26, 2024 04:17

samson-keung commented Oct 26, 2024

View reviewed changes

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

github-actions bot mentioned this pull request Oct 28, 2024

Weekly PR metrics report #31915

Closed

kaizencc approved these changes Oct 28, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 28, 2024

Merge branch 'main' into scheduler-reduce-key-permissions

404f0aa

gracelu0 approved these changes Oct 28, 2024

View reviewed changes

Merge branch 'main' into scheduler-reduce-key-permissions

09521ad

mergify bot merged commit 186b8ab into aws:main Oct 28, 2024
9 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2024

samson-keung deleted the scheduler-reduce-key-permissions branch October 28, 2024 18:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(scheduler-alpha): scheduler input always get transformed to string with extra double quotes #31894

fix(scheduler-alpha): scheduler input always get transformed to string with extra double quotes #31894

samson-keung commented Oct 24, 2024 •

edited

Loading

aws-cdk-automation left a comment

samson-keung Oct 26, 2024

kaizencc Oct 28, 2024

gracelu0 Oct 28, 2024

kaizencc Oct 28, 2024

mergify bot commented Oct 28, 2024

mergify bot commented Oct 28, 2024

gracelu0 Oct 28, 2024

mergify bot commented Oct 28, 2024

aws-cdk-automation commented Oct 28, 2024

mergify bot commented Oct 28, 2024

github-actions bot commented Oct 28, 2024

fix(scheduler-alpha): scheduler input always get transformed to string with extra double quotes #31894

fix(scheduler-alpha): scheduler input always get transformed to string with extra double quotes #31894

Conversation

samson-keung commented Oct 24, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

samson-keung Oct 26, 2024

Choose a reason for hiding this comment

kaizencc Oct 28, 2024

Choose a reason for hiding this comment

gracelu0 Oct 28, 2024

Choose a reason for hiding this comment

kaizencc Oct 28, 2024

Choose a reason for hiding this comment

mergify bot commented Oct 28, 2024

mergify bot commented Oct 28, 2024

gracelu0 Oct 28, 2024

Choose a reason for hiding this comment

mergify bot commented Oct 28, 2024

aws-cdk-automation commented Oct 28, 2024

AWS CodeBuild CI Report

mergify bot commented Oct 28, 2024

github-actions bot commented Oct 28, 2024

samson-keung commented Oct 24, 2024 •

edited

Loading