Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Unskip flaky tests in Prebuilt Rules FTR Integration tests #173998

Merged
merged 22 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,17 @@ export class RetryService extends FtrService {
});
}

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 @@ -27,6 +27,7 @@ export default ({ getService }: FtrProviderContext): void => {
const supertest = getService('supertest');
const log = getService('log');
const es = getService('es');
const retry = getService('retry');

describe('coverage_overview', () => {
beforeEach(async () => {
Expand Down Expand Up @@ -438,7 +439,7 @@ export default ({ getService }: FtrProviderContext): void => {
threat: generateThreatArray(1),
}),
]);
await installPrebuiltRulesAndTimelines(es, supertest);
await installPrebuiltRulesAndTimelines(es, supertest, retry);

const expectedRule = await createRule(supertest, log, {
...getSimpleRule('rule-1'),
Expand Down
26 changes: 26 additions & 0 deletions x-pack/test/detection_engine_api_integration/utils/es_indices.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 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 type { Client } from '@elastic/elasticsearch';

export const refreshSavedObjectIndices = async (es: Client) => {
// In cases such as when installing the prebuilt detection rules SO of type 'security-rule',
// The savedObjectsClient does this with a call with explicit `refresh: false`.
// So, despite of the fact that the endpoint waits until the prebuilt rule will be
// successfully indexed, it doesn't wait until they become "visible" for subsequent read
// operations.
// And this is usually what we do next in integration tests: we read these SOs with utility
// function such as getPrebuiltRulesAndTimelinesStatus().
// This can cause 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 });

// Additionally, we need to clear the cache to ensure that the next read operation will
// not return stale data.
await es.indices.clearCache({ index: ALL_SAVED_OBJECT_INDICES });
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export * from './create_signals_index';
export * from './delete_all_rules';
export * from './delete_all_alerts';
export * from './delete_all_timelines';
export * from './es_indices';
export * from './get_complex_rule';
export * from './get_complex_rule_output';
export * from './get_simple_rule';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from '@kbn/security-solution-plugin/common/api/detection_engine/prebuilt_rules';
import type { Client } from '@elastic/elasticsearch';
import type SuperTest from 'supertest';
import { ALL_SAVED_OBJECT_INDICES } from '@kbn/core-saved-objects-server';
import { refreshSavedObjectIndices } from '../es_indices';

/**
* (LEGACY)
Expand Down Expand Up @@ -40,17 +40,7 @@ export const installPrebuiltRulesAndTimelines = async (
.send()
.expect(200);

// Before we proceed, we need to refresh saved object indices.
// At the previous step we installed the prebuilt detection rules SO of type 'security-rule'.
// The savedObjectsClient does this with a call with explicit `refresh: false`.
// So, despite of the fact that the endpoint waits until the prebuilt rule will be
// successfully indexed, it doesn't wait until they become "visible" for subsequent read
// operations.
// And this is usually what we do next in integration tests: we read these SOs with utility
// function such as getPrebuiltRulesAndTimelinesStatus().
// This can cause 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 });
await refreshSavedObjectIndices(es);

return response.body;
};
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,15 @@ 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 +53,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 +73,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