Skip to content

Commit

Permalink
[http] explicitly create the server listener (#183591)
Browse files Browse the repository at this point in the history
## Summary

Related to #7104
Adapted from #183465

For `http2` support, we will have to change the way we configure the
HAPI server to manually provide the listener instead of passing down the
options for HAPI to create it.

This PR prepares that work, by creating the `http` or `https` (`tls`)
listener and passing it when creating the HAPI server instead of just
passing the `tls` options.

**Note:** no integration tests were added, because we already have the
right coverage for both tls and non-tls mode, so any change of behavior
introduced by the PR should be detectable by them.
  • Loading branch information
pgayvallet authored May 21, 2024
1 parent 2c9a89e commit db316ad
Show file tree
Hide file tree
Showing 21 changed files with 463 additions and 170 deletions.
12 changes: 2 additions & 10 deletions packages/core/http/core-http-server-internal/src/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,7 @@ import { Server, Request } from '@hapi/hapi';
import HapiStaticFiles from '@hapi/inert';
import url from 'url';
import { v4 as uuidv4 } from 'uuid';
import {
createServer,
getListenerOptions,
getServerOptions,
setTlsConfig,
getRequestId,
} from '@kbn/server-http-tools';

import { createServer, getServerOptions, setTlsConfig, getRequestId } from '@kbn/server-http-tools';
import type { Duration } from 'moment';
import { Observable, Subscription, firstValueFrom, pairwise, take } from 'rxjs';
import apm from 'elastic-apm-node';
Expand Down Expand Up @@ -235,9 +228,8 @@ export class HttpServer {
this.config = config;

const serverOptions = getServerOptions(config);
const listenerOptions = getListenerOptions(config);

this.server = createServer(serverOptions, listenerOptions);
this.server = createServer(serverOptions);
await this.server.register([HapiStaticFiles]);
if (config.compression.brotli.enabled) {
await this.server.register({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { Request, ResponseToolkit, Server } from '@hapi/hapi';
import { format as formatUrl } from 'url';
import { createServer, getListenerOptions, getServerOptions } from '@kbn/server-http-tools';
import { createServer, getServerOptions } from '@kbn/server-http-tools';
import type { Logger } from '@kbn/logging';

import { HttpConfig } from './http_config';
Expand All @@ -31,13 +31,10 @@ export class HttpsRedirectServer {
// Redirect server is configured in the same way as any other HTTP server
// within the platform with the only exception that it should always be a
// plain HTTP server, so we just ignore `tls` part of options.
this.server = createServer(
{
...getServerOptions(config, { configureTLS: false }),
port: config.ssl.redirectHttpFromPort,
},
getListenerOptions(config)
);
this.server = createServer({
...getServerOptions(config, { configureTLS: false }),
port: config.ssl.redirectHttpFromPort,
});

this.server.ext('onRequest', (request: Request, responseToolkit: ResponseToolkit) => {
return responseToolkit
Expand Down
5 changes: 2 additions & 3 deletions packages/kbn-cli-dev-mode/src/base_path_proxy_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { sampleSize } from 'lodash';
import * as Rx from 'rxjs';
import { take } from 'rxjs';
import { ByteSizeValue } from '@kbn/config-schema';
import { createServer, getListenerOptions, getServerOptions } from '@kbn/server-http-tools';
import { createServer, getServerOptions } from '@kbn/server-http-tools';

import { DevConfig, HttpConfig } from './config';
import { Log } from './log';
Expand Down Expand Up @@ -67,8 +67,7 @@ export class BasePathProxyServer {

public async start(options: BasePathProxyServerOptions) {
const serverOptions = getServerOptions(this.httpConfig);
const listenerOptions = getListenerOptions(this.httpConfig);
this.server = createServer(serverOptions, listenerOptions);
this.server = createServer(serverOptions);

// Register hapi plugin that adds proxying functionality. It can be configured
// through the route configuration object (see { handler: { proxy: ... } }).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ import { Server } from '@hapi/hapi';
import { EMPTY } from 'rxjs';
import moment from 'moment';
import supertest from 'supertest';
import {
getServerOptions,
getListenerOptions,
createServer,
IHttpConfig,
} from '@kbn/server-http-tools';
import { getServerOptions, createServer, type IHttpConfig } from '@kbn/server-http-tools';
import { ByteSizeValue } from '@kbn/config-schema';

import { BasePathProxyServer, BasePathProxyServerOptions } from '../base_path_proxy_server';
Expand Down Expand Up @@ -51,8 +46,7 @@ describe('BasePathProxyServer', () => {
};

const serverOptions = getServerOptions(config);
const listenerOptions = getListenerOptions(config);
server = createServer(serverOptions, listenerOptions);
server = createServer(serverOptions);

// setup and start the proxy server
const proxyConfig: IHttpConfig = { ...config, port: 10013 };
Expand Down Expand Up @@ -276,8 +270,7 @@ describe('BasePathProxyServer', () => {
} as IHttpConfig;

const serverOptions = getServerOptions(configWithBasePath);
const listenerOptions = getListenerOptions(configWithBasePath);
server = createServer(serverOptions, listenerOptions);
server = createServer(serverOptions);

server.route({
method: 'GET',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { sslSchema, getServerOptions, getListenerOptions } from '@kbn/server-http-tools';
import { sslSchema, getServerOptions } from '@kbn/server-http-tools';

export const hapiStartMock = jest.fn();
export const hapiStopMock = jest.fn();
Expand All @@ -18,12 +18,10 @@ export const createServerMock = jest.fn().mockImplementation(() => ({
route: hapiRouteMock,
}));
export const getServerOptionsMock = jest.fn().mockImplementation(getServerOptions);
export const getListenerOptionsMock = jest.fn().mockImplementation(getListenerOptions);

jest.doMock('@kbn/server-http-tools', () => ({
createServer: createServerMock,
getServerOptions: getServerOptionsMock,
getListenerOptions: getListenerOptionsMock,
sslSchema,
SslConfig: jest.fn(),
}));
4 changes: 0 additions & 4 deletions packages/kbn-health-gateway-server/src/server/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import {
createServerMock,
getServerOptionsMock,
getListenerOptionsMock,
hapiStartMock,
hapiStopMock,
hapiRouteMock,
Expand Down Expand Up @@ -56,9 +55,6 @@ describe('Server', () => {
expect(getServerOptionsMock.mock.calls[0][0]).toEqual(
expect.objectContaining({ ...mockConfig })
);
expect(getListenerOptionsMock.mock.calls[0][0]).toEqual(
expect.objectContaining({ ...mockConfig })
);
});

test('starts the Hapi server', async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/kbn-health-gateway-server/src/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import type { Server as HapiServer, ServerRoute as HapiServerRoute } from '@hapi/hapi';
import { createServer, getServerOptions, getListenerOptions } from '@kbn/server-http-tools';
import { createServer, getServerOptions } from '@kbn/server-http-tools';
import type { IConfigService } from '@kbn/config';
import type { Logger, LoggerFactory } from '@kbn/logging';
import { ServerConfig } from './server_config';
Expand Down Expand Up @@ -40,7 +40,7 @@ export class Server {

async start(): Promise<ServerStart> {
const serverConfig = new ServerConfig(this.config.atPathSync<ServerConfigType>('server'));
this.server = createServer(getServerOptions(serverConfig), getListenerOptions(serverConfig));
this.server = createServer(getServerOptions(serverConfig));

await this.server.start();
this.log.info(`Server running on ${this.server.info.uri}`);
Expand Down
5 changes: 3 additions & 2 deletions packages/kbn-server-http-tools/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
export type { IHttpConfig, ISslConfig, ICorsConfig } from './src/types';
export { createServer } from './src/create_server';
export { defaultValidationErrorHandler } from './src/default_validation_error_handler';
export { getListenerOptions } from './src/get_listener_options';
export { getServerOptions, getServerTLSOptions } from './src/get_server_options';
export { getServerListener } from './src/get_listener';
export { getServerOptions } from './src/get_server_options';
export { getServerTLSOptions } from './src/get_tls_options';
export { getRequestId } from './src/get_request_id';
export { setTlsConfig } from './src/set_tls_config';
export { sslSchema, SslConfig } from './src/ssl';
20 changes: 2 additions & 18 deletions packages/kbn-server-http-tools/src/create_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,7 @@
*/

import { Server, ServerOptions } from '@hapi/hapi';
import { ListenerOptions } from './get_listener_options';

export function createServer(serverOptions: ServerOptions, listenerOptions: ListenerOptions) {
const server = new Server(serverOptions);

server.listener.keepAliveTimeout = listenerOptions.keepaliveTimeout;
server.listener.setTimeout(listenerOptions.socketTimeout);
server.listener.on('timeout', (socket) => {
socket.destroy();
});
server.listener.on('clientError', (err, socket) => {
if (socket.writable) {
socket.end(Buffer.from('HTTP/1.1 400 Bad Request\r\n\r\n', 'ascii'));
} else {
socket.destroy(err);
}
});

return server;
export function createServer(serverOptions: ServerOptions) {
return new Server(serverOptions);
}
47 changes: 47 additions & 0 deletions packages/kbn-server-http-tools/src/get_listener.test.mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const getServerTLSOptionsMock = jest.fn();

jest.doMock('./get_tls_options', () => {
const actual = jest.requireActual('./get_tls_options');
return {
...actual,
getServerTLSOptions: getServerTLSOptionsMock,
};
});

export const createHttpServerMock = jest.fn(() => {
return {
on: jest.fn(),
setTimeout: jest.fn(),
};
});

jest.doMock('http', () => {
const actual = jest.requireActual('http');
return {
...actual,
createServer: createHttpServerMock,
};
});

export const createHttpsServerMock = jest.fn(() => {
return {
on: jest.fn(),
setTimeout: jest.fn(),
};
});

jest.doMock('https', () => {
const actual = jest.requireActual('https');
return {
...actual,
createServer: createHttpsServerMock,
};
});
139 changes: 139 additions & 0 deletions packages/kbn-server-http-tools/src/get_listener.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import {
getServerTLSOptionsMock,
createHttpServerMock,
createHttpsServerMock,
} from './get_listener.test.mocks';
import moment from 'moment';
import { ByteSizeValue } from '@kbn/config-schema';
import type { IHttpConfig } from './types';
import { getServerListener } from './get_listener';

const createConfig = (parts: Partial<IHttpConfig>): IHttpConfig => ({
host: 'localhost',
port: 5601,
socketTimeout: 120000,
keepaliveTimeout: 120000,
payloadTimeout: 20000,
shutdownTimeout: moment.duration(30, 'seconds'),
maxPayload: ByteSizeValue.parse('1048576b'),
...parts,
cors: {
enabled: false,
allowCredentials: false,
allowOrigin: ['*'],
...parts.cors,
},
ssl: {
enabled: false,
...parts.ssl,
},
restrictInternalApis: false,
});

describe('getServerListener', () => {
beforeEach(() => {
getServerTLSOptionsMock.mockReset();
createHttpServerMock.mockClear();
createHttpsServerMock.mockClear();
});

describe('when TLS is enabled', () => {
it('calls getServerTLSOptions with the correct parameters', () => {
const config = createConfig({ ssl: { enabled: true } });

getServerListener(config);

expect(getServerTLSOptionsMock).toHaveBeenCalledTimes(1);
expect(getServerTLSOptionsMock).toHaveBeenCalledWith(config.ssl);
});

it('calls https.createServer with the correct parameters', () => {
const config = createConfig({ ssl: { enabled: true } });

getServerTLSOptionsMock.mockReturnValue({ stub: true });

getServerListener(config);

expect(createHttpsServerMock).toHaveBeenCalledTimes(1);
expect(createHttpsServerMock).toHaveBeenCalledWith({
stub: true,
keepAliveTimeout: config.keepaliveTimeout,
});
});

it('properly configures the listener', () => {
const config = createConfig({ ssl: { enabled: true } });
const server = getServerListener(config);

expect(server.setTimeout).toHaveBeenCalledTimes(1);
expect(server.setTimeout).toHaveBeenCalledWith(config.socketTimeout);

expect(server.on).toHaveBeenCalledTimes(2);
expect(server.on).toHaveBeenCalledWith('clientError', expect.any(Function));
expect(server.on).toHaveBeenCalledWith('timeout', expect.any(Function));
});

it('returns the https server', () => {
const config = createConfig({ ssl: { enabled: true } });

const server = getServerListener(config);

const expectedServer = createHttpsServerMock.mock.results[0].value;

expect(server).toBe(expectedServer);
});
});

describe('when TLS is disabled', () => {
it('does not call getServerTLSOptions', () => {
const config = createConfig({ ssl: { enabled: false } });

getServerListener(config);

expect(getServerTLSOptionsMock).not.toHaveBeenCalled();
});

it('calls http.createServer with the correct parameters', () => {
const config = createConfig({ ssl: { enabled: false } });

getServerTLSOptionsMock.mockReturnValue({ stub: true });

getServerListener(config);

expect(createHttpServerMock).toHaveBeenCalledTimes(1);
expect(createHttpServerMock).toHaveBeenCalledWith({
keepAliveTimeout: config.keepaliveTimeout,
});
});

it('properly configures the listener', () => {
const config = createConfig({ ssl: { enabled: false } });
const server = getServerListener(config);

expect(server.setTimeout).toHaveBeenCalledTimes(1);
expect(server.setTimeout).toHaveBeenCalledWith(config.socketTimeout);

expect(server.on).toHaveBeenCalledTimes(2);
expect(server.on).toHaveBeenCalledWith('clientError', expect.any(Function));
expect(server.on).toHaveBeenCalledWith('timeout', expect.any(Function));
});

it('returns the http server', () => {
const config = createConfig({ ssl: { enabled: false } });

const server = getServerListener(config);

const expectedServer = createHttpServerMock.mock.results[0].value;

expect(server).toBe(expectedServer);
});
});
});
Loading

0 comments on commit db316ad

Please sign in to comment.