[8.15] [Http] Gracefully onboarding internal routes to VersionedRouter (#194789)

packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts

@@ -21,10 +21,10 @@ import type {
   VersionedRouteConfig,
   IKibanaResponse,
   RouteConfigOptions,
+  RouteMethod,
 } from '@kbn/core-http-server';
 import type { Mutable } from 'utility-types';
-import type { Method, VersionedRouterRoute } from './types';
-import type { CoreVersionedRouter } from './core_versioned_router';
+import type { HandlerResolutionStrategy, Method, VersionedRouterRoute } from './types';
 
 import { validate } from './validate';
 import {
@@ -38,9 +38,16 @@ import { injectResponseHeaders } from './inject_response_headers';
 
 import { resolvers } from './handler_resolvers';
 import { prepareVersionedRouteValidation, unwrapVersionedResponseBodyValidation } from './util';
+import { Router } from '../router';
 
 type Options = AddVersionOpts<unknown, unknown, unknown>;
 
+interface InternalVersionedRouteConfig<M extends RouteMethod> extends VersionedRouteConfig<M> {
+  isDev: boolean;
+  useVersionResolutionStrategyForInternalPaths: Map<string, boolean>;
+  defaultHandlerResolutionStrategy: HandlerResolutionStrategy;
+}
+
 // This validation is a pass-through so that we can apply our version-specific validation later
 export const passThroughValidation = {
   body: schema.nullable(schema.any()),
@@ -69,27 +76,41 @@ export class CoreVersionedRoute implements VersionedRoute {
     path,
     options,
   }: {
-    router: CoreVersionedRouter;
+    router: Router;
     method: Method;
     path: string;
-    options: VersionedRouteConfig<Method>;
+    options: InternalVersionedRouteConfig<Method>;
   }) {
     return new CoreVersionedRoute(router, method, path, options);
   }
 
+  public readonly options: VersionedRouteConfig<Method>;
+
   private useDefaultStrategyForPath: boolean;
   private isPublic: boolean;
+  private isDev: boolean;
   private enableQueryVersion: boolean;
+  private defaultHandlerResolutionStrategy: HandlerResolutionStrategy;
   private constructor(
-    private readonly router: CoreVersionedRouter,
+    private readonly router: Router,
     public readonly method: Method,
     public readonly path: string,
-    public readonly options: VersionedRouteConfig<Method>
+    internalOptions: InternalVersionedRouteConfig<Method>
   ) {
-    this.useDefaultStrategyForPath = router.useVersionResolutionStrategyForInternalPaths.has(path);
-    this.isPublic = this.options.access === 'public';
-    this.enableQueryVersion = this.options.enableQueryVersion === true;
-    this.router.router[this.method](
+    const {
+      isDev,
+      useVersionResolutionStrategyForInternalPaths,
+      defaultHandlerResolutionStrategy,
+      ...options
+    } = internalOptions;
+    this.isPublic = options.access === 'public';
+    this.isDev = isDev;
+    this.defaultHandlerResolutionStrategy = defaultHandlerResolutionStrategy;
+    this.useDefaultStrategyForPath =
+      this.isPublic || useVersionResolutionStrategyForInternalPaths.has(path);
+    this.enableQueryVersion = options.enableQueryVersion === true;
+    this.options = options;
+    this.router[this.method](
       {
         path: this.path,
         validate: passThroughValidation,
@@ -110,7 +131,7 @@ export class CoreVersionedRoute implements VersionedRoute {
 
   /** This method assumes that one or more versions handlers are registered  */
   private getDefaultVersion(): undefined | ApiVersion {
-    return resolvers[this.router.defaultHandlerResolutionStrategy](
+    return resolvers[this.defaultHandlerResolutionStrategy](
       [...this.handlers.keys()],
       this.options.access
     );
@@ -120,6 +141,24 @@ export class CoreVersionedRoute implements VersionedRoute {
     return this.handlers.size ? '[' + [...this.handlers.keys()].join(', ') + ']' : '<none>';
   }
 
+  private getVersion(req: KibanaRequest): ApiVersion | undefined {
+    let version;
+    const maybeVersion = readVersion(req, this.enableQueryVersion);
+    if (!maybeVersion) {
+      if (this.useDefaultStrategyForPath) {
+        version = this.getDefaultVersion();
+      } else if (!this.isDev && !this.isPublic) {
+        // When in production, we default internal routes to v1 to allow
+        // gracefully onboarding of un-versioned to versioned routes
+        version = '1';
+      }
+    } else {
+      version = maybeVersion;
+    }
+
+    return version;
+  }
+
   private requestHandler = async (
     ctx: RequestHandlerContextBase,
     originalReq: KibanaRequest,
@@ -132,14 +171,7 @@ export class CoreVersionedRoute implements VersionedRoute {
       });
     }
     const req = originalReq as Mutable<KibanaRequest>;
-    let version: undefined | ApiVersion;
-
-    const maybeVersion = readVersion(req, this.enableQueryVersion);
-    if (!maybeVersion && (this.isPublic || this.useDefaultStrategyForPath)) {
-      version = this.getDefaultVersion();
-    } else {
-      version = maybeVersion;
-    }
+    const version = this.getVersion(req);
     if (!version) {
       return res.badRequest({
         body: `Please specify a version via ${ELASTIC_HTTP_VERSION_HEADER} header. Available versions: ${this.versionsToString()}`,
@@ -192,7 +224,7 @@ export class CoreVersionedRoute implements VersionedRoute {
 
     const response = await handler.fn(ctx, req, res);
 
-    if (this.router.isDev && validation?.response?.[response.status]?.body) {
+    if (this.isDev && validation?.response?.[response.status]?.body) {
       const { [response.status]: responseValidation, unsafe } = validation.response;
       try {
         validate(
@@ -221,7 +253,7 @@ export class CoreVersionedRoute implements VersionedRoute {
   private validateVersion(version: string) {
     // We do an additional check here while we only have a single allowed public version
     // for all public Kibana HTTP APIs
-    if (this.router.isDev && this.isPublic) {
+    if (this.isDev && this.isPublic) {
       const message = isAllowedPublicVersion(version);
       if (message) {
         throw new Error(message);

packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts

@@ -73,10 +73,16 @@ export class CoreVersionedRouter implements VersionedRouter {
     (routeMethod: Method) =>
     (options: VersionedRouteConfig<Method>): VersionedRoute<Method, any> => {
       const route = CoreVersionedRoute.from({
-        router: this,
+        router: this.router,
         method: routeMethod,
         path: options.path,
-        options,
+        options: {
+          ...options,
+          defaultHandlerResolutionStrategy: this.defaultHandlerResolutionStrategy,
+          useVersionResolutionStrategyForInternalPaths:
+            this.useVersionResolutionStrategyForInternalPaths,
+          isDev: this.isDev,
+        },
       });
       this.routes.add(route);
       return route;

src/core/server/integration_tests/http/versioned_router.test.ts

@@ -292,38 +292,46 @@ describe('Routing versioned requests', () => {
     ).resolves.toEqual('1');
   });
 
-  it('requires version headers to be set for internal HTTP APIs', async () => {
+  it('defaults to v1 for internal HTTP APIs to allow gracefully onboarding internal routes to versioned router', async () => {
     await setupServer({ dev: false });
 
     router.versioned
-      .get({ path: '/my-path', access: 'internal' })
+      .get({ path: '/my-internal-path', access: 'internal' })
       .addVersion(
         { version: '1', validate: { response: { 200: { body: () => schema.number() } } } },
-        async (ctx, req, res) => res.ok()
+        async (ctx, req, res) => res.ok({ body: 'v1' })
       )
       .addVersion(
         { version: '2', validate: { response: { 200: { body: () => schema.number() } } } },
-        async (ctx, req, res) => res.ok()
+        async (ctx, req, res) => res.ok({ body: 'v2' })
       );
     await server.start();
 
     await expect(
       supertest
-        .get('/my-path')
+        .get('/my-internal-path')
         .unset('Elastic-Api-Version')
-        .expect(400)
-        .then(({ body }) => body.message)
-    ).resolves.toMatch(/Please specify.+version/);
+        .expect(200)
+        .then(({ text }) => text)
+    ).resolves.toMatch('v1');
   });
 
-  it('requires version headers to be set for public endpoints when in dev', async () => {
+  it('requires version headers to be set for internal and public endpoints when in dev', async () => {
     await setupServer({ dev: true });
     router.versioned
       .get({
         path: '/my-path',
         access: 'public',
       })
       .addVersion({ version: '2023-10-31', validate: false }, async (ctx, req, res) => res.ok());
+
+    router.versioned
+      .get({
+        path: '/my-internal-path',
+        access: 'internal',
+      })
+      .addVersion({ version: '1', validate: false }, async (ctx, req, res) => res.ok());
+
     await server.start();
 
     await expect(
@@ -333,6 +341,18 @@ describe('Routing versioned requests', () => {
         .expect(400)
         .then(({ body }) => body.message)
     ).resolves.toMatch(/Please specify.+version/);
+
+    await supertest.get('/my-path').set('Elastic-Api-Version', '2023-10-31').expect(200);
+
+    await expect(
+      supertest
+        .get('/my-internal-path')
+        .unset('Elastic-Api-Version')
+        .expect(400)
+        .then(({ body }) => body.message)
+    ).resolves.toMatch(/Please specify.+version/);
+
+    await supertest.get('/my-internal-path').set('Elastic-Api-Version', '1').expect(200);
   });
 
   it('errors when no handler could be found', async () => {

x-pack/test/kubernetes_security/basic/tests/aggregate.ts

@@ -122,13 +122,13 @@ export default function aggregateTests({ getService }: FtrProviderContext) {
     });
 
     it(`${AGGREGATE_ROUTE} handles a bad request`, async () => {
-      const response = await supertest.get(AGGREGATE_ROUTE).set('kbn-xsrf', 'foo').query({
+      const response = await getRoute().query({
         query: 'asdf',
         groupBy: ORCHESTRATOR_NAMESPACE,
         page: 0,
         index: MOCK_INDEX,
       });
-      expect(response.status).to.be(400);
+      expect(response.status).to.be(500);
     });
   });
 }