diff --git a/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.bypasserrorformat.md b/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.bypasserrorformat.md new file mode 100644 index 0000000000000..bbd97ab517d29 --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.bypasserrorformat.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [CustomHttpResponseOptions](./kibana-plugin-core-server.customhttpresponseoptions.md) > [bypassErrorFormat](./kibana-plugin-core-server.customhttpresponseoptions.bypasserrorformat.md) + +## CustomHttpResponseOptions.bypassErrorFormat property + +Bypass the default error formatting + +Signature: + +```typescript +bypassErrorFormat?: boolean; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.md b/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.md index 67242bbd4e2ef..82089c831d718 100644 --- a/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.md +++ b/docs/development/core/server/kibana-plugin-core-server.customhttpresponseoptions.md @@ -17,6 +17,7 @@ export interface CustomHttpResponseOptionsT | HTTP message to send to the client | +| [bypassErrorFormat](./kibana-plugin-core-server.customhttpresponseoptions.bypasserrorformat.md) | boolean | Bypass the default error formatting | | [headers](./kibana-plugin-core-server.customhttpresponseoptions.headers.md) | ResponseHeaders | HTTP Headers with additional information about response | | [statusCode](./kibana-plugin-core-server.customhttpresponseoptions.statuscode.md) | number | | diff --git a/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.bypasserrorformat.md b/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.bypasserrorformat.md new file mode 100644 index 0000000000000..98792c47d564f --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.bypasserrorformat.md @@ -0,0 +1,13 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [HttpResponseOptions](./kibana-plugin-core-server.httpresponseoptions.md) > [bypassErrorFormat](./kibana-plugin-core-server.httpresponseoptions.bypasserrorformat.md) + +## HttpResponseOptions.bypassErrorFormat property + +Bypass the default error formatting + +Signature: + +```typescript +bypassErrorFormat?: boolean; +``` diff --git a/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.md b/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.md index 9f31e86175f79..497adc6a5ec5d 100644 --- a/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.md +++ b/docs/development/core/server/kibana-plugin-core-server.httpresponseoptions.md @@ -17,5 +17,6 @@ export interface HttpResponseOptions | Property | Type | Description | | --- | --- | --- | | [body](./kibana-plugin-core-server.httpresponseoptions.body.md) | HttpResponsePayload | HTTP message to send to the client | +| [bypassErrorFormat](./kibana-plugin-core-server.httpresponseoptions.bypasserrorformat.md) | boolean | Bypass the default error formatting | | [headers](./kibana-plugin-core-server.httpresponseoptions.headers.md) | ResponseHeaders | HTTP Headers with additional information about response | diff --git a/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js b/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js index a43d3a09c7d70..f92d01d6454d5 100644 --- a/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js +++ b/packages/kbn-test/src/functional_tests/lib/run_kibana_server.js @@ -38,7 +38,7 @@ export async function runKibanaServer({ procs, config, options }) { ...extendNodeOptions(installDir), }, cwd: installDir || KIBANA_ROOT, - wait: /http server running/, + wait: /\[Kibana\]\[http\] http server running/, }); } diff --git a/packages/kbn-test/src/kbn_client/kbn_client_requester.ts b/packages/kbn-test/src/kbn_client/kbn_client_requester.ts index 31cd3a6899568..af75137d148e9 100644 --- a/packages/kbn-test/src/kbn_client/kbn_client_requester.ts +++ b/packages/kbn-test/src/kbn_client/kbn_client_requester.ts @@ -19,6 +19,10 @@ const isConcliftOnGetError = (error: any) => { ); }; +const isIgnorableError = (error: any, ignorableErrors: number[] = []) => { + return isAxiosResponseError(error) && ignorableErrors.includes(error.response.status); +}; + export const uriencode = ( strings: TemplateStringsArray, ...values: Array @@ -53,6 +57,7 @@ export interface ReqOptions { body?: any; retries?: number; headers?: Record; + ignoreErrors?: number[]; responseType?: ResponseType; } @@ -125,6 +130,10 @@ export class KbnClientRequester { const requestedRetries = options.retries !== undefined; const failedToGetResponse = isAxiosRequestError(error); + if (isIgnorableError(error, options.ignoreErrors)) { + return error.response; + } + let errorMessage; if (conflictOnGet) { errorMessage = `Conflict on GET (path=${options.path}, attempt=${attempt}/${maxAttempts})`; diff --git a/packages/kbn-test/src/kbn_client/kbn_client_status.ts b/packages/kbn-test/src/kbn_client/kbn_client_status.ts index 7e14e58309fa2..26c46917ae8dd 100644 --- a/packages/kbn-test/src/kbn_client/kbn_client_status.ts +++ b/packages/kbn-test/src/kbn_client/kbn_client_status.ts @@ -44,6 +44,8 @@ export class KbnClientStatus { const { data } = await this.requester.request({ method: 'GET', path: 'api/status', + // Status endpoint returns 503 if any services are in an unavailable state + ignoreErrors: [503], }); return data; } diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 1a82907849cea..7624a11a6f03f 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -138,6 +138,40 @@ test('log listening address after started when configured with BasePath and rewr `); }); +test('does not allow router registration after server is listening', async () => { + expect(server.isListening()).toBe(false); + + const { registerRouter } = await server.setup(config); + + const router1 = new Router('/foo', logger, enhanceWithContext); + expect(() => registerRouter(router1)).not.toThrowError(); + + await server.start(); + + expect(server.isListening()).toBe(true); + + const router2 = new Router('/bar', logger, enhanceWithContext); + expect(() => registerRouter(router2)).toThrowErrorMatchingInlineSnapshot( + `"Routers can be registered only when HTTP server is stopped."` + ); +}); + +test('allows router registration after server is listening via `registerRouterAfterListening`', async () => { + expect(server.isListening()).toBe(false); + + const { registerRouterAfterListening } = await server.setup(config); + + const router1 = new Router('/foo', logger, enhanceWithContext); + expect(() => registerRouterAfterListening(router1)).not.toThrowError(); + + await server.start(); + + expect(server.isListening()).toBe(true); + + const router2 = new Router('/bar', logger, enhanceWithContext); + expect(() => registerRouterAfterListening(router2)).not.toThrowError(); +}); + test('valid params', async () => { const router = new Router('/foo', logger, enhanceWithContext); diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index d845ac1b639b6..8b4c3b9416152 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -33,6 +33,7 @@ import { KibanaRouteOptions, KibanaRequestState, isSafeMethod, + RouterRoute, } from './router'; import { SessionStorageCookieOptions, @@ -52,6 +53,13 @@ export interface HttpServerSetup { * @param router {@link IRouter} - a router with registered route handlers. */ registerRouter: (router: IRouter) => void; + /** + * Add all the routes registered with `router` to HTTP server request listeners. + * Unlike `registerRouter`, this function allows routes to be registered even after the server + * has started listening for requests. + * @param router {@link IRouter} - a router with registered route handlers. + */ + registerRouterAfterListening: (router: IRouter) => void; registerStaticDir: (path: string, dirPath: string) => void; basePath: HttpServiceSetup['basePath']; csp: HttpServiceSetup['csp']; @@ -114,6 +122,17 @@ export class HttpServer { this.registeredRouters.add(router); } + private registerRouterAfterListening(router: IRouter) { + if (this.isListening()) { + for (const route of router.getRoutes()) { + this.configureRoute(route); + } + } else { + // Not listening yet, add to set of registeredRouters so that it can be added after listening has started. + this.registeredRouters.add(router); + } + } + public async setup(config: HttpConfig): Promise { const serverOptions = getServerOptions(config); const listenerOptions = getListenerOptions(config); @@ -130,6 +149,7 @@ export class HttpServer { return { registerRouter: this.registerRouter.bind(this), + registerRouterAfterListening: this.registerRouterAfterListening.bind(this), registerStaticDir: this.registerStaticDir.bind(this), registerOnPreRouting: this.registerOnPreRouting.bind(this), registerOnPreAuth: this.registerOnPreAuth.bind(this), @@ -170,45 +190,7 @@ export class HttpServer { for (const router of this.registeredRouters) { for (const route of router.getRoutes()) { - this.log.debug(`registering route handler for [${route.path}]`); - // Hapi does not allow payload validation to be specified for 'head' or 'get' requests - const validate = isSafeMethod(route.method) ? undefined : { payload: true }; - const { authRequired, tags, body = {}, timeout } = route.options; - const { accepts: allow, maxBytes, output, parse } = body; - - const kibanaRouteOptions: KibanaRouteOptions = { - xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method), - }; - - this.server.route({ - handler: route.handler, - method: route.method, - path: route.path, - options: { - auth: this.getAuthOption(authRequired), - app: kibanaRouteOptions, - tags: tags ? Array.from(tags) : undefined, - // TODO: This 'validate' section can be removed once the legacy platform is completely removed. - // We are telling Hapi that NP routes can accept any payload, so that it can bypass the default - // validation applied in ./http_tools#getServerOptions - // (All NP routes are already required to specify their own validation in order to access the payload) - validate, - // @ts-expect-error Types are outdated and doesn't allow `payload.multipart` to be `true` - payload: [allow, maxBytes, output, parse, timeout?.payload].some((x) => x !== undefined) - ? { - allow, - maxBytes, - output, - parse, - timeout: timeout?.payload, - multipart: true, - } - : undefined, - timeout: { - socket: timeout?.idleSocket ?? this.config!.socketTimeout, - }, - }, - }); + this.configureRoute(route); } } @@ -486,4 +468,46 @@ export class HttpServer { options: { auth: false }, }); } + + private configureRoute(route: RouterRoute) { + this.log.debug(`registering route handler for [${route.path}]`); + // Hapi does not allow payload validation to be specified for 'head' or 'get' requests + const validate = isSafeMethod(route.method) ? undefined : { payload: true }; + const { authRequired, tags, body = {}, timeout } = route.options; + const { accepts: allow, maxBytes, output, parse } = body; + + const kibanaRouteOptions: KibanaRouteOptions = { + xsrfRequired: route.options.xsrfRequired ?? !isSafeMethod(route.method), + }; + + this.server!.route({ + handler: route.handler, + method: route.method, + path: route.path, + options: { + auth: this.getAuthOption(authRequired), + app: kibanaRouteOptions, + tags: tags ? Array.from(tags) : undefined, + // TODO: This 'validate' section can be removed once the legacy platform is completely removed. + // We are telling Hapi that NP routes can accept any payload, so that it can bypass the default + // validation applied in ./http_tools#getServerOptions + // (All NP routes are already required to specify their own validation in order to access the payload) + validate, + // @ts-expect-error Types are outdated and doesn't allow `payload.multipart` to be `true` + payload: [allow, maxBytes, output, parse, timeout?.payload].some((x) => x !== undefined) + ? { + allow, + maxBytes, + output, + parse, + timeout: timeout?.payload, + multipart: true, + } + : undefined, + timeout: { + socket: timeout?.idleSocket ?? this.config!.socketTimeout, + }, + }, + }); + } } diff --git a/src/core/server/http/http_service.test.ts b/src/core/server/http/http_service.test.ts index 83279e99bc476..ebb9ad971b848 100644 --- a/src/core/server/http/http_service.test.ts +++ b/src/core/server/http/http_service.test.ts @@ -68,20 +68,32 @@ test('creates and sets up http server', async () => { start: jest.fn(), stop: jest.fn(), }; - mockHttpServer.mockImplementation(() => httpServer); + const notReadyHttpServer = { + isListening: () => false, + setup: jest.fn().mockReturnValue({ server: fakeHapiServer }), + start: jest.fn(), + stop: jest.fn(), + }; + mockHttpServer.mockImplementationOnce(() => httpServer); + mockHttpServer.mockImplementationOnce(() => notReadyHttpServer); const service = new HttpService({ coreId, configService, env, logger }); expect(mockHttpServer.mock.instances.length).toBe(1); expect(httpServer.setup).not.toHaveBeenCalled(); + expect(notReadyHttpServer.setup).not.toHaveBeenCalled(); await service.setup(setupDeps); expect(httpServer.setup).toHaveBeenCalled(); expect(httpServer.start).not.toHaveBeenCalled(); + expect(notReadyHttpServer.setup).toHaveBeenCalled(); + expect(notReadyHttpServer.start).toHaveBeenCalled(); + await service.start(); expect(httpServer.start).toHaveBeenCalled(); + expect(notReadyHttpServer.stop).toHaveBeenCalled(); }); test('spins up notReady server until started if configured with `autoListen:true`', async () => { @@ -102,6 +114,8 @@ test('spins up notReady server until started if configured with `autoListen:true .mockImplementationOnce(() => httpServer) .mockImplementationOnce(() => ({ setup: () => ({ server: notReadyHapiServer }), + start: jest.fn(), + stop: jest.fn().mockImplementation(() => notReadyHapiServer.stop()), })); const service = new HttpService({ @@ -163,7 +177,14 @@ test('stops http server', async () => { start: noop, stop: jest.fn(), }; - mockHttpServer.mockImplementation(() => httpServer); + const notReadyHttpServer = { + isListening: () => false, + setup: jest.fn().mockReturnValue({ server: fakeHapiServer }), + start: noop, + stop: jest.fn(), + }; + mockHttpServer.mockImplementationOnce(() => httpServer); + mockHttpServer.mockImplementationOnce(() => notReadyHttpServer); const service = new HttpService({ coreId, configService, env, logger }); @@ -171,6 +192,7 @@ test('stops http server', async () => { await service.start(); expect(httpServer.stop).toHaveBeenCalledTimes(0); + expect(notReadyHttpServer.stop).toHaveBeenCalledTimes(1); await service.stop(); @@ -188,7 +210,7 @@ test('stops not ready server if it is running', async () => { isListening: () => false, setup: jest.fn().mockReturnValue({ server: mockHapiServer }), start: noop, - stop: jest.fn(), + stop: jest.fn().mockImplementation(() => mockHapiServer.stop()), }; mockHttpServer.mockImplementation(() => httpServer); @@ -198,7 +220,7 @@ test('stops not ready server if it is running', async () => { await service.stop(); - expect(mockHapiServer.stop).toHaveBeenCalledTimes(1); + expect(mockHapiServer.stop).toHaveBeenCalledTimes(2); }); test('register route handler', async () => { @@ -231,6 +253,7 @@ test('returns http server contract on setup', async () => { mockHttpServer.mockImplementation(() => ({ isListening: () => false, setup: jest.fn().mockReturnValue(httpServer), + start: noop, stop: noop, })); diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index fdf9b738a9833..0d28506607682 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -8,7 +8,6 @@ import { Observable, Subscription, combineLatest, of } from 'rxjs'; import { first, map } from 'rxjs/operators'; -import { Server } from '@hapi/hapi'; import { pick } from '@kbn/std'; import type { RequestHandlerContext } from 'src/core/server'; @@ -20,7 +19,7 @@ import { CoreContext } from '../core_context'; import { PluginOpaqueId } from '../plugins'; import { CspConfigType, config as cspConfig } from '../csp'; -import { Router } from './router'; +import { IRouter, Router } from './router'; import { HttpConfig, HttpConfigType, config as httpConfig } from './http_config'; import { HttpServer } from './http_server'; import { HttpsRedirectServer } from './https_redirect_server'; @@ -30,6 +29,7 @@ import { RequestHandlerContextProvider, InternalHttpServiceSetup, InternalHttpServiceStart, + InternalNotReadyHttpServiceSetup, } from './types'; import { registerCoreHandlers } from './lifecycle_handlers'; @@ -54,7 +54,7 @@ export class HttpService private readonly logger: LoggerFactory; private readonly log: Logger; private readonly env: Env; - private notReadyServer?: Server; + private notReadyServer?: HttpServer; private internalSetup?: InternalHttpServiceSetup; private requestHandlerContext?: RequestHandlerContextContainer; @@ -88,9 +88,7 @@ export class HttpService const config = await this.config$.pipe(first()).toPromise(); - if (this.shouldListen(config)) { - await this.runNotReadyServer(config); - } + const notReadyServer = await this.setupNotReadyService({ config, context: deps.context }); const { registerRouter, ...serverContract } = await this.httpServer.setup(config); @@ -99,6 +97,8 @@ export class HttpService this.internalSetup = { ...serverContract, + notReadyServer, + externalUrl: new ExternalUrlConfig(config.externalUrl), createRouter: ( @@ -178,14 +178,51 @@ export class HttpService await this.httpsRedirectServer.stop(); } + private async setupNotReadyService({ + config, + context, + }: { + config: HttpConfig; + context: ContextSetup; + }): Promise { + if (!this.shouldListen(config)) { + return; + } + + const notReadySetup = await this.runNotReadyServer(config); + + // We cannot use the real context container since the core services may not yet be ready + const fakeContext: RequestHandlerContextContainer = new Proxy( + context.createContextContainer(), + { + get: (target, property, receiver) => { + if (property === 'createHandler') { + return Reflect.get(target, property, receiver); + } + throw new Error(`Unexpected access from fake context: ${String(property)}`); + }, + } + ); + + return { + registerRoutes: (path: string, registerCallback: (router: IRouter) => void) => { + const router = new Router( + path, + this.log, + fakeContext.createHandler.bind(null, this.coreContext.coreId) + ); + + registerCallback(router); + notReadySetup.registerRouterAfterListening(router); + }, + }; + } + private async runNotReadyServer(config: HttpConfig) { this.log.debug('starting NotReady server'); - const httpServer = new HttpServer(this.logger, 'NotReady', of(config.shutdownTimeout)); - const { server } = await httpServer.setup(config); - this.notReadyServer = server; - // use hapi server while KibanaResponseFactory doesn't allow specifying custom headers - // https://github.com/elastic/kibana/issues/33779 - this.notReadyServer.route({ + this.notReadyServer = new HttpServer(this.logger, 'NotReady', of(config.shutdownTimeout)); + const notReadySetup = await this.notReadyServer.setup(config); + notReadySetup.server.route({ path: '/{p*}', method: '*', handler: (req, responseToolkit) => { @@ -201,5 +238,7 @@ export class HttpService }, }); await this.notReadyServer.start(); + + return notReadySetup; } } diff --git a/src/core/server/http/integration_tests/router.test.ts b/src/core/server/http/integration_tests/router.test.ts index 5b297ab44f8bb..354ab1c65d565 100644 --- a/src/core/server/http/integration_tests/router.test.ts +++ b/src/core/server/http/integration_tests/router.test.ts @@ -15,6 +15,8 @@ import { contextServiceMock } from '../../context/context_service.mock'; import { loggingSystemMock } from '../../logging/logging_system.mock'; import { createHttpServer } from '../test_utils'; import { HttpService } from '../http_service'; +import { Router } from '../router'; +import { loggerMock } from '@kbn/logging/target/mocks'; let server: HttpService; let logger: ReturnType; @@ -1836,3 +1838,57 @@ describe('ETag', () => { .expect(304, ''); }); }); + +describe('registerRouterAfterListening', () => { + it('allows a router to be registered before server has started listening', async () => { + const { server: innerServer, createRouter, registerRouterAfterListening } = await server.setup( + setupDeps + ); + const router = createRouter('/'); + + router.get({ path: '/', validate: false }, (context, req, res) => { + return res.ok({ body: 'hello' }); + }); + + const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {}); + + const otherRouter = new Router('/test', loggerMock.create(), enhanceWithContext); + otherRouter.get({ path: '/afterListening', validate: false }, (context, req, res) => { + return res.ok({ body: 'hello from other router' }); + }); + + registerRouterAfterListening(otherRouter); + + await server.start(); + + await supertest(innerServer.listener).get('/').expect(200); + await supertest(innerServer.listener).get('/test/afterListening').expect(200); + }); + + it('allows a router to be registered after server has started listening', async () => { + const { server: innerServer, createRouter, registerRouterAfterListening } = await server.setup( + setupDeps + ); + const router = createRouter('/'); + + router.get({ path: '/', validate: false }, (context, req, res) => { + return res.ok({ body: 'hello' }); + }); + + await server.start(); + + await supertest(innerServer.listener).get('/').expect(200); + await supertest(innerServer.listener).get('/test/afterListening').expect(404); + + const enhanceWithContext = (fn: (...args: any[]) => any) => fn.bind(null, {}); + + const otherRouter = new Router('/test', loggerMock.create(), enhanceWithContext); + otherRouter.get({ path: '/afterListening', validate: false }, (context, req, res) => { + return res.ok({ body: 'hello from other router' }); + }); + + registerRouterAfterListening(otherRouter); + + await supertest(innerServer.listener).get('/test/afterListening').expect(200); + }); +}); diff --git a/src/core/server/http/router/index.ts b/src/core/server/http/router/index.ts index a958d330bf24d..5ba8143936563 100644 --- a/src/core/server/http/router/index.ts +++ b/src/core/server/http/router/index.ts @@ -9,7 +9,13 @@ export { filterHeaders } from './headers'; export type { Headers, ResponseHeaders, KnownHeaders } from './headers'; export { Router } from './router'; -export type { RequestHandler, RequestHandlerWrapper, IRouter, RouteRegistrar } from './router'; +export type { + RequestHandler, + RequestHandlerWrapper, + IRouter, + RouteRegistrar, + RouterRoute, +} from './router'; export { isKibanaRequest, isRealRequest, ensureRawRequest, KibanaRequest } from './request'; export type { KibanaRequestEvents, diff --git a/src/core/server/http/router/response.ts b/src/core/server/http/router/response.ts index e2babf719f67e..6cea7fcf4c949 100644 --- a/src/core/server/http/router/response.ts +++ b/src/core/server/http/router/response.ts @@ -62,6 +62,8 @@ export interface HttpResponseOptions { body?: HttpResponsePayload; /** HTTP Headers with additional information about response */ headers?: ResponseHeaders; + /** Bypass the default error formatting */ + bypassErrorFormat?: boolean; } /** @@ -79,6 +81,8 @@ export interface CustomHttpResponseOptions; diff --git a/src/core/server/http/types.ts b/src/core/server/http/types.ts index f007a77a2a21a..bbd296d6b1831 100644 --- a/src/core/server/http/types.ts +++ b/src/core/server/http/types.ts @@ -277,6 +277,11 @@ export interface HttpServiceSetup { getServerInfo: () => HttpServerInfo; } +/** @internal */ +export interface InternalNotReadyHttpServiceSetup { + registerRoutes(path: string, callback: (router: IRouter) => void): void; +} + /** @internal */ export interface InternalHttpServiceSetup extends Omit { @@ -287,6 +292,7 @@ export interface InternalHttpServiceSetup path: string, plugin?: PluginOpaqueId ) => IRouter; + registerRouterAfterListening: (router: IRouter) => void; registerStaticDir: (path: string, dirPath: string) => void; getAuthHeaders: GetAuthHeaders; registerRouteHandlerContext: < @@ -297,6 +303,7 @@ export interface InternalHttpServiceSetup contextName: ContextName, provider: RequestHandlerContextProvider ) => RequestHandlerContextContainer; + notReadyServer?: InternalNotReadyHttpServiceSetup; } /** @public */ diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts index dd295efacf6b8..fcc03f363139b 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.test.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.test.ts @@ -27,6 +27,7 @@ describe('IndexMigrator', () => { index: '.kibana', kibanaVersion: '7.10.0', log: loggingSystemMock.create().get(), + setStatus: jest.fn(), mappingProperties: {}, pollInterval: 1, scrollDuration: '1m', diff --git a/src/core/server/saved_objects/migrations/core/index_migrator.ts b/src/core/server/saved_objects/migrations/core/index_migrator.ts index 472fb4f8d1a39..14dba1db9b624 100644 --- a/src/core/server/saved_objects/migrations/core/index_migrator.ts +++ b/src/core/server/saved_objects/migrations/core/index_migrator.ts @@ -41,6 +41,8 @@ export class IndexMigrator { pollInterval: context.pollInterval, + setStatus: context.setStatus, + async isMigrated() { return !(await requiresMigration(context)); }, diff --git a/src/core/server/saved_objects/migrations/core/migration_context.ts b/src/core/server/saved_objects/migrations/core/migration_context.ts index 441c7efed049f..d7f7aff45a470 100644 --- a/src/core/server/saved_objects/migrations/core/migration_context.ts +++ b/src/core/server/saved_objects/migrations/core/migration_context.ts @@ -25,6 +25,7 @@ import { buildActiveMappings } from './build_active_mappings'; import { VersionedTransformer } from './document_migrator'; import * as Index from './elastic_index'; import { SavedObjectsMigrationLogger, MigrationLogger } from './migration_logger'; +import { KibanaMigratorStatus } from '../kibana'; export interface MigrationOpts { batchSize: number; @@ -34,6 +35,7 @@ export interface MigrationOpts { index: string; kibanaVersion: string; log: Logger; + setStatus: (status: KibanaMigratorStatus) => void; mappingProperties: SavedObjectsTypeMappingDefinitions; documentMigrator: VersionedTransformer; serializer: SavedObjectsSerializer; @@ -57,6 +59,7 @@ export interface Context { documentMigrator: VersionedTransformer; kibanaVersion: string; log: SavedObjectsMigrationLogger; + setStatus: (status: KibanaMigratorStatus) => void; batchSize: number; pollInterval: number; scrollDuration: string; @@ -70,7 +73,7 @@ export interface Context { * and various info needed to migrate the source index. */ export async function migrationContext(opts: MigrationOpts): Promise { - const { log, client } = opts; + const { log, client, setStatus } = opts; const alias = opts.index; const source = createSourceContext(await Index.fetchInfo(client, alias), alias); const dest = createDestContext(source, alias, opts.mappingProperties); @@ -82,6 +85,7 @@ export async function migrationContext(opts: MigrationOpts): Promise { dest, kibanaVersion: opts.kibanaVersion, log: new MigrationLogger(log), + setStatus, batchSize: opts.batchSize, documentMigrator: opts.documentMigrator, pollInterval: opts.pollInterval, diff --git a/src/core/server/saved_objects/migrations/core/migration_coordinator.test.ts b/src/core/server/saved_objects/migrations/core/migration_coordinator.test.ts index 9a045d0fbf7f9..63476a15d77cd 100644 --- a/src/core/server/saved_objects/migrations/core/migration_coordinator.test.ts +++ b/src/core/server/saved_objects/migrations/core/migration_coordinator.test.ts @@ -19,6 +19,7 @@ describe('coordinateMigration', () => { throw { body: { error: { index: '.foo', type: 'resource_already_exists_exception' } } }; }); const isMigrated = jest.fn(); + const setStatus = jest.fn(); isMigrated.mockResolvedValueOnce(false).mockResolvedValueOnce(true); @@ -27,6 +28,7 @@ describe('coordinateMigration', () => { runMigration, pollInterval, isMigrated, + setStatus, }); expect(runMigration).toHaveBeenCalledTimes(1); @@ -39,12 +41,14 @@ describe('coordinateMigration', () => { const pollInterval = 1; const runMigration = jest.fn(() => Promise.resolve()); const isMigrated = jest.fn(() => Promise.resolve(true)); + const setStatus = jest.fn(); await coordinateMigration({ log, runMigration, pollInterval, isMigrated, + setStatus, }); expect(isMigrated).not.toHaveBeenCalled(); }); @@ -55,6 +59,7 @@ describe('coordinateMigration', () => { throw new Error('Doh'); }); const isMigrated = jest.fn(() => Promise.resolve(true)); + const setStatus = jest.fn(); await expect( coordinateMigration({ @@ -62,6 +67,7 @@ describe('coordinateMigration', () => { runMigration, pollInterval, isMigrated, + setStatus, }) ).rejects.toThrow(/Doh/); expect(isMigrated).not.toHaveBeenCalled(); diff --git a/src/core/server/saved_objects/migrations/core/migration_coordinator.ts b/src/core/server/saved_objects/migrations/core/migration_coordinator.ts index 3e66d37ce6964..5b99f050b0ece 100644 --- a/src/core/server/saved_objects/migrations/core/migration_coordinator.ts +++ b/src/core/server/saved_objects/migrations/core/migration_coordinator.ts @@ -24,11 +24,16 @@ */ import _ from 'lodash'; +import { KibanaMigratorStatus } from '../kibana'; import { SavedObjectsMigrationLogger } from './migration_logger'; const DEFAULT_POLL_INTERVAL = 15000; -export type MigrationStatus = 'waiting' | 'running' | 'completed'; +export type MigrationStatus = + | 'waiting_to_start' + | 'waiting_for_other_nodes' + | 'running' + | 'completed'; export type MigrationResult = | { status: 'skipped' } @@ -43,6 +48,7 @@ export type MigrationResult = interface Opts { runMigration: () => Promise; isMigrated: () => Promise; + setStatus: (status: KibanaMigratorStatus) => void; log: SavedObjectsMigrationLogger; pollInterval?: number; } @@ -64,7 +70,9 @@ export async function coordinateMigration(opts: Opts): Promise try { return await opts.runMigration(); } catch (error) { - if (handleIndexExists(error, opts.log)) { + const waitingIndex = handleIndexExists(error, opts.log); + if (waitingIndex) { + opts.setStatus({ status: 'waiting_for_other_nodes', waitingIndex }); await waitForMigration(opts.isMigrated, opts.pollInterval); return { status: 'skipped' }; } @@ -77,11 +85,11 @@ export async function coordinateMigration(opts: Opts): Promise * and is the cue for us to fall into a polling loop, waiting for some * other Kibana instance to complete the migration. */ -function handleIndexExists(error: any, log: SavedObjectsMigrationLogger) { +function handleIndexExists(error: any, log: SavedObjectsMigrationLogger): string | undefined { const isIndexExistsError = _.get(error, 'body.error.type') === 'resource_already_exists_exception'; if (!isIndexExistsError) { - return false; + return undefined; } const index = _.get(error, 'body.error.index'); @@ -93,7 +101,7 @@ function handleIndexExists(error: any, log: SavedObjectsMigrationLogger) { `restarting Kibana.` ); - return true; + return index; } /** diff --git a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts index 58dcae7309eea..e09284b49c86e 100644 --- a/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts +++ b/src/core/server/saved_objects/migrations/kibana/kibana_migrator.ts @@ -52,6 +52,7 @@ export type IKibanaMigrator = Pick; export interface KibanaMigratorStatus { status: MigrationStatus; result?: MigrationResult[]; + waitingIndex?: string; } /** @@ -67,7 +68,7 @@ export class KibanaMigrator { private readonly serializer: SavedObjectsSerializer; private migrationResult?: Promise; private readonly status$ = new BehaviorSubject({ - status: 'waiting', + status: 'waiting_to_start', }); private readonly activeMappings: IndexMapping; private migrationsRetryDelay?: number; @@ -200,6 +201,7 @@ export class KibanaMigrator { kibanaVersion: this.kibanaVersion, log: this.log, mappingProperties: indexMap[index].typeMappings, + setStatus: (status) => this.status$.next(status), pollInterval: this.soMigrationsConfig.pollInterval, scrollDuration: this.soMigrationsConfig.scrollDuration, serializer: this.serializer, diff --git a/src/core/server/saved_objects/status.ts b/src/core/server/saved_objects/status.ts index 24e87d2924543..95bf6ddd9ff52 100644 --- a/src/core/server/saved_objects/status.ts +++ b/src/core/server/saved_objects/status.ts @@ -18,11 +18,20 @@ export const calculateStatus$ = ( ): Observable> => { const migratorStatus$: Observable> = rawMigratorStatus$.pipe( map((migrationStatus) => { - if (migrationStatus.status === 'waiting') { + if (migrationStatus.status === 'waiting_to_start') { return { level: ServiceStatusLevels.unavailable, summary: `SavedObjects service is waiting to start migrations`, }; + } else if (migrationStatus.status === 'waiting_for_other_nodes') { + return { + level: ServiceStatusLevels.unavailable, + summary: `SavedObjects service is waiting for other nodes to complete the migration`, + detail: + `If no other Kibana instance is attempting ` + + `migrations, you can get past this message by deleting index ${migrationStatus.waitingIndex} and ` + + `restarting Kibana.`, + }; } else if (migrationStatus.status === 'running') { return { level: ServiceStatusLevels.unavailable, diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index b4c6ee323cbac..327aee1a9dfc6 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -788,6 +788,7 @@ export class CspConfig implements ICspConfig { // @public export interface CustomHttpResponseOptions { body?: T; + bypassErrorFormat?: boolean; headers?: ResponseHeaders; // (undocumented) statusCode: number; @@ -1078,6 +1079,7 @@ export interface HttpResourcesServiceToolkit { // @public export interface HttpResponseOptions { body?: HttpResponsePayload; + bypassErrorFormat?: boolean; headers?: ResponseHeaders; } @@ -3261,7 +3263,7 @@ export const validBodyOutput: readonly ["data", "stream"]; // Warnings were encountered during analysis: // // src/core/server/elasticsearch/client/types.ts:94:7 - (ae-forgotten-export) The symbol "Explanation" needs to be exported by the entry point index.d.ts -// src/core/server/http/router/response.ts:297:3 - (ae-forgotten-export) The symbol "KibanaResponse" needs to be exported by the entry point index.d.ts +// src/core/server/http/router/response.ts:301:3 - (ae-forgotten-export) The symbol "KibanaResponse" needs to be exported by the entry point index.d.ts // src/core/server/plugins/types.ts:326:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts // src/core/server/plugins/types.ts:326:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts // src/core/server/plugins/types.ts:329:3 - (ae-forgotten-export) The symbol "SavedObjectsConfigType" needs to be exported by the entry point index.d.ts diff --git a/src/core/server/status/legacy_status.ts b/src/core/server/status/legacy_status.ts index b7d0965e31f68..1b3d139b1345e 100644 --- a/src/core/server/status/legacy_status.ts +++ b/src/core/server/status/legacy_status.ts @@ -95,7 +95,7 @@ const serviceStatusToHttpComponent = ( since: string ): StatusComponentHttp => ({ id: serviceName, - message: status.summary, + message: [status.summary, status.detail].filter(Boolean).join(' '), since, ...serviceStatusAttrs(status), }); diff --git a/src/core/server/status/routes/status.ts b/src/core/server/status/routes/status.ts index c1782570ecfa0..72f639231996f 100644 --- a/src/core/server/status/routes/status.ts +++ b/src/core/server/status/routes/status.ts @@ -12,7 +12,7 @@ import { schema } from '@kbn/config-schema'; import { IRouter } from '../../http'; import { MetricsServiceSetup } from '../../metrics'; -import { ServiceStatus, CoreStatus } from '../types'; +import { ServiceStatus, CoreStatus, ServiceStatusLevels } from '../types'; import { PluginName } from '../../plugins'; import { calculateLegacyStatus, LegacyStatusInfo } from '../legacy_status'; import { PackageInfo } from '../../config'; @@ -160,7 +160,8 @@ export const registerStatusRoute = ({ router, config, metrics, status }: Deps) = }, }; - return res.ok({ body }); + const statusCode = overall.level >= ServiceStatusLevels.unavailable ? 503 : 200; + return res.custom({ body, statusCode, bypassErrorFormat: true }); } ); }; diff --git a/src/core/server/status/status_service.ts b/src/core/server/status/status_service.ts index 7724e7a5e44b4..cfd4d92d91d3f 100644 --- a/src/core/server/status/status_service.ts +++ b/src/core/server/status/status_service.ts @@ -88,9 +88,7 @@ export class StatusService implements CoreService { // Create an unused subscription to ensure all underlying lazy observables are started. this.overallSubscription = overall$.subscribe(); - const router = http.createRouter(''); - registerStatusRoute({ - router, + const commonRouteDeps = { config: { allowAnonymous: statusConfig.allowAnonymous, packageInfo: this.coreContext.env.packageInfo, @@ -103,8 +101,27 @@ export class StatusService implements CoreService { plugins$: this.pluginsStatus.getAll$(), core$, }, + }; + + const router = http.createRouter(''); + registerStatusRoute({ + router, + ...commonRouteDeps, }); + if (http.notReadyServer && commonRouteDeps.config.allowAnonymous) { + http.notReadyServer.registerRoutes('', (notReadyRouter) => { + registerStatusRoute({ + router: notReadyRouter, + ...commonRouteDeps, + config: { + ...commonRouteDeps.config, + allowAnonymous: true, + }, + }); + }); + } + return { core$, overall$, diff --git a/test/api_integration/apis/saved_objects/migrations.ts b/test/api_integration/apis/saved_objects/migrations.ts index 87997ab4231a2..dcd34c604dc31 100644 --- a/test/api_integration/apis/saved_objects/migrations.ts +++ b/test/api_integration/apis/saved_objects/migrations.ts @@ -735,6 +735,7 @@ async function migrateIndex({ mappingProperties, batchSize: 10, log: getLogMock(), + setStatus: () => {}, pollInterval: 50, scrollDuration: '5m', serializer: new SavedObjectsSerializer(typeRegistry),