From 7654bfcd2e5c93be2261d65068b397e236ad576e Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Thu, 28 Sep 2023 17:20:48 -0400 Subject: [PATCH 01/10] Adds getServices method, tests, and docs --- .../asset_manager/common/constants_routes.ts | 1 + .../plugins/asset_manager/common/types_api.ts | 23 +++++++++++++- x-pack/plugins/asset_manager/docs/api.md | 24 +++++++++++++-- .../public/lib/public_assets_client.test.ts | 27 +++++++++++++++-- .../public/lib/public_assets_client.ts | 16 ++++++++-- .../server/routes/assets/services.ts | 30 +++++-------------- 6 files changed, 90 insertions(+), 31 deletions(-) diff --git a/x-pack/plugins/asset_manager/common/constants_routes.ts b/x-pack/plugins/asset_manager/common/constants_routes.ts index 1aef43f7383bd..d256e4264a035 100644 --- a/x-pack/plugins/asset_manager/common/constants_routes.ts +++ b/x-pack/plugins/asset_manager/common/constants_routes.ts @@ -16,3 +16,4 @@ export const GET_RELATED_ASSETS = base('/assets/related'); export const GET_ASSETS_DIFF = base('/assets/diff'); export const GET_HOSTS = base('/assets/hosts'); +export const GET_SERVICES = base('/assets/services'); diff --git a/x-pack/plugins/asset_manager/common/types_api.ts b/x-pack/plugins/asset_manager/common/types_api.ts index 11b5ea4bda3a4..3b63f26819202 100644 --- a/x-pack/plugins/asset_manager/common/types_api.ts +++ b/x-pack/plugins/asset_manager/common/types_api.ts @@ -196,6 +196,10 @@ export const sizeRT = rt.union([ createLiteralValueFromUndefinedRT(10), ]); export const assetDateRT = rt.union([dateRt, datemathStringRt]); + +/** + * Hosts + */ export const getHostAssetsQueryOptionsRT = rt.exact( rt.partial({ from: assetDateRT, @@ -204,8 +208,25 @@ export const getHostAssetsQueryOptionsRT = rt.exact( }) ); export type GetHostAssetsQueryOptions = rt.TypeOf; - export const getHostAssetsResponseRT = rt.type({ hosts: rt.array(assetRT), }); export type GetHostAssetsResponse = rt.TypeOf; + +/** + * Services + */ +export const getServiceAssetsQueryOptionsRT = rt.exact( + rt.partial({ + from: assetDateRT, + to: assetDateRT, + size: sizeRT, + parent: rt.string, + }) +); + +export type GetServiceAssetsQueryOptions = rt.TypeOf; +export const getServiceAssetsResponseRT = rt.type({ + services: rt.array(assetRT), +}); +export type GetServiceAssetsResponse = rt.TypeOf; diff --git a/x-pack/plugins/asset_manager/docs/api.md b/x-pack/plugins/asset_manager/docs/api.md index 755abfe4be373..66a1984297c16 100644 --- a/x-pack/plugins/asset_manager/docs/api.md +++ b/x-pack/plugins/asset_manager/docs/api.md @@ -165,9 +165,11 @@ it will be able to pull the properly-scoped client dependencies off of that requ ### Client methods +TODO: Link to a centralized asset document example that each response can reference? + #### getHosts -Get a group of host assets found within a specified time range. +Get a list of host assets found within a specified time range. | Parameter | Type | Required? | Description | | :-------- | :-------------- | :-------- | :--------------------------------------------------------------------- | @@ -184,4 +186,22 @@ Get a group of host assets found within a specified time range. } ``` -TODO: Link to a centralized asset document example that each response can reference? +#### getServices + +Get a list of service assets found within a specified time range. + +| Parameter | Type | Required? | Description | +| :-------- | :-------------- | :-------- | :--------------------------------------------------------------------- | +| from | datetime string | yes | ISO date string representing the START of the time range being queried | +| to | datetime string | yes | ISO date string representing the END of the time range being queried | +| parent | string | no | EAN value for a given parent service to filter services by | + +**Response** + +```json +{ + "services": [ + ...found service assets + ] +} +``` diff --git a/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts b/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts index 93cc541a34af4..2399db9b1296e 100644 --- a/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts +++ b/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts @@ -40,9 +40,32 @@ describe('Public assets client', () => { it('should return the direct results of http.get', async () => { const client = new PublicAssetsClient(http); - http.get.mockResolvedValueOnce('my result'); + http.get.mockResolvedValueOnce('my hosts'); const result = await client.getHosts({ from: 'x', to: 'y' }); - expect(result).toBe('my result'); + expect(result).toBe('my hosts'); + }); + }); + + describe('getServices', () => { + it('should call the REST API', async () => { + const client = new PublicAssetsClient(http); + await client.getServices({ from: 'x', to: 'y' }); + expect(http.get).toBeCalledTimes(1); + }); + + it('should include specified "from" and "to" parameters in http.get query', async () => { + const client = new PublicAssetsClient(http); + await client.getServices({ from: 'x', to: 'y' }); + expect(http.get).toBeCalledWith(expect.stringContaining('/api/'), { + query: { from: 'x', to: 'y' }, + }); + }); + + it('should return the direct results of http.get', async () => { + const client = new PublicAssetsClient(http); + http.get.mockResolvedValueOnce('my services'); + const result = await client.getServices({ from: 'x', to: 'y' }); + expect(result).toBe('my services'); }); }); }); diff --git a/x-pack/plugins/asset_manager/public/lib/public_assets_client.ts b/x-pack/plugins/asset_manager/public/lib/public_assets_client.ts index dd18386868f94..9509e61e073a6 100644 --- a/x-pack/plugins/asset_manager/public/lib/public_assets_client.ts +++ b/x-pack/plugins/asset_manager/public/lib/public_assets_client.ts @@ -6,9 +6,9 @@ */ import { HttpStart } from '@kbn/core/public'; -import { GetHostsOptionsPublic } from '../../common/types_client'; -import { GetHostAssetsResponse } from '../../common/types_api'; -import { GET_HOSTS } from '../../common/constants_routes'; +import { GetHostsOptionsPublic, GetServicesOptionsPublic } from '../../common/types_client'; +import { GetHostAssetsResponse, GetServiceAssetsResponse } from '../../common/types_api'; +import { GET_HOSTS, GET_SERVICES } from '../../common/constants_routes'; import { IPublicAssetsClient } from '../types'; export class PublicAssetsClient implements IPublicAssetsClient { @@ -23,4 +23,14 @@ export class PublicAssetsClient implements IPublicAssetsClient { return results; } + + async getServices(options: GetServicesOptionsPublic) { + const results = await this.http.get(GET_SERVICES, { + query: { + ...options, + }, + }); + + return results; + } } diff --git a/x-pack/plugins/asset_manager/server/routes/assets/services.ts b/x-pack/plugins/asset_manager/server/routes/assets/services.ts index 3852a0bb60d11..ea3c405940f75 100644 --- a/x-pack/plugins/asset_manager/server/routes/assets/services.ts +++ b/x-pack/plugins/asset_manager/server/routes/assets/services.ts @@ -5,34 +5,18 @@ * 2.0. */ -import * as rt from 'io-ts'; import datemath from '@kbn/datemath'; -import { - dateRt, - inRangeFromStringRt, - datemathStringRt, - createRouteValidationFunction, - createLiteralValueFromUndefinedRT, -} from '@kbn/io-ts-utils'; +import { createRouteValidationFunction } from '@kbn/io-ts-utils'; import { RequestHandlerContext } from '@kbn/core-http-request-handler-context-server'; +import { + GetServiceAssetsQueryOptions, + getServiceAssetsQueryOptionsRT, +} from '../../../common/types_api'; import { debug } from '../../../common/debug_log'; import { SetupRouteOptions } from '../types'; -import { ASSET_MANAGER_API_BASE } from '../../../common/constants_routes'; +import * as routePaths from '../../../common/constants_routes'; import { getClientsFromContext } from '../utils'; -const sizeRT = rt.union([inRangeFromStringRt(1, 100), createLiteralValueFromUndefinedRT(10)]); -const assetDateRT = rt.union([dateRt, datemathStringRt]); -const getServiceAssetsQueryOptionsRT = rt.exact( - rt.partial({ - from: assetDateRT, - to: assetDateRT, - size: sizeRT, - parent: rt.string, - }) -); - -export type GetServiceAssetsQueryOptions = rt.TypeOf; - export function servicesRoutes({ router, assetClient, @@ -40,7 +24,7 @@ export function servicesRoutes({ // GET /assets/services router.get( { - path: `${ASSET_MANAGER_API_BASE}/assets/services`, + path: routePaths.GET_SERVICES, validate: { query: createRouteValidationFunction(getServiceAssetsQueryOptionsRT), }, From fb298e3afcf0cd6de9eba05106617ad66b161771 Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Fri, 29 Sep 2023 14:16:05 -0400 Subject: [PATCH 02/10] Fixes types and adds EAN parsing for getServices --- .../public/lib/public_assets_client.test.ts | 10 +++++- .../accessors/hosts/get_hosts_by_assets.ts | 2 +- .../accessors/hosts/get_hosts_by_signals.ts | 2 +- .../server/lib/accessors/hosts/index.ts | 15 ++------- .../lib/accessors/hosts/shared_types.ts | 14 ++++++++ .../server/lib/accessors/index.ts | 19 ----------- .../services/get_services_by_assets.ts | 2 +- .../services/get_services_by_signals.ts | 31 ++++++++++++------ .../server/lib/accessors/services/index.ts | 16 ++-------- .../lib/accessors/services/shared_types.ts | 15 +++++++++ .../asset_manager/server/lib/asset_client.ts | 30 +++++++---------- .../server/lib/asset_client_types.ts | 32 +++++++++++++++++++ .../server/lib/collectors/services.ts | 3 ++ .../asset_manager/server/lib/parse_ean.ts | 16 ++++++++++ x-pack/plugins/asset_manager/server/types.ts | 7 +--- 15 files changed, 131 insertions(+), 83 deletions(-) create mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/index.ts create mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts create mode 100644 x-pack/plugins/asset_manager/server/lib/asset_client_types.ts create mode 100644 x-pack/plugins/asset_manager/server/lib/parse_ean.ts diff --git a/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts b/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts index 2399db9b1296e..5eca030e832db 100644 --- a/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts +++ b/x-pack/plugins/asset_manager/public/lib/public_assets_client.test.ts @@ -56,11 +56,19 @@ describe('Public assets client', () => { it('should include specified "from" and "to" parameters in http.get query', async () => { const client = new PublicAssetsClient(http); await client.getServices({ from: 'x', to: 'y' }); - expect(http.get).toBeCalledWith(expect.stringContaining('/api/'), { + expect(http.get).toBeCalledWith(routePaths.GET_SERVICES, { query: { from: 'x', to: 'y' }, }); }); + it('should include specified "parent" parameter in http.get query', async () => { + const client = new PublicAssetsClient(http); + await client.getServices({ from: 'x', to: 'y', parent: 'container:123' }); + expect(http.get).toBeCalledWith(routePaths.GET_SERVICES, { + query: { from: 'x', to: 'y', parent: 'container:123' }, + }); + }); + it('should return the direct results of http.get', async () => { const client = new PublicAssetsClient(http); http.get.mockResolvedValueOnce('my services'); diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts index f975df1cd82f4..0423bc40ba9cf 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts @@ -5,8 +5,8 @@ * 2.0. */ +import type { GetHostsOptionsInjected } from './shared_types'; import { Asset } from '../../../../common/types_api'; -import { GetHostsOptionsInjected } from '.'; import { getAssets } from '../../get_assets'; export async function getHostsByAssets( diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts index 93e601ae00f9c..1acc7a495e331 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts @@ -5,8 +5,8 @@ * 2.0. */ +import type { GetHostsOptionsInjected } from './shared_types'; import { Asset } from '../../../../common/types_api'; -import { GetHostsOptionsInjected } from '.'; import { collectHosts } from '../../collectors/hosts'; export async function getHostsBySignals( diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts index 1b60268d85389..974c0a51066c3 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts @@ -5,15 +5,6 @@ * 2.0. */ -import type { AssetClientDependencies } from '../../../types'; -import type { GetHostsOptionsPublic } from '../../../../common/types_client'; -import type { OptionsWithInjectedValues } from '..'; - -export type GetHostsOptions = GetHostsOptionsPublic & AssetClientDependencies; -export type GetHostsOptionsInjected = OptionsWithInjectedValues; - -export interface HostIdentifier { - 'asset.ean': string; - 'asset.id': string; - 'asset.name'?: string; -} +export type { GetHostsOptions, GetHostsOptionsInjected } from './shared_types'; +export { getHostsByAssets } from './get_hosts_by_assets'; +export { getHostsBySignals } from './get_hosts_by_signals'; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts new file mode 100644 index 0000000000000..13aebe0bacc76 --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts @@ -0,0 +1,14 @@ +/* + * 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 { GetHostsOptionsPublic } from '../../../../common/types_client'; +import { + AssetClientDependencies, + AssetClientOptionsWithInjectedValues, +} from '../../asset_client_types'; + +export type GetHostsOptions = GetHostsOptionsPublic & AssetClientDependencies; +export type GetHostsOptionsInjected = AssetClientOptionsWithInjectedValues; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/index.ts deleted file mode 100644 index 6fd9254a2182e..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/index.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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 { APMDataAccessConfig } from '@kbn/apm-data-access-plugin/server'; -import { MetricsDataClient } from '@kbn/metrics-data-access-plugin/server'; -import { SavedObjectsClientContract } from '@kbn/core/server'; -import { AssetManagerConfig } from '../../../common/config'; - -export interface InjectedValues { - sourceIndices: AssetManagerConfig['sourceIndices']; - getApmIndices: (soClient: SavedObjectsClientContract) => Promise; - metricsClient: MetricsDataClient; -} - -export type OptionsWithInjectedValues = T & InjectedValues; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts index 8e69bcbff4625..53d45d64179a4 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts @@ -6,7 +6,7 @@ */ import { Asset } from '../../../../common/types_api'; -import { GetServicesOptionsInjected } from '.'; +import { GetServicesOptionsInjected } from './shared_types'; import { getAssets } from '../../get_assets'; import { getAllRelatedAssets } from '../../get_all_related_assets'; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts index 720d6b3e30531..d74501b233b6e 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts @@ -6,8 +6,9 @@ */ import { Asset } from '../../../../common/types_api'; -import { GetServicesOptionsInjected } from '.'; +import { GetServicesOptionsInjected } from './shared_types'; import { collectServices } from '../../collectors/services'; +import { parseEan } from '../../parse_ean'; export async function getServicesBySignals( options: GetServicesOptionsInjected @@ -15,15 +16,25 @@ export async function getServicesBySignals( const filters = []; if (options.parent) { - filters.push({ - bool: { - should: [ - { term: { 'host.name': options.parent } }, - { term: { 'host.hostname': options.parent } }, - ], - minimum_should_match: 1, - }, - }); + const { kind, id } = parseEan(options.parent); + + if (kind === 'host') { + filters.push({ + bool: { + should: [{ term: { 'host.name': id } }, { term: { 'host.hostname': id } }], + minimum_should_match: 1, + }, + }); + } + + if (kind === 'container') { + filters.push({ + bool: { + should: [{ term: { 'container.id': id } }], + minimum_should_match: 1, + }, + }); + } } const apmIndices = await options.getApmIndices(options.savedObjectsClient); diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts index e8b52e4924c4d..a8a864f0ce2fd 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts @@ -5,16 +5,6 @@ * 2.0. */ -import { AssetClientDependencies } from '../../../types'; -import { GetServicesOptionsPublic } from '../../../../common/types_client'; -import { OptionsWithInjectedValues } from '..'; - -export type GetServicesOptions = GetServicesOptionsPublic & AssetClientDependencies; -export type GetServicesOptionsInjected = OptionsWithInjectedValues; - -export interface ServiceIdentifier { - 'asset.ean': string; - 'asset.id': string; - 'asset.name'?: string; - 'service.environment'?: string; -} +export type { GetServicesOptions, GetServicesOptionsInjected } from './shared_types'; +export { getServicesByAssets } from './get_services_by_assets'; +export { getServicesBySignals } from './get_services_by_signals'; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts new file mode 100644 index 0000000000000..7edef8d0e2646 --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts @@ -0,0 +1,15 @@ +/* + * 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 { GetServicesOptionsPublic } from '../../../../common/types_client'; +import { + AssetClientDependencies, + AssetClientOptionsWithInjectedValues, +} from '../../asset_client_types'; + +export type GetServicesOptions = GetServicesOptionsPublic & AssetClientDependencies; +export type GetServicesOptionsInjected = AssetClientOptionsWithInjectedValues; diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.ts index 8bf23313c663e..16e70e2a6e69a 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.ts @@ -5,30 +5,22 @@ * 2.0. */ -import { APMDataAccessConfig } from '@kbn/apm-data-access-plugin/server'; -import { MetricsDataClient } from '@kbn/metrics-data-access-plugin/server'; -import { SavedObjectsClientContract } from '@kbn/core/server'; -import { AssetManagerConfig } from '../../common/config'; import { Asset } from '../../common/types_api'; -import { OptionsWithInjectedValues } from './accessors'; -import { GetHostsOptions } from './accessors/hosts'; -import { GetServicesOptions } from './accessors/services'; -import { getHostsByAssets } from './accessors/hosts/get_hosts_by_assets'; -import { getHostsBySignals } from './accessors/hosts/get_hosts_by_signals'; -import { getServicesByAssets } from './accessors/services/get_services_by_assets'; -import { getServicesBySignals } from './accessors/services/get_services_by_signals'; - -interface AssetClientClassOptions { - sourceIndices: AssetManagerConfig['sourceIndices']; - source: AssetManagerConfig['lockedSource']; - getApmIndices: (soClient: SavedObjectsClientContract) => Promise; - metricsClient: MetricsDataClient; -} +import { getHostsByAssets, getHostsBySignals, GetHostsOptions } from './accessors/hosts'; +import { + getServicesByAssets, + getServicesBySignals, + GetServicesOptions, +} from './accessors/services'; +import { + AssetClientClassOptions, + AssetClientOptionsWithInjectedValues, +} from './asset_client_types'; export class AssetClient { constructor(private baseOptions: AssetClientClassOptions) {} - injectOptions(options: T): OptionsWithInjectedValues { + injectOptions(options: T): AssetClientOptionsWithInjectedValues { return { ...options, sourceIndices: this.baseOptions.sourceIndices, diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts b/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts new file mode 100644 index 0000000000000..d3ab980c2fd9f --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts @@ -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. + */ + +import { APMDataAccessConfig } from '@kbn/apm-data-access-plugin/server'; +import { ElasticsearchClient } from '@kbn/core-elasticsearch-server'; +import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; +import { MetricsDataClient } from '@kbn/metrics-data-access-plugin/server'; +import { AssetManagerConfig } from '../../common/config'; + +export interface AssetClientDependencies { + elasticsearchClient: ElasticsearchClient; + savedObjectsClient: SavedObjectsClientContract; +} + +interface AssetClientInjectedValues { + sourceIndices: AssetManagerConfig['sourceIndices']; + getApmIndices: (soClient: SavedObjectsClientContract) => Promise; + metricsClient: MetricsDataClient; +} + +export type AssetClientOptionsWithInjectedValues = T & AssetClientInjectedValues; + +export interface AssetClientClassOptions { + sourceIndices: AssetManagerConfig['sourceIndices']; + source: AssetManagerConfig['lockedSource']; + getApmIndices: (soClient: SavedObjectsClientContract) => Promise; + metricsClient: MetricsDataClient; +} diff --git a/x-pack/plugins/asset_manager/server/lib/collectors/services.ts b/x-pack/plugins/asset_manager/server/lib/collectors/services.ts index cf41dc96cd3f7..e000860c9f4c5 100644 --- a/x-pack/plugins/asset_manager/server/lib/collectors/services.ts +++ b/x-pack/plugins/asset_manager/server/lib/collectors/services.ts @@ -6,6 +6,7 @@ */ import { estypes } from '@elastic/elasticsearch'; +import { debug } from '../../../common/debug_log'; import { Asset } from '../../../common/types_api'; import { CollectorOptions, QUERY_MAX_SIZE } from '.'; @@ -94,6 +95,8 @@ export async function collectServices({ dsl.aggs!.services!.composite!.after = afterKey; } + debug(dsl); + const esResponse = await client.search(dsl); const { after_key: nextKey, buckets = [] } = (esResponse.aggregations?.services || {}) as any; diff --git a/x-pack/plugins/asset_manager/server/lib/parse_ean.ts b/x-pack/plugins/asset_manager/server/lib/parse_ean.ts new file mode 100644 index 0000000000000..e466549a1830f --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/parse_ean.ts @@ -0,0 +1,16 @@ +/* + * 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. + */ + +export function parseEan(ean: string) { + const [kind, id, ...rest] = ean.split(':'); + + if (!kind || !id || rest.length > 0) { + throw new Error(`${ean} is not a valid EAN`); + } + + return { kind, id }; +} diff --git a/x-pack/plugins/asset_manager/server/types.ts b/x-pack/plugins/asset_manager/server/types.ts index 431378c7c9a9f..a3e2b2346e383 100644 --- a/x-pack/plugins/asset_manager/server/types.ts +++ b/x-pack/plugins/asset_manager/server/types.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/server'; +import { ElasticsearchClient } from '@kbn/core/server'; import { ApmDataAccessPluginSetup, ApmDataAccessPluginStart, @@ -23,8 +23,3 @@ export interface AssetManagerPluginSetupDependencies { export interface AssetManagerPluginStartDependencies { apmDataAccess: ApmDataAccessPluginStart; } - -export interface AssetClientDependencies { - elasticsearchClient: ElasticsearchClient; - savedObjectsClient: SavedObjectsClientContract; -} From 7f4c183f9213207225a1971367c8077f5862a1cf Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 29 Sep 2023 18:27:08 +0000 Subject: [PATCH 03/10] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/asset_manager/tsconfig.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/asset_manager/tsconfig.json b/x-pack/plugins/asset_manager/tsconfig.json index 35972189e5287..46b69bfefb8cb 100644 --- a/x-pack/plugins/asset_manager/tsconfig.json +++ b/x-pack/plugins/asset_manager/tsconfig.json @@ -23,6 +23,8 @@ "@kbn/apm-data-access-plugin", "@kbn/core-http-browser-mocks", "@kbn/logging", - "@kbn/metrics-data-access-plugin" + "@kbn/metrics-data-access-plugin", + "@kbn/core-elasticsearch-server", + "@kbn/core-saved-objects-api-server" ] } From 5c7b12c44c8730e3f7c1491655dcc2b67e74e74e Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Tue, 3 Oct 2023 09:53:28 -0400 Subject: [PATCH 04/10] Adds unit tests for server client --- .../asset_manager/common/types_client.ts | 10 +- .../accessors/hosts/get_hosts_by_signals.ts | 2 +- .../server/lib/accessors/hosts/index.ts | 12 +- .../services/get_services_by_signals.ts | 2 +- .../server/lib/accessors/services/index.ts | 12 +- .../server/lib/asset_client.test.ts | 148 ++++++++++++++++++ .../asset_manager/server/lib/asset_client.ts | 10 +- .../server/lib/validators/validate_es_date.ts | 17 ++ .../server/lib/validators/validation_error.ts | 20 +++ .../server/routes/assets/hosts.ts | 16 +- .../server/routes/assets/services.ts | 16 +- .../metrics_data_access/server/client.mock.ts | 23 +++ .../metrics_data_access/server/index.ts | 1 + 13 files changed, 272 insertions(+), 17 deletions(-) create mode 100644 x-pack/plugins/asset_manager/server/lib/asset_client.test.ts create mode 100644 x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts create mode 100644 x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts create mode 100644 x-pack/plugins/metrics_data_access/server/client.mock.ts diff --git a/x-pack/plugins/asset_manager/common/types_client.ts b/x-pack/plugins/asset_manager/common/types_client.ts index 350a168da8965..79d2c11e42abf 100644 --- a/x-pack/plugins/asset_manager/common/types_client.ts +++ b/x-pack/plugins/asset_manager/common/types_client.ts @@ -5,13 +5,13 @@ * 2.0. */ -export interface GetHostsOptionsPublic { +export interface SharedAssetsOptionsPublic { from: string; - to: string; + to?: string; } -export interface GetServicesOptionsPublic { - from: string; - to: string; +export type GetHostsOptionsPublic = SharedAssetsOptionsPublic; + +export interface GetServicesOptionsPublic extends SharedAssetsOptionsPublic { parent?: string; } diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts index 1acc7a495e331..259e0a92a0159 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts @@ -19,7 +19,7 @@ export async function getHostsBySignals( const { assets } = await collectHosts({ client: options.elasticsearchClient, from: options.from, - to: options.to, + to: options.to || 'now', sourceIndices: { metrics: metricsIndices, logs: options.sourceIndices.logs, diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts index 974c0a51066c3..e8f19848632b2 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts @@ -5,6 +5,16 @@ * 2.0. */ -export type { GetHostsOptions, GetHostsOptionsInjected } from './shared_types'; +import { validateESDate } from '../../validators/validate_es_date'; +import type { GetHostsOptions, GetHostsOptionsInjected } from './shared_types'; export { getHostsByAssets } from './get_hosts_by_assets'; export { getHostsBySignals } from './get_hosts_by_signals'; + +export type { GetHostsOptions, GetHostsOptionsInjected }; + +export function validateGetHostsOptions(options: GetHostsOptions) { + validateESDate(options.from); + if (options.to) { + validateESDate(options.to); + } +} diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts index d74501b233b6e..34762ab12523a 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts @@ -41,7 +41,7 @@ export async function getServicesBySignals( const { assets } = await collectServices({ client: options.elasticsearchClient, from: options.from, - to: options.to, + to: options.to || 'now', sourceIndices: { apm: apmIndices, }, diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts index a8a864f0ce2fd..59efe817a0554 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts @@ -5,6 +5,16 @@ * 2.0. */ -export type { GetServicesOptions, GetServicesOptionsInjected } from './shared_types'; +import { validateESDate } from '../../validators/validate_es_date'; +import type { GetServicesOptions, GetServicesOptionsInjected } from './shared_types'; + +export type { GetServicesOptions, GetServicesOptionsInjected }; export { getServicesByAssets } from './get_services_by_assets'; export { getServicesBySignals } from './get_services_by_signals'; + +export function validateGetServicesOptions(options: GetServicesOptions) { + validateESDate(options.from); + if (options.to) { + validateESDate(options.to); + } +} diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts new file mode 100644 index 0000000000000..c3ad67923522e --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts @@ -0,0 +1,148 @@ +/* + * 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 { + ElasticsearchClientMock, + elasticsearchClientMock, +} from '@kbn/core-elasticsearch-client-server-mocks'; +import { AssetClient } from './asset_client'; +import { MetricsDataClient, MetricsDataClientMock } from '@kbn/metrics-data-access-plugin/server'; +import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks'; +import { SearchRequest } from '@elastic/elasticsearch/lib/api/types'; +import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; + +function createAssetClient(source: 'signals' | 'assets', metricsDataClient: MetricsDataClient) { + return new AssetClient({ + source, + sourceIndices: { + logs: 'my-logs*', + }, + getApmIndices: jest.fn(), + metricsClient: metricsDataClient, + }); +} + +describe('Public assets client', () => { + let metricsDataClientMock: MetricsDataClient = MetricsDataClientMock.create(); + let esClientMock: ElasticsearchClientMock = + elasticsearchClientMock.createScopedClusterClient().asCurrentUser; + let soClientMock: jest.Mocked; + + beforeEach(() => { + // Reset mocks + esClientMock = elasticsearchClientMock.createScopedClusterClient().asCurrentUser; + soClientMock = savedObjectsClientMock.create(); + metricsDataClientMock = MetricsDataClientMock.create(); + + // ES returns no results, just enough structure to not blow up + esClientMock.search.mockResolvedValueOnce({ + took: 1, + timed_out: false, + _shards: { + failed: 0, + successful: 1, + total: 1, + }, + hits: { + hits: [], + }, + }); + }); + + describe('class instantiation', () => { + it('should successfully instantiate with source: signals', () => { + createAssetClient('signals', metricsDataClientMock); + }); + + it('should successfully instantiate with source: assets', () => { + createAssetClient('assets', metricsDataClientMock); + }); + }); + + describe('getHosts', () => { + it('should query Elasticsearch via signals correctly', async () => { + const client = createAssetClient('signals', metricsDataClientMock); + + await client.getHosts({ + from: 'now-5d', + to: 'now-3d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }); + + expect(metricsDataClientMock.getMetricIndices).toHaveBeenCalledTimes(1); + expect(metricsDataClientMock.getMetricIndices).toHaveBeenCalledWith({ + savedObjectsClient: soClientMock, + }); + + const dsl = esClientMock.search.mock.lastCall?.[0] as SearchRequest | undefined; + const { bool } = dsl?.query || {}; + expect(bool).toBeDefined(); + + expect(bool?.filter).toEqual([ + { + range: { + '@timestamp': { + gte: 'now-5d', + lte: 'now-3d', + }, + }, + }, + ]); + + expect(bool?.must).toEqual([ + { + exists: { + field: 'host.hostname', + }, + }, + ]); + + expect(bool?.should).toEqual([ + { exists: { field: 'kubernetes.node.name' } }, + { exists: { field: 'kubernetes.pod.uid' } }, + { exists: { field: 'container.id' } }, + ]); + }); + + it('should query Elasticsearch via assets correctly', async () => { + const client = createAssetClient('assets', metricsDataClientMock); + + await client.getHosts({ + from: 'now-5d', + to: 'now-3d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }); + + expect(metricsDataClientMock.getMetricIndices).not.toHaveBeenCalled(); + + const dsl = esClientMock.search.mock.lastCall?.[0] as SearchRequest | undefined; + const { bool } = dsl?.query || {}; + expect(bool).toBeDefined(); + + expect(bool?.filter).toEqual([ + { + range: { + '@timestamp': { + gte: 'now-5d', + lte: 'now-3d', + }, + }, + }, + ]); + + expect(bool?.must).toEqual([ + { + terms: { + 'asset.kind': ['host'], + }, + }, + ]); + }); + }); +}); diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.ts index 16e70e2a6e69a..0cab6e3937ac6 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.ts @@ -6,11 +6,17 @@ */ import { Asset } from '../../common/types_api'; -import { getHostsByAssets, getHostsBySignals, GetHostsOptions } from './accessors/hosts'; +import { + getHostsByAssets, + getHostsBySignals, + validateGetHostsOptions, + GetHostsOptions, +} from './accessors/hosts'; import { getServicesByAssets, getServicesBySignals, GetServicesOptions, + validateGetServicesOptions, } from './accessors/services'; import { AssetClientClassOptions, @@ -30,6 +36,7 @@ export class AssetClient { } async getHosts(options: GetHostsOptions): Promise<{ hosts: Asset[] }> { + validateGetHostsOptions(options); const withInjected = this.injectOptions(options); if (this.baseOptions.source === 'assets') { return await getHostsByAssets(withInjected); @@ -39,6 +46,7 @@ export class AssetClient { } async getServices(options: GetServicesOptions): Promise<{ services: Asset[] }> { + validateGetServicesOptions(options); const withInjected = this.injectOptions(options); if (this.baseOptions.source === 'assets') { return await getServicesByAssets(withInjected); diff --git a/x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts b/x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts new file mode 100644 index 0000000000000..2ecd80a8d0e67 --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts @@ -0,0 +1,17 @@ +/* + * 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 datemath from '@kbn/datemath'; +import { AssetsValidationError } from './validation_error'; + +export function validateESDate(dateString: string) { + try { + datemath.parse(dateString); + } catch (error: any) { + throw new AssetsValidationError(`"${dateString}" is not a valid Elasticsearch date value`); + } +} diff --git a/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts b/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts new file mode 100644 index 0000000000000..05b56afc52991 --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ + +interface ErrorOptions { + statusCode?: number; +} + +export class AssetsValidationError extends Error { + public statusCode: number; + + constructor(message: string, { statusCode = 500 }: ErrorOptions = {}) { + super(message); + this.name = 'AssetsValidationError'; + this.statusCode = statusCode; + } +} diff --git a/x-pack/plugins/asset_manager/server/routes/assets/hosts.ts b/x-pack/plugins/asset_manager/server/routes/assets/hosts.ts index f7780f2ef4a6c..b260193647b4f 100644 --- a/x-pack/plugins/asset_manager/server/routes/assets/hosts.ts +++ b/x-pack/plugins/asset_manager/server/routes/assets/hosts.ts @@ -5,7 +5,6 @@ * 2.0. */ -import datemath from '@kbn/datemath'; import { createRouteValidationFunction } from '@kbn/io-ts-utils'; import { RequestHandlerContext } from '@kbn/core-http-request-handler-context-server'; import { GetHostAssetsQueryOptions, getHostAssetsQueryOptionsRT } from '../../../common/types_api'; @@ -13,6 +12,7 @@ import { debug } from '../../../common/debug_log'; import { SetupRouteOptions } from '../types'; import * as routePaths from '../../../common/constants_routes'; import { getClientsFromContext } from '../utils'; +import { AssetsValidationError } from '../../lib/validators/validation_error'; export function hostsRoutes({ router, @@ -27,13 +27,12 @@ export function hostsRoutes({ }, async (context, req, res) => { const { from = 'now-24h', to = 'now' } = req.query || {}; - const { elasticsearchClient, savedObjectsClient } = await getClientsFromContext(context); try { const response = await assetClient.getHosts({ - from: datemath.parse(from)!.toISOString(), - to: datemath.parse(to)!.toISOString(), + from, + to, elasticsearchClient, savedObjectsClient, }); @@ -41,6 +40,15 @@ export function hostsRoutes({ return res.ok({ body: response }); } catch (error: unknown) { debug('Error while looking up HOST asset records', error); + + if (error instanceof AssetsValidationError) { + return res.customError({ + statusCode: error.statusCode, + body: { + message: `Error while looking up host asset records - ${error.message}`, + }, + }); + } return res.customError({ statusCode: 500, body: { message: 'Error while looking up host asset records - ' + `${error}` }, diff --git a/x-pack/plugins/asset_manager/server/routes/assets/services.ts b/x-pack/plugins/asset_manager/server/routes/assets/services.ts index ea3c405940f75..0e7103512ebf9 100644 --- a/x-pack/plugins/asset_manager/server/routes/assets/services.ts +++ b/x-pack/plugins/asset_manager/server/routes/assets/services.ts @@ -5,7 +5,6 @@ * 2.0. */ -import datemath from '@kbn/datemath'; import { createRouteValidationFunction } from '@kbn/io-ts-utils'; import { RequestHandlerContext } from '@kbn/core-http-request-handler-context-server'; import { @@ -16,6 +15,7 @@ import { debug } from '../../../common/debug_log'; import { SetupRouteOptions } from '../types'; import * as routePaths from '../../../common/constants_routes'; import { getClientsFromContext } from '../utils'; +import { AssetsValidationError } from '../../lib/validators/validation_error'; export function servicesRoutes({ router, @@ -34,8 +34,8 @@ export function servicesRoutes({ const { elasticsearchClient, savedObjectsClient } = await getClientsFromContext(context); try { const response = await assetClient.getServices({ - from: datemath.parse(from)!.toISOString(), - to: datemath.parse(to)!.toISOString(), + from, + to, parent, elasticsearchClient, savedObjectsClient, @@ -44,6 +44,16 @@ export function servicesRoutes({ return res.ok({ body: response }); } catch (error: unknown) { debug('Error while looking up SERVICE asset records', error); + + if (error instanceof AssetsValidationError) { + return res.customError({ + statusCode: error.statusCode, + body: { + message: `Error while looking up service asset records - ${error.message}`, + }, + }); + } + return res.customError({ statusCode: 500, body: { message: 'Error while looking up service asset records - ' + `${error}` }, diff --git a/x-pack/plugins/metrics_data_access/server/client.mock.ts b/x-pack/plugins/metrics_data_access/server/client.mock.ts new file mode 100644 index 0000000000000..327c2890dd264 --- /dev/null +++ b/x-pack/plugins/metrics_data_access/server/client.mock.ts @@ -0,0 +1,23 @@ +/* + * 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 { MetricsDataClient } from './client/client'; + +export const MetricsDataClientMock = { + create: () => + ({ + getDefaultMetricIndices: jest.fn(async () => 'default-metrics-indices'), + getMetricIndices: jest.fn(async () => 'metric-indices'), + updateMetricIndices: jest.fn(async () => ({ + id: 'id', + type: 'mock-type', + attributes: { metricIndices: 'updated-indices' }, + references: [], + })), + setDefaultMetricIndicesHandler: jest.fn(), + } as unknown as MetricsDataClient), +}; diff --git a/x-pack/plugins/metrics_data_access/server/index.ts b/x-pack/plugins/metrics_data_access/server/index.ts index 919f5f357856e..78a18f8c00ef5 100644 --- a/x-pack/plugins/metrics_data_access/server/index.ts +++ b/x-pack/plugins/metrics_data_access/server/index.ts @@ -18,6 +18,7 @@ export type { export { metricsDataSourceSavedObjectName } from './saved_objects/metrics_data_source'; export { MetricsDataClient } from './client'; +export { MetricsDataClientMock } from './client.mock'; export function plugin(context: PluginInitializerContext) { return new MetricsDataPlugin(context); From 27894e8b0d622af9baabe1de1aed081e4a76eeb9 Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Tue, 3 Oct 2023 10:43:29 -0400 Subject: [PATCH 05/10] Removes binary source switching from asset client methods --- x-pack/plugins/asset_manager/common/config.ts | 8 --- .../{get_hosts_by_signals.ts => get_hosts.ts} | 4 +- .../accessors/hosts/get_hosts_by_assets.ts | 27 ---------- .../server/lib/accessors/hosts/index.ts | 3 +- ...services_by_signals.ts => get_services.ts} | 2 +- .../services/get_services_by_assets.ts | 46 ----------------- .../server/lib/accessors/services/index.ts | 3 +- .../server/lib/asset_client.test.ts | 51 ++----------------- .../asset_manager/server/lib/asset_client.ts | 37 +++----------- .../server/lib/asset_client_types.ts | 11 +--- x-pack/plugins/asset_manager/server/plugin.ts | 14 ++--- 11 files changed, 23 insertions(+), 183 deletions(-) rename x-pack/plugins/asset_manager/server/lib/accessors/hosts/{get_hosts_by_signals.ts => get_hosts.ts} (88%) delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts rename x-pack/plugins/asset_manager/server/lib/accessors/services/{get_services_by_signals.ts => get_services.ts} (96%) delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts diff --git a/x-pack/plugins/asset_manager/common/config.ts b/x-pack/plugins/asset_manager/common/config.ts index 0a57e37d497bb..22d1c2ace4578 100644 --- a/x-pack/plugins/asset_manager/common/config.ts +++ b/x-pack/plugins/asset_manager/common/config.ts @@ -23,14 +23,6 @@ export const configSchema = schema.object({ }, { defaultValue: INDEX_DEFAULTS } ), - // Choose an explicit source for asset queries. - // NOTE: This will eventually need to be able to cleverly switch - // between these values based on the availability of data in the - // indices, and possibly for each asset kind/type value. - // For now, we set this explicitly. - lockedSource: schema.oneOf([schema.literal('assets'), schema.literal('signals')], { - defaultValue: 'signals', - }), }); export type AssetManagerConfig = TypeOf; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts similarity index 88% rename from x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts rename to x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts index 259e0a92a0159..bbe663c2d6168 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_signals.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts @@ -9,9 +9,7 @@ import type { GetHostsOptionsInjected } from './shared_types'; import { Asset } from '../../../../common/types_api'; import { collectHosts } from '../../collectors/hosts'; -export async function getHostsBySignals( - options: GetHostsOptionsInjected -): Promise<{ hosts: Asset[] }> { +export async function getHosts(options: GetHostsOptionsInjected): Promise<{ hosts: Asset[] }> { const metricsIndices = await options.metricsClient.getMetricIndices({ savedObjectsClient: options.savedObjectsClient, }); diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts deleted file mode 100644 index 0423bc40ba9cf..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts_by_assets.ts +++ /dev/null @@ -1,27 +0,0 @@ -/* - * 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 type { GetHostsOptionsInjected } from './shared_types'; -import { Asset } from '../../../../common/types_api'; -import { getAssets } from '../../get_assets'; - -export async function getHostsByAssets( - options: GetHostsOptionsInjected -): Promise<{ hosts: Asset[] }> { - const hosts = await getAssets({ - elasticsearchClient: options.elasticsearchClient, - filters: { - kind: 'host', - from: options.from, - to: options.to, - }, - }); - - return { - hosts, - }; -} diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts index e8f19848632b2..a3e8b4b0c10ee 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts @@ -7,8 +7,7 @@ import { validateESDate } from '../../validators/validate_es_date'; import type { GetHostsOptions, GetHostsOptionsInjected } from './shared_types'; -export { getHostsByAssets } from './get_hosts_by_assets'; -export { getHostsBySignals } from './get_hosts_by_signals'; +export { getHosts } from './get_hosts'; export type { GetHostsOptions, GetHostsOptionsInjected }; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts similarity index 96% rename from x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts rename to x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts index 34762ab12523a..899a2e20215dd 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_signals.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts @@ -10,7 +10,7 @@ import { GetServicesOptionsInjected } from './shared_types'; import { collectServices } from '../../collectors/services'; import { parseEan } from '../../parse_ean'; -export async function getServicesBySignals( +export async function getServices( options: GetServicesOptionsInjected ): Promise<{ services: Asset[] }> { const filters = []; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts deleted file mode 100644 index 53d45d64179a4..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services_by_assets.ts +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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 { Asset } from '../../../../common/types_api'; -import { GetServicesOptionsInjected } from './shared_types'; -import { getAssets } from '../../get_assets'; -import { getAllRelatedAssets } from '../../get_all_related_assets'; - -export async function getServicesByAssets( - options: GetServicesOptionsInjected -): Promise<{ services: Asset[] }> { - if (options.parent) { - return getServicesByParent(options); - } - - const services = await getAssets({ - elasticsearchClient: options.elasticsearchClient, - filters: { - kind: 'service', - from: options.from, - to: options.to, - }, - }); - - return { services }; -} - -async function getServicesByParent( - options: GetServicesOptionsInjected -): Promise<{ services: Asset[] }> { - const { descendants } = await getAllRelatedAssets(options.elasticsearchClient, { - from: options.from, - to: options.to, - maxDistance: 5, - kind: ['service'], - size: 100, - relation: 'descendants', - ean: options.parent!, - }); - - return { services: descendants as Asset[] }; -} diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts index 59efe817a0554..0c11addce0dd0 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts @@ -9,8 +9,7 @@ import { validateESDate } from '../../validators/validate_es_date'; import type { GetServicesOptions, GetServicesOptionsInjected } from './shared_types'; export type { GetServicesOptions, GetServicesOptionsInjected }; -export { getServicesByAssets } from './get_services_by_assets'; -export { getServicesBySignals } from './get_services_by_signals'; +export { getServices } from './get_services'; export function validateGetServicesOptions(options: GetServicesOptions) { validateESDate(options.from); diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts index c3ad67923522e..77be9cf746220 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts @@ -15,9 +15,8 @@ import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks import { SearchRequest } from '@elastic/elasticsearch/lib/api/types'; import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; -function createAssetClient(source: 'signals' | 'assets', metricsDataClient: MetricsDataClient) { +function createAssetClient(metricsDataClient: MetricsDataClient) { return new AssetClient({ - source, sourceIndices: { logs: 'my-logs*', }, @@ -54,18 +53,14 @@ describe('Public assets client', () => { }); describe('class instantiation', () => { - it('should successfully instantiate with source: signals', () => { - createAssetClient('signals', metricsDataClientMock); - }); - - it('should successfully instantiate with source: assets', () => { - createAssetClient('assets', metricsDataClientMock); + it('should successfully instantiate', () => { + createAssetClient(metricsDataClientMock); }); }); describe('getHosts', () => { - it('should query Elasticsearch via signals correctly', async () => { - const client = createAssetClient('signals', metricsDataClientMock); + it('should query Elasticsearch correctly', async () => { + const client = createAssetClient(metricsDataClientMock); await client.getHosts({ from: 'now-5d', @@ -108,41 +103,5 @@ describe('Public assets client', () => { { exists: { field: 'container.id' } }, ]); }); - - it('should query Elasticsearch via assets correctly', async () => { - const client = createAssetClient('assets', metricsDataClientMock); - - await client.getHosts({ - from: 'now-5d', - to: 'now-3d', - elasticsearchClient: esClientMock, - savedObjectsClient: soClientMock, - }); - - expect(metricsDataClientMock.getMetricIndices).not.toHaveBeenCalled(); - - const dsl = esClientMock.search.mock.lastCall?.[0] as SearchRequest | undefined; - const { bool } = dsl?.query || {}; - expect(bool).toBeDefined(); - - expect(bool?.filter).toEqual([ - { - range: { - '@timestamp': { - gte: 'now-5d', - lte: 'now-3d', - }, - }, - }, - ]); - - expect(bool?.must).toEqual([ - { - terms: { - 'asset.kind': ['host'], - }, - }, - ]); - }); }); }); diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.ts index 0cab6e3937ac6..ba2687857171a 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.ts @@ -6,52 +6,29 @@ */ import { Asset } from '../../common/types_api'; -import { - getHostsByAssets, - getHostsBySignals, - validateGetHostsOptions, - GetHostsOptions, -} from './accessors/hosts'; -import { - getServicesByAssets, - getServicesBySignals, - GetServicesOptions, - validateGetServicesOptions, -} from './accessors/services'; -import { - AssetClientClassOptions, - AssetClientOptionsWithInjectedValues, -} from './asset_client_types'; +import { getHosts, validateGetHostsOptions, GetHostsOptions } from './accessors/hosts'; +import { getServices, GetServicesOptions, validateGetServicesOptions } from './accessors/services'; +import { AssetClientBaseOptions, AssetClientOptionsWithInjectedValues } from './asset_client_types'; export class AssetClient { - constructor(private baseOptions: AssetClientClassOptions) {} + constructor(private baseOptions: AssetClientBaseOptions) {} injectOptions(options: T): AssetClientOptionsWithInjectedValues { return { ...options, - sourceIndices: this.baseOptions.sourceIndices, - getApmIndices: this.baseOptions.getApmIndices, - metricsClient: this.baseOptions.metricsClient, + ...this.baseOptions, }; } async getHosts(options: GetHostsOptions): Promise<{ hosts: Asset[] }> { validateGetHostsOptions(options); const withInjected = this.injectOptions(options); - if (this.baseOptions.source === 'assets') { - return await getHostsByAssets(withInjected); - } else { - return await getHostsBySignals(withInjected); - } + return await getHosts(withInjected); } async getServices(options: GetServicesOptions): Promise<{ services: Asset[] }> { validateGetServicesOptions(options); const withInjected = this.injectOptions(options); - if (this.baseOptions.source === 'assets') { - return await getServicesByAssets(withInjected); - } else { - return await getServicesBySignals(withInjected); - } + return await getServices(withInjected); } } diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts b/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts index d3ab980c2fd9f..3f78be2f271c6 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts @@ -16,17 +16,10 @@ export interface AssetClientDependencies { savedObjectsClient: SavedObjectsClientContract; } -interface AssetClientInjectedValues { +export interface AssetClientBaseOptions { sourceIndices: AssetManagerConfig['sourceIndices']; getApmIndices: (soClient: SavedObjectsClientContract) => Promise; metricsClient: MetricsDataClient; } -export type AssetClientOptionsWithInjectedValues = T & AssetClientInjectedValues; - -export interface AssetClientClassOptions { - sourceIndices: AssetManagerConfig['sourceIndices']; - source: AssetManagerConfig['lockedSource']; - getApmIndices: (soClient: SavedObjectsClientContract) => Promise; - metricsClient: MetricsDataClient; -} +export type AssetClientOptionsWithInjectedValues = T & AssetClientBaseOptions; diff --git a/x-pack/plugins/asset_manager/server/plugin.ts b/x-pack/plugins/asset_manager/server/plugin.ts index 24563b5e0fbc1..6ea93a4302f49 100644 --- a/x-pack/plugins/asset_manager/server/plugin.ts +++ b/x-pack/plugins/asset_manager/server/plugin.ts @@ -57,7 +57,6 @@ export class AssetManagerServerPlugin this.logger.info('Server is enabled'); const assetClient = new AssetClient({ - source: this.config.lockedSource, sourceIndices: this.config.sourceIndices, getApmIndices: plugins.apmDataAccess.getApmIndices, metricsClient: plugins.metricsDataAccess.client, @@ -77,14 +76,11 @@ export class AssetManagerServerPlugin return; } - // create/update assets-* index template - if (this.config.lockedSource === 'assets') { - upsertTemplate({ - esClient: core.elasticsearch.client.asInternalUser, - template: assetsIndexTemplateConfig, - logger: this.logger, - }); - } + upsertTemplate({ + esClient: core.elasticsearch.client.asInternalUser, + template: assetsIndexTemplateConfig, + logger: this.logger, + }); return {}; } From 213232e83b6501441e7b99abe534cd5cee3ee3e2 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 3 Oct 2023 16:22:00 +0000 Subject: [PATCH 06/10] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/asset_manager/tsconfig.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/asset_manager/tsconfig.json b/x-pack/plugins/asset_manager/tsconfig.json index 46b69bfefb8cb..231cdfb4648e1 100644 --- a/x-pack/plugins/asset_manager/tsconfig.json +++ b/x-pack/plugins/asset_manager/tsconfig.json @@ -25,6 +25,7 @@ "@kbn/logging", "@kbn/metrics-data-access-plugin", "@kbn/core-elasticsearch-server", - "@kbn/core-saved-objects-api-server" + "@kbn/core-saved-objects-api-server", + "@kbn/core-saved-objects-api-server-mocks" ] } From 96f671539466471de4210205a0985f3166fdf1ac Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Tue, 3 Oct 2023 12:41:59 -0400 Subject: [PATCH 07/10] Removes locked source fork from API integration tests --- .buildkite/ftr_configs.yml | 3 +- ...ssets_source.ts => config_when_enabled.ts} | 3 +- .../config_with_signals_source.ts | 98 ------------------- .../tests/{with_assets_source => }/assets.ts | 6 +- .../{with_assets_source => }/assets_diff.ts | 6 +- .../assets_related.ts | 6 +- .../tests/{with_signals_source => }/basics.ts | 6 +- .../tests/{with_signals_source => }/hosts.ts | 4 +- .../tests/{with_assets_source => }/index.ts | 6 +- .../{with_assets_source => }/sample_assets.ts | 4 +- .../{with_signals_source => }/services.ts | 6 +- .../tests/with_assets_source/basics.ts | 29 ------ .../tests/with_signals_source/index.ts | 15 --- .../apis/asset_manager/types.ts | 2 +- 14 files changed, 26 insertions(+), 168 deletions(-) rename x-pack/test/api_integration/apis/asset_manager/{config_with_assets_source.ts => config_when_enabled.ts} (96%) delete mode 100644 x-pack/test/api_integration/apis/asset_manager/config_with_signals_source.ts rename x-pack/test/api_integration/apis/asset_manager/tests/{with_assets_source => }/assets.ts (98%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_assets_source => }/assets_diff.ts (98%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_assets_source => }/assets_related.ts (98%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_signals_source => }/basics.ts (87%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_signals_source => }/hosts.ts (94%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_assets_source => }/index.ts (73%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_assets_source => }/sample_assets.ts (98%) rename x-pack/test/api_integration/apis/asset_manager/tests/{with_signals_source => }/services.ts (95%) delete mode 100644 x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/basics.ts delete mode 100644 x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/index.ts diff --git a/.buildkite/ftr_configs.yml b/.buildkite/ftr_configs.yml index 309bf005084c9..3707062ff8ecc 100644 --- a/.buildkite/ftr_configs.yml +++ b/.buildkite/ftr_configs.yml @@ -70,7 +70,6 @@ disabled: - x-pack/test/plugin_api_perf/config.js - x-pack/test/screenshot_creation/config.ts - x-pack/test/fleet_packages/config.ts - - x-pack/test/api_integration/apis/asset_manager/config_with_assets_source.ts # Scalability testing config that we run in its own pipeline - x-pack/test/scalability/config.ts @@ -174,7 +173,7 @@ enabled: - x-pack/test/api_integration/config_security_trial.ts - x-pack/test/api_integration/apis/aiops/config.ts - x-pack/test/api_integration/apis/asset_manager/config_when_disabled.ts - - x-pack/test/api_integration/apis/asset_manager/config_with_signals_source.ts + - x-pack/test/api_integration/apis/asset_manager/config_when_enabled.ts - x-pack/test/api_integration/apis/cases/config.ts - x-pack/test/api_integration/apis/cloud_security_posture/config.ts - x-pack/test/api_integration/apis/console/config.ts diff --git a/x-pack/test/api_integration/apis/asset_manager/config_with_assets_source.ts b/x-pack/test/api_integration/apis/asset_manager/config_when_enabled.ts similarity index 96% rename from x-pack/test/api_integration/apis/asset_manager/config_with_assets_source.ts rename to x-pack/test/api_integration/apis/asset_manager/config_when_enabled.ts index 8d499ea3b82a2..71f73f9acb1f3 100644 --- a/x-pack/test/api_integration/apis/asset_manager/config_with_assets_source.ts +++ b/x-pack/test/api_integration/apis/asset_manager/config_when_enabled.ts @@ -38,7 +38,7 @@ export default async function createTestConfig({ return { ...baseIntegrationTestsConfig.getAll(), - testFiles: [require.resolve('./tests/with_assets_source')], + testFiles: [require.resolve('./tests')], services: { ...services, assetsSynthtraceEsClient: (context: InheritedFtrProviderContext) => { @@ -87,7 +87,6 @@ export default async function createTestConfig({ serverArgs: [ ...baseIntegrationTestsConfig.get('kbnTestServer.serverArgs'), '--xpack.assetManager.alphaEnabled=true', - `--xpack.assetManager.lockedSource=assets`, ], }, }; diff --git a/x-pack/test/api_integration/apis/asset_manager/config_with_signals_source.ts b/x-pack/test/api_integration/apis/asset_manager/config_with_signals_source.ts deleted file mode 100644 index 1d1c94673e0db..0000000000000 --- a/x-pack/test/api_integration/apis/asset_manager/config_with_signals_source.ts +++ /dev/null @@ -1,98 +0,0 @@ -/* - * 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 { - ApmSynthtraceEsClient, - ApmSynthtraceKibanaClient, - AssetsSynthtraceEsClient, - createLogger, - InfraSynthtraceEsClient, - LogLevel, -} from '@kbn/apm-synthtrace'; -import { FtrConfigProviderContext, kbnTestConfig } from '@kbn/test'; -import url from 'url'; -import { FtrProviderContext as InheritedFtrProviderContext } from '../../ftr_provider_context'; -import { InheritedServices } from './types'; - -interface AssetManagerConfig { - services: InheritedServices & { - assetsSynthtraceEsClient: ( - context: InheritedFtrProviderContext - ) => Promise; - infraSynthtraceEsClient: ( - context: InheritedFtrProviderContext - ) => Promise; - apmSynthtraceEsClient: (context: InheritedFtrProviderContext) => Promise; - }; -} - -export default async function createTestConfig({ - readConfigFile, -}: FtrConfigProviderContext): Promise { - const baseIntegrationTestsConfig = await readConfigFile(require.resolve('../../config.ts')); - const services = baseIntegrationTestsConfig.get('services'); - - return { - ...baseIntegrationTestsConfig.getAll(), - testFiles: [require.resolve('./tests/with_signals_source')], - services: { - ...services, - assetsSynthtraceEsClient: (context: InheritedFtrProviderContext) => { - return new AssetsSynthtraceEsClient({ - client: context.getService('es'), - logger: createLogger(LogLevel.info), - refreshAfterIndex: true, - }); - }, - infraSynthtraceEsClient: (context: InheritedFtrProviderContext) => { - return new InfraSynthtraceEsClient({ - client: context.getService('es'), - logger: createLogger(LogLevel.info), - refreshAfterIndex: true, - }); - }, - apmSynthtraceEsClient: async (context: InheritedFtrProviderContext) => { - const servers = baseIntegrationTestsConfig.get('servers'); - - const kibanaServer = servers.kibana as url.UrlObject; - const kibanaServerUrl = url.format(kibanaServer); - const kibanaServerUrlWithAuth = url - .format({ - ...url.parse(kibanaServerUrl), - auth: `elastic:${kbnTestConfig.getUrlParts().password}`, - }) - .slice(0, -1); - - const kibanaClient = new ApmSynthtraceKibanaClient({ - target: kibanaServerUrlWithAuth, - logger: createLogger(LogLevel.debug), - }); - const kibanaVersion = await kibanaClient.fetchLatestApmPackageVersion(); - await kibanaClient.installApmPackage(kibanaVersion); - - return new ApmSynthtraceEsClient({ - client: context.getService('es'), - logger: createLogger(LogLevel.info), - version: kibanaVersion, - refreshAfterIndex: true, - }); - }, - }, - kbnTestServer: { - ...baseIntegrationTestsConfig.get('kbnTestServer'), - serverArgs: [ - ...baseIntegrationTestsConfig.get('kbnTestServer.serverArgs'), - '--xpack.assetManager.alphaEnabled=true', - `--xpack.assetManager.lockedSource=signals`, - ], - }, - }; -} - -export type CreateTestConfig = Awaited>; - -export type AssetManagerServices = CreateTestConfig['services']; diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets.ts b/x-pack/test/api_integration/apis/asset_manager/tests/assets.ts similarity index 98% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/assets.ts index f5069a274ebb3..7514513971688 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/assets.ts @@ -7,9 +7,9 @@ import { AssetWithoutTimestamp } from '@kbn/assetManager-plugin/common/types_api'; import expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../../ftr_provider_context'; -import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from '../helpers'; -import { ASSETS_ENDPOINT } from '../constants'; +import { FtrProviderContext } from '../../../ftr_provider_context'; +import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from './helpers'; +import { ASSETS_ENDPOINT } from './constants'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets_diff.ts b/x-pack/test/api_integration/apis/asset_manager/tests/assets_diff.ts similarity index 98% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets_diff.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/assets_diff.ts index 15cf1a64dafaa..79c2db74d3f95 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets_diff.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/assets_diff.ts @@ -9,9 +9,9 @@ import { sortBy } from 'lodash'; import { AssetWithoutTimestamp } from '@kbn/assetManager-plugin/common/types_api'; import expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../../ftr_provider_context'; -import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from '../helpers'; -import { ASSETS_ENDPOINT } from '../constants'; +import { FtrProviderContext } from '../../../ftr_provider_context'; +import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from './helpers'; +import { ASSETS_ENDPOINT } from './constants'; const DIFF_ENDPOINT = `${ASSETS_ENDPOINT}/diff`; diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets_related.ts b/x-pack/test/api_integration/apis/asset_manager/tests/assets_related.ts similarity index 98% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets_related.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/assets_related.ts index 2cb05d89ed618..58d4988fd7ab4 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/assets_related.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/assets_related.ts @@ -9,9 +9,9 @@ import { pick } from 'lodash'; import { Asset, AssetWithoutTimestamp } from '@kbn/assetManager-plugin/common/types_api'; import expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../../ftr_provider_context'; -import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from '../helpers'; -import { ASSETS_ENDPOINT } from '../constants'; +import { FtrProviderContext } from '../../../ftr_provider_context'; +import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from './helpers'; +import { ASSETS_ENDPOINT } from './constants'; const RELATED_ASSETS_ENDPOINT = `${ASSETS_ENDPOINT}/related`; diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/basics.ts b/x-pack/test/api_integration/apis/asset_manager/tests/basics.ts similarity index 87% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/basics.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/basics.ts index ea64d5898b265..d0d44b88f68c1 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/basics.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/basics.ts @@ -6,7 +6,7 @@ */ import expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../../ftr_provider_context'; +import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); @@ -21,8 +21,8 @@ export default function ({ getService }: FtrProviderContext) { }); describe('assets index templates', () => { - it('should not be installed', async () => { - await esSupertest.get('/_index_template/assets').expect(404); + it('should always be installed', async () => { + await esSupertest.get('/_index_template/assets').expect(200); }); }); }); diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/hosts.ts b/x-pack/test/api_integration/apis/asset_manager/tests/hosts.ts similarity index 94% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/hosts.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/hosts.ts index 87b9a96944b56..1dab6373ddc9b 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/hosts.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/hosts.ts @@ -7,8 +7,8 @@ import { timerange, infra } from '@kbn/apm-synthtrace-client'; import expect from '@kbn/expect'; -import { ASSETS_ENDPOINT } from '../constants'; -import { FtrProviderContext } from '../../types'; +import { ASSETS_ENDPOINT } from './constants'; +import { FtrProviderContext } from '../types'; const HOSTS_ASSETS_ENDPOINT = `${ASSETS_ENDPOINT}/hosts`; diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/index.ts b/x-pack/test/api_integration/apis/asset_manager/tests/index.ts similarity index 73% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/index.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/index.ts index d92db4cff9958..f64733fa2e062 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/index.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/index.ts @@ -4,11 +4,13 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { FtrProviderContext } from '../../../../ftr_provider_context'; +import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { - describe('Asset Manager API Endpoints - with assets source', () => { + describe('Asset Manager API Endpoints', () => { loadTestFile(require.resolve('./basics')); + loadTestFile(require.resolve('./hosts')); + loadTestFile(require.resolve('./services')); loadTestFile(require.resolve('./sample_assets')); loadTestFile(require.resolve('./assets')); loadTestFile(require.resolve('./assets_diff')); diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/sample_assets.ts b/x-pack/test/api_integration/apis/asset_manager/tests/sample_assets.ts similarity index 98% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/sample_assets.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/sample_assets.ts index 618f5f49f8892..a9e7ebab188e8 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/sample_assets.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/sample_assets.ts @@ -7,8 +7,8 @@ import { Asset } from '@kbn/assetManager-plugin/common/types_api'; import expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../../ftr_provider_context'; -import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from '../helpers'; +import { FtrProviderContext } from '../../../ftr_provider_context'; +import { createSampleAssets, deleteSampleAssets, viewSampleAssetDocs } from './helpers'; export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertest'); diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/services.ts b/x-pack/test/api_integration/apis/asset_manager/tests/services.ts similarity index 95% rename from x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/services.ts rename to x-pack/test/api_integration/apis/asset_manager/tests/services.ts index 1dea627a2a918..dc00a025021c0 100644 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/services.ts +++ b/x-pack/test/api_integration/apis/asset_manager/tests/services.ts @@ -8,8 +8,8 @@ import { omit } from 'lodash'; import { apm, timerange } from '@kbn/apm-synthtrace-client'; import expect from '@kbn/expect'; -import { ASSETS_ENDPOINT } from '../constants'; -import { FtrProviderContext } from '../../types'; +import { ASSETS_ENDPOINT } from './constants'; +import { FtrProviderContext } from '../types'; const SERVICES_ASSETS_ENDPOINT = `${ASSETS_ENDPOINT}/services`; @@ -49,7 +49,7 @@ export default function ({ getService }: FtrProviderContext) { .query({ from, to, - parent: 'my-host-1', + parent: 'host:my-host-1', }) .expect(200); diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/basics.ts b/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/basics.ts deleted file mode 100644 index 451d49605a213..0000000000000 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_assets_source/basics.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * 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 expect from '@kbn/expect'; -import { FtrProviderContext } from '../../../../ftr_provider_context'; - -export default function ({ getService }: FtrProviderContext) { - const supertest = getService('supertest'); - const esSupertest = getService('esSupertest'); - - describe('during basic startup', () => { - describe('ping endpoint', () => { - it('returns a successful response', async () => { - const response = await supertest.get('/api/asset-manager/ping').expect(200); - expect(response.body).to.eql({ message: 'Asset Manager OK' }); - }); - }); - - describe('assets index template', () => { - it('should be installed', async () => { - await esSupertest.get('/_index_template/assets').expect(200); - }); - }); - }); -} diff --git a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/index.ts b/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/index.ts deleted file mode 100644 index 7c9c5fb4ae626..0000000000000 --- a/x-pack/test/api_integration/apis/asset_manager/tests/with_signals_source/index.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * 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 { FtrProviderContext } from '../../../../ftr_provider_context'; - -export default function ({ loadTestFile }: FtrProviderContext) { - describe('Asset Manager API Endpoints - with signals source', () => { - loadTestFile(require.resolve('./basics')); - loadTestFile(require.resolve('./hosts')); - loadTestFile(require.resolve('./services')); - }); -} diff --git a/x-pack/test/api_integration/apis/asset_manager/types.ts b/x-pack/test/api_integration/apis/asset_manager/types.ts index ba52dcb88a5b6..3a2b1656928fc 100644 --- a/x-pack/test/api_integration/apis/asset_manager/types.ts +++ b/x-pack/test/api_integration/apis/asset_manager/types.ts @@ -7,7 +7,7 @@ import { GenericFtrProviderContext } from '@kbn/test'; import { FtrProviderContext as InheritedFtrProviderContext } from '../../ftr_provider_context'; -import { AssetManagerServices } from './config_with_signals_source'; +import { AssetManagerServices } from './config_when_enabled'; export type InheritedServices = InheritedFtrProviderContext extends GenericFtrProviderContext< infer TServices, From 8938f918931b5a464d2a70cf135ed4a0519b4c55 Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Tue, 3 Oct 2023 15:57:46 -0400 Subject: [PATCH 08/10] Troubleshooting failed FTR tests --- .../server/{client.mock.ts => client_mock.ts} | 0 x-pack/plugins/metrics_data_access/server/index.ts | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename x-pack/plugins/metrics_data_access/server/{client.mock.ts => client_mock.ts} (100%) diff --git a/x-pack/plugins/metrics_data_access/server/client.mock.ts b/x-pack/plugins/metrics_data_access/server/client_mock.ts similarity index 100% rename from x-pack/plugins/metrics_data_access/server/client.mock.ts rename to x-pack/plugins/metrics_data_access/server/client_mock.ts diff --git a/x-pack/plugins/metrics_data_access/server/index.ts b/x-pack/plugins/metrics_data_access/server/index.ts index 78a18f8c00ef5..e76f2201b340a 100644 --- a/x-pack/plugins/metrics_data_access/server/index.ts +++ b/x-pack/plugins/metrics_data_access/server/index.ts @@ -18,7 +18,7 @@ export type { export { metricsDataSourceSavedObjectName } from './saved_objects/metrics_data_source'; export { MetricsDataClient } from './client'; -export { MetricsDataClientMock } from './client.mock'; +export { MetricsDataClientMock } from './client_mock'; export function plugin(context: PluginInitializerContext) { return new MetricsDataPlugin(context); From 34c6d1af13a5cac6cc3932c9d185b8c380db0c8e Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Wed, 4 Oct 2023 14:57:21 -0400 Subject: [PATCH 09/10] Improvements to assets server client unit tests --- .../server/lib/accessors/hosts/get_hosts.ts | 9 +- .../server/lib/accessors/hosts/index.ts | 19 -- .../lib/accessors/hosts/shared_types.ts | 14 -- .../lib/accessors/services/get_services.ts | 9 +- .../server/lib/accessors/services/index.ts | 19 -- .../lib/accessors/services/shared_types.ts | 15 -- .../server/lib/asset_client.test.ts | 209 +++++++++++++++++- .../asset_manager/server/lib/asset_client.ts | 9 +- .../server/lib/asset_client_types.ts | 5 +- .../lib/validators/validate_date_range.ts | 59 +++++ .../server/lib/validators/validate_es_date.ts | 17 -- 11 files changed, 288 insertions(+), 96 deletions(-) delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts delete mode 100644 x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts create mode 100644 x-pack/plugins/asset_manager/server/lib/validators/validate_date_range.ts delete mode 100644 x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts index bbe663c2d6168..632b234b5512f 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/get_hosts.ts @@ -5,9 +5,16 @@ * 2.0. */ -import type { GetHostsOptionsInjected } from './shared_types'; import { Asset } from '../../../../common/types_api'; import { collectHosts } from '../../collectors/hosts'; +import { GetHostsOptionsPublic } from '../../../../common/types_client'; +import { + AssetClientDependencies, + AssetClientOptionsWithInjectedValues, +} from '../../asset_client_types'; + +export type GetHostsOptions = GetHostsOptionsPublic & AssetClientDependencies; +export type GetHostsOptionsInjected = AssetClientOptionsWithInjectedValues; export async function getHosts(options: GetHostsOptionsInjected): Promise<{ hosts: Asset[] }> { const metricsIndices = await options.metricsClient.getMetricIndices({ diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts deleted file mode 100644 index a3e8b4b0c10ee..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/index.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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 { validateESDate } from '../../validators/validate_es_date'; -import type { GetHostsOptions, GetHostsOptionsInjected } from './shared_types'; -export { getHosts } from './get_hosts'; - -export type { GetHostsOptions, GetHostsOptionsInjected }; - -export function validateGetHostsOptions(options: GetHostsOptions) { - validateESDate(options.from); - if (options.to) { - validateESDate(options.to); - } -} diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts b/x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts deleted file mode 100644 index 13aebe0bacc76..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/hosts/shared_types.ts +++ /dev/null @@ -1,14 +0,0 @@ -/* - * 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 { GetHostsOptionsPublic } from '../../../../common/types_client'; -import { - AssetClientDependencies, - AssetClientOptionsWithInjectedValues, -} from '../../asset_client_types'; - -export type GetHostsOptions = GetHostsOptionsPublic & AssetClientDependencies; -export type GetHostsOptionsInjected = AssetClientOptionsWithInjectedValues; diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts index 899a2e20215dd..2df13930e2ed7 100644 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts +++ b/x-pack/plugins/asset_manager/server/lib/accessors/services/get_services.ts @@ -6,9 +6,16 @@ */ import { Asset } from '../../../../common/types_api'; -import { GetServicesOptionsInjected } from './shared_types'; import { collectServices } from '../../collectors/services'; import { parseEan } from '../../parse_ean'; +import { GetServicesOptionsPublic } from '../../../../common/types_client'; +import { + AssetClientDependencies, + AssetClientOptionsWithInjectedValues, +} from '../../asset_client_types'; + +export type GetServicesOptions = GetServicesOptionsPublic & AssetClientDependencies; +export type GetServicesOptionsInjected = AssetClientOptionsWithInjectedValues; export async function getServices( options: GetServicesOptionsInjected diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts deleted file mode 100644 index 0c11addce0dd0..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/index.ts +++ /dev/null @@ -1,19 +0,0 @@ -/* - * 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 { validateESDate } from '../../validators/validate_es_date'; -import type { GetServicesOptions, GetServicesOptionsInjected } from './shared_types'; - -export type { GetServicesOptions, GetServicesOptionsInjected }; -export { getServices } from './get_services'; - -export function validateGetServicesOptions(options: GetServicesOptions) { - validateESDate(options.from); - if (options.to) { - validateESDate(options.to); - } -} diff --git a/x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts b/x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts deleted file mode 100644 index 7edef8d0e2646..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/accessors/services/shared_types.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * 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 { GetServicesOptionsPublic } from '../../../../common/types_client'; -import { - AssetClientDependencies, - AssetClientOptionsWithInjectedValues, -} from '../../asset_client_types'; - -export type GetServicesOptions = GetServicesOptionsPublic & AssetClientDependencies; -export type GetServicesOptionsInjected = AssetClientOptionsWithInjectedValues; diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts index 77be9cf746220..a9bd5791bc51a 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.test.ts @@ -14,19 +14,60 @@ import { MetricsDataClient, MetricsDataClientMock } from '@kbn/metrics-data-acce import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks'; import { SearchRequest } from '@elastic/elasticsearch/lib/api/types'; import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; +import { AssetsValidationError } from './validators/validation_error'; +import { GetApmIndicesMethod } from './asset_client_types'; -function createAssetClient(metricsDataClient: MetricsDataClient) { +// Helper function allows test to verify error was thrown, +// verify error is of the right class type, and error has +// the expected metadata such as statusCode on it +function expectToThrowValidationErrorWithStatusCode( + testFn: () => Promise, + expectedError: Partial = {} +) { + return expect(async () => { + try { + return await testFn(); + } catch (error: any) { + if (error instanceof AssetsValidationError) { + if (expectedError.statusCode) { + expect(error.statusCode).toEqual(expectedError.statusCode); + } + if (expectedError.message) { + expect(error.message).toEqual(expect.stringContaining(expectedError.message)); + } + } + throw error; + } + }).rejects.toThrow(AssetsValidationError); +} + +function createGetApmIndicesMock(): jest.Mocked { + return jest.fn(async (client: SavedObjectsClientContract) => ({ + transaction: 'apm-mock-transaction-indices', + span: 'apm-mock-span-indices', + error: 'apm-mock-error-indices', + metric: 'apm-mock-metric-indices', + onboarding: 'apm-mock-onboarding-indices', + sourcemap: 'apm-mock-sourcemap-indices', + })); +} + +function createAssetClient( + metricsDataClient: MetricsDataClient, + getApmIndicesMock: jest.Mocked +) { return new AssetClient({ sourceIndices: { logs: 'my-logs*', }, - getApmIndices: jest.fn(), + getApmIndices: getApmIndicesMock, metricsClient: metricsDataClient, }); } -describe('Public assets client', () => { +describe('Server assets client', () => { let metricsDataClientMock: MetricsDataClient = MetricsDataClientMock.create(); + let getApmIndicesMock: jest.Mocked = createGetApmIndicesMock(); let esClientMock: ElasticsearchClientMock = elasticsearchClientMock.createScopedClusterClient().asCurrentUser; let soClientMock: jest.Mocked; @@ -36,6 +77,7 @@ describe('Public assets client', () => { esClientMock = elasticsearchClientMock.createScopedClusterClient().asCurrentUser; soClientMock = savedObjectsClientMock.create(); metricsDataClientMock = MetricsDataClientMock.create(); + getApmIndicesMock = createGetApmIndicesMock(); // ES returns no results, just enough structure to not blow up esClientMock.search.mockResolvedValueOnce({ @@ -54,13 +96,13 @@ describe('Public assets client', () => { describe('class instantiation', () => { it('should successfully instantiate', () => { - createAssetClient(metricsDataClientMock); + createAssetClient(metricsDataClientMock, getApmIndicesMock); }); }); describe('getHosts', () => { it('should query Elasticsearch correctly', async () => { - const client = createAssetClient(metricsDataClientMock); + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); await client.getHosts({ from: 'now-5d', @@ -103,5 +145,162 @@ describe('Public assets client', () => { { exists: { field: 'container.id' } }, ]); }); + + it('should reject with 400 for invalid "from" date', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getHosts({ + from: 'now-1zz', + to: 'now-3d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); + + it('should reject with 400 for invalid "to" date', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getHosts({ + from: 'now-5d', + to: 'now-3fe', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); + + it('should reject with 400 when "from" is a date that is after "to"', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getHosts({ + from: 'now', + to: 'now-5d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); + + it('should reject with 400 when "from" is in the future', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getHosts({ + from: 'now+1d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); + }); + + describe('getServices', () => { + it('should query Elasticsearch correctly', async () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + await client.getServices({ + from: 'now-5d', + to: 'now-3d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }); + + expect(getApmIndicesMock).toHaveBeenCalledTimes(1); + expect(getApmIndicesMock).toHaveBeenCalledWith(soClientMock); + + const dsl = esClientMock.search.mock.lastCall?.[0] as SearchRequest | undefined; + const { bool } = dsl?.query || {}; + expect(bool).toBeDefined(); + + expect(bool?.filter).toEqual([ + { + range: { + '@timestamp': { + gte: 'now-5d', + lte: 'now-3d', + }, + }, + }, + ]); + + expect(bool?.must).toEqual([ + { + exists: { + field: 'service.name', + }, + }, + ]); + + expect(bool?.should).toBeUndefined(); + }); + + it('should reject with 400 for invalid "from" date', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expect(() => + client.getServices({ + from: 'now-1zz', + to: 'now-3d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }) + ).rejects.toThrow(AssetsValidationError); + }); + + it('should reject with 400 for invalid "to" date', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getServices({ + from: 'now-5d', + to: 'now-3fe', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); + + it('should reject with 400 when "from" is a date that is after "to"', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getServices({ + from: 'now', + to: 'now-5d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); + + it('should reject with 400 when "from" is in the future', () => { + const client = createAssetClient(metricsDataClientMock, getApmIndicesMock); + + return expectToThrowValidationErrorWithStatusCode( + () => + client.getServices({ + from: 'now+1d', + elasticsearchClient: esClientMock, + savedObjectsClient: soClientMock, + }), + { statusCode: 400 } + ); + }); }); }); diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client.ts b/x-pack/plugins/asset_manager/server/lib/asset_client.ts index ba2687857171a..0349d50b7fd21 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client.ts @@ -6,9 +6,10 @@ */ import { Asset } from '../../common/types_api'; -import { getHosts, validateGetHostsOptions, GetHostsOptions } from './accessors/hosts'; -import { getServices, GetServicesOptions, validateGetServicesOptions } from './accessors/services'; +import { getHosts, GetHostsOptions } from './accessors/hosts/get_hosts'; +import { getServices, GetServicesOptions } from './accessors/services/get_services'; import { AssetClientBaseOptions, AssetClientOptionsWithInjectedValues } from './asset_client_types'; +import { validateStringDateRange } from './validators/validate_date_range'; export class AssetClient { constructor(private baseOptions: AssetClientBaseOptions) {} @@ -21,13 +22,13 @@ export class AssetClient { } async getHosts(options: GetHostsOptions): Promise<{ hosts: Asset[] }> { - validateGetHostsOptions(options); + validateStringDateRange(options.from, options.to); const withInjected = this.injectOptions(options); return await getHosts(withInjected); } async getServices(options: GetServicesOptions): Promise<{ services: Asset[] }> { - validateGetServicesOptions(options); + validateStringDateRange(options.from, options.to); const withInjected = this.injectOptions(options); return await getServices(withInjected); } diff --git a/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts b/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts index 3f78be2f271c6..a15886ce3a00a 100644 --- a/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts +++ b/x-pack/plugins/asset_manager/server/lib/asset_client_types.ts @@ -11,6 +11,9 @@ import { SavedObjectsClientContract } from '@kbn/core-saved-objects-api-server'; import { MetricsDataClient } from '@kbn/metrics-data-access-plugin/server'; import { AssetManagerConfig } from '../../common/config'; +export type GetApmIndicesMethod = ( + soClient: SavedObjectsClientContract +) => Promise; export interface AssetClientDependencies { elasticsearchClient: ElasticsearchClient; savedObjectsClient: SavedObjectsClientContract; @@ -18,7 +21,7 @@ export interface AssetClientDependencies { export interface AssetClientBaseOptions { sourceIndices: AssetManagerConfig['sourceIndices']; - getApmIndices: (soClient: SavedObjectsClientContract) => Promise; + getApmIndices: GetApmIndicesMethod; metricsClient: MetricsDataClient; } diff --git a/x-pack/plugins/asset_manager/server/lib/validators/validate_date_range.ts b/x-pack/plugins/asset_manager/server/lib/validators/validate_date_range.ts new file mode 100644 index 0000000000000..144e9c4eabccb --- /dev/null +++ b/x-pack/plugins/asset_manager/server/lib/validators/validate_date_range.ts @@ -0,0 +1,59 @@ +/* + * 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 datemath from '@kbn/datemath'; +import { Moment } from 'moment'; +import { AssetsValidationError } from './validation_error'; + +export function validateStringDateRange(from: string, to?: string) { + const parsedFrom = validateESDate(from); + validateDateInPast(parsedFrom); + if (to) { + const parsedTo = validateESDate(to); + validateDateRangeOrder(parsedFrom, parsedTo); + } +} + +export function validateDateInPast(date: Moment) { + const now = datemath.parse('now')!; + if (date.isAfter(now)) { + throw new AssetsValidationError(`Date cannot be in the future ${date.toISOString()}`, { + statusCode: 400, + }); + } +} + +export function validateDateRangeOrder(from: Moment, to: Moment) { + if (from.isAfter(to)) { + throw new AssetsValidationError( + `Invalid date range - given "from" value (${from.toISOString()}) is after given "to" value (${to.toISOString()})`, + { + statusCode: 400, + } + ); + } +} + +export function validateESDate(dateString: string) { + try { + const parsed = datemath.parse(dateString); + if (typeof parsed === 'undefined') { + throw new Error('Date string was parsed as undefined'); + } + if (parsed.toString() === 'Invalid date') { + throw new Error('Date string produced an invalid date'); + } + return parsed; + } catch (error: any) { + throw new AssetsValidationError( + `"${dateString}" is not a valid Elasticsearch date value - ${error}`, + { + statusCode: 400, + } + ); + } +} diff --git a/x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts b/x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts deleted file mode 100644 index 2ecd80a8d0e67..0000000000000 --- a/x-pack/plugins/asset_manager/server/lib/validators/validate_es_date.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * 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 datemath from '@kbn/datemath'; -import { AssetsValidationError } from './validation_error'; - -export function validateESDate(dateString: string) { - try { - datemath.parse(dateString); - } catch (error: any) { - throw new AssetsValidationError(`"${dateString}" is not a valid Elasticsearch date value`); - } -} From 98c0e7f0885a3d5e7be1a538f7a27978a49d9135 Mon Sep 17 00:00:00 2001 From: Jason Rhodes Date: Wed, 4 Oct 2023 15:08:45 -0400 Subject: [PATCH 10/10] Defaults validation error to 400 status, per review suggestion --- .../asset_manager/server/lib/validators/validation_error.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts b/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts index 05b56afc52991..0f094971fc528 100644 --- a/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts +++ b/x-pack/plugins/asset_manager/server/lib/validators/validation_error.ts @@ -12,7 +12,7 @@ interface ErrorOptions { export class AssetsValidationError extends Error { public statusCode: number; - constructor(message: string, { statusCode = 500 }: ErrorOptions = {}) { + constructor(message: string, { statusCode = 400 }: ErrorOptions = {}) { super(message); this.name = 'AssetsValidationError'; this.statusCode = statusCode;