From a75d777bea9f29cee3cc87ba966d0dbd2146e6a2 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Wed, 19 Jun 2019 16:32:37 +0200 Subject: [PATCH] New Platform and Legacy platform servers integration (#39047) * New and Legacy platforms share http server instance. Required to use a common security interceptor for incoming http requests * generate docs * remove excessive contract method * add test for New platform compatibility * address comments part #1 * log server running only for http server * fix test. mutate hapi request headers for BWC with legacy * return 503 on start * address @eli comments * address @joshdover comments --- ...gin-server.httpservicestart.islistening.md | 4 +- .../kibana-plugin-server.httpservicestart.md | 2 +- ...na-plugin-server.internalcorestart.http.md | 11 -- .../kibana-plugin-server.internalcorestart.md | 1 - .../functional_tests/lib/run_kibana_server.js | 2 +- .../reload_logging_config.test.js | 2 +- .../__snapshots__/http_server.test.ts.snap | 25 --- .../__snapshots__/http_service.test.ts.snap | 21 +++ src/core/server/http/http_server.test.ts | 10 +- src/core/server/http/http_server.ts | 10 +- src/core/server/http/http_service.mock.ts | 3 +- src/core/server/http/http_service.test.ts | 102 +++++++++++-- src/core/server/http/http_service.ts | 67 ++++++-- .../integration_tests/http_service.test.ts | 32 ++-- src/core/server/index.test.mocks.ts | 12 +- src/core/server/index.ts | 1 - .../__snapshots__/legacy_service.test.ts.snap | 41 ----- .../integration_tests/legacy_service.test.ts | 77 ++++++++++ .../legacy/legacy_platform_proxy.test.ts | 97 ------------ .../server/legacy/legacy_platform_proxy.ts | 105 ------------- src/core/server/legacy/legacy_service.test.ts | 143 +----------------- src/core/server/legacy/legacy_service.ts | 71 --------- src/core/server/server.api.md | 5 +- src/core/server/server.test.ts | 38 ++--- src/core/server/server.ts | 3 +- src/legacy/server/http/index.js | 4 +- src/legacy/server/kbn_server.d.ts | 1 - src/legacy/server/kbn_server.js | 3 +- tasks/config/run.js | 2 +- .../apis/security/basic_login.js | 7 + 30 files changed, 326 insertions(+), 576 deletions(-) delete mode 100644 docs/development/core/server/kibana-plugin-server.internalcorestart.http.md delete mode 100644 src/core/server/http/__snapshots__/http_server.test.ts.snap create mode 100644 src/core/server/legacy/integration_tests/legacy_service.test.ts delete mode 100644 src/core/server/legacy/legacy_platform_proxy.test.ts delete mode 100644 src/core/server/legacy/legacy_platform_proxy.ts diff --git a/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md b/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md index b86fd76180a24f..2818d6ead2c23b 100644 --- a/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md +++ b/docs/development/core/server/kibana-plugin-server.httpservicestart.islistening.md @@ -4,10 +4,10 @@ ## HttpServiceStart.isListening property -Indicates if http server is listening on a port +Indicates if http server is listening on a given port Signature: ```typescript -isListening: () => boolean; +isListening: (port: number) => boolean; ``` diff --git a/docs/development/core/server/kibana-plugin-server.httpservicestart.md b/docs/development/core/server/kibana-plugin-server.httpservicestart.md index dbcbbe787a17a7..2c9c4c8408f6b2 100644 --- a/docs/development/core/server/kibana-plugin-server.httpservicestart.md +++ b/docs/development/core/server/kibana-plugin-server.httpservicestart.md @@ -15,5 +15,5 @@ export interface HttpServiceStart | Property | Type | Description | | --- | --- | --- | -| [isListening](./kibana-plugin-server.httpservicestart.islistening.md) | () => boolean | Indicates if http server is listening on a port | +| [isListening](./kibana-plugin-server.httpservicestart.islistening.md) | (port: number) => boolean | Indicates if http server is listening on a given port | diff --git a/docs/development/core/server/kibana-plugin-server.internalcorestart.http.md b/docs/development/core/server/kibana-plugin-server.internalcorestart.http.md deleted file mode 100644 index 0e2a49ae179680..00000000000000 --- a/docs/development/core/server/kibana-plugin-server.internalcorestart.http.md +++ /dev/null @@ -1,11 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-server](./kibana-plugin-server.md) > [InternalCoreStart](./kibana-plugin-server.internalcorestart.md) > [http](./kibana-plugin-server.internalcorestart.http.md) - -## InternalCoreStart.http property - -Signature: - -```typescript -http: HttpServiceStart; -``` diff --git a/docs/development/core/server/kibana-plugin-server.internalcorestart.md b/docs/development/core/server/kibana-plugin-server.internalcorestart.md index 5f6c6617e641ca..3974ae05828915 100644 --- a/docs/development/core/server/kibana-plugin-server.internalcorestart.md +++ b/docs/development/core/server/kibana-plugin-server.internalcorestart.md @@ -15,6 +15,5 @@ export interface InternalCoreStart | Property | Type | Description | | --- | --- | --- | -| [http](./kibana-plugin-server.internalcorestart.http.md) | HttpServiceStart | | | [plugins](./kibana-plugin-server.internalcorestart.plugins.md) | PluginsServiceStart | | diff --git a/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js b/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js index 26a1509d5c4ecb..a5744d64988014 100644 --- a/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js +++ b/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js @@ -31,7 +31,7 @@ export async function runKibanaServer({ procs, config, options }) { ...process.env, }, cwd: installDir || KIBANA_ROOT, - wait: /Server running/, + wait: /http server running/, }); } diff --git a/src/cli/serve/integration_tests/reload_logging_config.test.js b/src/cli/serve/integration_tests/reload_logging_config.test.js index fbb8fd0431e84f..2b6f229ca9dae4 100644 --- a/src/cli/serve/integration_tests/reload_logging_config.test.js +++ b/src/cli/serve/integration_tests/reload_logging_config.test.js @@ -181,7 +181,7 @@ describe('Server logging configuration', function () { '--logging.json', 'false' ]); - watchFileUntil(logPath, /Server running at/, 2 * minute) + watchFileUntil(logPath, /http server running/, 2 * minute) .then(() => { // once the server is running, archive the log file and issue SIGHUP fs.renameSync(logPath, logPathArchived); diff --git a/src/core/server/http/__snapshots__/http_server.test.ts.snap b/src/core/server/http/__snapshots__/http_server.test.ts.snap deleted file mode 100644 index d3ddb6e0cff696..00000000000000 --- a/src/core/server/http/__snapshots__/http_server.test.ts.snap +++ /dev/null @@ -1,25 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`returns server and connection options on start 1`] = ` -Object { - "host": "127.0.0.1", - "port": 12345, - "routes": Object { - "cors": undefined, - "payload": Object { - "maxBytes": 1024, - }, - "validate": Object { - "failAction": [Function], - "options": Object { - "abortEarly": false, - }, - }, - }, - "state": Object { - "isHttpOnly": true, - "isSameSite": false, - "strictHeader": false, - }, -} -`; diff --git a/src/core/server/http/__snapshots__/http_service.test.ts.snap b/src/core/server/http/__snapshots__/http_service.test.ts.snap index 2ff4fa987c7e4b..04b78a84e818e4 100644 --- a/src/core/server/http/__snapshots__/http_service.test.ts.snap +++ b/src/core/server/http/__snapshots__/http_service.test.ts.snap @@ -7,3 +7,24 @@ Array [ ], ] `; + +exports[`spins up notReady server until started if configured with \`autoListen:true\`: 503 response 1`] = ` +Object { + "body": Array [ + Array [ + "Kibana server is not ready yet", + ], + ], + "code": Array [ + Array [ + 503, + ], + ], + "header": Array [ + Array [ + "Retry-After", + "30", + ], + ], +} +`; diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 00aba1255f04bd..88533504758bec 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -62,6 +62,13 @@ test('listening after started', async () => { await server.start(); expect(server.isListening()).toBe(true); + expect(loggingServiceMock.collect(logger).info).toMatchInlineSnapshot(` +Array [ + Array [ + "http server running", + ], +] +`); }); test('200 OK with body', async () => { @@ -580,11 +587,10 @@ test('returns server and connection options on start', async () => { ...config, port: 12345, }; - const { options, server: innerServer } = await server.setup(configWithPort); + const { server: innerServer } = await server.setup(configWithPort); expect(innerServer).toBeDefined(); expect(innerServer).toBe((server as any).server); - expect(options).toMatchSnapshot(); }); test('registers registerOnPostAuth interceptor several times', async () => { diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 6fed49ca8e635d..eb571bdb47ddd1 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -17,7 +17,7 @@ * under the License. */ -import { Request, Server, ServerOptions } from 'hapi'; +import { Request, Server } from 'hapi'; import { Logger } from '../logging'; import { HttpConfig } from './http_config'; @@ -37,7 +37,6 @@ import { BasePath } from './base_path_service'; export interface HttpServerSetup { server: Server; - options: ServerOptions; registerRouter: (router: Router) => void; /** * To define custom authentication and/or authorization mechanism for incoming requests. @@ -114,7 +113,6 @@ export class HttpServer { this.setupBasePathRewrite(config, basePathService); return { - options: serverOptions, registerRouter: this.registerRouter.bind(this), registerOnPreAuth: this.registerOnPreAuth.bind(this), registerOnPostAuth: this.registerOnPostAuth.bind(this), @@ -156,7 +154,8 @@ export class HttpServer { await this.server.start(); const serverPath = this.config!.rewriteBasePath || this.config!.basePath || ''; - this.log.debug(`http server running at ${this.server.info.uri}${serverPath}`); + this.log.info('http server running'); + this.log.debug(`http server listening on ${this.server.info.uri}${serverPath}`); } public async stop() { @@ -231,6 +230,9 @@ export class HttpServer { authenticate: adoptToHapiAuthFormat(fn, (req, { state, headers }) => { this.authState.set(req, state); this.authHeaders.set(req, headers); + // we mutate headers only for the backward compatibility with the legacy platform. + // where some plugin read directly from headers to identify whether a user is authenticated. + Object.assign(req.headers, headers); }), })); this.server.auth.strategy('session', 'login'); diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index aa65c3cefa4915..2ea0614645c60f 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -17,7 +17,7 @@ * under the License. */ -import { Server, ServerOptions } from 'hapi'; +import { Server } from 'hapi'; import { HttpService } from './http_service'; import { HttpServerSetup } from './http_server'; import { HttpServiceSetup } from './http_service'; @@ -27,7 +27,6 @@ type ServiceSetupMockType = jest.Mocked & { }; const createSetupContractMock = () => { const setupContract: ServiceSetupMockType = { - options: ({} as unknown) as ServerOptions, // we can mock some hapi server method when we need it server: {} as Server, registerOnPreAuth: jest.fn(), diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index 16f946ffcc7ae4..f003ba1314434d 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -23,6 +23,7 @@ import { noop } from 'lodash'; import { BehaviorSubject } from 'rxjs'; import { HttpService, Router } from '.'; import { HttpConfigType, config } from './http_config'; +import { httpServerMock } from './http_server.mocks'; import { Config, ConfigService, Env, ObjectToConfigAdapter } from '../config'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { getEnvOptions } from '../config/__mocks__/env'; @@ -43,6 +44,11 @@ const createConfigService = (value: Partial = {}) => { configService.setSchema(config.path, config.schema); return configService; }; +const fakeHapiServer = { + start: noop, + stop: noop, + route: noop, +}; afterEach(() => { jest.clearAllMocks(); @@ -56,9 +62,9 @@ test('creates and sets up http server', async () => { const httpServer = { isListening: () => false, - setup: jest.fn(), + setup: jest.fn().mockReturnValue({ server: fakeHapiServer }), start: jest.fn(), - stop: noop, + stop: jest.fn(), }; mockHttpServer.mockImplementation(() => httpServer); @@ -69,11 +75,62 @@ test('creates and sets up http server', async () => { expect(httpServer.setup).not.toHaveBeenCalled(); await service.setup(); - expect(httpServer.setup).toHaveBeenCalledTimes(1); + expect(httpServer.setup).toHaveBeenCalled(); expect(httpServer.start).not.toHaveBeenCalled(); await service.start(); - expect(httpServer.start).toHaveBeenCalledTimes(1); + expect(httpServer.start).toHaveBeenCalled(); +}); + +test('spins up notReady server until started if configured with `autoListen:true`', async () => { + const configService = createConfigService(); + const httpServer = { + isListening: () => false, + setup: jest.fn(), + start: jest.fn(), + stop: jest.fn(), + }; + const notReadyHapiServer = { + start: jest.fn(), + stop: jest.fn(), + route: jest.fn(), + }; + + mockHttpServer + .mockImplementationOnce(() => httpServer) + .mockImplementationOnce(() => ({ + setup: () => ({ server: notReadyHapiServer }), + })); + + const service = new HttpService({ + configService, + env: new Env('.', getEnvOptions()), + logger, + }); + + await service.setup(); + + const mockResponse: any = { + code: jest.fn().mockImplementation(() => mockResponse), + header: jest.fn().mockImplementation(() => mockResponse), + }; + const mockResponseToolkit = { + response: jest.fn().mockReturnValue(mockResponse), + }; + + const [[{ handler }]] = notReadyHapiServer.route.mock.calls; + const response503 = await handler(httpServerMock.createRawRequest(), mockResponseToolkit); + expect(response503).toBe(mockResponse); + expect({ + body: mockResponseToolkit.response.mock.calls, + code: mockResponse.code.mock.calls, + header: mockResponse.header.mock.calls, + }).toMatchSnapshot('503 response'); + + await service.start(); + + expect(httpServer.start).toBeCalledTimes(1); + expect(notReadyHapiServer.stop).toBeCalledTimes(1); }); // this is an integration test! @@ -121,7 +178,7 @@ test('logs error if already set up', async () => { const httpServer = { isListening: () => true, - setup: jest.fn(), + setup: jest.fn().mockReturnValue({ server: fakeHapiServer }), start: noop, stop: noop, }; @@ -139,7 +196,7 @@ test('stops http server', async () => { const httpServer = { isListening: () => false, - setup: noop, + setup: jest.fn().mockReturnValue({ server: fakeHapiServer }), start: noop, stop: jest.fn(), }; @@ -157,13 +214,39 @@ test('stops http server', async () => { expect(httpServer.stop).toHaveBeenCalledTimes(1); }); +test('stops not ready server if it is running', async () => { + const configService = createConfigService(); + const mockHapiServer = { + start: jest.fn(), + stop: jest.fn(), + route: jest.fn(), + }; + const httpServer = { + isListening: () => false, + setup: jest.fn().mockReturnValue({ server: mockHapiServer }), + start: noop, + stop: jest.fn(), + }; + mockHttpServer.mockImplementation(() => httpServer); + + const service = new HttpService({ configService, env, logger }); + + await service.setup(); + + await service.stop(); + + expect(mockHapiServer.stop).toHaveBeenCalledTimes(1); +}); + test('register route handler', async () => { const configService = createConfigService(); const registerRouterMock = jest.fn(); const httpServer = { isListening: () => false, - setup: () => ({ registerRouter: registerRouterMock }), + setup: jest + .fn() + .mockReturnValue({ server: fakeHapiServer, registerRouter: registerRouterMock }), start: noop, stop: noop, }; @@ -181,10 +264,7 @@ test('register route handler', async () => { test('returns http server contract on setup', async () => { const configService = createConfigService(); - const httpServer = { - server: {}, - options: { someOption: true }, - }; + const httpServer = { server: fakeHapiServer, options: { someOption: true } }; mockHttpServer.mockImplementation(() => ({ isListening: () => false, diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index fec3774e2f3664..a056300f6ed7ee 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -19,6 +19,7 @@ import { Observable, Subscription } from 'rxjs'; import { first, map } from 'rxjs/operators'; +import { Server } from 'hapi'; import { LoggerFactory } from '../logging'; import { CoreService } from '../../types'; @@ -34,8 +35,8 @@ export interface HttpServiceSetup extends HttpServerSetup { } /** @public */ export interface HttpServiceStart { - /** Indicates if http server is listening on a port */ - isListening: () => boolean; + /** Indicates if http server is listening on a given port */ + isListening: (port: number) => boolean; } /** @internal */ @@ -48,6 +49,7 @@ export class HttpService implements CoreService { + isListening: (port: number = 0) => { const server = this.secondaryServers.get(port); if (server) return server.isListening(); return this.httpServer.isListening(); @@ -110,6 +117,18 @@ export class HttpService implements CoreService) { const { port } = cfg; const config = await this.config$.pipe(first()).toPromise(); @@ -145,9 +164,37 @@ export class HttpService implements CoreService s.stop())); this.secondaryServers.clear(); } + + private async runNotReadyServer(config: HttpConfig) { + this.log.debug('starting NotReady server'); + const httpServer = new HttpServer(this.log); + const { server } = await httpServer.setup(config); + this.notReadyServer = server; + // use hapi server while Kibana ResponseFactory doesn't allow specifying custom headers + // https://github.com/elastic/kibana/issues/33779 + this.notReadyServer.route({ + path: '/{p*}', + method: '*', + handler: (req, responseToolkit) => { + this.log.debug(`Kibana server is not ready yet ${req.method}:${req.url}.`); + + // If server is not ready yet, because plugins or core can perform + // long running tasks (build assets, saved objects migrations etc.) + // we should let client know that and ask to retry after 30 seconds. + return responseToolkit + .response('Kibana server is not ready yet') + .code(503) + .header('Retry-After', '30'); + }, + }); + await this.notReadyServer.start(); + } } diff --git a/src/core/server/http/integration_tests/http_service.test.ts b/src/core/server/http/integration_tests/http_service.test.ts index eff012fc5075d2..d1345b0884438e 100644 --- a/src/core/server/http/integration_tests/http_service.test.ts +++ b/src/core/server/http/integration_tests/http_service.test.ts @@ -83,7 +83,7 @@ describe('http service', () => { .get(root, legacyUrl) .expect(200, 'ok from legacy server'); - expect(response.header['set-cookie']).toBe(undefined); + expect(response.header['set-cookie']).toHaveLength(1); }); it('passes authHeaders as request headers to the legacy platform', async () => { @@ -152,7 +152,7 @@ describe('http service', () => { expect(response.body.state).toEqual(user); expect(response.body.status).toEqual('authenticated'); - expect(response.header['set-cookie']).toBe(undefined); + expect(response.header['set-cookie']).toHaveLength(1); }); it('rewrites authorization header via authHeaders to make a request to Elasticsearch', async () => { @@ -164,7 +164,7 @@ describe('http service', () => { return t.authenticated({ headers: authHeaders }); }, cookieOptions); - const router = new Router(''); + const router = new Router('/new-platform'); router.get({ path: '/', validate: false }, async (req, res) => { const client = await elasticsearch.dataClient$.pipe(first()).toPromise(); client.asScoped(req); @@ -174,7 +174,7 @@ describe('http service', () => { await root.start(); - await kbnTestServer.request.get(root, '/').expect(200); + await kbnTestServer.request.get(root, '/new-platform/').expect(200); expect(clusterClientMock).toBeCalledTimes(1); const [firstCall] = clusterClientMock.mock.calls; const [, , headers] = firstCall; @@ -186,7 +186,7 @@ describe('http service', () => { const { http, elasticsearch } = await root.setup(); const { registerRouter } = http; - const router = new Router(''); + const router = new Router('/new-platform'); router.get({ path: '/', validate: false }, async (req, res) => { const client = await elasticsearch.dataClient$.pipe(first()).toPromise(); client.asScoped(req); @@ -197,7 +197,7 @@ describe('http service', () => { await root.start(); await kbnTestServer.request - .get(root, '/') + .get(root, '/new-platform/') .set('Authorization', authorizationHeader) .expect(200); @@ -218,7 +218,7 @@ describe('http service', () => { afterEach(async () => await root.shutdown()); it('supports passing request through to the route handler', async () => { - const router = new Router(''); + const router = new Router('/new-platform'); router.get({ path: '/', validate: false }, async (req, res) => res.ok({ content: 'ok' })); const { http } = await root.setup(); @@ -230,7 +230,7 @@ describe('http service', () => { http.registerRouter(router); await root.start(); - await kbnTestServer.request.get(root, '/').expect(200, { content: 'ok' }); + await kbnTestServer.request.get(root, '/new-platform/').expect(200, { content: 'ok' }); }); it('supports redirecting to configured url', async () => { @@ -239,7 +239,7 @@ describe('http service', () => { http.registerOnPostAuth(async (req, t) => t.redirected(redirectTo)); await root.start(); - const response = await kbnTestServer.request.get(root, '/').expect(302); + const response = await kbnTestServer.request.get(root, '/new-platform/').expect(302); expect(response.header.location).toBe(redirectTo); }); @@ -251,7 +251,7 @@ describe('http service', () => { await root.start(); await kbnTestServer.request - .get(root, '/') + .get(root, '/new-platform/') .expect(400, { statusCode: 400, error: 'Bad Request', message: 'unexpected error' }); }); @@ -262,7 +262,7 @@ describe('http service', () => { }); await root.start(); - await kbnTestServer.request.get(root, '/').expect({ + await kbnTestServer.request.get(root, '/new-platform/').expect({ statusCode: 500, error: 'Internal Server Error', message: 'An internal server error occurred', @@ -283,7 +283,7 @@ describe('http service', () => { } return t.next(); }); - const router = new Router(''); + const router = new Router('/new-platform'); router.get({ path: '/', validate: false }, async (req, res) => // @ts-ignore. don't complain customField is not defined on Request type res.ok({ customField: String(req.customField) }) @@ -291,7 +291,9 @@ describe('http service', () => { http.registerRouter(router); await root.start(); - await kbnTestServer.request.get(root, '/').expect(200, { customField: 'undefined' }); + await kbnTestServer.request + .get(root, '/new-platform/') + .expect(200, { customField: 'undefined' }); }); }); @@ -305,10 +307,10 @@ describe('http service', () => { it('supports Url change on the flight', async () => { const { http } = await root.setup(); http.registerOnPreAuth((req, t) => { - return t.redirected('/new-url', { forward: true }); + return t.redirected('/new-platform/new-url', { forward: true }); }); - const router = new Router('/'); + const router = new Router('/new-platform'); router.get({ path: '/new-url', validate: false }, async (req, res) => res.ok({ key: 'new-url-reached' }) ); diff --git a/src/core/server/index.test.mocks.ts b/src/core/server/index.test.mocks.ts index 4e61316fcff940..9526a7d79ee43d 100644 --- a/src/core/server/index.test.mocks.ts +++ b/src/core/server/index.test.mocks.ts @@ -18,9 +18,9 @@ */ import { httpServiceMock } from './http/http_service.mock'; -export const httpService = httpServiceMock.create(); +export const mockHttpService = httpServiceMock.create(); jest.doMock('./http/http_service', () => ({ - HttpService: jest.fn(() => httpService), + HttpService: jest.fn(() => mockHttpService), })); import { pluginServiceMock } from './plugins/plugins_service.mock'; @@ -30,9 +30,9 @@ jest.doMock('./plugins/plugins_service', () => ({ })); import { elasticsearchServiceMock } from './elasticsearch/elasticsearch_service.mock'; -export const elasticsearchService = elasticsearchServiceMock.create(); +export const mockElasticsearchService = elasticsearchServiceMock.create(); jest.doMock('./elasticsearch/elasticsearch_service', () => ({ - ElasticsearchService: jest.fn(() => elasticsearchService), + ElasticsearchService: jest.fn(() => mockElasticsearchService), })); export const mockLegacyService = { setup: jest.fn(), start: jest.fn(), stop: jest.fn() }; @@ -41,7 +41,7 @@ jest.mock('./legacy/legacy_service', () => ({ })); import { configServiceMock } from './config/config_service.mock'; -export const configService = configServiceMock.create(); +export const mockConfigService = configServiceMock.create(); jest.doMock('./config/config_service', () => ({ - ConfigService: jest.fn(() => configService), + ConfigService: jest.fn(() => mockConfigService), })); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 5df93863833a9d..51727b6e02cf13 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -141,7 +141,6 @@ export interface InternalCoreSetup { * @public */ export interface InternalCoreStart { - http: HttpServiceStart; plugins: PluginsServiceStart; } diff --git a/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap b/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap index bc7e8f72c4d61b..5319f093a4b8fb 100644 --- a/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap +++ b/src/core/server/legacy/__snapshots__/legacy_service.test.ts.snap @@ -46,27 +46,6 @@ Array [ exports[`once LegacyService is set up with connection info creates legacy kbnServer and closes it if \`listen\` fails. 1`] = `"something failed"`; -exports[`once LegacyService is set up with connection info proxy route responds with \`503\` if \`kbnServer\` is not ready yet.: 503 response 1`] = ` -Object { - "body": Array [ - Array [ - "Kibana server is not ready yet", - ], - ], - "code": Array [ - Array [ - 503, - ], - ], - "header": Array [ - Array [ - "Retry-After", - "30", - ], - ], -} -`; - exports[`once LegacyService is set up with connection info reconfigures logging configuration if new config is received.: applyLoggingConfiguration params 1`] = ` Array [ Array [ @@ -79,26 +58,6 @@ Array [ ] `; -exports[`once LegacyService is set up with connection info register proxy route.: proxy route options 1`] = ` -Array [ - Array [ - Object { - "handler": [Function], - "method": "*", - "options": Object { - "payload": Object { - "maxBytes": 9007199254740991, - "output": "stream", - "parse": false, - "timeout": false, - }, - }, - "path": "/{p*}", - }, - ], -] -`; - exports[`once LegacyService is set up with connection info throws if fails to retrieve initial config. 1`] = `"something failed"`; exports[`once LegacyService is set up without connection info reconfigures logging configuration if new config is received.: applyLoggingConfiguration params 1`] = ` diff --git a/src/core/server/legacy/integration_tests/legacy_service.test.ts b/src/core/server/legacy/integration_tests/legacy_service.test.ts new file mode 100644 index 00000000000000..f4b2d274700870 --- /dev/null +++ b/src/core/server/legacy/integration_tests/legacy_service.test.ts @@ -0,0 +1,77 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Router } from '../../http/'; +import * as kbnTestServer from '../../../../test_utils/kbn_server'; + +describe('legacy service', () => { + describe('http server', () => { + let root: ReturnType; + beforeEach(() => { + root = kbnTestServer.createRoot(); + }, 30000); + + afterEach(async () => await root.shutdown()); + + it("handles http request in Legacy platform if New platform doesn't handle it", async () => { + const rootUrl = '/route'; + const router = new Router(rootUrl); + router.get({ path: '/new-platform', validate: false }, (req, res) => + res.ok({ content: 'from-new-platform' }) + ); + + const { http } = await root.setup(); + http.registerRouter(router); + await root.start(); + + const legacyPlatformUrl = `${rootUrl}/legacy-platform`; + const kbnServer = kbnTestServer.getKbnServer(root); + kbnServer.server.route({ + method: 'GET', + path: legacyPlatformUrl, + handler: () => 'ok from legacy server', + }); + + await kbnTestServer.request + .get(root, '/route/new-platform') + .expect(200, { content: 'from-new-platform' }); + + await kbnTestServer.request.get(root, legacyPlatformUrl).expect(200, 'ok from legacy server'); + }); + it('throws error if Legacy and New platforms register handler for the same route', async () => { + const rootUrl = '/route'; + const router = new Router(rootUrl); + router.get({ path: '', validate: false }, (req, res) => + res.ok({ content: 'from-new-platform' }) + ); + + const { http } = await root.setup(); + http.registerRouter(router); + await root.start(); + + const kbnServer = kbnTestServer.getKbnServer(root); + expect(() => + kbnServer.server.route({ + method: 'GET', + path: rootUrl, + handler: () => 'ok from legacy server', + }) + ).toThrowErrorMatchingInlineSnapshot(`"New route /route conflicts with existing /route"`); + }); + }); +}); diff --git a/src/core/server/legacy/legacy_platform_proxy.test.ts b/src/core/server/legacy/legacy_platform_proxy.test.ts deleted file mode 100644 index 29c91bd0b61f93..00000000000000 --- a/src/core/server/legacy/legacy_platform_proxy.test.ts +++ /dev/null @@ -1,97 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { Server } from 'net'; - -import { LegacyPlatformProxy } from './legacy_platform_proxy'; - -let server: jest.Mocked; -let proxy: LegacyPlatformProxy; -beforeEach(() => { - server = { - addListener: jest.fn(), - address: jest - .fn() - .mockReturnValue({ port: 1234, family: 'test-family', address: 'test-address' }), - getConnections: jest.fn(), - } as any; - proxy = new LegacyPlatformProxy({ debug: jest.fn() } as any, server); -}); - -test('correctly redirects server events.', () => { - for (const eventName of ['clientError', 'close', 'connection', 'error', 'listening', 'upgrade']) { - expect(server.addListener).toHaveBeenCalledWith(eventName, expect.any(Function)); - - const listener = jest.fn(); - proxy.addListener(eventName, listener); - - // Emit several events, to make sure that server is not being listened with `once`. - const [, serverListener] = server.addListener.mock.calls.find( - ([serverEventName]) => serverEventName === eventName - )!; - - (serverListener as jest.Mock)(1, 2, 3, 4); - (serverListener as jest.Mock)(5, 6, 7, 8); - - expect(listener).toHaveBeenCalledTimes(2); - expect(listener).toHaveBeenCalledWith(1, 2, 3, 4); - - proxy.removeListener(eventName, listener); - } -}); - -test('returns `address` from the underlying server.', () => { - expect(proxy.address()).toEqual({ - address: 'test-address', - family: 'test-family', - port: 1234, - }); -}); - -test('`listen` calls callback immediately.', async () => { - const onListenComplete = jest.fn(); - - await proxy.listen(1234, 'host-1', onListenComplete); - - expect(onListenComplete).toHaveBeenCalledTimes(1); -}); - -test('`close` calls callback immediately.', async () => { - const onCloseComplete = jest.fn(); - - await proxy.close(onCloseComplete); - - expect(onCloseComplete).toHaveBeenCalledTimes(1); -}); - -test('returns connection count from the underlying server.', () => { - server.getConnections.mockImplementation(callback => callback(null, 0)); - const onGetConnectionsComplete = jest.fn(); - proxy.getConnections(onGetConnectionsComplete); - - expect(onGetConnectionsComplete).toHaveBeenCalledTimes(1); - expect(onGetConnectionsComplete).toHaveBeenCalledWith(null, 0); - onGetConnectionsComplete.mockReset(); - - server.getConnections.mockImplementation(callback => callback(null, 100500)); - proxy.getConnections(onGetConnectionsComplete); - - expect(onGetConnectionsComplete).toHaveBeenCalledTimes(1); - expect(onGetConnectionsComplete).toHaveBeenCalledWith(null, 100500); -}); diff --git a/src/core/server/legacy/legacy_platform_proxy.ts b/src/core/server/legacy/legacy_platform_proxy.ts deleted file mode 100644 index a78787d87c0550..00000000000000 --- a/src/core/server/legacy/legacy_platform_proxy.ts +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { EventEmitter } from 'events'; -import { Server } from 'net'; - -import { Logger } from '../logging'; - -/** - * List of the server events to be forwarded to the legacy platform. - */ -const ServerEventsToForward = [ - 'clientError', - 'close', - 'connection', - 'error', - 'listening', - 'upgrade', -]; - -/** - * Represents "proxy" between legacy and current platform. - * @internal - */ -export class LegacyPlatformProxy extends EventEmitter { - private readonly eventHandlers: Map void>; - - constructor(private readonly log: Logger, private readonly server: Server) { - super(); - - // HapiJS expects that the following events will be generated by `listener`, see: - // https://github.com/hapijs/hapi/blob/v14.2.0/lib/connection.js. - this.eventHandlers = new Map( - ServerEventsToForward.map(eventName => { - return [ - eventName, - (...args: any[]) => { - this.log.debug(`Event is being forwarded: ${eventName}`); - this.emit(eventName, ...args); - }, - ] as [string, (...args: any[]) => void]; - }) - ); - - for (const [eventName, eventHandler] of this.eventHandlers) { - this.server.addListener(eventName, eventHandler); - } - } - - /** - * Neither new nor legacy platform should use this method directly. - */ - public address() { - this.log.debug('"address" has been called.'); - - return this.server.address(); - } - - /** - * Neither new nor legacy platform should use this method directly. - */ - public listen(port: number, host: string, callback?: (error?: Error) => void) { - this.log.debug(`"listen" has been called (${host}:${port}).`); - - if (callback !== undefined) { - callback(); - } - } - - /** - * Neither new nor legacy platform should use this method directly. - */ - public close(callback?: (error?: Error) => void) { - this.log.debug('"close" has been called.'); - - if (callback !== undefined) { - callback(); - } - } - - /** - * Neither new nor legacy platform should use this method directly. - */ - public getConnections(callback: (error: Error | null, count?: number) => void) { - // This method is used by `even-better` (before we start platform). - // It seems that the latest version of parent `good` doesn't use this anymore. - this.server.getConnections(callback); - } -} diff --git a/src/core/server/legacy/legacy_service.test.ts b/src/core/server/legacy/legacy_service.test.ts index 386532ba6565b8..fa4d60520818e9 100644 --- a/src/core/server/legacy/legacy_service.test.ts +++ b/src/core/server/legacy/legacy_service.test.ts @@ -17,13 +17,11 @@ * under the License. */ -import { BehaviorSubject, Subject, throwError } from 'rxjs'; +import { BehaviorSubject, throwError } from 'rxjs'; -jest.mock('./legacy_platform_proxy'); jest.mock('../../../legacy/server/kbn_server'); jest.mock('../../../cli/cluster/cluster_manager'); -import { first } from 'rxjs/operators'; import { LegacyService } from '.'; // @ts-ignore: implicit any for JS file import MockClusterManager from '../../../cli/cluster/cluster_manager'; @@ -36,10 +34,8 @@ import { HttpServiceStart, BasePathProxyServer } from '../http'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { DiscoveredPlugin, DiscoveredPluginInternal } from '../plugins'; import { PluginsServiceSetup, PluginsServiceStart } from '../plugins/plugins_service'; -import { LegacyPlatformProxy } from './legacy_platform_proxy'; const MockKbnServer: jest.Mock = KbnServer as any; -const MockLegacyPlatformProxy: jest.Mock = LegacyPlatformProxy as any; let env: Env; let config$: BehaviorSubject; @@ -73,8 +69,6 @@ beforeEach(() => { core: { elasticsearch: { legacy: {} } as any, http: { - options: { someOption: 'foo', someAnotherOption: 'bar' }, - server: { listener: { addListener: jest.fn() }, route: jest.fn() }, auth: { getAuthHeaders: () => undefined, }, @@ -116,72 +110,6 @@ afterEach(() => { }); describe('once LegacyService is set up with connection info', () => { - test('register proxy route.', async () => { - const legacyService = new LegacyService({ env, logger, configService: configService as any }); - await legacyService.setup(setupDeps); - await legacyService.start(startDeps); - - expect(setupDeps.core.http.server.route.mock.calls).toMatchSnapshot('proxy route options'); - }); - - test('proxy route responds with `503` if `kbnServer` is not ready yet.', async () => { - configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); - const legacyService = new LegacyService({ env, logger, configService: configService as any }); - - const kbnServerListen$ = new Subject(); - MockKbnServer.prototype.listen = jest.fn(() => { - kbnServerListen$.next(); - return kbnServerListen$.toPromise(); - }); - - // Wait until listen is called and proxy route is registered, but don't allow - // listen to complete and make kbnServer available. - await legacyService.setup(setupDeps); - const legacySetupPromise = legacyService.start(startDeps); - await kbnServerListen$.pipe(first()).toPromise(); - - const mockResponse: any = { - code: jest.fn().mockImplementation(() => mockResponse), - header: jest.fn().mockImplementation(() => mockResponse), - }; - const mockResponseToolkit = { - response: jest.fn().mockReturnValue(mockResponse), - abandon: Symbol('abandon'), - }; - const mockRequest = { raw: { req: { a: 1 }, res: { b: 2 } } }; - - const [[{ handler }]] = setupDeps.core.http.server.route.mock.calls; - const response503 = await handler(mockRequest, mockResponseToolkit); - - expect(response503).toBe(mockResponse); - expect({ - body: mockResponseToolkit.response.mock.calls, - code: mockResponse.code.mock.calls, - header: mockResponse.header.mock.calls, - }).toMatchSnapshot('503 response'); - - // Make sure request hasn't been passed to the legacy platform. - const [mockedLegacyPlatformProxy] = MockLegacyPlatformProxy.mock.instances; - expect(mockedLegacyPlatformProxy.emit).not.toHaveBeenCalled(); - - // Now wait until kibana is ready and try to request once again. - kbnServerListen$.complete(); - await legacySetupPromise; - mockResponseToolkit.response.mockClear(); - - const responseProxy = await handler(mockRequest, mockResponseToolkit); - expect(responseProxy).toBe(mockResponseToolkit.abandon); - expect(mockResponseToolkit.response).not.toHaveBeenCalled(); - - // Make sure request has been passed to the legacy platform. - expect(mockedLegacyPlatformProxy.emit).toHaveBeenCalledTimes(1); - expect(mockedLegacyPlatformProxy.emit).toHaveBeenCalledWith( - 'request', - mockRequest.raw.req, - mockRequest.raw.res - ); - }); - test('creates legacy kbnServer and calls `listen`.', async () => { configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); const legacyService = new LegacyService({ env, logger, configService: configService as any }); @@ -195,11 +123,6 @@ describe('once LegacyService is set up with connection info', () => { { setupDeps, startDeps, - serverOptions: { - listener: expect.any(LegacyPlatformProxy), - someAnotherOption: 'bar', - someOption: 'foo', - }, handledConfigPaths: ['foo.bar'], logger, } @@ -223,11 +146,6 @@ describe('once LegacyService is set up with connection info', () => { { setupDeps, startDeps, - serverOptions: { - listener: expect.any(LegacyPlatformProxy), - someAnotherOption: 'bar', - someOption: 'foo', - }, handledConfigPaths: ['foo.bar'], logger, } @@ -312,59 +230,24 @@ describe('once LegacyService is set up with connection info', () => { expect(mockKbnServer.applyLoggingConfiguration).not.toHaveBeenCalled(); expect(loggingServiceMock.collect(logger).error).toEqual([[configError]]); }); - - test('proxy route abandons request processing and forwards it to the legacy Kibana', async () => { - const legacyService = new LegacyService({ env, logger, configService: configService as any }); - const mockResponseToolkit = { response: jest.fn(), abandon: Symbol('abandon') }; - const mockRequest = { raw: { req: { a: 1 }, res: { b: 2 } } }; - - await legacyService.setup(setupDeps); - await legacyService.start(startDeps); - - const [[{ handler }]] = setupDeps.core.http.server.route.mock.calls; - const response = await handler(mockRequest, mockResponseToolkit); - - expect(response).toBe(mockResponseToolkit.abandon); - expect(mockResponseToolkit.response).not.toHaveBeenCalled(); - - // Make sure request has been passed to the legacy platform. - const [mockedLegacyPlatformProxy] = MockLegacyPlatformProxy.mock.instances; - expect(mockedLegacyPlatformProxy.emit).toHaveBeenCalledTimes(1); - expect(mockedLegacyPlatformProxy.emit).toHaveBeenCalledWith( - 'request', - mockRequest.raw.req, - mockRequest.raw.res - ); - }); }); describe('once LegacyService is set up without connection info', () => { - const disabledHttpStartDeps = { - core: { - http: { - isListening: () => false, - }, - plugins: { contracts: new Map() }, - }, - plugins: {}, - }; let legacyService: LegacyService; beforeEach(async () => { legacyService = new LegacyService({ env, logger, configService: configService as any }); await legacyService.setup(setupDeps); - await legacyService.start(disabledHttpStartDeps); + await legacyService.start(startDeps); }); test('creates legacy kbnServer with `autoListen: false`.', () => { - expect(setupDeps.core.http.server.route).not.toHaveBeenCalled(); expect(MockKbnServer).toHaveBeenCalledTimes(1); expect(MockKbnServer).toHaveBeenCalledWith( { server: { autoListen: true } }, { setupDeps, - startDeps: disabledHttpStartDeps, - serverOptions: { autoListen: false }, + startDeps, handledConfigPaths: ['foo.bar'], logger, } @@ -405,15 +288,7 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { }); await devClusterLegacyService.setup(setupDeps); - await devClusterLegacyService.start({ - core: { - http: { - isListening: () => false, - }, - plugins: { contracts: new Map() }, - }, - plugins: {}, - }); + await devClusterLegacyService.start(startDeps); expect(MockClusterManager.create.mock.calls).toMatchSnapshot( 'cluster manager without base path proxy' @@ -433,15 +308,7 @@ describe('once LegacyService is set up in `devClusterMaster` mode', () => { }); await devClusterLegacyService.setup(setupDeps); - await devClusterLegacyService.start({ - core: { - http: { - isListening: () => false, - }, - plugins: { contracts: new Map() }, - }, - plugins: {}, - }); + await devClusterLegacyService.start(startDeps); expect(MockClusterManager.create).toBeCalledTimes(1); diff --git a/src/core/server/legacy/legacy_service.ts b/src/core/server/legacy/legacy_service.ts index 22e092f95df8c0..a50f049db22310 100644 --- a/src/core/server/legacy/legacy_service.ts +++ b/src/core/server/legacy/legacy_service.ts @@ -17,7 +17,6 @@ * under the License. */ -import { Server as HapiServer } from 'hapi'; import { combineLatest, ConnectableObservable, EMPTY, Observable, Subscription } from 'rxjs'; import { first, map, mergeMap, publishReplay, tap } from 'rxjs/operators'; import { CoreService } from '../../types'; @@ -27,7 +26,6 @@ import { CoreContext } from '../core_context'; import { DevConfig, DevConfigType } from '../dev'; import { BasePathProxyServer, HttpConfig, HttpConfigType } from '../http'; import { Logger } from '../logging'; -import { LegacyPlatformProxy } from './legacy_platform_proxy'; interface LegacyKbnServer { applyLoggingConfiguration: (settings: Readonly>) => void; @@ -149,17 +147,6 @@ export class LegacyService implements CoreService { // eslint-disable-next-line @typescript-eslint/no-var-requires const KbnServer = require('../../../legacy/server/kbn_server'); const kbnServer: LegacyKbnServer = new KbnServer(getLegacyRawConfig(config), { - // If core HTTP service is run we'll receive internal server reference and - // options that were used to create that server so that we can properly - // bridge with the "legacy" Kibana. If server isn't run (e.g. if process is - // managed by ClusterManager or optimizer) then we won't have that info, - // so we can't start "legacy" server either. - serverOptions: startDeps.core.http.isListening() - ? { - ...setupDeps.core.http.options, - listener: this.setupProxyListener(setupDeps.core.http.server), - } - : { autoListen: false }, handledConfigPaths: await this.coreContext.configService.getUsedPaths(), setupDeps, startDeps, @@ -188,62 +175,4 @@ export class LegacyService implements CoreService { return kbnServer; } - - private setupProxyListener(server: HapiServer) { - const { setupDeps } = this; - if (!setupDeps) { - throw new Error('Legacy service is not setup yet.'); - } - - const legacyProxy = new LegacyPlatformProxy( - this.coreContext.logger.get('legacy-proxy'), - server.listener - ); - - // We register Kibana proxy middleware right before we start server to allow - // all new platform plugins register their routes, so that `legacyProxy` - // handles only requests that aren't handled by the new platform. - server.route({ - path: '/{p*}', - method: '*', - options: { - payload: { - output: 'stream', - parse: false, - timeout: false, - // Having such a large value here will allow legacy routes to override - // maximum allowed payload size set in the core http server if needed. - maxBytes: Number.MAX_SAFE_INTEGER, - }, - }, - handler: async (request, responseToolkit) => { - const { req, res } = request.raw; - const authHeaders = setupDeps.core.http.auth.getAuthHeaders(request); - if (authHeaders) { - // some plugins in Legacy relay on headers.authorization presence - req.headers = Object.assign(req.headers, authHeaders); - } - if (this.kbnServer === undefined) { - this.log.debug(`Kibana server is not ready yet ${req.method}:${req.url}.`); - - // If legacy server is not ready yet (e.g. it's still in optimization phase), - // we should let client know that and ask to retry after 30 seconds. - return responseToolkit - .response('Kibana server is not ready yet') - .code(503) - .header('Retry-After', '30'); - } - - this.log.trace(`Request will be handled by proxy ${req.method}:${req.url}.`); - - // Forward request and response objects to the legacy platform. This method - // is used whenever new platform doesn't know how to handle the request. - legacyProxy.emit('request', req, res); - - return responseToolkit.abandon; - }, - }); - - return legacyProxy; - } } diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 67f90e782a73d8..14dea167d3189f 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -16,7 +16,6 @@ import { ResponseObject } from 'hapi'; import { ResponseToolkit } from 'hapi'; import { Schema } from '@kbn/config-schema'; import { Server } from 'hapi'; -import { ServerOptions } from 'hapi'; import { Type } from '@kbn/config-schema'; import { TypeOf } from '@kbn/config-schema'; import { Url } from 'url'; @@ -160,7 +159,7 @@ export interface HttpServiceSetup extends HttpServerSetup { // @public (undocumented) export interface HttpServiceStart { - isListening: () => boolean; + isListening: (port: number) => boolean; } // @internal (undocumented) @@ -175,8 +174,6 @@ export interface InternalCoreSetup { // @public (undocumented) export interface InternalCoreStart { - // (undocumented) - http: HttpServiceStart; // (undocumented) plugins: PluginsServiceStart; } diff --git a/src/core/server/server.test.ts b/src/core/server/server.test.ts index 257b9e72fd081a..694888ab6243eb 100644 --- a/src/core/server/server.test.ts +++ b/src/core/server/server.test.ts @@ -18,11 +18,11 @@ */ import { - elasticsearchService, - httpService, + mockElasticsearchService, + mockHttpService, mockLegacyService, mockPluginsService, - configService, + mockConfigService, } from './index.test.mocks'; import { BehaviorSubject } from 'rxjs'; @@ -36,7 +36,7 @@ const env = new Env('.', getEnvOptions()); const logger = loggingServiceMock.create(); beforeEach(() => { - configService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); + mockConfigService.atPath.mockReturnValue(new BehaviorSubject({ autoListen: true })); }); afterEach(() => { @@ -47,15 +47,15 @@ const config$ = new BehaviorSubject(new ObjectToConfigAdapter({})); test('sets up services on "setup"', async () => { const server = new Server(config$, env, logger); - expect(httpService.setup).not.toHaveBeenCalled(); - expect(elasticsearchService.setup).not.toHaveBeenCalled(); + expect(mockHttpService.setup).not.toHaveBeenCalled(); + expect(mockElasticsearchService.setup).not.toHaveBeenCalled(); expect(mockPluginsService.setup).not.toHaveBeenCalled(); expect(mockLegacyService.setup).not.toHaveBeenCalled(); await server.setup(); - expect(httpService.setup).toHaveBeenCalledTimes(1); - expect(elasticsearchService.setup).toHaveBeenCalledTimes(1); + expect(mockHttpService.setup).toHaveBeenCalledTimes(1); + expect(mockElasticsearchService.setup).toHaveBeenCalledTimes(1); expect(mockPluginsService.setup).toHaveBeenCalledTimes(1); expect(mockLegacyService.setup).toHaveBeenCalledTimes(1); }); @@ -63,21 +63,21 @@ test('sets up services on "setup"', async () => { test('runs services on "start"', async () => { const server = new Server(config$, env, logger); - expect(httpService.setup).not.toHaveBeenCalled(); + expect(mockHttpService.setup).not.toHaveBeenCalled(); expect(mockLegacyService.start).not.toHaveBeenCalled(); await server.setup(); - expect(httpService.start).not.toHaveBeenCalled(); + expect(mockHttpService.start).not.toHaveBeenCalled(); expect(mockLegacyService.start).not.toHaveBeenCalled(); await server.start(); - expect(httpService.start).toHaveBeenCalledTimes(1); + expect(mockHttpService.start).toHaveBeenCalledTimes(1); expect(mockLegacyService.start).toHaveBeenCalledTimes(1); }); test('does not fail on "setup" if there are unused paths detected', async () => { - configService.getUnusedPaths.mockResolvedValue(['some.path', 'another.path']); + mockConfigService.getUnusedPaths.mockResolvedValue(['some.path', 'another.path']); const server = new Server(config$, env, logger); @@ -89,29 +89,29 @@ test('stops services on "stop"', async () => { await server.setup(); - expect(httpService.stop).not.toHaveBeenCalled(); - expect(elasticsearchService.stop).not.toHaveBeenCalled(); + expect(mockHttpService.stop).not.toHaveBeenCalled(); + expect(mockElasticsearchService.stop).not.toHaveBeenCalled(); expect(mockPluginsService.stop).not.toHaveBeenCalled(); expect(mockLegacyService.stop).not.toHaveBeenCalled(); await server.stop(); - expect(httpService.stop).toHaveBeenCalledTimes(1); - expect(elasticsearchService.stop).toHaveBeenCalledTimes(1); + expect(mockHttpService.stop).toHaveBeenCalledTimes(1); + expect(mockElasticsearchService.stop).toHaveBeenCalledTimes(1); expect(mockPluginsService.stop).toHaveBeenCalledTimes(1); expect(mockLegacyService.stop).toHaveBeenCalledTimes(1); }); test(`doesn't setup core services if config validation fails`, async () => { - configService.setSchema.mockImplementation(() => { + mockConfigService.setSchema.mockImplementation(() => { throw new Error('invalid config'); }); const server = new Server(config$, env, logger); await expect(server.setupConfigSchemas()).rejects.toThrowErrorMatchingInlineSnapshot( `"invalid config"` ); - expect(httpService.setup).not.toHaveBeenCalled(); - expect(elasticsearchService.setup).not.toHaveBeenCalled(); + expect(mockHttpService.setup).not.toHaveBeenCalled(); + expect(mockElasticsearchService.setup).not.toHaveBeenCalled(); expect(mockPluginsService.setup).not.toHaveBeenCalled(); expect(mockLegacyService.setup).not.toHaveBeenCalled(); }); diff --git a/src/core/server/server.ts b/src/core/server/server.ts index fdda808cd3c5a9..01f2673c3f9e5c 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -85,11 +85,9 @@ export class Server { } public async start() { - const httpStart = await this.http.start(); const pluginsStart = await this.plugins.start({}); const coreStart = { - http: httpStart, plugins: pluginsStart, }; @@ -98,6 +96,7 @@ export class Server { plugins: mapToObject(pluginsStart.contracts), }); + await this.http.start(); return coreStart; } diff --git a/src/legacy/server/http/index.js b/src/legacy/server/http/index.js index 0b9c6607422097..40ac2baa032d63 100644 --- a/src/legacy/server/http/index.js +++ b/src/legacy/server/http/index.js @@ -21,14 +21,14 @@ import { format } from 'url'; import { resolve } from 'path'; import _ from 'lodash'; import Boom from 'boom'; -import Hapi from 'hapi'; + import { setupVersionCheck } from './version_check'; import { registerHapiPlugins } from './register_hapi_plugins'; import { setupBasePathProvider } from './setup_base_path_provider'; import { setupXsrf } from './xsrf'; export default async function (kbnServer, server, config) { - kbnServer.server = new Hapi.Server(kbnServer.newPlatform.params.serverOptions); + kbnServer.server = kbnServer.newPlatform.setup.core.http.server; server = kbnServer.server; setupBasePathProvider(kbnServer); diff --git a/src/legacy/server/kbn_server.d.ts b/src/legacy/server/kbn_server.d.ts index 09d274af330ac5..eba6a16674705f 100644 --- a/src/legacy/server/kbn_server.d.ts +++ b/src/legacy/server/kbn_server.d.ts @@ -103,7 +103,6 @@ export default class KbnServer { }; stop: null; params: { - serverOptions: ElasticsearchServiceSetup; handledConfigPaths: Unpromise>; }; }; diff --git a/src/legacy/server/kbn_server.js b/src/legacy/server/kbn_server.js index 78a8829dbb08a2..72ac219b0413be 100644 --- a/src/legacy/server/kbn_server.js +++ b/src/legacy/server/kbn_server.js @@ -55,7 +55,7 @@ export default class KbnServer { this.rootDir = rootDir; this.settings = settings || {}; - const { setupDeps, startDeps, serverOptions, handledConfigPaths, logger } = core; + const { setupDeps, startDeps, handledConfigPaths, logger } = core; this.newPlatform = { coreContext: { logger, @@ -64,7 +64,6 @@ export default class KbnServer { start: startDeps, stop: null, params: { - serverOptions, handledConfigPaths, }, }; diff --git a/tasks/config/run.js b/tasks/config/run.js index 4e95f594b94eed..09b1c1bd6c588e 100644 --- a/tasks/config/run.js +++ b/tasks/config/run.js @@ -29,7 +29,7 @@ module.exports = function (grunt) { return { options: { wait: false, - ready: /Server running/, + ready: /http server running/, quiet: false, failOnError: false }, diff --git a/x-pack/test/api_integration/apis/security/basic_login.js b/x-pack/test/api_integration/apis/security/basic_login.js index 34161a22fe76e4..1d10b3f8803a56 100644 --- a/x-pack/test/api_integration/apis/security/basic_login.js +++ b/x-pack/test/api_integration/apis/security/basic_login.js @@ -23,6 +23,13 @@ export default function ({ getService }) { expect(response.headers.location).to.be('/login?next=%2Fabc%2Fxyz'); }); + it('should redirect non-AJAX New platform requests to the login page if not authenticated', async () => { + const response = await supertest.get('/core/') + .expect(302); + + expect(response.headers.location).to.be('/login?next=%2Fcore%2F'); + }); + it('should reject API requests if client is not authenticated', async () => { await supertest .get('/api/security/v1/me')