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

[backport 2.8] Replace re2 with RegExp in timeline and add unit tests #4208

Merged
merged 1 commit into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
116 changes: 78 additions & 38 deletions src/dev/build/tasks/patch_native_modules_task.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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({
Expand All @@ -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<typeof read>).mockImplementation(async () => {
return JSON.stringify({ version: mockPackage.version });
});

return { config, build };
}
Expand All @@ -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<typeof download>).mock.calls.length).toBe(1);
expect((download as jest.MockedFunction<typeof download>).mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"destination": <absolute path>/.native_modules/re2/linux-arm64-83.tar.gz,
"destination": <absolute path>/.native_modules/mock-native-module/linux-arm64.tar.gz,
"log": <ToolingLog>,
"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'
);
});
});
69 changes: 18 additions & 51 deletions src/dev/build/tasks/patch_native_modules_task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}),
]);
});
});
Loading