From 079211150a1e525c62de673755e995e0ee2ac36c Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Mon, 2 Aug 2021 13:45:37 -0400 Subject: [PATCH 1/5] Validate OSS to X-pack dependencies in plugins service --- .../server/plugins/plugins_service.test.ts | 121 ++++++++++++++++++ src/core/server/plugins/plugins_service.ts | 15 +++ 2 files changed, 136 insertions(+) diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 3cd645cb74ac00..8d98be40778dce 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -52,6 +52,21 @@ expect.addSnapshotSerializer(createAbsolutePathSerializer()); }); }); +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'; +[ + OSS_PLUGIN_PATH_POSIX, + OSS_PLUGIN_PATH_WINDOWS, + XPACK_PLUGIN_PATH_POSIX, + XPACK_PLUGIN_PATH_WINDOWS, +].forEach((path) => { + jest.doMock(join(path, 'server'), () => ({}), { + virtual: true, + }); +}); + const createPlugin = ( id: string, { @@ -247,6 +262,112 @@ describe('PluginsService', () => { expect(standardMockPluginSystem.setupPlugins).not.toHaveBeenCalled(); }); + describe('OSS to X-Pack dependencies', () => { + function mockDiscoveryResults({ + ossPath, + xPackPath, + dependency, + }: { + ossPath: string; + xPackPath: string; + dependency: 'requiredPlugin' | 'requiredBundle' | 'optionalPlugin'; + }) { + mockDiscover.mockReturnValue({ + error$: from([]), + plugin$: from([ + createPlugin('ossPlugin', { + path: ossPath, + version: 'some-version', + configPath: 'path', + requiredPlugins: dependency === 'requiredPlugin' ? ['xPackPlugin'] : [], + requiredBundles: dependency === 'requiredBundle' ? ['xPackPlugin'] : undefined, + optionalPlugins: dependency === 'optionalPlugin' ? ['xPackPlugin'] : [], + }), + createPlugin('xPackPlugin', { + path: xPackPath, + 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 "ossPlugin", which is prohibited.` + ); + expect(standardMockPluginSystem.addPlugin).not.toHaveBeenCalled(); + } + async function expectSuccess() { + await expect(pluginsService.discover({ environment: environmentPreboot })).resolves.toEqual( + expect.anything() + ); + expect(standardMockPluginSystem.addPlugin).toHaveBeenCalledTimes(2); + } + + describe('throws if an OSS plugin requires an X-Pack plugin', () => { + it('in POSIX', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH_POSIX, + xPackPath: XPACK_PLUGIN_PATH_POSIX, + dependency: 'requiredPlugin', + }); + await expectError(); + }); + + it('in Windows', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH_WINDOWS, + xPackPath: XPACK_PLUGIN_PATH_WINDOWS, + dependency: 'requiredPlugin', + }); + await expectError(); + }); + }); + + describe('throws if an OSS plugin requires an X-Pack bundle', () => { + it('in POSIX', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH_POSIX, + xPackPath: XPACK_PLUGIN_PATH_POSIX, + dependency: 'requiredBundle', + }); + await expectError(); + }); + + it('in Windows', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH_WINDOWS, + xPackPath: XPACK_PLUGIN_PATH_WINDOWS, + dependency: 'requiredBundle', + }); + await expectError(); + }); + }); + + describe('does not throw if an OSS plugin has an optional dependency on an X-Pack plugin', () => { + it('in POSIX', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH_POSIX, + xPackPath: XPACK_PLUGIN_PATH_POSIX, + dependency: 'optionalPlugin', + }); + await expectSuccess(); + }); + + it('in Windows', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH_WINDOWS, + xPackPath: XPACK_PLUGIN_PATH_WINDOWS, + dependency: 'optionalPlugin', + }); + 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 05bb60fb22c6db..5ced131cdb6657 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -86,6 +86,9 @@ export interface PluginsServiceDiscoverDeps { environment: InternalEnvironmentServicePreboot; } +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 + /** @internal */ export class PluginsService implements CoreService { private readonly log: Logger; @@ -336,6 +339,18 @@ export class PluginsService implements CoreService Date: Mon, 2 Aug 2021 13:47:15 -0400 Subject: [PATCH 2/5] Remove ESLint rule for OSS to X-Pack restricted paths This is no longer necessary now that we validate OSS to X-Pack plugin dependencies in the plugins service. Since there is no way to differentiate "optional" and "required" dependencies in ESLint rules, this rule needs to be removed to allow such optional dependencies. --- .eslintrc.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 9e0e749e74e511..ee91c775c73c07 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/**/*'], From daebba1abc5c2b9588a3865b82fd6a802f3797fc Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Tue, 3 Aug 2021 10:51:15 -0400 Subject: [PATCH 3/5] PR review feedback * Improve error message * Add source field to PluginWrapper --- src/core/server/plugins/plugin.test.ts | 44 ++++++++++ src/core/server/plugins/plugin.ts | 14 +++ .../server/plugins/plugins_service.test.ts | 85 +++++-------------- src/core/server/plugins/plugins_service.ts | 9 +- 4 files changed, 84 insertions(+), 68 deletions(-) diff --git a/src/core/server/plugins/plugin.test.ts b/src/core/server/plugins/plugin.test.ts index 610bc1cac5a3be..31706e01e4b84a 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 76ed4f8f24b3d4..dc861b4489dcf3 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 8d98be40778dce..6f350799a8ecba 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -52,16 +52,9 @@ expect.addSnapshotSerializer(createAbsolutePathSerializer()); }); }); -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'; -[ - OSS_PLUGIN_PATH_POSIX, - OSS_PLUGIN_PATH_WINDOWS, - XPACK_PLUGIN_PATH_POSIX, - XPACK_PLUGIN_PATH_WINDOWS, -].forEach((path) => { +const OSS_PLUGIN_PATH = '/kibana/src/plugins/ossPlugin'; +const XPACK_PLUGIN_PATH = '/kibana/x-pack/plugins/xPackPlugin'; +[OSS_PLUGIN_PATH, XPACK_PLUGIN_PATH].forEach((path) => { jest.doMock(join(path, 'server'), () => ({}), { virtual: true, }); @@ -272,6 +265,7 @@ describe('PluginsService', () => { xPackPath: string; dependency: 'requiredPlugin' | 'requiredBundle' | 'optionalPlugin'; }) { + // 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([ @@ -296,7 +290,7 @@ describe('PluginsService', () => { async function expectError() { await expect(pluginsService.discover({ environment: environmentPreboot })).rejects.toThrow( - `X-Pack plugin or bundle with id "xPackPlugin" is required by OSS plugin "ossPlugin", which is prohibited.` + `X-Pack plugin or bundle with id "xPackPlugin" is required by OSS plugin "ossPlugin", which is prohibited. Consider making this an optional dependency instead.` ); expect(standardMockPluginSystem.addPlugin).not.toHaveBeenCalled(); } @@ -307,64 +301,31 @@ describe('PluginsService', () => { expect(standardMockPluginSystem.addPlugin).toHaveBeenCalledTimes(2); } - describe('throws if an OSS plugin requires an X-Pack plugin', () => { - it('in POSIX', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH_POSIX, - xPackPath: XPACK_PLUGIN_PATH_POSIX, - dependency: 'requiredPlugin', - }); - await expectError(); - }); - - it('in Windows', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH_WINDOWS, - xPackPath: XPACK_PLUGIN_PATH_WINDOWS, - dependency: 'requiredPlugin', - }); - await expectError(); + it('throws if an OSS plugin requires an X-Pack plugin', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH, + xPackPath: XPACK_PLUGIN_PATH, + dependency: 'requiredPlugin', }); + await expectError(); }); - describe('throws if an OSS plugin requires an X-Pack bundle', () => { - it('in POSIX', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH_POSIX, - xPackPath: XPACK_PLUGIN_PATH_POSIX, - dependency: 'requiredBundle', - }); - await expectError(); - }); - - it('in Windows', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH_WINDOWS, - xPackPath: XPACK_PLUGIN_PATH_WINDOWS, - dependency: 'requiredBundle', - }); - await expectError(); + it('throws if an OSS plugin requires an X-Pack bundle', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH, + xPackPath: XPACK_PLUGIN_PATH, + dependency: 'requiredBundle', }); + await expectError(); }); - describe('does not throw if an OSS plugin has an optional dependency on an X-Pack plugin', () => { - it('in POSIX', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH_POSIX, - xPackPath: XPACK_PLUGIN_PATH_POSIX, - dependency: 'optionalPlugin', - }); - await expectSuccess(); - }); - - it('in Windows', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH_WINDOWS, - xPackPath: XPACK_PLUGIN_PATH_WINDOWS, - dependency: 'optionalPlugin', - }); - await expectSuccess(); + it('does not throw if an OSS plugin has an optional dependency on an X-Pack plugin', async () => { + mockDiscoveryResults({ + ossPath: OSS_PLUGIN_PATH, + xPackPath: XPACK_PLUGIN_PATH, + dependency: 'optionalPlugin', }); + await expectSuccess(); }); }); diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index 5ced131cdb6657..a937bf5ff2c2c2 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -86,9 +86,6 @@ export interface PluginsServiceDiscoverDeps { environment: InternalEnvironmentServicePreboot; } -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 - /** @internal */ export class PluginsService implements CoreService { private readonly log: Logger; @@ -340,12 +337,12 @@ export class PluginsService implements CoreService Date: Wed, 4 Aug 2021 09:39:24 -0400 Subject: [PATCH 4/5] Refactor plugin dependency validation into separate method --- src/core/server/plugins/plugins_service.ts | 76 ++++++++++++---------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts index a937bf5ff2c2c2..9def7554ccd09e 100644 --- a/src/core/server/plugins/plugins_service.ts +++ b/src/core/server/plugins/plugins_service.ts @@ -314,39 +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, From 07bb8ec32ce5cf8b78502c7f74bff57d9b81530d Mon Sep 17 00:00:00 2001 From: Joe Portner <5295965+jportner@users.noreply.github.com> Date: Wed, 4 Aug 2021 09:54:19 -0400 Subject: [PATCH 5/5] Add extra unit test cases for X-Pack and external plugin dependencies --- .../server/plugins/plugins_service.test.ts | 72 +++++++++---------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts index 6f350799a8ecba..a9827dc60fb781 100644 --- a/src/core/server/plugins/plugins_service.test.ts +++ b/src/core/server/plugins/plugins_service.test.ts @@ -54,7 +54,8 @@ expect.addSnapshotSerializer(createAbsolutePathSerializer()); const OSS_PLUGIN_PATH = '/kibana/src/plugins/ossPlugin'; const XPACK_PLUGIN_PATH = '/kibana/x-pack/plugins/xPackPlugin'; -[OSS_PLUGIN_PATH, XPACK_PLUGIN_PATH].forEach((path) => { +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, }); @@ -255,30 +256,23 @@ describe('PluginsService', () => { expect(standardMockPluginSystem.setupPlugins).not.toHaveBeenCalled(); }); - describe('OSS to X-Pack dependencies', () => { - function mockDiscoveryResults({ - ossPath, - xPackPath, - dependency, - }: { - ossPath: string; - xPackPath: string; - dependency: 'requiredPlugin' | 'requiredBundle' | 'optionalPlugin'; - }) { + 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('ossPlugin', { - path: ossPath, + createPlugin('sourcePlugin', { + path: sourcePluginPath, version: 'some-version', configPath: 'path', - requiredPlugins: dependency === 'requiredPlugin' ? ['xPackPlugin'] : [], - requiredBundles: dependency === 'requiredBundle' ? ['xPackPlugin'] : undefined, - optionalPlugins: dependency === 'optionalPlugin' ? ['xPackPlugin'] : [], + requiredPlugins: dependencyType === 'requiredPlugin' ? ['xPackPlugin'] : [], + requiredBundles: dependencyType === 'requiredBundle' ? ['xPackPlugin'] : undefined, + optionalPlugins: dependencyType === 'optionalPlugin' ? ['xPackPlugin'] : [], }), createPlugin('xPackPlugin', { - path: xPackPath, + path: XPACK_PLUGIN_PATH, version: 'some-version', configPath: 'path', requiredPlugins: [], @@ -290,7 +284,7 @@ describe('PluginsService', () => { async function expectError() { await expect(pluginsService.discover({ environment: environmentPreboot })).rejects.toThrow( - `X-Pack plugin or bundle with id "xPackPlugin" is required by OSS plugin "ossPlugin", which is prohibited. Consider making this an optional dependency instead.` + `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(); } @@ -298,35 +292,37 @@ describe('PluginsService', () => { await expect(pluginsService.discover({ environment: environmentPreboot })).resolves.toEqual( expect.anything() ); - expect(standardMockPluginSystem.addPlugin).toHaveBeenCalledTimes(2); + expect(standardMockPluginSystem.addPlugin).toHaveBeenCalled(); } - it('throws if an OSS plugin requires an X-Pack plugin', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH, - xPackPath: XPACK_PLUGIN_PATH, - dependency: 'requiredPlugin', - }); - await expectError(); - }); - - it('throws if an OSS plugin requires an X-Pack bundle', async () => { - mockDiscoveryResults({ - ossPath: OSS_PLUGIN_PATH, - xPackPath: XPACK_PLUGIN_PATH, - dependency: 'requiredBundle', - }); - await expectError(); + 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({ - ossPath: OSS_PLUGIN_PATH, - xPackPath: XPACK_PLUGIN_PATH, - dependency: 'optionalPlugin', + 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 () => {