address Pierres comments

x-pack/plugins/licensing/server/index.ts

@@ -12,7 +12,4 @@ export const plugin = (context: PluginInitializerContext) => new LicensingPlugin
 export * from '../common/types';
 export * from './types';
 export { config } from './licensing_config';
-export {
-  CheckLicense,
-  licenseCheckerRouteHandlerWrapper,
-} from './license_checker_route_handler_wrapper';
+export { CheckLicense, wrapRouteWithLicenseCheck } from './wrap_route_with_license_check';

x-pack/plugins/licensing/server/licensing_checker_route_handler_wrapper.test.ts → x-pack/plugins/licensing/server/wrap_route_with_license_check.test.ts

@@ -6,10 +6,7 @@
 
 import { httpServerMock } from 'src/core/server/mocks';
 
-import {
-  licenseCheckerRouteHandlerWrapper,
-  CheckLicense,
-} from './license_checker_route_handler_wrapper';
+import { wrapRouteWithLicenseCheck, CheckLicense } from './wrap_route_with_license_check';
 
 const context = {
   licensing: {
@@ -18,11 +15,11 @@ const context = {
 } as any;
 const request = httpServerMock.createKibanaRequest();
 
-describe('licenseCheckerRouteHandlerWrapper', () => {
+describe('wrapRouteWithLicenseCheck', () => {
   it('calls route handler if checkLicense returns "valid": true', async () => {
     const checkLicense: CheckLicense = () => ({ valid: true, message: null });
     const routeHandler = jest.fn();
-    const wrapper = licenseCheckerRouteHandlerWrapper(checkLicense, routeHandler);
+    const wrapper = wrapRouteWithLicenseCheck(checkLicense, routeHandler);
     const response = httpServerMock.createResponseFactory();
 
     await wrapper(context, request, response);
@@ -34,7 +31,7 @@ describe('licenseCheckerRouteHandlerWrapper', () => {
   it('does not call route handler if checkLicense returns "valid": false', async () => {
     const checkLicense: CheckLicense = () => ({ valid: false, message: 'reason' });
     const routeHandler = jest.fn();
-    const wrapper = licenseCheckerRouteHandlerWrapper(checkLicense, routeHandler);
+    const wrapper = wrapRouteWithLicenseCheck(checkLicense, routeHandler);
     const response = httpServerMock.createResponseFactory();
 
     await wrapper(context, request, response);
@@ -49,11 +46,26 @@ describe('licenseCheckerRouteHandlerWrapper', () => {
     const routeHandler = () => {
       throw new Error('reason');
     };
-    const wrapper = licenseCheckerRouteHandlerWrapper(checkLicense, routeHandler);
+    const wrapper = wrapRouteWithLicenseCheck(checkLicense, routeHandler);
     const response = httpServerMock.createResponseFactory();
 
     await expect(wrapper(context, request, response)).rejects.toThrowErrorMatchingInlineSnapshot(
       `"reason"`
     );
   });
+
+  it('allows an exception to bubble up if "checkLicense" throws', async () => {
+    const checkLicense: CheckLicense = () => {
+      throw new Error('reason');
+    };
+    const routeHandler = jest.fn();
+    const wrapper = wrapRouteWithLicenseCheck(checkLicense, routeHandler);
+    const response = httpServerMock.createResponseFactory();
+
+    await expect(wrapper(context, request, response)).rejects.toThrowErrorMatchingInlineSnapshot(
+      `"reason"`
+    );
+
+    expect(routeHandler).toHaveBeenCalledTimes(0);
+  });
 });

x-pack/plugins/licensing/server/license_checker_route_handler_wrapper.ts → x-pack/plugins/licensing/server/wrap_route_with_license_check.ts

@@ -18,7 +18,7 @@ export type CheckLicense = (
   license: ILicense
 ) => { valid: false; message: string } | { valid: true; message: null };
 
-export function licenseCheckerRouteHandlerWrapper<P, Q, B>(
+export function wrapRouteWithLicenseCheck<P, Q, B>(
   checkLicense: CheckLicense,
   handler: RequestHandler<P, Q, B>
 ): RequestHandler<P, Q, B> {

x-pack/plugins/logstash/server/lib/check_license/check_license.test.ts

@@ -7,7 +7,7 @@ import { licensingMock } from '../../../../licensing/server/mocks';
 import { checkLicense } from './check_license';
 
 describe('check_license', function() {
-  describe('returns "valid": false & message', () => {
+  describe('returns "valid": false & message when', () => {
     it('license information is not available', () => {
       const license = licensingMock.createLicenseMock();
       license.isAvailable = false;
@@ -48,6 +48,7 @@ describe('check_license', function() {
       expect(message).toStrictEqual(expect.any(String));
     });
   });
+
   it('returns "valid": true without message otherwise', () => {
     const license = licensingMock.createLicenseMock();
 

x-pack/plugins/logstash/server/plugin.ts

@@ -5,6 +5,7 @@
  */
 import {
   CoreSetup,
+  CoreStart,
   ICustomClusterClient,
   Logger,
   Plugin,
@@ -23,18 +24,25 @@ interface SetupDeps {
 export class LogstashPlugin implements Plugin {
   private readonly logger: Logger;
   private esClient?: ICustomClusterClient;
+  private coreSetup?: CoreSetup;
   constructor(context: PluginInitializerContext) {
     this.logger = context.logger.get();
   }
 
   setup(core: CoreSetup, deps: SetupDeps) {
     this.logger.debug('Setting up Logstash plugin');
-    const esClient = core.elasticsearch.createClient('logstash');
 
-    registerRoutes(core.http.createRouter(), esClient, deps.security);
+    this.coreSetup = core;
+    registerRoutes(core.http.createRouter(), deps.security);
   }
 
-  start() {}
+  start(core: CoreStart) {
+    const esClient = core.elasticsearch.legacy.createClient('logstash');
+
+    this.coreSetup!.http.registerRouteHandlerContext('logstash', async (context, request) => {
+      return { esClient: esClient.asScoped(request) };
+    });
+  }
   stop() {
     if (this.esClient) {
       this.esClient.close();

x-pack/plugins/logstash/server/routes/cluster/load.ts

@@ -3,35 +3,32 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { ICustomClusterClient, IRouter } from 'src/core/server';
-import { licenseCheckerRouteHandlerWrapper } from '../../../../licensing/server';
+import { IRouter } from 'src/core/server';
+import { wrapRouteWithLicenseCheck } from '../../../../licensing/server';
 import { Cluster } from '../../models/cluster';
 import { checkLicense } from '../../lib/check_license';
 
-export function registerClusterLoadRoute(router: IRouter, esClient: ICustomClusterClient) {
+export function registerClusterLoadRoute(router: IRouter) {
   router.get(
     {
       path: '/api/logstash/cluster',
       validate: false,
     },
-    licenseCheckerRouteHandlerWrapper(
-      checkLicense,
-      router.handleLegacyErrors(async (context, request, response) => {
-        try {
-          const client = esClient.asScoped(request);
-          const info = await client.callAsCurrentUser('info');
-          return response.ok({
-            body: {
-              cluster: Cluster.fromUpstreamJSON(info).downstreamJSON,
-            },
-          });
-        } catch (err) {
-          if (err.status === 403) {
-            return response.ok();
-          }
-          return response.internalError();
+    wrapRouteWithLicenseCheck(checkLicense, async (context, request, response) => {
+      try {
+        const client = context.logstash!.esClient;
+        const info = await client.callAsCurrentUser('info');
+        return response.ok({
+          body: {
+            cluster: Cluster.fromUpstreamJSON(info).downstreamJSON,
+          },
+        });
+      } catch (err) {
+        if (err.status === 403) {
+          return response.ok();
         }
-      })
-    )
+        return response.internalError();
+      }
+    })
   );
 }

x-pack/plugins/logstash/server/routes/index.ts

@@ -3,7 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { IRouter, ICustomClusterClient } from 'src/core/server';
+import { IRouter } from 'src/core/server';
 import { SecurityPluginSetup } from '../../../security/server';
 import { registerClusterLoadRoute } from './cluster';
 import {
@@ -14,19 +14,15 @@ import {
 import { registerPipelinesListRoute, registerPipelinesDeleteRoute } from './pipelines';
 import { registerUpgradeRoute } from './upgrade';
 
-export function registerRoutes(
-  router: IRouter,
-  esClient: ICustomClusterClient,
-  security?: SecurityPluginSetup
-) {
-  registerClusterLoadRoute(router, esClient);
+export function registerRoutes(router: IRouter, security?: SecurityPluginSetup) {
+  registerClusterLoadRoute(router);
 
-  registerPipelineDeleteRoute(router, esClient);
-  registerPipelineLoadRoute(router, esClient);
-  registerPipelineSaveRoute(router, esClient, security);
+  registerPipelineDeleteRoute(router);
+  registerPipelineLoadRoute(router);
+  registerPipelineSaveRoute(router, security);
 
-  registerPipelinesListRoute(router, esClient);
-  registerPipelinesDeleteRoute(router, esClient);
+  registerPipelinesListRoute(router);
+  registerPipelinesDeleteRoute(router);
 
-  registerUpgradeRoute(router, esClient);
+  registerUpgradeRoute(router);
 }

x-pack/plugins/logstash/server/routes/pipeline/delete.ts

@@ -4,13 +4,13 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { schema } from '@kbn/config-schema';
-import { ICustomClusterClient, IRouter } from 'src/core/server';
+import { IRouter } from 'src/core/server';
 import { INDEX_NAMES } from '../../../common/constants';
-import { licenseCheckerRouteHandlerWrapper } from '../../../../licensing/server';
+import { wrapRouteWithLicenseCheck } from '../../../../licensing/server';
 
 import { checkLicense } from '../../lib/check_license';
 
-export function registerPipelineDeleteRoute(router: IRouter, esClient: ICustomClusterClient) {
+export function registerPipelineDeleteRoute(router: IRouter) {
   router.delete(
     {
       path: '/api/logstash/pipeline/{id}',
@@ -20,10 +20,10 @@ export function registerPipelineDeleteRoute(router: IRouter, esClient: ICustomCl
         }),
       },
     },
-    licenseCheckerRouteHandlerWrapper(
+    wrapRouteWithLicenseCheck(
       checkLicense,
       router.handleLegacyErrors(async (context, request, response) => {
-        const client = esClient.asScoped(request);
+        const client = context.logstash!.esClient;
 
         await client.callAsCurrentUser('delete', {
           index: INDEX_NAMES.PIPELINES,

x-pack/plugins/logstash/server/routes/pipeline/load.ts

@@ -4,14 +4,14 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { schema } from '@kbn/config-schema';
-import { ICustomClusterClient, IRouter } from 'src/core/server';
+import { IRouter } from 'src/core/server';
 
 import { INDEX_NAMES } from '../../../common/constants';
 import { Pipeline } from '../../models/pipeline';
-import { licenseCheckerRouteHandlerWrapper } from '../../../../licensing/server';
+import { wrapRouteWithLicenseCheck } from '../../../../licensing/server';
 import { checkLicense } from '../../lib/check_license';
 
-export function registerPipelineLoadRoute(router: IRouter, esClient: ICustomClusterClient) {
+export function registerPipelineLoadRoute(router: IRouter) {
   router.get(
     {
       path: '/api/logstash/pipeline/{id}',
@@ -21,10 +21,10 @@ export function registerPipelineLoadRoute(router: IRouter, esClient: ICustomClus
         }),
       },
     },
-    licenseCheckerRouteHandlerWrapper(
+    wrapRouteWithLicenseCheck(
       checkLicense,
       router.handleLegacyErrors(async (context, request, response) => {
-        const client = esClient.asScoped(request);
+        const client = context.logstash!.esClient;
 
         const result = await client.callAsCurrentUser('get', {
           index: INDEX_NAMES.PIPELINES,

x-pack/plugins/logstash/server/routes/pipeline/save.ts

@@ -5,19 +5,15 @@
  */
 import { schema } from '@kbn/config-schema';
 import { i18n } from '@kbn/i18n';
-import { ICustomClusterClient, IRouter } from 'src/core/server';
+import { IRouter } from 'src/core/server';
 
 import { INDEX_NAMES } from '../../../common/constants';
 import { Pipeline } from '../../models/pipeline';
-import { licenseCheckerRouteHandlerWrapper } from '../../../../licensing/server';
+import { wrapRouteWithLicenseCheck } from '../../../../licensing/server';
 import { SecurityPluginSetup } from '../../../../security/server';
 import { checkLicense } from '../../lib/check_license';
 
-export function registerPipelineSaveRoute(
-  router: IRouter,
-  esClient: ICustomClusterClient,
-  security?: SecurityPluginSetup
-) {
+export function registerPipelineSaveRoute(router: IRouter, security?: SecurityPluginSetup) {
   router.put(
     {
       path: '/api/logstash/pipeline/{id}',
@@ -34,7 +30,7 @@ export function registerPipelineSaveRoute(
         }),
       },
     },
-    licenseCheckerRouteHandlerWrapper(
+    wrapRouteWithLicenseCheck(
       checkLicense,
       router.handleLegacyErrors(async (context, request, response) => {
         try {
@@ -44,7 +40,7 @@ export function registerPipelineSaveRoute(
             username = user?.username;
           }
 
-          const client = esClient.asScoped(request);
+          const client = context.logstash!.esClient;
           const pipeline = Pipeline.fromDownstreamJSON(request.body, request.params.id, username);
 
           await client.callAsCurrentUser('index', {

x-pack/plugins/logstash/server/routes/pipelines/delete.ts

@@ -4,8 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { schema } from '@kbn/config-schema';
-import { APICaller, ICustomClusterClient, IRouter } from 'src/core/server';
-import { licenseCheckerRouteHandlerWrapper } from '../../../../licensing/server';
+import { APICaller, IRouter } from 'src/core/server';
+import { wrapRouteWithLicenseCheck } from '../../../../licensing/server';
 
 import { INDEX_NAMES } from '../../../common/constants';
 import { checkLicense } from '../../lib/check_license';
@@ -31,7 +31,7 @@ async function deletePipelines(callWithRequest: APICaller, pipelineIds: string[]
   };
 }
 
-export function registerPipelinesDeleteRoute(router: IRouter, esClient: ICustomClusterClient) {
+export function registerPipelinesDeleteRoute(router: IRouter) {
   router.post(
     {
       path: '/api/logstash/pipelines/delete',
@@ -41,10 +41,10 @@ export function registerPipelinesDeleteRoute(router: IRouter, esClient: ICustomC
         }),
       },
     },
-    licenseCheckerRouteHandlerWrapper(
+    wrapRouteWithLicenseCheck(
       checkLicense,
       router.handleLegacyErrors(async (context, request, response) => {
-        const client = esClient.asScoped(request);
+        const client = context.logstash!.esClient;
         const results = await deletePipelines(client.callAsCurrentUser, request.body.pipelineIds);
 
         return response.ok({ body: { results } });

x-pack/plugins/logstash/server/routes/pipelines/list.ts

@@ -5,8 +5,8 @@
  */
 import { i18n } from '@kbn/i18n';
 import { SearchResponse } from 'elasticsearch';
-import { APICaller, ICustomClusterClient, IRouter } from 'src/core/server';
-import { licenseCheckerRouteHandlerWrapper } from '../../../../licensing/server';
+import { APICaller, IRouter } from 'src/core/server';
+import { wrapRouteWithLicenseCheck } from '../../../../licensing/server';
 
 import { INDEX_NAMES, ES_SCROLL_SETTINGS } from '../../../common/constants';
 import { PipelineListItem } from '../../models/pipeline_list_item';
@@ -27,17 +27,17 @@ async function fetchPipelines(callWithRequest: APICaller) {
   return fetchAllFromScroll(response, callWithRequest);
 }
 
-export function registerPipelinesListRoute(router: IRouter, esClient: ICustomClusterClient) {
+export function registerPipelinesListRoute(router: IRouter) {
   router.get(
     {
       path: '/api/logstash/pipelines',
       validate: false,
     },
-    licenseCheckerRouteHandlerWrapper(
+    wrapRouteWithLicenseCheck(
       checkLicense,
       router.handleLegacyErrors(async (context, request, response) => {
         try {
-          const client = esClient.asScoped(request);
+          const client = context.logstash!.esClient;
           const pipelinesHits = await fetchPipelines(client.callAsCurrentUser);
 
           const pipelines = pipelinesHits.map(pipeline => {