Validate current node version #19154

mistic · 2018-05-17T00:32:17Z

This pull request should close the issue #12976 and it's also a replacement for the one already opened #16811 (we must close the old one).

Basically this pull request will allow Kibana to check if the NodeJS version in use satisfies the one defined into the package.json.

The tests were built with jest, as required in the contributing guidelines, so currently both mocha and jest will search for unit tests inside src/utils folder.

elasticmachine · 2018-05-17T02:39:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

epixa · 2018-05-17T18:22:16Z

@mistic I made a minor tweak to this title to make it a bit more clear what it was about

mistic · 2018-05-17T18:30:02Z

@epixa good one. I just left on the title the same name I have in the branch but this one it's more clear!

jbudz · 2018-05-21T15:45:37Z

src/cli/cli.js

 import Command from './command';
 import serveCommand from './serve/serve';

 const argv = process.env.kbnWorkerArgv ? JSON.parse(process.env.kbnWorkerArgv) : process.argv.slice();
 const program = new Command('bin/kibana');

+// Validates current the NodeJS version compatibility when Kibana starts.
+NodeVersion.runValidator(() => {


i'm thinking we'll want to run this as part of the server start process here.

we'll have the standard logger available

we'll have config available

there's going to be more bootstrap checks in the future and we'll have more info available at that point. the downside is we may fatal beforehand if someone's a breaking change, but i'm okay with that. thoughts?

Good point @jbudz. Thinking about the reasons (1 and 2) you provide I really agree with you!

I'd actually like to see this check at the first possible moment (before even babel-register) because requiring a specific node version sometimes means that we won't even be able to get to the server start process without hitting a syntax error or something.

if i'm honest with myself this was a ploy to provide a config for disabling the version check. in the above case

syntax error, server doesn't start

api has changed, server doesn't fatal, version check exits

as long as it's before any writes the worst case is a vague error, but the person knows they've been messing with things at that point.

i'm still not a fan of forcing an exact version match with no workarounds. package mangers, manual security updates and swapping distributions all seem valid to me. with that said i'm in the minority on this so lets leave it and if there's an issue we can revisit

If the version check is in a module by itself we can offer a workaround 😉

echo '' > src/babel-register/node_version_check.js

Please don't tell anyone to change Kibana source files :P

whoops, by leave it as is i mean ignore my comment. i missed the module part above.

@spalger that was the reason I had in my head when I first choose to run the code inside the cli file but thinking a bit better even running it in CLI won't preventing syntax errors related with babel when running in Node <= 4.

In order to fully support this behaviour I need to rewrite node_version.js as an ES5 module and run it from src/babel-register/index.js.

@spalger @jbudz @tylersmalley do you agree with it?

We almost went there in https://github.com/elastic/kibana/pull/19104/files/bd0f4e1bfe41abe39f9d61caa8a583b10cc80f58#diff-202f54e3b2f5233afda1e3c964211e5d, but maybe this pushes it over the edge and it's time to create a src/setup_node_env.js module or something that combines babel-register, ts-node/register, and the node version check...

…de version validator.

tylersmalley · 2018-05-22T17:34:38Z

package.json

@@ -192,7 +192,7 @@
    "rison-node": "1.0.0",
    "rxjs": "5.4.3",
    "script-loader": "0.7.2",
-    "semver": "5.1.0",
+    "semver": "5.5.0",


Can we update this to use a caret and track the most recent minor?

Yes ofc @tylersmalley! I have already done it.

…or versions.

spalger · 2018-05-22T18:03:12Z

src/setup_node_env/node_version_validator.js

@@ -0,0 +1,15 @@
+/* eslint-disable no-var */


Rather than overrides like this, and changes in other files that aren't enforced with eslint but are designed to ensure the node_version check can run before node craps out...

I think we should split this eslint config override into two:

files that run before the node_version check and aren't allowed to use things like let, const, or destructuring syntax

scripts/*

src/setup_node_env/*

files that are not babel-ified but run after the node version check and can use anything except import

most of the items currently in https://github.com/elastic/kibana/blob/master/.eslintrc.js#L140-L166

spalger · 2018-05-22T18:03:31Z

src/setup_node_env/index.js

@@ -0,0 +1,3 @@
+require('./node_version_validator');
+require('./ts_node_register');


Lets keep the ts-node stuff in with babel_register, with babel 7 it actually will be a part of babel_register, and I think it's important that places like the plugin helpers that are pulling in babel_register specifically get typescript support too

mistic · 2018-05-23T18:18:37Z

@spalger I have already completed the changes you requested 🎉
Let me know about your thoughts now 😎

…dator inside setup_node_env folder.

…view Node version validator elastic#12976 pr review 3

spalger

LGTM, @jbudz @tylersmalley one of you want to take another look at this?

…tic/kibana into node-version-validator-elastic#12976

elasticmachine · 2018-05-24T20:38:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

jbudz

Works for me. We can probably remove the version check from yarn kbn bootstrap now too? (doesn't need to be in this PR

* feat(12976): node version validation at runtime. * refact(12976): move the code into a static utilities class. * test(12976): added first test case using jest. * test(12976): added test cases for node_version. * feat(12976): create setup env node to bootstrap babel, ts-node and node version validator. * refact(12976): migrated node version code from es6 to es5. * feat(12976): node version validation at runtime. * refact(12976): move the code into a static utilities class. * test(12976): added first test case using jest. * test(12976): added test cases for node_version. * feat(12976): create setup env node to bootstrap babel, ts-node and node version validator. * refact(12976): migrated node version code from es6 to es5. * fix(12976): remove one level from ts node register cache directory link. * chore(12976): added caret to semver dependecy in order to support minor versions. * refact(12976): small change from named import to default import on node version validator. * refact(12976): removed ts_node_register and add the code to babel_register. * feat(12976): split eslint config in order to properly support files built to run before and after node version validator. refact(12976): convert script files to es5 code. refact(12976): delete inline eslint configs from node version check related files. * refact(12976): remove ts node register file. * refact(12976): completely port setup_node_env to es5. * refact(12976): remove babel_register invokation from external dependencies in scripts. * refact(12976): move node_version code directly into node_version_validator inside setup_node_env folder. * refact(12976): only node version validator for kbn script.

mistic · 2018-05-25T18:02:36Z

6.x/6.4: 229fd27

* Validate current node version (#19154) * feat(12976): node version validation at runtime. * refact(12976): move the code into a static utilities class. * test(12976): added first test case using jest. * test(12976): added test cases for node_version. * feat(12976): create setup env node to bootstrap babel, ts-node and node version validator. * refact(12976): migrated node version code from es6 to es5. * feat(12976): node version validation at runtime. * refact(12976): move the code into a static utilities class. * test(12976): added first test case using jest. * test(12976): added test cases for node_version. * feat(12976): create setup env node to bootstrap babel, ts-node and node version validator. * refact(12976): migrated node version code from es6 to es5. * fix(12976): remove one level from ts node register cache directory link. * chore(12976): added caret to semver dependecy in order to support minor versions. * refact(12976): small change from named import to default import on node version validator. * refact(12976): removed ts_node_register and add the code to babel_register. * feat(12976): split eslint config in order to properly support files built to run before and after node version validator. refact(12976): convert script files to es5 code. refact(12976): delete inline eslint configs from node version check related files. * refact(12976): remove ts node register file. * refact(12976): completely port setup_node_env to es5. * refact(12976): remove babel_register invokation from external dependencies in scripts. * refact(12976): move node_version code directly into node_version_validator inside setup_node_env folder. * refact(12976): only node version validator for kbn script. * Fix check_file_casing babel_register reference (#19457) * chore(12976): fixed license headers on new files.

k3a · 2018-08-26T13:11:13Z

I am using Arch Linux which has rolling updates so it tries to have latest packages.
After recent update Kibana won't start with
Kibana does not support the current Node.js version v10.9.0. Please use Node.js v8.11.4.

After removing this check from setup_node_env/node_version_validator.js, it started and looks quite normal.

Are you sure it is a good idea to prevent your software to work with newer Nodejs without any concrete incompatibility reason?

mistic · 2018-08-27T13:38:17Z

@k3a we have this kind of check because officially we only support the nove version we ship with each version (only one certain node version at a time).

k3a · 2018-08-27T14:19:08Z

I see but perhaps you could consider adding some way to override this behavior. For example by --unsupported flag or something like that? I.e. to allow it run with any Node while making sure the one running it knows it is unsupported and may not work well?

…

On August 27, 2018 3:38:39 PM GMT+02:00, Tiago Costa ***@***.***> wrote: @k3a we have this kind of check because officially we only support the nove version we ship with each version (only one certain node version at a time). -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #19154 (comment)

epixa · 2018-08-27T14:24:25Z

Given that we ship the node.js binary with our Kibana distributions and that Kibana could break entirely with any node.js version that isn't the one we test against, what's the use case for running with different node versions? Is this a dev-only concern?

mistic · 2018-08-27T15:00:16Z

@k3a Also you don't need to remove setup_node_env/node_version_validator.js. If you need/want to run another node version than the one we support, you just need to update the package.json engines.node field to the version you want to run. However as @epixa mentioned, using a non officially supported version could make Kibana break entirely.

Stargateur · 2020-11-25T10:02:25Z

#84266

that ridiculous, you are creating problem that shouldn't exist, node 10 is lts support, follow semver, there is no reason to do this. You disallow any system that follow semver.

Just put as a warning if you want not hard error !

mistic added review Team:Operations Team label for Operations Team labels May 17, 2018

mistic requested a review from jbudz May 17, 2018 00:32

elastic deleted a comment from elasticmachine May 17, 2018

epixa changed the title ~~Node version validator #12976~~ Validate current node version May 17, 2018

jbudz reviewed May 21, 2018

View reviewed changes

jbudz requested a review from tylersmalley May 21, 2018 15:47

jbudz mentioned this pull request May 21, 2018

Add node js version warning message #16811

Closed

jbudz approved these changes May 21, 2018

View reviewed changes

mistic added 4 commits May 22, 2018 17:20

feat(12976): node version validation at runtime.

53adf3c

refact(12976): move the code into a static utilities class.

8c81717

test(12976): added first test case using jest.

2cef843

test(12976): added test cases for node_version.

3bf0ff7

mistic force-pushed the node-version-validator-#12976 branch from 4b62802 to 3bf0ff7 Compare May 22, 2018 16:20

mistic added 9 commits May 22, 2018 17:24

feat(12976): create setup env node to bootstrap babel, ts-node and no…

b3d61c9

…de version validator.

refact(12976): migrated node version code from es6 to es5.

389e29a

feat(12976): node version validation at runtime.

0a5a1bb

refact(12976): move the code into a static utilities class.

22d6bcc

test(12976): added first test case using jest.

11f6014

test(12976): added test cases for node_version.

c6ba7d2

feat(12976): create setup env node to bootstrap babel, ts-node and no…

170bfff

…de version validator.

refact(12976): migrated node version code from es6 to es5.

61ab0b9

fix(12976): remove one level from ts node register cache directory link.

82650c4

mistic force-pushed the node-version-validator-#12976 branch from e213b74 to 82650c4 Compare May 22, 2018 16:59

tylersmalley reviewed May 22, 2018

View reviewed changes

chore(12976): added caret to semver dependecy in order to support min…

5e25d23

…or versions.

spalger suggested changes May 22, 2018

View reviewed changes

elastic deleted a comment from elasticmachine May 23, 2018

Merge branch 'master' into node-version-validator-elastic#12976

ff4dee5

elastic deleted a comment from elasticmachine May 23, 2018

Merge branch 'master' into node-version-validator-elastic#12976

0bf6d16

elastic deleted a comment from elasticmachine May 24, 2018

mistic added 2 commits May 24, 2018 19:20

refact(12976): move node_version code directly into node_version_vali…

9fee0e0

…dator inside setup_node_env folder.

Merge pull request #3 from mistic/node-version-validator-#12976-pr-re…

5478934

…view Node version validator elastic#12976 pr review 3

spalger approved these changes May 24, 2018

View reviewed changes

mistic added 2 commits May 24, 2018 20:19

refact(12976): only node version validator for kbn script.

1345168

Merge branch 'node-version-validator-elastic#12976' of github.com:mis…

6e82a1f

…tic/kibana into node-version-validator-elastic#12976

elastic deleted a comment from elasticmachine May 24, 2018

jbudz approved these changes May 24, 2018

View reviewed changes

mistic added v7.0.0 v6.4.0 labels May 25, 2018

mistic merged commit 194aba1 into elastic:master May 25, 2018

mistic mentioned this pull request May 25, 2018

[6.x] Validate current node version (#19154) #19452

Merged

jbudz mentioned this pull request Oct 23, 2018

Fail at startup if the current node version is not exactly the same as .node-version #17456

Closed

Stargateur mentioned this pull request Nov 25, 2020

Node version compatibility #84266

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate current node version #19154

Validate current node version #19154

mistic commented May 17, 2018

elasticmachine commented May 17, 2018

epixa commented May 17, 2018

mistic commented May 17, 2018 •

edited

jbudz May 21, 2018

mistic May 21, 2018

spalger May 21, 2018

jbudz May 21, 2018

spalger May 21, 2018

epixa May 21, 2018

jbudz May 21, 2018

mistic May 21, 2018

spalger May 21, 2018

tylersmalley May 22, 2018

mistic May 22, 2018

spalger May 22, 2018 •

edited

spalger May 22, 2018

mistic commented May 23, 2018

spalger left a comment •

edited

elasticmachine commented May 24, 2018

jbudz left a comment

mistic commented May 25, 2018 •

edited

k3a commented Aug 26, 2018

mistic commented Aug 27, 2018

k3a commented Aug 27, 2018 via email

epixa commented Aug 27, 2018

mistic commented Aug 27, 2018 •

edited

Stargateur commented Nov 25, 2020 •

edited

		@@ -0,0 +1,3 @@
		require('./node_version_validator');
		require('./ts_node_register');

Validate current node version #19154

Validate current node version #19154

Conversation

mistic commented May 17, 2018

elasticmachine commented May 17, 2018

💚 Build Succeeded

epixa commented May 17, 2018

mistic commented May 17, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalger May 22, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mistic commented May 23, 2018

spalger left a comment • edited

Choose a reason for hiding this comment

elasticmachine commented May 24, 2018

💚 Build Succeeded

jbudz left a comment

Choose a reason for hiding this comment

mistic commented May 25, 2018 • edited

k3a commented Aug 26, 2018

mistic commented Aug 27, 2018

k3a commented Aug 27, 2018 via email

epixa commented Aug 27, 2018

mistic commented Aug 27, 2018 • edited

Stargateur commented Nov 25, 2020 • edited

mistic commented May 17, 2018 •

edited

spalger May 22, 2018 •

edited

spalger left a comment •

edited

mistic commented May 25, 2018 •

edited

mistic commented Aug 27, 2018 •

edited

Stargateur commented Nov 25, 2020 •

edited