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

[SPARK-44279][BUILD] Upgrade optionator to ^0.9.3 #41955

Closed
wants to merge 4 commits into from

Conversation

bjornjorgensen
Copy link
Contributor

@bjornjorgensen bjornjorgensen commented Jul 12, 2023

What changes were proposed in this pull request?

This PR proposes a change in the package.json file to update the resolution for the optionator package to ^0.9.3.

I've added a resolutions field to package.json and specified the optionator package version as ^0.9.3.
This will ensure that our project uses optionator version 0.9.3 or the latest minor or patch version (due to the caret ^), regardless of any other version that may be specified in the dependencies or nested dependencies of our project.

Why are the changes needed?

CVE-2023-26115

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA

@github-actions github-actions bot added the BUILD label Jul 12, 2023
@bjornjorgensen
Copy link
Contributor Author

bjornjorgensen commented Jul 12, 2023

jonschlinkert/word-wrap#40

@sarutak and @srowen

$ npm install

added 1 package, removed 1 package, changed 2 packages, and audited 119 packages in 525ms

15 packages are looking for funding
  run `npm fund` for details

3 moderate severity vulnerabilities

To address all issues, run:
  npm audit fix

Run `npm audit` for details.
(base) [bjorn@amd7g dev]$ npm audit fix

added 1 package, removed 1 package, changed 2 packages, and audited 119 packages in 731ms

15 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

@sarutak
Copy link
Member

sarutak commented Jul 12, 2023

@bjornjorgensen
Thank you fixing this issue.
How about adding "optionator": "^0.9.3" to package.json rather than using forked repo?
Also, could you please fill out the description because it's recorded as commit log.

@bjornjorgensen
Copy link
Contributor Author

(base) [bjorn@amd7g dev]$ npm install

up to date, audited 119 packages in 495ms

15 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

@sarutak
Copy link
Member

sarutak commented Jul 12, 2023

Oh wait, ESLint v8.44.0 seems to be already released, which includes the CVE fix.
https://github.com/eslint/eslint/releases/tag/v8.44.0
eslint/eslint@cf88439

@bjornjorgensen It's better to upgrade ESLint right?

@bjornjorgensen
Copy link
Contributor Author

hmm.. eslint/eslint#17319 yes, eslint v8.44.0 have this fix.

ok, I can try to upgrade eslint.
I have never used this so I can tell whats better or not. :)

@bjornjorgensen
Copy link
Contributor Author

(base) [bjorn@amd7g dev]$ npm install

added 22 packages, removed 44 packages, changed 21 packages, and audited 97 packages in 37s

23 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

@bjornjorgensen
Copy link
Contributor Author

but now we get "node_modules/@aashutoshrathi/word-wrap" witch is the forked repo.

@sarutak
Copy link
Member

sarutak commented Jul 12, 2023

@bjornjorgensen The change SGTM.
Could you update the title and description so that we can easily understand this PR upgrades ESLInt to v8.44.0?

but now we get "node_modules/@aashutoshrathi/word-wrap" witch is the forked repo.

I think it's no problem because Spark doesn't directly depend on the forked word-wrap and not need to maintain it.

@bjornjorgensen bjornjorgensen changed the title [SPARK-44279][BUILD] Upgrade word-wrap [SPARK-44279][BUILD] Upgrade Eslint to v8.44.0 Jul 12, 2023
@bjornjorgensen
Copy link
Contributor Author

yes, and I have updated the JIRA ticket for eslint.

@sarutak
Copy link
Member

sarutak commented Jul 12, 2023

@bjornjorgensen
Please mention which change in the release note fixes the CVE in the description to justify this change.
For example

The following change in the release note fixes the CVE.
[cf88439](https://github.com/eslint/eslint/commit/cf884390ad8071d88eae05df9321100f1770363d) chore: upgrade optionator@0.9.3 (https://github.com/eslint/eslint/pull/17319) (Milos Djermanovic)

Copy link
Member

@sarutak sarutak left a comment

Choose a reason for hiding this comment

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

LGTM. Waiting for the CI.

@bjornjorgensen
Copy link
Contributor Author

We got an error

Run ./dev/lint-js
  ./dev/lint-js
  shell: sh -e {0}
  env:
    LC_ALL: C.UTF-8
    LANG: C.UTF-8
    PYSPARK_DRIVER_PYTHON: python3.9
    PYSPARK_PYTHON: python3.9
    GITHUB_PREV_SHA: c63[2](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:2)b[3](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:3)11e9f66eb5ec6[4](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:4)49779fd6164fc698a91b
    APACHE_SPARK_REF: d9248e83bbb3af49333608bebe7149b1aaeca738
    JAVA_HOME: /__t/Java_Temurin-Hotspot_jdk/8.0.372-7/x64
    JAVA_HOME_8_X64: /__t/Java_Temurin-Hotspot_jdk/8.0.372-7/x64
npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but package-lock.json was generated for lockfileVersion@2. I'll try to do my best with it!
added 96 packages in 1.261s

Oops! Something went wrong! :(

ESLint: 8.44.0

TypeError: Module.createRequire is not a function
    at Object.<anonymous> (/__w/spark/spark/dev/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:239[5](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:5):2[6](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:6))
    at Module._compile (internal/modules/cjs/loader.js:[7](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:7)7[8](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:8):30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:78[9](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:9):[10](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:10))
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:[12](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:12))
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:[17](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:18))
    at require (internal/modules/cjs/helpers.js:25:[18](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:19))
    at Object.<anonymous> (/__w/spark/spark/dev/node_modules/eslint/lib/cli-engine/cli-engine.js:33:5)
    at Module._compile (internal/modules/cjs/loader.js:778:[30](https://github.com/bjornjorgensen/spark/actions/runs/5534166744/jobs/10100472963#step:20:31))
lint-js checks failed.

@bjornjorgensen
Copy link
Contributor Author

The error message TypeError: Module.createRequire is not a function suggests that the version of Node.js you are running is not compatible with the version of ESLint you're trying to use (v8.44.0 in this case). The Module.createRequire function is a method in Node.js that was added in version 12.2.0.

ESLint 8.x requires Node.js ^12.22.0 || ^14.17.0 || >=16.0.0. If your Node.js version is below these, you may encounter the error. To solve this issue, you need to upgrade your Node.js to a compatible version.

You can check your Node.js version by running node -v in your terminal. If the version is below 12.22.0, you will need to upgrade it. Here are the general steps to upgrade Node.js:

  1. Clear npm's cache: npm cache clean -f
  2. Install n (Node's version manager): npm install -g n
  3. Install latest Node.js version: n stable or a specific version n 12.22.0

After upgrading Node.js, try running ESLint again. If you still get errors, you might want to delete your node_modules folder and package-lock.json file (if you're using npm) or yarn.lock (if you're using yarn), and then run npm install or yarn install to reinstall your packages.

Please make sure to update your CI/CD scripts as well to use a compatible Node.js version if they are also running ESLint.

As the log seems to be from a GitHub Actions run, you might need to specify the Node.js version in your GitHub Actions configuration (the .github/workflows/*.yml files), using something like the actions/setup-node action with the node-version parameter.

@sarutak
Copy link
Member

sarutak commented Jul 12, 2023

@bjornjorgensen
Hmm, the container image for the CI seems based on Ubuntu 20.04, which provides an old version of nodejs.
So, let's just add "optionator": "^0.9.3" to devDependencies section in package.json for now.

@bjornjorgensen bjornjorgensen changed the title [SPARK-44279][BUILD] Upgrade Eslint to v8.44.0 [SPARK-44279][BUILD] Upgrade optionator to ^0.9.3 Jul 12, 2023
@bjornjorgensen bjornjorgensen changed the title [SPARK-44279][BUILD] Upgrade optionator to ^0.9.3 DONT MERGE [SPARK-44279][BUILD] Upgrade optionator to ^0.9.3 Jul 12, 2023
@bjornjorgensen bjornjorgensen changed the title DONT MERGE [SPARK-44279][BUILD] Upgrade optionator to ^0.9.3 [SPARK-44279][BUILD] Upgrade optionator to ^0.9.3 Jul 12, 2023
@sarutak
Copy link
Member

sarutak commented Jul 13, 2023

Merging to master. Thank you @bjornjorgensen !

@sarutak sarutak closed this in d35fda6 Jul 13, 2023
@sarutak
Copy link
Member

sarutak commented Jul 13, 2023

NOTE: Once we upgrade ESLint to +v8.44.0 in the future, we can remove this change.

@bjornjorgensen bjornjorgensen deleted the word-wrap branch July 14, 2023 10:53
@bjornjorgensen
Copy link
Contributor Author

Hi @sarutak I did run docker build spark/dev/create-release/spark-rm
/Dockerfile and then I get this

Processing triggers for libc-bin (2.31-0ubuntu9.12) ...

================================================================================
================================================================================

                              DEPRECATION WARNING

  Node.js 12.x is no longer actively supported!

  You will not receive security or critical stability updates for this version.

  You should migrate to a supported version of Node.js as soon as possible.
  Use the installation script that corresponds to the version of Node.js you
  wish to install. e.g.

   * https://deb.nodesource.com/setup_16.x — Node.js 16 "Gallium"
   * https://deb.nodesource.com/setup_18.x — Node.js 18 LTS "Hydrogen" (recommended)
   * https://deb.nodesource.com/setup_19.x — Node.js 19 "Nineteen"
   * https://deb.nodesource.com/setup_20.x — Node.js 20 "Iron" (current)

  Please see https://github.com/nodejs/Release for details about which
  version may be appropriate for you.

  The NodeSource Node.js distributions repository contains
  information both about supported versions of Node.js and supported Linux
  distributions. To learn more about usage, see the repository:
    https://github.com/nodesource/distributions

================================================================================
================================================================================

Continuing in 20 seconds ...


================================================================================
▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓
================================================================================

                           SCRIPT DEPRECATION WARNING


  This script, located at https://deb.nodesource.com/setup_X, used to
  install Node.js is deprecated now and will eventually be made inactive.

  Please visit the NodeSource distributions Github and follow the
  instructions to migrate your repo.
  https://github.com/nodesource/distributions

  The NodeSource Node.js Linux distributions GitHub repository contains
  information about which versions of Node.js and which Linux distributions
  are supported and how to install it.
  https://github.com/nodesource/distributions


                          SCRIPT DEPRECATION WARNING

================================================================================
▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓
================================================================================

TO AVOID THIS WAIT MIGRATE THE SCRIPT
Continuing in 60 seconds (press Ctrl-C to abort) ...

Should we try to upgrade node.js to version 18 for spark 4.0.0 ? also cc @dongjoon-hyun

@dongjoon-hyun
Copy link
Member

Thank you for ccing me, @bjornjorgensen .

@sarutak
Copy link
Member

sarutak commented Nov 20, 2023

@bjornjorgensen
Thank you for pinging me and sorry for the late reply.
I reproduced this issue and I'm OK to upgrade Node.js and NPM.

@dongjoon-hyun
BTW, if we upgrade them in spark/dev/create-release/spark-rm/Dockerfile, should we upgrade them in dev/infra/Dockerfile for the consistency?

ragnarok56 pushed a commit to ragnarok56/spark that referenced this pull request Mar 2, 2024
### What changes were proposed in this pull request?
This PR proposes a change in the package.json file to update the resolution for the `optionator` package to ^0.9.3.

I've added a resolutions field to package.json and specified the `optionator` package version as ^0.9.3.
This will ensure that our project uses `optionator` version 0.9.3 or the latest minor or patch version (due to the caret ^), regardless of any other version that may be specified in the dependencies or nested dependencies of our project.

### Why are the changes needed?
[CVE-2023-26115](https://nvd.nist.gov/vuln/detail/CVE-2023-26115)

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
Pass GA

Closes apache#41955 from bjornjorgensen/word-wrap.

Authored-by: Bjørn Jørgensen <bjornjorgensen@gmail.com>
Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants