Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core/server/plugins] don't run discovery in dev server parent process (take 2) #79358

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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