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

New Platform and Legacy platform servers integration #39047

Merged
merged 12 commits into from
Jun 19, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -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

<b>Signature:</b>

```typescript
isListening: () => boolean;
isListening: (port: number) => boolean;
```
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ export interface HttpServiceStart

| Property | Type | Description |
| --- | --- | --- |
| [isListening](./kibana-plugin-server.httpservicestart.islistening.md) | <code>() =&gt; boolean</code> | Indicates if http server is listening on a port |
| [isListening](./kibana-plugin-server.httpservicestart.islistening.md) | <code>(port: number) =&gt; boolean</code> | Indicates if http server is listening on a given port |

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,5 @@ export interface InternalCoreStart

| Property | Type | Description |
| --- | --- | --- |
| [http](./kibana-plugin-server.internalcorestart.http.md) | <code>HttpServiceStart</code> | |
| [plugins](./kibana-plugin-server.internalcorestart.plugins.md) | <code>PluginsServiceStart</code> | |

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export async function runKibanaServer({ procs, config, options }) {
...process.env,
},
cwd: installDir || KIBANA_ROOT,
wait: /Server running/,
wait: /http server running/,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 0 additions & 25 deletions src/core/server/http/__snapshots__/http_server.test.ts.snap

This file was deleted.

21 changes: 21 additions & 0 deletions src/core/server/http/__snapshots__/http_service.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
10 changes: 6 additions & 4 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary template string. Also, can you elaborate why this log needs to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to log with info level something similar to Server running from the Legacy platform. I decided to use http server running, also re-phrase this one to highlight the difference between them.

this.log.debug(`http server listens at ${this.server.info.uri}${serverPath}`);
mshustov marked this conversation as resolved.
Show resolved Hide resolved
}

public async stop() {
Expand Down Expand Up @@ -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');
Expand Down
3 changes: 1 addition & 2 deletions src/core/server/http/http_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -27,7 +27,6 @@ type ServiceSetupMockType = jest.Mocked<HttpServiceSetup> & {
};
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(),
Expand Down
78 changes: 67 additions & 11 deletions src/core/server/http/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -43,6 +44,11 @@ const createConfigService = (value: Partial<HttpConfigType> = {}) => {
configService.setSchema(config.path, config.schema);
return configService;
};
const dummyHapiServer = {
mshustov marked this conversation as resolved.
Show resolved Hide resolved
start: noop,
stop: noop,
route: noop,
};

afterEach(() => {
jest.clearAllMocks();
Expand All @@ -56,9 +62,9 @@ test('creates and sets up http server', async () => {

const httpServer = {
isListening: () => false,
setup: jest.fn(),
setup: jest.fn().mockReturnValue({ server: dummyHapiServer }),
start: jest.fn(),
stop: noop,
stop: jest.fn(),
};
mockHttpServer.mockImplementation(() => httpServer);

Expand All @@ -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!
Expand Down Expand Up @@ -121,7 +178,7 @@ test('logs error if already set up', async () => {

const httpServer = {
isListening: () => true,
setup: jest.fn(),
setup: jest.fn().mockReturnValue({ server: dummyHapiServer }),
start: noop,
stop: noop,
};
Expand All @@ -139,7 +196,7 @@ test('stops http server', async () => {

const httpServer = {
isListening: () => false,
setup: noop,
setup: jest.fn().mockReturnValue({ server: dummyHapiServer }),
start: noop,
stop: jest.fn(),
};
Expand All @@ -163,7 +220,9 @@ test('register route handler', async () => {
const registerRouterMock = jest.fn();
const httpServer = {
isListening: () => false,
setup: () => ({ registerRouter: registerRouterMock }),
setup: jest
.fn()
.mockReturnValue({ server: dummyHapiServer, registerRouter: registerRouterMock }),
start: noop,
stop: noop,
};
Expand All @@ -181,10 +240,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: dummyHapiServer, options: { someOption: true } };

mockHttpServer.mockImplementation(() => ({
isListening: () => false,
Expand Down
Loading