From a3f1702b6458258d4800a7b1ed0bb1790975b38c Mon Sep 17 00:00:00 2001 From: Manasvini B Suryanarayana Date: Fri, 16 Dec 2022 20:54:34 +0000 Subject: [PATCH] Adds plugin manifest config to define OpenSearch plugin dependency and verifies if it is installed Resolves Issue -https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2799 Signed-off-by: Manasvini B Suryanarayana --- CHANGELOG.md | 2 +- extensions/opensearch-dashboards-sdk-js | 1 + .../public/plugins/plugins_service.test.ts | 1 + .../discovery/plugin_manifest_parser.test.ts | 91 +++++++++++++++++++ .../discovery/plugin_manifest_parser.ts | 26 ++++++ .../integration_tests/plugins_service.test.ts | 3 + src/core/server/plugins/plugin.test.ts | 1 + src/core/server/plugins/plugin.ts | 2 + .../server/plugins/plugin_context.test.ts | 1 + .../server/plugins/plugins_service.test.ts | 3 + .../server/plugins/plugins_system.test.ts | 77 +++++++++++++++- src/core/server/plugins/plugins_system.ts | 37 +++++++- src/core/server/plugins/types.ts | 6 ++ 13 files changed, 247 insertions(+), 4 deletions(-) create mode 160000 extensions/opensearch-dashboards-sdk-js diff --git a/CHANGELOG.md b/CHANGELOG.md index fb7c3fd87322..4f3b5999c47d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,7 +82,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add satisfaction survey link to help menu ([#3676] (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3676)) - [Vis Builder] Add persistence to visualizations inner state ([#3751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3751)) - [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397)) - +- Add plugin manifest config to define OpenSearch plugin dependency and verify if it is installed on the cluster ([#3116](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3116)) ### 🐛 Bug Fixes - [Vis Builder] Fixes auto bounds for timeseries bar chart visualization ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401)) diff --git a/extensions/opensearch-dashboards-sdk-js b/extensions/opensearch-dashboards-sdk-js new file mode 160000 index 000000000000..014cbfa1cc22 --- /dev/null +++ b/extensions/opensearch-dashboards-sdk-js @@ -0,0 +1 @@ +Subproject commit 014cbfa1cc22b664ae97f818a8f99e90fac3d599 diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index 5ca7c491f72f..5fa3bf888b5c 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -84,6 +84,7 @@ function createManifest( version: 'some-version', configPath: ['path'], requiredPlugins: required, + requiredOpenSearchPlugins: optional, optionalPlugins: optional, requiredBundles: [], }; diff --git a/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts b/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts index 6103b2d748b6..50d1abb1ba72 100644 --- a/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts +++ b/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts @@ -279,6 +279,91 @@ test('return error when manifest contains unrecognized properties', async () => }); }); +describe('requiredOpenSearchPlugins', () => { + test('return error when plugin `requiredOpenSearchPlugins` is a string and not an array of string', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb( + null, + Buffer.from( + JSON.stringify({ + id: 'id1', + version: '7.0.0', + server: true, + requiredOpenSearchPlugins: 'abc', + }) + ) + ); + }); + + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `The "requiredOpenSearchPlugins" in plugin manifest for "id1" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`, + type: PluginDiscoveryErrorType.InvalidManifest, + path: pluginManifestPath, + }); + }); + + test('return error when `requiredOpenSearchPlugins` is not a string', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb( + null, + Buffer.from(JSON.stringify({ id: 'id2', version: '7.0.0', requiredOpenSearchPlugins: 2 })) + ); + }); + + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `The "requiredOpenSearchPlugins" in plugin manifest for "id2" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`, + type: PluginDiscoveryErrorType.InvalidManifest, + path: pluginManifestPath, + }); + }); + + test('return error when plugin requiredOpenSearchPlugins is an array that contains non-string values', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb( + null, + Buffer.from( + JSON.stringify({ id: 'id3', version: '7.0.0', requiredOpenSearchPlugins: ['plugin1', 2] }) + ) + ); + }); + + await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({ + message: `The "requiredOpenSearchPlugins" in plugin manifest for "id3" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`, + type: PluginDiscoveryErrorType.InvalidManifest, + path: pluginManifestPath, + }); + }); + + test('Happy path when plugin `requiredOpenSearchPlugins` is an array of string', async () => { + mockReadFile.mockImplementation((path, cb) => { + cb( + null, + Buffer.from( + JSON.stringify({ + id: 'id1', + version: '7.0.0', + server: true, + requiredOpenSearchPlugins: ['plugin1', 'plugin2'], + }) + ) + ); + }); + + await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({ + id: 'id1', + configPath: 'id_1', + version: '7.0.0', + opensearchDashboardsVersion: '7.0.0', + optionalPlugins: [], + requiredPlugins: [], + requiredOpenSearchPlugins: ['plugin1', 'plugin2'], + requiredBundles: [], + server: true, + ui: false, + }); + }); +}); + describe('configPath', () => { test('falls back to plugin id if not specified', async () => { mockReadFile.mockImplementation((path, cb) => { @@ -339,6 +424,7 @@ test('set defaults for all missing optional fields', async () => { opensearchDashboardsVersion: '7.0.0', optionalPlugins: [], requiredPlugins: [], + requiredOpenSearchPlugins: [], requiredBundles: [], server: true, ui: false, @@ -357,6 +443,7 @@ test('return all set optional fields as they are in manifest', async () => { opensearchDashboardsVersion: '7.0.0', requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'], optionalPlugins: ['some-optional-plugin'], + requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'], ui: true, }) ) @@ -371,6 +458,7 @@ test('return all set optional fields as they are in manifest', async () => { optionalPlugins: ['some-optional-plugin'], requiredBundles: [], requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'], + requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'], server: false, ui: true, }); @@ -387,6 +475,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches version: 'some-version', opensearchDashboardsVersion: '7.0.0-alpha2', requiredPlugins: ['some-required-plugin'], + requiredOpenSearchPlugins: [], server: true, }) ) @@ -400,6 +489,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches opensearchDashboardsVersion: '7.0.0-alpha2', optionalPlugins: [], requiredPlugins: ['some-required-plugin'], + requiredOpenSearchPlugins: [], requiredBundles: [], server: true, ui: false, @@ -430,6 +520,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version is `ope opensearchDashboardsVersion: 'opensearchDashboards', optionalPlugins: [], requiredPlugins: ['some-required-plugin'], + requiredOpenSearchPlugins: [], requiredBundles: [], server: true, ui: true, diff --git a/src/core/server/plugins/discovery/plugin_manifest_parser.ts b/src/core/server/plugins/discovery/plugin_manifest_parser.ts index 0f39f14b0ce5..52bfa1b84f65 100644 --- a/src/core/server/plugins/discovery/plugin_manifest_parser.ts +++ b/src/core/server/plugins/discovery/plugin_manifest_parser.ts @@ -65,6 +65,7 @@ const KNOWN_MANIFEST_FIELDS = (() => { version: true, configPath: true, requiredPlugins: true, + requiredOpenSearchPlugins: true, optionalPlugins: true, ui: true, server: true, @@ -160,6 +161,28 @@ export async function parseManifest( ); } + if ( + manifest.requiredOpenSearchPlugins !== undefined && + (!Array.isArray(manifest.requiredOpenSearchPlugins) || + !manifest.requiredOpenSearchPlugins.every((plugin) => typeof plugin === 'string')) + ) { + throw PluginDiscoveryError.invalidManifest( + manifestPath, + new Error( + `The "requiredOpenSearchPlugins" in plugin manifest for "${manifest.id}" should be an array of strings.` + ) + ); + } + + if ( + Array.isArray(manifest.requiredOpenSearchPlugins) && + manifest.requiredOpenSearchPlugins.length > 0 + ) { + log.warn( + `Plugin ${manifest.id} has a dependency on following OpenSearch plugin(s): "${manifest.requiredOpenSearchPlugins}". All dependency plugins have to be installed on the cluster to be able to start OpenSearch Dashboard.` + ); + } + const expectedOpenSearchDashboardsVersion = typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion ? manifest.opensearchDashboardsVersion @@ -202,6 +225,9 @@ export async function parseManifest( opensearchDashboardsVersion: expectedOpenSearchDashboardsVersion, configPath: manifest.configPath || snakeCase(manifest.id), requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [], + requiredOpenSearchPlugins: Array.isArray(manifest.requiredOpenSearchPlugins) + ? manifest.requiredOpenSearchPlugins + : [], optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [], requiredBundles: Array.isArray(manifest.requiredBundles) ? manifest.requiredBundles : [], ui: includesUiPlugin, diff --git a/src/core/server/plugins/integration_tests/plugins_service.test.ts b/src/core/server/plugins/integration_tests/plugins_service.test.ts index 34310ea3de37..4d1660f65c75 100644 --- a/src/core/server/plugins/integration_tests/plugins_service.test.ts +++ b/src/core/server/plugins/integration_tests/plugins_service.test.ts @@ -57,6 +57,7 @@ describe('PluginsService', () => { disabled = false, version = 'some-version', requiredPlugins = [], + requiredOpenSearchPlugins = [], requiredBundles = [], optionalPlugins = [], opensearchDashboardsVersion = '7.0.0', @@ -68,6 +69,7 @@ describe('PluginsService', () => { disabled?: boolean; version?: string; requiredPlugins?: string[]; + requiredOpenSearchPlugins?: string[]; requiredBundles?: string[]; optionalPlugins?: string[]; opensearchDashboardsVersion?: string; @@ -84,6 +86,7 @@ describe('PluginsService', () => { configPath: `${configPath}${disabled ? '-disabled' : ''}`, opensearchDashboardsVersion, requiredPlugins, + requiredOpenSearchPlugins, requiredBundles, optionalPlugins, server, diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index 9543d379493c..04df84ab8d12 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -69,6 +69,7 @@ function createPluginManifest(manifestProps: Partial = {}): Plug configPath: 'path', opensearchDashboardsVersion: '7.0.0', requiredPlugins: ['some-required-dep'], + requiredOpenSearchPlugins: ['some-os-plugins'], optionalPlugins: ['some-optional-dep'], requiredBundles: [], server: true, diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index 89a2aecc82f3..cc53e1b443e9 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -66,6 +66,7 @@ export class PluginWrapper< public readonly configPath: PluginManifest['configPath']; public readonly requiredPlugins: PluginManifest['requiredPlugins']; public readonly optionalPlugins: PluginManifest['optionalPlugins']; + public readonly requiredOpenSearchPlugins: PluginManifest['requiredOpenSearchPlugins']; public readonly requiredBundles: PluginManifest['requiredBundles']; public readonly includesServerPlugin: PluginManifest['server']; public readonly includesUiPlugin: PluginManifest['ui']; @@ -95,6 +96,7 @@ export class PluginWrapper< this.configPath = params.manifest.configPath; this.requiredPlugins = params.manifest.requiredPlugins; this.optionalPlugins = params.manifest.optionalPlugins; + this.requiredOpenSearchPlugins = params.manifest.requiredOpenSearchPlugins; this.requiredBundles = params.manifest.requiredBundles; this.includesServerPlugin = params.manifest.server; this.includesUiPlugin = params.manifest.ui; diff --git a/src/core/server/plugins/plugin_context.test.ts b/src/core/server/plugins/plugin_context.test.ts index c55603679c35..ca46a97a237e 100644 --- a/src/core/server/plugins/plugin_context.test.ts +++ b/src/core/server/plugins/plugin_context.test.ts @@ -56,6 +56,7 @@ function createPluginManifest(manifestProps: Partial = {}): Plug configPath: 'path', opensearchDashboardsVersion: '7.0.0', requiredPlugins: ['some-required-dep'], + requiredOpenSearchPlugins: ['some-backend-plugin'], requiredBundles: [], optionalPlugins: ['some-optional-dep'], server: true, diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index b020e56539a7..c3ee05d60ed8 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -78,6 +78,7 @@ const createPlugin = ( disabled = false, version = 'some-version', requiredPlugins = [], + requiredOpenSearchPlugins = [], requiredBundles = [], optionalPlugins = [], opensearchDashboardsVersion = '7.0.0', @@ -89,6 +90,7 @@ const createPlugin = ( disabled?: boolean; version?: string; requiredPlugins?: string[]; + requiredOpenSearchPlugins?: string[]; requiredBundles?: string[]; optionalPlugins?: string[]; opensearchDashboardsVersion?: string; @@ -105,6 +107,7 @@ const createPlugin = ( configPath: `${configPath}${disabled ? '-disabled' : ''}`, opensearchDashboardsVersion, requiredPlugins, + requiredOpenSearchPlugins, requiredBundles, optionalPlugins, server, diff --git a/src/core/server/plugins/plugins_system.test.ts b/src/core/server/plugins/plugins_system.test.ts index 2e8aa6b4a4c0..997ef45b7f28 100644 --- a/src/core/server/plugins/plugins_system.test.ts +++ b/src/core/server/plugins/plugins_system.test.ts @@ -53,9 +53,16 @@ function createPlugin( { required = [], optional = [], + requiredOSPlugin = [], server = true, ui = true, - }: { required?: string[]; optional?: string[]; server?: boolean; ui?: boolean } = {} + }: { + required?: string[]; + optional?: string[]; + requiredOSPlugin?: string[]; + server?: boolean; + ui?: boolean; + } = {} ) { return new PluginWrapper({ path: 'some-path', @@ -65,6 +72,7 @@ function createPlugin( configPath: 'path', opensearchDashboardsVersion: '7.0.0', requiredPlugins: required, + requiredOpenSearchPlugins: requiredOSPlugin, optionalPlugins: optional, requiredBundles: [], server, @@ -187,7 +195,7 @@ test('correctly orders plugins and returns exposed values for "setup" and "start } const plugins = new Map([ [ - createPlugin('order-4', { required: ['order-2'] }), + createPlugin('order-4', { required: ['order-2'], requiredOSPlugin: ['test-plugin'] }), { setup: { 'order-2': 'added-as-2' }, start: { 'order-2': 'started-as-2' }, @@ -244,6 +252,17 @@ test('correctly orders plugins and returns exposed values for "setup" and "start startContextMap.get(plugin.name) ); + const opensearch = startDeps.opensearch; + opensearch.client.asInternalUser.cat.plugins.mockResolvedValue({ + body: [ + { + name: 'node-1', + component: 'test-plugin', + version: 'v1', + }, + ], + } as any); + expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(` Array [ Array [ @@ -484,6 +503,16 @@ describe('start', () => { afterAll(() => { jest.useRealTimers(); }); + const opensearch = startDeps.opensearch; + opensearch.client.asInternalUser.cat.plugins.mockResolvedValue({ + body: [ + { + name: 'node-1', + component: 'test-plugin', + version: 'v1', + }, + ], + } as any); it('throws timeout error if "start" was not completed in 30 sec.', async () => { const plugin: PluginWrapper = createPlugin('timeout-start'); jest.spyOn(plugin, 'setup').mockResolvedValue({}); @@ -517,4 +546,48 @@ describe('start', () => { const log = logger.get.mock.results[0].value as jest.Mocked; expect(log.info).toHaveBeenCalledWith(`Starting [2] plugins: [order-1,order-0]`); }); + + it('validates opensearch plugin installation', async () => { + [ + createPlugin('order-1', { requiredOSPlugin: ['test-plugin'] }), + createPlugin('order-2'), + ].forEach((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + pluginsSystem.addPlugin(plugin); + }); + + await pluginsSystem.setupPlugins(setupDeps); + await pluginsSystem.startPlugins(startDeps); + expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1); + }); + + it('validates opensearch plugin installation and errors out when plugin is not installed', async () => { + [ + createPlugin('id-1', { requiredOSPlugin: ['missing-opensearch-dep'] }), + createPlugin('id-2'), + ].forEach((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + pluginsSystem.addPlugin(plugin); + }); + + await pluginsSystem.setupPlugins(setupDeps); + + await expect(pluginsSystem.startPlugins(startDeps)).rejects.toMatchInlineSnapshot( + `[Error: Plugin "id-1" has a dependency on OpenSearch plugin "missing-opensearch-dep" which is not installed on the cluster.]` + ); + }); + + it('validates opensearch plugin installation and does not error out when there is no dependency', async () => { + [createPlugin('id-1'), createPlugin('id-2')].forEach((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + pluginsSystem.addPlugin(plugin); + }); + await pluginsSystem.setupPlugins(setupDeps); + const pluginsStart = await pluginsSystem.startPlugins(startDeps); + expect(pluginsStart).toBeInstanceOf(Map); + expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/core/server/plugins/plugins_system.ts b/src/core/server/plugins/plugins_system.ts index b3adc71323f9..539723537c79 100644 --- a/src/core/server/plugins/plugins_system.ts +++ b/src/core/server/plugins/plugins_system.ts @@ -158,13 +158,48 @@ export class PluginsSystem { timeout: 30 * Sec, errorMessage: `Start lifecycle of "${pluginName}" plugin wasn't completed in 30sec. Consider disabling the plugin and re-start.`, }); - contracts.set(pluginName, contract); } + await this.healthCheckOpenSearchPlugins(deps); return contracts; } + private async healthCheckOpenSearchPlugins(deps: PluginsServiceStartDeps) { + // make _cat/plugins?format=json call to the OpenSearch instance + const opensearchPlugins = await this.getOpensearchPlugins(deps); + for (const pluginName of this.satupPlugins) { + this.log.debug(`For plugin "${pluginName}"...`); + const plugin = this.plugins.get(pluginName)!; + const pluginBackendDeps = new Set([...plugin.requiredOpenSearchPlugins]); + pluginBackendDeps.forEach((opensearchPlugin) => { + if (!opensearchPlugins.find((obj) => obj.component === opensearchPlugin)) { + this.log.error( + `OpenSearch plugin "${opensearchPlugin}" is not installed on the cluster for the OpenSearch Dashboard plugin to function as expected.` + ); + throw new Error( + `Plugin "${pluginName}" has a dependency on OpenSearch plugin "${opensearchPlugin}" which is not installed on the cluster.` + ); + } + }); + } + } + + private async getOpensearchPlugins(deps: PluginsServiceStartDeps) { + // Makes cat.plugin api call to fetch list of OpenSearch plugins installed on the cluster + try { + const { body } = await deps.opensearch.client.asInternalUser.cat.plugins({ + format: 'JSON1', + }); + return body; + } catch (error) { + this.log.warn( + `Cat API call to OpenSearch to get list of plugins installed on the cluster has failed: ${error}` + ); + return []; + } + } + public async stopPlugins() { if (this.plugins.size === 0 || this.satupPlugins.length === 0) { return; diff --git a/src/core/server/plugins/types.ts b/src/core/server/plugins/types.ts index cf769b45daa3..12b28f48f237 100644 --- a/src/core/server/plugins/types.ts +++ b/src/core/server/plugins/types.ts @@ -154,6 +154,12 @@ export interface PluginManifest { */ readonly requiredPlugins: readonly PluginName[]; + /** + * An optional list of component names of the backend OpenSearch plugins that **must be** installed on the cluster + * for this plugin to function properly. + */ + readonly requiredOpenSearchPlugins: readonly PluginName[]; + /** * List of plugin ids that this plugin's UI code imports modules from that are * not in `requiredPlugins`.