Skip to content

Commit 636ae73

Browse files
authored
Revert "Adds plugin manifest config to define OpenSearch plugin dependency and verifies if it is installed (#3116) (#4195)" (#4205)
This reverts commit e85ea81. Signed-off-by: Kawika Avilla <kavilla414@gmail.com>
1 parent e85ea81 commit 636ae73

11 files changed

+3
-230
lines changed

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

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

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

-79
Original file line numberDiff line numberDiff line change
@@ -247,79 +247,6 @@ 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-
323250
describe('configPath', () => {
324251
test('falls back to plugin id if not specified', async () => {
325252
mockReadFilePromise.mockResolvedValue(
@@ -374,7 +301,6 @@ test('set defaults for all missing optional fields', async () => {
374301
opensearchDashboardsVersion: '7.0.0',
375302
optionalPlugins: [],
376303
requiredPlugins: [],
377-
requiredOpenSearchPlugins: [],
378304
requiredBundles: [],
379305
server: true,
380306
ui: false,
@@ -391,7 +317,6 @@ test('return all set optional fields as they are in manifest', async () => {
391317
opensearchDashboardsVersion: '7.0.0',
392318
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
393319
optionalPlugins: ['some-optional-plugin'],
394-
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
395320
ui: true,
396321
})
397322
)
@@ -405,7 +330,6 @@ test('return all set optional fields as they are in manifest', async () => {
405330
optionalPlugins: ['some-optional-plugin'],
406331
requiredBundles: [],
407332
requiredPlugins: ['some-required-plugin', 'some-required-plugin-2'],
408-
requiredOpenSearchPlugins: ['test-opensearch-plugin-1', 'test-opensearch-plugin-2'],
409333
server: false,
410334
ui: true,
411335
});
@@ -420,7 +344,6 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
420344
version: 'some-version',
421345
opensearchDashboardsVersion: '7.0.0-alpha2',
422346
requiredPlugins: ['some-required-plugin'],
423-
requiredOpenSearchPlugins: [],
424347
server: true,
425348
})
426349
)
@@ -433,7 +356,6 @@ test('return manifest when plugin expected OpenSearch Dashboards version matches
433356
opensearchDashboardsVersion: '7.0.0-alpha2',
434357
optionalPlugins: [],
435358
requiredPlugins: ['some-required-plugin'],
436-
requiredOpenSearchPlugins: [],
437359
requiredBundles: [],
438360
server: true,
439361
ui: false,
@@ -461,7 +383,6 @@ test('return manifest when plugin expected OpenSearch Dashboards version is `ope
461383
opensearchDashboardsVersion: 'opensearchDashboards',
462384
optionalPlugins: [],
463385
requiredPlugins: ['some-required-plugin'],
464-
requiredOpenSearchPlugins: [],
465386
requiredBundles: [],
466387
server: true,
467388
ui: true,

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

-26
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ const KNOWN_MANIFEST_FIELDS = (() => {
6161
version: true,
6262
configPath: true,
6363
requiredPlugins: true,
64-
requiredOpenSearchPlugins: true,
6564
optionalPlugins: true,
6665
ui: true,
6766
server: true,
@@ -157,28 +156,6 @@ export async function parseManifest(
157156
);
158157
}
159158

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-
182159
const expectedOpenSearchDashboardsVersion =
183160
typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion
184161
? manifest.opensearchDashboardsVersion
@@ -221,9 +198,6 @@ export async function parseManifest(
221198
opensearchDashboardsVersion: expectedOpenSearchDashboardsVersion,
222199
configPath: manifest.configPath || snakeCase(manifest.id),
223200
requiredPlugins: Array.isArray(manifest.requiredPlugins) ? manifest.requiredPlugins : [],
224-
requiredOpenSearchPlugins: Array.isArray(manifest.requiredOpenSearchPlugins)
225-
? manifest.requiredOpenSearchPlugins
226-
: [],
227201
optionalPlugins: Array.isArray(manifest.optionalPlugins) ? manifest.optionalPlugins : [],
228202
requiredBundles: Array.isArray(manifest.requiredBundles) ? manifest.requiredBundles : [],
229203
ui: includesUiPlugin,

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

-3
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ describe('PluginsService', () => {
5757
disabled = false,
5858
version = 'some-version',
5959
requiredPlugins = [],
60-
requiredOpenSearchPlugins = [],
6160
requiredBundles = [],
6261
optionalPlugins = [],
6362
opensearchDashboardsVersion = '7.0.0',
@@ -69,7 +68,6 @@ describe('PluginsService', () => {
6968
disabled?: boolean;
7069
version?: string;
7170
requiredPlugins?: string[];
72-
requiredOpenSearchPlugins?: string[];
7371
requiredBundles?: string[];
7472
optionalPlugins?: string[];
7573
opensearchDashboardsVersion?: string;
@@ -86,7 +84,6 @@ describe('PluginsService', () => {
8684
configPath: `${configPath}${disabled ? '-disabled' : ''}`,
8785
opensearchDashboardsVersion,
8886
requiredPlugins,
89-
requiredOpenSearchPlugins,
9087
requiredBundles,
9188
optionalPlugins,
9289
server,

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

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

src/core/server/plugins/plugin.ts

-2
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ 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'];
7069
public readonly requiredBundles: PluginManifest['requiredBundles'];
7170
public readonly includesServerPlugin: PluginManifest['server'];
7271
public readonly includesUiPlugin: PluginManifest['ui'];
@@ -96,7 +95,6 @@ export class PluginWrapper<
9695
this.configPath = params.manifest.configPath;
9796
this.requiredPlugins = params.manifest.requiredPlugins;
9897
this.optionalPlugins = params.manifest.optionalPlugins;
99-
this.requiredOpenSearchPlugins = params.manifest.requiredOpenSearchPlugins;
10098
this.requiredBundles = params.manifest.requiredBundles;
10199
this.includesServerPlugin = params.manifest.server;
102100
this.includesUiPlugin = params.manifest.ui;

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

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

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

-3
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ const createPlugin = (
7878
disabled = false,
7979
version = 'some-version',
8080
requiredPlugins = [],
81-
requiredOpenSearchPlugins = [],
8281
requiredBundles = [],
8382
optionalPlugins = [],
8483
opensearchDashboardsVersion = '7.0.0',
@@ -90,7 +89,6 @@ const createPlugin = (
9089
disabled?: boolean;
9190
version?: string;
9291
requiredPlugins?: string[];
93-
requiredOpenSearchPlugins?: string[];
9492
requiredBundles?: string[];
9593
optionalPlugins?: string[];
9694
opensearchDashboardsVersion?: string;
@@ -107,7 +105,6 @@ const createPlugin = (
107105
configPath: `${configPath}${disabled ? '-disabled' : ''}`,
108106
opensearchDashboardsVersion,
109107
requiredPlugins,
110-
requiredOpenSearchPlugins,
111108
requiredBundles,
112109
optionalPlugins,
113110
server,

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

+2-75
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,9 @@ function createPlugin(
5353
{
5454
required = [],
5555
optional = [],
56-
requiredOSPlugin = [],
5756
server = true,
5857
ui = true,
59-
}: {
60-
required?: string[];
61-
optional?: string[];
62-
requiredOSPlugin?: string[];
63-
server?: boolean;
64-
ui?: boolean;
65-
} = {}
58+
}: { required?: string[]; optional?: string[]; server?: boolean; ui?: boolean } = {}
6659
) {
6760
return new PluginWrapper({
6861
path: 'some-path',
@@ -72,7 +65,6 @@ function createPlugin(
7265
configPath: 'path',
7366
opensearchDashboardsVersion: '7.0.0',
7467
requiredPlugins: required,
75-
requiredOpenSearchPlugins: requiredOSPlugin,
7668
optionalPlugins: optional,
7769
requiredBundles: [],
7870
server,
@@ -195,7 +187,7 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
195187
}
196188
const plugins = new Map([
197189
[
198-
createPlugin('order-4', { required: ['order-2'], requiredOSPlugin: ['test-plugin'] }),
190+
createPlugin('order-4', { required: ['order-2'] }),
199191
{
200192
setup: { 'order-2': 'added-as-2' },
201193
start: { 'order-2': 'started-as-2' },
@@ -252,17 +244,6 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
252244
startContextMap.get(plugin.name)
253245
);
254246

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-
266247
expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(`
267248
Array [
268249
Array [
@@ -503,16 +484,6 @@ describe('start', () => {
503484
afterAll(() => {
504485
jest.useRealTimers();
505486
});
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);
516487
it('throws timeout error if "start" was not completed in 30 sec.', async () => {
517488
const plugin: PluginWrapper = createPlugin('timeout-start');
518489
jest.spyOn(plugin, 'setup').mockResolvedValue({});
@@ -546,48 +517,4 @@ describe('start', () => {
546517
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
547518
expect(log.info).toHaveBeenCalledWith(`Starting [2] plugins: [order-1,order-0]`);
548519
});
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-
});
593520
});

0 commit comments

Comments
 (0)