From 44868e3411e01f1a9d6b0aeb1408d16c5251b6a2 Mon Sep 17 00:00:00 2001 From: Alexi Doak <109488926+doakalexi@users.noreply.github.com> Date: Thu, 2 Nov 2023 13:00:13 -0700 Subject: [PATCH] [ResponseOps] Elasticsearch query rule with ES|QL threshold validation (#170463) Resolves https://github.com/elastic/kibana/issues/170360 ## Summary We should be throwing an error if a user tries to create an ESQL es query rule where `thresholdCompartor != '>'` or `threshold != 0` or `timeField` is not defined. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### To verify - Go to [dev tools](http://localhost:5601/app/dev_tools#/console) - Run the following and edit thresholdComparator, threshold, or timeField and verify that you see errors thrown. ``` POST kbn:/api/alerting/rule { "params": { "searchType": "esqlQuery", "esqlQuery": { "esql": """from kibana_sample_data_logs | keep bytes, clientip, host, geo.dest | where geo.dest != "GB" | stats sumbytes = sum(bytes) by clientip, host | WHERE sumbytes > 5000 | sort sumbytes desc | limit 10""" }, "timeWindowSize": 1, "timeWindowUnit": "d", "thresholdComparator": "<", "threshold": [ 1000 ], "size": 10, "timeField": "date" }, "consumer": "stackAlerts", "rule_type_id": ".es-query", "schedule": { "interval": "5d" }, "name": "test rule" } ``` (cherry picked from commit 0e7798a4024b6f9f60700e60101868960172796d) --- .../es_query/rule_type_params.test.ts | 25 ++++ .../rule_types/es_query/rule_type_params.ts | 23 +++- .../builtin_alert_types/es_query/esql_only.ts | 130 ++++++++++++++++-- 3 files changed, 167 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.test.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.test.ts index b6011e72bbcc7..128fd339f001b 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.test.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.test.ts @@ -354,6 +354,31 @@ describe('ruleType Params validate()', () => { expect(onValidate()).not.toThrow(); }); + describe('esqlQuery search type', () => { + beforeEach(() => { + params = { ...DefaultParams, searchType: 'esqlQuery', esqlQuery: { esql: 'from test' } }; + delete params.esQuery; + delete params.index; + }); + + it('fails for invalid thresholdComparator', async () => { + params.thresholdComparator = Comparator.LT; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot( + `"[thresholdComparator]: is required to be greater than"` + ); + }); + + it('fails for invalid threshold', async () => { + params.threshold = [7]; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot(`"[threshold]: is required to be 0"`); + }); + + it('fails for undefined timeField', async () => { + params.timeField = undefined; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot(`"[timeField]: is required"`); + }); + }); + function onValidate(): () => void { return () => validate(); } diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts index 5469c6fa60247..d1f99b4e97b93 100644 --- a/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts +++ b/x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type_params.ts @@ -159,7 +159,28 @@ function validateParams(anyParams: unknown): string | undefined { } } - if (isSearchSourceRule(searchType) || isEsqlQueryRule(searchType)) { + if (isSearchSourceRule(searchType)) { + return; + } + + if (isEsqlQueryRule(searchType)) { + const { timeField } = anyParams as EsQueryRuleParams; + + if (!timeField) { + return i18n.translate('xpack.stackAlerts.esQuery.esqlTimeFieldErrorMessage', { + defaultMessage: '[timeField]: is required', + }); + } + if (thresholdComparator !== Comparator.GT) { + return i18n.translate('xpack.stackAlerts.esQuery.esqlThresholdComparatorErrorMessage', { + defaultMessage: '[thresholdComparator]: is required to be greater than', + }); + } + if (threshold && threshold[0] !== 0) { + return i18n.translate('xpack.stackAlerts.esQuery.esqlThresholdErrorMessage', { + defaultMessage: '[threshold]: is required to be 0', + }); + } return; } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/esql_only.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/esql_only.ts index fc548df7a3426..1fb542151dc56 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/esql_only.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/esql_only.ts @@ -73,12 +73,10 @@ export default function ruleTests({ getService }: FtrProviderContext) { await createRule({ name: 'never fire', esqlQuery: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 0', - size: 100, }); await createRule({ name: 'always fire', esqlQuery: 'from .kibana-alerting-test-data | stats c = count(date) | where c > -1', - size: 100, }); const docs = await waitForDocs(2); @@ -115,13 +113,13 @@ export default function ruleTests({ getService }: FtrProviderContext) { await createRule({ name: 'never fire', esqlQuery: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 0', - size: 100, + timeField: 'date_epoch_millis', }); await createRule({ name: 'always fire', esqlQuery: 'from .kibana-alerting-test-data | stats c = count(date) | where c > -1', - size: 100, + timeField: 'date_epoch_millis', }); @@ -144,7 +142,6 @@ export default function ruleTests({ getService }: FtrProviderContext) { await createRule({ name: 'always fire', esqlQuery: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 1', - size: 100, }); const docs = await waitForDocs(1); @@ -168,7 +165,7 @@ export default function ruleTests({ getService }: FtrProviderContext) { await createRule({ name: 'fire then recovers', esqlQuery: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 1', - size: 100, + notifyWhen: 'onActionGroupChange', timeWindowSize: RULE_INTERVAL_SECONDS, }); @@ -215,12 +212,10 @@ export default function ruleTests({ getService }: FtrProviderContext) { await createRule({ name: 'never fire', esqlQuery: 'from test-data-stream | stats c = count(@timestamp) | where c < 0', - size: 100, }); await createRule({ name: 'always fire', esqlQuery: 'from test-data-stream | stats c = count(@timestamp) | where c > -1', - size: 100, }); const docs = await waitForDocs(2); @@ -238,6 +233,122 @@ export default function ruleTests({ getService }: FtrProviderContext) { } }); + it('throws an error if the thresholdComparator is not >', async () => { + const { body } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'test', + consumer: 'alerts', + enabled: true, + rule_type_id: RULE_TYPE_ID, + schedule: { interval: `${RULE_INTERVAL_SECONDS}s` }, + actions: [], + notify_when: 'onActiveAlert', + params: { + size: 100, + timeWindowSize: RULE_INTERVAL_SECONDS * 5, + timeWindowUnit: 's', + thresholdComparator: '<', + threshold: [0], + searchType: 'esqlQuery', + timeField: 'date', + esqlQuery: { + esql: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 0', + }, + }, + }) + .expect(400); + expect(body.message).to.be( + 'params invalid: [thresholdComparator]: is required to be greater than' + ); + }); + + it('throws an error if the threshold is not [0]', async () => { + const { body } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'test', + consumer: 'alerts', + enabled: true, + rule_type_id: RULE_TYPE_ID, + schedule: { interval: `${RULE_INTERVAL_SECONDS}s` }, + actions: [], + notify_when: 'onActiveAlert', + params: { + size: 100, + timeWindowSize: RULE_INTERVAL_SECONDS * 5, + timeWindowUnit: 's', + thresholdComparator: '>', + threshold: [100], + searchType: 'esqlQuery', + timeField: 'date', + esqlQuery: { + esql: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 0', + }, + }, + }) + .expect(400); + expect(body.message).to.be('params invalid: [threshold]: is required to be 0'); + }); + + it('throws an error if the timeField is undefined', async () => { + const { body } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'test', + consumer: 'alerts', + enabled: true, + rule_type_id: RULE_TYPE_ID, + schedule: { interval: `${RULE_INTERVAL_SECONDS}s` }, + actions: [], + notify_when: 'onActiveAlert', + params: { + size: 100, + timeWindowSize: RULE_INTERVAL_SECONDS * 5, + timeWindowUnit: 's', + thresholdComparator: '>', + threshold: [0], + searchType: 'esqlQuery', + esqlQuery: { + esql: 'from .kibana-alerting-test-data | stats c = count(date) | where c < 0', + }, + }, + }) + .expect(400); + expect(body.message).to.be('params invalid: [timeField]: is required'); + }); + + it('throws an error if the esqlQuery is undefined', async () => { + const { body } = await supertest + .post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'test', + consumer: 'alerts', + enabled: true, + rule_type_id: RULE_TYPE_ID, + schedule: { interval: `${RULE_INTERVAL_SECONDS}s` }, + actions: [], + notify_when: 'onActiveAlert', + params: { + size: 100, + timeWindowSize: RULE_INTERVAL_SECONDS * 5, + timeWindowUnit: 's', + thresholdComparator: '>', + threshold: [0], + searchType: 'esqlQuery', + timeField: 'date', + }, + }) + .expect(400); + expect(body.message).to.be( + 'params invalid: [esqlQuery.esql]: expected value of type [string] but got [undefined]' + ); + }); + async function waitForDocs(count: number): Promise { return await esTestIndexToolOutput.waitForDocs( ES_TEST_INDEX_SOURCE, @@ -248,7 +359,6 @@ export default function ruleTests({ getService }: FtrProviderContext) { interface CreateRuleParams { name: string; - size: number; esqlQuery: string; timeWindowSize?: number; timeField?: string; @@ -316,7 +426,7 @@ export default function ruleTests({ getService }: FtrProviderContext) { actions: [action, recoveryAction], notify_when: params.notifyWhen || 'onActiveAlert', params: { - size: params.size, + size: 100, timeWindowSize: params.timeWindowSize || RULE_INTERVAL_SECONDS * 5, timeWindowUnit: 's', thresholdComparator: '>',