Skip to content

Commit

Permalink
Fix performance issues affecting rules management (#135311)
Browse files Browse the repository at this point in the history
  • Loading branch information
xcrzx authored Jul 11, 2022
1 parent d8553f5 commit c7a5b13
Show file tree
Hide file tree
Showing 40 changed files with 735 additions and 970 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/security_solution/cypress/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
videos
screenshots
downloads

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export const goToTheRuleDetailsOf = (ruleName: string) => {

export const loadPrebuiltDetectionRules = () => {
cy.get(LOAD_PREBUILT_RULES_BTN)
.should('exist')
.should('be.enabled')
.pipe(($el) => $el.trigger('click'))
.should('be.disabled');
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,31 @@
* 2.0.
*/

export const SECURITY_ACTIONS_PREFIX = 'securitySolution';
import { APP_UI_ID } from '../../../../common/constants';

export const SINGLE_RULE_ACTIONS = {
ENABLE: `${SECURITY_ACTIONS_PREFIX} singleRuleActions enable`,
DISABLE: `${SECURITY_ACTIONS_PREFIX} singleRuleActions disable`,
DUPLICATE: `${SECURITY_ACTIONS_PREFIX} singleRuleActions duplicate`,
EXPORT: `${SECURITY_ACTIONS_PREFIX} singleRuleActions export`,
DELETE: `${SECURITY_ACTIONS_PREFIX} singleRuleActions delete`,
PREVIEW: `${SECURITY_ACTIONS_PREFIX} singleRuleActions preview`,
SAVE: `${SECURITY_ACTIONS_PREFIX} singleRuleActions save`,
ENABLE: `${APP_UI_ID} singleRuleActions enable`,
DISABLE: `${APP_UI_ID} singleRuleActions disable`,
DUPLICATE: `${APP_UI_ID} singleRuleActions duplicate`,
EXPORT: `${APP_UI_ID} singleRuleActions export`,
DELETE: `${APP_UI_ID} singleRuleActions delete`,
PREVIEW: `${APP_UI_ID} singleRuleActions preview`,
SAVE: `${APP_UI_ID} singleRuleActions save`,
};

export const BULK_RULE_ACTIONS = {
ENABLE: `${SECURITY_ACTIONS_PREFIX} bulkRuleActions enable`,
DISABLE: `${SECURITY_ACTIONS_PREFIX} bulkRuleActions disable`,
DUPLICATE: `${SECURITY_ACTIONS_PREFIX} bulkRuleActions duplicate`,
EXPORT: `${SECURITY_ACTIONS_PREFIX} bulkRuleActions export`,
DELETE: `${SECURITY_ACTIONS_PREFIX} bulkRuleActions delete`,
EDIT: `${SECURITY_ACTIONS_PREFIX} bulkRuleActions edit`,
ENABLE: `${APP_UI_ID} bulkRuleActions enable`,
DISABLE: `${APP_UI_ID} bulkRuleActions disable`,
DUPLICATE: `${APP_UI_ID} bulkRuleActions duplicate`,
EXPORT: `${APP_UI_ID} bulkRuleActions export`,
DELETE: `${APP_UI_ID} bulkRuleActions delete`,
EDIT: `${APP_UI_ID} bulkRuleActions edit`,
};

export const RULES_TABLE_ACTIONS = {
REFRESH: `${SECURITY_ACTIONS_PREFIX} rulesTable refresh`,
FILTER: `${SECURITY_ACTIONS_PREFIX} rulesTable filter`,
LOAD_PREBUILT: `${SECURITY_ACTIONS_PREFIX} rulesTable loadPrebuilt`,
PREVIEW_ON: `${SECURITY_ACTIONS_PREFIX} rulesTable technicalPreview on`,
PREVIEW_OFF: `${SECURITY_ACTIONS_PREFIX} rulesTable technicalPreview off`,
REFRESH: `${APP_UI_ID} rulesTable refresh`,
FILTER: `${APP_UI_ID} rulesTable filter`,
LOAD_PREBUILT: `${APP_UI_ID} rulesTable loadPrebuilt`,
PREVIEW_ON: `${APP_UI_ID} rulesTable technicalPreview on`,
PREVIEW_OFF: `${APP_UI_ID} rulesTable technicalPreview off`,
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@
* 2.0.
*/

import React from 'react';
import { waitFor } from '@testing-library/react';
import type { ReactWrapper } from 'enzyme';
import { shallow, mount } from 'enzyme';

import { mount } from 'enzyme';
import React from 'react';
import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
import { useAppToastsMock } from '../../../../common/hooks/use_app_toasts.mock';
import { TestProviders } from '../../../../common/mock';
import '../../../../common/mock/match_media';
import { PrePackagedRulesPrompt } from './load_empty_prompt';
import { getPrePackagedRulesStatus } from '../../../containers/detection_engine/rules/api';
import { useAppToastsMock } from '../../../../common/hooks/use_app_toasts.mock';
import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
import { PrePackagedRulesPrompt } from './load_empty_prompt';

jest.mock('react-router-dom', () => {
const original = jest.requireActual('react-router-dom');
Expand Down Expand Up @@ -76,7 +76,9 @@ describe('PrePackagedRulesPrompt', () => {
});

it('renders correctly', () => {
const wrapper = shallow(<PrePackagedRulesPrompt {...props} />);
const wrapper = mount(<PrePackagedRulesPrompt {...props} />, {
wrappingComponent: TestProviders,
});

expect(wrapper.find('EmptyPrompt')).toHaveLength(1);
});
Expand All @@ -93,7 +95,9 @@ describe('LoadPrebuiltRulesAndTemplatesButton', () => {
timelines_not_updated: 0,
});

const wrapper: ReactWrapper = mount(<PrePackagedRulesPrompt {...props} />);
const wrapper: ReactWrapper = mount(<PrePackagedRulesPrompt {...props} />, {
wrappingComponent: TestProviders,
});
await waitFor(() => {
wrapper.update();

Expand All @@ -114,7 +118,9 @@ describe('LoadPrebuiltRulesAndTemplatesButton', () => {
timelines_not_updated: 0,
});

const wrapper: ReactWrapper = mount(<PrePackagedRulesPrompt {...props} />);
const wrapper: ReactWrapper = mount(<PrePackagedRulesPrompt {...props} />, {
wrappingComponent: TestProviders,
});
await waitFor(() => {
wrapper.update();

Expand All @@ -135,7 +141,9 @@ describe('LoadPrebuiltRulesAndTemplatesButton', () => {
timelines_not_updated: 0,
});

const wrapper: ReactWrapper = mount(<PrePackagedRulesPrompt {...props} />);
const wrapper: ReactWrapper = mount(<PrePackagedRulesPrompt {...props} />, {
wrappingComponent: TestProviders,
});
await waitFor(() => {
wrapper.update();

Expand All @@ -157,7 +165,10 @@ describe('LoadPrebuiltRulesAndTemplatesButton', () => {
});

const wrapper: ReactWrapper = mount(
<PrePackagedRulesPrompt {...{ ...props, loading: true }} />
<PrePackagedRulesPrompt {...{ ...props, loading: true }} />,
{
wrappingComponent: TestProviders,
}
);
await waitFor(() => {
wrapper.update();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { fetchInstalledIntegrations } from '../../../containers/detection_engine
// import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
// import * as i18n from './translations';

const ONE_MINUTE = 60000;

export interface UseInstalledIntegrationsArgs {
packages?: string[];
}
Expand All @@ -34,6 +36,7 @@ export const useInstalledIntegrations = ({ packages }: UseInstalledIntegrationsA
},
{
keepPreviousData: true,
staleTime: ONE_MINUTE * 5,
onError: (e) => {
// Suppressing for now to prevent excessive errors when fleet isn't configured
// addError(e, { title: i18n.INTEGRATIONS_FETCH_FAILURE });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,14 +434,13 @@ describe('Detections Rules API', () => {
});

test('check parameter url when creating pre-packaged rules', async () => {
await createPrepackagedRules({ signal: abortCtrl.signal });
await createPrepackagedRules();
expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules/prepackaged', {
signal: abortCtrl.signal,
method: 'PUT',
});
});
test('happy path', async () => {
const resp = await createPrepackagedRules({ signal: abortCtrl.signal });
const resp = await createPrepackagedRules();
expect(resp).toEqual({
rules_installed: 0,
rules_updated: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import type {
FetchRulesResponse,
Rule,
FetchRuleProps,
BasicFetchProps,
ImportDataProps,
ExportDocumentsProps,
ImportDataResponse,
Expand Down Expand Up @@ -237,9 +236,7 @@ export const performBulkAction = async <Action extends BulkAction>({
*
* @throws An error if response is not OK
*/
export const createPrepackagedRules = async ({
signal,
}: BasicFetchProps): Promise<{
export const createPrepackagedRules = async (): Promise<{
rules_installed: number;
rules_updated: number;
timelines_installed: number;
Expand All @@ -252,7 +249,6 @@ export const createPrepackagedRules = async ({
timelines_updated: number;
}>(DETECTION_ENGINE_PREPACKAGED_URL, {
method: 'PUT',
signal,
});

return result;
Expand Down Expand Up @@ -401,7 +397,7 @@ export const fetchTags = async ({ signal }: { signal: AbortSignal }): Promise<st
export const getPrePackagedRulesStatus = async ({
signal,
}: {
signal: AbortSignal;
signal: AbortSignal | undefined;
}): Promise<PrePackagedRulesStatusResponse> =>
KibanaServices.get().http.fetch<PrePackagedRulesStatusResponse>(
DETECTION_ENGINE_PREPACKAGED_RULES_STATUS_URL,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { createRule } from './api';
import * as i18n from './translations';
import { transformOutput } from './transforms';
import { useInvalidateRules } from './use_find_rules_query';
import { useInvalidatePrePackagedRulesStatus } from './use_pre_packaged_rules_status';

interface CreateRuleReturn {
isLoading: boolean;
Expand All @@ -29,6 +30,7 @@ export const useCreateRule = (): ReturnCreateRule => {
const [isLoading, setIsLoading] = useState(false);
const { addError } = useAppToasts();
const invalidateRules = useInvalidateRules();
const invalidatePrePackagedRulesStatus = useInvalidatePrePackagedRulesStatus();

useEffect(() => {
let isSubscribed = true;
Expand All @@ -43,6 +45,7 @@ export const useCreateRule = (): ReturnCreateRule => {
signal: abortCtrl.signal,
});
invalidateRules();
invalidatePrePackagedRulesStatus();
if (isSubscribed) {
setRuleId(createRuleResponse.id);
}
Expand All @@ -62,7 +65,7 @@ export const useCreateRule = (): ReturnCreateRule => {
isSubscribed = false;
abortCtrl.abort();
};
}, [rule, addError, invalidateRules]);
}, [rule, addError, invalidateRules, invalidatePrePackagedRulesStatus]);

return [{ isLoading, ruleId }, setRule];
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 { useMutation } from 'react-query';
import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
import { createPrepackagedRules } from './api';
import * as i18n from './translations';
import { useInvalidateRules } from './use_find_rules_query';
import { useInvalidatePrePackagedRulesStatus } from './use_pre_packaged_rules_status';

export const useInstallPrePackagedRules = () => {
const { addError, addSuccess } = useAppToasts();
const invalidateRules = useInvalidateRules();
const invalidatePrePackagedRulesStatus = useInvalidatePrePackagedRulesStatus();

return useMutation(() => createPrepackagedRules(), {
onError: (err) => {
addError(err, { title: i18n.RULE_AND_TIMELINE_PREPACKAGED_FAILURE });
},
onSuccess: (result) => {
addSuccess(getSuccessToastMessage(result));
// Always invalidate all rules and the prepackaged rules status cache as
// the number of rules might change after the installation
invalidatePrePackagedRulesStatus();
invalidateRules();
},
});
};

const getSuccessToastMessage = (result: {
rules_installed: number;
rules_updated: number;
timelines_installed: number;
timelines_updated: number;
}) => {
const {
rules_installed: rulesInstalled,
rules_updated: rulesUpdated,
timelines_installed: timelinesInstalled,
timelines_updated: timelinesUpdated,
} = result;
if (rulesInstalled === 0 && (timelinesInstalled > 0 || timelinesUpdated > 0)) {
return i18n.TIMELINE_PREPACKAGED_SUCCESS;
} else if ((rulesInstalled > 0 || rulesUpdated > 0) && timelinesInstalled === 0) {
return i18n.RULE_PREPACKAGED_SUCCESS;
} else {
return i18n.RULE_AND_TIMELINE_PREPACKAGED_SUCCESS;
}
};
Loading

0 comments on commit c7a5b13

Please sign in to comment.