Adds plugin manifest config to define OpenSearch plugin dependency an…

…d verifies if it is installed Resolves Issue -opensearch-project#2799 Signed-off-by: Manasvini B Suryanarayana <[email protected]>
manasvinibs · Mar 2, 2023 · 419f774 · 419f774
1 parent 84efac7
commit 419f774
Show file tree

Hide file tree

Showing 12 changed files with 246 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -56,7 +56,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Vis Builder] Add app filter and query persistence without using state container ([#3100](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3100))
 - [Optimizer] Increase timeout waiting for the exiting of an optimizer worker ([#3193](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3193))
 - [Data] Update `createAggConfig` so that newly created configs can be added to beginning of `aggConfig` array ([#3160](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3160))
-
+- 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
@@ -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
@@ -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: 'id1',
+      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
@@ -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
@@ -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
@@ -69,6 +69,7 @@ function createPluginManifest(manifestProps: Partial<PluginManifest> = {}): 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
@@ -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
@@ -56,6 +56,7 @@ function createPluginManifest(manifestProps: Partial<PluginManifest> = {}): 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
@@ -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,