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.

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

Switch to NodeJS LTS 20 "Iron" #9171

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hashworks
Copy link
Contributor

@hashworks hashworks commented Jan 10, 2025

NodeJS 18 "Hydrogen" will be EOL in April 2025.

Closes #8463.

Note: The latest LTS (22 "Jod") fails to build:

ERROR [bootstrap] failed:
ERROR Error: Command failed with exit code 1: /usr/lib/node_modules/yarn/bin/yarn.js run osd:bootstrap
      (node:2565) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
      (Use `node --trace-deprecation ...` to show where the warning was created)
      Node.js process-warning detected:

      DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
          at node:punycode:3:9
          at BuiltinModule.compileForInternalLoader (node:internal/bootstrap/realm:399:7)
          at BuiltinModule.compileForPublicLoader (node:internal/bootstrap/realm:338:10)
          at loadBuiltinModule (node:internal/modules/helpers:110:7)
          at Function._load (node:internal/modules/cjs/loader:1095:17)
          at TracingChannel.traceSync (node:diagnostics_channel:322:14)
          at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
          at Module.<anonymous> (node:internal/modules/cjs/loader:1335:12)
          at Module.patchedRequire (/build/opensearch-dashboards/src/OpenSearch-Dashboards/node_modules/require-in-the-middle/index.js:217:27)
          at Module.Hook._require.Module.require (/build/opensearch-dashboards/src/OpenSearch-Dashboards/node_modules/require-in-the-middle/index.js:142:27)

Changelog

  • security: Bump NodeJS to 20.18.1 "Iron" due to pending NodeJS v18 "Hydrogen" EOL (April '25)
  • security: Bump elastic-apm-node to 4.10.0 to support NodeJS v20

Tests

yarn test:jest failures

Due to test failures I'm marking this a draft. Could a reviewer check the test workflow result?

Test Suites: 6 failed, 1 skipped, 474 passed, 480 of 481 total
Tests:       8 failed, 28 skipped, 8 todo, 4667 passed, 4711 total
Snapshots:   1515 passed, 1515 total
Time:        384.071 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

NodeJS 18 "Hydrogen" will be EOL in April 2025.

Closes issue opensearch-project#8463.

Note: The latest LTS (22 "Jod") fails to build.

Signed-off-by: hashworks <[email protected]>
`3.43.0` fails with v20:
```
error [email protected]: The engine "node" is incompatible with this module. Expected version "^8.6.0 || 10 || 12 || 14 || 16 || 17 || 18 || 19". Got "20.18.1"
```

Signed-off-by: hashworks <[email protected]>
Copy link
Contributor

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

Copy link
Contributor

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

Copy link
Contributor

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

1 similar comment
Copy link
Contributor

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "Bump NodeJS to 20.18.1 "Iron" due to pending NodeJS v18 "Hydrogen" EOL (April '25)". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "chore". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "security". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

1 similar comment
Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "security". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@github-actions github-actions bot added Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry and removed failed changeset labels Jan 10, 2025
@hashworks
Copy link
Contributor Author

Regarding elastic-apm-node: We could go as low as 3.45.0, but I had more test failures with it than with the latest version (4.10.0).

@hashworks
Copy link
Contributor Author

Changelog would be:

  • security: Bump NodeJS to 20.18.1 "Iron" due to pending NodeJS v18 "Hydrogen" EOL (April '25)
  • security: Bump elastic-apm-node to 4.10.0 to support NodeJS v20

But it seems I'm not allowed to add it here.

Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.00%. Comparing base (e401613) to head (d49e58c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9171      +/-   ##
==========================================
- Coverage   61.02%   61.00%   -0.03%     
==========================================
  Files        3813     3813              
  Lines       91396    91396              
  Branches    14439    14439              
==========================================
- Hits        55771    55752      -19     
- Misses      32067    32082      +15     
- Partials     3558     3562       +4     
Flag Coverage Δ
Linux_1 ?
Linux_2 ?
Linux_3 ?
Linux_4 ?
Windows_1 29.09% <ø> (ø)
Windows_2 56.45% <ø> (+0.04%) ⬆️
Windows_3 38.03% <ø> (+<0.01%) ⬆️
Windows_4 29.03% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -27,8 +27,8 @@ In general, we recommend four tiers of tests:

# Requirements
* Install the latest NodeJS, [NPM](https://www.npmjs.com/get-npm) and [Yarn](https://classic.yarnpkg.com/en/docs/install/#mac-stable)
* `nvm install v18.19.0`
* Install the specified NodeJS, [NPM](https://www.npmjs.com/get-npm) and [Yarn](https://classic.yarnpkg.com/en/docs/install/#mac-stable)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would we require a "specific" version of Node.js for testing?

Copy link
Contributor Author

@hashworks hashworks Jan 10, 2025

Choose a reason for hiding this comment

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

If you install the latest NodeJS version (v23) it won't build.

Copy link
Member

Choose a reason for hiding this comment

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

nit: we may want to advise users to simply run nvm install so that they always get the version defined in our .nvmrc in the case these docs get outdated.

Not needed for this change though.

@d-rowe
Copy link
Member

d-rowe commented Jan 16, 2025

@hashworks thanks for taking the time to contribute! Taking a look at the build, test, and lint workflow it seems that the lock file may be outdated or generated with the prior node version. Can you quickly verify if thats the case?

@d-rowe
Copy link
Member

d-rowe commented Jan 16, 2025

Changelog would be:

  • security: Bump NodeJS to 20.18.1 "Iron" due to pending NodeJS v18 "Hydrogen" EOL (April '25)
  • security: Bump elastic-apm-node to 4.10.0 to support NodeJS v20

But it seems I'm not allowed to add it here.

If you add these in the Changelog section in your PR section, the changelog workflow should auto generate and commit these to the changelog file.

@hashworks
Copy link
Contributor Author

If you add these in the Changelog section in your PR section, the changelog workflow should auto generate and commit these to the changelog file.

I'll try again, but doing so caused the Bot Comment Spam above.

Copy link
Contributor

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "security". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

@github-actions github-actions bot added failed changeset and removed Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry labels Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] EOL of Node.js version 18.x used in Opensearch-dashboards
3 participants