fix(custom-resources): correctly convert values to Date type #28398

sakurai-ryo · 2023-12-17T11:04:39Z

Description

The following issue reports an error that occurs when calling an API that takes the Date type as a parameter, such as GetMetricData API, from a Custom Resource Lambda function, where the parameter is passed as string type to the AWS SDK.
#27962
https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/client/cloudwatch/command/GetMetricDataCommand/#:~:text=Description-,EndTime,-Required

To resolve this error, the string type must be properly converted to Date type when calling the AWS SDK from Lambda.
In this PR, I added the conversion to Date type in the same way as the existing conversion to number and Uint8Array types.
Uint8Array: #27034
number: #27112

Major changes

`update-sdkv3-parameters-model.ts` script

If the type is timestamp in the smithy specification, write d to the state machine so that it can be converted to a Date type later.
https://smithy.io/2.0/spec/simple-types.html#timestamp

update-sdkv3-parameters-model.sh script was not called from anywhere, so I called it manually and updated the JSON file.
Please let me know if there is a problem.

`sdk-v2-to-v3-adapter` module

I added code to convert value marked d in state machine to Date type.
If the conversion to Date type fails, the Date class does not throw an exception, so the error is handled in a slightly tricky way.
Also added a unit test for this process.

`integ-tests-alpha` module

Added integ-test to verify that errors reported in the related issue have been resolved.
The IAM Policy added internally by the call to adPolicyStatementFromSdkCall looks like the following and does not call GetMetricData correctly, so the addToRolePolicy method was used to explicitly add a new Policy is added explicitly with the addToRolePolicy method.

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Action": [
                "monitoring:GetMetricData"
            ],
            "Resource": [
                "*"
            ],
            "Effect": "Allow"
        }
    ]
}

aws-cdk/packages/aws-cdk-lib/custom-resources/lib/helpers-internal/sdk-v3-metadata.json

Line 174 in 1a9c30e

"iamPrefix": "monitoring"

fixes #27962

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

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.

sakurai-ryo · 2023-12-18T11:00:30Z

Exemption Request: Since this PR is an internal behavior fix, I don't think it is necessary to change the README.

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

kaizencc

Thank you for the very detailed PR @sakurai-ryo! This looks mostly reasonable to me, but because I'm not too familiar with this part of our repo I'm going to toss it over to @mrgrain or @MrArnoldPalmer for a second look.

kaizencc · 2023-12-19T20:12:31Z

scripts/update-sdkv3-parameters-model.ts

@@ -332,6 +338,10 @@ function isNumber(shape: SmithyShape): boolean {
    isShape('byte')(shape);
 }

+function isDate(shape: SmithyShape): boolean {
+  return isShape('timestamp')(shape)


Suggested change

return isShape('timestamp')(shape)

return isShape('timestamp')(shape);

Oops, I fixed it.

kaizencc · 2023-12-19T20:16:23Z

packages/@aws-cdk/sdk-v2-to-v3-adapter/lib/sdk-v2-to-v3.json

Mostly because I'm not too familiar with this section of the repo, can you explain how you arrived at these updates?

Should be coming from running the update-sdkv3-parameters-model.sh script.

kaizencc · 2023-12-19T20:16:29Z

packages/@aws-cdk/sdk-v2-to-v3-adapter/lib/sdk-v3-metadata.json

As well as these updates...

Should be coming from running the update-sdkv3-parameters-model.sh script.

…fix-integ-tests-date-type

…i-ryo/aws-cdk into fix-integ-tests-date-type

paulhcsun · 2024-01-30T01:01:27Z

@sakurai-ryo There are some merge conflicts that need to be addressed. The changes look good to me, @kaizencc @mrgrain are there any other questions or changes to be made?

…fix-integ-tests-date-type

sakurai-ryo · 2024-01-30T11:39:26Z

Thanks @paulhcsun! Fixed conflicts.

…fix-integ-tests-date-type

paulhcsun · 2024-02-29T19:47:51Z

hey @sakurai-ryo,

Apologies for not getting around to this. It doesn't seem like there are any other major concerns and I'm good to merge this in. It seems like the integ snapshots are outdated. Could you rerun with latest changes to update those? I'll keep an eye on this and approve when the build is happy.

…fix-integ-tests-date-type

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.

sakurai-ryo · 2024-03-01T13:33:58Z

Hi @paulhcsun
I know the CDK team is busy, so that's not a problem at all.
Thanks for taking care of this PR.
I fixed the conflicts, please check it out.

paulhcsun

Thanks for this fix @sakurai-ryo! And thanks again for your patience and understanding.

aws-cdk-automation · 2024-03-04T19:25:45Z

AWS CodeBuild CI Report

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

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

mergify · 2024-03-04T19:26:13Z

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

sakurai-ryo added 2 commits December 17, 2023 19:52

fix: add date type

9d01d6f

fix: add integ test

3d2a8dc

github-actions bot added the valued-contributor [Pilot] contributed between 6-12 PRs to the CDK label Dec 17, 2023

aws-cdk-automation requested a review from a team December 17, 2023 11:04

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p1 labels Dec 17, 2023

aws-cdk-automation previously requested changes Dec 17, 2023

View reviewed changes

sakurai-ryo added 6 commits December 17, 2023 21:21

fix: change comment

2b170fe

add: add test for custom resource

88f6fb4

fix: test

0abb026

chore: update integ-tests snapshot

78ddc27

fix: update snapshot

6c82036

fix: tests

1027fed

sakurai-ryo marked this pull request as ready for review December 18, 2023 11:00

aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. pr/needs-maintainer-review This PR needs a review from a Core Team Member labels Dec 18, 2023

sakurai-ryo changed the title ~~feat(sdk-v2-to-v3-adapter): add support for coercing value to Date~~ fix(custom-resources): correctly convert values to Date type. Dec 18, 2023

sakurai-ryo changed the title ~~fix(custom-resources): correctly convert values to Date type.~~ fix(custom-resources): correctly convert values to Date type Dec 18, 2023

kaizencc reviewed Dec 19, 2023

View reviewed changes

sakurai-ryo and others added 3 commits December 20, 2023 11:21

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

43a1e19

…fix-integ-tests-date-type

chore: fix for the review

b6dbb8a

Merge branch 'main' into fix-integ-tests-date-type

ff6661f

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

Monthly PR metrics report - Dec 2023 #28544

Closed

sakurai-ryo added 4 commits January 13, 2024 18:30

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

5c35bf7

…fix-integ-tests-date-type

update snapshots

4d922a7

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

e67ea46

…fix-integ-tests-date-type

Merge branch 'fix-integ-tests-date-type' of https://github.com/sakura…

6c98684

…i-ryo/aws-cdk into fix-integ-tests-date-type

sakurai-ryo added 5 commits January 14, 2024 00:59

update snapshots

6e5ce2a

host zone id

f0f2499

from main

ea13775

update

7452caf

update

694dcdc

sakurai-ryo added 5 commits January 30, 2024 15:48

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

b4e85bf

…fix-integ-tests-date-type

update sdk metadata

a10d873

Update TypeCoercionStateMachine to include 'd' type

016fb77

update snapshot

a7b669f

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

20c5d75

…fix-integ-tests-date-type

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

487bff1

…fix-integ-tests-date-type

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

5689888

…fix-integ-tests-date-type

aws-cdk-automation previously requested changes Mar 1, 2024

View reviewed changes

sakurai-ryo added 2 commits March 1, 2024 21:25

add package

ebadd26

fix conflict

54d803b

Merge branch 'main' into fix-integ-tests-date-type

45c475e

paulhcsun approved these changes Mar 4, 2024

View reviewed changes

mergify bot merged commit 38bdb92 into aws:main Mar 4, 2024
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(custom-resources): correctly convert values to Date type #28398

fix(custom-resources): correctly convert values to Date type #28398

sakurai-ryo commented Dec 17, 2023 •

edited

aws-cdk-automation left a comment

sakurai-ryo commented Dec 18, 2023

kaizencc left a comment

kaizencc Dec 19, 2023

sakurai-ryo Dec 20, 2023

kaizencc Dec 19, 2023

mrgrain Dec 20, 2023

kaizencc Dec 19, 2023

mrgrain Dec 20, 2023

paulhcsun commented Jan 30, 2024

sakurai-ryo commented Jan 30, 2024

paulhcsun commented Feb 29, 2024

aws-cdk-automation left a comment

sakurai-ryo commented Mar 1, 2024

paulhcsun left a comment

aws-cdk-automation commented Mar 4, 2024

mergify bot commented Mar 4, 2024

	return isShape('timestamp')(shape)
	return isShape('timestamp')(shape);

fix(custom-resources): correctly convert values to Date type #28398

fix(custom-resources): correctly convert values to Date type #28398

Conversation

sakurai-ryo commented Dec 17, 2023 • edited

Description

Major changes

update-sdkv3-parameters-model.ts script

sdk-v2-to-v3-adapter module

integ-tests-alpha module

aws-cdk-automation left a comment

Choose a reason for hiding this comment

sakurai-ryo commented Dec 18, 2023

kaizencc left a comment

Choose a reason for hiding this comment

kaizencc Dec 19, 2023

Choose a reason for hiding this comment

sakurai-ryo Dec 20, 2023

Choose a reason for hiding this comment

kaizencc Dec 19, 2023

Choose a reason for hiding this comment

mrgrain Dec 20, 2023

Choose a reason for hiding this comment

kaizencc Dec 19, 2023

Choose a reason for hiding this comment

mrgrain Dec 20, 2023

Choose a reason for hiding this comment

paulhcsun commented Jan 30, 2024

sakurai-ryo commented Jan 30, 2024

paulhcsun commented Feb 29, 2024

aws-cdk-automation left a comment

Choose a reason for hiding this comment

sakurai-ryo commented Mar 1, 2024

paulhcsun left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Mar 4, 2024

AWS CodeBuild CI Report

mergify bot commented Mar 4, 2024

sakurai-ryo commented Dec 17, 2023 •

edited

`update-sdkv3-parameters-model.ts` script

`sdk-v2-to-v3-adapter` module

`integ-tests-alpha` module