[feat] create additional http servers (#36804)

* [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
elastic · May 29, 2019 · 461a6c0 · 461a6c0
1 parent 8f693be
commit 461a6c0
Show file tree

Hide file tree

Showing 14 changed files with 173 additions and 52 deletions.
diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.http.md b/docs/development/core/server/kibana-plugin-server.coresetup.http.md
@@ -13,5 +13,6 @@ http: {
         registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
         getBasePathFor: HttpServiceSetup['getBasePathFor'];
         setBasePathFor: HttpServiceSetup['setBasePathFor'];
+        createNewServer: HttpServiceSetup['createNewServer'];
     };
 ```
diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.md b/docs/development/core/server/kibana-plugin-server.coresetup.md
@@ -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> |  |
 
diff --git a/...evelopment/core/server/kibana-plugin-server.httpservicesetup.createnewserver.md b/...evelopment/core/server/kibana-plugin-server.httpservicesetup.createnewserver.md
@@ -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>;
+```
diff --git a/docs/development/core/server/kibana-plugin-server.httpservicesetup.md b/docs/development/core/server/kibana-plugin-server.httpservicesetup.md
@@ -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> |  |
+
diff --git a/docs/development/core/server/kibana-plugin-server.md b/docs/development/core/server/kibana-plugin-server.md
@@ -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. |
@@ -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. |

diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts
@@ -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);
 });
@@ -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/')
@@ -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/')
@@ -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/')
@@ -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/')
@@ -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')
@@ -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')
@@ -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')
@@ -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')
@@ -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/')
@@ -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/')
@@ -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/')
@@ -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')
@@ -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')
@@ -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;
   });
 
@@ -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;
   });
 
@@ -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();
   });
 });
 
@@ -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"`
   );
 });
@@ -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)
@@ -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)
@@ -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);
@@ -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);
@@ -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 });
@@ -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 });
@@ -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 });
@@ -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('/')
@@ -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' });
@@ -845,7 +845,7 @@ describe('#auth.get()', () => {
     );
 
     registerRouter(router);
-    await server.start(config);
+    await server.start();
 
     await supertest(innerServer.listener)
       .get('/')

diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts
@@ -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<
@@ -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);
 
@@ -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');
     }
@@ -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() {

diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts
@@ -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 = {
@@ -35,6 +37,8 @@ const createSetupContractMock = () => {
       get: jest.fn(),
       isAuthenticated: jest.fn(),
     },
+    createNewServer: async (cfg: Partial<HttpConfig>): Promise<HttpServerSetup> =>
+      ({} as HttpServerSetup),
   };
   return setupContract;
 };

diff --git a/src/core/server/http/http_service.test.mocks.ts b/src/core/server/http/http_service.test.mocks.ts
@@ -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,
+  };
+});