Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 32 additions & 0 deletions x-pack/plugins/apm/common/apm_api/parse_endpoint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

type Method = 'get' | 'post' | 'put' | 'delete';

export function parseEndpoint(
endpoint: string,
pathParams: Record<string, any> = {}
) {
const [method, rawPathname] = endpoint.split(' ');

// replace template variables with path params
const pathname = Object.keys(pathParams).reduce((acc, paramName) => {
return acc.replace(`{${paramName}}`, pathParams[paramName]);
}, rawPathname);

return { method: parseMethod(method), pathname };
}

export function parseMethod(method: string) {
const res = method.trim().toLowerCase() as Method;

if (!['get', 'post', 'put', 'delete'].includes(res)) {
throw new Error('Endpoint was not prefixed with a valid HTTP method');
}

return res;
}
24 changes: 10 additions & 14 deletions x-pack/plugins/apm/public/services/rest/createCallApmApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
*/

import { CoreSetup, CoreStart } from 'kibana/public';
import { parseEndpoint } from '../../../common/apm_api/parse_endpoint';
import { FetchOptions } from '../../../common/fetch_options';
import { callApi } from './callApi';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { APMAPI } from '../../../server/routes/create_apm_api';
import type { APMAPI } from '../../../server/routes/create_apm_api';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { Client } from '../../../server/routes/typings';
import type { Client } from '../../../server/routes/typings';

export type APMClient = Client<APMAPI['_S']>;
export type AutoAbortedAPMClient = Client<APMAPI['_S'], { abortable: false }>;
Expand All @@ -24,8 +25,8 @@ export type APMClientOptions = Omit<
signal: AbortSignal | null;
params?: {
body?: any;
query?: any;
path?: any;
query?: Record<string, any>;
path?: Record<string, any>;
};
};

Expand All @@ -37,20 +38,15 @@ export let callApmApi: APMClient = () => {

export function createCallApmApi(core: CoreStart | CoreSetup) {
callApmApi = ((options: APMClientOptions) => {
const { endpoint, params = {}, ...opts } = options;
const path = (params.path || {}) as Record<string, any>;
const [method, pathname] = endpoint.split(' ');

const formattedPathname = Object.keys(path).reduce((acc, paramName) => {
return acc.replace(`{${paramName}}`, path[paramName]);
}, pathname);
const { endpoint, params, ...opts } = options;
const { method, pathname } = parseEndpoint(endpoint, params?.path);

return callApi(core, {
...opts,
method,
pathname: formattedPathname,
body: params.body,
query: params.query,
pathname,
body: params?.body,
query: params?.query,
});
}) as APMClient;
}
Expand Down
90 changes: 45 additions & 45 deletions x-pack/plugins/apm/server/routes/create_api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ import { isLeft } from 'fp-ts/lib/Either';
import { KibanaRequest, RouteRegistrar } from 'src/core/server';
import { RequestAbortedError } from '@elastic/elasticsearch/lib/errors';
import agent from 'elastic-apm-node';
import { parseMethod } from '../../../common/apm_api/parse_endpoint';
import { merge } from '../../../common/runtime_types/merge';
import { strictKeysRt } from '../../../common/runtime_types/strict_keys_rt';
import { APMConfig } from '../..';
import { ServerAPI } from '../typings';
import { RouteParamsRT, ServerAPI } from '../typings';
import { jsonRt } from '../../../common/runtime_types/json_rt';
import type { ApmPluginRequestHandlerContext } from '../typings';

const debugRt = t.exact(
const inspectRt = t.exact(
t.partial({
query: t.exact(t.partial({ _inspect: jsonRt.pipe(t.boolean) })),
})
Expand Down Expand Up @@ -71,24 +72,10 @@ export function createApi() {
const { params, endpoint, options, handler } = route;

const [method, path] = endpoint.split(' ');

const typedRouterMethod = method.trim().toLowerCase() as
| 'get'
| 'post'
| 'put'
| 'delete';

if (!['get', 'post', 'put', 'delete'].includes(typedRouterMethod)) {
throw new Error(
"Couldn't register route, as endpoint was not prefixed with a valid HTTP method"
);
}
const typedRouterMethod = parseMethod(method);

// For all runtime types with props, we create an exact
// version that will strip all keys that are unvalidated.

const paramsRt = params ? merge([params, debugRt]) : debugRt;

const anyObject = schema.object({}, { unknowns: 'allow' });

(router[typedRouterMethod] as RouteRegistrar<
Expand Down Expand Up @@ -119,44 +106,24 @@ export function createApi() {
inspectableEsQueriesMap.set(request, []);

try {
const paramMap = pickBy(
{
path: request.params,
body: request.body,
query: {
_inspect: 'false',
...request.query,
},
},
isNotEmpty
);

const result = strictKeysRt(paramsRt).decode(paramMap);

if (isLeft(result)) {
throw Boom.badRequest(PathReporter.report(result)[0]);
}
const validParams = validateParams(request, params);
const data = await handler({
request,
context: {
...context,
plugins,
// Only return values for parameters that have runtime types,
// but always include query as _inspect is always set even if
// it's not defined in the route.
params: mergeLodash(
{ query: { _inspect: false } },
pickBy(result.right, isNotEmpty)
),
params: validParams,
config,
logger,
},
});

const body = {
...data,
_inspectableEsQueries: inspectableEsQueriesMap.get(request),
};
const body = { ...data };
if (validParams.query._inspect) {
body._inspectableEsQueries = inspectableEsQueriesMap.get(
request
);
}

// cleanup
inspectableEsQueriesMap.delete(request);
Expand Down Expand Up @@ -192,3 +159,36 @@ export function createApi() {

return api;
}

function validateParams(
request: KibanaRequest,
params: RouteParamsRT | undefined
) {
const paramsRt = params ? merge([params, inspectRt]) : inspectRt;
const paramMap = pickBy(
{
path: request.params,
body: request.body,
query: {
_inspect: 'false',
// @ts-ignore
...request.query,
},
},
isNotEmpty
);

const result = strictKeysRt(paramsRt).decode(paramMap);

if (isLeft(result)) {
throw Boom.badRequest(PathReporter.report(result)[0]);
}

// Only return values for parameters that have runtime types,
// but always include query as _inspect is always set even if
// it's not defined in the route.
return mergeLodash(
{ query: { _inspect: false } },
pickBy(result.right, isNotEmpty)
);
}
3 changes: 2 additions & 1 deletion x-pack/plugins/apm/server/routes/observability_overview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export const observabilityOverviewHasDataRoute = createRoute({
options: { tags: ['access:apm'] },
handler: async ({ context, request }) => {
const setup = await setupRequest(context, request);
return { hasData: await getHasData({ setup }) };
const res = await getHasData({ setup });
return { hasData: res };
},
});

Expand Down
11 changes: 8 additions & 3 deletions x-pack/plugins/apm/server/routes/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,13 @@ type MaybeOptional<T extends { params: Record<string, any> }> = RequiredKeys<
? { params?: T['params'] }
: { params: T['params'] };

export type MaybeParams<
TRouteState,
TEndpoint extends keyof TRouteState & string
> = TRouteState[TEndpoint] extends { params: t.Any }
? MaybeOptional<{ params: t.OutputOf<TRouteState[TEndpoint]['params']> }>
: {};

export type Client<
TRouteState,
TOptions extends { abortable: boolean } = { abortable: true }
Expand All @@ -144,9 +151,7 @@ export type Client<
> & {
forceCache?: boolean;
endpoint: TEndpoint;
} & (TRouteState[TEndpoint] extends { params: t.Any }
? MaybeOptional<{ params: t.OutputOf<TRouteState[TEndpoint]['params']> }>
: {}) &
} & MaybeParams<TRouteState, TEndpoint> &
(TOptions extends { abortable: true } ? { signal: AbortSignal | null } : {})
) => Promise<
TRouteState[TEndpoint] extends { ret: any }
Expand Down
50 changes: 50 additions & 0 deletions x-pack/test/apm_api_integration/common/apm_api_supertest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { format } from 'url';
import supertest from 'supertest';
import { MaybeParams } from '../../../plugins/apm/server/routes/typings';
import { parseEndpoint } from '../../../plugins/apm/common/apm_api/parse_endpoint';
import { APMAPI } from '../../../plugins/apm/server/routes/create_apm_api';
import type { APIReturnType } from '../../../plugins/apm/public/services/rest/createCallApmApi';

export function createApmApiSupertest(st: supertest.SuperTest<supertest.Test>) {
return async <TPath extends keyof APMAPI['_S']>(
options: {
endpoint: TPath;
} & MaybeParams<APMAPI['_S'], TPath>
): Promise<{
status: number;
body: APIReturnType<TPath>;
}> => {
const { endpoint } = options;

// @ts-expect-error
const params = 'params' in options ? options.params : {};

const { method, pathname } = parseEndpoint(endpoint, params?.path);
const url = format({ pathname, query: params?.query });

const res = params.body
? await st[method](url).send(params.body).set('kbn-xsrf', 'foo')
: await st[method](url).set('kbn-xsrf', 'foo');

// supertest doesn't throw on http errors
if (res.status !== 200) {
const e = new Error(
`Unhandled ApmApiSupertest error. Status: "${
res.status
}". Endpoint: "${endpoint}". ${JSON.stringify(res.body)}`
);
// @ts-expect-error
e.res = res;
throw e;
}

return res;
};
}
4 changes: 2 additions & 2 deletions x-pack/test/apm_api_integration/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { FtrConfigProviderContext } from '@kbn/test/types/ftr';
import supertestAsPromised from 'supertest-as-promised';
import supertest from 'supertest';
Copy link
Copy Markdown
Owner Author

@sorenlouv sorenlouv Mar 25, 2021

Choose a reason for hiding this comment

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

supertest-as-promised is deprecated since 2016: WhoopInc/supertest-as-promised@ab56510

fyi: @spalger perhaps the dep should be removed from kibana entirely?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's planned elastic#95267

Copy link
Copy Markdown
Owner Author

@sorenlouv sorenlouv Mar 25, 2021

Choose a reason for hiding this comment

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

Wow, what a coincidence that I notice that the day after Tyler created that issue :D Great!

import { format, UrlObject } from 'url';
import path from 'path';
import { InheritedFtrProviderContext, InheritedServices } from './ftr_provider_context';
Expand All @@ -33,7 +33,7 @@ const supertestAsApmUser = (kibanaServer: UrlObject, apmUser: ApmUser) => async
auth: `${apmUser}:${APM_TEST_PASSWORD}`,
});

return supertestAsPromised(url);
return supertest(url);
};

export function createTestConfig(config: Config) {
Expand Down
Loading