Skip to content

Commit

Permalink
[Security Solutions][Detection Engine] Adds e2e FTR runtime support a…
Browse files Browse the repository at this point in the history
…nd 213 tests for exception lists (#83764)

## Summary

Adds support to the end to end (e2e) functional test runner (FTR) support for rule runtime tests as well as 213 tests for the exception lists which include value based lists. Previously we had limited runtime support, but as I scaled up runtime tests from 5 to 200+ I noticed in a lot of areas we had to use improved techniques for determinism.

The runtime support being added is our next step of tests. Up to now most of our e2e FTR tests have been structural testing of REST and API integration tests. Basically up to now 95% tests are API structural as:

* Call REST input related to a rule such as GET/PUT/POST/PATCH/DELETE.
* Check REST output of the rule, did it match expected output body and status code?
* In some rare cases we check if the the rule can be executed and we get a status of 'succeeded'

With only a small part of our tests ~5%, `generating_signals.ts` was checking the signals being produced. However, we cannot have confidence in runtime based tests until the structural tests have been built up and run through the weeks against PR's to ensure that those are stable and deterministic.

Now that we have confidence and 90%+ coverage of the structural REST based tests, we are building up newer sets of tests which allow us to do runtime based validation tests to increase confidence that:

* Detection engine produces signals as expected
* Structure of the signals are as expected, including signal on signals
* Exceptions to signals are working as expected
* Most runtime bugs can be TDD'ed with e2e FTR's and regressions
* Whack-a-mole will not happen
* Consistency and predictability of signals is validated
* Refactoring can occur with stronger confidence
* Runtime tests are reference points for answering questions about existing bugs or adding new ones to test if users are experiencing unexpected behaviors  
* Scaling tests can happen without failures
* Velocity for creating tests increases as the utilities and examples increase

Lastly, this puts us within striking distance of creating FTR's for different common class of runtime situations such as:
* Creating tests that exercise each rule against a set of data criteria and get signal hits
* Creating tests that validate the rule overrides operate as expected against data sets
* Creating tests that validate malfunctions, corner cases, or misuse cases such as data sets that are _all_ arrays or data sets that put numbers as strings or throws in an expected `null` instead of a value. 

These tests follow the pattern of:
* Add the smallest data set to a folder in data.json (not gzip format)
* Add the smallest mapping to that folder (mapping.json) 
* Call REST input related to exception lists, value lists, adding prepackaged rules, etc...
* Call REST input related endpoint with utilities to create and activate the rule
* Wait for the rule to go into the `succeeded` phase
* Wait for the N exact signals specific to that rule to be available
* Check against the set of signals to ensure that the matches are exactly as expected 

Example of one runtime test:

A keyword data set is added to a folder called "keyword" but you can add one anywhere you want under `es_archives`, I just grouped mine depending on the situation of the runtime. Small non-gzipped tests `data.json` and `mappings.json` are the best approach for small focused tests. For _larger_ tests and cases I would and sometimes do use things such as auditbeat but try to avoid using larger data sets in favor of smaller focused test cases to validate the runtime is operating as expected.

```ts
{
  "type": "doc",
  "value": {
    "id": "1",
    "index": "long",
    "source": {
      "@timestamp": "2020-10-28T05:00:53.000Z",
      "long": 1
    },
    "type": "_doc"
  }
}

{
  "type": "doc",
  "value": {
    "id": "2",
    "index": "long",
    "source": {
      "@timestamp": "2020-10-28T05:01:53.000Z",
      "long": 2
    },
    "type": "_doc"
  }
}

{
  "type": "doc",
  "value": {
    "id": "3",
    "index": "long",
    "source": {
      "@timestamp": "2020-10-28T05:02:53.000Z",
      "long": 3
    },
    "type": "_doc"
  }
}

{
  "type": "doc",
  "value": {
    "id": "4",
    "index": "long",
    "source": {
      "@timestamp": "2020-10-28T05:03:53.000Z",
      "long": 4
    },
    "type": "_doc"
  }
}
```

Mapping is added. Note that this is "ECS tolerant" but not necessarily all ECS meaning I can and will try to keep things simple where I can, but I have ensured that  `"@timestamp"` is at least there.

```ts
{
  "type": "index",
  "value": {
    "index": "long",
    "mappings": {
      "properties": {
        "@timestamp": {
          "type": "date"
        },
        "long": { "type": "long" }
      }
    },
    "settings": {
      "index": {
        "number_of_replicas": "1",
        "number_of_shards": "1"
      }
    }
  }
}
```

Test is written with test utilities where the `beforeEach` and `afterEach` try and clean up the indexes and load/unload the archives to keep one test from effecting another. Note this is never going to be 100% possible so see below on how we add more determinism in case something escapes the sandbox. 
```ts
    beforeEach(async () => {
      await createSignalsIndex(supertest);
      await createListsIndex(supertest);
      await esArchiver.load('rule_exceptions/keyword');
    });

    afterEach(async () => {
      await deleteSignalsIndex(supertest);
      await deleteAllAlerts(supertest);
      await deleteAllExceptions(es);
      await deleteListsIndex(supertest);
      await esArchiver.unload('rule_exceptions/keyword');
    });

    describe('"is" operator', () => {
      it('should filter 1 single keyword if it is set as an exception', async () => {
        const rule = getRuleForSignalTesting(['keyword']);
        const { id } = await createRuleWithExceptionEntries(supertest, rule, [
          [
            {
              field: 'keyword',
              operator: 'included',
              type: 'match',
              value: 'word one',
            },
          ],
        ]);
        await waitForRuleSuccess(supertest, id);
        await waitForSignalsToBePresent(supertest, 3, [id]);
        const signalsOpen = await getSignalsById(supertest, id);
        const hits = signalsOpen.hits.hits.map((hit) => hit._source.keyword).sort();
        expect(hits).to.eql(['word four', 'word three', 'word two']);
      });
   });
```

### Changes for better determinism
To support more determinism there are changes and utilities added which can be tuned during any sporadic failures we might encounter as well as better support unexpected changes to other Elastic Stack pieces such as alerting, task manager, etc...

Get simple rule and others are now defaulting to false, meaning that the structural tests will no longer activate a rule and run it on task manger. This should cut down on error outputs as well as reduce stress and potentials for left over rules interfering with the runtime rules. 
```ts
export const getSimpleRule = (ruleId = 'rule-1', enabled = false): QueryCreateSchema => ({
```

Not mandatory to use, but for most tests that should be runtime based tests, I use this function below which will enable it by default and run it using settings such as `type: 'query'`, `query: '*:*',` `from: '1900-01-01T00:00:00.000Z'`, to cut down on boiler plate noise. However, people can use whatever they want out of the grab bag or if their test is more readable to hand craft a REST request to create signals, or if they just want to call this and override where they want to, then 👍 .
 ```ts
export const getRuleForSignalTesting = (index: string[], ruleId = 'rule-1', enabled = true)
```

This waits for a rule to succeed before continuing
```ts
await waitForRuleSuccess(supertest, id);
```

I added a required array of id that _waits_ only for that particular id here. This is useful in case another test did not cleanup and you are getting signals being produced or left behind but need to wait specifically for yours.
```ts
await waitForSignalsToBePresent(supertest, 4, [id]);
```

I only get the signals for a particular rule id using either the auto-generated id or the rule_id. It's safer to use the ones from the auto-generated id but either of these are fine if you're careful enough. 
```ts
const signalsOpen = await getSignalsById(supertest, id);
const signalsOpen = await getSignalsByIds(supertest, [createdId]);
const signalsOpen = await getSignalsByRuleIds(supertest, ['signal-on-signal']);
```

I delete all alerts now through a series of steps where it properly removes all rules using the rules bulk_delete and does it in such a way that all the API keys and alerting will be the best it can destroyed as well as double check that the alerts are showing up as being cleaned up before continuing.
```ts
deleteAllAlerts()
```

When not explicitly testing something structural, prefer to use the utilities which can and will do retries in case there are over the wire failures or es failures. Examples are:
```ts
installPrePackagedRules()
waitForRuleSuccess()
importFile() // This does a _lot_ of checks to ensure that the file is fully imported before continuing
```

Some of these utilities might still do a `expect(200);` but as we are and should use regular structural tests to cover those problems, these will probably be more and more removed when/if we hit test failures in favor of doing retries, waitFor, and countDowns.

### Checklist

Delete any items that are not applicable to this PR.

- [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
  • Loading branch information
FrankHassanabad authored Nov 20, 2020
1 parent 52c6b7b commit 5f4c211
Show file tree
Hide file tree
Showing 84 changed files with 7,328 additions and 462 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,13 @@ export const getCreateExceptionListMinimalSchemaMockWithoutId = (): CreateExcept
name: NAME,
type: ENDPOINT_TYPE,
});

/**
* Useful for end to end testing with detections
*/
export const getCreateExceptionListDetectionSchemaMock = (): CreateExceptionListSchema => ({
description: DESCRIPTION,
list_id: LIST_ID,
name: NAME,
type: 'detection',
});
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,11 @@ export const getCreateSavedQueryRulesSchemaMock = (ruleId = 'rule-1'): SavedQuer
});

export const getCreateThreatMatchRulesSchemaMock = (
ruleId = 'rule-1'
ruleId = 'rule-1',
enabled = false
): ThreatMatchCreateSchema => ({
description: 'Detecting root and admin users',
enabled,
name: 'Query with a rule id',
query: 'user.name: root or user.name: admin',
severity: 'high',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,12 @@ export const getThreatMatchingSchemaMock = (anchorDate: string = ANCHOR_DATE): R
* Useful for e2e backend tests where it doesn't have date time and other
* server side properties attached to it.
*/
export const getThreatMatchingSchemaPartialMock = (): Partial<RulesSchema> => {
export const getThreatMatchingSchemaPartialMock = (enabled = false): Partial<RulesSchema> => {
return {
author: [],
created_by: 'elastic',
description: 'Detecting root and admin users',
enabled: true,
enabled,
false_positives: [],
from: 'now-6m',
immutable: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import expect from '@kbn/expect';
import { PrePackagedRulesAndTimelinesSchema } from '../../../../plugins/security_solution/common/detection_engine/schemas/response';

import { DETECTION_ENGINE_PREPACKAGED_URL } from '../../../../plugins/security_solution/common/constants';
import { FtrProviderContext } from '../../common/ftr_provider_context';
Expand All @@ -13,6 +14,7 @@ import {
deleteAllAlerts,
deleteAllTimelines,
deleteSignalsIndex,
installPrePackagedRules,
waitFor,
} from '../../utils';

Expand Down Expand Up @@ -45,71 +47,36 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
await deleteAllTimelines(es);
});

it('should contain rules_installed, rules_updated, timelines_installed, and timelines_updated', async () => {
const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(Object.keys(body)).to.eql([
it('should create the prepackaged rules and return a count greater than zero, rules_updated to be zero, and contain the correct keys', async () => {
let responseBody: unknown;
await waitFor(async () => {
const { body, status } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send();
if (status === 200) {
responseBody = body;
}
return status === 200;
}, DETECTION_ENGINE_PREPACKAGED_URL);

const prepackagedRules = responseBody as PrePackagedRulesAndTimelinesSchema;
expect(prepackagedRules.rules_installed).to.be.greaterThan(0);
expect(prepackagedRules.rules_updated).to.eql(0);
expect(Object.keys(prepackagedRules)).to.eql([
'rules_installed',
'rules_updated',
'timelines_installed',
'timelines_updated',
]);
});

it('should create the prepackaged rules and return a count greater than zero', async () => {
const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(body.rules_installed).to.be.greaterThan(0);
});

it('should create the prepackaged timelines and return a count greater than zero', async () => {
const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(body.timelines_installed).to.be.greaterThan(0);
});

it('should create the prepackaged rules that the rules_updated is of size zero', async () => {
const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(body.rules_updated).to.eql(0);
});

it('should create the prepackaged timelines and the timelines_updated is of size zero', async () => {
const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(body.timelines_updated).to.eql(0);
});

it('should be possible to call the API twice and the second time the number of rules installed should be zero', async () => {
await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);
it('should be possible to call the API twice and the second time the number of rules installed should be zero as well as timeline', async () => {
await installPrePackagedRules(supertest);

// NOTE: I call the GET call until eventually it becomes consistent and that the number of rules to install are zero.
// This is to reduce flakiness where it can for a short period of time try to install the same rule twice.
Expand All @@ -119,39 +86,23 @@ export default ({ getService }: FtrProviderContext): void => {
.set('kbn-xsrf', 'true')
.expect(200);
return body.rules_not_installed === 0;
});

const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(body.rules_installed).to.eql(0);
});

it('should be possible to call the API twice and the second time the number of timelines installed should be zero', async () => {
await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);
}, `${DETECTION_ENGINE_PREPACKAGED_URL}/_status`);

let responseBody: unknown;
await waitFor(async () => {
const { body } = await supertest
.get(`${DETECTION_ENGINE_PREPACKAGED_URL}/_status`)
const { body, status } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.expect(200);
return body.timelines_not_installed === 0;
});

const { body } = await supertest
.put(DETECTION_ENGINE_PREPACKAGED_URL)
.set('kbn-xsrf', 'true')
.send()
.expect(200);

expect(body.timelines_installed).to.eql(0);
.send();
if (status === 200) {
responseBody = body;
}
return status === 200;
}, DETECTION_ENGINE_PREPACKAGED_URL);

const prepackagedRules = responseBody as PrePackagedRulesAndTimelinesSchema;
expect(prepackagedRules.rules_installed).to.eql(0);
expect(prepackagedRules.timelines_installed).to.eql(0);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext) => {
const supertest = getService('supertest');
const es = getService('es');

describe('create_rules', () => {
describe('validation errors', () => {
Expand All @@ -51,7 +50,7 @@ export default ({ getService }: FtrProviderContext) => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should create a single rule with a rule_id', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const es = getService('es');

describe('create_rules_bulk', () => {
describe('validation errors', () => {
Expand Down Expand Up @@ -54,7 +53,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should create a single rule with a rule_id', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const es = getService('es');

describe('delete_rules', () => {
describe('deleting rules', () => {
Expand All @@ -34,7 +33,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should delete a single rule with a rule_id', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const es = getService('es');

describe('delete_rules_bulk', () => {
describe('deleting rules bulk using DELETE', () => {
Expand All @@ -34,7 +33,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should delete a single rule with a rule_id', async () => {
Expand Down Expand Up @@ -146,7 +145,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should delete a single rule with a rule_id', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const es = getService('es');

describe('export_rules', () => {
describe('exporting rules', () => {
Expand All @@ -32,7 +31,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should set the response content types to be expected', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const es = getService('es');

describe('find_rules', () => {
beforeEach(async () => {
Expand All @@ -32,7 +31,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
});

it('should return an empty find body correctly if no rules are loaded', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
await deleteAllRulesStatuses(es);
});

Expand All @@ -45,7 +45,7 @@ export default ({ getService }: FtrProviderContext): void => {
});

it('should return a single rule status when a single rule is loaded from a find status with defaults added', async () => {
const resBody = await createRule(supertest, getSimpleRule());
const resBody = await createRule(supertest, getSimpleRule('rule-1', true));

await waitForRuleSuccess(supertest, resBody.id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default ({ getService }: FtrProviderContext): void => {

afterEach(async () => {
await deleteSignalsIndex(supertest);
await deleteAllAlerts(es);
await deleteAllAlerts(supertest);
await deleteAllTimelines(es);
});

Expand Down
Loading

0 comments on commit 5f4c211

Please sign in to comment.