Skip to content

Commit 9ee9bc7

Browse files
Spencerspalgerkibanamachine
authored
[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>
1 parent dc0bccf commit 9ee9bc7

File tree

4 files changed

+93
-34
lines changed

4 files changed

+93
-34
lines changed

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

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -102,35 +102,42 @@ const createPlugin = (
102102
});
103103
};
104104

105-
describe('PluginsService', () => {
106-
beforeEach(async () => {
107-
mockPackage.raw = {
108-
branch: 'feature-v1',
109-
version: 'v1',
110-
build: {
111-
distributable: true,
112-
number: 100,
113-
sha: 'feature-v1-build-sha',
114-
},
115-
};
105+
async function testSetup(options: { isDevClusterMaster?: boolean } = {}) {
106+
mockPackage.raw = {
107+
branch: 'feature-v1',
108+
version: 'v1',
109+
build: {
110+
distributable: true,
111+
number: 100,
112+
sha: 'feature-v1-build-sha',
113+
},
114+
};
116115

117-
coreId = Symbol('core');
118-
env = Env.createDefault(REPO_ROOT, getEnvOptions());
116+
coreId = Symbol('core');
117+
env = Env.createDefault(REPO_ROOT, {
118+
...getEnvOptions(),
119+
isDevClusterMaster: options.isDevClusterMaster ?? false,
120+
});
119121

120-
config$ = new BehaviorSubject<Record<string, any>>({ plugins: { initialize: true } });
121-
const rawConfigService = rawConfigServiceMock.create({ rawConfig$: config$ });
122-
configService = new ConfigService(rawConfigService, env, logger);
123-
await configService.setSchema(config.path, config.schema);
124-
pluginsService = new PluginsService({ coreId, env, logger, configService });
122+
config$ = new BehaviorSubject<Record<string, any>>({ plugins: { initialize: true } });
123+
const rawConfigService = rawConfigServiceMock.create({ rawConfig$: config$ });
124+
configService = new ConfigService(rawConfigService, env, logger);
125+
await configService.setSchema(config.path, config.schema);
126+
pluginsService = new PluginsService({ coreId, env, logger, configService });
125127

126-
[mockPluginSystem] = MockPluginsSystem.mock.instances as any;
127-
mockPluginSystem.uiPlugins.mockReturnValue(new Map());
128+
[mockPluginSystem] = MockPluginsSystem.mock.instances as any;
129+
mockPluginSystem.uiPlugins.mockReturnValue(new Map());
128130

129-
environmentSetup = environmentServiceMock.createSetupContract();
130-
});
131+
environmentSetup = environmentServiceMock.createSetupContract();
132+
}
131133

132-
afterEach(() => {
133-
jest.clearAllMocks();
134+
afterEach(() => {
135+
jest.clearAllMocks();
136+
});
137+
138+
describe('PluginsService', () => {
139+
beforeEach(async () => {
140+
await testSetup();
134141
});
135142

136143
describe('#discover()', () => {
@@ -613,3 +620,29 @@ describe('PluginsService', () => {
613620
});
614621
});
615622
});
623+
624+
describe('PluginService when isDevClusterMaster is true', () => {
625+
beforeEach(async () => {
626+
await testSetup({
627+
isDevClusterMaster: true,
628+
});
629+
});
630+
631+
describe('#discover()', () => {
632+
it('does not try to run discovery', async () => {
633+
await expect(pluginsService.discover({ environment: environmentSetup })).resolves
634+
.toMatchInlineSnapshot(`
635+
Object {
636+
"pluginTree": undefined,
637+
"uiPlugins": Object {
638+
"browserConfigs": Map {},
639+
"internal": Map {},
640+
"public": Map {},
641+
},
642+
}
643+
`);
644+
645+
expect(mockDiscover).not.toHaveBeenCalled();
646+
});
647+
});
648+
});

src/core/server/plugins/plugins_service.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
import Path from 'path';
21-
import { Observable } from 'rxjs';
21+
import { Observable, EMPTY } from 'rxjs';
2222
import { filter, first, map, mergeMap, tap, toArray } from 'rxjs/operators';
2323
import { pick } from '@kbn/std';
2424

@@ -86,9 +86,11 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
8686
private readonly config$: Observable<PluginsConfig>;
8787
private readonly pluginConfigDescriptors = new Map<PluginName, PluginConfigDescriptor>();
8888
private readonly uiPluginInternalInfo = new Map<PluginName, InternalPluginInfo>();
89+
private readonly discoveryDisabled: boolean;
8990

9091
constructor(private readonly coreContext: CoreContext) {
9192
this.log = coreContext.logger.get('plugins-service');
93+
this.discoveryDisabled = coreContext.env.isDevClusterMaster;
9294
this.pluginsSystem = new PluginsSystem(coreContext);
9395
this.configService = coreContext.configService;
9496
this.config$ = coreContext.configService
@@ -97,13 +99,17 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS
9799
}
98100

99101
public async discover({ environment }: PluginsServiceDiscoverDeps) {
100-
this.log.debug('Discovering plugins');
101-
102102
const config = await this.config$.pipe(first()).toPromise();
103103

104-
const { error$, plugin$ } = discover(config, this.coreContext, {
105-
uuid: environment.instanceUuid,
106-
});
104+
const { error$, plugin$ } = this.discoveryDisabled
105+
? {
106+
error$: EMPTY,
107+
plugin$: EMPTY,
108+
}
109+
: discover(config, this.coreContext, {
110+
uuid: environment.instanceUuid,
111+
});
112+
107113
await this.handleDiscoveryErrors(error$);
108114
await this.handleDiscoveredPlugins(plugin$);
109115

src/core/server/server.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,20 @@ test(`doesn't setup core services if legacy config validation fails`, async () =
215215
expect(mockStatusService.setup).not.toHaveBeenCalled();
216216
expect(mockLoggingService.setup).not.toHaveBeenCalled();
217217
});
218+
219+
test(`doesn't validate config if env.isDevClusterMaster is true`, async () => {
220+
const devParentEnv = Env.createDefault(REPO_ROOT, {
221+
...getEnvOptions(),
222+
isDevClusterMaster: true,
223+
});
224+
225+
const server = new Server(rawConfigService, devParentEnv, logger);
226+
await server.setup();
227+
228+
expect(mockEnsureValidConfiguration).not.toHaveBeenCalled();
229+
expect(mockContextService.setup).toHaveBeenCalled();
230+
expect(mockAuditTrailService.setup).toHaveBeenCalled();
231+
expect(mockHttpService.setup).toHaveBeenCalled();
232+
expect(mockElasticsearchService.setup).toHaveBeenCalled();
233+
expect(mockSavedObjectsService.setup).toHaveBeenCalled();
234+
});

src/core/server/server.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,13 @@ export class Server {
117117
});
118118
const legacyConfigSetup = await this.legacy.setupLegacyConfig();
119119

120-
// Immediately terminate in case of invalid configuration
121-
// This needs to be done after plugin discovery
122-
await this.configService.validate();
123-
await ensureValidConfiguration(this.configService, legacyConfigSetup);
120+
// rely on dev server to validate config, don't validate in the parent process
121+
if (!this.env.isDevClusterMaster) {
122+
// Immediately terminate in case of invalid configuration
123+
// This needs to be done after plugin discovery
124+
await this.configService.validate();
125+
await ensureValidConfiguration(this.configService, legacyConfigSetup);
126+
}
124127

125128
const contextServiceSetup = this.context.setup({
126129
// We inject a fake "legacy plugin" with dependencies on every plugin so that legacy plugins:

0 commit comments

Comments
 (0)