diff --git a/.eslintrc.js b/.eslintrc.js index a57ea5e03865..2c55dd2528ef 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -470,11 +470,6 @@ module.exports = { errorMessage: 'Server modules cannot be imported into client modules or shared modules.', }, - { - target: ['src/**/*'], - from: ['x-pack/**/*'], - errorMessage: 'OSS cannot import x-pack files.', - }, { target: ['src/core/**/*'], from: ['plugins/**/*', 'src/plugins/**/*'], diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index 610bc1cac5a3..31706e01e4b8 100644 --- a/src/core/server/plugins/plugin.test.ts +++ b/src/core/server/plugins/plugin.test.ts @@ -39,6 +39,11 @@ jest.doMock(join('plugin-with-wrong-initializer-path', 'server'), () => ({ plugi virtual: true, }); +const OSS_PLUGIN_PATH_POSIX = '/kibana/src/plugins/ossPlugin'; +const OSS_PLUGIN_PATH_WINDOWS = 'C:\\kibana\\src\\plugins\\ossPlugin'; +const XPACK_PLUGIN_PATH_POSIX = '/kibana/x-pack/plugins/xPackPlugin'; +const XPACK_PLUGIN_PATH_WINDOWS = 'C:\\kibana\\x-pack\\plugins\\xPackPlugin'; + function createPluginManifest(manifestProps: Partial = {}): PluginManifest { return { id: 'some-plugin-id', @@ -97,10 +102,49 @@ test('`constructor` correctly initializes plugin instance', () => { expect(plugin.name).toBe('some-plugin-id'); expect(plugin.configPath).toBe('path'); expect(plugin.path).toBe('some-plugin-path'); + expect(plugin.source).toBe('external'); // see below for test cases for non-external sources (OSS and X-Pack) expect(plugin.requiredPlugins).toEqual(['some-required-dep']); expect(plugin.optionalPlugins).toEqual(['some-optional-dep']); }); +describe('`constructor` correctly sets non-external source', () => { + function createPlugin(path: string) { + const manifest = createPluginManifest(); + const opaqueId = Symbol(); + return new PluginWrapper({ + path, + manifest, + opaqueId, + initializerContext: createPluginInitializerContext( + coreContext, + opaqueId, + manifest, + instanceInfo + ), + }); + } + + test('for OSS plugin in POSIX', () => { + const plugin = createPlugin(OSS_PLUGIN_PATH_POSIX); + expect(plugin.source).toBe('oss'); + }); + + test('for OSS plugin in Windows', () => { + const plugin = createPlugin(OSS_PLUGIN_PATH_WINDOWS); + expect(plugin.source).toBe('oss'); + }); + + test('for X-Pack plugin in POSIX', () => { + const plugin = createPlugin(XPACK_PLUGIN_PATH_POSIX); + expect(plugin.source).toBe('x-pack'); + }); + + test('for X-Pack plugin in Windows', () => { + const plugin = createPlugin(XPACK_PLUGIN_PATH_WINDOWS); + expect(plugin.source).toBe('x-pack'); + }); +}); + test('`setup` fails if `plugin` initializer is not exported', () => { const manifest = createPluginManifest(); const opaqueId = Symbol(); diff --git a/src/core/server/plugins/plugin.ts b/src/core/server/plugins/plugin.ts index 76ed4f8f24b3..dc861b4489dc 100644 --- a/src/core/server/plugins/plugin.ts +++ b/src/core/server/plugins/plugin.ts @@ -27,6 +27,9 @@ import { } from './types'; import { CorePreboot, CoreSetup, CoreStart } from '..'; +const OSS_PATH_REGEX = /[\/|\\]src[\/|\\]plugins[\/|\\]/; // Matches src/plugins directory on POSIX and Windows +const XPACK_PATH_REGEX = /[\/|\\]x-pack[\/|\\]plugins[\/|\\]/; // Matches x-pack/plugins directory on POSIX and Windows + /** * Lightweight wrapper around discovered plugin that is responsible for instantiating * plugin and dispatching proper context and dependencies into plugin's lifecycle hooks. @@ -40,6 +43,7 @@ export class PluginWrapper< TPluginsStart extends object = object > { public readonly path: string; + public readonly source: 'oss' | 'x-pack' | 'external'; public readonly manifest: PluginManifest; public readonly opaqueId: PluginOpaqueId; public readonly name: PluginManifest['id']; @@ -70,6 +74,7 @@ export class PluginWrapper< } ) { this.path = params.path; + this.source = getPluginSource(params.path); this.manifest = params.manifest; this.opaqueId = params.opaqueId; this.initializerContext = params.initializerContext; @@ -204,3 +209,12 @@ export class PluginWrapper< return this.manifest.type === PluginType.preboot; } } + +function getPluginSource(path: string): 'oss' | 'x-pack' | 'external' { + if (OSS_PATH_REGEX.test(path)) { + return 'oss'; + } else if (XPACK_PATH_REGEX.test(path)) { + return 'x-pack'; + } + return 'external'; +} diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 3cd645cb74ac..a9827dc60fb7 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -52,6 +52,15 @@ expect.addSnapshotSerializer(createAbsolutePathSerializer()); }); }); +const OSS_PLUGIN_PATH = '/kibana/src/plugins/ossPlugin'; +const XPACK_PLUGIN_PATH = '/kibana/x-pack/plugins/xPackPlugin'; +const EXTERNAL_PLUGIN_PATH = '/kibana/plugins/externalPlugin'; +[OSS_PLUGIN_PATH, XPACK_PLUGIN_PATH, EXTERNAL_PLUGIN_PATH].forEach((path) => { + jest.doMock(join(path, 'server'), () => ({}), { + virtual: true, + }); +}); + const createPlugin = ( id: string, { @@ -247,6 +256,75 @@ describe('PluginsService', () => { expect(standardMockPluginSystem.setupPlugins).not.toHaveBeenCalled(); }); + describe('X-Pack dependencies', () => { + function mockDiscoveryResults(params: { sourcePluginPath: string; dependencyType: string }) { + const { sourcePluginPath, dependencyType } = params; + // Each plugin's source is derived from its path; the PluginWrapper test suite contains more detailed test cases around the paths (for both POSIX and Windows) + mockDiscover.mockReturnValue({ + error$: from([]), + plugin$: from([ + createPlugin('sourcePlugin', { + path: sourcePluginPath, + version: 'some-version', + configPath: 'path', + requiredPlugins: dependencyType === 'requiredPlugin' ? ['xPackPlugin'] : [], + requiredBundles: dependencyType === 'requiredBundle' ? ['xPackPlugin'] : undefined, + optionalPlugins: dependencyType === 'optionalPlugin' ? ['xPackPlugin'] : [], + }), + createPlugin('xPackPlugin', { + path: XPACK_PLUGIN_PATH, + version: 'some-version', + configPath: 'path', + requiredPlugins: [], + optionalPlugins: [], + }), + ]), + }); + } + + async function expectError() { + await expect(pluginsService.discover({ environment: environmentPreboot })).rejects.toThrow( + `X-Pack plugin or bundle with id "xPackPlugin" is required by OSS plugin "sourcePlugin", which is prohibited. Consider making this an optional dependency instead.` + ); + expect(standardMockPluginSystem.addPlugin).not.toHaveBeenCalled(); + } + async function expectSuccess() { + await expect(pluginsService.discover({ environment: environmentPreboot })).resolves.toEqual( + expect.anything() + ); + expect(standardMockPluginSystem.addPlugin).toHaveBeenCalled(); + } + + it('throws if an OSS plugin requires an X-Pack plugin or bundle', async () => { + for (const dependencyType of ['requiredPlugin', 'requiredBundle']) { + mockDiscoveryResults({ sourcePluginPath: OSS_PLUGIN_PATH, dependencyType }); + await expectError(); + } + }); + + it('does not throw if an OSS plugin has an optional dependency on an X-Pack plugin', async () => { + mockDiscoveryResults({ + sourcePluginPath: OSS_PLUGIN_PATH, + dependencyType: 'optionalPlugin', + }); + await expectSuccess(); + }); + + it('does not throw if an X-Pack plugin depends on an X-Pack plugin or bundle', async () => { + for (const dependencyType of ['requiredPlugin', 'requiredBundle', 'optionalPlugin']) { + mockDiscoveryResults({ sourcePluginPath: XPACK_PLUGIN_PATH, dependencyType }); + await expectSuccess(); + } + }); + + it('does not throw if an external plugin depends on an X-Pack plugin or bundle', async () => { + for (const dependencyType of ['requiredPlugin', 'requiredBundle', 'optionalPlugin']) { + mockDiscoveryResults({ sourcePluginPath: EXTERNAL_PLUGIN_PATH, dependencyType }); + await expectSuccess(); + } + }); + }); + it('properly detects plugins that should be disabled.', async () => { jest .spyOn(configService, 'isEnabledAtPath') diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 05bb60fb22c6..9def7554ccd0 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -314,27 +314,7 @@ export class PluginsService implements CoreService + ) { + const { name, manifest, requiredBundles, requiredPlugins } = plugin; + + // validate that `requiredBundles` ids point to a discovered plugin which `includesUiPlugin` + for (const requiredBundleId of requiredBundles) { + if (!pluginEnableStatuses.has(requiredBundleId)) { + throw new Error( + `Plugin bundle with id "${requiredBundleId}" is required by plugin "${name}" but it is missing.` + ); + } + + const requiredPlugin = pluginEnableStatuses.get(requiredBundleId)!.plugin; + if (!requiredPlugin.includesUiPlugin) { + throw new Error( + `Plugin bundle with id "${requiredBundleId}" is required by plugin "${name}" but it doesn't have a UI bundle.` + ); + } + + if (requiredPlugin.manifest.type !== plugin.manifest.type) { + throw new Error( + `Plugin bundle with id "${requiredBundleId}" is required by plugin "${name}" and expected to have "${manifest.type}" type, but its type is "${requiredPlugin.manifest.type}".` + ); + } + } + + // validate that OSS plugins do not have required dependencies on X-Pack plugins + if (plugin.source === 'oss') { + for (const id of [...requiredPlugins, ...requiredBundles]) { + const requiredPlugin = pluginEnableStatuses.get(id); + if (requiredPlugin && requiredPlugin.plugin.source === 'x-pack') { + throw new Error( + `X-Pack plugin or bundle with id "${id}" is required by OSS plugin "${name}", which is prohibited. Consider making this an optional dependency instead.` + ); + } + } + } + } + private shouldEnablePlugin( pluginName: PluginName, pluginEnableStatuses: Map,