Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HTTP] Expose versioned router #153858

Merged
merged 51 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
cfd23a7
moved ApiVersion type to common http package
jloleysens Mar 28, 2023
5a41847
runtime check for version number
jloleysens Mar 28, 2023
52b25b9
rename shared version header constant, added test for passing through…
jloleysens Mar 28, 2023
ffa9f66
move route validator to own package so that it can be shared
jloleysens Mar 28, 2023
ba1a3a1
ran yarn kbn bootstrap
jloleysens Mar 28, 2023
9d7506d
import route validator from new package
jloleysens Mar 28, 2023
2db39e1
remove dependence on core internal router
jloleysens Mar 28, 2023
8af51c5
pointless change to tsconfig.json
jloleysens Mar 28, 2023
f937521
remove dependence on core internal router
jloleysens Mar 28, 2023
6f01b5c
fix mock response factory
jloleysens Mar 28, 2023
25d6249
added versioned router mocks
jloleysens Mar 28, 2023
983596b
ran yarn kbn bootstrap
jloleysens Mar 28, 2023
ce98092
added versioned router mock
jloleysens Mar 28, 2023
b8c694e
merge imports from same package
jloleysens Mar 28, 2023
1364c7a
expose the versioned router on the IRouter interface
jloleysens Mar 28, 2023
b98044b
added an example and removed auto return type inference
jloleysens Mar 28, 2023
02f330d
fix output validation by actually passing in the response :facepalm: …
jloleysens Mar 28, 2023
7b88ded
just use the const
jloleysens Mar 28, 2023
f67e2d5
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Mar 28, 2023
a27e49f
[CI] Auto-commit changed files from 'node scripts/generate codeowners'
kibanamachine Mar 28, 2023
3ad27c0
remove circular dep
jloleysens Mar 28, 2023
d9d05d5
added IRouterWithVersion to address type woes
jloleysens Mar 28, 2023
e75ed38
Merge branch 'main' into expose-versioned-router
jloleysens Mar 28, 2023
ab53f28
make default for checking responses false and update tests
jloleysens Mar 28, 2023
9665928
make body, param, query nullable
jloleysens Mar 29, 2023
37cef77
added some integration tests for the versioned router
jloleysens Mar 29, 2023
e0538cf
mock out badRequest
jloleysens Mar 29, 2023
435b26f
Merge branch 'main' into expose-versioned-router
jloleysens Mar 29, 2023
047dfce
Merge branch 'main' into expose-versioned-router
jloleysens Mar 30, 2023
1fff686
do better than any
jloleysens Mar 31, 2023
a3fd786
remove inline class def
jloleysens Mar 31, 2023
5635219
factor out route version validation logic
jloleysens Mar 31, 2023
e6e8eed
added validation function
jloleysens Mar 31, 2023
03ab1b9
added test and fixed logic
jloleysens Mar 31, 2023
9a3e7c7
added integration test case for missing version header
jloleysens Mar 31, 2023
42ed016
fix typo
jloleysens Mar 31, 2023
6a67877
Merge branch 'main' into expose-versioned-router
jloleysens Apr 3, 2023
fa60493
move versioned router mock
jloleysens Apr 3, 2023
f2a532b
update exports and use the versioned router mock
jloleysens Apr 3, 2023
3ddc635
move versioned router implementation
jloleysens Apr 3, 2023
3489693
remove the validator package
jloleysens Apr 3, 2023
ca0c431
update a few import paths and ts config references
jloleysens Apr 3, 2023
3ea8c48
remove ts project reference
jloleysens Apr 3, 2023
41df80c
go back to using actual CoreKibanaRequest!
jloleysens Apr 3, 2023
154821a
ran yarn kbn bootstrap
jloleysens Apr 3, 2023
40fd5e1
remove 3 unnecessary code owners entries
jloleysens Apr 3, 2023
3d51eec
remove unnecessary default
jloleysens Apr 3, 2023
5388234
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Apr 3, 2023
e723e06
change the version missing response
jloleysens Apr 3, 2023
df0211a
Merge branch 'main' into expose-versioned-router
jloleysens Apr 3, 2023
2c5c840
Merge branch 'main' into expose-versioned-router
jloleysens Apr 3, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ packages/core/http/core-http-router-server-mocks @elastic/kibana-core
packages/core/http/core-http-server @elastic/kibana-core
packages/core/http/core-http-server-internal @elastic/kibana-core
packages/core/http/core-http-server-mocks @elastic/kibana-core
packages/core/http/core-http-versioned-router-server-internal @elastic/kibana-core
packages/core/i18n/core-i18n-browser @elastic/kibana-core
packages/core/i18n/core-i18n-browser-internal @elastic/kibana-core
packages/core/i18n/core-i18n-browser-mocks @elastic/kibana-core
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@
"@kbn/core-http-router-server-internal": "link:packages/core/http/core-http-router-server-internal",
"@kbn/core-http-server": "link:packages/core/http/core-http-server",
"@kbn/core-http-server-internal": "link:packages/core/http/core-http-server-internal",
"@kbn/core-http-versioned-router-server-internal": "link:packages/core/http/core-http-versioned-router-server-internal",
"@kbn/core-i18n-browser": "link:packages/core/i18n/core-i18n-browser",
"@kbn/core-i18n-browser-internal": "link:packages/core/i18n/core-i18n-browser-internal",
"@kbn/core-i18n-server": "link:packages/core/i18n/core-i18n-server",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { HttpResponse, HttpFetchOptionsWithPath } from '@kbn/core-http-brow

import { Fetch } from './fetch';
import { BasePath } from './base_path';
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';

function delay<T>(duration: number) {
return new Promise<T>((r) => setTimeout(r, duration));
Expand Down Expand Up @@ -479,6 +480,14 @@ describe('Fetch', () => {

expect(ndjson).toEqual(content);
});

it('should pass through version as a header', async () => {
fetchMock.get('*', { body: {} });
await fetchInstance.fetch('/my/path', { asResponse: true, version: '99' });
expect(fetchMock.lastOptions()!.headers).toEqual(
expect.objectContaining({ [ELASTIC_HTTP_VERSION_HEADER.toLowerCase()]: '99' })
);
});
});

describe('interception', () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/core/http/core-http-browser-internal/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
HttpResponse,
HttpFetchOptionsWithPath,
} from '@kbn/core-http-browser';
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
import { HttpFetchError } from './http_fetch_error';
import { HttpInterceptController } from './http_intercept_controller';
import { interceptRequest, interceptResponse } from './intercept';
Expand Down Expand Up @@ -110,6 +111,7 @@ export class Fetch {

private createRequest(options: HttpFetchOptionsWithPath): Request {
const context = this.params.executionContext.withGlobalContext(options.context);
const { version } = options;
// Merge and destructure options out that are not applicable to the Fetch API.
const {
query,
Expand All @@ -128,6 +130,7 @@ export class Fetch {
'Content-Type': 'application/json',
...options.headers,
'kbn-version': this.params.kibanaVersion,
[ELASTIC_HTTP_VERSION_HEADER]: version,
...(!isEmpty(context) ? new ExecutionContextContainer(context).toHeader() : {}),
}),
};
Expand Down
4 changes: 4 additions & 0 deletions packages/core/http/core-http-browser/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import type { Observable } from 'rxjs';
import type { MaybePromise } from '@kbn/utility-types';
import type { KibanaExecutionContext } from '@kbn/core-execution-context-common';
import type { ApiVersion } from '@kbn/core-http-common';

/** @public */
export interface HttpSetup {
Expand Down Expand Up @@ -280,6 +281,9 @@ export interface HttpFetchOptions extends HttpRequestInit {
asResponse?: boolean;

context?: KibanaExecutionContext;

/** @experimental */
version?: ApiVersion;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/core/http/core-http-browser/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
],
"kbn_references": [
"@kbn/utility-types",
"@kbn/core-execution-context-common"
"@kbn/core-execution-context-common",
"@kbn/core-http-common"
],
"exclude": [
"target/**/*",
Expand Down
3 changes: 3 additions & 0 deletions packages/core/http/core-http-common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
*/

export type { IExternalUrlPolicy } from './src/external_url_policy';

export type { ApiVersion } from './src/versioning';
export { ELASTIC_HTTP_VERSION_HEADER } from './src/versioning';
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test/jest_node',
rootDir: '../../../..',
roots: ['<rootDir>/packages/core/http/core-http-versioned-router-server-internal'],
};
/**
* A Kibana HTTP API version
* @note assumption that version will be monotonically increasing number where: version > 0.
* @experimental
*/
export type ApiVersion = `${number}`;

/** @internal */
export const ELASTIC_HTTP_VERSION_HEADER = 'elastic-api-version' as const;
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,3 @@ export {
isKibanaResponse,
KibanaResponse,
} from './src/response';
export { RouteValidator } from './src/validator';
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import {
RawRequest,
FakeRawRequest,
} from '@kbn/core-http-server';
import { RouteValidator } from './validator';
import { isSafeMethod } from './route';
import { KibanaSocket } from './socket';
import { RouteValidator } from './validator';

const requestSymbol = Symbol('request');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ import type {
RouterRoute,
IRouter,
RequestHandler,
VersionedRouter,
IRouterWithVersion,
} from '@kbn/core-http-server';
import { validBodyOutput } from '@kbn/core-http-server';
import { RouteValidator } from './validator';
import { CoreVersionedRouter } from './versioned_router';
import { CoreKibanaRequest } from './request';
import { kibanaResponseFactory } from './response';
import { HapiResponseAdapter } from './response_adapter';
import { wrapErrors } from './error_wrapper';
import { RouteValidator } from './validator';

export type ContextEnhancer<
P,
Expand Down Expand Up @@ -120,7 +123,7 @@ function validOptions(
* @internal
*/
export class Router<Context extends RequestHandlerContextBase = RequestHandlerContextBase>
implements IRouter<Context>
implements IRouterWithVersion<Context>
{
public routes: Array<Readonly<RouterRoute>> = [];
public get: IRouter<Context>['get'];
Expand Down Expand Up @@ -202,6 +205,14 @@ export class Router<Context extends RequestHandlerContextBase = RequestHandlerCo
return hapiResponseAdapter.toInternalError();
}
}

private versionedRouter: undefined | VersionedRouter<Context> = undefined;
public get versioned(): VersionedRouter<Context> {
if (this.versionedRouter === undefined) {
this.versionedRouter = CoreVersionedRouter.from({ router: this });
}
return this.versionedRouter;
}
Comment on lines +209 to +215
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the implementation for exposing the versioned router

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on recent discussions around architectural complexity specifically related to core's packages footprint, perhaps we should move the versioned router into the main core-http-router-server-internal package.

}

const convertEsUnauthorized = (e: EsNotAuthorizedError): ErrorHttpResponseOptions => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,56 @@
* Side Public License, v 1.
*/

import { hapiMocks } from '@kbn/hapi-mocks';
import { schema } from '@kbn/config-schema';
import type { IRouter, RequestHandler } from '@kbn/core-http-server';
import { httpServiceMock, httpServerMock } from '@kbn/core-http-server-mocks';
import { VERSION_HEADER } from './core_versioned_route';
import type { ApiVersion } from '@kbn/core-http-common';
import type { IRouter, KibanaResponseFactory, RequestHandler } from '@kbn/core-http-server';
import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common';
import { createRouter } from './mocks';
import { CoreVersionedRouter } from '.';
import { kibanaResponseFactory } from '@kbn/core-http-router-server-internal';
import { passThroughValidation } from './core_versioned_route';
import { CoreKibanaRequest } from '../request';

const createRequest = (
{
version,
body,
params,
query,
}: { version: undefined | ApiVersion; body?: object; params?: object; query?: object } = {
version: '1',
}
) =>
CoreKibanaRequest.from(
hapiMocks.createRequest({
payload: body,
params,
query,
headers: { [ELASTIC_HTTP_VERSION_HEADER]: version },
app: { requestId: 'fakeId' },
}),
passThroughValidation
);

describe('Versioned route', () => {
let router: IRouter;
let responseFactory: jest.Mocked<KibanaResponseFactory>;
const handlerFn: RequestHandler = async (ctx, req, res) => res.ok({ body: { foo: 1 } });
beforeEach(() => {
router = httpServiceMock.createRouter();
responseFactory = {
custom: jest.fn(({ body, statusCode }) => ({
options: {},
status: statusCode,
payload: body,
})),
badRequest: jest.fn(({ body }) => ({ status: 400, payload: body, options: {} })),
ok: jest.fn(({ body } = {}) => ({
options: {},
status: 200,
payload: body,
})),
} as any;
router = createRouter();
});

it('can register multiple handlers', () => {
Expand Down Expand Up @@ -48,6 +86,31 @@ describe('Versioned route', () => {
);
});

it('only allows versions that are numbers greater than 0', () => {
const versionedRouter = CoreVersionedRouter.from({ router });
expect(() =>
versionedRouter
.get({ path: '/test/{id}', access: 'internal' })
.addVersion({ version: 'foo' as ApiVersion, validate: false }, handlerFn)
).toThrowError(
`Invalid version number. Received "foo", expected any finite, whole number greater than 0.`
);
expect(() =>
versionedRouter
.get({ path: '/test/{id}', access: 'internal' })
.addVersion({ version: '-1', validate: false }, handlerFn)
).toThrowError(
`Invalid version number. Received "-1", expected any finite, whole number greater than 0.`
);
expect(() =>
versionedRouter
.get({ path: '/test/{id}', access: 'internal' })
.addVersion({ version: '1.1', validate: false }, handlerFn)
).toThrowError(
`Invalid version number. Received "1.1", expected any finite, whole number greater than 0.`
);
});

it('runs request and response validations', async () => {
let handler: RequestHandler;

Expand All @@ -57,7 +120,7 @@ describe('Versioned route', () => {
let validatedOutputBody = false;

(router.post as jest.Mock).mockImplementation((opts: unknown, fn) => (handler = fn));
const versionedRouter = CoreVersionedRouter.from({ router });
const versionedRouter = CoreVersionedRouter.from({ router, validateResponses: true });
versionedRouter.post({ path: '/test/{id}', access: 'internal' }).addVersion(
{
version: '1',
Expand Down Expand Up @@ -103,13 +166,13 @@ describe('Versioned route', () => {

const kibanaResponse = await handler!(
{} as any,
httpServerMock.createKibanaRequest({
headers: { [VERSION_HEADER]: '1' },
createRequest({
version: '1',
body: { foo: 1 },
params: { foo: 1 },
query: { foo: 1 },
}),
kibanaResponseFactory
responseFactory
);

expect(kibanaResponse.status).toBe(200);
Expand All @@ -126,18 +189,14 @@ describe('Versioned route', () => {
versionedRouter.post({ access: 'internal', path: '/test/{id}' });

await expect(
handler!(
{} as any,
httpServerMock.createKibanaRequest({
headers: { [VERSION_HEADER]: '999' },
}),
kibanaResponseFactory
)
).resolves.toEqual({
options: {},
payload: 'No version "999" available for [post] [/test/{id}]. Available versions are: "none"',
status: 406,
});
handler!({} as any, createRequest({ version: '999' }), responseFactory)
).resolves.toEqual(
expect.objectContaining({
payload:
'No version "999" available for [post] [/test/{id}]. Available versions are: <none>',
status: 406,
})
);
});

it('returns the expected output if no version was provided to versioned route', async () => {
Expand All @@ -150,17 +209,10 @@ describe('Versioned route', () => {
.addVersion({ validate: false, version: '1' }, handlerFn);

await expect(
handler!(
{} as any,
httpServerMock.createKibanaRequest({
headers: {},
}),
kibanaResponseFactory
)
handler!({} as any, createRequest({ version: undefined }), responseFactory)
).resolves.toEqual({
options: {},
payload:
'Version expected at [post] [/test/{id}]. Please specify a version using the "Elastic-Api-Version" header. Available versions are: "1"',
payload: `Version expected at [post] [/test/{id}]. Please specify a version using the "${ELASTIC_HTTP_VERSION_HEADER}" header. Available versions are: [1]`,
status: 406,
});
});
Expand All @@ -179,11 +231,11 @@ describe('Versioned route', () => {
await expect(
handler!(
{} as any,
httpServerMock.createKibanaRequest({
headers: { [VERSION_HEADER]: '1' },
createRequest({
version: '1',
body: {},
}),
kibanaResponseFactory
responseFactory
)
).resolves.toEqual({
options: {},
Expand Down
Loading