Skip to content

Commit

Permalink
[Security Solution] Fix flaky test: `detection_rules/bulk_edit_rules_…
Browse files Browse the repository at this point in the history
…actions.cy.ts` (elastic#163698)

Fixes: elastic#154721

## Summary

- Fixes flaky test:
`x-pack/plugins/security_solution/cypress/e2e/detection_response/rule_management/rule_actions/bulk_actions/bulk_edit_rules_actions.cy.ts`
- Test title: `Detection rules, bulk edit of rule actions`


## Details

For: `Detection rules, bulk edit of rule actions - Restricted action
privileges - User with no privileges can't add rule actions`
- Since this test logs in with a user with missing privileges, the
"Missing privileges" callout is shown at the top of the Rules Table. The
`selectNumberOfRules();` command selects rules one by one by clicking on
their checkboxes. However, flakiness was caused when the callout was
rendered while the selection of the rules was happening, causing a
layout shift that caused the selection of a checkbox to lose focus, and
not being able to be checked. This was solved by waiting the callout to
be rendered before the selection of rules start, with the new
`waitForCallOutToBeShown`method.

For: `Detection rules, bulk edit of rule actions - All actions
privileges - before/beforeEach Clause`
- Tests were failing in the `beforeEach` clause because the first test,
mentioned above, would be logged in with a `ROLES.hunter_no_actions`
role, and logging in with a user with permissions happened in a `before`
clause instead of a `beforeEach` clause. This caused the rest of the
suite to continue with a role without permissions, and the setup of the
second test would fail as the API requests done would fail with `401`.
Moving the initial logging-in from a `before` clause to a `beforeEach`
clause solved this issue.

For: `Detection rules, bulk edit of rule actions - All actions
privileges - Add a rule action to rules (existing connector)`
- This flakiness was extremely rare, but could be reproduced after about
400 iterations. It was caused by a similar reason as the first case
above: while rules were being selected one by one, the table would auto
refresh and focus would be lost from the checkbox that was about to be
selected. This aws fixed by disabling autorefresh in the setup.

### Other changes
- Prevents the installation of `security_detection_engine` package and
creates mock rules instead.
- Creates the `waitForCallOutToBeShown` method and moves the callout IDs
spread across different files to a a single file where they are exported
from.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jpdjere and kibanamachine authored Aug 16, 2023
1 parent 736b16d commit d725c28
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ import { PAGE_TITLE } from '../../screens/common/page';
import { login, visitWithoutDateRange, waitForPageWithoutDateRange } from '../../tasks/login';
import { goToRuleDetails } from '../../tasks/alerts_detection_rules';
import { createRule, deleteCustomRule } from '../../tasks/api_calls/rules';
import { getCallOut, waitForCallOutToBeShown } from '../../tasks/common/callouts';
import {
getCallOut,
NEED_ADMIN_FOR_UPDATE_CALLOUT,
waitForCallOutToBeShown,
} from '../../tasks/common/callouts';

const loadPageAsPlatformEngineerUser = (url: string) => {
login(ROLES.soc_manager);
Expand All @@ -31,8 +35,6 @@ describe(
'Detections > Need Admin Callouts indicating an admin is needed to migrate the alert data set',
{ tags: tag.ESS },
() => {
const NEED_ADMIN_FOR_UPDATE_CALLOUT = 'need-admin-for-update-rules';

before(() => {
// First, we have to open the app on behalf of a privileged user in order to initialize it.
// Otherwise the app will be disabled and show a "welcome"-like page.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ import { PAGE_TITLE } from '../../screens/common/page';
import { login, visitWithoutDateRange, waitForPageWithoutDateRange } from '../../tasks/login';
import { goToRuleDetails } from '../../tasks/alerts_detection_rules';
import { createRule, deleteCustomRule } from '../../tasks/api_calls/rules';
import { getCallOut, waitForCallOutToBeShown, dismissCallOut } from '../../tasks/common/callouts';
import {
getCallOut,
waitForCallOutToBeShown,
dismissCallOut,
MISSING_PRIVILEGES_CALLOUT,
} from '../../tasks/common/callouts';

const loadPageAsReadOnlyUser = (url: string) => {
login(ROLES.reader);
Expand All @@ -39,8 +44,6 @@ const waitForPageTitleToBeShown = () => {
};

describe('Detections > Callouts', { tags: tag.ESS }, () => {
const MISSING_PRIVILEGES_CALLOUT = 'missing-user-privileges';

before(() => {
// First, we have to open the app on behalf of a privileged user in order to initialize it.
// Otherwise the app will be disabled and show a "welcome"-like page.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,11 @@ import {
dismissCallOut,
getCallOut,
waitForCallOutToBeShown,
MISSING_PRIVILEGES_CALLOUT,
} from '../../../../tasks/common/callouts';
import { login, visitWithoutDateRange } from '../../../../tasks/login';
import { SECURITY_DETECTIONS_RULES_URL } from '../../../../urls/navigation';

const MISSING_PRIVILEGES_CALLOUT = 'missing-user-privileges';

describe('All rules - read only', { tags: tag.ESS }, () => {
before(() => {
cleanKibana();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@

import type { RuleActionArray } from '@kbn/securitysolution-io-ts-alerting-types';
import { ROLES } from '@kbn/security-solution-plugin/common/test';
import {
MISSING_PRIVILEGES_CALLOUT,
waitForCallOutToBeShown,
} from '../../../../../tasks/common/callouts';
import { createRuleAssetSavedObject } from '../../../../../helpers/rules';
import { tag } from '../../../../../tags';

import {
Expand Down Expand Up @@ -34,6 +39,7 @@ import {
waitForRulesTableToBeLoaded,
selectNumberOfRules,
goToEditRuleActionsSettingsOf,
disableAutoRefresh,
} from '../../../../../tasks/alerts_detection_rules';
import {
waitForBulkEditActionToFinish,
Expand All @@ -57,28 +63,28 @@ import {
getMachineLearningRule,
getNewTermsRule,
} from '../../../../../objects/rule';
import { excessivelyInstallAllPrebuiltRules } from '../../../../../tasks/api_calls/prebuilt_rules';
import {
createAndInstallMockedPrebuiltRules,
excessivelyInstallAllPrebuiltRules,
preventPrebuiltRulesPackageInstallation,
} from '../../../../../tasks/api_calls/prebuilt_rules';

const ruleNameToAssert = 'Custom rule name with actions';
const expectedNumberOfCustomRulesToBeEdited = 7;
// 7 custom rules of different types + 3 prebuilt.
// 7 custom rules of different types + 2 prebuilt.
// number of selected rules doesn't matter, we only want to make sure they will be edited an no modal window displayed as for other actions
const expectedNumberOfRulesToBeEdited = expectedNumberOfCustomRulesToBeEdited + 3;
const expectedNumberOfRulesToBeEdited = expectedNumberOfCustomRulesToBeEdited + 2;

const expectedExistingSlackMessage = 'Existing slack action';
const expectedSlackMessage = 'Slack action test message';

// TODO: Fix flakiness and unskip https://github.com/elastic/kibana/issues/154721
describe.skip(
describe(
'Detection rules, bulk edit of rule actions',
{ tags: [tag.ESS, tag.BROKEN_IN_SERVERLESS] },
() => {
before(() => {
beforeEach(() => {
cleanKibana();
login();
});

beforeEach(() => {
deleteAlertsAndRules();
deleteConnectors();
cy.task('esArchiverResetKibana');
Expand Down Expand Up @@ -111,12 +117,27 @@ describe.skip(
createRule(getNewRule({ saved_id: 'mocked', rule_id: '7' }));

createSlackConnector();

// Prevent prebuilt rules package installation and mock two prebuilt rules
preventPrebuiltRulesPackageInstallation();

const RULE_1 = createRuleAssetSavedObject({
name: 'Test rule 1',
rule_id: 'rule_1',
});
const RULE_2 = createRuleAssetSavedObject({
name: 'Test rule 2',
rule_id: 'rule_2',
});

createAndInstallMockedPrebuiltRules({ rules: [RULE_1, RULE_2] });
});

context('Restricted action privileges', () => {
it("User with no privileges can't add rule actions", () => {
login(ROLES.hunter_no_actions);
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL, ROLES.hunter_no_actions);
waitForCallOutToBeShown(MISSING_PRIVILEGES_CALLOUT, 'primary');
waitForRulesTableToBeLoaded();

selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited);
Expand All @@ -129,8 +150,10 @@ describe.skip(

context('All actions privileges', () => {
beforeEach(() => {
login();
visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL);
waitForRulesTableToBeLoaded();
disableAutoRefresh();
});

it('Add a rule action to rules (existing connector)', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ import {
dismissCallOut,
getCallOut,
waitForCallOutToBeShown,
MISSING_PRIVILEGES_CALLOUT,
} from '../../../../tasks/common/callouts';
import { login, visitWithoutDateRange } from '../../../../tasks/login';
import { EXCEPTIONS_URL } from '../../../../urls/navigation';

const MISSING_PRIVILEGES_CALLOUT = 'missing-user-privileges';

describe('Shared exception lists - read only', { tags: tag.ESS }, () => {
before(() => {
cy.task('esArchiverResetKibana');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

import { callOutWithId, CALLOUT_DISMISS_BTN } from '../../screens/common/callouts';

export const NEED_ADMIN_FOR_UPDATE_CALLOUT = 'need-admin-for-update-rules';
export const MISSING_PRIVILEGES_CALLOUT = 'missing-user-privileges';

export const getCallOut = (id: string, options?: Cypress.Timeoutable) => {
return cy.get(callOutWithId(id), options);
};
Expand Down

0 comments on commit d725c28

Please sign in to comment.