From c51f67112eb35e41bf3d15174b2a09cd3fd6ce0c Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Tue, 16 May 2023 14:55:07 -0700 Subject: [PATCH] [2.x backport] Replace re2 with RegExp in timeline and add unit tests Remove re2 usage and replace it with JavaScript built-in RegExp object. Also add more unit tests to make sure that using RegExp has same expressions as using re2 library. Issue Resolve https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3901 Backport PR https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3908 Signed-off-by: Anan Zhuang --- CHANGELOG.md | 1 + package.json | 1 - .../tasks/patch_native_modules_task.test.ts | 116 ++++++++++++------ .../build/tasks/patch_native_modules_task.ts | 69 +++-------- .../server/series_functions/label.js | 6 +- .../server/series_functions/label.test.js | 20 +++ yarn.lock | 51 ++------ 7 files changed, 126 insertions(+), 138 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 943bfabe107a..07bd9233589e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -123,6 +123,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add disablePrototypePoisoningProtection configuration to prevent JS client from erroring when cluster utilizes JS reserved words ([#2992](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2992)) - [Multiple DataSource] Add support for SigV4 authentication ([#3058](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3058)) - [Multiple DataSource] Refactor test connection to support SigV4 auth type ([#3456](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3456)) +- Replace re2 with RegExp in timeline and add unit tests ([#3908](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3908)) ### 🐛 Bug Fixes diff --git a/package.json b/package.json index fd983ec0cabe..beba139107ec 100644 --- a/package.json +++ b/package.json @@ -204,7 +204,6 @@ "pegjs": "0.10.0", "proxy-from-env": "1.0.0", "query-string": "^6.13.2", - "re2": "1.17.4", "react": "^16.14.0", "react-dom": "^16.12.0", "react-input-range": "^1.3.0", diff --git a/src/dev/build/tasks/patch_native_modules_task.test.ts b/src/dev/build/tasks/patch_native_modules_task.test.ts index f3a3daa432c3..71553d39cd9c 100644 --- a/src/dev/build/tasks/patch_native_modules_task.test.ts +++ b/src/dev/build/tasks/patch_native_modules_task.test.ts @@ -9,8 +9,8 @@ import { createAnyInstanceSerializer, createAbsolutePathSerializer, } from '@osd/dev-utils'; -import { Build, Config } from '../lib'; -import { PatchNativeModules } from './patch_native_modules_task'; +import { Build, Config, read, download, untar, gunzip } from '../lib'; +import { createPatchNativeModulesTask } from './patch_native_modules_task'; const log = new ToolingLog(); const testWriter = new ToolingLogCollectingWriter(); @@ -19,16 +19,16 @@ expect.addSnapshotSerializer(createAnyInstanceSerializer(Config)); expect.addSnapshotSerializer(createAnyInstanceSerializer(ToolingLog)); expect.addSnapshotSerializer(createAbsolutePathSerializer()); -jest.mock('../lib/download'); -jest.mock('../lib/fs', () => ({ - ...jest.requireActual('../lib/fs'), - untar: jest.fn(), - gunzip: jest.fn(), -})); - -const { untar } = jest.requireMock('../lib/fs'); -const { gunzip } = jest.requireMock('../lib/fs'); -const { download } = jest.requireMock('../lib/download'); +jest.mock('../lib', () => { + const originalModule = jest.requireActual('../lib'); + return { + ...originalModule, + download: jest.fn(), + gunzip: jest.fn(), + untar: jest.fn(), + read: jest.fn(), + }; +}); async function setup() { const config = await Config.create({ @@ -38,14 +38,15 @@ async function setup() { linux: false, linuxArm: false, darwin: false, + windows: false, }, }); const build = new Build(config); - download.mockImplementation(() => {}); - untar.mockImplementation(() => {}); - gunzip.mockImplementation(() => {}); + (read as jest.MockedFunction).mockImplementation(async () => { + return JSON.stringify({ version: mockPackage.version }); + }); return { config, build }; } @@ -55,38 +56,77 @@ beforeEach(() => { jest.clearAllMocks(); }); -it('patch native modules task downloads the correct platform package', async () => { - const { config, build } = await setup(); - config.targetPlatforms.linuxArm = true; - await PatchNativeModules.run(config, log, build); - expect(download.mock.calls.length).toBe(1); - expect(download.mock.calls).toMatchInlineSnapshot(` +const mockPackage = { + name: 'mock-native-module', + version: '1.0.0', + destinationPath: 'path/to/destination', + extractMethod: 'untar', + archives: { + 'linux-arm64': { + url: 'https://example.com/mock-native-module/linux-arm64.tar.gz', + sha256: 'mock-sha256', + }, + 'linux-x64': { + url: 'https://example.com/mock-native-module/linux-x64.gz', + sha256: 'mock-sha256', + }, + }, +}; + +describe('patch native modules task', () => { + it('patch native modules task downloads the correct platform package', async () => { + const { config, build } = await setup(); + config.targetPlatforms.linuxArm = true; + const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackage]); + await PatchNativeModulesWithMock.run(config, log, build); + expect((download as jest.MockedFunction).mock.calls.length).toBe(1); + expect((download as jest.MockedFunction).mock.calls).toMatchInlineSnapshot(` Array [ Array [ Object { - "destination": /.native_modules/re2/linux-arm64-83.tar.gz, + "destination": /.native_modules/mock-native-module/linux-arm64.tar.gz, "log": , "retries": 3, - "sha256": "d86ced75b794fbf518b90908847b3c09a50f3ff5a2815aa30f53080f926a2873", - "url": "https://d1v1sj258etie.cloudfront.net/node-re2/releases/download/1.17.4/linux-arm64-83.tar.gz", + "sha256": "mock-sha256", + "url": "https://example.com/mock-native-module/linux-arm64.tar.gz", }, ], ] `); -}); + }); -it('for .tar.gz artifact, patch native modules task unzip it via untar', async () => { - const { config, build } = await setup(); - config.targetPlatforms.linuxArm = true; - await PatchNativeModules.run(config, log, build); - expect(untar.mock.calls.length).toBe(1); - expect(gunzip.mock.calls.length).toBe(0); -}); + it('for .tar.gz artifact, patch native modules task unzip it via untar', async () => { + const { config, build } = await setup(); + config.targetPlatforms.linuxArm = true; + const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackage]); + await PatchNativeModulesWithMock.run(config, log, build); + expect(untar).toHaveBeenCalled(); + expect(gunzip).not.toHaveBeenCalled(); + }); -it('for .gz artifact, patch native modules task unzip it via gunzip', async () => { - const { config, build } = await setup(); - config.targetPlatforms.linux = true; - await PatchNativeModules.run(config, log, build); - expect(untar.mock.calls.length).toBe(0); - expect(gunzip.mock.calls.length).toBe(1); + it('for .gz artifact, patch native modules task unzip it via gunzip', async () => { + const mockPackageGZ = { + ...mockPackage, + extractMethod: 'gunzip', + }; + const { config, build } = await setup(); + config.targetPlatforms.linux = true; + const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackageGZ]); + await PatchNativeModulesWithMock.run(config, log, build); + expect(gunzip).toHaveBeenCalled(); + expect(untar).not.toHaveBeenCalled(); + }); + + it('throws error for unsupported extract methods', async () => { + const mockPackageUnsupported = { + ...mockPackage, + extractMethod: 'unsupported', + }; + const { config, build } = await setup(); + config.targetPlatforms.linux = true; + const PatchNativeModulesWithMock = createPatchNativeModulesTask([mockPackageUnsupported]); + await expect(PatchNativeModulesWithMock.run(config, log, build)).rejects.toThrow( + 'Extract method of unsupported is not supported' + ); + }); }); diff --git a/src/dev/build/tasks/patch_native_modules_task.ts b/src/dev/build/tasks/patch_native_modules_task.ts index 3bd9fa63c358..b8c8d8a5b9fb 100644 --- a/src/dev/build/tasks/patch_native_modules_task.ts +++ b/src/dev/build/tasks/patch_native_modules_task.ts @@ -52,45 +52,7 @@ interface Package { >; } -/* Process for updating URLs and checksums after bumping the version of `re2` or NodeJS: - * 1. Match the `version` with the version in the yarn.lock file. - * 2. Match the module version, the digits at the end of the filename, with the output of - * `node -p process.versions.modules`. - * 3. Confirm that the URLs exist for each platform-architecture combo on - * https://github.com/uhop/node-re2/releases/tag/[VERSION]; reach out to maintainers for ARM - * releases of `re2` as they currently don't have an official ARM release. - * 4. Generate new checksums for each artifact by downloading each one and calling - * `shasum -a 256` or `sha256sum` on the downloaded file. - */ -const packages: Package[] = [ - { - name: 're2', - version: '1.17.4', - destinationPath: 'node_modules/re2/build/Release/re2.node', - extractMethod: 'gunzip', - archives: { - 'darwin-x64': { - url: 'https://github.com/uhop/node-re2/releases/download/1.17.4/darwin-x64-83.gz', - sha256: '9112ed93c1544ecc6397f7ff20bd2b28f3b04c7fbb54024e10f9a376a132a87d', - }, - 'linux-x64': { - url: 'https://github.com/uhop/node-re2/releases/download/1.17.4/linux-x64-83.gz', - sha256: '86e03540783a18c41f81df0aec320b1f64aca6cbd3a87fc1b7a9b4109c5f5986', - }, - 'linux-arm64': { - url: - 'https://d1v1sj258etie.cloudfront.net/node-re2/releases/download/1.17.4/linux-arm64-83.tar.gz', - sha256: 'd86ced75b794fbf518b90908847b3c09a50f3ff5a2815aa30f53080f926a2873', - overriddenExtractMethod: 'untar', - overriddenDestinationPath: 'node_modules/re2/build/Release', - }, - 'win32-x64': { - url: 'https://github.com/uhop/node-re2/releases/download/1.17.4/win32-x64-83.gz', - sha256: '2f842d9757288afd4bd5dec0e7b370a4c3e89ac98050598b17abb9e8e00e3294', - }, - }, - }, -]; +export const packages: Package[] = []; async function getInstalledVersion(config: Config, packageName: string) { const packageJSONPath = config.resolveFromRepo( @@ -145,15 +107,20 @@ async function patchModule( } } -export const PatchNativeModules: Task = { - description: 'Patching platform-specific native modules', - async run(config, log, build) { - for (const pkg of packages) { - await Promise.all( - config.getTargetPlatforms().map(async (platform) => { - await patchModule(config, log, build, platform, pkg); - }) - ); - } - }, -}; +export function createPatchNativeModulesTask(customPackages?: Package[]): Task { + return { + description: 'Patching platform-specific native modules', + async run(config, log, build) { + const targetPackages = customPackages || packages; + for (const pkg of targetPackages) { + await Promise.all( + config.getTargetPlatforms().map(async (platform) => { + await patchModule(config, log, build, platform, pkg); + }) + ); + } + }, + }; +} + +export const PatchNativeModules = createPatchNativeModulesTask(); diff --git a/src/plugins/vis_type_timeline/server/series_functions/label.js b/src/plugins/vis_type_timeline/server/series_functions/label.js index c935d537081f..4649ee6cf53f 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/label.js +++ b/src/plugins/vis_type_timeline/server/series_functions/label.js @@ -62,10 +62,8 @@ export default new Chainable('label', { const config = args.byName; return alter(args, function (eachSeries) { if (config.regex) { - // not using a standard `import` so that if there's an issue with the re2 native module - // that it doesn't prevent OpenSearch Dashboards from starting up and we only have an issue using Timeline labels - const RE2 = require('re2'); - eachSeries.label = eachSeries.label.replace(new RE2(config.regex), config.label); + const regex = new RegExp(config.regex); + eachSeries.label = eachSeries.label.replace(regex, config.label); } else { eachSeries.label = config.label; } diff --git a/src/plugins/vis_type_timeline/server/series_functions/label.test.js b/src/plugins/vis_type_timeline/server/series_functions/label.test.js index c6dc832914a3..69268b385b07 100644 --- a/src/plugins/vis_type_timeline/server/series_functions/label.test.js +++ b/src/plugins/vis_type_timeline/server/series_functions/label.test.js @@ -51,4 +51,24 @@ describe('label.js', () => { expect(r.output.list[0].label).to.equal('beerative'); }); }); + + it('can use a regex to capture groups to modify series label', () => { + return invoke(fn, [seriesList, 'beer$2', '(N)(egative)']).then((r) => { + expect(r.output.list[0].label).to.equal('beeregative'); + }); + }); + + it('can handle different regex patterns', () => { + const seriesListCopy1 = JSON.parse(JSON.stringify(seriesList)); + const seriesListCopy2 = JSON.parse(JSON.stringify(seriesList)); + + return Promise.all([ + invoke(fn, [seriesListCopy1, 'beer$1 - $2', '(N)(egative)']).then((r) => { + expect(r.output.list[0].label).to.equal('beerN - egative'); + }), + invoke(fn, [seriesListCopy2, 'beer$1_$2', '(N)(eg.*)']).then((r) => { + expect(r.output.list[0].label).to.equal('beerN_egative'); + }), + ]); + }); }); diff --git a/yarn.lock b/yarn.lock index 85ce73dec424..8ba76ae9164f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4342,7 +4342,7 @@ agentkeepalive@^3.4.1: dependencies: humanize-ms "^1.2.1" -agentkeepalive@^4.1.3: +agentkeepalive@^4.1.3, agentkeepalive@^4.2.1: version "4.3.0" resolved "https://registry.yarnpkg.com/agentkeepalive/-/agentkeepalive-4.3.0.tgz#bb999ff07412653c1803b3ced35e50729830a255" integrity sha512-7Epl1Blf4Sy37j4v9f9FjICCh4+KAQOyXgHEwlyBiAQLbhKdq/i2QQU3amQalS/wPhdPzDXPL5DMR5bkn+YeWg== @@ -4351,15 +4351,6 @@ agentkeepalive@^4.1.3: depd "^2.0.0" humanize-ms "^1.2.1" -agentkeepalive@^4.2.1: - version "4.2.1" - resolved "https://registry.yarnpkg.com/agentkeepalive/-/agentkeepalive-4.2.1.tgz#a7975cbb9f83b367f06c90cc51ff28fe7d499717" - integrity sha512-Zn4cw2NEqd+9fiSVWMscnjyQ1a8Yfoc5oBajLeo5w+YBHgDUcEBY2hS4YpTz6iN5f/2zQiktcuM6tS8x1p9dpA== - dependencies: - debug "^4.1.0" - depd "^1.1.2" - humanize-ms "^1.2.1" - aggregate-error@^3.0.0: version "3.1.0" resolved "https://registry.yarnpkg.com/aggregate-error/-/aggregate-error-3.1.0.tgz#92670ff50f5359bdb7a3e0d40d0ec30c5737687a" @@ -7129,7 +7120,7 @@ delayed-stream@~1.0.0: delegates@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/delegates/-/delegates-1.0.0.tgz#84c6e159b81904fdca59a0ef44cd870d31250f9a" - integrity sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o= + integrity sha512-bd2L678uiWATM6m5Z1VzNCErI3jiGzt6HGY8OVICs40JQq/HALfbyNJmp0UDakEY4pMMaN0Ly5om/B1VI/+xfQ== delete-empty@^2.0.0: version "2.0.0" @@ -7140,11 +7131,6 @@ delete-empty@^2.0.0: relative "^3.0.2" rimraf "^2.6.2" -depd@^1.1.2: - version "1.1.2" - resolved "https://registry.yarnpkg.com/depd/-/depd-1.1.2.tgz#9bcd52e14c097763e749b274c4346ed2e560b5a9" - integrity sha1-m81S4UwJd2PnSbJ0xDRu0uVgtak= - depd@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/depd/-/depd-2.0.0.tgz#b696163cc757560d09cf22cc8fad1571b79e76df" @@ -10246,11 +10232,6 @@ inquirer@^7.0.0, inquirer@^7.3.3: strip-ansi "^6.0.0" through "^2.3.6" -install-artifact-from-github@^1.3.0: - version "1.3.1" - resolved "https://registry.yarnpkg.com/install-artifact-from-github/-/install-artifact-from-github-1.3.1.tgz#eefaad9af35d632e5d912ad1569c1de38c3c2462" - integrity sha512-3l3Bymg2eKDsN5wQuMfgGEj2x6l5MCAv0zPL6rxHESufFVlEAKW/6oY9F1aGgvY/EgWm5+eWGRjINveL4X7Hgg== - internal-slot@^1.0.3: version "1.0.3" resolved "https://registry.yarnpkg.com/internal-slot/-/internal-slot-1.0.3.tgz#7347e307deeea2faac2ac6205d4bc7d34967f59c" @@ -12740,14 +12721,7 @@ minipass-sized@^1.0.3: dependencies: minipass "^3.0.0" -minipass@^3.0.0, minipass@^3.1.1: - version "3.1.6" - resolved "https://registry.yarnpkg.com/minipass/-/minipass-3.1.6.tgz#3b8150aa688a711a1521af5e8779c1d3bb4f45ee" - integrity sha512-rty5kpw9/z8SX9dmxblFA6edItUmwJgMeYDZRrwlIVN27i8gysGbznJwUggw2V/FVqFSDdWy040ZPS811DYAqQ== - dependencies: - yallist "^4.0.0" - -minipass@^3.1.0, minipass@^3.1.3, minipass@^3.1.6: +minipass@^3.0.0, minipass@^3.1.0, minipass@^3.1.1, minipass@^3.1.3, minipass@^3.1.6: version "3.3.6" resolved "https://registry.yarnpkg.com/minipass/-/minipass-3.3.6.tgz#7bba384db3a1520d18c9c0e5251c3444e95dd94a" integrity sha512-DxiNidxSEK+tHG6zOIklvNOwm3hvCrbUrdtzY74U6HKTJxvIDfOUL5W5P2Ghd3DTkhhKPYGqeNUIh5qcM4YBfw== @@ -12755,11 +12729,9 @@ minipass@^3.1.0, minipass@^3.1.3, minipass@^3.1.6: yallist "^4.0.0" minipass@^4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/minipass/-/minipass-4.0.0.tgz#7cebb0f9fa7d56f0c5b17853cbe28838a8dbbd3b" - integrity sha512-g2Uuh2jEKoht+zvO6vJqXmYpflPqzRBT+Th2h01DKh5z7wbY/AZ2gCQ78cP70YoHPyFdY30YBV5WxgLOEwOykw== - dependencies: - yallist "^4.0.0" + version "4.2.8" + resolved "https://registry.yarnpkg.com/minipass/-/minipass-4.2.8.tgz#f0010f64393ecfc1d1ccb5f582bcaf45f48e1a3a" + integrity sha512-fNzuVyifolSLFL4NzpF+wEF4qrgqaaKX0haXPQEdQ7NKAN+WecoKMHV09YcuL/DHxrUsYQOK3MiuDf7Ip2OXfQ== minipass@^5.0.0: version "5.0.0" @@ -13012,7 +12984,7 @@ nan@^2.12.1: resolved "https://registry.yarnpkg.com/nan/-/nan-2.15.0.tgz#3f34a473ff18e15c1b5626b62903b5ad6e665fee" integrity sha512-8ZtvEnA2c5aYCZYd1cvgdnU6cqwixRoYg70xPLWUws5ORTa/lnw+u4amixRS/Ac5U5mQVgp9pnlSUnbNWFaWZQ== -nan@^2.14.2, nan@^2.15.0, nan@^2.17.0: +nan@^2.14.2, nan@^2.17.0: version "2.17.0" resolved "https://registry.yarnpkg.com/nan/-/nan-2.17.0.tgz#c0150a2368a182f033e9aa5195ec76ea41a199cb" integrity sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ== @@ -14622,15 +14594,6 @@ re-reselect@^3.4.0: resolved "https://registry.yarnpkg.com/re-reselect/-/re-reselect-3.4.0.tgz#0f2303f3c84394f57f0cd31fea08a1ca4840a7cd" integrity sha512-JsecfN+JlckncVXTWFWjn0Vk6uInl8GSf4eEd9tTk5qXHlgqkPdILpnYpgZcISXNYAzvfvsCZviaDk8AxyS5sg== -re2@1.17.4: - version "1.17.4" - resolved "https://registry.yarnpkg.com/re2/-/re2-1.17.4.tgz#7bf29290bdde963014e77bd2c2e799a6d788386e" - integrity sha512-xyZ4h5PqE8I9tAxTh3G0UttcK5ufrcUxReFjGzfX61vtanNbS1XZHjnwRSyPcLgChI4KLxVgOT/ioZXnUAdoTA== - dependencies: - install-artifact-from-github "^1.3.0" - nan "^2.15.0" - node-gyp "^8.4.1" - react-ace@^7.0.5: version "7.0.5" resolved "https://registry.yarnpkg.com/react-ace/-/react-ace-7.0.5.tgz#798299fd52ddf3a3dcc92afc5865538463544f01"