[APM] Use callWithInternalUser for agent configuration endpoin… (elas…

…tic#50211) * [APM] Use callWithInternalUser for agent configuration endpoints Closes elastic#50050. * Review feedback * Use internalClient for agent conf queries only
dgieselaar · Nov 13, 2019 · 46879ee · 46879ee
1 parent 7dc6d62
commit 46879ee
Show file tree

Hide file tree

Showing 21 changed files with 133 additions and 38 deletions.
diff --git a/x-pack/legacy/plugins/apm/index.ts b/x-pack/legacy/plugins/apm/index.ts
@@ -91,7 +91,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;
@@ -113,12 +114,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
@@ -135,8 +145,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
@@ -84,10 +84,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 StringMap;
 
+  const callMethod = clientAsInternalUser
+    ? cluster.callWithInternalUser.bind(cluster)
+    : cluster.callWithRequest.bind(cluster, req);
+
   return {
     search: async <Hits = unknown, U extends SearchParams = {}>(
       params: U,
@@ -110,22 +123,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<
+      return (callMethod('search', nextParams) as unknown) as Promise<
         AggregationSearchResponseWithTotalHitsAsObject<Hits, U>
       >;
     },
     index: <Body>(params: IndexDocumentParams<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
@@ -10,21 +10,25 @@ import { uiSettingsServiceMock } from 'src/core/server/mocks';
 
 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', () => {
@@ -46,6 +50,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
@@ -36,7 +36,8 @@ export async function setupRequest(req: Legacy.Request) {
     start: moment.utc(query.start).valueOf(),
     end: moment.utc(query.end).valueOf(),
     uiFiltersES: await decodeUiFilters(server, query.uiFilters),
-    client: getESClient(req),
+    client: getESClient(req, { clientAsInternalUser: false }),
+    internalClient: getESClient(req, { clientAsInternalUser: true }),
     config
   };
 }
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, config } = setup;
+  const { internalClient, config } = setup;
 
   const params: IndexDocumentParams<AgentConfiguration> = {
     type: '_doc',
@@ -45,5 +45,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, config } = setup;
+  const { internalClient, config } = setup;
 
   const params = {
     refresh: 'wait_for',
     index: config.get<string>('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, config } = setup;
+  const { internalClient, config } = 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);
 }
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, config } = setup;
+  const { internalClient, config } = setup;
 
   const params = {
     index: config.get<string>('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, config } = setup;
+  const { internalClient, config } = setup;
 
   const params = {
     type: '_doc',
@@ -28,5 +28,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
@@ -15,7 +15,8 @@ describe('search configurations', () => {
       environment: 'production',
       setup: ({
         config: { get: () => '' },
-        client: { search: async () => searchMocks }
+        client: { search: async () => searchMocks },
+        internalClient: { search: async () => searchMocks }
       } as unknown) as Setup
     });
 
@@ -29,7 +30,8 @@ describe('search configurations', () => {
       environment: 'production',
       setup: ({
         config: { get: () => '' },
-        client: { search: async () => searchMocks }
+        client: { search: async () => searchMocks },
+        internalClient: { search: async () => searchMocks }
       } as unknown) as Setup
     });
 

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
@@ -21,7 +21,7 @@ export async function searchConfigurations({
   environment?: string;
   setup: Setup;
 }) {
-  const { client, config } = setup;
+  const { internalClient, config } = setup;
 
   // sorting order
   // 1. exact match: service.name AND service.environment (eg. opbeans-node / production)
@@ -50,7 +50,7 @@ export async function searchConfigurations({
     }
   };
 
-  const resp = await client.search<AgentConfiguration>(params);
+  const resp = await internalClient.search<AgentConfiguration>(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
@@ -20,6 +20,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -43,6 +44,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -83,6 +85,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -122,6 +125,7 @@ describe('getTransactionBreakdown', () => {
         start: 0,
         end: 500000,
         client: { search: clientSpy } as any,
+        internalClient: { search: clientSpy } as any,
         config: {
           get: () => 'myIndex' as any,
           has: () => true
@@ -145,6 +149,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