[APM] Version public APIs (#158233)

Closes #155245 Adds a version to the `endpoint` name for public APIs. It's required for public APIs and enforced via type checks. Example: `GET /api/apm/settings/agent-configuration/environments 2023-05-22`. The reason why it's part of the endpoint is because it's the simplest change to make, requires very little runtime changes and doesn't dramatically change the way we handle type-safety for our server routes. --------- Co-authored-by: kibanamachine <[email protected]>
elastic · May 23, 2023 · 1908568 · 1908568
1 parent c8cd3b5
commit 1908568
Show file tree

Hide file tree

Showing 33 changed files with 328 additions and 236 deletions.
diff --git a/packages/kbn-server-route-repository/src/format_request.ts b/packages/kbn-server-route-repository/src/format_request.ts
@@ -9,12 +9,12 @@
 import { parseEndpoint } from './parse_endpoint';
 
 export function formatRequest(endpoint: string, pathParams: Record<string, any> = {}) {
-  const { method, pathname: rawPathname } = parseEndpoint(endpoint);
+  const { method, pathname: rawPathname, version } = parseEndpoint(endpoint);
 
   // replace template variables with path params
   const pathname = Object.keys(pathParams).reduce((acc, paramName) => {
     return acc.replace(`{${paramName}}`, pathParams[paramName]);
   }, rawPathname);
 
-  return { method, pathname };
+  return { method, pathname, version };
 }
diff --git a/packages/kbn-server-route-repository/src/parse_endpoint.ts b/packages/kbn-server-route-repository/src/parse_endpoint.ts
@@ -13,10 +13,15 @@ export function parseEndpoint(endpoint: string) {
 
   const method = parts[0].trim().toLowerCase() as Method;
   const pathname = parts[1].trim();
+  const version = parts[2]?.trim();
 
   if (!['get', 'post', 'put', 'delete'].includes(method)) {
-    throw new Error('Endpoint was not prefixed with a valid HTTP method');
+    throw new Error(`Endpoint ${endpoint} was not prefixed with a valid HTTP method`);
   }
 
-  return { method, pathname };
+  if (!version && pathname.startsWith('/api')) {
+    throw new Error(`Missing version for public endpoint ${endpoint}`);
+  }
+
+  return { method, pathname, version };
 }
diff --git a/packages/kbn-server-route-repository/src/test_types.ts b/packages/kbn-server-route-repository/src/test_types.ts
@@ -17,7 +17,7 @@ function assertType<TShape = never>(value: TShape) {
 // If a params codec is not set, its type should not be available in
 // the request handler.
 createServerRouteFactory<{}, {}>()({
-  endpoint: 'endpoint_without_params',
+  endpoint: 'GET /internal/endpoint_without_params',
   handler: async (resources) => {
     // @ts-expect-error Argument of type '{}' is not assignable to parameter of type '{ params: any; }'.
     assertType<{ params: any }>(resources);
@@ -27,7 +27,7 @@ createServerRouteFactory<{}, {}>()({
 // If a params codec is set, its type _should_ be available in the
 // request handler.
 createServerRouteFactory<{}, {}>()({
-  endpoint: 'endpoint_with_params',
+  endpoint: 'GET /internal/endpoint_with_params',
   params: t.type({
     path: t.type({
       serviceName: t.string,
@@ -40,7 +40,7 @@ createServerRouteFactory<{}, {}>()({
 
 // Resources should be passed to the request handler.
 createServerRouteFactory<{ context: { getSpaceId: () => string } }, {}>()({
-  endpoint: 'endpoint_with_params',
+  endpoint: 'GET /internal/endpoint_with_params',
   params: t.type({
     path: t.type({
       serviceName: t.string,
@@ -54,7 +54,7 @@ createServerRouteFactory<{ context: { getSpaceId: () => string } }, {}>()({
 
 // Create options are available when registering a route.
 createServerRouteFactory<{}, { options: { tags: string[] } }>()({
-  endpoint: 'endpoint_with_params',
+  endpoint: 'GET /internal/endpoint_with_params',
   params: t.type({
     path: t.type({
       serviceName: t.string,
@@ -68,19 +68,39 @@ createServerRouteFactory<{}, { options: { tags: string[] } }>()({
   },
 });
 
+// Public APIs should be versioned
+createServerRouteFactory<{}, { options: { tags: string[] } }>()({
+  // @ts-expect-error
+  endpoint: 'GET /api/endpoint_with_params',
+  options: {
+    // @ts-expect-error
+    tags: [],
+  },
+  // @ts-expect-error
+  handler: async (resources) => {},
+});
+
+createServerRouteFactory<{}, { options: { tags: string[] } }>()({
+  endpoint: 'GET /api/endpoint_with_params 2023-05-22',
+  options: {
+    tags: [],
+  },
+  handler: async (resources) => {},
+});
+
 const createServerRoute = createServerRouteFactory<{}, {}>();
 
 const repository = {
   ...createServerRoute({
-    endpoint: 'endpoint_without_params',
+    endpoint: 'GET /internal/endpoint_without_params',
     handler: async () => {
       return {
         noParamsForMe: true,
       };
     },
   }),
   ...createServerRoute({
-    endpoint: 'endpoint_with_params',
+    endpoint: 'GET /internal/endpoint_with_params',
     params: t.type({
       path: t.type({
         serviceName: t.string,
@@ -93,7 +113,7 @@ const repository = {
     },
   }),
   ...createServerRoute({
-    endpoint: 'endpoint_with_optional_params',
+    endpoint: 'GET /internal/endpoint_with_optional_params',
     params: t.partial({
       query: t.partial({
         serviceName: t.string,
@@ -112,21 +132,21 @@ type TestRepository = typeof repository;
 // EndpointOf should return all valid endpoints of a repository
 
 assertType<Array<EndpointOf<TestRepository>>>([
-  'endpoint_with_params',
-  'endpoint_without_params',
-  'endpoint_with_optional_params',
+  'GET /internal/endpoint_with_params',
+  'GET /internal/endpoint_without_params',
+  'GET /internal/endpoint_with_optional_params',
 ]);
 
 // @ts-expect-error Type '"this_endpoint_does_not_exist"' is not assignable to type '"endpoint_without_params" | "endpoint_with_params" | "endpoint_with_optional_params"'
 assertType<Array<EndpointOf<TestRepository>>>(['this_endpoint_does_not_exist']);
 
 // ReturnOf should return the return type of a request handler.
 
-assertType<ReturnOf<TestRepository, 'endpoint_without_params'>>({
+assertType<ReturnOf<TestRepository, 'GET /internal/endpoint_without_params'>>({
   noParamsForMe: true,
 });
 
-const noParamsInvalid: ReturnOf<TestRepository, 'endpoint_without_params'> = {
+const noParamsInvalid: ReturnOf<TestRepository, 'GET /internal/endpoint_without_params'> = {
   // @ts-expect-error type '{ paramsForMe: boolean; }' is not assignable to type '{ noParamsForMe: boolean; }'.
   paramsForMe: true,
 };
@@ -140,21 +160,21 @@ const client: TestClient = {} as any;
 // It should respect any additional create options.
 
 // @ts-expect-error Property 'timeout' is missing
-client('endpoint_without_params', {});
+client('GET /internal/endpoint_without_params', {});
 
-client('endpoint_without_params', {
+client('GET /internal/endpoint_without_params', {
   timeout: 1,
 });
 
 // It does not allow params for routes without a params codec
-client('endpoint_without_params', {
+client('GET /internal/endpoint_without_params', {
   // @ts-expect-error Object literal may only specify known properties, and 'params' does not exist in type
   params: {},
   timeout: 1,
 });
 
 // It requires params for routes with a params codec
-client('endpoint_with_params', {
+client('GET /internal/endpoint_with_params', {
   params: {
     // @ts-expect-error property 'serviceName' is missing in type '{}'
     path: {},
@@ -163,12 +183,12 @@ client('endpoint_with_params', {
 });
 
 // Params are optional if the codec has no required keys
-client('endpoint_with_optional_params', {
+client('GET /internal/endpoint_with_optional_params', {
   timeout: 1,
 });
 
 // If optional, an error will still occur if the params do not match
-client('endpoint_with_optional_params', {
+client('GET /internal/endpoint_with_optional_params', {
   timeout: 1,
   params: {
     // @ts-expect-error Object literal may only specify known properties, and 'path' does not exist in type
@@ -177,7 +197,7 @@ client('endpoint_with_optional_params', {
 });
 
 // The return type is correctly inferred
-client('endpoint_with_params', {
+client('GET /internal/endpoint_with_params', {
   params: {
     path: {
       serviceName: '',

diff --git a/packages/kbn-server-route-repository/src/typings.ts b/packages/kbn-server-route-repository/src/typings.ts
@@ -35,20 +35,32 @@ export interface RouteState {
 export type ServerRouteHandlerResources = Record<string, any>;
 export type ServerRouteCreateOptions = Record<string, any>;
 
+type ValidateEndpoint<TEndpoint extends string> = string extends TEndpoint
+  ? true
+  : TEndpoint extends `${string} ${string} ${string}`
+  ? true
+  : TEndpoint extends `${string} ${infer TPathname}`
+  ? TPathname extends `/internal/${string}`
+    ? true
+    : false
+  : false;
+
 export type ServerRoute<
   TEndpoint extends string,
   TRouteParamsRT extends RouteParamsRT | undefined,
   TRouteHandlerResources extends ServerRouteHandlerResources,
   TReturnType,
   TRouteCreateOptions extends ServerRouteCreateOptions
-> = {
-  endpoint: TEndpoint;
-  params?: TRouteParamsRT;
-  handler: ({}: TRouteHandlerResources &
-    (TRouteParamsRT extends RouteParamsRT
-      ? DecodedRequestParamsOfType<TRouteParamsRT>
-      : {})) => Promise<TReturnType>;
-} & TRouteCreateOptions;
+> = ValidateEndpoint<TEndpoint> extends true
+  ? {
+      endpoint: TEndpoint;
+      params?: TRouteParamsRT;
+      handler: ({}: TRouteHandlerResources &
+        (TRouteParamsRT extends RouteParamsRT
+          ? DecodedRequestParamsOfType<TRouteParamsRT>
+          : {})) => Promise<TReturnType>;
+    } & TRouteCreateOptions
+  : never;
 
 export type ServerRouteRepository = Record<
   string,

diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.stories.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.stories.tsx
@@ -23,7 +23,7 @@ const stories: Meta<{}> = {
       const transactionTypeStatus = FETCH_STATUS.SUCCESS;
 
       mockApmApiCallResponse(
-        `GET /api/apm/services/{serviceName}/annotation/search`,
+        `GET /api/apm/services/{serviceName}/annotation/search 2023-05-22`,
         () => ({ annotations: [] })
       );
       mockApmApiCallResponse(

diff --git a/...ttings/agent_configurations/agent_configuration_create_edit/service_page/service_page.tsx b/...ttings/agent_configurations/agent_configuration_create_edit/service_page/service_page.tsx
@@ -41,7 +41,7 @@ export function ServicePage({ newConfig, setNewConfig, onClickNext }: Props) {
     (callApmApi) => {
       if (newConfig.service.name) {
         return callApmApi(
-          'GET /api/apm/settings/agent-configuration/environments',
+          'GET /api/apm/settings/agent-configuration/environments 2023-05-22',
           {
             params: {
               query: { serviceName: omitAllOption(newConfig.service.name) },
@@ -65,7 +65,7 @@ export function ServicePage({ newConfig, setNewConfig, onClickNext }: Props) {
       }
 
       const { agentName } = await callApmApi(
-        'GET /api/apm/settings/agent-configuration/agent_name',
+        'GET /api/apm/settings/agent-configuration/agent_name 2023-05-22',
         {
           params: { query: { serviceName } },
         }

diff --git a/...ettings/agent_configurations/agent_configuration_create_edit/settings_page/save_config.ts b/...ettings/agent_configurations/agent_configuration_create_edit/settings_page/save_config.ts
@@ -25,7 +25,7 @@ export async function saveConfig({
   toasts: NotificationsStart['toasts'];
 }) {
   try {
-    await callApmApi('PUT /api/apm/settings/agent-configuration', {
+    await callApmApi('PUT /api/apm/settings/agent-configuration 2023-05-22', {
       signal: null,
       params: {
         query: { overwrite: isEditMode },

diff --git a/x-pack/plugins/apm/public/components/app/settings/agent_configurations/index.tsx b/x-pack/plugins/apm/public/components/app/settings/agent_configurations/index.tsx
@@ -30,7 +30,8 @@ export function AgentConfigurations() {
     data = INITIAL_DATA,
     status,
   } = useFetcher(
-    (callApmApi) => callApmApi('GET /api/apm/settings/agent-configuration'),
+    (callApmApi) =>
+      callApmApi('GET /api/apm/settings/agent-configuration 2023-05-22'),
     [],
     { preservePreviousData: false, showToastOnError: false }
   );

diff --git a/...ins/apm/public/components/app/settings/agent_configurations/list/confirm_delete_modal.tsx b/...ins/apm/public/components/app/settings/agent_configurations/list/confirm_delete_modal.tsx
@@ -17,7 +17,7 @@ import {
 import { useApmPluginContext } from '../../../../../context/apm_plugin/use_apm_plugin_context';
 
 type Config =
-  APIReturnType<'GET /api/apm/settings/agent-configuration'>['configurations'][0];
+  APIReturnType<'GET /api/apm/settings/agent-configuration 2023-05-22'>['configurations'][0];
 
 interface Props {
   config: Config;
@@ -71,17 +71,20 @@ async function deleteConfig(
   toasts: NotificationsStart['toasts']
 ) {
   try {
-    await callApmApi('DELETE /api/apm/settings/agent-configuration', {
-      signal: null,
-      params: {
-        body: {
-          service: {
-            name: config.service.name,
-            environment: config.service.environment,
+    await callApmApi(
+      'DELETE /api/apm/settings/agent-configuration 2023-05-22',
+      {
+        signal: null,
+        params: {
+          body: {
+            service: {
+              name: config.service.name,
+              environment: config.service.environment,
+            },
           },
         },
-      },
-    });
+      }
+    );
 
     toasts.addSuccess({
       title: i18n.translate(

diff --git a/x-pack/plugins/apm/public/components/app/settings/agent_configurations/list/index.tsx b/x-pack/plugins/apm/public/components/app/settings/agent_configurations/list/index.tsx
@@ -29,7 +29,7 @@ import { TimestampTooltip } from '../../../../shared/timestamp_tooltip';
 import { ConfirmDeleteModal } from './confirm_delete_modal';
 
 type Config =
-  APIReturnType<'GET /api/apm/settings/agent-configuration'>['configurations'][0];
+  APIReturnType<'GET /api/apm/settings/agent-configuration 2023-05-22'>['configurations'][0];
 
 interface Props {
   status: FETCH_STATUS;

diff --git a/x-pack/plugins/apm/public/components/app/settings/agent_keys/create_agent_key.tsx b/x-pack/plugins/apm/public/components/app/settings/agent_keys/create_agent_key.tsx
@@ -80,15 +80,18 @@ export function CreateAgentKeyFlyout({ onCancel, onSuccess, onError }: Props) {
         privileges.push(PrivilegeType.AGENT_CONFIG);
       }
 
-      const { agentKey } = await callApmApi('POST /api/apm/agent_keys', {
-        signal: null,
-        params: {
-          body: {
-            name,
-            privileges,
+      const { agentKey } = await callApmApi(
+        'POST /api/apm/agent_keys 2023-05-22',
+        {
+          signal: null,
+          params: {
+            body: {
+              name,
+              privileges,
+            },
           },
-        },
-      });
+        }
+      );
 
       onSuccess(agentKey);
     } catch (error) {

diff --git a/...ck/plugins/apm/public/components/routing/settings/edit_agent_configuration_route_view.tsx b/...ck/plugins/apm/public/components/routing/settings/edit_agent_configuration_route_view.tsx
@@ -21,9 +21,12 @@ export function EditAgentConfigurationRouteView() {
 
   const res = useFetcher(
     (callApmApi) => {
-      return callApmApi('GET /api/apm/settings/agent-configuration/view', {
-        params: { query: { name, environment } },
-      });
+      return callApmApi(
+        'GET /api/apm/settings/agent-configuration/view 2023-05-22',
+        {
+          params: { query: { name, environment } },
+        }
+      );
     },
     [name, environment]
   );

diff --git a/x-pack/plugins/apm/public/context/annotations/annotations_context.tsx b/x-pack/plugins/apm/public/context/annotations/annotations_context.tsx
@@ -32,7 +32,7 @@ export function AnnotationsContextProvider({
     (callApmApi) => {
       if (start && end && serviceName) {
         return callApmApi(
-          'GET /api/apm/services/{serviceName}/annotation/search',
+          'GET /api/apm/services/{serviceName}/annotation/search 2023-05-22',
           {
             params: {
               path: {