Skip to content

Commit 9f90a82

Browse files
committed
Adds plugin manifest config to define OpenSearch plugin dependency and verifies if it is installed
Resolves Issue -#2799 Signed-off-by: Manasvini B Suryanarayana <manasvis@amazon.com>
1 parent 84efac7 commit 9f90a82

12 files changed

+218
-4
lines changed

CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
5656
- [Vis Builder] Add app filter and query persistence without using state container ([#3100](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3100))
5757
- [Optimizer] Increase timeout waiting for the exiting of an optimizer worker ([#3193](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3193))
5858
- [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))
59-
59+
- 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))
6060

6161
### 🐛 Bug Fixes
6262

src/core/public/plugins/plugins_service.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ function createManifest(
8484
version: 'some-version',
8585
configPath: ['path'],
8686
requiredPlugins: required,
87+
requiredOpenSearchPlugins: optional,
8788
optionalPlugins: optional,
8889
requiredBundles: [],
8990
};

src/core/server/plugins/discovery/plugin_manifest_parser.test.ts

+60
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,60 @@ test('return error when manifest contains unrecognized properties', async () =>
279279
});
280280
});
281281

282+
test('return error when plugin `requiredOpenSearchPlugins` is a string and not an array of string', async () => {
283+
mockReadFile.mockImplementation((path, cb) => {
284+
cb(
285+
null,
286+
Buffer.from(
287+
JSON.stringify({
288+
id: 'id1',
289+
version: '7.0.0',
290+
server: true,
291+
requiredOpenSearchPlugins: 'abc',
292+
})
293+
)
294+
);
295+
});
296+
297+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
298+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id1" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
299+
type: PluginDiscoveryErrorType.InvalidManifest,
300+
path: pluginManifestPath,
301+
});
302+
});
303+
304+
test('return error when `requiredOpenSearchPlugins` is not a string', async () => {
305+
mockReadFile.mockImplementation((path, cb) => {
306+
cb(
307+
null,
308+
Buffer.from(JSON.stringify({ id: 'id2', version: '7.0.0', requiredOpenSearchPlugins: 2 }))
309+
);
310+
});
311+
312+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
313+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id2" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
314+
type: PluginDiscoveryErrorType.InvalidManifest,
315+
path: pluginManifestPath,
316+
});
317+
});
318+
319+
test('return error when plugin requiredOpenSearchPlugins is an array that contains non-string values', async () => {
320+
mockReadFile.mockImplementation((path, cb) => {
321+
cb(
322+
null,
323+
Buffer.from(
324+
JSON.stringify({ id: 'id3', version: '7.0.0', requiredOpenSearchPlugins: ['plugin1', 2] })
325+
)
326+
);
327+
});
328+
329+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
330+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id3" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
331+
type: PluginDiscoveryErrorType.InvalidManifest,
332+
path: pluginManifestPath,
333+
});
334+
});
335+
282336
describe('configPath', () => {
283337
test('falls back to plugin id if not specified', async () => {
284338
mockReadFile.mockImplementation((path, cb) => {
@@ -339,6 +393,7 @@ test('set defaults for all missing optional fields', async () => {
339393
opensearchDashboardsVersion: '7.0.0',
340394
optionalPlugins: [],
341395
requiredPlugins: [],
396+
requiredOpenSearchPlugins: [],
342397
requiredBundles: [],
343398
server: true,
344399
ui: false,
@@ -357,6 +412,7 @@ test('return all set optional fields as they are in manifest', async () => {
357412
opensearchDashboardsVersion: '7.0.0',
358413
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
359414
optionalPlugins: ['some-optional-plugin'],
415+
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
360416
ui: true,
361417
})
362418
)
@@ -371,6 +427,7 @@ test('return all set optional fields as they are in manifest', async () => {
371427
optionalPlugins: ['some-optional-plugin'],
372428
requiredBundles: [],
373429
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
430+
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
374431
server: false,
375432
ui: true,
376433
});
@@ -387,6 +444,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
387444
version: 'some-version',
388445
opensearchDashboardsVersion: '7.0.0-alpha2',
389446
requiredPlugins: ['some-required-plugin'],
447+
requiredOpenSearchPlugins: [],
390448
server: true,
391449
})
392450
)
@@ -400,6 +458,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
400458
opensearchDashboardsVersion: '7.0.0-alpha2',
401459
optionalPlugins: [],
402460
requiredPlugins: ['some-required-plugin'],
461+
requiredOpenSearchPlugins: [],
403462
requiredBundles: [],
404463
server: true,
405464
ui: false,
@@ -430,6 +489,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version is `ope
430489
opensearchDashboardsVersion: 'opensearchDashboards',
431490
optionalPlugins: [],
432491
requiredPlugins: ['some-required-plugin'],
492+
requiredOpenSearchPlugins: [],
433493
requiredBundles: [],
434494
server: true,
435495
ui: true,

src/core/server/plugins/discovery/plugin_manifest_parser.ts

+23
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ const KNOWN_MANIFEST_FIELDS = (() => {
6565
version: true,
6666
configPath: true,
6767
requiredPlugins: true,
68+
requiredOpenSearchPlugins: true,
6869
optionalPlugins: true,
6970
ui: true,
7071
server: true,
@@ -160,6 +161,25 @@ export async function parseManifest(
160161
);
161162
}
162163

164+
if (
165+
manifest.requiredOpenSearchPlugins !== undefined &&
166+
(!Array.isArray(manifest.requiredOpenSearchPlugins) ||
167+
!manifest.requiredOpenSearchPlugins.every((plugin) => typeof plugin === 'string'))
168+
) {
169+
throw PluginDiscoveryError.invalidManifest(
170+
manifestPath,
171+
new Error(
172+
`The "requiredOpenSearchPlugins" in plugin manifest for "${manifest.id}" should be an array of strings.`
173+
)
174+
);
175+
}
176+
177+
if (Array.isArray(manifest.requiredOpenSearchPlugins)) {
178+
log.warn(
179+
`Plugin ${manifest.id} has a dependency on OpenSearch plugin(s). All dependency plugins have to be installed on the cluster to be able to start OpenSearch Dashboard.`
180+
);
181+
}
182+
163183
const expectedOpenSearchDashboardsVersion =
164184
typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion
165185
? manifest.opensearchDashboardsVersion
@@ -202,6 +222,9 @@ export async function parseManifest(
202222
opensearchDashboardsVersion: expectedOpenSearchDashboardsVersion,
203223
configPath: manifest.configPath || snakeCase(manifest.id),
204224
requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [],
225+
requiredOpenSearchPlugins: Array.isArray(manifest.requiredOpenSearchPlugins)
226+
? manifest.requiredOpenSearchPlugins
227+
: [],
205228
optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [],
206229
requiredBundles: Array.isArray(manifest.requiredBundles) ? manifest.requiredBundles : [],
207230
ui: includesUiPlugin,

src/core/server/plugins/integration_tests/plugins_service.test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ describe('PluginsService', () => {
5757
disabled = false,
5858
version = 'some-version',
5959
requiredPlugins = [],
60+
requiredOpenSearchPlugins = [],
6061
requiredBundles = [],
6162
optionalPlugins = [],
6263
opensearchDashboardsVersion = '7.0.0',
@@ -68,6 +69,7 @@ describe('PluginsService', () => {
6869
disabled?: boolean;
6970
version?: string;
7071
requiredPlugins?: string[];
72+
requiredOpenSearchPlugins?: string[];
7173
requiredBundles?: string[];
7274
optionalPlugins?: string[];
7375
opensearchDashboardsVersion?: string;
@@ -84,6 +86,7 @@ describe('PluginsService', () => {
8486
configPath: `${configPath}${disabled ? '-disabled' : ''}`,
8587
opensearchDashboardsVersion,
8688
requiredPlugins,
89+
requiredOpenSearchPlugins,
8790
requiredBundles,
8891
optionalPlugins,
8992
server,

src/core/server/plugins/plugin.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ function createPluginManifest(manifestProps: Partial<PluginManifest> = {}): Plug
6969
configPath: 'path',
7070
opensearchDashboardsVersion: '7.0.0',
7171
requiredPlugins: ['some-required-dep'],
72+
requiredOpenSearchPlugins: ['some-os-plugins'],
7273
optionalPlugins: ['some-optional-dep'],
7374
requiredBundles: [],
7475
server: true,

src/core/server/plugins/plugin.ts

+2
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export class PluginWrapper<
6666
public readonly configPath: PluginManifest['configPath'];
6767
public readonly requiredPlugins: PluginManifest['requiredPlugins'];
6868
public readonly optionalPlugins: PluginManifest['optionalPlugins'];
69+
public readonly requiredOpenSearchPlugins: PluginManifest['requiredOpenSearchPlugins'];
6970
public readonly requiredBundles: PluginManifest['requiredBundles'];
7071
public readonly includesServerPlugin: PluginManifest['server'];
7172
public readonly includesUiPlugin: PluginManifest['ui'];
@@ -95,6 +96,7 @@ export class PluginWrapper<
9596
this.configPath = params.manifest.configPath;
9697
this.requiredPlugins = params.manifest.requiredPlugins;
9798
this.optionalPlugins = params.manifest.optionalPlugins;
99+
this.requiredOpenSearchPlugins = params.manifest.requiredOpenSearchPlugins;
98100
this.requiredBundles = params.manifest.requiredBundles;
99101
this.includesServerPlugin = params.manifest.server;
100102
this.includesUiPlugin = params.manifest.ui;

src/core/server/plugins/plugin_context.test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ function createPluginManifest(manifestProps: Partial<PluginManifest> = {}): Plug
5656
configPath: 'path',
5757
opensearchDashboardsVersion: '7.0.0',
5858
requiredPlugins: ['some-required-dep'],
59+
requiredOpenSearchPlugins: ['some-backend-plugin'],
5960
requiredBundles: [],
6061
optionalPlugins: ['some-optional-dep'],
6162
server: true,

src/core/server/plugins/plugins_service.test.ts

+3
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ const createPlugin = (
7878
disabled = false,
7979
version = 'some-version',
8080
requiredPlugins = [],
81+
requiredOpenSearchPlugins = [],
8182
requiredBundles = [],
8283
optionalPlugins = [],
8384
opensearchDashboardsVersion = '7.0.0',
@@ -89,6 +90,7 @@ const createPlugin = (
8990
disabled?: boolean;
9091
version?: string;
9192
requiredPlugins?: string[];
93+
requiredOpenSearchPlugins?: string[];
9294
requiredBundles?: string[];
9395
optionalPlugins?: string[];
9496
opensearchDashboardsVersion?: string;
@@ -105,6 +107,7 @@ const createPlugin = (
105107
configPath: `${configPath}${disabled ? '-disabled' : ''}`,
106108
opensearchDashboardsVersion,
107109
requiredPlugins,
110+
requiredOpenSearchPlugins,
108111
requiredBundles,
109112
optionalPlugins,
110113
server,

src/core/server/plugins/plugins_system.test.ts

+88-2
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,16 @@ function createPlugin(
5353
{
5454
required = [],
5555
optional = [],
56+
requiredOSPlugin = [],
5657
server = true,
5758
ui = true,
58-
}: { required?: string[]; optional?: string[]; server?: boolean; ui?: boolean } = {}
59+
}: {
60+
required?: string[];
61+
optional?: string[];
62+
requiredOSPlugin?: string[];
63+
server?: boolean;
64+
ui?: boolean;
65+
} = {}
5966
) {
6067
return new PluginWrapper({
6168
path: 'some-path',
@@ -65,6 +72,7 @@ function createPlugin(
6572
configPath: 'path',
6673
opensearchDashboardsVersion: '7.0.0',
6774
requiredPlugins: required,
75+
requiredOpenSearchPlugins: requiredOSPlugin,
6876
optionalPlugins: optional,
6977
requiredBundles: [],
7078
server,
@@ -187,7 +195,7 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
187195
}
188196
const plugins = new Map([
189197
[
190-
createPlugin('order-4', { required: ['order-2'] }),
198+
createPlugin('order-4', { required: ['order-2'], requiredOSPlugin: ['test-plugin'] }),
191199
{
192200
setup: { 'order-2': 'added-as-2' },
193201
start: { 'order-2': 'started-as-2' },
@@ -244,6 +252,17 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
244252
startContextMap.get(plugin.name)
245253
);
246254

255+
const opensearch = startDeps.opensearch;
256+
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
257+
body: [
258+
{
259+
name: 'node-1',
260+
component: 'test-plugin',
261+
version: 'v1',
262+
},
263+
],
264+
} as any);
265+
247266
expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(`
248267
Array [
249268
Array [
@@ -517,4 +536,71 @@ describe('start', () => {
517536
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
518537
expect(log.info).toHaveBeenCalledWith(`Starting [2] plugins: [order-1,order-0]`);
519538
});
539+
540+
it('validates opensearch plugin installation', async () => {
541+
[
542+
createPlugin('order-1', { requiredOSPlugin: ['test-plugin'] }),
543+
createPlugin('order-2'),
544+
].forEach((plugin, index) => {
545+
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
546+
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
547+
pluginsSystem.addPlugin(plugin);
548+
});
549+
550+
const opensearch = startDeps.opensearch;
551+
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
552+
body: [
553+
{
554+
name: 'node-1',
555+
component: 'test-plugin',
556+
version: 'v1',
557+
},
558+
],
559+
} as any);
560+
await pluginsSystem.setupPlugins(setupDeps);
561+
await pluginsSystem.startPlugins(startDeps);
562+
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
563+
});
564+
565+
it('validates opensearch plugin installation and errors out when plugin is not installed', async () => {
566+
[
567+
createPlugin('id-1', { requiredOSPlugin: ['missing-opensearch-dep'] }),
568+
createPlugin('id-2'),
569+
].forEach((plugin, index) => {
570+
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
571+
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
572+
pluginsSystem.addPlugin(plugin);
573+
});
574+
const opensearch = startDeps.opensearch;
575+
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
576+
body: [
577+
{
578+
name: 'node-1',
579+
component: 'test-plugin1',
580+
version: 'v1',
581+
},
582+
],
583+
} as any);
584+
await pluginsSystem.setupPlugins(setupDeps);
585+
586+
await expect(pluginsSystem.startPlugins(startDeps)).rejects.toMatchInlineSnapshot(
587+
`[Error: Plugin "id-1" has a dependency on OpenSearch plugin "missing-opensearch-dep" which is not installed on the cluster.]`
588+
);
589+
});
590+
591+
it('validates opensearch plugin installation and does not errors out when there is no dependency', async () => {
592+
[createPlugin('id-1'), createPlugin('id-2')].forEach((plugin, index) => {
593+
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
594+
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
595+
pluginsSystem.addPlugin(plugin);
596+
});
597+
const opensearch = startDeps.opensearch;
598+
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
599+
body: [],
600+
} as any);
601+
await pluginsSystem.setupPlugins(setupDeps);
602+
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
603+
expect(pluginsStart).toBeInstanceOf(Map);
604+
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
605+
});
520606
});

0 commit comments

Comments
 (0)