Skip to content

Commit

Permalink
[Security Solution] Prevent rules table auto-refreshing on window foc…
Browse files Browse the repository at this point in the history
…us when auto-refresh disabled (elastic#165250)

**Fixes: elastic#165221

## Summary

This PR prevents rules management table from being auto-refreshed on window focus or on reconnect if auto-refresh is disabled.

*Before:*

Auto-refresh is switched off

https://github.com/elastic/kibana/assets/3775283/9536cc47-27b3-424a-a0a6-b3d32d40b9e2

Auto-refresh is switched off AND a rule is selected

https://github.com/elastic/kibana/assets/3775283/dbf9c899-85cc-465b-8f78-303ebee0cece

*After:*

Auto-refresh is switched off

https://github.com/elastic/kibana/assets/3775283/e7b98d15-5f33-44dc-b064-1014ec9382f9

Auto-refresh is switched off AND a rule is selected

https://github.com/elastic/kibana/assets/3775283/39a8fa9d-a2e8-451c-879b-5c08492518c9


## 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


## Flaky test runner

[rules_table_auto_refresh.cy.ts (150 runs)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3027) 🟢
  • Loading branch information
maximpn authored Sep 14, 2023
1 parent df99636 commit ca244ec
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ export const RulesTableContextProvider = ({ children }: RulesTableContextProvide
pagination,
},
{
// We don't need refreshes on windows focus and reconnects if auto-refresh if off
refetchOnWindowFocus: isRefreshOn && !isActionInProgress,
refetchOnReconnect: isRefreshOn && !isActionInProgress,
refetchInterval: isRefreshOn && !isActionInProgress && autoRefreshSettings.value,
keepPreviousData: true, // Use this option so that the state doesn't jump between "success" and "loading" on page change
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
expectNumberOfRules,
selectRulesByName,
getRuleRow,
setRulesTableAutoRefreshIntervalSetting,
} from '../../../../tasks/alerts_detection_rules';
import { login, visit, visitWithoutDateRange } from '../../../../tasks/login';

Expand All @@ -30,8 +31,7 @@ import { createRule } from '../../../../tasks/api_calls/rules';
import { cleanKibana } from '../../../../tasks/common';
import { getNewRule } from '../../../../objects/rule';

const DEFAULT_RULE_REFRESH_INTERVAL_VALUE = 60000;
const NUM_OF_TEST_RULES = 6;
const RULES_TABLE_REFRESH_INTERVAL_MS = 60000;

// TODO: https://github.com/elastic/kibana/issues/161540
describe(
Expand All @@ -42,82 +42,128 @@ describe(
cleanKibana();
login();

for (let i = 1; i <= NUM_OF_TEST_RULES; ++i) {
createRule(getNewRule({ name: `Test rule ${i}`, rule_id: `${i}`, enabled: false }));
}
setRulesTableAutoRefreshIntervalSetting({
enabled: true,
refreshInterval: RULES_TABLE_REFRESH_INTERVAL_MS,
});
createRule(getNewRule({ name: 'Test rule 1', rule_id: '1', enabled: false }));
});

beforeEach(() => {
login();
});

it('Auto refreshes rules', () => {
mockGlobalClock();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
it('gets deactivated when any rule selected and activated after rules unselected', () => {
visit(DETECTIONS_RULE_MANAGEMENT_URL);

expectNumberOfRules(RULES_MANAGEMENT_TABLE, 1);

// check refresh settings if it's enabled before selecting
expectAutoRefreshIsEnabled();

selectAllRules();

expectNumberOfRules(RULES_MANAGEMENT_TABLE, NUM_OF_TEST_RULES);
// auto refresh should be deactivated (which means disabled without an ability to enable it) after rules selected
expectAutoRefreshIsDeactivated();

// // mock 1 minute passing to make sure refresh is conducted
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('not.exist');
cy.tick(DEFAULT_RULE_REFRESH_INTERVAL_VALUE);
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('be.visible');
clearAllRuleSelection();

cy.contains(REFRESH_RULES_STATUS, 'Updated now');
// after all rules unselected, auto refresh should be reset to its previous state
expectAutoRefreshIsEnabled();
});

it('should prevent table from rules refetch if any rule selected', () => {
mockGlobalClock();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
describe('when enabled', () => {
beforeEach(() => {
mockGlobalClock();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);

expectNumberOfRules(RULES_MANAGEMENT_TABLE, NUM_OF_TEST_RULES);
expectNumberOfRules(RULES_MANAGEMENT_TABLE, 1);
});

selectRulesByName(['Test rule 1']);
it('refreshes rules after refresh interval has passed', () => {
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('not.exist');
cy.tick(RULES_TABLE_REFRESH_INTERVAL_MS);
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('be.visible');

// mock 1 minute passing to make sure refresh is not conducted
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('not.exist');
cy.tick(DEFAULT_RULE_REFRESH_INTERVAL_VALUE);
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('not.exist');
cy.contains(REFRESH_RULES_STATUS, 'Updated now');
});

// ensure rule is still selected
getRuleRow('Test rule 1').find(EUI_CHECKBOX).should('be.checked');
it('refreshes rules on window focus', () => {
cy.tick(RULES_TABLE_REFRESH_INTERVAL_MS / 2);

cy.get(REFRESH_RULES_STATUS).should('have.not.text', 'Updated now');
cy.window().trigger('blur');
cy.window().trigger('focus');

cy.contains(REFRESH_RULES_STATUS, 'Updated now');
});
});

it('should disable auto refresh when any rule selected and enable it after rules unselected', () => {
visit(DETECTIONS_RULE_MANAGEMENT_URL);
describe('when disabled', () => {
beforeEach(() => {
mockGlobalClock();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);
expectNumberOfRules(RULES_MANAGEMENT_TABLE, 1);
});

expectNumberOfRules(RULES_MANAGEMENT_TABLE, NUM_OF_TEST_RULES);
it('does NOT refresh rules after refresh interval has passed', () => {
disableAutoRefresh();
cy.tick(RULES_TABLE_REFRESH_INTERVAL_MS * 2); // Make sure enough time has passed to verify auto-refresh doesn't happen

// check refresh settings if it's enabled before selecting
expectAutoRefreshIsEnabled();
cy.contains(REFRESH_RULES_STATUS, 'Updated 2 minutes ago');
});

selectAllRules();
it('does NOT refresh rules on window focus', () => {
disableAutoRefresh();
cy.tick(RULES_TABLE_REFRESH_INTERVAL_MS * 2); // Make sure enough time has passed to verify auto-refresh doesn't happen

// auto refresh should be deactivated (which means disabled without an ability to enable it) after rules selected
expectAutoRefreshIsDeactivated();
cy.window().trigger('blur');
cy.window().trigger('focus');

clearAllRuleSelection();
// We need to make sure window focus event doesn't cause refetching. Without some delay
// the following expectations always pass even. It happens since 'focus' event gets handled
// in an async way so the status text is updated with some delay.
// eslint-disable-next-line cypress/no-unnecessary-waiting
cy.wait(1000);

// after all rules unselected, auto refresh should be reset to its previous state
expectAutoRefreshIsEnabled();
});
// By using a custom timeout make sure it doesn't wait too long due to global timeout configuration
// so the expected text appears after a refresh and the test passes while it shouldn't.
cy.contains(REFRESH_RULES_STATUS, 'Updated 2 minutes ago', { timeout: 10000 });
});

it('should not enable auto refresh after rules were unselected if auto refresh was disabled', () => {
visit(DETECTIONS_RULE_MANAGEMENT_URL);
it('does NOT get enabled after rules were unselected', () => {
disableAutoRefresh();
cy.tick(RULES_TABLE_REFRESH_INTERVAL_MS * 2); // Make sure enough time has passed to verify auto-refresh doesn't happen

expectNumberOfRules(RULES_MANAGEMENT_TABLE, NUM_OF_TEST_RULES);
selectAllRules();

disableAutoRefresh();
expectAutoRefreshIsDeactivated();

selectAllRules();
clearAllRuleSelection();

expectAutoRefreshIsDeactivated();
// after all rules unselected, auto refresh should still be disabled
expectAutoRefreshIsDisabled();
});
});

clearAllRuleSelection();
describe('when one rule is selected', () => {
it('does NOT refresh after refresh interval has passed', () => {
mockGlobalClock();
visitWithoutDateRange(DETECTIONS_RULE_MANAGEMENT_URL);

expectNumberOfRules(RULES_MANAGEMENT_TABLE, 1);

selectRulesByName(['Test rule 1']);

// mock 1 minute passing to make sure refresh is not conducted
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('not.exist');
cy.tick(RULES_TABLE_REFRESH_INTERVAL_MS * 2); // Make sure enough time has passed
cy.get(RULES_TABLE_AUTOREFRESH_INDICATOR).should('not.exist');

// ensure rule is still selected
getRuleRow('Test rule 1').find(EUI_CHECKBOX).should('be.checked');

// after all rules unselected, auto refresh should still be disabled
expectAutoRefreshIsDisabled();
cy.get(REFRESH_RULES_STATUS).should('have.not.text', 'Updated now');
});
});
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { DEFAULT_RULES_TABLE_REFRESH_SETTING } from '@kbn/security-solution-plugin/common/constants';
import {
COLLAPSED_ACTION_BTN,
CUSTOM_RULES_BTN,
Expand Down Expand Up @@ -64,6 +65,7 @@ import { PAGE_CONTENT_SPINNER } from '../screens/common/page';

import { goToRuleEditSettings } from './rule_details';
import { goToActionsStepTab } from './create_new_rule';
import { setKibanaSetting } from './api_calls/kibana_advanced_settings';

export const getRulesManagementTableRows = () => cy.get(RULES_MANAGEMENT_TABLE).find(RULES_ROW);

Expand Down Expand Up @@ -516,3 +518,29 @@ const unselectRuleByName = (ruleName: string) => {
cy.log(`Make sure rule "${ruleName}" has been unselected`);
getRuleRow(ruleName).find(EUI_CHECKBOX).should('not.be.checked');
};

/**
* Set Kibana `securitySolution:rulesTableRefresh` setting looking like
*
* ```
* { "on": true, "value": 60000 }
* ```
*
* @param enabled whether the auto-refresh is enabled
* @param refreshInterval refresh interval in milliseconds
*/
export const setRulesTableAutoRefreshIntervalSetting = ({
enabled,
refreshInterval,
}: {
enabled: boolean;
refreshInterval: number; // milliseconds
}) => {
setKibanaSetting(
DEFAULT_RULES_TABLE_REFRESH_SETTING,
JSON.stringify({
on: enabled,
value: refreshInterval,
})
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,27 @@
* 2.0.
*/

import { SECURITY_SOLUTION_SHOW_RELATED_INTEGRATIONS_ID } from '@kbn/management-settings-ids';
import { ENABLE_EXPANDABLE_FLYOUT_SETTING } from '@kbn/security-solution-plugin/common/constants';
import { rootRequest } from '../common';

const kibanaSettings = (body: Cypress.RequestBody) => {
export const setKibanaSetting = (key: string, value: boolean | number | string) => {
rootRequest({
method: 'POST',
url: 'internal/kibana/settings',
body,
body: { changes: { [key]: value } },
headers: { 'kbn-xsrf': 'cypress-creds', 'x-elastic-internal-origin': 'security-solution' },
});
};

const relatedIntegrationsBody = (status: boolean): Cypress.RequestBody => {
return { changes: { 'securitySolution:showRelatedIntegrations': status } };
};

export const enableRelatedIntegrations = () => {
kibanaSettings(relatedIntegrationsBody(true));
setKibanaSetting(SECURITY_SOLUTION_SHOW_RELATED_INTEGRATIONS_ID, true);
};

export const disableRelatedIntegrations = () => {
kibanaSettings(relatedIntegrationsBody(false));
setKibanaSetting(SECURITY_SOLUTION_SHOW_RELATED_INTEGRATIONS_ID, false);
};

export const disableExpandableFlyout = () => {
const body = { changes: { 'securitySolution:enableExpandableFlyout': false } };
kibanaSettings(body);
setKibanaSetting(ENABLE_EXPANDABLE_FLYOUT_SETTING, false);
};
3 changes: 2 additions & 1 deletion x-pack/test/security_solution_cypress/cypress/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@kbn/config-schema",
"@kbn/lists-plugin",
"@kbn/securitysolution-list-constants",
"@kbn/security-plugin"
"@kbn/security-plugin",
"@kbn/management-settings-ids"
]
}

0 comments on commit ca244ec

Please sign in to comment.