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

[vtadmin-web] Enforce node + npm versions with .npmrc #9336

Merged
merged 3 commits into from
Dec 8, 2021

Conversation

doeg
Copy link
Contributor

@doeg doeg commented Dec 7, 2021

Signed-off-by: Sara Bee 855595+doeg@users.noreply.github.com

Description

This closes #9321 by enforcing installed node/npm versions match the semvers specified in the package.json's engines property. We do this with npm's engine-strict flag.

(Rant about "why does this even require a separate, special flag and yet another got-dang repo dotfile arghhhh" has been redacted.)

🚨 Going forward, make vtadmin_web_proto_types will fail if a developer's local node/npm install is too old. Here's what it looks like:

🍕~/workspace/vitess $ nvm use 14.15.3
Now using node v14.15.3 (npm v6.14.9)

🍕~/workspace/vitess $ make vtadmin_web_proto_types
npm ERR! code ENOTSUP
npm ERR! notsup Unsupported engine for vtadmin@0.1.0: wanted: {"node":">=16.13.0","npm":">=8.1.0"} (current: {"node":"14.15.3","npm":"6.14.9"})
npm ERR! notsup Not compatible with your version of node/npm: vtadmin@0.1.0
npm ERR! notsup Not compatible with your version of node/npm: vtadmin@0.1.0
npm ERR! notsup Required: {"node":">=16.13.0","npm":">=8.1.0"}
npm ERR! notsup Actual:   {"npm":"6.14.9","node":"14.15.3"}

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/sarabee/.npm/_logs/2021-12-07T19_47_41_543Z-debug.log
make: *** [vtadmin_web_install] Error 1

🍕~/workspace/vitess $ nvm use 16.13.0
Now using node v16.13.0 (npm v8.1.0)

🍕~/workspace/vitess $ make vtadmin_web_proto_types

up to date, audited 2208 packages in 3s

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

39 vulnerabilities (22 moderate, 15 high, 2 critical)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

🍕~/workspace/vitess $

(Should probably do an npm audit soon... 🤔 Also, I'm not convinced this output is super... approachable? But it's better than nothing.)

This check also runs (and is enforced in) CI, since npm install is a prerequisite for... well, all of our front-end tests/linters/formatters/etc. :) Here's what that looks like when I intentionally downgrade the GitHub Action's node version:

Screen Shot 2021-12-07 at 2 53 40 PM

(Note: the linters/formatters running even if npm install fails is "by design" (ugh), described in vtadmin_web_lint.yml below.)

Finally, I ran make vtadmin_web_proto_types and committed the changes. The chonky package-lock.json diff is due to the thing described in #9321.

Related Issue(s)

Checklist

  • Should this PR be backported? No
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

Anyone running VTAdmin will now be required to have the local node version (on their machine, in the Dockerfile, etc.) match the versions specified in the package.json file. (I added the "release notes" tag in case this is worth mentioning.)

doeg added 3 commits December 7, 2021 14:38
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
@@ -344,6 +330,20 @@ export namespace vtadmin {
*/
public getWorkflows(request: vtadmin.IGetWorkflowsRequest): Promise<vtadmin.GetWorkflowsResponse>;

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

thank you! :-)

@doeg doeg merged commit 130875a into vitessio:main Dec 8, 2021
@doeg doeg deleted the sarabee-npmrc branch December 8, 2021 15:03
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.

[vtadmin-web] Enforce node version in CI
2 participants