Skip to content

Commit f28d5a2

Browse files
dhurley14dhurley14
committed
[SECURITY SOLUTION] [Detections] Add / Update e2e tests to ensure initial rule runs are successful (#68441)
* adds/modifies e2e tests to ensure find_status returns succeeded after rules are created, instead of just 'going to run' * add documentation around newly created e2e tests explaining bug and specific regression to be on the lookout for if these start failing
1 parent 622470c commit f28d5a2

File tree

3 files changed

+108
-3
lines changed

3 files changed

+108
-3
lines changed

‎x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66

77
import expect from '@kbn/expect';
88

9-
import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants';
9+
import {
10+
DETECTION_ENGINE_RULES_URL,
11+
DETECTION_ENGINE_RULES_STATUS_URL,
12+
} from '../../../../plugins/security_solution/common/constants';
1013
import { FtrProviderContext } from '../../common/ftr_provider_context';
1114
import {
1215
createSignalsIndex,
@@ -65,6 +68,46 @@ export default ({ getService }: FtrProviderContext) => {
6568
expect(bodyToCompare).to.eql(getSimpleRuleOutput());
6669
});
6770

71+
/*
72+
This test is to ensure no future regressions introduced by the following scenario
73+
a call to updateApiKey was invalidating the api key used by the
74+
rule while the rule was executing, or even before it executed,
75+
on the first rule run.
76+
this pr https://github.com/elastic/kibana/pull/68184
77+
fixed this by finding the true source of a bug that required the manual
78+
api key update, and removed the call to that function.
79+
80+
When the api key is updated before / while the rule is executing, the alert
81+
executor no longer has access to a service to update the rule status
82+
saved object in Elasticsearch. Because of this, we cannot set the rule into
83+
a 'failure' state, so the user ends up seeing 'going to run' as that is the
84+
last status set for the rule before it erupts in an error that cannot be
85+
recorded inside of the executor.
86+
87+
This adds an e2e test for the backend to catch that in case
88+
this pops up again elsewhere.
89+
*/
90+
it('should create a single rule with a rule_id and validate it ran successfully', async () => {
91+
const simpleRule = getSimpleRule();
92+
const { body } = await supertest
93+
.post(DETECTION_ENGINE_RULES_URL)
94+
.set('kbn-xsrf', 'true')
95+
.send(simpleRule)
96+
.expect(200);
97+
98+
// wait for Task Manager to execute the rule and update status
99+
await new Promise((resolve) => setTimeout(resolve, 5000));
100+
const { body: statusBody } = await supertest
101+
.post(DETECTION_ENGINE_RULES_STATUS_URL)
102+
.set('kbn-xsrf', 'true')
103+
.send({ ids: [body.id] })
104+
.expect(200);
105+
106+
const bodyToCompare = removeServerGeneratedProperties(body);
107+
expect(bodyToCompare).to.eql(getSimpleRuleOutput());
108+
expect(statusBody[body.id].current_status.status).to.eql('succeeded');
109+
});
110+
68111
it('should create a single rule without an input index', async () => {
69112
const { index, ...payload } = getSimpleRule();
70113
const { index: _index, ...expected } = getSimpleRuleOutput();

‎x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66

77
import expect from '@kbn/expect';
88

9-
import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants';
9+
import {
10+
DETECTION_ENGINE_RULES_URL,
11+
DETECTION_ENGINE_RULES_STATUS_URL,
12+
} from '../../../../plugins/security_solution/common/constants';
1013
import { FtrProviderContext } from '../../common/ftr_provider_context';
1114
import {
1215
createSignalsIndex,
@@ -68,6 +71,46 @@ export default ({ getService }: FtrProviderContext): void => {
6871
expect(bodyToCompare).to.eql(getSimpleRuleOutput());
6972
});
7073

74+
/*
75+
This test is to ensure no future regressions introduced by the following scenario
76+
a call to updateApiKey was invalidating the api key used by the
77+
rule while the rule was executing, or even before it executed,
78+
on the first rule run.
79+
this pr https://github.com/elastic/kibana/pull/68184
80+
fixed this by finding the true source of a bug that required the manual
81+
api key update, and removed the call to that function.
82+
83+
When the api key is updated before / while the rule is executing, the alert
84+
executor no longer has access to a service to update the rule status
85+
saved object in Elasticsearch. Because of this, we cannot set the rule into
86+
a 'failure' state, so the user ends up seeing 'going to run' as that is the
87+
last status set for the rule before it erupts in an error that cannot be
88+
recorded inside of the executor.
89+
90+
This adds an e2e test for the backend to catch that in case
91+
this pops up again elsewhere.
92+
*/
93+
it('should create a single rule with a rule_id and validate it ran successfully', async () => {
94+
const simpleRule = getSimpleRule();
95+
const { body } = await supertest
96+
.post(`${DETECTION_ENGINE_RULES_URL}/_bulk_create`)
97+
.set('kbn-xsrf', 'true')
98+
.send([simpleRule])
99+
.expect(200);
100+
101+
// wait for Task Manager to execute the rule and update status
102+
await new Promise((resolve) => setTimeout(resolve, 5000));
103+
const { body: statusBody } = await supertest
104+
.post(DETECTION_ENGINE_RULES_STATUS_URL)
105+
.set('kbn-xsrf', 'true')
106+
.send({ ids: [body[0].id] })
107+
.expect(200);
108+
109+
const bodyToCompare = removeServerGeneratedProperties(body[0]);
110+
expect(bodyToCompare).to.eql(getSimpleRuleOutput());
111+
expect(statusBody[body[0].id].current_status.status).to.eql('succeeded');
112+
});
113+
71114
it('should create a single rule without a rule_id', async () => {
72115
const { body } = await supertest
73116
.post(`${DETECTION_ENGINE_RULES_URL}/_bulk_create`)

‎x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,25 @@ export default ({ getService }: FtrProviderContext): void => {
4242
expect(body).to.eql({});
4343
});
4444

45+
/*
46+
This test is to ensure no future regressions introduced by the following scenario
47+
a call to updateApiKey was invalidating the api key used by the
48+
rule while the rule was executing, or even before it executed,
49+
on the first rule run.
50+
this pr https://github.com/elastic/kibana/pull/68184
51+
fixed this by finding the true source of a bug that required the manual
52+
api key update, and removed the call to that function.
53+
54+
When the api key is updated before / while the rule is executing, the alert
55+
executor no longer has access to a service to update the rule status
56+
saved object in Elasticsearch. Because of this, we cannot set the rule into
57+
a 'failure' state, so the user ends up seeing 'going to run' as that is the
58+
last status set for the rule before it erupts in an error that cannot be
59+
recorded inside of the executor.
60+
61+
This adds an e2e test for the backend to catch that in case
62+
this pops up again elsewhere.
63+
*/
4564
it('should return a single rule status when a single rule is loaded from a find status with defaults added', async () => {
4665
// add a single rule
4766
const { body: resBody } = await supertest
@@ -61,7 +80,7 @@ export default ({ getService }: FtrProviderContext): void => {
6180
.expect(200);
6281

6382
// expected result for status should be 'going to run' or 'succeeded
64-
expect(['succeeded', 'going to run']).to.contain(body[resBody.id].current_status.status);
83+
expect(body[resBody.id].current_status.status).to.eql('succeeded');
6584
});
6685
});
6786
};

0 commit comments

Comments
 (0)