From b4827ace2771a19d12e8c5b06630f099961adf7c Mon Sep 17 00:00:00 2001 From: Brandon Kobel Date: Mon, 21 Sep 2020 09:41:09 -0700 Subject: [PATCH] Adding KibanaRequest#uuid (#76822) * Adding KibanaRequest#uuid * Adding tests * Fixing test which was mocking uuid.v4() to get expected IDs * Fixing another mock * Updating docs Co-authored-by: Elastic Machine --- ...kibana-plugin-core-server.kibanarequest.md | 1 + ...a-plugin-core-server.kibanarequest.uuid.md | 18 ++++++++++++ .../client/cluster_client.test.ts | 4 +-- .../legacy/cluster_client.test.ts | 4 ++- src/core/server/http/http_server.mocks.ts | 2 +- src/core/server/http/http_server.ts | 2 ++ .../http/integration_tests/request.test.ts | 21 ++++++++++++++ src/core/server/http/router/request.test.ts | 28 +++++++++++++++++++ src/core/server/http/router/request.ts | 14 ++++++++-- .../routes/integration_tests/import.test.ts | 7 +++-- src/core/server/server.api.md | 1 + .../server/client/audit_trail_client.test.ts | 4 ++- 12 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 docs/development/core/server/kibana-plugin-core-server.kibanarequest.uuid.md diff --git a/docs/development/core/server/kibana-plugin-core-server.kibanarequest.md b/docs/development/core/server/kibana-plugin-core-server.kibanarequest.md index 05e408ab499952..0a49ee6e63d6c7 100644 --- a/docs/development/core/server/kibana-plugin-core-server.kibanarequest.md +++ b/docs/development/core/server/kibana-plugin-core-server.kibanarequest.md @@ -33,4 +33,5 @@ export declare class KibanaRequestRecursiveReadonly<KibanaRequestRoute<Method>> | matched route details | | [socket](./kibana-plugin-core-server.kibanarequest.socket.md) | | IKibanaSocket | [IKibanaSocket](./kibana-plugin-core-server.ikibanasocket.md) | | [url](./kibana-plugin-core-server.kibanarequest.url.md) | | Url | a WHATWG URL standard object. | +| [uuid](./kibana-plugin-core-server.kibanarequest.uuid.md) | | string | A UUID to identify this request. | diff --git a/docs/development/core/server/kibana-plugin-core-server.kibanarequest.uuid.md b/docs/development/core/server/kibana-plugin-core-server.kibanarequest.uuid.md new file mode 100644 index 00000000000000..8b980b82d0adbe --- /dev/null +++ b/docs/development/core/server/kibana-plugin-core-server.kibanarequest.uuid.md @@ -0,0 +1,18 @@ + + +[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md) > [uuid](./kibana-plugin-core-server.kibanarequest.uuid.md) + +## KibanaRequest.uuid property + +A UUID to identify this request. + +Signature: + +```typescript +readonly uuid: string; +``` + +## Remarks + +This value is NOT sourced from the incoming request's `X-Opaque-Id` header. it is always a UUID uniquely identifying the request. + diff --git a/src/core/server/elasticsearch/client/cluster_client.test.ts b/src/core/server/elasticsearch/client/cluster_client.test.ts index 121ef3aa42d513..e35d9962e9e7ee 100644 --- a/src/core/server/elasticsearch/client/cluster_client.test.ts +++ b/src/core/server/elasticsearch/client/cluster_client.test.ts @@ -206,7 +206,7 @@ describe('ClusterClient', () => { const clusterClient = new ClusterClient(config, logger, getAuthHeaders); const request = httpServerMock.createKibanaRequest({ - kibanaRequestState: { requestId: 'my-fake-id' }, + kibanaRequestState: { requestId: 'my-fake-id', requestUuid: 'ignore-this-id' }, }); clusterClient.asScoped(request); @@ -284,7 +284,7 @@ describe('ClusterClient', () => { const clusterClient = new ClusterClient(config, logger, getAuthHeaders); const request = httpServerMock.createKibanaRequest({ headers: { foo: 'request' }, - kibanaRequestState: { requestId: 'from request' }, + kibanaRequestState: { requestId: 'from request', requestUuid: 'ignore-this-id' }, }); clusterClient.asScoped(request); diff --git a/src/core/server/elasticsearch/legacy/cluster_client.test.ts b/src/core/server/elasticsearch/legacy/cluster_client.test.ts index 73d941053e84b7..745ef4304d0b1c 100644 --- a/src/core/server/elasticsearch/legacy/cluster_client.test.ts +++ b/src/core/server/elasticsearch/legacy/cluster_client.test.ts @@ -351,7 +351,9 @@ describe('#asScoped', () => { test('passes x-opaque-id header with request id', () => { clusterClient.asScoped( - httpServerMock.createKibanaRequest({ kibanaRequestState: { requestId: 'alpha' } }) + httpServerMock.createKibanaRequest({ + kibanaRequestState: { requestId: 'alpha', requestUuid: 'ignore-this-id' }, + }) ); expect(MockScopedClusterClient).toHaveBeenCalledTimes(1); diff --git a/src/core/server/http/http_server.mocks.ts b/src/core/server/http/http_server.mocks.ts index 6d096b76263b53..9deaa73d8aacfa 100644 --- a/src/core/server/http/http_server.mocks.ts +++ b/src/core/server/http/http_server.mocks.ts @@ -68,7 +68,7 @@ function createKibanaRequestMock

({ routeAuthRequired, validation = {}, kibanaRouteOptions = { xsrfRequired: true }, - kibanaRequestState = { requestId: '123' }, + kibanaRequestState = { requestId: '123', requestUuid: '123e4567-e89b-12d3-a456-426614174000' }, auth = { isAuthenticated: true }, }: RequestFixtureOptions = {}) { const queryString = stringify(query, { sort: false }); diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 7609f23fe0c51d..2440f2b1da0bd2 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -19,6 +19,7 @@ import { Server } from 'hapi'; import HapiStaticFiles from 'inert'; import url from 'url'; +import uuid from 'uuid'; import { Logger, LoggerFactory } from '../logging'; import { HttpConfig } from './http_config'; @@ -315,6 +316,7 @@ export class HttpServer { request.app = { ...(request.app ?? {}), requestId: getRequestId(request, config.requestId), + requestUuid: uuid.v4(), } as KibanaRequestState; return responseToolkit.continue; }); diff --git a/src/core/server/http/integration_tests/request.test.ts b/src/core/server/http/integration_tests/request.test.ts index 0727ff848c189f..0170e94867c064 100644 --- a/src/core/server/http/integration_tests/request.test.ts +++ b/src/core/server/http/integration_tests/request.test.ts @@ -16,6 +16,11 @@ * specific language governing permissions and limitations * under the License. */ + +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'), +})); + import supertest from 'supertest'; import { HttpService } from '../http_service'; @@ -308,4 +313,20 @@ describe('KibanaRequest', () => { expect(resp3.body).toEqual({ requestId: 'gamma' }); }); }); + + describe('request uuid', () => { + it('generates a UUID', async () => { + const { server: innerServer, createRouter } = await server.setup(setupDeps); + const router = createRouter('/'); + router.get({ path: '/', validate: false }, async (context, req, res) => { + return res.ok({ body: { requestUuid: req.uuid } }); + }); + await server.start(); + + const st = supertest(innerServer.listener); + + const resp1 = await st.get('/').expect(200); + expect(resp1.body.requestUuid).toBe('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'); + }); + }); }); diff --git a/src/core/server/http/router/request.test.ts b/src/core/server/http/router/request.test.ts index e741121f3d70c2..0bf81a7aca8520 100644 --- a/src/core/server/http/router/request.test.ts +++ b/src/core/server/http/router/request.test.ts @@ -55,6 +55,34 @@ describe('KibanaRequest', () => { }); }); + describe('uuid property', () => { + it('uses the request.app.requestUuid property if present', () => { + const request = httpServerMock.createRawRequest({ + app: { requestUuid: '123e4567-e89b-12d3-a456-426614174000' }, + }); + const kibanaRequest = KibanaRequest.from(request); + expect(kibanaRequest.uuid).toEqual('123e4567-e89b-12d3-a456-426614174000'); + }); + + it('generates a new UUID if request.app property is not present', () => { + // Undefined app property + const request = httpServerMock.createRawRequest({ + app: undefined, + }); + const kibanaRequest = KibanaRequest.from(request); + expect(kibanaRequest.uuid).toEqual('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'); + }); + + it('generates a new UUID if request.app.requestUuid property is not present', () => { + // Undefined app.requestUuid property + const request = httpServerMock.createRawRequest({ + app: {}, + }); + const kibanaRequest = KibanaRequest.from(request); + expect(kibanaRequest.uuid).toEqual('xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx'); + }); + }); + describe('get all headers', () => { it('returns all headers', () => { const request = httpServerMock.createRawRequest({ diff --git a/src/core/server/http/router/request.ts b/src/core/server/http/router/request.ts index e04f8585981b52..903eb75022df35 100644 --- a/src/core/server/http/router/request.ts +++ b/src/core/server/http/router/request.ts @@ -44,6 +44,7 @@ export interface KibanaRouteOptions extends RouteOptionsApp { */ export interface KibanaRequestState extends ApplicationState { requestId: string; + requestUuid: string; } /** @@ -152,6 +153,14 @@ export class KibanaRequest< * per request. */ public readonly id: string; + /** + * A UUID to identify this request. + * + * @remarks + * This value is NOT sourced from the incoming request's `X-Opaque-Id` header. it + * is always a UUID uniquely identifying the request. + */ + public readonly uuid: string; /** a WHATWG URL standard object. */ public readonly url: Url; /** matched route details */ @@ -189,10 +198,11 @@ export class KibanaRequest< // until that time we have to expose all the headers private readonly withoutSecretHeaders: boolean ) { - // The `requestId` property will not be populated for requests that are 'faked' by internal systems that leverage + // The `requestId` and `requestUuid` properties will not be populated for requests that are 'faked' by internal systems that leverage // KibanaRequest in conjunction with scoped Elaticcsearch and SavedObjectsClient in order to pass credentials. - // In these cases, the id defaults to a newly generated UUID. + // In these cases, the ids default to a newly generated UUID. this.id = (request.app as KibanaRequestState | undefined)?.requestId ?? uuid.v4(); + this.uuid = (request.app as KibanaRequestState | undefined)?.requestUuid ?? uuid.v4(); this.url = request.url; this.headers = deepFreeze({ ...request.headers }); diff --git a/src/core/server/saved_objects/routes/integration_tests/import.test.ts b/src/core/server/saved_objects/routes/integration_tests/import.test.ts index 0bc03fbcf80381..67be2b56b4447e 100644 --- a/src/core/server/saved_objects/routes/integration_tests/import.test.ts +++ b/src/core/server/saved_objects/routes/integration_tests/import.test.ts @@ -471,7 +471,11 @@ describe(`POST ${URL}`, () => { describe('createNewCopies enabled', () => { it('imports objects, regenerating all IDs/reference IDs present, and resetting all origin IDs', async () => { - mockUuidv4.mockReturnValueOnce('new-id-1').mockReturnValueOnce('new-id-2'); + mockUuidv4 + .mockReturnValueOnce('foo') // a uuid.v4() is generated for the request.id + .mockReturnValueOnce('foo') // another uuid.v4() is used for the request.uuid + .mockReturnValueOnce('new-id-1') + .mockReturnValueOnce('new-id-2'); savedObjectsClient.bulkGet.mockResolvedValueOnce({ saved_objects: [mockIndexPattern] }); const obj1 = { type: 'visualization', @@ -490,7 +494,6 @@ describe(`POST ${URL}`, () => { const result = await supertest(httpSetup.server.listener) .post(`${URL}?createNewCopies=true`) .set('content-Type', 'multipart/form-data; boundary=EXAMPLE') - .set('x-opaque-id', uuidv4()) // prevents src/core/server/http/http_tools.ts from using our mocked uuidv4 to generate a unique ID for this request .send( [ '--EXAMPLE', diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 11a14457784fd1..d755ef3e1b6765 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -943,6 +943,7 @@ export class KibanaRequest { beforeEach(() => { event$ = new Subject(); client = new AuditTrailClient( - httpServerMock.createKibanaRequest({ kibanaRequestState: { requestId: 'request id alpha' } }), + httpServerMock.createKibanaRequest({ + kibanaRequestState: { requestId: 'request id alpha', requestUuid: 'ignore-me' }, + }), event$, deps );