From 9ee9bc7f2000540f266cb45d9c87fc2a3f745899 Mon Sep 17 00:00:00 2001
From: Spencer <email@spalger.com>
Date: Fri, 2 Oct 2020 16:41:05 -0700
Subject: [PATCH] [core/server/plugins] don't run discovery in dev server
 parent process (take 2) (#79358)

Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
---
 .../server/plugins/plugins_service.test.ts    | 85 +++++++++++++------
 src/core/server/plugins/plugins_service.ts    | 18 ++--
 src/core/server/server.test.ts                | 17 ++++
 src/core/server/server.ts                     | 11 ++-
 4 files changed, 95 insertions(+), 36 deletions(-)

diff --git a/src/core/server/plugins/plugins_service.test.ts b/src/core/server/plugins/plugins_service.test.ts
index d36fd2251176..64a382e539fb 100644
--- a/src/core/server/plugins/plugins_service.test.ts
+++ b/src/core/server/plugins/plugins_service.test.ts
@@ -102,35 +102,42 @@ const createPlugin = (
   });
 };
 
-describe('PluginsService', () => {
-  beforeEach(async () => {
-    mockPackage.raw = {
-      branch: 'feature-v1',
-      version: 'v1',
-      build: {
-        distributable: true,
-        number: 100,
-        sha: 'feature-v1-build-sha',
-      },
-    };
+async function testSetup(options: { isDevClusterMaster?: boolean } = {}) {
+  mockPackage.raw = {
+    branch: 'feature-v1',
+    version: 'v1',
+    build: {
+      distributable: true,
+      number: 100,
+      sha: 'feature-v1-build-sha',
+    },
+  };
 
-    coreId = Symbol('core');
-    env = Env.createDefault(REPO_ROOT, getEnvOptions());
-
-    config$ = new BehaviorSubject<Record<string, any>>({ plugins: { initialize: true } });
-    const rawConfigService = rawConfigServiceMock.create({ rawConfig$: config$ });
-    configService = new ConfigService(rawConfigService, env, logger);
-    await configService.setSchema(config.path, config.schema);
-    pluginsService = new PluginsService({ coreId, env, logger, configService });
-
-    [mockPluginSystem] = MockPluginsSystem.mock.instances as any;
-    mockPluginSystem.uiPlugins.mockReturnValue(new Map());
-
-    environmentSetup = environmentServiceMock.createSetupContract();
+  coreId = Symbol('core');
+  env = Env.createDefault(REPO_ROOT, {
+    ...getEnvOptions(),
+    isDevClusterMaster: options.isDevClusterMaster ?? false,
   });
 
-  afterEach(() => {
-    jest.clearAllMocks();
+  config$ = new BehaviorSubject<Record<string, any>>({ plugins: { initialize: true } });
+  const rawConfigService = rawConfigServiceMock.create({ rawConfig$: config$ });
+  configService = new ConfigService(rawConfigService, env, logger);
+  await configService.setSchema(config.path, config.schema);
+  pluginsService = new PluginsService({ coreId, env, logger, configService });
+
+  [mockPluginSystem] = MockPluginsSystem.mock.instances as any;
+  mockPluginSystem.uiPlugins.mockReturnValue(new Map());
+
+  environmentSetup = environmentServiceMock.createSetupContract();
+}
+
+afterEach(() => {
+  jest.clearAllMocks();
+});
+
+describe('PluginsService', () => {
+  beforeEach(async () => {
+    await testSetup();
   });
 
   describe('#discover()', () => {
@@ -613,3 +620,29 @@ describe('PluginsService', () => {
     });
   });
 });
+
+describe('PluginService when isDevClusterMaster is true', () => {
+  beforeEach(async () => {
+    await testSetup({
+      isDevClusterMaster: true,
+    });
+  });
+
+  describe('#discover()', () => {
+    it('does not try to run discovery', async () => {
+      await expect(pluginsService.discover({ environment: environmentSetup })).resolves
+        .toMatchInlineSnapshot(`
+              Object {
+                "pluginTree": undefined,
+                "uiPlugins": Object {
+                  "browserConfigs": Map {},
+                  "internal": Map {},
+                  "public": Map {},
+                },
+              }
+            `);
+
+      expect(mockDiscover).not.toHaveBeenCalled();
+    });
+  });
+});
diff --git a/src/core/server/plugins/plugins_service.ts b/src/core/server/plugins/plugins_service.ts
index e8fe42ee491c..a1062bde7765 100644
--- a/src/core/server/plugins/plugins_service.ts
+++ b/src/core/server/plugins/plugins_service.ts
@@ -18,7 +18,7 @@
  */
 
 import Path from 'path';
-import { Observable } from 'rxjs';
+import { Observable, EMPTY } from 'rxjs';
 import { filter, first, map, mergeMap, tap, toArray } from 'rxjs/operators';
 import { pick } from '@kbn/std';
 
@@ -86,9 +86,11 @@ 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 discoveryDisabled: boolean;
 
   constructor(private readonly coreContext: CoreContext) {
     this.log = coreContext.logger.get('plugins-service');
+    this.discoveryDisabled = coreContext.env.isDevClusterMaster;
     this.pluginsSystem = new PluginsSystem(coreContext);
     this.configService = coreContext.configService;
     this.config$ = coreContext.configService
@@ -97,13 +99,17 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
   }
 
   public async discover({ environment }: PluginsServiceDiscoverDeps) {
-    this.log.debug('Discovering plugins');
-
     const config = await this.config$.pipe(first()).toPromise();
 
-    const { error$, plugin$ } = discover(config, this.coreContext, {
-      uuid: environment.instanceUuid,
-    });
+    const { error$, plugin$ } = this.discoveryDisabled
+      ? {
+          error$: EMPTY,
+          plugin$: EMPTY,
+        }
+      : discover(config, this.coreContext, {
+          uuid: environment.instanceUuid,
+        });
+
     await this.handleDiscoveryErrors(error$);
     await this.handleDiscoveredPlugins(plugin$);
 
diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts
index 3258840d09df..51defb7d0392 100644
--- a/src/core/server/server.test.ts
+++ b/src/core/server/server.test.ts
@@ -215,3 +215,20 @@ test(`doesn't setup core services if legacy config validation fails`, async () =
   expect(mockStatusService.setup).not.toHaveBeenCalled();
   expect(mockLoggingService.setup).not.toHaveBeenCalled();
 });
+
+test(`doesn't validate config if env.isDevClusterMaster is true`, async () => {
+  const devParentEnv = Env.createDefault(REPO_ROOT, {
+    ...getEnvOptions(),
+    isDevClusterMaster: true,
+  });
+
+  const server = new Server(rawConfigService, devParentEnv, logger);
+  await server.setup();
+
+  expect(mockEnsureValidConfiguration).not.toHaveBeenCalled();
+  expect(mockContextService.setup).toHaveBeenCalled();
+  expect(mockAuditTrailService.setup).toHaveBeenCalled();
+  expect(mockHttpService.setup).toHaveBeenCalled();
+  expect(mockElasticsearchService.setup).toHaveBeenCalled();
+  expect(mockSavedObjectsService.setup).toHaveBeenCalled();
+});
diff --git a/src/core/server/server.ts b/src/core/server/server.ts
index ece10db41962..600f45e0b50d 100644
--- a/src/core/server/server.ts
+++ b/src/core/server/server.ts
@@ -117,10 +117,13 @@ export class Server {
     });
     const legacyConfigSetup = await this.legacy.setupLegacyConfig();
 
-    // Immediately terminate in case of invalid configuration
-    // This needs to be done after plugin discovery
-    await this.configService.validate();
-    await ensureValidConfiguration(this.configService, legacyConfigSetup);
+    // rely on dev server to validate config, don't validate in the parent process
+    if (!this.env.isDevClusterMaster) {
+      // Immediately terminate in case of invalid configuration
+      // This needs to be done after plugin discovery
+      await this.configService.validate();
+      await ensureValidConfiguration(this.configService, legacyConfigSetup);
+    }
 
     const contextServiceSetup = this.context.setup({
       // We inject a fake "legacy plugin" with dependencies on every plugin so that legacy plugins: