From f28d5a2ccd710fcf9f6df346ff9e777aa892a447 Mon Sep 17 00:00:00 2001 From: "Devin W. Hurley" Date: Tue, 9 Jun 2020 08:41:37 -0400 Subject: [PATCH] [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 --- .../security_and_spaces/tests/create_rules.ts | 45 ++++++++++++++++++- .../tests/create_rules_bulk.ts | 45 ++++++++++++++++++- .../tests/find_statuses.ts | 21 ++++++++- 3 files changed, 108 insertions(+), 3 deletions(-) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules.ts index 449168d73f4ef..8847b0c3250ac 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules.ts @@ -6,7 +6,10 @@ import expect from '@kbn/expect'; -import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants'; +import { + DETECTION_ENGINE_RULES_URL, + DETECTION_ENGINE_RULES_STATUS_URL, +} from '../../../../plugins/security_solution/common/constants'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { createSignalsIndex, @@ -65,6 +68,46 @@ export default ({ getService }: FtrProviderContext) => { expect(bodyToCompare).to.eql(getSimpleRuleOutput()); }); + /* + This test is to ensure no future regressions introduced by the following scenario + a call to updateApiKey was invalidating the api key used by the + rule while the rule was executing, or even before it executed, + on the first rule run. + this pr https://github.com/elastic/kibana/pull/68184 + fixed this by finding the true source of a bug that required the manual + api key update, and removed the call to that function. + + When the api key is updated before / while the rule is executing, the alert + executor no longer has access to a service to update the rule status + saved object in Elasticsearch. Because of this, we cannot set the rule into + a 'failure' state, so the user ends up seeing 'going to run' as that is the + last status set for the rule before it erupts in an error that cannot be + recorded inside of the executor. + + This adds an e2e test for the backend to catch that in case + this pops up again elsewhere. + */ + it('should create a single rule with a rule_id and validate it ran successfully', async () => { + const simpleRule = getSimpleRule(); + const { body } = await supertest + .post(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send(simpleRule) + .expect(200); + + // wait for Task Manager to execute the rule and update status + await new Promise((resolve) => setTimeout(resolve, 5000)); + const { body: statusBody } = await supertest + .post(DETECTION_ENGINE_RULES_STATUS_URL) + .set('kbn-xsrf', 'true') + .send({ ids: [body.id] }) + .expect(200); + + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql(getSimpleRuleOutput()); + expect(statusBody[body.id].current_status.status).to.eql('succeeded'); + }); + it('should create a single rule without an input index', async () => { const { index, ...payload } = getSimpleRule(); const { index: _index, ...expected } = getSimpleRuleOutput(); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts index 6c3b1c45e202e..47b09fd1fe3c7 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/create_rules_bulk.ts @@ -6,7 +6,10 @@ import expect from '@kbn/expect'; -import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants'; +import { + DETECTION_ENGINE_RULES_URL, + DETECTION_ENGINE_RULES_STATUS_URL, +} from '../../../../plugins/security_solution/common/constants'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { createSignalsIndex, @@ -68,6 +71,46 @@ export default ({ getService }: FtrProviderContext): void => { expect(bodyToCompare).to.eql(getSimpleRuleOutput()); }); + /* + This test is to ensure no future regressions introduced by the following scenario + a call to updateApiKey was invalidating the api key used by the + rule while the rule was executing, or even before it executed, + on the first rule run. + this pr https://github.com/elastic/kibana/pull/68184 + fixed this by finding the true source of a bug that required the manual + api key update, and removed the call to that function. + + When the api key is updated before / while the rule is executing, the alert + executor no longer has access to a service to update the rule status + saved object in Elasticsearch. Because of this, we cannot set the rule into + a 'failure' state, so the user ends up seeing 'going to run' as that is the + last status set for the rule before it erupts in an error that cannot be + recorded inside of the executor. + + This adds an e2e test for the backend to catch that in case + this pops up again elsewhere. + */ + it('should create a single rule with a rule_id and validate it ran successfully', async () => { + const simpleRule = getSimpleRule(); + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_bulk_create`) + .set('kbn-xsrf', 'true') + .send([simpleRule]) + .expect(200); + + // wait for Task Manager to execute the rule and update status + await new Promise((resolve) => setTimeout(resolve, 5000)); + const { body: statusBody } = await supertest + .post(DETECTION_ENGINE_RULES_STATUS_URL) + .set('kbn-xsrf', 'true') + .send({ ids: [body[0].id] }) + .expect(200); + + const bodyToCompare = removeServerGeneratedProperties(body[0]); + expect(bodyToCompare).to.eql(getSimpleRuleOutput()); + expect(statusBody[body[0].id].current_status.status).to.eql('succeeded'); + }); + it('should create a single rule without a rule_id', async () => { const { body } = await supertest .post(`${DETECTION_ENGINE_RULES_URL}/_bulk_create`) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses.ts index 6ff5a8c552ab7..f659857b89a97 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/find_statuses.ts @@ -42,6 +42,25 @@ export default ({ getService }: FtrProviderContext): void => { expect(body).to.eql({}); }); + /* + This test is to ensure no future regressions introduced by the following scenario + a call to updateApiKey was invalidating the api key used by the + rule while the rule was executing, or even before it executed, + on the first rule run. + this pr https://github.com/elastic/kibana/pull/68184 + fixed this by finding the true source of a bug that required the manual + api key update, and removed the call to that function. + + When the api key is updated before / while the rule is executing, the alert + executor no longer has access to a service to update the rule status + saved object in Elasticsearch. Because of this, we cannot set the rule into + a 'failure' state, so the user ends up seeing 'going to run' as that is the + last status set for the rule before it erupts in an error that cannot be + recorded inside of the executor. + + This adds an e2e test for the backend to catch that in case + this pops up again elsewhere. + */ it('should return a single rule status when a single rule is loaded from a find status with defaults added', async () => { // add a single rule const { body: resBody } = await supertest @@ -61,7 +80,7 @@ export default ({ getService }: FtrProviderContext): void => { .expect(200); // expected result for status should be 'going to run' or 'succeeded - expect(['succeeded', 'going to run']).to.contain(body[resBody.id].current_status.status); + expect(body[resBody.id].current_status.status).to.eql('succeeded'); }); }); };