Allow optional OSS to X-Pack dependencies

.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/**/*'],

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> = {}): 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();

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';
+}

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

@@ -52,6 +52,14 @@ 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) => {
+  jest.doMock(join(path, 'server'), () => ({}), {
+    virtual: true,
+  });
+});
+
 const createPlugin = (
   id: string,
   {
@@ -247,6 +255,80 @@ 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';
+      }) {
+        // 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,
+              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. 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).toHaveBeenCalledTimes(2);
+      }
+
+      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('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();
+      });
+    });
+
     it('properly detects plugins that should be disabled.', async () => {
       jest
         .spyOn(configService, 'isEnabledAtPath')

src/core/server/plugins/plugins_service.ts

@@ -336,6 +336,18 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
         }
       }
 
+      // validate that OSS plugins do not have required dependencies on X-Pack plugins
+      if (plugin.source === 'oss') {
+        for (const id of [...plugin.requiredPlugins, ...plugin.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 "${pluginName}", which is prohibited. Consider making this an optional dependency instead.`
+            );
+          }
+        }
+      }
+
       const pluginEnablement = this.shouldEnablePlugin(pluginName, pluginEnableStatuses);
 
       if (pluginEnablement.enabled) {