[8.x] [Http] Gracefully onboarding internal routes to `VersionedRouter` (#194789) (#194902)

# Backport This will backport the following commits from `main` to `8.x`: - [[Http] Gracefully onboarding internal routes to `VersionedRouter` (#194789)](https://github.com/elastic/kibana/pull/194789)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jean-Louis Leysens <jeanlouis.leysens@elastic.co>
2025-04-24 01:38:56 -04:00 · 2024-10-04 18:34:06 +10:00 · 2024-10-04 18:34:06 +10:00 · bc434697c9
commit bc434697c9
parent 18b2b5c371
5 changed files with 84 additions and 31 deletions
--- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts
+++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.test.ts
@ -548,7 +548,7 @@ describe('Versioned route', () => {
    expect(
      // @ts-expect-error
      versionedRoute.getSecurity({
-        headers: {},
+        headers: { [ELASTIC_HTTP_VERSION_HEADER]: '99' },
      })
    ).toStrictEqual(securityConfigDefault);

@ -569,7 +569,7 @@ describe('Versioned route', () => {
    expect(
      // @ts-expect-error
      versionedRoute.getSecurity({
-        headers: {},
+        headers: { [ELASTIC_HTTP_VERSION_HEADER]: '99' },
      })
    ).toStrictEqual(securityConfigDefault);
    expect(router.get).toHaveBeenCalledTimes(1);
--- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts
+++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_route.ts
@ -25,10 +25,10 @@ import type {
  RouteConfigOptions,
  RouteSecurityGetter,
  RouteSecurity,
+  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 {
@ -44,9 +44,16 @@ import { validRouteSecurity } from '../security_route_config_validator';
 import { resolvers } from './handler_resolvers';
 import { prepareVersionedRouteValidation, unwrapVersionedResponseBodyValidation } from './util';
 import type { RequestLike } from './route_version_utils';
+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()),
@ -75,29 +82,43 @@ 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 defaultSecurityConfig: RouteSecurity | undefined;
+  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.defaultSecurityConfig = validRouteSecurity(this.options.security, this.options.options);
-    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.defaultSecurityConfig = validRouteSecurity(options.security, options.options);
+    this.options = options;
+    this.router[this.method](
      {
        path: this.path,
        validate: passThroughValidation,
@ -119,7 +140,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
    );
@ -132,8 +153,14 @@ export class CoreVersionedRoute implements VersionedRoute {
  private getVersion(req: RequestLike): ApiVersion | undefined {
    let version;
    const maybeVersion = readVersion(req, this.enableQueryVersion);
-    if (!maybeVersion && (this.isPublic || this.useDefaultStrategyForPath)) {
-      version = this.getDefaultVersion();
+    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;
    }
@ -207,7 +234,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(
@ -236,7 +263,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);
--- a/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts
+++ b/packages/core/http/core-http-router-server-internal/src/versioned_router/core_versioned_router.ts
@ -74,10 +74,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;
--- a/src/core/server/integration_tests/http/versioned_router.test.ts
+++ b/src/core/server/integration_tests/http/versioned_router.test.ts
@ -293,31 +293,31 @@ 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({
@ -325,6 +325,14 @@ describe('Routing versioned requests', () => {
        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(
@ -334,6 +342,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 () => {
--- a/x-pack/test/kubernetes_security/basic/tests/aggregate.ts
+++ b/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);
    });
  });
 }