Skip to content

Commit

Permalink
[core/server/plugins] don't run discovery in dev server parent proces…
Browse files Browse the repository at this point in the history
…s (take 2) (#79358)

Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 2, 2020
1 parent dc0bccf commit 9ee9bc7
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 34 deletions.
81 changes: 57 additions & 24 deletions src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
coreId = Symbol('core');
env = Env.createDefault(REPO_ROOT, {
...getEnvOptions(),
isDevClusterMaster: options.isDevClusterMaster ?? false,
});

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 });
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());
[mockPluginSystem] = MockPluginsSystem.mock.instances as any;
mockPluginSystem.uiPlugins.mockReturnValue(new Map());

environmentSetup = environmentServiceMock.createSetupContract();
});
environmentSetup = environmentServiceMock.createSetupContract();
}

afterEach(() => {
jest.clearAllMocks();
afterEach(() => {
jest.clearAllMocks();
});

describe('PluginsService', () => {
beforeEach(async () => {
await testSetup();
});

describe('#discover()', () => {
Expand Down Expand Up @@ -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();
});
});
});
18 changes: 12 additions & 6 deletions src/core/server/plugins/plugins_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand All @@ -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$);

Expand Down
17 changes: 17 additions & 0 deletions src/core/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
11 changes: 7 additions & 4 deletions src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 9ee9bc7

Please sign in to comment.