Skip to content

Commit

Permalink
[Security Solution] Fix flaky test: x-pack/test/detection_engine_api_…
Browse files Browse the repository at this point in the history
…integration/security_and_spaces/update_prebuilt_rules_package/update_prebuilt_rules_package·ts - update_prebuilt_rules_package should allow user to install prebuilt rules from scratch, then install new rules and upgrade existing rules from the new package (#163241)

Fixes: #162658

## Summary

- Fixes flaky test:
`x-pack/test/detection_engine_api_integration/security_and_spaces/update_prebuilt_rules_package/update_prebuilt_rules_package·ts`
- Test title: `update_prebuilt_rules_package should allow user to
install prebuilt rules from scratch, then install new rules and upgrade
existing rules from the new package`

## Passing flaky test runner

300 runs:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2799

## Root cause and what this PR does

- On initial investigation, the flaky test runner was executed for this
test with 300 iterations and all of them succeeded. This gives us great
confidence that the test is not actually flaky.
- Further investigation showed that @kibanamachine reported this tests
as failing when running the CI for two backport PRs to `8.9`:
-
https://buildkite.com/elastic/kibana-on-merge/builds/33282#0189987d-3a80-49c2-8332-3105ec3c2109
-
https://buildkite.com/elastic/kibana-on-merge/builds/33444#0189b1fa-4bc4-4422-9ce9-5c9a24f11ad5
- These flakiness was caused **by a race condition** between the writing
of rules into indeces, and them being made available for reading. This
is a known issue, already and solved for other integration test, by
manually refreshing ES's indeces.
- In order to reduce the probability of flakiness in a similar scenario,
this PR adds code to refresh the indices after each rule installation or
upgrade during the test.

## Refactor

- Moves the refreshing of the indexes within the utility function that
write to indexes:
    - `installPrebuiltRules`
    - `upgradePrebuiltRules`
    - `installPrebuiltRulesAndTimelines` (legacy, but still used)
    - `installPrebuiltRulesFleetPackage`
- Creates 2 new utils:
- `installPrebuiltRulesPackageByVersion`, which installs
`security_detection_engine` package via Fleet API, with its version
passed in as param
- `getInstalledRules`, reusable function to fetch all installed rules
    -

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jpdjere and kibanamachine committed Aug 11, 2023
1 parent a86c016 commit 2ba659d
Show file tree
Hide file tree
Showing 16 changed files with 249 additions and 153 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ export default ({ getService }: FtrProviderContext): void => {
threat: generateThreatArray(1),
}),
]);
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

const expectedRule = await createRule(supertest, log, {
...getSimpleRule('rule-1'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ALL_SAVED_OBJECT_INDICES } from '@kbn/core-saved-objects-server';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import { deleteAllPrebuiltRuleAssets, deleteAllRules } from '../../utils';
import { getPrebuiltRulesStatus } from '../../utils/prebuilt_rules/get_prebuilt_rules_status';
import { installPrebuiltRulesPackageByVersion } from '../../utils/prebuilt_rules/install_fleet_package_by_url';

// eslint-disable-next-line import/no-default-export
export default ({ getService }: FtrProviderContext): void => {
Expand Down Expand Up @@ -55,18 +56,17 @@ export default ({ getService }: FtrProviderContext): void => {
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_install).toBe(0);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);

const EPM_URL = `/api/fleet/epm/packages/security_detection_engine/99.0.0`;

const bundledInstallResponse = await supertest
.post(EPM_URL)
.set('kbn-xsrf', 'xxxx')
.type('application/json')
.send({ force: true })
.expect(200);
const bundledInstallResponse = await installPrebuiltRulesPackageByVersion(
es,
supertest,
'99.0.0'
);

// As opposed to "registry"
expect(bundledInstallResponse.body._meta.install_source).toBe('bundled');
expect(bundledInstallResponse._meta.install_source).toBe('bundled');

// Refresh ES indices to avoid race conditions between write and reading of indeces
// See implementation utility function at x-pack/test/detection_engine_api_integration/utils/prebuilt_rules/install_prebuilt_rules_fleet_package.ts
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

// Verify that status is updated after package installation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { ALL_SAVED_OBJECT_INDICES } from '@kbn/core-saved-objects-server';
import { DETECTION_ENGINE_RULES_URL_FIND } from '@kbn/security-solution-plugin/common/constants';
import expect from 'expect';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import { deleteAllPrebuiltRuleAssets, deleteAllRules } from '../../utils';
import { getInstalledRules } from '../../utils/prebuilt_rules/get_installed_rules';
import { getPrebuiltRulesStatus } from '../../utils/prebuilt_rules/get_prebuilt_rules_status';
import { installPrebuiltRules } from '../../utils/prebuilt_rules/install_prebuilt_rules';

Expand Down Expand Up @@ -38,8 +37,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_install).toBe(0);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);

await installPrebuiltRules(supertest);
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });
await installPrebuiltRules(es, supertest);

// Verify that status is updated after package installation
const statusAfterPackageInstallation = await getPrebuiltRulesStatus(supertest);
Expand All @@ -48,11 +46,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);

// Get installed rules
const { body: rulesResponse } = await supertest
.get(DETECTION_ENGINE_RULES_URL_FIND)
.set('kbn-xsrf', 'true')
.send()
.expect(200);
const rulesResponse = await getInstalledRules(supertest);

// Assert that installed rules are from package 99.0.0 and not from prerelease (beta) package
expect(rulesResponse.data.length).toBe(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/
import expect from 'expect';
import { ALL_SAVED_OBJECT_INDICES } from '@kbn/core-saved-objects-server';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import { deleteAllRules, getPrebuiltRulesAndTimelinesStatus } from '../../utils';
import { deleteAllPrebuiltRuleAssets } from '../../utils/prebuilt_rules/delete_all_prebuilt_rule_assets';
Expand Down Expand Up @@ -36,8 +35,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(statusBeforePackageInstallation.rules_not_updated).toBe(0);

// Install the package with 15000 prebuilt historical version of rules rules and 750 unique rules
await installPrebuiltRulesAndTimelines(supertest);
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });
await installPrebuiltRulesAndTimelines(es, supertest);

// Verify that status is updated after package installation
const statusAfterPackageInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/
import expect from 'expect';
import { ALL_SAVED_OBJECT_INDICES } from '@kbn/core-saved-objects-server';
import { FtrProviderContext } from '../../common/ftr_provider_context';
import {
deleteAllRules,
Expand Down Expand Up @@ -43,44 +42,22 @@ export default ({ getService }: FtrProviderContext): void => {
expect(statusBeforePackageInstallation.rules_not_updated).toBe(0);

await installPrebuiltRulesFleetPackage({
es,
supertest,
overrideExistingPackage: true,
});

// Before we proceed, we need to refresh saved object indices. This comment will explain why.
// At the previous step we installed the Fleet package with prebuilt detection rules.
// Prebuilt rules are assets that Fleet indexes as saved objects of a certain type.
// Fleet does this via a savedObjectsClient.import() call with explicit `refresh: false`.
// So, despite of the fact that the endpoint waits until the prebuilt rule assets will be
// successfully indexed, it doesn't wait until they become "visible" for subsequent read
// operations. Which is what we do next: we read these SOs in getPrebuiltRulesAndTimelinesStatus().
// Now, the time left until the next refresh can be anything from 0 to the default value, and
// it depends on the time when savedObjectsClient.import() call happens relative to the time of
// the next refresh. Also, probably the refresh time can be delayed when ES is under load?
// Anyway, here we have a race condition between a write and subsequent read operation, and to
// fix it deterministically we have to refresh saved object indices and wait until it's done.
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

// Verify that status is updated after package installation
const statusAfterPackageInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
expect(statusAfterPackageInstallation.rules_installed).toBe(0);
expect(statusAfterPackageInstallation.rules_not_installed).toBeGreaterThan(0);
expect(statusAfterPackageInstallation.rules_not_updated).toBe(0);

// Verify that all previously not installed rules were installed
const response = await installPrebuiltRulesAndTimelines(supertest);
const response = await installPrebuiltRulesAndTimelines(es, supertest);
expect(response.rules_installed).toBe(statusAfterPackageInstallation.rules_not_installed);
expect(response.rules_updated).toBe(0);

// Similar to the previous refresh, we need to do it again between the two operations:
// - previous write operation: install prebuilt rules and timelines
// - subsequent read operation: get prebuilt rules and timelines status
// You may ask why? I'm not sure, probably because the write operation can install the Fleet
// package under certain circumstances, and it all works with `refresh: false` again.
// Anyway, there were flaky runs failing specifically at one of the next assertions,
// which means some kind of the same race condition we have here too.
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

// Verify that status is updated after rules installation
const statusAfterRuleInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
expect(statusAfterRuleInstallation.rules_installed).toBe(response.rules_installed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return the number of installed prebuilt rules after installing them', async () => {
await createPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

const { stats } = await getPrebuiltRulesStatus(supertest);
expect(stats).toMatchObject({
Expand All @@ -95,7 +95,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should notify the user again that a rule is available for install after it is deleted', async () => {
await createPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);
await deleteRule(supertest, 'rule-1');

const { stats } = await getPrebuiltRulesStatus(supertest);
Expand All @@ -110,7 +110,7 @@ export default ({ getService }: FtrProviderContext): void => {
it('should return available rule updates', async () => {
const ruleAssetSavedObjects = getRuleAssetSavedObjects();
await createPrebuiltRuleAssetSavedObjects(es, ruleAssetSavedObjects);
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

// Clear previous rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand All @@ -130,15 +130,15 @@ export default ({ getService }: FtrProviderContext): void => {
it('should not return any available update if rule has been successfully upgraded', async () => {
const ruleAssetSavedObjects = getRuleAssetSavedObjects();
await createPrebuiltRuleAssetSavedObjects(es, ruleAssetSavedObjects);
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

// Clear previous rule assets
await deleteAllPrebuiltRuleAssets(es);
// Increment the version of one of the installed rules and create the new rule assets
ruleAssetSavedObjects[0]['security-rule'].version += 1;
await createPrebuiltRuleAssetSavedObjects(es, ruleAssetSavedObjects);
// Upgrade all rules
await upgradePrebuiltRules(supertest);
await upgradePrebuiltRules(es, supertest);

const { stats } = await getPrebuiltRulesStatus(supertest);
expect(stats).toMatchObject({
Expand All @@ -152,7 +152,7 @@ export default ({ getService }: FtrProviderContext): void => {
it('should not return any updates if none are available', async () => {
const ruleAssetSavedObjects = getRuleAssetSavedObjects();
await createPrebuiltRuleAssetSavedObjects(es, ruleAssetSavedObjects);
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

// Clear previous rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand Down Expand Up @@ -193,7 +193,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return the number of installed prebuilt rules after installing them', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

const { stats } = await getPrebuiltRulesStatus(supertest);
expect(stats).toMatchObject({
Expand All @@ -206,7 +206,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should notify the user again that a rule is available for install after it is deleted', async () => {
await createPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);
await deleteRule(supertest, 'rule-1');

const { stats } = await getPrebuiltRulesStatus(supertest);
Expand All @@ -220,7 +220,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return available rule updates when previous historical versions available', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

// Add a new version of one of the installed rules
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
Expand All @@ -238,7 +238,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return available rule updates when previous historical versions unavailable', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

// Delete the previous versions of rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand All @@ -261,7 +261,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should not return available rule updates after rule has been upgraded', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRules(supertest);
await installPrebuiltRules(es, supertest);

// Delete the previous versions of rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand All @@ -272,7 +272,7 @@ export default ({ getService }: FtrProviderContext): void => {
]);

// Upgrade the rule
await upgradePrebuiltRules(supertest);
await upgradePrebuiltRules(es, supertest);

const { stats } = await getPrebuiltRulesStatus(supertest);
expect(stats).toMatchObject({
Expand Down Expand Up @@ -339,7 +339,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return the number of installed prebuilt rules after installing them', async () => {
await createPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

const body = await getPrebuiltRulesAndTimelinesStatus(supertest);
expect(body).toMatchObject({
Expand All @@ -352,7 +352,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should notify the user again that a rule is available for install after it is deleted', async () => {
await createPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);
await deleteRule(supertest, 'rule-1');

const body = await getPrebuiltRulesAndTimelinesStatus(supertest);
Expand All @@ -367,7 +367,7 @@ export default ({ getService }: FtrProviderContext): void => {
it('should return available rule updates', async () => {
const ruleAssetSavedObjects = getRuleAssetSavedObjects();
await createPrebuiltRuleAssetSavedObjects(es, ruleAssetSavedObjects);
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

// Clear previous rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand All @@ -387,7 +387,7 @@ export default ({ getService }: FtrProviderContext): void => {
it('should not return any updates if none are available', async () => {
const ruleAssetSavedObjects = getRuleAssetSavedObjects();
await createPrebuiltRuleAssetSavedObjects(es, ruleAssetSavedObjects);
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

// Clear previous rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand Down Expand Up @@ -428,7 +428,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return the number of installed prebuilt rules after installing them', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

const body = await getPrebuiltRulesAndTimelinesStatus(supertest);
expect(body).toMatchObject({
Expand All @@ -441,7 +441,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should notify the user again that a rule is available for install after it is deleted', async () => {
await createPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);
await deleteRule(supertest, 'rule-1');

const body = await getPrebuiltRulesAndTimelinesStatus(supertest);
Expand All @@ -455,7 +455,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return available rule updates when previous historical versions available', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

// Add a new version of one of the installed rules
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [
Expand All @@ -473,7 +473,7 @@ export default ({ getService }: FtrProviderContext): void => {

it('should return available rule updates when previous historical versions unavailable', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, getRuleAssetSavedObjects());
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

// Delete the previous versions of rule assets
await deleteAllPrebuiltRuleAssets(es);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default ({ getService }: FtrProviderContext): void => {
});

it('should return the number of installed timeline templates after installing them', async () => {
await installPrebuiltRulesAndTimelines(supertest);
await installPrebuiltRulesAndTimelines(es, supertest);

const body = await getPrebuiltRulesAndTimelinesStatus(supertest);
expect(body).toMatchObject({
Expand Down
Loading

0 comments on commit 2ba659d

Please sign in to comment.