From fbfc31c1dbb97f73c6084cf179c3f29e2aa290de 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 | 1 + .../public/plugins/plugins_service.test.ts | 1 + .../discovery/plugin_manifest_parser.test.ts | 6 ++ .../discovery/plugin_manifest_parser.ts | 25 ++++++ .../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 | 90 ++++++++++++++++++- src/core/server/plugins/plugins_system.ts | 30 ++++++- src/core/server/plugins/types.ts | 6 ++ 12 files changed, 166 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cf44724103e..73e7f0c5094b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [I18n] Register ru, ru-RU locale ([#2817](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2817)) - Add yarn opensearch arg to setup plugin dependencies ([#2544](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2544)) - [Multi DataSource] Test the connection to an external data source when creating or updating ([#2973](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2973)) +- Adds plugin manifest config to define OpenSearch plugin dependency and verifies if it is installed on the cluster ([#3116](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3116)) ### 🐛 Bug Fixes 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..9fb1cf758068 100644 --- a/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts +++ b/src/core/server/plugins/discovery/plugin_manifest_parser.test.ts @@ -339,6 +339,7 @@ test('set defaults for all missing optional fields', async () => { opensearchDashboardsVersion: '7.0.0', optionalPlugins: [], requiredPlugins: [], + requiredOpenSearchPlugins: [], requiredBundles: [], server: true, ui: false, @@ -357,6 +358,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 +373,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 +390,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 +404,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 +435,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..fa465a53346d 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,27 @@ export async function parseManifest( ); } + if ( + manifest.requiredOpenSearchPlugins !== undefined && + !Array.isArray(manifest.requiredOpenSearchPlugins) + ) { + throw PluginDiscoveryError.invalidManifest( + manifestPath, + new Error( + `The "requiredOpenSearchPlugins" in plugin manifest for "${manifest.id}" should either be a string or an array of strings.` + ) + ); + } + + if ( + manifest.requiredOpenSearchPlugins !== undefined && + Array.isArray(manifest.requiredOpenSearchPlugins) + ) { + log.warn( + `Plugin ${manifest.id} has a dependency on backend OpenSearch plugin. You need to install it on the cluster for data source.` + ); + } + const expectedOpenSearchDashboardsVersion = typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion ? manifest.opensearchDashboardsVersion @@ -202,6 +224,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..892f74cf5d0c 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.mockResolvedValueOnce({ + body: [ + { + name: 'node-1', + component: 'test-plugin', + version: 'v1', + }, + ], + } as any); + expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(` Array [ Array [ @@ -517,4 +536,71 @@ 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); + }); + + const opensearch = startDeps.opensearch; + opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({ + body: [ + { + name: 'node-1', + component: 'test-plugin', + version: 'v1', + }, + ], + } as any); + 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); + }); + const opensearch = startDeps.opensearch; + opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({ + body: [ + { + name: 'node-1', + component: 'test-plugin1', + version: 'v1', + }, + ], + } as any); + await pluginsSystem.setupPlugins(setupDeps); + + await expect(pluginsSystem.startPlugins(startDeps)).rejects.toMatchInlineSnapshot( + `[Error: Plugin "id-1" has a dependency on OpenSearch data source plugin "missing-opensearch-dep" which needs to be installed.]` + ); + }); + + it('validates opensearch plugin installation and does not errors 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); + }); + const opensearch = startDeps.opensearch; + opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({ + body: [], + } as any); + 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..be6e44a63377 100644 --- a/src/core/server/plugins/plugins_system.ts +++ b/src/core/server/plugins/plugins_system.ts @@ -158,13 +158,41 @@ 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 on http://localhost:9200/ + 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 dependency "${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 data source plugin "${opensearchPlugin}" which needs to be installed.` + ); + } + }); + } + } + + private async getOpensearchPlugins(deps: PluginsServiceStartDeps) { + // Makes cat.plugin api call to fetch list of OpenSearch plugins installed on the cluster + const { body } = await deps.opensearch.client.asInternalUser.cat.plugins({ + format: 'JSON', + }); + return body; + } + 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..e41ba540e908 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 data source 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`.