Skip to content

Commit

Permalink
Adds plugin manifest config to define OpenSearch plugin dependency an…
Browse files Browse the repository at this point in the history
…d verifies if it is installed

Resolves Issue -opensearch-project#2799

Signed-off-by: Manasvini B Suryanarayana <manasvis@amazon.com>
  • Loading branch information
manasvinibs committed Dec 21, 2022
1 parent 5764d6c commit 27f3147
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [I18n] Register ru, ru-RU locale ([#2817](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2817))
- Add yarn opensearch arg to setup plugin dependencies ([#2544](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2544))
- [Multi DataSource] Test the connection to an external data source when creating or updating ([#2973](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2973))
- 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

Expand Down
1 change: 1 addition & 0 deletions src/core/public/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ function createManifest(
version: 'some-version',
configPath: ['path'],
requiredPlugins: required,
requiredOpenSearchPlugins: optional,
optionalPlugins: optional,
requiredBundles: [],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ test('set defaults for all missing optional fields', async () => {
opensearchDashboardsVersion: '7.0.0',
optionalPlugins: [],
requiredPlugins: [],
requiredOpenSearchPlugins: [],
requiredBundles: [],
server: true,
ui: false,
Expand All @@ -357,6 +358,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,
})
)
Expand All @@ -371,6 +373,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,
});
Expand All @@ -387,6 +390,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,
})
)
Expand All @@ -400,6 +404,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,
Expand Down Expand Up @@ -430,6 +435,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,
Expand Down
25 changes: 25 additions & 0 deletions src/core/server/plugins/discovery/plugin_manifest_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ const KNOWN_MANIFEST_FIELDS = (() => {
version: true,
configPath: true,
requiredPlugins: true,
requiredOpenSearchPlugins: true,
optionalPlugins: true,
ui: true,
server: true,
Expand Down Expand Up @@ -160,6 +161,27 @@ export async function parseManifest(
);
}

if (
manifest.requiredOpenSearchPlugins !== undefined &&
!Array.isArray(manifest.requiredOpenSearchPlugins)
) {
throw PluginDiscoveryError.invalidManifest(
manifestPath,
new Error(
`The "requiredOpenSearchPlugins" in plugin manifest for "${manifest.id}" should either be a string or an array of strings.`
)
);
}

if (
manifest.requiredOpenSearchPlugins !== undefined &&
Array.isArray(manifest.requiredOpenSearchPlugins)
) {
log.warn(
`Plugin ${manifest.id} has a dependency on backend OpenSearch plugin. You need to install it on the cluster for data source.`
);
}

const expectedOpenSearchDashboardsVersion =
typeof manifest.opensearchDashboardsVersion === 'string' && manifest.opensearchDashboardsVersion
? manifest.opensearchDashboardsVersion
Expand Down Expand Up @@ -202,6 +224,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ describe('PluginsService', () => {
disabled = false,
version = 'some-version',
requiredPlugins = [],
requiredOpenSearchPlugins = [],
requiredBundles = [],
optionalPlugins = [],
opensearchDashboardsVersion = '7.0.0',
Expand All @@ -68,6 +69,7 @@ describe('PluginsService', () => {
disabled?: boolean;
version?: string;
requiredPlugins?: string[];
requiredOpenSearchPlugins?: string[];
requiredBundles?: string[];
optionalPlugins?: string[];
opensearchDashboardsVersion?: string;
Expand All @@ -84,6 +86,7 @@ describe('PluginsService', () => {
configPath: `${configPath}${disabled ? '-disabled' : ''}`,
opensearchDashboardsVersion,
requiredPlugins,
requiredOpenSearchPlugins,
requiredBundles,
optionalPlugins,
server,
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/plugins/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugin_context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const createPlugin = (
disabled = false,
version = 'some-version',
requiredPlugins = [],
requiredOpenSearchPlugins = [],
requiredBundles = [],
optionalPlugins = [],
opensearchDashboardsVersion = '7.0.0',
Expand All @@ -89,6 +90,7 @@ const createPlugin = (
disabled?: boolean;
version?: string;
requiredPlugins?: string[];
requiredOpenSearchPlugins?: string[];
requiredBundles?: string[];
optionalPlugins?: string[];
opensearchDashboardsVersion?: string;
Expand All @@ -105,6 +107,7 @@ const createPlugin = (
configPath: `${configPath}${disabled ? '-disabled' : ''}`,
opensearchDashboardsVersion,
requiredPlugins,
requiredOpenSearchPlugins,
requiredBundles,
optionalPlugins,
server,
Expand Down
90 changes: 88 additions & 2 deletions src/core/server/plugins/plugins_system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,16 @@ function createPlugin(
{
required = [],
optional = [],
requiredOSPlugin = [],
server = true,
ui = true,
}: { required?: string[]; optional?: string[]; server?: boolean; ui?: boolean } = {}
}: {
required?: string[];
optional?: string[];
requiredOSPlugin?: string[];
server?: boolean;
ui?: boolean;
} = {}
) {
return new PluginWrapper({
path: 'some-path',
Expand All @@ -65,6 +72,7 @@ function createPlugin(
configPath: 'path',
opensearchDashboardsVersion: '7.0.0',
requiredPlugins: required,
requiredOpenSearchPlugins: requiredOSPlugin,
optionalPlugins: optional,
requiredBundles: [],
server,
Expand Down Expand Up @@ -187,7 +195,7 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
}
const plugins = new Map([
[
createPlugin('order-4', { required: ['order-2'] }),
createPlugin('order-4', { required: ['order-2'], requiredOSPlugin: ['test-plugin'] }),
{
setup: { 'order-2': 'added-as-2' },
start: { 'order-2': 'started-as-2' },
Expand Down Expand Up @@ -244,6 +252,17 @@ test('correctly orders plugins and returns exposed values for "setup" and "start
startContextMap.get(plugin.name)
);

const opensearch = startDeps.opensearch;
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
body: [
{
name: 'node-1',
component: 'test-plugin',
version: 'v1',
},
],
} as any);

expect([...(await pluginsSystem.setupPlugins(setupDeps))]).toMatchInlineSnapshot(`
Array [
Array [
Expand Down Expand Up @@ -517,4 +536,71 @@ describe('start', () => {
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
expect(log.info).toHaveBeenCalledWith(`Starting [2] plugins: [order-1,order-0]`);
});

it('validates opensearch plugin installation', async () => {
[
createPlugin('order-1', { requiredOSPlugin: ['test-plugin'] }),
createPlugin('order-2'),
].forEach((plugin, index) => {
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
pluginsSystem.addPlugin(plugin);
});

const opensearch = startDeps.opensearch;
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
body: [
{
name: 'node-1',
component: 'test-plugin',
version: 'v1',
},
],
} as any);
await pluginsSystem.setupPlugins(setupDeps);
await pluginsSystem.startPlugins(startDeps);
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
});

it('validates opensearch plugin installation and errors out when plugin is not installed', async () => {
[
createPlugin('id-1', { requiredOSPlugin: ['missing-opensearch-dep'] }),
createPlugin('id-2'),
].forEach((plugin, index) => {
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
pluginsSystem.addPlugin(plugin);
});
const opensearch = startDeps.opensearch;
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
body: [
{
name: 'node-1',
component: 'test-plugin1',
version: 'v1',
},
],
} as any);
await pluginsSystem.setupPlugins(setupDeps);

await expect(pluginsSystem.startPlugins(startDeps)).rejects.toMatchInlineSnapshot(
`[Error: Plugin "id-1" has a dependency on OpenSearch data source plugin "missing-opensearch-dep" which needs to be installed.]`
);
});

it('validates opensearch plugin installation and does not errors out when there is no dependency', async () => {
[createPlugin('id-1'), createPlugin('id-2')].forEach((plugin, index) => {
jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`);
jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`);
pluginsSystem.addPlugin(plugin);
});
const opensearch = startDeps.opensearch;
opensearch.client.asInternalUser.cat.plugins.mockResolvedValueOnce({
body: [],
} as any);
await pluginsSystem.setupPlugins(setupDeps);
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
expect(pluginsStart).toBeInstanceOf(Map);
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
});
});
30 changes: 29 additions & 1 deletion src/core/server/plugins/plugins_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,41 @@ export class PluginsSystem {
timeout: 30 * Sec,
errorMessage: `Start lifecycle of "${pluginName}" plugin wasn't completed in 30sec. Consider disabling the plugin and re-start.`,
});

contracts.set(pluginName, contract);
}
await this.healthCheckOpenSearchPlugins(deps);

return contracts;
}

private async healthCheckOpenSearchPlugins(deps: PluginsServiceStartDeps) {
// make _cat/plugins?format=json call on http://localhost:9200/
const opensearchPlugins = await this.getOpensearchPlugins(deps);
for (const pluginName of this.satupPlugins) {
this.log.debug(`For plugin "${pluginName}"...`);
const plugin = this.plugins.get(pluginName)!;
const pluginBackendDeps = new Set([...plugin.requiredOpenSearchPlugins]);
pluginBackendDeps.forEach((opensearchPlugin) => {
if (!opensearchPlugins.find((obj) => obj.component === opensearchPlugin)) {
this.log.error(
`OpenSearch plugin dependency "${opensearchPlugin}" is not installed on the cluster for the OpenSearch Dashboard plugin to function as expected.`
);
throw new Error(
`Plugin "${pluginName}" has a dependency on OpenSearch data source plugin "${opensearchPlugin}" which needs to be installed.`
);
}
});
}
}

private async getOpensearchPlugins(deps: PluginsServiceStartDeps) {
// Makes cat.plugin api call to fetch list of OpenSearch plugins installed on the cluster
const { body } = await deps.opensearch.client.asInternalUser.cat.plugins<any[]>({
format: 'JSON',
});
return body;
}

public async stopPlugins() {
if (this.plugins.size === 0 || this.satupPlugins.length === 0) {
return;
Expand Down
6 changes: 6 additions & 0 deletions src/core/server/plugins/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ export interface PluginManifest {
*/
readonly requiredPlugins: readonly PluginName[];

/**
* An optional list of component names of the data source OpenSearch plugins that **must be** installed on the cluster
* for this plugin to function properly.
*/
readonly requiredOpenSearchPlugins: readonly PluginName[];

/**
* List of plugin ids that this plugin's UI code imports modules from that are
* not in `requiredPlugins`.
Expand Down

0 comments on commit 27f3147

Please sign in to comment.