fix(custom-resource-handlers): better fallback for require failures #30095

glitchassassin · 2024-05-07T17:56:15Z

Issue # (if applicable)

Reason for this change

Fallback to existing AWS SDK import misses a rare/flaky edge case where the npm install passes, but the subsequent require fails

Description of changes

Fall back to the pre-existing AWS SDK if requiring the latest version fails

Description of how you validated changes

Fixed no-op test "installs the latest SDK"
Added test "falls back to installed sdk if require fails"

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

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.

glitchassassin · 2024-05-07T19:03:24Z

Clarification Request

I am not sure I understand how to run/resolve the integration test snapshots - would appreciate some guidance

glitchassassin · 2024-05-07T21:01:59Z

...and it seems the pr-linter-exception-labeler action may be broken too - it's failing with an error

GavinZZ · 2024-05-08T20:54:23Z

@glitchassassin Thanks for drafting the PR. The CodeBuild CI failed because you updated the custom resource handler file, which means the code hash of the handler file changes. This results in all these integration tests that uses this custom resource when comparing the snapshots.

What you can do is to look at the build log and search for !!!!!!!!. You should then see a list of failed integration tests. For each failed tests, you can run yarn integ aws-elasticloadbalancingv2/test/integ.alb.oidc --update-on-failed (replace the path with the actual test).

More can be found in https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md.

glitchassassin · 2024-05-13T21:26:14Z

Thanks Gavin. I ran the integration tests like this and then committed the results:

yarn integ custom-resources/test/aws-custom-resource/integ.aws-custom-resource.js --disable-update-workflow --update-on-failed

I got a SUCCESS message locally for most of them. A few failed with an error similar to this:

The stack named LogGroupDefaultTestDeployAssert353EE07A failed to deploy: CREATE_FAILED (The following resource(s) failed to create: [AwsApiCallCloudWatchLogsdescribeResourcePolicies]. ): Response object is too long.

I pushed up the changes for the ones that passed, but it looks like CodeBuild is still failing all of those integration tests. Have I missed a step?

GavinZZ · 2024-05-13T21:31:52Z

Prior to running the integ tests, did you run yarn build in npx lerna run build --scope=@aws-cdk/custom-resource-handlers and npx lerna run build --scope=aws-cdk-lib to make sure the correct lambda handler file is used in integ tests?

glitchassassin · 2024-05-15T17:59:40Z

I'm pretty sure I did, but just in case, I re-ran the build and tried the integration tests again. I'm not seeing any differences.

I noticed that on some of them (e.g. aws-elasticloadbalancingv2/test/integ.alb.oidc.js) the build reports "destructive changes" because I provided a different hosted zone/domain name per framework-integ/README.md. I'm not sure how to resolve this.

GavinZZ · 2024-05-15T20:43:47Z

For the breaking changes, using integ.alb.oidc test as an example, I saw the template.json and assets.json are modified manually. That could cause breaking changes.

glitchassassin · 2024-05-15T21:15:23Z

yes, those changes were generated by following these steps to set the hosted zone:

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk-testing/framework-integ/README.md#error-for-this-test-you-must-provide-your-own-hostedzoneidhostedzonenamedomainname

pahud · 2024-05-22T20:47:46Z

Hi @glitchassassin

I'll open your PR branch later today and run the integ tests from my IDE. You can reach out to me on cdk.dev if you need any discussion around the PR contribution. I am more than happy to help.

pahud · 2024-05-23T16:35:30Z

Hi @glitchassassin

I have created a PR to your branch to update the integ tests
glitchassassin#2

Can you merge my PR to your branch and this PR should update immediately. Let's see if the CI would pass.

update integ tests

pahud · 2024-05-24T16:24:33Z

OK looks like your PR got new integ error

@aws-cdk-testing/framework-integ:   CHANGED    custom-resources/test/aws-custom-resource/integ.aws-custom-resource-dynamodb 1.258s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ: [~] AWS::Lambda::Function AWS679f53fac002430cb0da5b7982bd22872D164C4C 
@aws-cdk-testing/framework-integ:  â””â”€ [~] Code
@aws-cdk-testing/framework-integ:      â””â”€ [~] .S3Key:
@aws-cdk-testing/framework-integ:          â”œâ”€ [-] 4c349b026f9dadd9a6737d4682c2da3e53101244798442ccec89ee0a0bd96f26.zip
@aws-cdk-testing/framework-integ:          â””â”€ [+] 107e4ea7fe26b0049a79003e5710556207812d452795845039064ba20e7b7d56.zip
@aws-cdk-testing/framework-integ:   CHANGED    custom-resources/test/aws-custom-resource/integ.aws-custom-resource-athena 1.381s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ: [~] AWS::Lambda::Function AWS679f53fac002430cb0da5b7982bd22872D164C4C 
@aws-cdk-testing/framework-integ:  â””â”€ [~] Code
@aws-cdk-testing/framework-integ:      â””â”€ [~] .S3Key:
@aws-cdk-testing/framework-integ:          â”œâ”€ [-] 4c349b026f9dadd9a6737d4682c2da3e53101244798442ccec89ee0a0bd96f26.zip
@aws-cdk-testing/framework-integ:          â””â”€ [+] 107e4ea7fe26b0049a79003e5710556207812d452795845039064ba20e7b7d56.zip
@aws-cdk-testing/framework-integ:   UNCHANGED  custom-resources/test/aws-custom-resource/integ.aws-custom-resource-ssm 1.368s
@aws-cdk-testing/framework-integ:   UNCHANGED  custom-resources/test/aws-custom-resource/integ.invoke-function-payload 1.276s
@aws-cdk-testing/framework-integ:   UNCHANGED  custom-resources/test/aws-custom-resource/integ.cross-account-assumeRole 1.506s
@aws-cdk-testing/framework-integ:   CHANGED    custom-resources/test/aws-custom-resource/integ.aws-custom-resource 1.662s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ: [~] AWS::Lambda::Function AWS679f53fac002430cb0da5b7982bd22872D164C4C 
@aws-cdk-testing/framework-integ:  â””â”€ [~] Code
@aws-cdk-testing/framework-integ:      â””â”€ [~] .S3Key:
@aws-cdk-testing/framework-integ:          â”œâ”€ [-] 4c349b026f9dadd9a6737d4682c2da3e53101244798442ccec89ee0a0bd96f26.zip
@aws-cdk-testing/framework-integ:          â””â”€ [+] 107e4ea7fe26b0049a79003e5710556207812d452795845039064ba20e7b7d56.zip

and

@aws-cdk-testing/framework-integ:   CHANGED    aws-synthetics/test/integ.canary-auto-delete-lambda 4.905s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ: [~] AWS::Lambda::Function AWS679f53fac002430cb0da5b7982bd22872D164C4C 
@aws-cdk-testing/framework-integ:  â””â”€ [~] Code
@aws-cdk-testing/framework-integ:      â””â”€ [~] .S3Key:
@aws-cdk-testing/framework-integ:          â”œâ”€ [-] 746da84b10e215c552e68b6d2061024e4429f0386f43a35ef5e4d2940655692e.zip
@aws-cdk-testing/framework-integ:          â””â”€ [+] 107e4ea7fe26b0049a79003e5710556207812d452795845039064ba20e7b7d56.zip
@aws-cdk-testing/framework-integ:   UNCHANGED  pipelines/test/integ.pipeline-with-stack-outputs-in-custom-step 1.233s
@aws-cdk-testing/framework-integ:   CHANGED    custom-resources/test/aws-custom-resource/integ.aws-custom-resource-vpc 3.48s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ: [~] AWS::Lambda::Function AWS679f53fac002430cb0da5b7982bd22872D164C4C 
@aws-cdk-testing/framework-integ:  â””â”€ [~] Code
@aws-cdk-testing/framework-integ:      â””â”€ [~] .S3Key:
@aws-cdk-testing/framework-integ:          â”œâ”€ [-] 4c349b026f9dadd9a6737d4682c2da3e53101244798442ccec89ee0a0bd96f26.zip
@aws-cdk-testing/framework-integ:          â””â”€ [+] 107e4ea7fe26b0049a79003e5710556207812d452795845039064ba20e7b7d56.zip

You'll need to re-run all the CHANGED integ tests and update all their snapshots.

I can't run all of them for you. Please reach out to me on cdk.dev slack if you need any help I can show you how to do that with a video.

aws-cdk-automation · 2024-05-29T00:10:10Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

glitchassassin · 2024-05-29T00:19:44Z

not abandoned; still working through integration test issues

aws-cdk-automation · 2024-05-29T20:50:21Z

AWS CodeBuild CI Report

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

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

glitchassassin · 2024-06-05T20:05:58Z

Cross-posting from the Slack discussion:

I'm not sure there's a good way to test these changes in an integration test, as it's the handler source code that is changing. There is a unit test for the change, but I'm not sure how to force the error in an integration test scenario. Any suggestions, or should I request an exemption?

aws-cdk-automation · 2024-06-06T00:09:14Z

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

aws-cdk-automation · 2024-06-06T00:11:04Z

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

mergify · 2024-06-06T12:02:51Z

⚠️ The sha of the head commit of this PR conflicts with #30469. Mergify cannot evaluate rules on this PR. ⚠️

glitchassassin · 2024-06-06T12:03:04Z

Reopening here: #30469

glitchassassin added 4 commits May 6, 2024 16:26

Fix AWS SDK fallback

d678efc

Fixing tests

aa55b7d

Cleaning up debugging

034f582

Simplifying require logic

5aac71e

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label May 7, 2024

aws-cdk-automation requested a review from a team May 7, 2024 17:56

github-actions bot added bug This issue is a bug. p2 labels May 7, 2024

aws-cdk-automation requested changes May 7, 2024

View reviewed changes

glitchassassin changed the title ~~Fixing custom resource require failures~~ fix(custom-resource-handlers): Better fallback for require failures May 7, 2024

glitchassassin changed the title ~~fix(custom-resource-handlers): Better fallback for require failures~~ fix(custom-resource-handlers): better fallback for require failures May 7, 2024

github-actions bot mentioned this pull request May 13, 2024

Weekly PR metrics report #30176

Closed

updating integration tests

ef15e7d

GavinZZ self-requested a review May 13, 2024 21:32

update integ tests

77bbb00

Merge pull request #2 from pahud/pahud-fix

ad5687f

update integ tests

glitchassassin added 3 commits May 28, 2024 15:13

Updating integration tests

814dc1d

more integration tests

637364c

fixing bucket-auto-delete-objects test

edd79db

updating integration tests

84bad7e

glitchassassin added 2 commits May 29, 2024 11:53

more integration tests

30e59ed

opensearch integration tests

26a472e

TheRealAmazonKendra self-assigned this May 29, 2024

fixing remaining integration tests

15a0b97

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

Monthly PR metrics report #30414

Closed

aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Jun 6, 2024

aws-cdk-automation closed this Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(custom-resource-handlers): better fallback for require failures #30095

fix(custom-resource-handlers): better fallback for require failures #30095

glitchassassin commented May 7, 2024

aws-cdk-automation left a comment

glitchassassin commented May 7, 2024

glitchassassin commented May 7, 2024

GavinZZ commented May 8, 2024 •

edited

Loading

glitchassassin commented May 13, 2024

GavinZZ commented May 13, 2024

glitchassassin commented May 15, 2024

GavinZZ commented May 15, 2024

glitchassassin commented May 15, 2024

pahud commented May 22, 2024

pahud commented May 23, 2024

pahud commented May 24, 2024

aws-cdk-automation commented May 29, 2024

glitchassassin commented May 29, 2024

aws-cdk-automation commented May 29, 2024

glitchassassin commented Jun 5, 2024

aws-cdk-automation commented Jun 6, 2024

aws-cdk-automation commented Jun 6, 2024

mergify bot commented Jun 6, 2024

glitchassassin commented Jun 6, 2024

fix(custom-resource-handlers): better fallback for require failures #30095

fix(custom-resource-handlers): better fallback for require failures #30095

Conversation

glitchassassin commented May 7, 2024

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

glitchassassin commented May 7, 2024

glitchassassin commented May 7, 2024

GavinZZ commented May 8, 2024 • edited Loading

glitchassassin commented May 13, 2024

GavinZZ commented May 13, 2024

glitchassassin commented May 15, 2024

GavinZZ commented May 15, 2024

glitchassassin commented May 15, 2024

pahud commented May 22, 2024

pahud commented May 23, 2024

pahud commented May 24, 2024

aws-cdk-automation commented May 29, 2024

glitchassassin commented May 29, 2024

aws-cdk-automation commented May 29, 2024

AWS CodeBuild CI Report

glitchassassin commented Jun 5, 2024

aws-cdk-automation commented Jun 6, 2024

aws-cdk-automation commented Jun 6, 2024

mergify bot commented Jun 6, 2024

glitchassassin commented Jun 6, 2024

GavinZZ commented May 8, 2024 •

edited

Loading