From add56be7d628cad8c0d55682e54d0598e7718587 Mon Sep 17 00:00:00 2001 From: Zacqary Adam Xeper Date: Tue, 5 May 2020 10:33:41 -0500 Subject: [PATCH] [Metrics UI] Require filterQuery to be ES JSON (#64937) --- .../components/expression.tsx | 2 +- .../infra/server/lib/alerting/common/utils.ts | 26 +++++++++++++++++++ ...r_inventory_metric_threshold_alert_type.ts | 16 +++++------- .../metric_threshold_executor.ts | 15 ++--------- .../register_metric_threshold_alert_type.ts | 13 +++++----- .../apis/infra/metrics_alerting.ts | 15 ----------- 6 files changed, 41 insertions(+), 46 deletions(-) create mode 100644 x-pack/plugins/infra/server/lib/alerting/common/utils.ts diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx index 5e14babddcb0..c1213b8ddfa1 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression.tsx @@ -251,7 +251,7 @@ export const Expressions: React.FC = props => { context={alertsContext} derivedIndexPattern={derivedIndexPattern} source={source} - filterQuery={alertParams.filterQuery} + filterQuery={alertParams.filterQueryText} groupBy={alertParams.groupBy} /> diff --git a/x-pack/plugins/infra/server/lib/alerting/common/utils.ts b/x-pack/plugins/infra/server/lib/alerting/common/utils.ts new file mode 100644 index 000000000000..a2c5b27c38fd --- /dev/null +++ b/x-pack/plugins/infra/server/lib/alerting/common/utils.ts @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { isEmpty } from 'lodash'; +import { schema } from '@kbn/config-schema'; + +export const oneOfLiterals = (arrayOfLiterals: Readonly) => + schema.string({ + validate: value => + arrayOfLiterals.includes(value) ? undefined : `must be one of ${arrayOfLiterals.join(' | ')}`, + }); + +export const validateIsStringElasticsearchJSONFilter = (value: string) => { + const errorMessage = 'filterQuery must be a valid Elasticsearch filter expressed in JSON'; + try { + const parsedValue = JSON.parse(value); + if (!isEmpty(parsedValue.bool)) { + return undefined; + } + return errorMessage; + } catch (e) { + return errorMessage; + } +}; diff --git a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_alert_type.ts index 3b6a1b5557bc..71cde0175bef 100644 --- a/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/inventory_metric_threshold/register_inventory_metric_threshold_alert_type.ts @@ -11,19 +11,13 @@ import { createInventoryMetricThresholdExecutor, FIRED_ACTIONS, } from './inventory_metric_threshold_executor'; -import { METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID } from './types'; +import { METRIC_INVENTORY_THRESHOLD_ALERT_TYPE_ID, Comparator } from './types'; import { InfraBackendLibs } from '../../infra_types'; +import { oneOfLiterals, validateIsStringElasticsearchJSONFilter } from '../common/utils'; const condition = schema.object({ threshold: schema.arrayOf(schema.number()), - comparator: schema.oneOf([ - schema.literal('>'), - schema.literal('<'), - schema.literal('>='), - schema.literal('<='), - schema.literal('between'), - schema.literal('outside'), - ]), + comparator: oneOfLiterals(Object.values(Comparator)), timeUnit: schema.string(), timeSize: schema.number(), metric: schema.string(), @@ -37,7 +31,9 @@ export const registerMetricInventoryThresholdAlertType = (libs: InfraBackendLibs { criteria: schema.arrayOf(condition), nodeType: schema.string(), - filterQuery: schema.maybe(schema.string()), + filterQuery: schema.maybe( + schema.string({ validate: validateIsStringElasticsearchJSONFilter }) + ), sourceId: schema.string(), }, { unknowns: 'allow' } diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts index 5c34a058577a..ec9389537835 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts @@ -58,18 +58,7 @@ const getParsedFilterQuery: ( filterQuery: string | undefined ) => Record | Array> = filterQuery => { if (!filterQuery) return {}; - try { - return JSON.parse(filterQuery).bool; - } catch (e) { - return [ - { - query_string: { - query: filterQuery, - analyze_wildcard: true, - }, - }, - ]; - } + return JSON.parse(filterQuery).bool; }; export const getElasticsearchMetricQuery = ( @@ -265,8 +254,8 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs, alertId: s const currentValues = await getMetric( services, criterion, - config.fields.timestamp, config.metricAlias, + config.fields.timestamp, groupBy, filterQuery ); diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts index 23611559a184..e40cee1b9dda 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/register_metric_threshold_alert_type.ts @@ -11,12 +11,7 @@ import { METRIC_EXPLORER_AGGREGATIONS } from '../../../../common/http_api/metric import { createMetricThresholdExecutor, FIRED_ACTIONS } from './metric_threshold_executor'; import { METRIC_THRESHOLD_ALERT_TYPE_ID, Comparator } from './types'; import { InfraBackendLibs } from '../../infra_types'; - -const oneOfLiterals = (arrayOfLiterals: Readonly) => - schema.string({ - validate: value => - arrayOfLiterals.includes(value) ? undefined : `must be one of ${arrayOfLiterals.join(' | ')}`, - }); +import { oneOfLiterals, validateIsStringElasticsearchJSONFilter } from '../common/utils'; export function registerMetricThresholdAlertType(libs: InfraBackendLibs) { const baseCriterion = { @@ -68,7 +63,11 @@ export function registerMetricThresholdAlertType(libs: InfraBackendLibs) { { criteria: schema.arrayOf(schema.oneOf([countCriterion, nonCountCriterion])), groupBy: schema.maybe(schema.string()), - filterQuery: schema.maybe(schema.string()), + filterQuery: schema.maybe( + schema.string({ + validate: validateIsStringElasticsearchJSONFilter, + }) + ), sourceId: schema.string(), alertOnNoData: schema.maybe(schema.boolean()), }, diff --git a/x-pack/test/api_integration/apis/infra/metrics_alerting.ts b/x-pack/test/api_integration/apis/infra/metrics_alerting.ts index 5c43e8938a8c..b4ae29a2f3f8 100644 --- a/x-pack/test/api_integration/apis/infra/metrics_alerting.ts +++ b/x-pack/test/api_integration/apis/infra/metrics_alerting.ts @@ -57,21 +57,6 @@ export default function({ getService }: FtrProviderContext) { expect(result.hits).to.be.ok(); expect(result.aggregations).to.be.ok(); }); - it('should work with a filterQuery in KQL format', async () => { - const searchBody = getElasticsearchMetricQuery( - getSearchParams('avg'), - '@timestamp', - undefined, - '"agent.hostname":"foo"' - ); - const result = await client.search({ - index, - body: searchBody, - }); - expect(result.error).to.not.be.ok(); - expect(result.hits).to.be.ok(); - expect(result.aggregations).to.be.ok(); - }); }); describe('querying with a groupBy parameter', () => { for (const aggType of aggs) {