Skip to content

Commit 726908a

Browse files
[Http] Prevent the internal HttpService being used in tests outside Core (#223238)
## Summary Objective: avoid exposing the entire API surface area of `HttpService` (and it's internal preboot/setup/start contracts) to test code outside core. * Refactor the `createHttpService -> createInternalHttpService` mock test helper moved to scoped folder `./src/core/server/integration_tests/utilities` * Create an "integration test ready" version of the original `createHttpService` for plugins to use from `@kbn/core-http-server-mocks` * Clean up some types * Refactor the 2 plugin test usages of the internal `HttpService` ### Notes * We have been exposing this surface area already in `src/core/packages/http/server-mocks/src/http_service.mock.ts`. But it seems to not have a adoption outside Core code... I wonder if we need a concept of `mock-internal`? * I don't think this is a **massive** issue, I just happened to realise this when [messing around](#222956) with an `HttpService` related refactor. Would be nice to not leak more information about internal code than necessary (it's clearly too late for that, but maybe we can improve the situation for HttpService a bit). --------- Co-authored-by: kibanamachine <[email protected]>
1 parent 970cad1 commit 726908a

File tree

63 files changed

+616
-507
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+616
-507
lines changed

src/core/packages/http/server-mocks/index.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,13 @@ export type {
1818
InternalHttpServiceSetupMock,
1919
InternalHttpServiceStartMock,
2020
} from './src/http_service.mock';
21-
export { createCoreContext, createHttpService, createConfigService } from './src/test_utils';
21+
export {
22+
createCoreContext,
23+
createHttpService,
24+
createConfigService,
25+
type HttpIntegrationTestService,
26+
type HttpIntegrationServicePrebootContractMock,
27+
type HttpIntegrationServiceSetupContractMock,
28+
type HttpIntegrationServiceStartContractMock,
29+
type HttpIntegrationServiceStopContractMock,
30+
} from './src/test_utils';

src/core/packages/http/server-mocks/src/test_utils.ts

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import { ByteSizeValue } from '@kbn/config-schema';
1414
import { Env } from '@kbn/config';
1515
import { getEnvOptions, configServiceMock } from '@kbn/config-mocks';
1616
import type { CoreContext } from '@kbn/core-base-server-internal';
17+
import { contextServiceMock } from '@kbn/core-http-context-server-mocks';
18+
import { executionContextServiceMock } from '@kbn/core-execution-context-server-mocks';
1719
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
20+
import type { IRouter } from '@kbn/core-http-server';
1821
import {
1922
type HttpConfigType,
2023
type ExternalUrlConfigType,
@@ -125,21 +128,58 @@ export const createCoreContext = (overrides: Partial<CoreContext> = {}): CoreCon
125128
});
126129

127130
/**
128-
* Creates a concrete HttpService with a mocked context.
131+
* A mock of the HttpService that can be used in tests.
132+
*
133+
* @remarks intended to mirror the pubilc HTTP contracts where possible to avoid
134+
* drifting or leaking too many internal details of the actual HttpService.
129135
*/
130-
export const createHttpService = ({
131-
buildNum,
132-
...overrides
133-
}: Partial<CoreContext & { buildNum: number }> = {}): HttpService => {
134-
const ctx = createCoreContext(overrides);
135-
if (buildNum !== undefined) {
136-
ctx.env = {
137-
...ctx.env,
138-
packageInfo: {
139-
...ctx.env.packageInfo,
140-
buildNum,
141-
},
142-
};
143-
}
144-
return new HttpService(ctx);
136+
export interface HttpIntegrationTestService {
137+
preboot: () => Promise<void>;
138+
setup: () => Promise<{ createRouter: (path: string) => IRouter<any>; server: { listener: any } }>;
139+
start: () => Promise<void>;
140+
stop: () => Promise<void>;
141+
}
142+
143+
export type HttpIntegrationServicePrebootContractMock = Awaited<
144+
ReturnType<HttpIntegrationTestService['preboot']>
145+
>;
146+
147+
export type HttpIntegrationServiceSetupContractMock = Awaited<
148+
ReturnType<HttpIntegrationTestService['setup']>
149+
>;
150+
151+
export type HttpIntegrationServiceStartContractMock = Awaited<
152+
ReturnType<HttpIntegrationTestService['start']>
153+
>;
154+
155+
export type HttpIntegrationServiceStopContractMock = Awaited<
156+
ReturnType<HttpIntegrationTestService['stop']>
157+
>;
158+
159+
/**
160+
* Creates an HTTP service instance for external services to test against.
161+
* @public
162+
*/
163+
export const createHttpService = (): HttpIntegrationTestService => {
164+
const ctx = createCoreContext();
165+
const svc = new HttpService(ctx);
166+
return {
167+
preboot: async () => {
168+
await svc.preboot({
169+
context: contextServiceMock.createPrebootContract(),
170+
});
171+
},
172+
setup: () => {
173+
return svc.setup({
174+
context: contextServiceMock.createSetupContract(),
175+
executionContext: executionContextServiceMock.createInternalSetupContract(),
176+
});
177+
},
178+
start: async () => {
179+
await svc.start();
180+
},
181+
stop: async () => {
182+
await svc.stop();
183+
},
184+
};
145185
};

src/core/packages/http/server-mocks/tsconfig.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
"@kbn/repo-info",
2323
"@kbn/config",
2424
"@kbn/core-base-server-internal",
25+
"@kbn/core-http-context-server-mocks",
26+
"@kbn/core-execution-context-server-mocks",
2527
],
2628
"exclude": [
2729
"target/**/*",

src/core/packages/test-helpers/test-utils/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
99

10-
export { setupServer } from './src/setup_server';
10+
export { setupServer, type SetupServerReturn } from './src/setup_server';
1111
export { createExportableType } from './src/create_exportable_type';
1212
export { createHiddenTypeVariants } from './src/create_hidden_type_variants';

src/core/packages/test-helpers/test-utils/src/setup_server.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99

1010
import { executionContextServiceMock } from '@kbn/core-execution-context-server-mocks';
1111
import { ContextService } from '@kbn/core-http-context-server-internal';
12-
import { createHttpService, createCoreContext } from '@kbn/core-http-server-mocks';
12+
import { HttpService } from '@kbn/core-http-server-internal';
13+
import { createCoreContext } from '@kbn/core-http-server-mocks';
1314
import { contextServiceMock } from '@kbn/core-http-context-server-mocks';
1415
import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks';
1516
import { typeRegistryMock } from '@kbn/core-saved-objects-base-server-mocks';
@@ -43,8 +44,7 @@ function createCoreServerRequestHandlerContextMock() {
4344
export const setupServer = async (coreId: symbol = defaultCoreId) => {
4445
const coreContext = createCoreContext({ coreId });
4546
const contextService = new ContextService(coreContext);
46-
47-
const server = createHttpService(coreContext);
47+
const server = new HttpService(coreContext);
4848
await server.preboot({ context: contextServiceMock.createPrebootContract() });
4949
const httpSetup = await server.setup({
5050
context: contextService.setup({ pluginDependencies: new Map() }),
@@ -57,8 +57,19 @@ export const setupServer = async (coreId: symbol = defaultCoreId) => {
5757
});
5858

5959
return {
60-
server,
61-
httpSetup,
60+
server: {
61+
listener: httpSetup.server.listener,
62+
start: async () => {
63+
await server.start();
64+
},
65+
stop: async () => {
66+
await server.stop();
67+
},
68+
},
69+
createRouter: httpSetup.createRouter.bind(httpSetup),
70+
registerRouteHandlerContext: httpSetup.registerRouteHandlerContext.bind(httpSetup),
6271
handlerContext,
6372
};
6473
};
74+
75+
export type SetupServerReturn = Awaited<ReturnType<typeof setupServer>>;

src/core/packages/test-helpers/test-utils/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
"@kbn/core-saved-objects-base-server-mocks",
2222
"@kbn/core-saved-objects-server",
2323
"@kbn/core-saved-objects-server-mocks",
24-
"@kbn/core-ui-settings-server-mocks"
24+
"@kbn/core-ui-settings-server-mocks",
25+
"@kbn/core-http-server-internal"
2526
],
2627
"exclude": [
2728
"target/**/*",

src/core/server/integration_tests/capabilities/capabilities_service.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import { getEnvOptions } from '@kbn/config-mocks';
1414
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
1515
import { executionContextServiceMock } from '@kbn/core-execution-context-server-mocks';
1616
import { contextServiceMock } from '@kbn/core-http-context-server-mocks';
17+
import type { CapabilitiesSetup } from '@kbn/core-capabilities-server';
18+
import { CapabilitiesService } from '@kbn/core-capabilities-server-internal';
1719
import {
1820
HttpService,
1921
InternalHttpServicePreboot,
2022
InternalHttpServiceSetup,
2123
} from '@kbn/core-http-server-internal';
22-
import { createHttpService } from '@kbn/core-http-server-mocks';
23-
import type { CapabilitiesSetup } from '@kbn/core-capabilities-server';
24-
import { CapabilitiesService } from '@kbn/core-capabilities-server-internal';
24+
import { createInternalHttpService } from '../utilities';
2525

2626
const coreId = Symbol('core');
2727

@@ -36,7 +36,7 @@ describe('CapabilitiesService', () => {
3636
let serviceSetup: CapabilitiesSetup;
3737

3838
beforeEach(async () => {
39-
server = createHttpService();
39+
server = createInternalHttpService();
4040
httpPreboot = await server.preboot({ context: contextServiceMock.createPrebootContract() });
4141
httpSetup = await server.setup({
4242
context: contextServiceMock.createSetupContract(),

src/core/server/integration_tests/core_app/bundle_routes.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
1414
import { executionContextServiceMock } from '@kbn/core-execution-context-server-mocks';
1515
import { contextServiceMock } from '@kbn/core-http-context-server-mocks';
1616
import type { IRouter } from '@kbn/core-http-server';
17-
import { HttpService } from '@kbn/core-http-server-internal';
18-
import { createHttpService } from '@kbn/core-http-server-mocks';
1917
import { registerRouteForBundle, FileHashCache } from '@kbn/core-apps-server-internal';
18+
import { HttpService } from '@kbn/core-http-server-internal';
19+
import { createInternalHttpService } from '../utilities';
2020

2121
const buildHash = 'buildHash';
2222
const fooPluginFixture = resolve(__dirname, './__fixtures__/plugin/foo');
@@ -32,7 +32,7 @@ describe('bundle routes', () => {
3232
logger = loggingSystemMock.create();
3333
fileHashCache = new FileHashCache();
3434

35-
server = createHttpService({ logger });
35+
server = createInternalHttpService({ logger });
3636
await server.preboot({ context: contextServiceMock.createPrebootContract() });
3737
});
3838

src/core/server/integration_tests/http/lifecycle.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ import { executionContextServiceMock } from '@kbn/core-execution-context-server-
1515
import { contextServiceMock } from '@kbn/core-http-context-server-mocks';
1616
import { ensureRawRequest } from '@kbn/core-http-router-server-internal';
1717
import { HttpService } from '@kbn/core-http-server-internal';
18-
import { createHttpService } from '@kbn/core-http-server-mocks';
1918
import { Env } from '@kbn/config';
2019
import { REPO_ROOT } from '@kbn/repo-info';
2120
import { getEnvOptions } from '@kbn/config-mocks';
21+
import { createInternalHttpService } from '../utilities';
2222

2323
let server: HttpService;
2424

@@ -35,7 +35,7 @@ const kibanaVersion = Env.createDefault(REPO_ROOT, getEnvOptions()).packageInfo.
3535

3636
beforeEach(async () => {
3737
logger = loggingSystemMock.create();
38-
server = createHttpService({ logger });
38+
server = createInternalHttpService({ logger });
3939
await server.preboot({ context: contextServiceMock.createPrebootContract() });
4040
});
4141

src/core/server/integration_tests/http/lifecycle_handlers.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,15 @@ import supertest from 'supertest';
1111
import { kibanaPackageJson } from '@kbn/repo-info';
1212
import type { IRouter, RouteRegistrar } from '@kbn/core-http-server';
1313
import { contextServiceMock } from '@kbn/core-http-context-server-mocks';
14-
import { createConfigService, createHttpService } from '@kbn/core-http-server-mocks';
14+
import { createConfigService } from '@kbn/core-http-server-mocks';
1515
import { HttpService, HttpServerSetup } from '@kbn/core-http-server-internal';
1616
import { executionContextServiceMock } from '@kbn/core-execution-context-server-mocks';
1717
import { schema } from '@kbn/config-schema';
1818
import { IConfigServiceMock } from '@kbn/config-mocks';
1919
import { Logger } from '@kbn/logging';
2020
import { loggerMock } from '@kbn/logging-mocks';
2121
import { KIBANA_BUILD_NR_HEADER } from '@kbn/core-http-common';
22+
import { createInternalHttpService } from '../utilities';
2223

2324
const actualVersion = kibanaPackageJson.version;
2425
const versionHeader = 'kbn-version';
@@ -63,7 +64,7 @@ describe('core lifecycle handlers', () => {
6364
beforeEach(async () => {
6465
const configService = createConfigService(testConfig);
6566
logger = loggerMock.create();
66-
server = createHttpService({ configService, logger });
67+
server = createInternalHttpService({ configService, logger });
6768
await server.preboot({ context: contextServiceMock.createPrebootContract() });
6869
const serverSetup = await server.setup(setupDeps);
6970
router = serverSetup.createRouter('/');
@@ -270,7 +271,7 @@ describe('core lifecycle handlers', () => {
270271
restrictInternalApis: true,
271272
},
272273
});
273-
server = createHttpService({ configService });
274+
server = createInternalHttpService({ configService });
274275
await server.preboot({ context: contextServiceMock.createPrebootContract() });
275276
const serverSetup = await server.setup(setupDeps);
276277
router = serverSetup.createRouter('/');
@@ -348,7 +349,7 @@ describe('core lifecycle handlers with restrict internal routes enforced', () =>
348349
beforeEach(async () => {
349350
logger = loggerMock.create();
350351
const configService = createConfigService({ server: { restrictInternalApis: true } });
351-
server = createHttpService({ configService, logger });
352+
server = createInternalHttpService({ configService, logger });
352353

353354
await server.preboot({ context: contextServiceMock.createPrebootContract() });
354355
const serverSetup = await server.setup(setupDeps);
@@ -427,7 +428,7 @@ describe('core lifecycle handlers with no strict client version check', () => {
427428
},
428429
},
429430
});
430-
server = createHttpService({ configService, logger, buildNum: 1234 });
431+
server = createInternalHttpService({ configService, logger, buildNum: 1234 });
431432
await server.preboot({ context: contextServiceMock.createPrebootContract() });
432433
const serverSetup = await server.setup(setupDeps);
433434
router = serverSetup.createRouter('/');

0 commit comments

Comments
 (0)