Skip to content

Commit

Permalink
Add new core service to expose functionality to verify plugin compati…
Browse files Browse the repository at this point in the history
…bility with OpenSearch plugins

Signed-off-by: Manasvini B Suryanarayana <[email protected]>
  • Loading branch information
manasvinibs committed Aug 9, 2023
1 parent 3131ce2 commit d268eff
Show file tree
Hide file tree
Showing 23 changed files with 493 additions and 132 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Reduce the amount of comments in compiled CSS ([#4648](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4648))
- [Saved Object Service] Customize saved objects service status ([#4696](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4696))
- [Discover] Update styles to compatible with OUI `next` theme ([#4644](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4644))
- [Decouple] Allow plugin manifest config to define semver compatible OpenSearch plugin and verify if it is installed on the cluster([#4612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4612))
- [Decouple] Add new version compatibility core service which export functionality for plugins to verify if their OpenSearch plugin counterpart is installed on the cluster or has incompatible version to configure the plugin behavior([#4612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4612))

### 🐛 Bug Fixes

Expand Down
52 changes: 18 additions & 34 deletions src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/core/public/plugins/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function createManifest(
requiredPlugins: required,
optionalPlugins: optional,
requiredBundles: [],
requiredOpenSearchPlugins: [{ id: 'plugin-1', versionRange: 'some-version' }],
} as DiscoveredPlugin;
}

Expand Down
2 changes: 1 addition & 1 deletion src/core/public/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function createManifest(
version: 'some-version',
configPath: ['path'],
requiredPlugins: required,
requiredOpenSearchPlugins: optional,
requiredOpenSearchPlugins: [{ id: 'OS-Plugin', versionRange: '1.0.0' }],
optionalPlugins: optional,
requiredBundles: [],
};
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import { StatusServiceSetup } from './status';
import { Auditor, AuditTrailSetup, AuditTrailStart } from './audit_trail';
import { AppenderConfigType, appendersSchema, LoggingServiceSetup } from './logging';
import { CoreUsageDataStart } from './core_usage_data';
import { VersionCompatibilityServiceStart } from './version_compatibility/types';

// Because of #79265 we need to explicity import, then export these types for
// scripts/telemetry_check.js to work as expected
Expand Down Expand Up @@ -482,6 +483,8 @@ export interface CoreStart {
auditTrail: AuditTrailStart;
/** @internal {@link CoreUsageDataStart} */
coreUsageData: CoreUsageDataStart;
/** {@link VersionCompatibilityServiceStart} */
versionCompatibility: VersionCompatibilityServiceStart;
}

export {
Expand All @@ -493,6 +496,7 @@ export {
PluginsServiceStart,
PluginOpaqueId,
AuditTrailStart,
VersionCompatibilityServiceStart,
};

/**
Expand Down
2 changes: 2 additions & 0 deletions src/core/server/internal_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { InternalStatusServiceSetup } from './status';
import { AuditTrailSetup, AuditTrailStart } from './audit_trail';
import { InternalLoggingServiceSetup } from './logging';
import { CoreUsageDataStart } from './core_usage_data';
import { VersionCompatibilityServiceStart } from './version_compatibility';

/** @internal */
export interface InternalCoreSetup {
Expand Down Expand Up @@ -78,6 +79,7 @@ export interface InternalCoreStart {
uiSettings: InternalUiSettingsServiceStart;
auditTrail: AuditTrailStart;
coreUsageData: CoreUsageDataStart;
versionCompatibility: VersionCompatibilityServiceStart;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/core/server/legacy/legacy_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ export class LegacyService implements CoreService {
throw new Error('core.start.coreUsageData.getCoreUsageData is unsupported in legacy');
},
},
versionCompatibility: startDeps.core.versionCompatibility,
};

const router = setupDeps.core.http.createRouter('', this.legacyId);
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { environmentServiceMock } from './environment/environment_service.mock';
import { statusServiceMock } from './status/status_service.mock';
import { auditTrailServiceMock } from './audit_trail/audit_trail_service.mock';
import { coreUsageDataServiceMock } from './core_usage_data/core_usage_data_service.mock';
import { versionCompatibilityServiceMock } from './version_compatibility/version_compatibility.mock';

export { configServiceMock } from './config/mocks';
export { httpServerMock } from './http/http_server.mocks';
Expand All @@ -69,6 +70,7 @@ export { statusServiceMock } from './status/status_service.mock';
export { contextServiceMock } from './context/context_service.mock';
export { capabilitiesServiceMock } from './capabilities/capabilities_service.mock';
export { coreUsageDataServiceMock } from './core_usage_data/core_usage_data_service.mock';
export { versionCompatibilityServiceMock } from './version_compatibility/version_compatibility.mock';

export function pluginInitializerContextConfigMock<T>(config: T) {
const globalConfig: SharedGlobalConfig = {
Expand Down Expand Up @@ -172,6 +174,7 @@ function createCoreStartMock() {
savedObjects: savedObjectsServiceMock.createStartContract(),
uiSettings: uiSettingsServiceMock.createStartContract(),
coreUsageData: coreUsageDataServiceMock.createStartContract(),
versionCompatibility: versionCompatibilityServiceMock.createStartContract(),
};

return mock;
Expand Down Expand Up @@ -206,6 +209,7 @@ function createInternalCoreStartMock() {
uiSettings: uiSettingsServiceMock.createStartContract(),
auditTrail: auditTrailServiceMock.createStartContract(),
coreUsageData: coreUsageDataServiceMock.createStartContract(),
versionCompatibility: versionCompatibilityServiceMock.createStartContract(),
};
return startDeps;
}
Expand Down
1 change: 1 addition & 0 deletions src/core/server/plugins/plugin_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,5 +270,6 @@ export function createPluginStartContext<TPlugin, TPluginDependencies>(
},
auditTrail: deps.auditTrail,
coreUsageData: deps.coreUsageData,
versionCompatibility: deps.versionCompatibility,
};
}
1 change: 1 addition & 0 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ describe('PluginsService', () => {
requiredPlugins: [],
requiredBundles: [],
optionalPlugins: [],
requiredOpenSearchPlugins: [],
},
];

Expand Down
15 changes: 14 additions & 1 deletion src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ import { CoreContext } from '../core_context';
import { Logger } from '../logging';
import { discover, PluginDiscoveryError, PluginDiscoveryErrorType } from './discovery';
import { PluginWrapper } from './plugin';
import { DiscoveredPlugin, PluginConfigDescriptor, PluginName, InternalPluginInfo } from './types';
import {
DiscoveredPlugin,
PluginConfigDescriptor,
PluginName,
InternalPluginInfo,
CompatibleOpenSearchPluginVersions,
} from './types';
import { PluginsConfig, PluginsConfigType } from './plugins_config';
import { PluginsSystem } from './plugins_system';
import { InternalCoreSetup, InternalCoreStart } from '../internal_types';
Expand Down Expand Up @@ -97,6 +103,10 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
private readonly config$: Observable<PluginsConfig>;
private readonly pluginConfigDescriptors = new Map<PluginName, PluginConfigDescriptor>();
private readonly uiPluginInternalInfo = new Map<PluginName, InternalPluginInfo>();
private readonly openSearchPluginInfo = new Map<
PluginName,
readonly CompatibleOpenSearchPluginVersions[]
>();

constructor(private readonly coreContext: CoreContext) {
this.log = coreContext.logger.get('plugins-service');
Expand Down Expand Up @@ -128,6 +138,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
public: uiPlugins,
browserConfigs: this.generateUiPluginsConfigs(uiPlugins),
},
requiredOpensearchPlugins: this.openSearchPluginInfo,
};
}

Expand Down Expand Up @@ -253,6 +264,8 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
});
}

this.openSearchPluginInfo.set(plugin.name, plugin.requiredOpenSearchPlugins);

pluginEnableStatuses.set(plugin.name, { plugin, isEnabled });
})
)
Expand Down
50 changes: 2 additions & 48 deletions src/core/server/plugins/plugins_system.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ describe('start', () => {
expect(log.info).toHaveBeenCalledWith(`Starting [2] plugins: [order-1,order-0]`);
});

it('validates opensearch plugin installation when dependency is fulfilled', async () => {
it('validates plugin start when opensearch dependency is fulfilled', async () => {
[
createPlugin('order-1', {
requiredOSPlugin: [{ id: 'test-plugin', versionRange: '^2.0.0' }],
Expand All @@ -565,12 +565,9 @@ describe('start', () => {
await pluginsSystem.setupPlugins(setupDeps);
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
expect(pluginsStart).toBeInstanceOf(Map);
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
expect(log.warn).toHaveBeenCalledTimes(0);
});

it('validates opensearch plugin installation and does not error out when plugin is not installed', async () => {
it('validates plugin start when opensearch plugin dependency is not installed', async () => {
[
createPlugin('id-1', {
requiredOSPlugin: [{ id: 'missing-opensearch-dep', versionRange: '^2.0.0' }],
Expand All @@ -585,48 +582,5 @@ describe('start', () => {
await pluginsSystem.setupPlugins(setupDeps);
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
expect(pluginsStart).toBeInstanceOf(Map);
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
expect(log.warn).toHaveBeenCalledTimes(1);
expect(log.warn).toHaveBeenCalledWith(
`OpenSearch plugin "missing-opensearch-dep" is not installed on the cluster for the OpenSearch Dashboards plugin to function as expected.`
);
});

it('validates opensearch plugin installation and log warning when plugin exist but version is incompatible', async () => {
[
createPlugin('id-1', {
requiredOSPlugin: [{ id: 'test-plugin', versionRange: '^1.0.0' }],
}),
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);
});

await pluginsSystem.setupPlugins(setupDeps);
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
expect(pluginsStart).toBeInstanceOf(Map);
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
expect(log.warn).toHaveBeenCalledTimes(1);
expect(log.warn).toHaveBeenCalledWith(
`OpenSearch plugin "test-plugin" is not installed on the cluster for the OpenSearch Dashboards plugin to function as expected.`
);
});

it('validates opensearch plugin installation and does not warn 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);
});
await pluginsSystem.setupPlugins(setupDeps);
const pluginsStart = await pluginsSystem.startPlugins(startDeps);
expect(pluginsStart).toBeInstanceOf(Map);
expect(opensearch.client.asInternalUser.cat.plugins).toHaveBeenCalledTimes(1);
const log = logger.get.mock.results[0].value as jest.Mocked<Logger>;
expect(log.warn).toHaveBeenCalledTimes(0);
});
});
48 changes: 4 additions & 44 deletions src/core/server/plugins/plugins_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,10 @@
*/

import { withTimeout } from '@osd/std';
import semver from 'semver';
import { CatPluginsResponse } from '@opensearch-project/opensearch/api/types';
import { CoreContext } from '../core_context';
import { Logger } from '../logging';
import { PluginWrapper } from './plugin';
import { DiscoveredPlugin, PluginName, CompatibleOpenSearchPluginVersions } from './types';
import { DiscoveredPlugin, PluginName } from './types';
import { createPluginSetupContext, createPluginStartContext } from './plugin_context';
import { PluginsServiceSetupDeps, PluginsServiceStartDeps } from './plugins_service';
import { PluginDependencies } from '.';
Expand Down Expand Up @@ -167,49 +165,10 @@ export class PluginsSystem {
return contracts;
}

private async healthCheckOpenSearchPlugins(deps: PluginsServiceStartDeps) {
// make _cat/plugins?format=json call to the OpenSearch instance
const opensearchInstalledPlugins = await this.getOpenSearchPlugins(deps);
public async healthCheckOpenSearchPlugins(deps: PluginsServiceStartDeps) {
for (const pluginName of this.satupPlugins) {
this.log.debug(`For plugin "${pluginName}"...`);
const plugin = this.plugins.get(pluginName)!;
const pluginOpenSearchDeps = new Set([...plugin.requiredOpenSearchPlugins]);
pluginOpenSearchDeps.forEach(async (depPlugin) => {
// add check to see if the installing Dashboards plugin version is compatible with installed OpenSearch plugin
if (
!(await this.isVersionCompatibleOSPluginInstalled(opensearchInstalledPlugins, depPlugin))
) {
this.log.warn(
`OpenSearch plugin "${depPlugin.id}" is not installed on the cluster for the OpenSearch Dashboards plugin to function as expected.`
);
}
});
}
}

private async isVersionCompatibleOSPluginInstalled(
opensearchInstalledPlugins: CatPluginsResponse,
depPlugin: CompatibleOpenSearchPluginVersions
) {
return opensearchInstalledPlugins.find(
(obj) =>
obj.component === depPlugin.id &&
semver.satisfies(semver.coerce(obj.version)!.version, depPlugin.versionRange)
);
}

private async getOpenSearchPlugins(deps: PluginsServiceStartDeps) {
// Makes cat.plugin api call to fetch list of OpenSearch plugins installed on the cluster
try {
const { body } = await deps.opensearch.client.asInternalUser.cat.plugins<any[]>({
format: 'JSON',
});
return body;
} catch (error) {
this.log.warn(
`Cat API call to OpenSearch to get list of plugins installed on the cluster has failed: ${error}`
);
return [];
await deps.versionCompatibility.verifyOpenSearchPluginsState(pluginName);
}
}

Expand Down Expand Up @@ -251,6 +210,7 @@ export class PluginsSystem {
uiPluginNames.includes(p)
),
requiredBundles: plugin.manifest.requiredBundles,
requiredOpenSearchPlugins: plugin.manifest.requiredOpenSearchPlugins,
},
];
})
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 @@ -255,6 +255,12 @@ export interface DiscoveredPlugin {
*/
readonly optionalPlugins: readonly PluginName[];

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

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

0 comments on commit d268eff

Please sign in to comment.