[APM] Use callWithInternalUser for agent configuration endpoin… (#50211)

* [APM] Use callWithInternalUser for agent configuration endpoints Closes #50050. * Review feedback * Use internalClient for agent conf queries only
elastic · Nov 13, 2019 · f317c25 · f317c25
1 parent 7bb968c
commit f317c25
Show file tree

Hide file tree

Showing 21 changed files with 135 additions and 36 deletions.
diff --git a/x-pack/legacy/plugins/apm/index.ts b/x-pack/legacy/plugins/apm/index.ts
@@ -87,7 +87,7 @@ export const apm: LegacyPluginInitializer = kibana => {
         catalogue: ['apm'],
         privileges: {
           all: {
-            api: ['apm'],
+            api: ['apm', 'apm_write'],
             catalogue: ['apm'],
             savedObject: {
               all: [],

diff --git a/x-pack/legacy/plugins/apm/public/utils/testHelpers.tsx b/x-pack/legacy/plugins/apm/public/utils/testHelpers.tsx
@@ -97,6 +97,7 @@ interface MockSetup {
   start: number;
   end: number;
   client: any;
+  internalClient: any;
   config: {
     get: any;
     has: any;
@@ -122,12 +123,21 @@ export async function inspectSearchParams(
     }
   });
 
+  const internalClientSpy = jest.fn().mockReturnValueOnce({
+    hits: {
+      total: 0
+    }
+  });
+
   const mockSetup = {
     start: 1528113600000,
     end: 1528977600000,
     client: {
       search: clientSpy
     } as any,
+    internalClient: {
+      search: internalClientSpy
+    } as any,
     config: {
       get: () => 'myIndex' as any,
       has: () => true
@@ -153,8 +163,15 @@ export async function inspectSearchParams(
     // we're only extracting the search params
   }
 
+  let params;
+  if (clientSpy.mock.calls.length) {
+    params = clientSpy.mock.calls[0][0];
+  } else {
+    params = internalClientSpy.mock.calls[0][0];
+  }
+
   return {
-    params: clientSpy.mock.calls[0][0],
+    params,
     teardown: () => clientSpy.mockClear()
   };
 }

diff --git a/x-pack/legacy/plugins/apm/server/lib/errors/distribution/__tests__/get_buckets.test.ts b/x-pack/legacy/plugins/apm/server/lib/errors/distribution/__tests__/get_buckets.test.ts
@@ -31,6 +31,9 @@ describe('timeseriesFetcher', () => {
         client: {
           search: clientSpy
         } as any,
+        internalClient: {
+          search: clientSpy
+        } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true

diff --git a/x-pack/legacy/plugins/apm/server/lib/helpers/es_client.ts b/x-pack/legacy/plugins/apm/server/lib/helpers/es_client.ts
@@ -92,10 +92,23 @@ interface APMOptions {
   includeLegacyData: boolean;
 }
 
-export function getESClient(req: Legacy.Request) {
+interface ClientCreateOptions {
+  clientAsInternalUser?: boolean;
+}
+
+export type ESClient = ReturnType<typeof getESClient>;
+
+export function getESClient(
+  req: Legacy.Request,
+  { clientAsInternalUser = false }: ClientCreateOptions = {}
+) {
   const cluster = req.server.plugins.elasticsearch.getCluster('data');
   const query = req.query as Record<string, unknown>;
 
+  const callMethod = clientAsInternalUser
+    ? cluster.callWithInternalUser.bind(cluster)
+    : cluster.callWithRequest.bind(cluster, req);
+
   return {
     search: async <
       TDocument = unknown,
@@ -121,20 +134,18 @@ export function getESClient(req: Legacy.Request) {
         console.log(JSON.stringify(nextParams.body, null, 4));
       }
 
-      return (cluster.callWithRequest(
-        req,
-        'search',
-        nextParams
-      ) as unknown) as Promise<ESSearchResponse<TDocument, TSearchRequest>>;
+      return (callMethod('search', nextParams) as unknown) as Promise<
+        ESSearchResponse<TDocument, TSearchRequest>
+      >;
     },
     index: <Body>(params: APMIndexDocumentParams<Body>) => {
-      return cluster.callWithRequest(req, 'index', params);
+      return callMethod('index', params);
     },
     delete: (params: IndicesDeleteParams) => {
-      return cluster.callWithRequest(req, 'delete', params);
+      return callMethod('delete', params);
     },
     indicesCreate: (params: IndicesCreateParams) => {
-      return cluster.callWithRequest(req, 'indices.create', params);
+      return callMethod('indices.create', params);
     }
   };
 }
diff --git a/x-pack/legacy/plugins/apm/server/lib/helpers/setup_request.test.ts b/x-pack/legacy/plugins/apm/server/lib/helpers/setup_request.test.ts
@@ -21,21 +21,25 @@ jest.mock('../settings/apm_indices/get_apm_indices', () => ({
 
 function getMockRequest() {
   const callWithRequestSpy = jest.fn();
+  const callWithInternalUserSpy = jest.fn();
   const mockRequest = ({
     params: {},
     query: {},
     server: {
       config: () => ({ get: () => 'apm-*' }),
       plugins: {
         elasticsearch: {
-          getCluster: () => ({ callWithRequest: callWithRequestSpy })
+          getCluster: () => ({
+            callWithRequest: callWithRequestSpy,
+            callWithInternalUser: callWithInternalUserSpy
+          })
         }
       }
     },
     getUiSettingsService: () => ({ get: async () => false })
   } as any) as Legacy.Request;
 
-  return { callWithRequestSpy, mockRequest };
+  return { callWithRequestSpy, callWithInternalUserSpy, mockRequest };
 }
 
 describe('setupRequest', () => {
@@ -57,6 +61,27 @@ describe('setupRequest', () => {
     });
   });
 
+  it('should call callWithInternalUser with default args', async () => {
+    const { mockRequest, callWithInternalUserSpy } = getMockRequest();
+    const { internalClient } = await setupRequest(mockRequest);
+    await internalClient.search({
+      index: 'apm-*',
+      body: { foo: 'bar' }
+    } as any);
+    expect(callWithInternalUserSpy).toHaveBeenCalledWith('search', {
+      index: 'apm-*',
+      body: {
+        foo: 'bar',
+        query: {
+          bool: {
+            filter: [{ range: { 'observer.version_major': { gte: 7 } } }]
+          }
+        }
+      },
+      ignore_throttled: true
+    });
+  });
+
   describe('observer.version_major filter', () => {
     describe('if index is apm-*', () => {
       it('should merge `observer.version_major` filter with existing boolean filters', async () => {

diff --git a/x-pack/legacy/plugins/apm/server/lib/helpers/setup_request.ts b/x-pack/legacy/plugins/apm/server/lib/helpers/setup_request.ts
@@ -41,7 +41,8 @@ export async function setupRequest(req: Legacy.Request) {
     start: moment.utc(query.start).valueOf(),
     end: moment.utc(query.end).valueOf(),
     uiFiltersES,
-    client: getESClient(req),
+    client: getESClient(req, { clientAsInternalUser: false }),
+    internalClient: getESClient(req, { clientAsInternalUser: true }),
     config,
     indices
   };

diff --git a/...acy/plugins/apm/server/lib/settings/agent_configuration/create_or_update_configuration.ts b/...acy/plugins/apm/server/lib/settings/agent_configuration/create_or_update_configuration.ts
@@ -21,7 +21,7 @@ export async function createOrUpdateConfiguration({
   >;
   setup: Setup;
 }) {
-  const { client, indices } = setup;
+  const { internalClient, indices } = setup;
 
   const params: APMIndexDocumentParams<AgentConfiguration> = {
     refresh: true,
@@ -44,5 +44,5 @@ export async function createOrUpdateConfiguration({
     params.id = configurationId;
   }
 
-  return client.index(params);
+  return internalClient.index(params);
 }
diff --git a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/delete_configuration.ts b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/delete_configuration.ts
@@ -13,13 +13,13 @@ export async function deleteConfiguration({
   configurationId: string;
   setup: Setup;
 }) {
-  const { client, indices } = setup;
+  const { internalClient, indices } = setup;
 
   const params = {
     refresh: 'wait_for',
     index: indices['apm_oss.apmAgentConfigurationIndex'],
     id: configurationId
   };
 
-  return client.delete(params);
+  return internalClient.delete(params);
 }
diff --git a/...ib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts b/...ib/settings/agent_configuration/get_environments/get_existing_environments_for_service.ts
@@ -19,7 +19,7 @@ export async function getExistingEnvironmentsForService({
   serviceName: string | undefined;
   setup: Setup;
 }) {
-  const { client, indices } = setup;
+  const { internalClient, indices } = setup;
 
   const bool = serviceName
     ? { filter: [{ term: { [SERVICE_NAME]: serviceName } }] }
@@ -42,7 +42,7 @@ export async function getExistingEnvironmentsForService({
     }
   };
 
-  const resp = await client.search(params);
+  const resp = await internalClient.search(params);
   const buckets = idx(resp.aggregations, _ => _.environments.buckets) || [];
   return buckets.map(bucket => bucket.key as string);
 }
diff --git a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/list_configurations.ts b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/list_configurations.ts
@@ -12,13 +12,13 @@ export type AgentConfigurationListAPIResponse = PromiseReturnType<
   typeof listConfigurations
 >;
 export async function listConfigurations({ setup }: { setup: Setup }) {
-  const { client, indices } = setup;
+  const { internalClient, indices } = setup;
 
   const params = {
     index: indices['apm_oss.apmAgentConfigurationIndex']
   };
 
-  const resp = await client.search<AgentConfiguration>(params);
+  const resp = await internalClient.search<AgentConfiguration>(params);
   return resp.hits.hits.map(item => ({
     id: item._id,
     ...item._source

diff --git a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/mark_applied_by_agent.ts b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/mark_applied_by_agent.ts
@@ -16,7 +16,7 @@ export async function markAppliedByAgent({
   body: AgentConfiguration;
   setup: Setup;
 }) {
-  const { client, indices } = setup;
+  const { internalClient, indices } = setup;
 
   const params = {
     index: indices['apm_oss.apmAgentConfigurationIndex'],
@@ -27,5 +27,5 @@ export async function markAppliedByAgent({
     }
   };
 
-  return client.index<AgentConfiguration>(params);
+  return internalClient.index<AgentConfiguration>(params);
 }
diff --git a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/search.test.ts b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/search.test.ts
@@ -16,6 +16,7 @@ describe('search configurations', () => {
       setup: ({
         config: { get: () => '' },
         client: { search: async () => searchMocks },
+        internalClient: { search: async () => searchMocks },
         indices: {
           apm_oss: {
             sourcemapIndices: 'myIndex',
@@ -41,6 +42,7 @@ describe('search configurations', () => {
       setup: ({
         config: { get: () => '' },
         client: { search: async () => searchMocks },
+        internalClient: { search: async () => searchMocks },
         indices: {
           apm_oss: {
             sourcemapIndices: 'myIndex',

diff --git a/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/search.ts b/x-pack/legacy/plugins/apm/server/lib/settings/agent_configuration/search.ts
@@ -20,7 +20,7 @@ export async function searchConfigurations({
   environment?: string;
   setup: Setup;
 }) {
-  const { client, indices } = setup;
+  const { internalClient, indices } = setup;
 
   // sorting order
   // 1. exact match: service.name AND service.environment (eg. opbeans-node / production)
@@ -49,7 +49,9 @@ export async function searchConfigurations({
     }
   };
 
-  const resp = await client.search<AgentConfiguration, typeof params>(params);
+  const resp = await internalClient.search<AgentConfiguration, typeof params>(
+    params
+  );
   const { hits } = resp.hits;
 
   const exactMatch = hits.find(

diff --git a/x-pack/legacy/plugins/apm/server/lib/transaction_groups/fetcher.test.ts b/x-pack/legacy/plugins/apm/server/lib/transaction_groups/fetcher.test.ts
@@ -13,6 +13,9 @@ function getSetup() {
     client: {
       search: jest.fn()
     } as any,
+    internalClient: {
+      search: jest.fn()
+    } as any,
     config: {
       get: jest.fn<any, string[]>((key: string) => {
         switch (key) {

diff --git a/x-pack/legacy/plugins/apm/server/lib/transactions/breakdown/index.test.ts b/x-pack/legacy/plugins/apm/server/lib/transactions/breakdown/index.test.ts
@@ -30,6 +30,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -54,6 +55,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -95,6 +97,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -135,6 +138,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -159,6 +163,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true

diff --git a/x-pack/legacy/plugins/apm/server/lib/transactions/charts/get_anomaly_data/index.test.ts b/x-pack/legacy/plugins/apm/server/lib/transactions/charts/get_anomaly_data/index.test.ts
@@ -26,6 +26,7 @@ describe('getAnomalySeries', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true

diff --git a/x-pack/legacy/plugins/apm/server/lib/transactions/charts/get_timeseries_data/fetcher.test.ts b/x-pack/legacy/plugins/apm/server/lib/transactions/charts/get_timeseries_data/fetcher.test.ts
@@ -21,6 +21,7 @@ describe('timeseriesFetcher', () => {
         start: 1528113600000,
         end: 1528977600000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true