Skip to content

Commit

Permalink
[8.11] [ResponseOps] Elasticsearch query rule with ES|QL threshold va…
Browse files Browse the repository at this point in the history
…lidation (elastic#170463) (elastic#170481)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[ResponseOps] Elasticsearch query rule with ES|QL threshold
validation (elastic#170463)](elastic#170463)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Alexi
Doak","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-11-02T20:00:13Z","message":"[ResponseOps]
Elasticsearch query rule with ES|QL threshold validation
(elastic#170463)\n\nResolves
https://github.com/elastic/kibana/issues/170360\r\n\r\n##
Summary\r\n\r\nWe should be throwing an error if a user tries to create
an ESQL es\r\nquery rule where `thresholdCompartor != '>'` or `threshold
!= 0` or\r\n`timeField` is not defined.\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### To
verify\r\n\r\n- Go to [dev
tools](http://localhost:5601/app/dev_tools#/console)\r\n- Run the
following and edit thresholdComparator, threshold, or\r\ntimeField and
verify that you see errors thrown.\r\n```\r\nPOST
kbn:/api/alerting/rule\r\n{\r\n \"params\": {\r\n \"searchType\":
\"esqlQuery\",\r\n \"esqlQuery\": {\r\n \"esql\": \"\"\"from
kibana_sample_data_logs\r\n| keep bytes, clientip, host, geo.dest\r\n|
where geo.dest != \"GB\"\r\n| stats sumbytes = sum(bytes) by clientip,
host\r\n| WHERE sumbytes > 5000\r\n| sort sumbytes desc\r\n| limit
10\"\"\"\r\n },\r\n \"timeWindowSize\": 1,\r\n \"timeWindowUnit\":
\"d\",\r\n \"thresholdComparator\": \"<\",\r\n \"threshold\": [\r\n
1000\r\n ],\r\n \"size\": 10,\r\n \"timeField\": \"date\"\r\n },\r\n
\"consumer\": \"stackAlerts\",\r\n \"rule_type_id\": \".es-query\",\r\n
\"schedule\": {\r\n \"interval\": \"5d\"\r\n },\r\n \"name\": \"test
rule\"\r\n}\r\n```","sha":"0e7798a4024b6f9f60700e60101868960172796d","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:ResponseOps","v8.11.0","v8.12.0"],"number":170463,"url":"https://github.com/elastic/kibana/pull/170463","mergeCommit":{"message":"[ResponseOps]
Elasticsearch query rule with ES|QL threshold validation
(elastic#170463)\n\nResolves
https://github.com/elastic/kibana/issues/170360\r\n\r\n##
Summary\r\n\r\nWe should be throwing an error if a user tries to create
an ESQL es\r\nquery rule where `thresholdCompartor != '>'` or `threshold
!= 0` or\r\n`timeField` is not defined.\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### To
verify\r\n\r\n- Go to [dev
tools](http://localhost:5601/app/dev_tools#/console)\r\n- Run the
following and edit thresholdComparator, threshold, or\r\ntimeField and
verify that you see errors thrown.\r\n```\r\nPOST
kbn:/api/alerting/rule\r\n{\r\n \"params\": {\r\n \"searchType\":
\"esqlQuery\",\r\n \"esqlQuery\": {\r\n \"esql\": \"\"\"from
kibana_sample_data_logs\r\n| keep bytes, clientip, host, geo.dest\r\n|
where geo.dest != \"GB\"\r\n| stats sumbytes = sum(bytes) by clientip,
host\r\n| WHERE sumbytes > 5000\r\n| sort sumbytes desc\r\n| limit
10\"\"\"\r\n },\r\n \"timeWindowSize\": 1,\r\n \"timeWindowUnit\":
\"d\",\r\n \"thresholdComparator\": \"<\",\r\n \"threshold\": [\r\n
1000\r\n ],\r\n \"size\": 10,\r\n \"timeField\": \"date\"\r\n },\r\n
\"consumer\": \"stackAlerts\",\r\n \"rule_type_id\": \".es-query\",\r\n
\"schedule\": {\r\n \"interval\": \"5d\"\r\n },\r\n \"name\": \"test
rule\"\r\n}\r\n```","sha":"0e7798a4024b6f9f60700e60101868960172796d"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/170463","number":170463,"mergeCommit":{"message":"[ResponseOps]
Elasticsearch query rule with ES|QL threshold validation
(elastic#170463)\n\nResolves
https://github.com/elastic/kibana/issues/170360\r\n\r\n##
Summary\r\n\r\nWe should be throwing an error if a user tries to create
an ESQL es\r\nquery rule where `thresholdCompartor != '>'` or `threshold
!= 0` or\r\n`timeField` is not defined.\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n\r\n### To
verify\r\n\r\n- Go to [dev
tools](http://localhost:5601/app/dev_tools#/console)\r\n- Run the
following and edit thresholdComparator, threshold, or\r\ntimeField and
verify that you see errors thrown.\r\n```\r\nPOST
kbn:/api/alerting/rule\r\n{\r\n \"params\": {\r\n \"searchType\":
\"esqlQuery\",\r\n \"esqlQuery\": {\r\n \"esql\": \"\"\"from
kibana_sample_data_logs\r\n| keep bytes, clientip, host, geo.dest\r\n|
where geo.dest != \"GB\"\r\n| stats sumbytes = sum(bytes) by clientip,
host\r\n| WHERE sumbytes > 5000\r\n| sort sumbytes desc\r\n| limit
10\"\"\"\r\n },\r\n \"timeWindowSize\": 1,\r\n \"timeWindowUnit\":
\"d\",\r\n \"thresholdComparator\": \"<\",\r\n \"threshold\": [\r\n
1000\r\n ],\r\n \"size\": 10,\r\n \"timeField\": \"date\"\r\n },\r\n
\"consumer\": \"stackAlerts\",\r\n \"rule_type_id\": \".es-query\",\r\n
\"schedule\": {\r\n \"interval\": \"5d\"\r\n },\r\n \"name\": \"test
rule\"\r\n}\r\n```","sha":"0e7798a4024b6f9f60700e60101868960172796d"}}]}]
BACKPORT-->

Co-authored-by: Alexi Doak <[email protected]>
  • Loading branch information
kibanamachine and doakalexi authored Nov 2, 2023
1 parent 287589d commit 5b0b46d
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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',
});

Expand All @@ -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);
Expand All @@ -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,
});
Expand Down Expand Up @@ -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);
Expand All @@ -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<any[]> {
return await esTestIndexToolOutput.waitForDocs(
ES_TEST_INDEX_SOURCE,
Expand All @@ -248,7 +359,6 @@ export default function ruleTests({ getService }: FtrProviderContext) {

interface CreateRuleParams {
name: string;
size: number;
esqlQuery: string;
timeWindowSize?: number;
timeField?: string;
Expand Down Expand Up @@ -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: '>',
Expand Down

0 comments on commit 5b0b46d

Please sign in to comment.