From 8ecd2d67a02f9de2a0e4f8778dcca3781dceaa90 Mon Sep 17 00:00:00 2001 From: Alexi Doak <109488926+doakalexi@users.noreply.github.com> Date: Mon, 15 May 2023 13:08:30 -0400 Subject: [PATCH] [ResponseOps] es query rule params not compatible between 8.6 and 8.7 (#157710) Resolves https://github.com/elastic/kibana/issues/156856 ## Summary Adds a default value to `aggType` and `groupBy` fields ### 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 --- .../es_query/rule_type_params.test.ts | 22 +++++++++++++ .../rule_types/es_query/rule_type_params.ts | 4 +-- .../server/data/lib/core_query_types.test.ts | 22 +++++++++++++ .../server/data/lib/core_query_types.ts | 4 +-- .../builtin_alert_types/es_query/rule.ts | 32 +++++++++++++++++-- 5 files changed, 78 insertions(+), 6 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 ff642003c1dc9..7e99ded206b2c 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 @@ -144,6 +144,28 @@ describe('ruleType Params validate()', () => { ); }); + it('uses default value "count" if aggType is undefined', async () => { + params.aggType = undefined; + expect(onValidate()).not.toThrow(); + }); + + it('fails for invalid groupBy', async () => { + params.groupBy = 42; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot( + `"[groupBy]: expected value of type [string] but got [number]"` + ); + + params.groupBy = '-not-a-valid-groupBy-'; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot( + `"[groupBy]: invalid groupBy: \\"-not-a-valid-groupBy-\\""` + ); + }); + + it('uses default value "all" if groupBy is undefined', async () => { + params.groupBy = undefined; + expect(onValidate()).not.toThrow(); + }); + it('fails for invalid aggField', async () => { params.aggField = 42; expect(onValidate()).toThrowErrorMatchingInlineSnapshot( 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 6348437ba7198..b9380eeafe304 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 @@ -41,11 +41,11 @@ const EsQueryRuleParamsSchemaProperties = { threshold: schema.arrayOf(schema.number(), { minSize: 1, maxSize: 2 }), thresholdComparator: getComparatorSchemaType(validateComparator), // aggregation type - aggType: schema.string({ validate: validateAggType }), + aggType: schema.string({ validate: validateAggType, defaultValue: 'count' }), // aggregation field aggField: schema.maybe(schema.string({ minLength: 1 })), // how to group - groupBy: schema.string({ validate: validateGroupBy }), + groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }), // field to group on (for groupBy: top) termField: schema.maybe(schema.string({ minLength: 1 })), // limit on number of groups returned diff --git a/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.test.ts b/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.test.ts index 6ffed5accf9cd..0289223f525c8 100644 --- a/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.test.ts +++ b/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.test.ts @@ -109,6 +109,28 @@ export function runTests(schema: ObjectType, defaultTypeParams: Record { + params.aggType = undefined; + expect(onValidate()).not.toThrow(); + }); + + it('fails for invalid groupBy', async () => { + params.groupBy = 42; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot( + `"[groupBy]: expected value of type [string] but got [number]"` + ); + + params.groupBy = '-not-a-valid-groupBy-'; + expect(onValidate()).toThrowErrorMatchingInlineSnapshot( + `"[groupBy]: invalid groupBy: \\"-not-a-valid-groupBy-\\""` + ); + }); + + it('uses default value "all" if groupBy is undefined', async () => { + params.groupBy = undefined; + expect(onValidate()).not.toThrow(); + }); + it('fails for invalid aggField', async () => { params.aggField = 42; expect(onValidate()).toThrowErrorMatchingInlineSnapshot( diff --git a/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.ts b/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.ts index 6fe6e39172379..c9ba7dc013d86 100644 --- a/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.ts +++ b/x-pack/plugins/triggers_actions_ui/server/data/lib/core_query_types.ts @@ -22,11 +22,11 @@ export const CoreQueryParamsSchemaProperties = { // field in index used for date/time timeField: schema.string({ minLength: 1 }), // aggregation type - aggType: schema.string({ validate: validateAggType }), + aggType: schema.string({ validate: validateAggType, defaultValue: 'count' }), // aggregation field aggField: schema.maybe(schema.string({ minLength: 1 })), // how to group - groupBy: schema.string({ validate: validateGroupBy }), + groupBy: schema.string({ validate: validateGroupBy, defaultValue: 'all' }), // field to group on (for groupBy: top) termField: schema.maybe(schema.string({ minLength: 1 })), // filter field diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/rule.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/rule.ts index 6a2314bec3a0b..f422be7be19d7 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/rule.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group3/builtin_alert_types/es_query/rule.ts @@ -979,6 +979,34 @@ export default function ruleTests({ getService }: FtrProviderContext) { }); }); + describe('aggType and groupBy', () => { + it('sets aggType: "count" and groupBy: "all" when they are undefined', async () => { + endDate = new Date().toISOString(); + + await createEsDocumentsInGroups(ES_GROUPS_TO_WRITE, endDate); + + await createRule({ + name: 'always fire', + esQuery: `{\n \"query\":{\n \"match_all\" : {}\n }\n}`, + size: 100, + thresholdComparator: '>', + threshold: [0], + aggType: undefined, + groupBy: undefined, + }); + + const docs = await waitForDocs(2); + + expect(docs[0]._source.hits.length).greaterThan(0); + const messagePattern = + /rule 'always fire' is active:\n\n- Value: \d+\n- Conditions Met: Number of matching documents is greater than 0 over 20s\n- Timestamp: \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d{3}Z/; + expect(docs[0]._source.params.message).to.match(messagePattern); + + expect(docs[1]._source.hits.length).to.be(0); + expect(docs[1]._source.params.message).to.match(/rule 'always fire' is recovered/); + }); + }); + async function waitForDocs(count: number): Promise { return await esTestIndexToolOutput.waitForDocs( ES_TEST_INDEX_SOURCE, @@ -1080,8 +1108,8 @@ export default function ruleTests({ getService }: FtrProviderContext) { thresholdComparator: params.thresholdComparator, threshold: params.threshold, searchType: params.searchType, - aggType: params.aggType ?? 'count', - groupBy: params.groupBy ?? 'all', + aggType: params.aggType, + groupBy: params.groupBy, aggField: params.aggField, termField: params.termField, termSize: params.termSize,