From 0f67c7865955033585c54976d78bc91b62d7a23c Mon Sep 17 00:00:00 2001 From: Jesus Wahrman <41008968+jesuswr@users.noreply.github.com> Date: Mon, 20 Jan 2025 13:40:53 +0100 Subject: [PATCH] [core.http] Add warning header to deprecated endpoints (#205926) ## Summary resolves https://github.com/elastic/kibana/issues/105692 This PR adds a pre response handler that sets a warning header if the requested endpoint is deprecated. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine --- .../http/router-server-internal/index.ts | 1 + .../src/get_warning_header_message.test.ts | 41 ++++ .../src/get_warning_header_message.ts | 19 ++ .../router-server-internal/src/router.test.ts | 8 +- .../http/router-server-internal/src/router.ts | 8 +- .../core_versioned_route.test.ts | 17 +- .../versioned_router/core_versioned_route.ts | 33 ++- .../core_versioned_router.test.ts | 2 + .../versioned_router/core_versioned_router.ts | 11 +- .../http/router-server-internal/tsconfig.json | 4 +- .../server-internal/src/http_server.test.ts | 7 +- .../server-internal/src/http_service.test.ts | 2 +- .../http/server-internal/src/http_service.ts | 4 +- .../src/lifecycle/on_pre_response.ts | 6 +- .../src/lifecycle_handlers.test.ts | 45 ++++ .../server-internal/src/lifecycle_handlers.ts | 23 ++- .../src/register_lifecycle_handlers.ts | 5 + .../server/src/lifecycle/on_pre_response.ts | 2 + .../packages/http/server/src/router/route.ts | 2 + .../http/http2_protocol.test.ts | 9 +- .../http/http_server.test.ts | 7 +- .../integration_tests/http/lifecycle.test.ts | 194 ++++++++++++++++++ .../integration_tests/http/router.test.ts | 9 +- .../http/tls_config_reload.test.ts | 7 +- 24 files changed, 427 insertions(+), 39 deletions(-) create mode 100644 src/core/packages/http/router-server-internal/src/get_warning_header_message.test.ts create mode 100644 src/core/packages/http/router-server-internal/src/get_warning_header_message.ts diff --git a/src/core/packages/http/router-server-internal/index.ts b/src/core/packages/http/router-server-internal/index.ts index f0cd26b1ba90..1d3e01675850 100644 --- a/src/core/packages/http/router-server-internal/index.ts +++ b/src/core/packages/http/router-server-internal/index.ts @@ -21,3 +21,4 @@ export { isKibanaRequest, isRealRequest, ensureRawRequest, CoreKibanaRequest } f export { isSafeMethod } from './src/route'; export { HapiResponseAdapter } from './src/response_adapter'; export { kibanaResponseFactory, lifecycleResponseFactory, KibanaResponse } from './src/response'; +export { getWarningHeaderMessageFromRouteDeprecation } from './src/get_warning_header_message'; diff --git a/src/core/packages/http/router-server-internal/src/get_warning_header_message.test.ts b/src/core/packages/http/router-server-internal/src/get_warning_header_message.test.ts new file mode 100644 index 000000000000..a58907af4152 --- /dev/null +++ b/src/core/packages/http/router-server-internal/src/get_warning_header_message.test.ts @@ -0,0 +1,41 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { RouteDeprecationInfo } from '@kbn/core-http-server'; +import { getWarningHeaderMessageFromRouteDeprecation } from './get_warning_header_message'; + +describe('getWarningHeaderMessageFromRouteDeprecation', () => { + it('creates the warning with a default message if the deprecation object does not have one', () => { + const kibanaVersion = '12.31.45'; + const expectedMessage = `299 Kibana-${kibanaVersion} "This endpoint deprecated"`; + const deprecationObject: RouteDeprecationInfo = { + reason: { type: 'deprecate' }, + severity: 'warning', + documentationUrl: 'fakeurl.com', + }; + expect(getWarningHeaderMessageFromRouteDeprecation(deprecationObject, expectedMessage)).toMatch( + expectedMessage + ); + }); + + it('creates the warning with the deprecation object message', () => { + const kibanaVersion = '12.31.45'; + const msg = 'Custom deprecation message for this object'; + const expectedMessage = `299 Kibana-${kibanaVersion} "${msg}"`; + const deprecationObject: RouteDeprecationInfo = { + reason: { type: 'deprecate' }, + severity: 'warning', + documentationUrl: 'fakeurl.com', + message: msg, + }; + expect(getWarningHeaderMessageFromRouteDeprecation(deprecationObject, expectedMessage)).toMatch( + expectedMessage + ); + }); +}); diff --git a/src/core/packages/http/router-server-internal/src/get_warning_header_message.ts b/src/core/packages/http/router-server-internal/src/get_warning_header_message.ts new file mode 100644 index 000000000000..bc69a4d5c0cc --- /dev/null +++ b/src/core/packages/http/router-server-internal/src/get_warning_header_message.ts @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import type { RouteDeprecationInfo } from '@kbn/core-http-server'; + +export function getWarningHeaderMessageFromRouteDeprecation( + deprecationObject: RouteDeprecationInfo, + kibanaVersion: string +): string { + const msg = deprecationObject.message ?? 'This endpoint is deprecated'; + const warningMessage = `299 Kibana-${kibanaVersion} "${msg}"`; + return warningMessage; +} diff --git a/src/core/packages/http/router-server-internal/src/router.test.ts b/src/core/packages/http/router-server-internal/src/router.test.ts index ee1b3d234b71..54e5e11c6252 100644 --- a/src/core/packages/http/router-server-internal/src/router.test.ts +++ b/src/core/packages/http/router-server-internal/src/router.test.ts @@ -14,6 +14,7 @@ import { createRequestMock } from '@kbn/hapi-mocks/src/request'; import { createFooValidation } from './router.test.util'; import { Router, type RouterOptions } from './router'; import type { RouteValidatorRequestAndResponses } from '@kbn/core-http-server'; +import { getEnvOptions, createTestEnv } from '@kbn/config-mocks'; const mockResponse = { code: jest.fn().mockImplementation(() => mockResponse), @@ -26,9 +27,12 @@ const mockResponseToolkit = { const logger = loggingSystemMock.create().get(); const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {}); +const options = getEnvOptions(); +options.cliArgs.dev = false; +const env = createTestEnv({ envOptions: options }); const routerOptions: RouterOptions = { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', useVersionResolutionStrategyForInternalPaths: [], @@ -273,7 +277,7 @@ describe('Router', () => { it('registers pluginId if provided', () => { const pluginId = Symbol('test'); - const router = new Router('', logger, enhanceWithContext, { pluginId }); + const router = new Router('', logger, enhanceWithContext, { pluginId, env }); expect(router.pluginId).toBe(pluginId); }); diff --git a/src/core/packages/http/router-server-internal/src/router.ts b/src/core/packages/http/router-server-internal/src/router.ts index fcf70341b0c4..df1675a03b6a 100644 --- a/src/core/packages/http/router-server-internal/src/router.ts +++ b/src/core/packages/http/router-server-internal/src/router.ts @@ -30,6 +30,7 @@ import type { IKibanaResponse, } from '@kbn/core-http-server'; import type { RouteSecurityGetter } from '@kbn/core-http-server'; +import { Env } from '@kbn/config'; import { CoreVersionedRouter } from './versioned_router'; import { CoreKibanaRequest, getProtocolFromRequest } from './request'; import { kibanaResponseFactory } from './response'; @@ -72,8 +73,7 @@ export type InternalRouterRoute = Omit & { /** @internal */ export interface RouterOptions { - /** Whether we are running in development */ - isDev?: boolean; + env: Env; /** Plugin for which this router was registered */ pluginId?: symbol; @@ -203,7 +203,7 @@ export class Router { let router: Router; @@ -33,6 +39,7 @@ describe('Versioned route', () => { versionedRouter = CoreVersionedRouter.from({ router, log: loggingSystemMock.createLogger(), + env: notDevEnv, }); }); @@ -198,7 +205,7 @@ describe('Versioned route', () => { it('allows public versions other than "2023-10-31"', () => { expect(() => - CoreVersionedRouter.from({ router, log: loggingSystemMock.createLogger(), isDev: false }) + CoreVersionedRouter.from({ router, log: loggingSystemMock.createLogger(), env: notDevEnv }) .get({ access: 'public', path: '/foo' }) .addVersion({ version: '2023-01-31', validate: false }, (ctx, req, res) => res.ok()) ).not.toThrow(); @@ -297,7 +304,7 @@ describe('Versioned route', () => { beforeEach(() => { versionedRouter = CoreVersionedRouter.from({ router, - isDev: true, + env: devEnv, log: loggingSystemMock.createLogger(), }); }); @@ -346,7 +353,7 @@ describe('Versioned route', () => { (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); versionedRouter = CoreVersionedRouter.from({ router, - isDev: true, + env: devEnv, log: loggingSystemMock.createLogger(), }); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( @@ -379,7 +386,7 @@ describe('Versioned route', () => { (router.registerRoute as jest.Mock).mockImplementation((opts) => (handler = opts.handler)); versionedRouter = CoreVersionedRouter.from({ router, - isDev: true, + env: devEnv, log: loggingSystemMock.createLogger(), }); versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion( @@ -411,7 +418,7 @@ describe('Versioned route', () => { it('allows using default resolution for specific internal routes', async () => { versionedRouter = CoreVersionedRouter.from({ router, - isDev: true, + env: devEnv, log: loggingSystemMock.createLogger(), useVersionResolutionStrategyForInternalPaths: ['/bypass_me/{id?}'], }); diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts index 04560c3e76e5..a1243fd8c4b0 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_route.ts @@ -25,6 +25,7 @@ import type { } from '@kbn/core-http-server'; import { Request } from '@hapi/hapi'; import { Logger } from '@kbn/logging'; +import { Env } from '@kbn/config'; import type { HandlerResolutionStrategy, Method, Options } from './types'; import { @@ -34,7 +35,7 @@ import { readVersion, removeQueryVersion, } from './route_version_utils'; -import { injectVersionHeader } from '../util'; +import { injectResponseHeaders, injectVersionHeader } from '../util'; import { validRouteSecurity } from '../security_route_config_validator'; import { resolvers } from './handler_resolvers'; @@ -44,9 +45,10 @@ import { RequestHandlerEnhanced, Router } from '../router'; import { kibanaResponseFactory as responseFactory } from '../response'; import { validateHapiRequest } from '../route'; import { RouteValidator } from '../validator'; +import { getWarningHeaderMessageFromRouteDeprecation } from '../get_warning_header_message'; interface InternalVersionedRouteConfig extends VersionedRouteConfig { - isDev: boolean; + env: Env; useVersionResolutionStrategyForInternalPaths: Map; defaultHandlerResolutionStrategy: HandlerResolutionStrategy; } @@ -86,7 +88,7 @@ export class CoreVersionedRoute implements VersionedRoute { private useDefaultStrategyForPath: boolean; private isPublic: boolean; - private isDev: boolean; + private env: Env; private enableQueryVersion: boolean; private defaultSecurityConfig: RouteSecurity | undefined; private defaultHandlerResolutionStrategy: HandlerResolutionStrategy; @@ -98,13 +100,13 @@ export class CoreVersionedRoute implements VersionedRoute { internalOptions: InternalVersionedRouteConfig ) { const { - isDev, + env, useVersionResolutionStrategyForInternalPaths, defaultHandlerResolutionStrategy, ...options } = internalOptions; this.isPublic = options.access === 'public'; - this.isDev = isDev; + this.env = env; this.defaultHandlerResolutionStrategy = defaultHandlerResolutionStrategy; this.useDefaultStrategyForPath = this.isPublic || useVersionResolutionStrategyForInternalPaths.has(path); @@ -146,7 +148,7 @@ export class CoreVersionedRoute implements VersionedRoute { if (!maybeVersion) { if (this.useDefaultStrategyForPath) { version = this.getDefaultVersion(); - } else if (!this.isDev && !this.isPublic) { + } else if (!this.env.mode.dev && !this.isPublic) { // When in production, we default internal routes to v1 to allow // gracefully onboarding of un-versioned to versioned routes version = '1'; @@ -211,9 +213,22 @@ export class CoreVersionedRoute implements VersionedRoute { return injectVersionHeader(version, error); } - const response = await handler.fn(kibanaRequest, responseFactory); + let response = await handler.fn(kibanaRequest, responseFactory); - if (this.isDev && validation?.response?.[response.status]?.body) { + // we don't want to overwrite the header value + if (handler.options.options?.deprecated && !response.options.headers?.warning) { + response = injectResponseHeaders( + { + warning: getWarningHeaderMessageFromRouteDeprecation( + handler.options.options.deprecated, + this.env.packageInfo.version + ), + }, + response + ); + } + + if (this.env.mode.dev && validation?.response?.[response.status]?.body) { const { [response.status]: responseValidation, unsafe } = validation.response; try { const validator = RouteValidator.from({ @@ -235,7 +250,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.isDev && this.isPublic) { + if (this.env.mode.dev && this.isPublic) { const message = isAllowedPublicVersion(version); if (message) { throw new Error(message); diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.test.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.test.ts index 7b9fbbf93880..bba25a4a9799 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.test.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.test.ts @@ -11,6 +11,7 @@ import { loggingSystemMock } from '@kbn/core-logging-server-mocks'; import { Router } from '../router'; import { CoreVersionedRouter } from '.'; import { createRouter } from './mocks'; +import { createTestEnv } from '@kbn/config-mocks'; const pluginId = Symbol('test'); describe('Versioned router', () => { @@ -21,6 +22,7 @@ describe('Versioned router', () => { versionedRouter = CoreVersionedRouter.from({ router, log: loggingSystemMock.createLogger(), + env: createTestEnv(), }); }); diff --git a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.ts b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.ts index 0570ac3cf099..16b5b60e9598 100644 --- a/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.ts +++ b/src/core/packages/http/router-server-internal/src/versioned_router/core_versioned_router.ts @@ -15,6 +15,7 @@ import type { } from '@kbn/core-http-server'; import { omit } from 'lodash'; import { Logger } from '@kbn/logging'; +import { Env } from '@kbn/config'; import { CoreVersionedRoute } from './core_versioned_route'; import type { HandlerResolutionStrategy, Method } from './types'; import type { Router } from '../router'; @@ -30,7 +31,7 @@ export interface VersionedRouterArgs { */ defaultHandlerResolutionStrategy?: HandlerResolutionStrategy; /** Whether Kibana is running in a dev environment */ - isDev?: boolean; + env: Env; /** * List of internal paths that should use the default handler resolution strategy. By default this * is no routes ([]) because ONLY Elastic clients are intended to call internal routes. @@ -57,14 +58,14 @@ export class CoreVersionedRouter implements VersionedRouter { router, log, defaultHandlerResolutionStrategy, - isDev, + env, useVersionResolutionStrategyForInternalPaths, }: VersionedRouterArgs) { return new CoreVersionedRouter( router, log, defaultHandlerResolutionStrategy, - isDev, + env, useVersionResolutionStrategyForInternalPaths ); } @@ -72,7 +73,7 @@ export class CoreVersionedRouter implements VersionedRouter { public readonly router: Router, private readonly log: Logger, public readonly defaultHandlerResolutionStrategy: HandlerResolutionStrategy = 'oldest', - public readonly isDev: boolean = false, + public readonly env: Env, useVersionResolutionStrategyForInternalPaths: string[] = [] ) { this.pluginId = this.router.pluginId; @@ -94,7 +95,7 @@ export class CoreVersionedRouter implements VersionedRouter { defaultHandlerResolutionStrategy: this.defaultHandlerResolutionStrategy, useVersionResolutionStrategyForInternalPaths: this.useVersionResolutionStrategyForInternalPaths, - isDev: this.isDev, + env: this.env, }, }); this.routes.add(route); diff --git a/src/core/packages/http/router-server-internal/tsconfig.json b/src/core/packages/http/router-server-internal/tsconfig.json index 7887583e6e46..15bac8a66e44 100644 --- a/src/core/packages/http/router-server-internal/tsconfig.json +++ b/src/core/packages/http/router-server-internal/tsconfig.json @@ -19,7 +19,9 @@ "@kbn/core-logging-server-mocks", "@kbn/logging", "@kbn/core-http-common", - "@kbn/logging-mocks" + "@kbn/logging-mocks", + "@kbn/config-mocks", + "@kbn/config" ], "exclude": [ "target/**/*", diff --git a/src/core/packages/http/server-internal/src/http_server.test.ts b/src/core/packages/http/server-internal/src/http_server.test.ts index 72a94177233b..9b5c1c392935 100644 --- a/src/core/packages/http/server-internal/src/http_server.test.ts +++ b/src/core/packages/http/server-internal/src/http_server.test.ts @@ -32,9 +32,14 @@ import { KBN_CERT_PATH, KBN_KEY_PATH } from '@kbn/dev-utils'; import moment from 'moment'; import { of, Observable, BehaviorSubject } from 'rxjs'; import { mockCoreContext } from '@kbn/core-base-server-mocks'; +import { createTestEnv, getEnvOptions } from '@kbn/config-mocks'; + +const options = getEnvOptions(); +options.cliArgs.dev = false; +const env = createTestEnv({ envOptions: options }); const routerOptions: RouterOptions = { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', useVersionResolutionStrategyForInternalPaths: [], diff --git a/src/core/packages/http/server-internal/src/http_service.test.ts b/src/core/packages/http/server-internal/src/http_service.test.ts index 9740cb023335..e2aa841ffad3 100644 --- a/src/core/packages/http/server-internal/src/http_service.test.ts +++ b/src/core/packages/http/server-internal/src/http_service.test.ts @@ -486,7 +486,7 @@ test('passes versioned config to router', async () => { expect.any(Object), // logger expect.any(Function), // context enhancer expect.objectContaining({ - isDev: true, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'newest', useVersionResolutionStrategyForInternalPaths: ['/foo'], diff --git a/src/core/packages/http/server-internal/src/http_service.ts b/src/core/packages/http/server-internal/src/http_service.ts index 4b3f3a37f494..5d43a7a20388 100644 --- a/src/core/packages/http/server-internal/src/http_service.ts +++ b/src/core/packages/http/server-internal/src/http_service.ts @@ -147,7 +147,7 @@ export class HttpService this.log, prebootServerRequestHandlerContext.createHandler.bind(null, this.coreContext.coreId), { - isDev: this.env.mode.dev, + env: this.env, versionedRouterOptions: getVersionedRouterOptions(config), } ); @@ -196,7 +196,7 @@ export class HttpService ) => { const enhanceHandler = this.requestHandlerContext!.createHandler.bind(null, pluginId); const router = new Router(path, this.log, enhanceHandler, { - isDev: this.env.mode.dev, + env: this.env, versionedRouterOptions: getVersionedRouterOptions(config), pluginId, }); diff --git a/src/core/packages/http/server-internal/src/lifecycle/on_pre_response.ts b/src/core/packages/http/server-internal/src/lifecycle/on_pre_response.ts index 7f489e4bb5f7..f8cafcf31366 100644 --- a/src/core/packages/http/server-internal/src/lifecycle/on_pre_response.ts +++ b/src/core/packages/http/server-internal/src/lifecycle/on_pre_response.ts @@ -66,11 +66,13 @@ export function adoptToHapiOnPreResponseFormat(fn: OnPreResponseHandler, log: Lo try { if (response) { - const statusCode: number = isBoom(response) + const isResponseBoom = isBoom(response); + const statusCode: number = isResponseBoom ? response.output.statusCode : response.statusCode; + const headers: ResponseHeaders = isResponseBoom ? {} : response.headers; - const result = await fn(CoreKibanaRequest.from(request), { statusCode }, toolkit); + const result = await fn(CoreKibanaRequest.from(request), { statusCode, headers }, toolkit); if (preResponseResult.isNext(result)) { if (result.headers) { diff --git a/src/core/packages/http/server-internal/src/lifecycle_handlers.test.ts b/src/core/packages/http/server-internal/src/lifecycle_handlers.test.ts index 28f8c70ebbb1..b86b48379cf6 100644 --- a/src/core/packages/http/server-internal/src/lifecycle_handlers.test.ts +++ b/src/core/packages/http/server-internal/src/lifecycle_handlers.test.ts @@ -22,6 +22,7 @@ import { INTERNAL_API_RESTRICTED_LOGGER_NAME, createBuildNrMismatchLoggerPreResponseHandler, createCustomHeadersPreResponseHandler, + createDeprecationWarningHeaderPreResponseHandler, createRestrictInternalRoutesPostAuthHandler, createVersionCheckPostAuthHandler, createXsrfPostAuthHandler, @@ -547,6 +548,50 @@ describe('customHeaders pre-response handler', () => { }); }); +describe('deprecation header pre-response handler', () => { + let toolkit: ToolkitMock; + + beforeEach(() => { + toolkit = createToolkit(); + }); + + it('adds the deprecation warning header to the request going to a deprecated route', () => { + const kibanaVersion = '19.73.41'; + const deprecationMessage = 'This is a deprecated endpoint message in the tests'; + const warningHeader = `299 Kibana-${kibanaVersion} "${deprecationMessage}"`; + const handler = createDeprecationWarningHeaderPreResponseHandler(kibanaVersion); + + handler( + { route: { options: { deprecated: { message: deprecationMessage } } } } as any, + {} as any, + toolkit + ); + + expect(toolkit.next).toHaveBeenCalledTimes(1); + expect(toolkit.next).toHaveBeenCalledWith({ + headers: { + warning: warningHeader, + }, + }); + }); + + it('does not add the deprecation warning header to the request going to a non-deprecated route', () => { + const kibanaVersion = '19.73.41'; + const deprecationMessage = 'This is a deprecated endpoint message in the tests'; + const warningHeader = `299 Kibana-${kibanaVersion} "${deprecationMessage}"`; + const handler = createDeprecationWarningHeaderPreResponseHandler(kibanaVersion); + + handler({ route: { options: { deprecated: {} } } } as any, {} as any, toolkit); + + expect(toolkit.next).toHaveBeenCalledTimes(1); + expect(toolkit.next).not.toHaveBeenCalledWith({ + headers: { + warning: warningHeader, + }, + }); + }); +}); + describe('build number mismatch logger on error pre-response handler', () => { let logger: jest.Mocked; diff --git a/src/core/packages/http/server-internal/src/lifecycle_handlers.ts b/src/core/packages/http/server-internal/src/lifecycle_handlers.ts index c460559fcec5..1bd1466b4c3b 100644 --- a/src/core/packages/http/server-internal/src/lifecycle_handlers.ts +++ b/src/core/packages/http/server-internal/src/lifecycle_handlers.ts @@ -13,7 +13,10 @@ import type { OnPreResponseInfo, KibanaRequest, } from '@kbn/core-http-server'; -import { isSafeMethod } from '@kbn/core-http-router-server-internal'; +import { + getWarningHeaderMessageFromRouteDeprecation, + isSafeMethod, +} from '@kbn/core-http-router-server-internal'; import { Logger } from '@kbn/logging'; import { KIBANA_BUILD_NR_HEADER } from '@kbn/core-http-common'; import { HttpConfig } from './http_config'; @@ -120,6 +123,24 @@ export const createCustomHeadersPreResponseHandler = (config: HttpConfig): OnPre }; }; +export const createDeprecationWarningHeaderPreResponseHandler = ( + kibanaVersion: string +): OnPreResponseHandler => { + return (request, response, toolkit) => { + // we don't want to overwrite the header value + if (!request.route.options.deprecated || response.headers?.warning) { + return toolkit.next(); + } + const additionalHeaders = { + warning: getWarningHeaderMessageFromRouteDeprecation( + request.route.options.deprecated, + kibanaVersion + ), + }; + return toolkit.next({ headers: { ...additionalHeaders } }); + }; +}; + const shouldLogBuildNumberMismatch = ( serverBuild: { number: number; string: string }, request: KibanaRequest, diff --git a/src/core/packages/http/server-internal/src/register_lifecycle_handlers.ts b/src/core/packages/http/server-internal/src/register_lifecycle_handlers.ts index f11810f9afc3..a504bb418843 100644 --- a/src/core/packages/http/server-internal/src/register_lifecycle_handlers.ts +++ b/src/core/packages/http/server-internal/src/register_lifecycle_handlers.ts @@ -17,6 +17,7 @@ import { createVersionCheckPostAuthHandler, createBuildNrMismatchLoggerPreResponseHandler, createXsrfPostAuthHandler, + createDeprecationWarningHeaderPreResponseHandler, } from './lifecycle_handlers'; export const registerCoreHandlers = ( @@ -27,6 +28,10 @@ export const registerCoreHandlers = ( ) => { // add headers based on config registrar.registerOnPreResponse(createCustomHeadersPreResponseHandler(config)); + // add headers for deprecated endpoints + registrar.registerOnPreResponse( + createDeprecationWarningHeaderPreResponseHandler(env.packageInfo.version) + ); // add extra request checks stuff registrar.registerOnPostAuth(createXsrfPostAuthHandler(config)); if (config.versioned.strictClientVersionCheck !== false) { diff --git a/src/core/packages/http/server/src/lifecycle/on_pre_response.ts b/src/core/packages/http/server/src/lifecycle/on_pre_response.ts index 8ca5e91aaa50..79cd5e24719f 100644 --- a/src/core/packages/http/server/src/lifecycle/on_pre_response.ts +++ b/src/core/packages/http/server/src/lifecycle/on_pre_response.ts @@ -65,6 +65,8 @@ export interface OnPreResponseExtensions { */ export interface OnPreResponseInfo { statusCode: number; + /** So any pre response handler can check the headers if needed, to avoid an overwrite for example */ + headers?: ResponseHeaders; } /** diff --git a/src/core/packages/http/server/src/router/route.ts b/src/core/packages/http/server/src/router/route.ts index c8d0683234dc..adce8aae39e3 100644 --- a/src/core/packages/http/server/src/router/route.ts +++ b/src/core/packages/http/server/src/router/route.ts @@ -128,6 +128,8 @@ export interface RouteDeprecationInfo { documentationUrl: string; /** * The description message to be displayed for the deprecation. + * This will also appear in the '299 Kibana-{version} {message}' header warning when someone calls the route. + * Keep the message concise to avoid long header values. It is recommended to keep the message under 255 characters. * Check the README for writing deprecations in `src/core/server/deprecations/README.mdx` */ message?: string; diff --git a/src/core/server/integration_tests/http/http2_protocol.test.ts b/src/core/server/integration_tests/http/http2_protocol.test.ts index 8f22a7a144c5..5c519ec61ad9 100644 --- a/src/core/server/integration_tests/http/http2_protocol.test.ts +++ b/src/core/server/integration_tests/http/http2_protocol.test.ts @@ -22,6 +22,11 @@ import { } from '@kbn/core-http-server-internal'; import { mockCoreContext } from '@kbn/core-base-server-mocks'; import type { Logger } from '@kbn/logging'; +import { createTestEnv, getEnvOptions } from '@kbn/config-mocks'; + +const options = getEnvOptions(); +options.cliArgs.dev = false; +const env = createTestEnv({ envOptions: options }); const CSP_CONFIG = cspConfig.schema.validate({}); const EXTERNAL_URL_CONFIG = externalUrlConfig.schema.validate({}); @@ -74,7 +79,7 @@ describe('Http2 - Smoke tests', () => { innerServerListener = innerServer.listener; const router = new Router('', logger, enhanceWithContext, { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', }, @@ -177,7 +182,7 @@ describe('Http2 - Smoke tests', () => { innerServerListener = innerServer.listener; const router = new Router('', logger, enhanceWithContext, { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', }, diff --git a/src/core/server/integration_tests/http/http_server.test.ts b/src/core/server/integration_tests/http/http_server.test.ts index 5e17cbbc9497..295932890fec 100644 --- a/src/core/server/integration_tests/http/http_server.test.ts +++ b/src/core/server/integration_tests/http/http_server.test.ts @@ -16,6 +16,11 @@ import { Router } from '@kbn/core-http-router-server-internal'; import { HttpServer, HttpConfig } from '@kbn/core-http-server-internal'; import { mockCoreContext } from '@kbn/core-base-server-mocks'; import type { Logger } from '@kbn/logging'; +import { createTestEnv, getEnvOptions } from '@kbn/config-mocks'; + +const options = getEnvOptions(); +options.cliArgs.dev = false; +const env = createTestEnv({ envOptions: options }); describe('Http server', () => { let server: HttpServer; @@ -60,7 +65,7 @@ describe('Http server', () => { innerServerListener = innerServer.listener; const router = new Router('', logger, enhanceWithContext, { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', }, diff --git a/src/core/server/integration_tests/http/lifecycle.test.ts b/src/core/server/integration_tests/http/lifecycle.test.ts index 1063f00e48ed..b97107ade0da 100644 --- a/src/core/server/integration_tests/http/lifecycle.test.ts +++ b/src/core/server/integration_tests/http/lifecycle.test.ts @@ -16,6 +16,9 @@ import { contextServiceMock } from '@kbn/core-http-context-server-mocks'; import { ensureRawRequest } from '@kbn/core-http-router-server-internal'; import { HttpService } from '@kbn/core-http-server-internal'; import { createHttpService } from '@kbn/core-http-server-mocks'; +import { Env } from '@kbn/config'; +import { REPO_ROOT } from '@kbn/repo-info'; +import { getEnvOptions } from '@kbn/config-mocks'; let server: HttpService; @@ -28,6 +31,8 @@ const setupDeps = { executionContext: executionContextServiceMock.createInternalSetupContract(), }; +const kibanaVersion = Env.createDefault(REPO_ROOT, getEnvOptions()).packageInfo.version; + beforeEach(async () => { logger = loggingSystemMock.create(); server = createHttpService({ logger }); @@ -1503,6 +1508,195 @@ describe('runs with default preResponse handlers', () => { }); }); +describe('runs with default preResponse deprecation handlers', () => { + const deprecationMessage = 'This is a deprecated endpoint for testing reasons'; + const warningString = `299 Kibana-${kibanaVersion} "${deprecationMessage}"`; + + it('should handle a deprecated route and include deprecation warning headers', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get( + { + path: '/deprecated', + validate: false, + options: { + deprecated: { + documentationUrl: 'https://fake-url.com', + reason: { type: 'deprecate' }, + severity: 'warning', + message: deprecationMessage, + }, + }, + }, + (context, req, res) => res.ok({}) + ); + + await server.start(); + + const response = await supertest(innerServer.listener).get('/deprecated').expect(200); + + expect(response.header.warning).toMatch(warningString); + }); + + it('should not add a deprecation warning header to a non deprecated route', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.get( + { + path: '/test', + validate: false, + }, + (context, req, res) => res.ok({}) + ); + + await server.start(); + + const response = await supertest(innerServer.listener).get('/test').expect(200); + + expect(response.header.warning).toBeUndefined(); + }); + + it('should not overwrite the warning header if it was already set', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + const expectedWarningHeader = 'This should not get overwritten'; + + router.get( + { + path: '/deprecated', + validate: false, + options: { + deprecated: { + documentationUrl: 'https://fake-url.com', + reason: { type: 'deprecate' }, + severity: 'warning', + message: deprecationMessage, + }, + }, + }, + (context, req, res) => res.ok({ headers: { warning: expectedWarningHeader } }) + ); + + await server.start(); + + const response = await supertest(innerServer.listener).get('/deprecated').expect(200); + expect(response.header.warning).toMatch(expectedWarningHeader); + }); + + it('should return the warning header in deprecated v1 but not in non deprecated v2', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + + router.versioned + .get({ + access: 'internal', + path: '/test', + }) + .addVersion( + { + version: '1', + validate: false, + options: { + deprecated: { + documentationUrl: 'https://fake-url.com', + reason: { type: 'deprecate' }, + severity: 'warning', + message: deprecationMessage, + }, + }, + }, + async (ctx, req, res) => { + return res.ok({ body: { v: '1' } }); + } + ) + .addVersion( + { + version: '2', + validate: false, + }, + async (ctx, req, res) => { + return res.ok({ body: { v: '2' } }); + } + ); + + await server.start(); + + let response = await supertest(innerServer.listener) + .get('/test') + .set('Elastic-Api-Version', '1') + .expect(200); + + expect(response.body.v).toMatch('1'); + expect(response.header.warning).toMatch(warningString); + + response = await supertest(innerServer.listener) + .get('/test') + .set('Elastic-Api-Version', '2') + .expect(200); + + expect(response.body.v).toMatch('2'); + expect(response.header.warning).toBeUndefined(); + }); + + it('should not overwrite the warning header if it was already set (versioned)', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + const expectedWarningHeader = 'This should not get overwritten'; + + router.versioned + .get({ + access: 'internal', + path: '/test', + }) + .addVersion( + { + version: '1', + validate: false, + options: { + deprecated: { + documentationUrl: 'https://fake-url.com', + reason: { type: 'deprecate' }, + severity: 'warning', + message: deprecationMessage, + }, + }, + }, + async (ctx, req, res) => { + return res.ok({ body: { v: '1' }, headers: { warning: expectedWarningHeader } }); + } + ) + .addVersion( + { + version: '2', + validate: false, + }, + async (ctx, req, res) => { + return res.ok({ body: { v: '2' } }); + } + ); + + await server.start(); + + let response = await supertest(innerServer.listener) + .get('/test') + .set('Elastic-Api-Version', '1') + .expect(200); + + expect(response.body.v).toMatch('1'); + expect(response.header.warning).toMatch(expectedWarningHeader); + + response = await supertest(innerServer.listener) + .get('/test') + .set('Elastic-Api-Version', '2') + .expect(200); + + expect(response.body.v).toMatch('2'); + expect(response.header.warning).toBeUndefined(); + }); +}); + describe('run interceptors in the right order', () => { it('with Auth registered', async () => { const { diff --git a/src/core/server/integration_tests/http/router.test.ts b/src/core/server/integration_tests/http/router.test.ts index c0a690e479e6..b8fadbb34216 100644 --- a/src/core/server/integration_tests/http/router.test.ts +++ b/src/core/server/integration_tests/http/router.test.ts @@ -22,6 +22,11 @@ import { Router } from '@kbn/core-http-router-server-internal'; import { createHttpService } from '@kbn/core-http-server-mocks'; import type { HttpService } from '@kbn/core-http-server-internal'; import { loggerMock } from '@kbn/logging-mocks'; +import { createTestEnv, getEnvOptions } from '@kbn/config-mocks'; + +const options = getEnvOptions(); +options.cliArgs.dev = false; +const env = createTestEnv({ envOptions: options }); let server: HttpService; let logger: ReturnType; @@ -2266,7 +2271,7 @@ describe('registerRouterAfterListening', () => { const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {}); const otherRouter = new Router('/test', loggerMock.create(), enhanceWithContext, { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', }, @@ -2303,7 +2308,7 @@ describe('registerRouterAfterListening', () => { const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {}); const otherRouter = new Router('/test', loggerMock.create(), enhanceWithContext, { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', }, diff --git a/src/core/server/integration_tests/http/tls_config_reload.test.ts b/src/core/server/integration_tests/http/tls_config_reload.test.ts index c19e92361af3..205c6580d145 100644 --- a/src/core/server/integration_tests/http/tls_config_reload.test.ts +++ b/src/core/server/integration_tests/http/tls_config_reload.test.ts @@ -23,6 +23,11 @@ import { import { isServerTLS, flattenCertificateChain, fetchPeerCertificate } from './tls_utils'; import { mockCoreContext } from '@kbn/core-base-server-mocks'; import type { Logger } from '@kbn/logging'; +import { createTestEnv, getEnvOptions } from '@kbn/config-mocks'; + +const options = getEnvOptions(); +options.cliArgs.dev = false; +const env = createTestEnv({ envOptions: options }); const CSP_CONFIG = cspConfig.schema.validate({}); const EXTERNAL_URL_CONFIG = externalUrlConfig.schema.validate({}); @@ -70,7 +75,7 @@ describe('HttpServer - TLS config', () => { const listener = innerServer.listener; const router = new Router('', logger, enhanceWithContext, { - isDev: false, + env, versionedRouterOptions: { defaultHandlerResolutionStrategy: 'oldest', },