Skip to content

Commit

Permalink
[Security Solution] Unskip flaky tests in Prebuilt Rules FTR Integrat…
Browse files Browse the repository at this point in the history
…ion tests (elastic#173998)

**Addresses:**
elastic#172107
elastic#171380

## Summary

Unskip skipped tests in:

1.
`x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/bundled_prebuilt_rules_package/install_latest_bundled_prebuilt_rules.ts`
2.
`x-pack/test/security_solution_api_integration/test_suites/detections_response/default_license/prebuilt_rules/management/fleet_integration.ts`

- Makes the `retryDelay` in the **RetryService** in
`packages/kbn-ftr-common-functional-services/services/retry/retry.ts` a
configurable parameter - used in our `retry` util to shorten the wait
period to 200ms.
- Creates `retry` wrapper util for our FTR Integration tests, that wraps
`retry.try` from the **RetryService**, to implement maximum attempts.
- Uses this `retry` wrapper in test utils that install the
`security_detection_engine` Fleet package, and asserts that rules have
been succesfully installed - and retries if they haven't.
- Creates `refreshSavedObjectIndices` reusable util that cleans cache
and refreshes indices. Centralizes comment spread around file into this
single file.
- Use this new util to clean the cache in util that install the Fleet
package, and utils that read the prebuilt rule status.

## Flaky test runner

**Before changes:**
- For both `bundled_prebuilt_rules_package` and `management`:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4688
🟢 (250 and 250 runs)
- `bundled_prebuilt_rules_package`:
- ESS:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4805
(500 runs)
- Serverless:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4807
(500 runs)
- `management`
- ESS:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4806
(500 runs)
- Serverless:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4808
(500 runs)

**After changes:**
- `bundled_prebuilt_rules_package`:
- ESS:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4825
🟢 (500 runs)
- Serverless:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4826
🟢 (500 runs)
- `management`
- ESS:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4827
🟢 (500 runs)
- Serverless:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4828
🟢 (500 runs)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 81d6478)

# Conflicts:
#	x-pack/test/security_solution_api_integration/tsconfig.json
  • Loading branch information
jpdjere committed Jan 12, 2024
1 parent bd042cb commit 544ba3b
Show file tree
Hide file tree
Showing 23 changed files with 354 additions and 181 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,29 @@ export class RetryService extends FtrService {
public async tryForTime<T>(
timeout: number,
block: () => Promise<T>,
onFailureBlock?: () => Promise<T>
onFailureBlock?: () => Promise<T>,
retryDelay?: number
) {
return await retryForSuccess(this.log, {
timeout,
methodName: 'retry.tryForTime',
block,
onFailureBlock,
retryDelay,
});
}

public async try<T>(block: () => Promise<T>, onFailureBlock?: () => Promise<T>) {
public async try<T>(
block: () => Promise<T>,
onFailureBlock?: () => Promise<T>,
retryDelay?: number
) {
return await retryForSuccess(this.log, {
timeout: this.config.get('timeouts.try'),
methodName: 'retry.try',
block,
onFailureBlock,
retryDelay,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,21 @@ interface Options<T> {
onFailureBlock?: () => Promise<T>;
onFailure?: ReturnType<typeof defaultOnFailure>;
accept?: (v: T) => boolean;
retryDelay?: number;
}

export async function retryForSuccess<T>(log: ToolingLog, options: Options<T>) {
const { timeout, methodName, block, onFailureBlock, accept = returnTrue } = options;
const {
timeout,
methodName,
block,
onFailureBlock,
accept = returnTrue,
retryDelay = 502,
} = options;
const { onFailure = defaultOnFailure(methodName) } = options;

const start = Date.now();
const retryDelay = 502;
const criticalWebDriverErrors = ['NoSuchSessionError', 'NoSuchWindowError'];
let lastError;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export default ({ getService }: FtrProviderContext) => {
);
await updateRule(supertest, ruleToUpdate);

const status = await getPrebuiltRulesAndTimelinesStatus(supertest);
const status = await getPrebuiltRulesAndTimelinesStatus(es, supertest);
expect(status.rules_not_installed).toBe(0);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ export default ({ getService }: FtrProviderContext) => {
})
.expect(200);

const status = await getPrebuiltRulesAndTimelinesStatus(supertest);
const status = await getPrebuiltRulesAndTimelinesStatus(es, supertest);
expect(status.rules_not_installed).toEqual(0);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ export default ({ getService }: FtrProviderContext): void => {
const es = getService('es');
const supertest = getService('supertest');
const log = getService('log');
const retry = getService('retry');

// FLAKY: https://github.com/elastic/kibana/issues/171380
/* This test simulates an air-gapped environment in which the user doesn't have access to EPR.
/* We first download the package from the registry as done during build time, and then
/* attempt to install it from the local file system. The API response from EPM provides
/* us with the information of whether the package was installed from the registry or
/* from a package that was bundled with Kibana */
describe.skip('@ess @serverless @skipInQA install_bundled_prebuilt_rules', () => {
describe('@ess @serverless @skipInQA install_bundled_prebuilt_rules', () => {
beforeEach(async () => {
await deleteAllRules(supertest, log);
await deleteAllPrebuiltRuleAssets(es);
Expand All @@ -52,15 +52,16 @@ export default ({ getService }: FtrProviderContext): void => {

it('should install prebuilt rules from the package that comes bundled with Kibana', async () => {
// Verify that status is empty before package installation
const statusBeforePackageInstallation = await getPrebuiltRulesStatus(supertest);
const statusBeforePackageInstallation = await getPrebuiltRulesStatus(es, supertest);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_installed).toBe(0);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_install).toBe(0);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);

const bundledInstallResponse = await installPrebuiltRulesPackageByVersion(
es,
supertest,
'99.0.0'
'99.0.0',
retry
);

// As opposed to "registry"
Expand All @@ -71,7 +72,7 @@ export default ({ getService }: FtrProviderContext): void => {
await es.indices.refresh({ index: ALL_SAVED_OBJECT_INDICES });

// Verify that status is updated after package installation
const statusAfterPackageInstallation = await getPrebuiltRulesStatus(supertest);
const statusAfterPackageInstallation = await getPrebuiltRulesStatus(es, supertest);
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_installed).toBe(0);
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_to_install).toBeGreaterThan(0);
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default ({ getService }: FtrProviderContext): void => {
const es = getService('es');
const supertest = getService('supertest');
const log = getService('log');
const retry = getService('retry');

/* This test makes use of the mock packages created in the '/fleet_bundled_packages' folder,
/* in order to assert that, in production environments, the latest stable version of the package
Expand All @@ -39,15 +40,16 @@ export default ({ getService }: FtrProviderContext): void => {

it('should install latest stable version and ignore prerelease packages', async () => {
// Verify that status is empty before package installation
const statusBeforePackageInstallation = await getPrebuiltRulesStatus(supertest);
const statusBeforePackageInstallation = await getPrebuiltRulesStatus(es, supertest);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_installed).toBe(0);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_install).toBe(0);
expect(statusBeforePackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);

// Install package without specifying version to check if latest stable version is installed
const fleetPackageInstallationResponse = await installPrebuiltRulesPackageViaFleetAPI(
es,
supertest
supertest,
retry
);

expect(fleetPackageInstallationResponse.items.length).toBe(1);
Expand All @@ -59,7 +61,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(prebuiltRulesFleetPackage.status).toBe(200);

// Get status of our prebuilt rules (nothing should be instaled yet)
const statusAfterPackageInstallation = await getPrebuiltRulesStatus(supertest);
const statusAfterPackageInstallation = await getPrebuiltRulesStatus(es, supertest);
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_installed).toBe(0);
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_to_install).toBe(1); // 1 rule in package 99.0.0
expect(statusAfterPackageInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);
Expand All @@ -68,7 +70,7 @@ export default ({ getService }: FtrProviderContext): void => {
await installPrebuiltRules(es, supertest);

// Verify that status is updated after package installation
const statusAfterRulesInstallation = await getPrebuiltRulesStatus(supertest);
const statusAfterRulesInstallation = await getPrebuiltRulesStatus(es, supertest);
expect(statusAfterRulesInstallation.stats.num_prebuilt_rules_installed).toBe(1); // 1 rule in package 99.0.0
expect(statusAfterRulesInstallation.stats.num_prebuilt_rules_to_install).toBe(0);
expect(statusAfterRulesInstallation.stats.num_prebuilt_rules_to_upgrade).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ export default ({ getService }: FtrProviderContext): void => {

it('should install a package containing 15000 prebuilt rules without crashing', async () => {
// Verify that status is empty before package installation
const statusBeforePackageInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
const statusBeforePackageInstallation = await getPrebuiltRulesAndTimelinesStatus(
es,
supertest
);
expect(statusBeforePackageInstallation.rules_installed).toBe(0);
expect(statusBeforePackageInstallation.rules_not_installed).toBe(0);
expect(statusBeforePackageInstallation.rules_not_updated).toBe(0);
Expand All @@ -40,7 +43,10 @@ export default ({ getService }: FtrProviderContext): void => {
await installPrebuiltRulesAndTimelines(es, supertest);

// Verify that status is updated after package installation
const statusAfterPackageInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
const statusAfterPackageInstallation = await getPrebuiltRulesAndTimelinesStatus(
es,
supertest
);
expect(statusAfterPackageInstallation.rules_installed).toBe(750);
expect(statusAfterPackageInstallation.rules_not_installed).toBe(0);
expect(statusAfterPackageInstallation.rules_not_updated).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export default ({ getService }: FtrProviderContext): void => {
const es = getService('es');
const supertest = getService('supertest');
const log = getService('log');
const retry = getService('retry');

describe('@ess @serverless @skipInQA install_prebuilt_rules_from_real_package', () => {
beforeEach(async () => {
Expand All @@ -33,10 +34,12 @@ export default ({ getService }: FtrProviderContext): void => {
* Unlike other tests that use mocks, this test uses actual rules from the
* package storage and checks that they are installed.
*/
// TODO: Fix and unskip https://github.com/elastic/kibana/issues/172107
it.skip('should install prebuilt rules from the package storage', async () => {
it('should install prebuilt rules from the package storage', async () => {
// Verify that status is empty before package installation
const statusBeforePackageInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
const statusBeforePackageInstallation = await getPrebuiltRulesAndTimelinesStatus(
es,
supertest
);
expect(statusBeforePackageInstallation.rules_installed).toBe(0);
expect(statusBeforePackageInstallation.rules_not_installed).toBe(0);
expect(statusBeforePackageInstallation.rules_not_updated).toBe(0);
Expand All @@ -45,10 +48,14 @@ export default ({ getService }: FtrProviderContext): void => {
es,
supertest,
overrideExistingPackage: true,
retryService: retry,
});

// Verify that status is updated after package installation
const statusAfterPackageInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
const statusAfterPackageInstallation = await getPrebuiltRulesAndTimelinesStatus(
es,
supertest
);
expect(statusAfterPackageInstallation.rules_installed).toBe(0);
expect(statusAfterPackageInstallation.rules_not_installed).toBeGreaterThan(0);
expect(statusAfterPackageInstallation.rules_not_updated).toBe(0);
Expand All @@ -59,7 +66,7 @@ export default ({ getService }: FtrProviderContext): void => {
expect(response.rules_updated).toBe(0);

// Verify that status is updated after rules installation
const statusAfterRuleInstallation = await getPrebuiltRulesAndTimelinesStatus(supertest);
const statusAfterRuleInstallation = await getPrebuiltRulesAndTimelinesStatus(es, supertest);
expect(statusAfterRuleInstallation.rules_installed).toBe(response.rules_installed);
expect(statusAfterRuleInstallation.rules_not_installed).toBe(0);
expect(statusAfterRuleInstallation.rules_not_updated).toBe(0);
Expand Down
Loading

0 comments on commit 544ba3b

Please sign in to comment.