Skip to content

Commit

Permalink
[feat] create additional http servers (elastic#36804)
Browse files Browse the repository at this point in the history
* [feat] create additional http servers

allow for additional http servers to be created, tracked and returned

* respond to pr feedback

* tweak test

* update documentation

* destructure port, remove unnecessary imports

* [fix] export correct type

* [feat] expose createNewServer to plugins

* [fix] respond to pr feedback

* todo: add schema validation & integration test

* use reach

* [fix] use validateKey to validate partial

* [fix] change config shadowing

* check kibana port & prevent shadowing

* centralize start/stop for servers, add integration test

* remove unnecessary property

* never forget your await

* remove option to pass config into start

* fix pr feedback

* fix documentation

* fix test failures
  • Loading branch information
toddself authored and jkakavas committed May 30, 2019
1 parent 64ccc79 commit 8ae5167
Show file tree
Hide file tree
Showing 14 changed files with 173 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ http: {
registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
getBasePathFor: HttpServiceSetup['getBasePathFor'];
setBasePathFor: HttpServiceSetup['setBasePathFor'];
createNewServer: HttpServiceSetup['createNewServer'];
};
```
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ export interface CoreSetup
| Property | Type | Description |
| --- | --- | --- |
| [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | <code>{`<p/>` adminClient$: Observable&lt;ClusterClient&gt;;`<p/>` dataClient$: Observable&lt;ClusterClient&gt;;`<p/>` }</code> | |
| [http](./kibana-plugin-server.coresetup.http.md) | <code>{`<p/>` registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];`<p/>` registerAuth: HttpServiceSetup['registerAuth'];`<p/>` registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];`<p/>` getBasePathFor: HttpServiceSetup['getBasePathFor'];`<p/>` setBasePathFor: HttpServiceSetup['setBasePathFor'];`<p/>` }</code> | |
| [http](./kibana-plugin-server.coresetup.http.md) | <code>{`<p/>` registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];`<p/>` registerAuth: HttpServiceSetup['registerAuth'];`<p/>` registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];`<p/>` getBasePathFor: HttpServiceSetup['getBasePathFor'];`<p/>` setBasePathFor: HttpServiceSetup['setBasePathFor'];`<p/>` createNewServer: HttpServiceSetup['createNewServer'];`<p/>` }</code> | |
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) &gt; [createNewServer](./kibana-plugin-server.httpservicesetup.createnewserver.md)

## HttpServiceSetup.createNewServer property

<b>Signature:</b>

```typescript
createNewServer: (cfg: Partial<HttpConfig>) => Promise<HttpServerSetup>;
```
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@

[Home](./index) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md)

## HttpServiceSetup type
## HttpServiceSetup interface


<b>Signature:</b>

```typescript
export declare type HttpServiceSetup = HttpServerSetup;
export interface HttpServiceSetup extends HttpServerSetup
```
## Properties
| Property | Type | Description |
| --- | --- | --- |
| [createNewServer](./kibana-plugin-server.httpservicesetup.createnewserver.md) | <code>(cfg: Partial&lt;HttpConfig&gt;) =&gt; Promise&lt;HttpServerSetup&gt;</code> | |
2 changes: 1 addition & 1 deletion docs/development/core/server/kibana-plugin-server.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [CoreStart](./kibana-plugin-server.corestart.md) | Context passed to the plugins <code>start</code> method. |
| [DiscoveredPlugin](./kibana-plugin-server.discoveredplugin.md) | Small container object used to expose information about discovered plugins that may or may not have been started. |
| [ElasticsearchServiceSetup](./kibana-plugin-server.elasticsearchservicesetup.md) | |
| [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | |
| [HttpServiceStart](./kibana-plugin-server.httpservicestart.md) | |
| [InternalCoreStart](./kibana-plugin-server.internalcorestart.md) | |
| [Logger](./kibana-plugin-server.logger.md) | Logger exposes all the necessary methods to log any type of information and this is the interface used by the logging consumers including plugins. |
Expand All @@ -49,7 +50,6 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [AuthenticationHandler](./kibana-plugin-server.authenticationhandler.md) | |
| [ElasticsearchClientConfig](./kibana-plugin-server.elasticsearchclientconfig.md) | |
| [Headers](./kibana-plugin-server.headers.md) | |
| [HttpServiceSetup](./kibana-plugin-server.httpservicesetup.md) | |
| [OnPostAuthHandler](./kibana-plugin-server.onpostauthhandler.md) | |
| [OnPreAuthHandler](./kibana-plugin-server.onpreauthhandler.md) | |
| [PluginInitializer](./kibana-plugin-server.plugininitializer.md) | The <code>plugin</code> export at the root of a plugin's <code>server</code> directory should conform to this interface. |
Expand Down
60 changes: 30 additions & 30 deletions src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test('listening after started', async () => {
expect(server.isListening()).toBe(false);

await server.setup(config);
await server.start(config);
await server.start();

expect(server.isListening()).toBe(true);
});
Expand All @@ -72,7 +72,7 @@ test('200 OK with body', async () => {

const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);
await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/')
Expand All @@ -92,7 +92,7 @@ test('202 Accepted with body', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/')
Expand All @@ -112,7 +112,7 @@ test('204 No content', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/')
Expand All @@ -134,7 +134,7 @@ test('400 Bad request with error', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/')
Expand Down Expand Up @@ -164,7 +164,7 @@ test('valid params', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/some-string')
Expand Down Expand Up @@ -194,7 +194,7 @@ test('invalid params', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/some-string')
Expand Down Expand Up @@ -227,7 +227,7 @@ test('valid query', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/?bar=test&quux=123')
Expand Down Expand Up @@ -257,7 +257,7 @@ test('invalid query', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/?bar=test')
Expand Down Expand Up @@ -290,7 +290,7 @@ test('valid body', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.post('/foo/')
Expand Down Expand Up @@ -324,7 +324,7 @@ test('invalid body', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.post('/foo/')
Expand Down Expand Up @@ -357,7 +357,7 @@ test('handles putting', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.put('/foo/')
Expand Down Expand Up @@ -388,7 +388,7 @@ test('handles deleting', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.delete('/foo/3')
Expand All @@ -414,7 +414,7 @@ test('filtered headers', async () => {
const { registerRouter, server: innerServer } = await server.setup(config);
registerRouter(router);

await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/foo/?bar=quux')
Expand Down Expand Up @@ -444,10 +444,10 @@ describe('with `basepath: /bar` and `rewriteBasePath: false`', () => {
res.ok({ key: 'value:/foo' })
);

const { registerRouter, server: innerServer } = await server.setup(config);
const { registerRouter, server: innerServer } = await server.setup(configWithBasePath);
registerRouter(router);

await server.start(configWithBasePath);
await server.start();
innerServerListener = innerServer.listener;
});

Expand Down Expand Up @@ -508,7 +508,7 @@ describe('with `basepath: /bar` and `rewriteBasePath: true`', () => {
const { registerRouter, server: innerServer } = await server.setup(configWithBasePath);
registerRouter(router);

await server.start(configWithBasePath);
await server.start();
innerServerListener = innerServer.listener;
});

Expand Down Expand Up @@ -571,10 +571,10 @@ describe('with defined `redirectHttpFromPort`', () => {
const router = new Router('/');
router.get({ path: '/', validate: false }, async (req, res) => res.ok({ key: 'value:/' }));

const { registerRouter } = await server.setup(config);
const { registerRouter } = await server.setup(configWithSSL);
registerRouter(router);

await server.start(configWithSSL);
await server.start();
});
});

Expand Down Expand Up @@ -610,7 +610,7 @@ test('registers registerOnPostAuth interceptor several times', async () => {
});

test('throws an error if starts without set up', async () => {
await expect(server.start(config)).rejects.toThrowErrorMatchingInlineSnapshot(
await expect(server.start()).rejects.toThrowErrorMatchingInlineSnapshot(
`"Http server is not setup up yet"`
);
});
Expand All @@ -634,7 +634,7 @@ test('#getBasePathFor() returns base path associated with an incoming request',
router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: getBasePathFor(req) }));
registerRouter(router);

await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200)
Expand Down Expand Up @@ -668,7 +668,7 @@ test('#getBasePathFor() is based on server base path', async () => {
);
registerRouter(router);

await server.start(configWithBasePath);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200)
Expand Down Expand Up @@ -727,7 +727,7 @@ test('Should enable auth for a route by default if registerAuth has been called'
.mockImplementation((req, sessionStorage, t) => t.authenticated({}));
await registerAuth(authenticate, cookieOptions);

await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200);
Expand All @@ -744,7 +744,7 @@ test('Should support disabling auth for a route', async () => {
const authenticate = jest.fn();
await registerAuth(authenticate, cookieOptions);

await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200);
Expand All @@ -764,7 +764,7 @@ describe('#auth.isAuthenticated()', () => {

await registerAuth((req, sessionStorage, t) => t.authenticated({}), cookieOptions);

await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200, { isAuthenticated: true });
Expand All @@ -781,7 +781,7 @@ describe('#auth.isAuthenticated()', () => {

await registerAuth((req, sessionStorage, t) => t.authenticated({}), cookieOptions);

await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200, { isAuthenticated: false });
Expand All @@ -796,7 +796,7 @@ describe('#auth.isAuthenticated()', () => {
);
registerRouter(router);

await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200, { isAuthenticated: false });
Expand All @@ -815,7 +815,7 @@ describe('#auth.get()', () => {
const router = new Router('');
router.get({ path: '/', validate: false }, async (req, res) => res.ok(auth.get(req)));
registerRouter(router);
await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/')
Expand All @@ -828,7 +828,7 @@ describe('#auth.get()', () => {
router.get({ path: '/', validate: false }, async (req, res) => res.ok(auth.get(req)));

registerRouter(router);
await server.start(config);
await server.start();
await supertest(innerServer.listener)
.get('/')
.expect(200, { status: 'unknown' });
Expand All @@ -845,7 +845,7 @@ describe('#auth.get()', () => {
);

registerRouter(router);
await server.start(config);
await server.start();

await supertest(innerServer.listener)
.get('/')
Expand Down
12 changes: 5 additions & 7 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export interface HttpServerSetup {

export class HttpServer {
private server?: Server;
private config?: HttpConfig;
private registeredRouters = new Set<Router>();
private authRegistered = false;
private basePathCache = new WeakMap<
Expand Down Expand Up @@ -124,6 +125,7 @@ export class HttpServer {
public setup(config: HttpConfig): HttpServerSetup {
const serverOptions = getServerOptions(config);
this.server = createServer(serverOptions);
this.config = config;

this.setupBasePathRewrite(config);

Expand All @@ -149,7 +151,7 @@ export class HttpServer {
};
}

public async start(config: HttpConfig) {
public async start() {
if (this.server === undefined) {
throw new Error('Http server is not setup up yet');
}
Expand All @@ -170,12 +172,8 @@ export class HttpServer {
}

await this.server.start();

this.log.debug(
`http server running at ${this.server.info.uri}${
config.rewriteBasePath ? config.basePath : ''
}`
);
const serverPath = this.config!.rewriteBasePath || this.config!.basePath || '';
this.log.debug(`http server running at ${this.server.info.uri}${serverPath}`);
}

public async stop() {
Expand Down
4 changes: 4 additions & 0 deletions src/core/server/http/http_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import { Server, ServerOptions } from 'hapi';
import { HttpService } from './http_service';
import { HttpConfig } from './http_config';
import { HttpServerSetup } from './http_server';

const createSetupContractMock = () => {
const setupContract = {
Expand All @@ -35,6 +37,8 @@ const createSetupContractMock = () => {
get: jest.fn(),
isAuthenticated: jest.fn(),
},
createNewServer: async (cfg: Partial<HttpConfig>): Promise<HttpServerSetup> =>
({} as HttpServerSetup),
};
return setupContract;
};
Expand Down
11 changes: 8 additions & 3 deletions src/core/server/http/http_service.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@

export const mockHttpServer = jest.fn();

jest.mock('./http_server', () => ({
HttpServer: mockHttpServer,
}));
jest.mock('./http_server', () => {
const realHttpServer = jest.requireActual('./http_server');

return {
...realHttpServer,
HttpServer: mockHttpServer,
};
});
Loading

0 comments on commit 8ae5167

Please sign in to comment.