diff --git 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 index d403eeb31f6a9..dbfa302b85326 100644 --- 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 @@ -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; +interface InternalVersionedRouteConfig extends VersionedRouteConfig { + isDev: boolean; + useVersionResolutionStrategyForInternalPaths: Map; + 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; + options: InternalVersionedRouteConfig; }) { return new CoreVersionedRoute(router, method, path, options); } + public readonly options: VersionedRouteConfig; + 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 + internalOptions: InternalVersionedRouteConfig ) { - 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(', ') + ']' : ''; } + 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; - 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); diff --git 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 index 32dfee36ddf91..091a5f80cb294 100644 --- 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 @@ -73,10 +73,16 @@ export class CoreVersionedRouter implements VersionedRouter { (routeMethod: Method) => (options: VersionedRouteConfig): VersionedRoute => { 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; diff --git a/src/core/server/integration_tests/http/versioned_router.test.ts b/src/core/server/integration_tests/http/versioned_router.test.ts index 72daedb990dff..e49d3005100e7 100644 --- a/src/core/server/integration_tests/http/versioned_router.test.ts +++ b/src/core/server/integration_tests/http/versioned_router.test.ts @@ -292,31 +292,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({ @@ -324,6 +324,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( @@ -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 () => { diff --git a/x-pack/test/kubernetes_security/basic/tests/aggregate.ts b/x-pack/test/kubernetes_security/basic/tests/aggregate.ts index f9e03ea042703..e5d72ed038b42 100644 --- 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); }); }); }