Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(lambda): Migrate away from NODEJS_10_X and NODEJS_12_X to NODEJS_14_X #20595

Merged
merged 14 commits into from
Jun 3, 2022
Merged

feat(lambda): Migrate away from NODEJS_10_X and NODEJS_12_X to NODEJS_14_X #20595

merged 14 commits into from
Jun 3, 2022

Conversation

robertd
Copy link
Contributor

@robertd robertd commented Jun 2, 2022 •
edited
Loading

Spiritual successor of #20531

Screen Shot 2022-06-01 at 9 08 18 PM

  • Migrated away (tests, docs, internal use) from NODEJS_10_X to NODEJS_14_X
  • Migrated away (tests, docs, internal use) from NODEJS_12_X to NODEJS_14_X
  • Added NODEJS_16_X to CustomResources
  • Update CustomResources to use nodejs14.x by default
  • Updated integration tests across the board. Some integ tests introduced destructive changes (expected with change of nodejs runtime)

Closes #20568
Closes #19992
Closes #20474

All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

@gitpod-io
Copy link

gitpod-io bot commented Jun 2, 2022

@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Jun 2, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team June 2, 2022 21:57
@robertd
Copy link
Contributor Author

robertd commented Jun 2, 2022 •
edited
Loading

  • Migrated away (tests, docs, internal use) from NODEJS_10_X to NODEJS_14_X
  • Migrated away (tests, docs, internal use) from NODEJS_12_X to NODEJS_14_X
  • Added NODEJS_16_X to CustomResources
  • Update CustomResources to use nodejs14.x by default
  • Updated integration tests across the board. Some integ tests introduced destructive changes (expected with change of nodejs runtime)

@robertd robertd changed the title feat(nodejs): Migrate away from NODEJS_10_X and NODEJS_12_X to NODEJS_14_X feat(lambda): Migrate away from NODEJS_10_X and NODEJS_12_X to NODEJS_14_X Jun 2, 2022
@robertd robertd marked this pull request as ready for review June 3, 2022 02:11
@robertd
Copy link
Contributor Author

robertd commented Jun 3, 2022

@jogold @pahud @rix0rrr Can you please review and fast-track this PR since it touches so many modules? Thanks!

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@pahud
Copy link
Contributor

pahud commented Jun 3, 2022

I didn't see any specific issues. The core team should look into this PR very soon. Thank you for yet another awesome PR @robertd !

kaizencc
kaizencc approved these changes Jun 3, 2022
View reviewed changes
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @robertd, thanks for doing this! Looks like the changes fall into the following categories:

  • integ test version updates -> these are all fine
  • hash changes -> also fine
  • updating lambda runtimes -> the manual work you did, also great
  • weird things here and there that I tried to call out. not necessarily anything you can do, but wanted to call attention to them.
  • the does-not-exist.json file, which shouldn't be in this PR :).

Want to get this in fast, so I'm going to investigate a few of my own comments, hope that's ok.

packages/aws-cdk/does-not-exist.json Show resolved Hide resolved
...pshot/asset.41fc8f2dc7c01b34dda9916c7f763e7b7909eb629da9ffe879cb786114aae736.bundle/index.js Show resolved Hide resolved
...ambda/test/runtime.inlinecode.integ.snapshot/aws-cdk-lambda-runtime-inlinecode.template.json Show resolved Hide resolved
.../s3-bucket.lit.integ.snapshot/aws-cdk-firehose-delivery-stream-s3-all-properties.assets.json Show resolved Hide resolved
...es/@aws-cdk-containers/ecs-service-extensions/test/assign-public-ip.integ.snapshot/tree.json Show resolved Hide resolved
@kaizencc kaizencc added the pr/do-not-merge This PR should not be merged at this time. label Jun 3, 2022
@kaizencc kaizencc added effort/medium Medium work item – several days of effort and removed pr/do-not-merge This PR should not be merged at this time. effort/small Small work item – less than a day of effort labels Jun 3, 2022
@mergify
Copy link
Contributor

mergify bot commented Jun 3, 2022

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 mergify bot merged commit 4537b3f into aws:main Jun 3, 2022
@robertd
Copy link
Contributor Author

robertd commented Jun 3, 2022

@kaizencc & @jogold Thanks for stepping in and taking a look. Much appreciated. New integ was really messing with me at the end there since I've been doing this bulky PR three times now :D. I believe new integ workflows require some more polishing down the road (my personal observation), as I was getting some changes introduced that I wasn't able to explain logically where they are coming from. New integ process is definitely more verbose and touches larger file surface than the process before. I was running yarn integ-runner --update-on-failed --dry-run --parallel-regions us-west-2 in every module that was affected by this change.

Again... thanks for stepping in and pushing this across the finish line. ❤️

@robertd robertd deleted the robertd/lambda branch June 4, 2022 03:12
@McDoit
Copy link
Contributor

McDoit commented Jun 14, 2022

Would be utterly amazing to get this backported to CDKv1, as migrations to v2 might take time but deploys will stop working for older nodejs versions on v1...

@tmitchel2
Copy link

This is causing failures because some aws code build images dont support nodejs 14 yet as per #20739

daschaa pushed a commit to daschaa/aws-cdk that referenced this pull request Jul 9, 2022
@robertd @daschaa
feat(lambda): Migrate away from NODEJS_10_X and NODEJS_12_X to NODEJS…
48fd66f 
…_14_X (aws#20595)

Spiritual successor of aws#20531

<img width="487" alt="Screen Shot 2022-06-01 at 9 08 18 PM" src="https://user-images.githubusercontent.com/31543/171552028-7153063a-1c19-4972-ab89-6f280a4e9326.png">

- Migrated away (tests, docs, internal use) from NODEJS_10_X to NODEJS_14_X
- Migrated away (tests, docs, internal use) from NODEJS_12_X to NODEJS_14_X
- Added NODEJS_16_X to CustomResources
- Update CustomResources to use nodejs14.x by default
- Updated integration tests across the board. Some integ tests introduced destructive changes (expected with change of nodejs runtime)

Closes aws#20568
Closes aws#19992
Closes aws#20474

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

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

Do you know when this will be release @robertd? I can't see this PR referenced in CHANGELOG and AWS is hassling me about out-of-support Lambda functions...

@jacobtolar
Copy link

@NickDarvey Looks like it is in 2.28.0 or higher ( CHANGELOG.v2.md ) but was never backported to 1.x

@robertd
Copy link
Contributor Author

robertd commented Aug 15, 2022

What @jacobtolar said. I'd suggest updating to CDK v2 if you can.

@keisukekomeda keisukekomeda mentioned this pull request Aug 17, 2022
Closed
2 tasks
rix0rrr added a commit that referenced this pull request Sep 29, 2022
@rix0rrr
fix: change Node 10 to 14 everywhere
f469cd6 
This is a backport of #20595.
@rix0rrr rix0rrr mentioned this pull request Sep 29, 2022
Merged
mergify bot pushed a commit that referenced this pull request Sep 30, 2022
@rix0rrr
fix: change Node 10 to 14 everywhere (#22289)
8b0757e 
This is a backport of #20595.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@awsdataarchitect awsdataarchitect mentioned this pull request May 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaizencc kaizencc kaizencc approved these changes

@jogold jogold jogold left review comments

@pahud pahud pahud left review comments

Assignees
No one assigned
Labels
bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

custom_resources: Node version 12 out of support
9 participants
@robertd @aws-cdk-automation @pahud @McDoit @tmitchel2 @NickDarvey @jacobtolar @jogold @kaizencc