Skip to content

Commit 9f525d5

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 da501e4 commit 9f525d5

12 files changed

+231
-3
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
8585
- Add satisfaction survey link to help menu ([#3676] (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3676))
8686
- [Vis Builder] Add persistence to visualizations inner state ([#3751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3751))
8787
- [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397))
88+
- 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))
8889
- [Multiple Datasource] Support Amazon OpenSearch Serverless ([#3957](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3957))
8990
- Add support for Node.js >=14.20.1 <19 ([#4071](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4071))
9091
- Bundle Node.js 14 as a fallback for operating systems that cannot run Node.js 18 ([#4151](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4151))

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

+79
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,79 @@ test('return error when manifest contains unrecognized properties', async () =>
247247
});
248248
});
249249

250+
describe('requiredOpenSearchPlugins', () => {
251+
test('return error when plugin `requiredOpenSearchPlugins` is a string and not an array of string', async () => {
252+
mockReadFilePromise.mockResolvedValue(
253+
Buffer.from(
254+
JSON.stringify({
255+
id: 'id1',
256+
version: '7.0.0',
257+
server: true,
258+
requiredOpenSearchPlugins: 'abc',
259+
})
260+
)
261+
);
262+
263+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
264+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id1" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
265+
type: PluginDiscoveryErrorType.InvalidManifest,
266+
path: pluginManifestPath,
267+
});
268+
});
269+
270+
test('return error when `requiredOpenSearchPlugins` is not a string', async () => {
271+
mockReadFilePromise.mockResolvedValue(
272+
Buffer.from(JSON.stringify({ id: 'id2', version: '7.0.0', requiredOpenSearchPlugins: 2 }))
273+
);
274+
275+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
276+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id2" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
277+
type: PluginDiscoveryErrorType.InvalidManifest,
278+
path: pluginManifestPath,
279+
});
280+
});
281+
282+
test('return error when plugin requiredOpenSearchPlugins is an array that contains non-string values', async () => {
283+
mockReadFilePromise.mockResolvedValue(
284+
Buffer.from(
285+
JSON.stringify({ id: 'id3', version: '7.0.0', requiredOpenSearchPlugins: ['plugin1', 2] })
286+
)
287+
);
288+
289+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
290+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id3" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
291+
type: PluginDiscoveryErrorType.InvalidManifest,
292+
path: pluginManifestPath,
293+
});
294+
});
295+
296+
test('Happy path when plugin `requiredOpenSearchPlugins` is an array of string', async () => {
297+
mockReadFilePromise.mockResolvedValue(
298+
Buffer.from(
299+
JSON.stringify({
300+
id: 'id1',
301+
version: '7.0.0',
302+
server: true,
303+
requiredOpenSearchPlugins: ['plugin1', 'plugin2'],
304+
})
305+
)
306+
);
307+
308+
await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
309+
id: 'id1',
310+
configPath: 'id_1',
311+
version: '7.0.0',
312+
opensearchDashboardsVersion: '7.0.0',
313+
optionalPlugins: [],
314+
requiredPlugins: [],
315+
requiredOpenSearchPlugins: ['plugin1', 'plugin2'],
316+
requiredBundles: [],
317+
server: true,
318+
ui: false,
319+
});
320+
});
321+
});
322+
250323
describe('configPath', () => {
251324
test('falls back to plugin id if not specified', async () => {
252325
mockReadFilePromise.mockResolvedValue(
@@ -301,6 +374,7 @@ test('set defaults for all missing optional fields', async () => {
301374
opensearchDashboardsVersion: '7.0.0',
302375
optionalPlugins: [],
303376
requiredPlugins: [],
377+
requiredOpenSearchPlugins: [],
304378
requiredBundles: [],
305379
server: true,
306380
ui: false,
@@ -317,6 +391,7 @@ test('return all set optional fields as they are in manifest', async () => {
317391
opensearchDashboardsVersion: '7.0.0',
318392
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
319393
optionalPlugins: ['some-optional-plugin'],
394+
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
320395
ui: true,
321396
})
322397
)
@@ -330,6 +405,7 @@ test('return all set optional fields as they are in manifest', async () => {
330405
optionalPlugins: ['some-optional-plugin'],
331406
requiredBundles: [],
332407
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
408+
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
333409
server: false,
334410
ui: true,
335411
});
@@ -344,6 +420,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
344420
version: 'some-version',
345421
opensearchDashboardsVersion: '7.0.0-alpha2',
346422
requiredPlugins: ['some-required-plugin'],
423+
requiredOpenSearchPlugins: [],
347424
server: true,
348425
})
349426
)
@@ -356,6 +433,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
356433
opensearchDashboardsVersion: '7.0.0-alpha2',
357434
optionalPlugins: [],
358435
requiredPlugins: ['some-required-plugin'],
436+
requiredOpenSearchPlugins: [],
359437
requiredBundles: [],
360438
server: true,
361439
ui: false,
@@ -383,6 +461,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version is `ope
383461
opensearchDashboardsVersion: 'opensearchDashboards',
384462
optionalPlugins: [],
385463
requiredPlugins: ['some-required-plugin'],
464+
requiredOpenSearchPlugins: [],
386465
requiredBundles: [],
387466
server: true,
388467
ui: true,

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

+26
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ const KNOWN_MANIFEST_FIELDS = (() => {
6161
version: true,
6262
configPath: true,
6363
requiredPlugins: true,
64+
requiredOpenSearchPlugins: true,
6465
optionalPlugins: true,
6566
ui: true,
6667
server: true,
@@ -156,6 +157,28 @@ export async function parseManifest(
156157
);
157158
}
158159

160+
if (
161+
manifest.requiredOpenSearchPlugins !== undefined &&
162+
(!Array.isArray(manifest.requiredOpenSearchPlugins) ||
163+
!manifest.requiredOpenSearchPlugins.every((plugin) => typeof plugin === 'string'))
164+
) {
165+
throw PluginDiscoveryError.invalidManifest(
166+
manifestPath,
167+
new Error(
168+
`The "requiredOpenSearchPlugins" in plugin manifest for "${manifest.id}" should be an array of strings.`
169+
)
170+
);
171+
}
172+
173+
if (
174+
Array.isArray(manifest.requiredOpenSearchPlugins) &&
175+
manifest.requiredOpenSearchPlugins.length > 0
176+
) {
177+
log.info(
178+
`Plugin ${manifest.id} has a dependency on following OpenSearch plugin(s): "${manifest.requiredOpenSearchPlugins}".`
179+
);
180+
}
181+
159182
const expectedOpenSearchDashboardsVersion =
160183
typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion
161184
? manifest.opensearchDashboardsVersion
@@ -198,6 +221,9 @@ export async function parseManifest(
198221
opensearchDashboardsVersion: expectedOpenSearchDashboardsVersion,
199222
configPath: manifest.configPath || snakeCase(manifest.id),
200223
requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [],
224+
requiredOpenSearchPlugins: Array.isArray(manifest.requiredOpenSearchPlugins)
225+
? manifest.requiredOpenSearchPlugins
226+
: [],
201227
optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [],
202228
requiredBundles: Array.isArray(manifest.requiredBundles) ? manifest.requiredBundles : [],
203229
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

+75-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.mockResolvedValue({
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 [
@@ -484,6 +503,16 @@ describe('start', () => {
484503
afterAll(() => {
485504
jest.useRealTimers();
486505
});
506+
const opensearch = startDeps.opensearch;
507+
opensearch.client.asInternalUser.cat.plugins.mockResolvedValue({
508+
body: [
509+
{
510+
name: 'node-1',
511+
component: 'test-plugin',
512+
version: 'v1',
513+
},
514+
],
515+
} as any);
487516
it('throws timeout error if "start" was not completed in 30 sec.', async () => {
488517
const plugin: PluginWrapper = createPlugin('timeout-start');
489518
jest.spyOn(plugin, 'setup').mockResolvedValue({});
@@ -517,4 +546,48 @@ describe('start', () => {
517546
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
518547
expect(log.info).toHaveBeenCalledWith(`Starting [2] plugins: [order-1,order-0]`);
519548
});
549+
550+
it('validates opensearch plugin installation when dependency is fulfilled', async () => {
551+
[
552+
createPlugin('order-1', { requiredOSPlugin: ['test-plugin'] }),
553+
createPlugin('order-2'),
554+
].forEach((plugin, index) => {
555+
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
556+
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
557+
pluginsSystem.addPlugin(plugin);
558+
});
559+
560+
await pluginsSystem.setupPlugins(setupDeps);
561+
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
562+
expect(pluginsStart).toBeInstanceOf(Map);
563+
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
564+
});
565+
566+
it('validates opensearch plugin installation and does not error out when plugin is not installed', async () => {
567+
[
568+
createPlugin('id-1', { requiredOSPlugin: ['missing-opensearch-dep'] }),
569+
createPlugin('id-2'),
570+
].forEach((plugin, index) => {
571+
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
572+
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
573+
pluginsSystem.addPlugin(plugin);
574+
});
575+
576+
await pluginsSystem.setupPlugins(setupDeps);
577+
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
578+
expect(pluginsStart).toBeInstanceOf(Map);
579+
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
580+
});
581+
582+
it('validates opensearch plugin installation and does not error out when there is no dependency', async () => {
583+
[createPlugin('id-1'), createPlugin('id-2')].forEach((plugin, index) => {
584+
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
585+
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
586+
pluginsSystem.addPlugin(plugin);
587+
});
588+
await pluginsSystem.setupPlugins(setupDeps);
589+
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
590+
expect(pluginsStart).toBeInstanceOf(Map);
591+
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
592+
});
520593
});

0 commit comments

Comments
 (0)