Skip to content

Commit a3f1702

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

13 files changed

+247
-4
lines changed

CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
8282
- Add satisfaction survey link to help menu ([#3676] (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3676))
8383
- [Vis Builder] Add persistence to visualizations inner state ([#3751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3751))
8484
- [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397))
85-
85+
- 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))
8686
### 🐛 Bug Fixes
8787

8888
- [Vis Builder] Fixes auto bounds for timeseries bar chart visualization ([2401](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2401))
+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Subproject commit 014cbfa1cc22b664ae97f818a8f99e90fac3d599

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

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

282+
describe('requiredOpenSearchPlugins', () => {
283+
test('return error when plugin `requiredOpenSearchPlugins` is a string and not an array of string', async () => {
284+
mockReadFile.mockImplementation((path, cb) => {
285+
cb(
286+
null,
287+
Buffer.from(
288+
JSON.stringify({
289+
id: 'id1',
290+
version: '7.0.0',
291+
server: true,
292+
requiredOpenSearchPlugins: 'abc',
293+
})
294+
)
295+
);
296+
});
297+
298+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
299+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id1" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
300+
type: PluginDiscoveryErrorType.InvalidManifest,
301+
path: pluginManifestPath,
302+
});
303+
});
304+
305+
test('return error when `requiredOpenSearchPlugins` is not a string', async () => {
306+
mockReadFile.mockImplementation((path, cb) => {
307+
cb(
308+
null,
309+
Buffer.from(JSON.stringify({ id: 'id2', version: '7.0.0', requiredOpenSearchPlugins: 2 }))
310+
);
311+
});
312+
313+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
314+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id2" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
315+
type: PluginDiscoveryErrorType.InvalidManifest,
316+
path: pluginManifestPath,
317+
});
318+
});
319+
320+
test('return error when plugin requiredOpenSearchPlugins is an array that contains non-string values', async () => {
321+
mockReadFile.mockImplementation((path, cb) => {
322+
cb(
323+
null,
324+
Buffer.from(
325+
JSON.stringify({ id: 'id3', version: '7.0.0', requiredOpenSearchPlugins: ['plugin1', 2] })
326+
)
327+
);
328+
});
329+
330+
await expect(parseManifest(pluginPath, packageInfo, logger)).rejects.toMatchObject({
331+
message: `The "requiredOpenSearchPlugins" in plugin manifest for "id3" should be an array of strings. (invalid-manifest, ${pluginManifestPath})`,
332+
type: PluginDiscoveryErrorType.InvalidManifest,
333+
path: pluginManifestPath,
334+
});
335+
});
336+
337+
test('Happy path when plugin `requiredOpenSearchPlugins` is an array of string', async () => {
338+
mockReadFile.mockImplementation((path, cb) => {
339+
cb(
340+
null,
341+
Buffer.from(
342+
JSON.stringify({
343+
id: 'id1',
344+
version: '7.0.0',
345+
server: true,
346+
requiredOpenSearchPlugins: ['plugin1', 'plugin2'],
347+
})
348+
)
349+
);
350+
});
351+
352+
await expect(parseManifest(pluginPath, packageInfo, logger)).resolves.toEqual({
353+
id: 'id1',
354+
configPath: 'id_1',
355+
version: '7.0.0',
356+
opensearchDashboardsVersion: '7.0.0',
357+
optionalPlugins: [],
358+
requiredPlugins: [],
359+
requiredOpenSearchPlugins: ['plugin1', 'plugin2'],
360+
requiredBundles: [],
361+
server: true,
362+
ui: false,
363+
});
364+
});
365+
});
366+
282367
describe('configPath', () => {
283368
test('falls back to plugin id if not specified', async () => {
284369
mockReadFile.mockImplementation((path, cb) => {
@@ -339,6 +424,7 @@ test('set defaults for all missing optional fields', async () => {
339424
opensearchDashboardsVersion: '7.0.0',
340425
optionalPlugins: [],
341426
requiredPlugins: [],
427+
requiredOpenSearchPlugins: [],
342428
requiredBundles: [],
343429
server: true,
344430
ui: false,
@@ -357,6 +443,7 @@ test('return all set optional fields as they are in manifest', async () => {
357443
opensearchDashboardsVersion: '7.0.0',
358444
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
359445
optionalPlugins: ['some-optional-plugin'],
446+
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
360447
ui: true,
361448
})
362449
)
@@ -371,6 +458,7 @@ test('return all set optional fields as they are in manifest', async () => {
371458
optionalPlugins: ['some-optional-plugin'],
372459
requiredBundles: [],
373460
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
461+
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
374462
server: false,
375463
ui: true,
376464
});
@@ -387,6 +475,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
387475
version: 'some-version',
388476
opensearchDashboardsVersion: '7.0.0-alpha2',
389477
requiredPlugins: ['some-required-plugin'],
478+
requiredOpenSearchPlugins: [],
390479
server: true,
391480
})
392481
)
@@ -400,6 +489,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
400489
opensearchDashboardsVersion: '7.0.0-alpha2',
401490
optionalPlugins: [],
402491
requiredPlugins: ['some-required-plugin'],
492+
requiredOpenSearchPlugins: [],
403493
requiredBundles: [],
404494
server: true,
405495
ui: false,
@@ -430,6 +520,7 @@ test('return manifest when plugin expected OpenSearch Dashboards version is `ope
430520
opensearchDashboardsVersion: 'opensearchDashboards',
431521
optionalPlugins: [],
432522
requiredPlugins: ['some-required-plugin'],
523+
requiredOpenSearchPlugins: [],
433524
requiredBundles: [],
434525
server: true,
435526
ui: true,

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

+26
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,28 @@ 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 (
178+
Array.isArray(manifest.requiredOpenSearchPlugins) &&
179+
manifest.requiredOpenSearchPlugins.length > 0
180+
) {
181+
log.warn(
182+
`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.`
183+
);
184+
}
185+
163186
const expectedOpenSearchDashboardsVersion =
164187
typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion
165188
? manifest.opensearchDashboardsVersion
@@ -202,6 +225,9 @@ export async function parseManifest(
202225
opensearchDashboardsVersion: expectedOpenSearchDashboardsVersion,
203226
configPath: manifest.configPath || snakeCase(manifest.id),
204227
requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [],
228+
requiredOpenSearchPlugins: Array.isArray(manifest.requiredOpenSearchPlugins)
229+
? manifest.requiredOpenSearchPlugins
230+
: [],
205231
optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [],
206232
requiredBundles: Array.isArray(manifest.requiredBundles) ? manifest.requiredBundles : [],
207233
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,

0 commit comments

Comments
 (0)