From c4c8367091169937a05887291619a45402c0107e Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Wed, 10 Nov 2021 17:59:03 +0100 Subject: [PATCH] fix(dependency): updated tar to 6.x in shared-metrics (#415) refs 75234 Tar 6 has already dropped Node support < 10 See https://github.com/npm/node-tar/blob/065e3850dfcfa439fd7d4bce0407ba616f85f576/package.json#L48 We currently receive security updates in tar 5.x, but that can stop quickly, because Node 8 support has stopped in 2020 already. We should just update tar to 6.x in shared-metrics because: Node 8 (and lower) is officially not supported for two more packages: - semver 7.x - lru-cache@6.x (and there are probably tons of more, which just don't make use of the engines keyword) If you set `npm config set engine-strict true`, you will see that you cannot install collector with Node 8 (without the tar update). That menas, it is already a problem. `tar` is used in nativeModuleRetry#copyPrecompiled (https://github.com/instana/nodejs/blob/dc2160bddab5a25d0b6bab25c5d10bee8b5f1bd5/packages/shared-metrics/src/util/nativeModuleRetry.js#L75), but this piece of code is not covered in our tests. Where is nativeModuleRetry script used? Only in shared-metrics. - in gc.js - in lubuv.js Options: 1) ~Add tests for copyPrecompiled to ensure that Node 8 still works~ There is a test! 2) Disallow copyPrecompiled for Node < 10 3) Do nothing, because the execution of `tar.x` is catched in the codebase and the user won't experience anything bad. But (!) if anything goes wrong with requiring tar in Node 8 or before `tar.x` is executed and it's not catched, we can bring down the customers application. We'd need to try/catch the require statement + tar.x ---- Other usages of `tar`: - in tests - in dummy apps - in docker examples - in subdependencies of dev dependencies - these should be updated in a separate and general version update We decided to go with 2). Co-authored-by: kirrg001 --- .../collector/test/nativeModuleRetry/test.js | 31 ++++++---- packages/shared-metrics/package-lock.json | 60 ++++++++++--------- packages/shared-metrics/package.json | 3 +- packages/shared-metrics/src/gc.js | 3 +- packages/shared-metrics/src/libuv.js | 4 +- .../src/util/nativeModuleRetry.js | 24 ++++++-- 6 files changed, 78 insertions(+), 47 deletions(-) diff --git a/packages/collector/test/nativeModuleRetry/test.js b/packages/collector/test/nativeModuleRetry/test.js index 02c8431213..0b4f73030a 100644 --- a/packages/collector/test/nativeModuleRetry/test.js +++ b/packages/collector/test/nativeModuleRetry/test.js @@ -7,6 +7,7 @@ const async = require('async'); const { expect } = require('chai'); +const semver = require('semver'); const fs = require('fs'); const os = require('os'); const path = require('path'); @@ -38,7 +39,7 @@ describe('retry loading native addons', function () { let foundAtLeastOneUnsupported; for (let i = 0; i < allMetrics.length; i++) { if (allMetrics[i].data.libuv) { - expect(allMetrics[i].data.libuv.statsSupported).to.not.exist; + expect(allMetrics[i].data.libuv.statsSupported).to.be.false; foundAtLeastOneUnsupported = true; break; } @@ -50,11 +51,16 @@ describe('retry loading native addons', function () { const libuv = aggregated.libuv; expect(libuv).to.exist; expect(libuv).to.be.an('object'); - expect(libuv.statsSupported).to.be.true; - expect(libuv.min).to.be.a('number'); - expect(libuv.max).to.be.a('number'); - expect(libuv.sum).to.be.a('number'); - expect(libuv.lag).to.be.a('number'); + + if (semver.lt(process.version, '10.0.0')) { + expect(libuv.statsSupported).to.be.false; + } else { + expect(libuv.statsSupported).to.be.true; + expect(libuv.min).to.be.a('number'); + expect(libuv.max).to.be.a('number'); + expect(libuv.sum).to.be.a('number'); + expect(libuv.lag).to.be.a('number'); + } } }, { @@ -67,7 +73,7 @@ describe('retry loading native addons', function () { let foundAtLeastOneUnsupported; for (let i = 0; i < allMetrics.length; i++) { if (allMetrics[i].data.libuv) { - expect(allMetrics[i].data.gc.statsSupported).to.not.exist; + expect(allMetrics[i].data.gc.statsSupported).to.be.false; foundAtLeastOneUnsupported = true; break; } @@ -79,9 +85,14 @@ describe('retry loading native addons', function () { const gc = aggregated.gc; expect(gc).to.exist; expect(gc).to.be.an('object'); - expect(gc.statsSupported).to.be.true; - expect(gc.minorGcs).to.exist; - expect(gc.majorGcs).to.exist; + + if (semver.lt(process.version, '10.0.0')) { + expect(gc.statsSupported).to.be.false; + } else { + expect(gc.statsSupported).to.be.true; + expect(gc.minorGcs).to.exist; + expect(gc.majorGcs).to.exist; + } } } ]; diff --git a/packages/shared-metrics/package-lock.json b/packages/shared-metrics/package-lock.json index 4109fe3e49..0269c9b864 100644 --- a/packages/shared-metrics/package-lock.json +++ b/packages/shared-metrics/package-lock.json @@ -388,6 +388,11 @@ "readdirp": "~3.2.0" } }, + "chownr": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/chownr/-/chownr-2.0.0.tgz", + "integrity": "sha512-bIomtDF5KGpdogkLd9VspvFzk9KfpyyGlS8YFVZl7TGPBHL5snIOnxeshwVgPteQ9b4Eydl+pVbIyE1DcvCWgQ==" + }, "cliui": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/cliui/-/cliui-5.0.0.tgz", @@ -698,6 +703,15 @@ "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==", "dev": true }, + "semver": { + "version": "7.3.5", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.5.tgz", + "integrity": "sha512-PoeGJYh8HK4BTO/a9Tf6ZG3veo/A7ZVsYrSA6J8ny9nb3B1VrpkuN+z9OE5wfE5p6H4LchYZsegiQgbJD94ZFQ==", + "dev": true, + "requires": { + "lru-cache": "^6.0.0" + } + }, "strip-ansi": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.0.tgz", @@ -1503,7 +1517,6 @@ "version": "6.0.0", "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-6.0.0.tgz", "integrity": "sha512-Jo6dJ04CmSjuznwJSS3pUeWmd/H0ffTlkXXgwZi+eq1UCmqQwCh+eLsYOYCwY991i2Fah4h1BEMCx4qThGbsiA==", - "dev": true, "requires": { "yallist": "^4.0.0" } @@ -1533,9 +1546,9 @@ "integrity": "sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw==" }, "minipass": { - "version": "3.1.3", - "resolved": "https://registry.npmjs.org/minipass/-/minipass-3.1.3.tgz", - "integrity": "sha512-Mgd2GdMVzY+x3IJ+oHnVM+KG3lA5c8tnabyJKmHSaG2kAGpudxuOf8ToDkhumF7UzME7DecbQE9uOZhNm7PuJg==", + "version": "3.1.5", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-3.1.5.tgz", + "integrity": "sha512-+8NzxD82XQoNKNrl1d/FSi+X8wAEWR+sbYAfIvub4Nz0d22plFG72CEVVaufV8PNf4qSslFTD8VMOxNVhHCjTw==", "requires": { "yallist": "^4.0.0" } @@ -1549,6 +1562,11 @@ "yallist": "^4.0.0" } }, + "mkdirp": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-1.0.4.tgz", + "integrity": "sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw==" + }, "mocha": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/mocha/-/mocha-7.2.0.tgz", @@ -2155,7 +2173,6 @@ "version": "7.3.5", "resolved": "https://registry.npmjs.org/semver/-/semver-7.3.5.tgz", "integrity": "sha512-PoeGJYh8HK4BTO/a9Tf6ZG3veo/A7ZVsYrSA6J8ny9nb3B1VrpkuN+z9OE5wfE5p6H4LchYZsegiQgbJD94ZFQ==", - "dev": true, "requires": { "lru-cache": "^6.0.0" } @@ -2406,31 +2423,16 @@ } }, "tar": { - "version": "5.0.11", - "resolved": "https://registry.npmjs.org/tar/-/tar-5.0.11.tgz", - "integrity": "sha512-E6q48d5y4XSCD+Xmwc0yc8lXuyDK38E0FB8N4S/drQRtXOMUhfhDxbB0xr2KKDhNfO51CFmoa6Oz00nAkWsjnA==", - "requires": { - "chownr": "^1.1.4", - "fs-minipass": "^2.1.0", - "minipass": "^3.1.3", - "minizlib": "^2.1.2", - "mkdirp": "^0.5.5", + "version": "6.1.11", + "resolved": "https://registry.npmjs.org/tar/-/tar-6.1.11.tgz", + "integrity": "sha512-an/KZQzQUkZCkuoAA64hM92X0Urb6VpRhAFllDzz44U2mcD5scmT3zBc4VgVpkugF580+DQn8eAFSyoQt0tznA==", + "requires": { + "chownr": "^2.0.0", + "fs-minipass": "^2.0.0", + "minipass": "^3.0.0", + "minizlib": "^2.1.1", + "mkdirp": "^1.0.3", "yallist": "^4.0.0" - }, - "dependencies": { - "chownr": { - "version": "1.1.4", - "resolved": "https://registry.npmjs.org/chownr/-/chownr-1.1.4.tgz", - "integrity": "sha512-jJ0bqzaylmJtVnNgzTeSOs8DPavpbYgEr/b0YL8/2GO3xJEhInFmhKMUnEJQjZumK7KXGFhUy89PrsJWlakBVg==" - }, - "mkdirp": { - "version": "0.5.5", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz", - "integrity": "sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ==", - "requires": { - "minimist": "^1.2.5" - } - } } }, "text-table": { diff --git a/packages/shared-metrics/package.json b/packages/shared-metrics/package.json index e3454ab709..415e864503 100644 --- a/packages/shared-metrics/package.json +++ b/packages/shared-metrics/package.json @@ -71,7 +71,8 @@ "detect-libc": "^1.0.3", "event-loop-lag": "^1.4.0", "recursive-copy": "^2.0.13", - "tar": "^5.0.11" + "semver": "^7.3.5", + "tar": "^6.1.11" }, "devDependencies": { "@types/tar": "^4.0.5", diff --git a/packages/shared-metrics/src/gc.js b/packages/shared-metrics/src/gc.js index 19f978a75a..af5d7518ea 100644 --- a/packages/shared-metrics/src/gc.js +++ b/packages/shared-metrics/src/gc.js @@ -37,7 +37,8 @@ exports.currentPayload = { majorGcs: 0, incrementalMarkings: 0, weakCallbackProcessing: 0, - gcPause: 0 + gcPause: 0, + statsSupported: false }; exports.activate = function activate() { diff --git a/packages/shared-metrics/src/libuv.js b/packages/shared-metrics/src/libuv.js index 0319725e0c..742294fd39 100644 --- a/packages/shared-metrics/src/libuv.js +++ b/packages/shared-metrics/src/libuv.js @@ -47,5 +47,7 @@ function sense() { stats.statsSupported = true; return stats; } - return {}; + return { + statsSupported: false + }; } diff --git a/packages/shared-metrics/src/util/nativeModuleRetry.js b/packages/shared-metrics/src/util/nativeModuleRetry.js index 22969d2674..e44e53b8b4 100644 --- a/packages/shared-metrics/src/util/nativeModuleRetry.js +++ b/packages/shared-metrics/src/util/nativeModuleRetry.js @@ -11,8 +11,8 @@ const EventEmitter = require('events'); const copy = require('recursive-copy'); const fs = require('fs'); const os = require('os'); +const semver = require('semver'); const path = require('path'); -const tar = require('tar'); const detectLibc = require('detect-libc'); /** @@ -171,6 +171,20 @@ function copyPrecompiled(opts, loaderEmitter, callback) { logger.info(`Found a precompiled version for ${opts.nativeModuleName} ${label}, unpacking.`); + /** + * tar@6 has dropped support for Node < 10 + * It might work to require tar@6 or to execute commands with tar@6 and Node < 10, + * but we don't want to risk that a customers application fails - especially if tar@6 adds + * breaking changes. We decided to disallow this feature. + */ + if (semver.lt(process.version, '10.0.0')) { + logger.info(`Skipped copying precompiled version for ${opts.nativeModuleName} ${label} with Node < 10.`); + callback(false); + return; + } + + const tar = require('tar'); + tar .x({ cwd: os.tmpdir(), @@ -203,10 +217,10 @@ function copyPrecompiled(opts, loaderEmitter, callback) { // is, node_modules/${opts.nativeModuleName}). Node.js' module loading infrastructure // (lib/internal/modules/cjs/loader.js and lib/internal/modules/package_json_reader.js) have built-in // caching on multiple levels (for example, package.json locations and package.json contents). If Node.js - // has tried unsuccessfully to load a module or read a package.json from a particular path, it will remember - // and not try to load anything from that path again (a `false` will be put into the cache for that cache - // key). Instead, we force a new path, by adding precompiled to the module path and use the absolute path to - // the module to load it. + // has tried unsuccessfully to load a module or read a package.json from a particular path, + // it will remember and not try to load anything from that path again (a `false` will be + // put into the cache for that cache key). Instead, we force a new path, by adding precompiled + // to the module path and use the absolute path to the module to load it. opts.loadFrom = targetDir; callback(true); }