From aaac0110bdae05a780addb52af9507035e5b98b6 Mon Sep 17 00:00:00 2001 From: Juan Pablo Djeredjian Date: Tue, 20 Dec 2022 12:28:01 +0100 Subject: [PATCH] [Security Solution] Add skipped rules to Bulk Edit rules API response (#147345) **Addresses:** https://github.com/elastic/kibana/issues/145093 **Related to:** https://github.com/elastic/kibana/issues/139802 ## Summary - Extends Bulk Edit API to return a new `skipped` property for rules whose updating was skipped. See [#145093](https://github.com/elastic/kibana/issues/145093) for details on when a rule is skipped. - In `x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts`, refactored the methods `bulkEdit` and `bulkEditOcc` to smaller methods, following an immutable approach. - Updated all related tests and expanded coverage. (unit, integration and e2e) - Update success toast message so that the user is informed if rules were skipped. https://user-images.githubusercontent.com/5354282/199806913-eb70e7a6-0435-486a-96f1-dd0e8abaffe2.mp4 ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - https://github.com/elastic/security-docs/issues/2684 - [ ] [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 - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### 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: Xavier Mouligneau --- x-pack/plugins/alerting/common/bulk_edit.ts | 16 + x-pack/plugins/alerting/common/index.ts | 1 + .../server/routes/bulk_edit_rules.test.ts | 3 +- .../common/apply_bulk_edit_operation.test.ts | 316 +++++---- .../common/apply_bulk_edit_operation.ts | 9 +- .../retry_if_bulk_edit_conflicts.test.ts | 39 +- .../common/retry_if_bulk_edit_conflicts.ts | 19 +- .../server/rules_client/methods/bulk_edit.ts | 665 ++++++++++++------ .../rules_client/tests/bulk_edit.test.ts | 586 ++++++++++++++- .../api/rules/bulk_actions/request_schema.ts | 4 +- .../api/rules/bulk_actions/response_schema.ts | 53 ++ .../e2e/detection_rules/bulk_edit_rules.cy.ts | 49 +- .../bulk_edit_rules_actions.cy.ts | 6 +- .../bulk_edit_rules_data_view.cy.ts | 115 ++- .../cypress/tasks/rules_bulk_edit.ts | 37 +- .../rule_management/api/api.ts | 2 + .../logic/bulk_actions/translations.ts | 13 +- .../components/rules_table/helpers.ts | 1 + .../detection_engine/rules/translations.ts | 24 +- .../routes/__mocks__/test_adapters.ts | 5 +- .../api/rules/bulk_actions/route.test.ts | 138 +++- .../api/rules/bulk_actions/route.ts | 84 ++- .../logic/bulk_actions/bulk_edit_rules.ts | 1 - .../bulk_actions/rule_params_modifier.test.ts | 111 ++- .../bulk_actions/rule_params_modifier.ts | 111 ++- .../translations/translations/fr-FR.json | 2 - .../translations/translations/ja-JP.json | 2 - .../translations/translations/zh-CN.json | 2 - .../group3/tests/alerting/bulk_edit.ts | 7 +- .../group10/perform_bulk_action.ts | 489 ++++++++++--- .../group10/perform_bulk_action_dry_run.ts | 70 +- 31 files changed, 2313 insertions(+), 667 deletions(-) create mode 100644 x-pack/plugins/alerting/common/bulk_edit.ts create mode 100644 x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/response_schema.ts diff --git a/x-pack/plugins/alerting/common/bulk_edit.ts b/x-pack/plugins/alerting/common/bulk_edit.ts new file mode 100644 index 0000000000000..73773032f5280 --- /dev/null +++ b/x-pack/plugins/alerting/common/bulk_edit.ts @@ -0,0 +1,16 @@ +/* + * 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 { Rule } from './rule'; + +export type BulkEditSkipReason = 'RULE_NOT_MODIFIED'; + +export interface BulkActionSkipResult { + id: Rule['id']; + name?: Rule['name']; + skip_reason: BulkEditSkipReason; +} diff --git a/x-pack/plugins/alerting/common/index.ts b/x-pack/plugins/alerting/common/index.ts index eeb3db0be0066..795a05dcb802c 100644 --- a/x-pack/plugins/alerting/common/index.ts +++ b/x-pack/plugins/alerting/common/index.ts @@ -17,6 +17,7 @@ export * from './rule_navigation'; export * from './alert_instance'; export * from './alert_summary'; export * from './builtin_action_groups'; +export * from './bulk_edit'; export * from './disabled_action_groups'; export * from './rule_notify_when_type'; export * from './parse_duration'; diff --git a/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts b/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts index b70e4734ab4ff..0ff709c252f56 100644 --- a/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts +++ b/x-pack/plugins/alerting/server/routes/bulk_edit_rules.test.ts @@ -71,7 +71,7 @@ describe('bulkEditInternalRulesRoute', () => { }, ], }; - const bulkEditResult = { rules: mockedAlerts, errors: [], total: 1 }; + const bulkEditResult = { rules: mockedAlerts, errors: [], total: 1, skipped: [] }; it('bulk edits rules with tags action', async () => { const licenseState = licenseStateMock.create(); @@ -97,6 +97,7 @@ describe('bulkEditInternalRulesRoute', () => { body: { total: 1, errors: [], + skipped: [], rules: [ expect.objectContaining({ id: '1', diff --git a/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts b/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts index 03d688b6bcb3f..49ed183ceb39d 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.test.ts @@ -14,96 +14,166 @@ describe('applyBulkEditOperation', () => { const ruleMock: Partial = { tags: ['tag-1', 'tag-2'], }; - expect( - applyBulkEditOperation( - { - field: 'tags', - value: ['add-tag'], - operation: 'add', - }, - ruleMock - ) - ).toHaveProperty('tags', ['tag-1', 'tag-2', 'add-tag']); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['add-tag'], + operation: 'add', + }, + ruleMock + ); + + expect(modifiedAttributes).toHaveProperty('tags', ['tag-1', 'tag-2', 'add-tag']); + expect(isAttributeModified).toBe(true); }); test('should add multiple tags', () => { const ruleMock: Partial = { tags: ['tag-1', 'tag-2'], }; - expect( - applyBulkEditOperation( - { - field: 'tags', - value: ['add-tag-1', 'add-tag-2'], - operation: 'add', - }, - ruleMock - ) - ).toHaveProperty('tags', ['tag-1', 'tag-2', 'add-tag-1', 'add-tag-2']); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['add-tag-1', 'add-tag-2'], + operation: 'add', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', [ + 'tag-1', + 'tag-2', + 'add-tag-1', + 'add-tag-2', + ]); + expect(isAttributeModified).toBe(true); }); test('should not have duplicated tags when added existed ones', () => { const ruleMock: Partial = { tags: ['tag-1', 'tag-2'], }; - expect( - applyBulkEditOperation( - { - field: 'tags', - value: ['tag-1', 'tag-3'], - operation: 'add', - }, - ruleMock - ) - ).toHaveProperty('tags', ['tag-1', 'tag-2', 'tag-3']); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['tag-1', 'tag-3'], + operation: 'add', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['tag-1', 'tag-2', 'tag-3']); + expect(isAttributeModified).toBe(true); }); test('should delete tag', () => { const ruleMock: Partial = { tags: ['tag-1', 'tag-2'], }; - expect( - applyBulkEditOperation( - { - field: 'tags', - value: ['tag-1'], - operation: 'delete', - }, - ruleMock - ) - ).toHaveProperty('tags', ['tag-2']); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['tag-1'], + operation: 'delete', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['tag-2']); + expect(isAttributeModified).toBe(true); }); test('should delete multiple tags', () => { const ruleMock: Partial = { tags: ['tag-1', 'tag-2'], }; - expect( - applyBulkEditOperation( - { - field: 'tags', - value: ['tag-1', 'tag-2'], - operation: 'delete', - }, - ruleMock - ) - ).toHaveProperty('tags', []); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['tag-1', 'tag-2'], + operation: 'delete', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', []); + expect(isAttributeModified).toBe(true); }); test('should rewrite tags', () => { const ruleMock: Partial = { tags: ['tag-1', 'tag-2'], }; - expect( - applyBulkEditOperation( - { - field: 'tags', - value: ['rewrite-tag'], - operation: 'set', - }, - ruleMock - ) - ).toHaveProperty('tags', ['rewrite-tag']); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['rewrite-tag'], + operation: 'set', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['rewrite-tag']); + expect(isAttributeModified).toBe(true); + }); + + test('should return isAttributeModified=false when only adding already existing tags', () => { + const ruleMock: Partial = { + tags: ['tag-1', 'tag-2'], + }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['tag-1', 'tag-2'], + operation: 'add', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['tag-1', 'tag-2']); + expect(isAttributeModified).toBe(false); + }); + + test('should return isAttributeModified=false when adding no tags', () => { + const ruleMock: Partial = { + tags: ['tag-1', 'tag-2'], + }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: [], + operation: 'add', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['tag-1', 'tag-2']); + expect(isAttributeModified).toBe(false); + }); + + test('should return isAttributeModified=false when deleting no tags', () => { + const ruleMock: Partial = { + tags: ['tag-1', 'tag-2'], + }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: [], + operation: 'delete', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['tag-1', 'tag-2']); + expect(isAttributeModified).toBe(false); + }); + + test('should return isAttributeModified=false when deleting non-existing tags', () => { + const ruleMock: Partial = { + tags: ['tag-1', 'tag-2'], + }; + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'tags', + value: ['tag-3'], + operation: 'delete', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('tags', ['tag-1', 'tag-2']); + expect(isAttributeModified).toBe(false); }); }); @@ -112,60 +182,60 @@ describe('applyBulkEditOperation', () => { const ruleMock = { actions: [{ id: 'mock-action-id', group: 'default', params: {} }], }; - expect( - applyBulkEditOperation( - { - field: 'actions', - value: [ - { id: 'mock-add-action-id-1', group: 'default', params: {} }, - { id: 'mock-add-action-id-2', group: 'default', params: {} }, - ], - operation: 'add', - }, - ruleMock - ) - ).toHaveProperty('actions', [ + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'actions', + value: [ + { id: 'mock-add-action-id-1', group: 'default', params: {} }, + { id: 'mock-add-action-id-2', group: 'default', params: {} }, + ], + operation: 'add', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('actions', [ { id: 'mock-action-id', group: 'default', params: {} }, { id: 'mock-add-action-id-1', group: 'default', params: {} }, { id: 'mock-add-action-id-2', group: 'default', params: {} }, ]); + expect(isAttributeModified).toBe(true); }); test('should add action with different params and same id', () => { const ruleMock = { actions: [{ id: 'mock-action-id', group: 'default', params: { test: 1 } }], }; - expect( - applyBulkEditOperation( - { - field: 'actions', - value: [{ id: 'mock-action-id', group: 'default', params: { test: 2 } }], - operation: 'add', - }, - ruleMock - ) - ).toHaveProperty('actions', [ + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'actions', + value: [{ id: 'mock-action-id', group: 'default', params: { test: 2 } }], + operation: 'add', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('actions', [ { id: 'mock-action-id', group: 'default', params: { test: 1 } }, { id: 'mock-action-id', group: 'default', params: { test: 2 } }, ]); + expect(isAttributeModified).toBe(true); }); test('should rewrite actions', () => { const ruleMock = { actions: [{ id: 'mock-action-id', group: 'default', params: {} }], }; - expect( - applyBulkEditOperation( - { - field: 'actions', - value: [{ id: 'mock-rewrite-action-id-1', group: 'default', params: {} }], - operation: 'set', - }, - ruleMock - ) - ).toHaveProperty('actions', [ + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'actions', + value: [{ id: 'mock-rewrite-action-id-1', group: 'default', params: {} }], + operation: 'set', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('actions', [ { id: 'mock-rewrite-action-id-1', group: 'default', params: {} }, ]); + expect(isAttributeModified).toBe(true); }); }); @@ -174,16 +244,16 @@ describe('applyBulkEditOperation', () => { const ruleMock = { actions: [{ id: 'mock-action-id', group: 'default', params: {} }], }; - expect( - applyBulkEditOperation( - { - field: 'throttle', - value: '1d', - operation: 'set', - }, - ruleMock - ) - ).toHaveProperty('throttle', '1d'); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'throttle', + value: '1d', + operation: 'set', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('throttle', '1d'); + expect(isAttributeModified).toBe(true); }); }); @@ -192,16 +262,16 @@ describe('applyBulkEditOperation', () => { const ruleMock = { actions: [{ id: 'mock-action-id', group: 'default', params: {} }], }; - expect( - applyBulkEditOperation( - { - field: 'notifyWhen', - value: 'onThrottleInterval', - operation: 'set', - }, - ruleMock - ) - ).toHaveProperty('notifyWhen', 'onThrottleInterval'); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'notifyWhen', + value: 'onThrottleInterval', + operation: 'set', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('notifyWhen', 'onThrottleInterval'); + expect(isAttributeModified).toBe(true); }); }); @@ -210,16 +280,16 @@ describe('applyBulkEditOperation', () => { const ruleMock = { actions: [{ id: 'mock-action-id', group: 'default', params: {} }], }; - expect( - applyBulkEditOperation( - { - field: 'schedule', - value: { interval: '1d' }, - operation: 'set', - }, - ruleMock - ) - ).toHaveProperty('schedule', { interval: '1d' }); + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + { + field: 'schedule', + value: { interval: '1d' }, + operation: 'set', + }, + ruleMock + ); + expect(modifiedAttributes).toHaveProperty('schedule', { interval: '1d' }); + expect(isAttributeModified).toBe(true); }); }); }); diff --git a/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.ts b/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.ts index e40d8e6c8c854..b5ca53642496e 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/apply_bulk_edit_operation.ts @@ -4,7 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { set, get } from 'lodash'; +import { set, get, isEqual } from 'lodash'; import type { BulkEditOperation, BulkEditFields } from '../types'; // defining an union type that will passed directly to generic function as a workaround for the issue similar to @@ -27,6 +27,8 @@ export const applyBulkEditOperation = (operation: BulkEditOper return arr.filter((item) => !itemsSet.has(item)); }; + const originalFieldValue = get(rule, operation.field); + switch (operation.operation) { case 'set': set(rule, operation.field, operation.value); @@ -49,5 +51,8 @@ export const applyBulkEditOperation = (operation: BulkEditOper break; } - return rule; + return { + modifiedAttributes: rule, + isAttributeModified: !isEqual(originalFieldValue, get(rule, operation.field)), + }; }; diff --git a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.test.ts b/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.test.ts index 5053b367b938e..c5570a7422d02 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.test.ts @@ -9,6 +9,7 @@ import { KueryNode } from '@kbn/es-query'; import { retryIfBulkEditConflicts } from './retry_if_bulk_edit_conflicts'; import { RETRY_IF_CONFLICTS_ATTEMPTS } from './wait_before_next_retry'; import { loggingSystemMock } from '@kbn/core/server/mocks'; +import { BulkEditSkipReason } from '../../../common/bulk_edit'; const mockFilter: KueryNode = { type: 'function', @@ -28,6 +29,11 @@ const mockSuccessfulResult = { { id: '2', type: 'alert', attributes: { name: 'Test rule 2' }, references: [] }, ], errors: [], + skipped: [ + { id: 'skip-1', name: 'skip-1', skip_reason: 'RULE_NOT_MODIFIED' as BulkEditSkipReason }, + { id: 'skip-2', name: 'skip-2', skip_reason: 'RULE_NOT_MODIFIED' as BulkEditSkipReason }, + { id: 'skip-5', name: 'skip-5', skip_reason: 'RULE_NOT_MODIFIED' as BulkEditSkipReason }, + ], }; async function OperationSuccessful() { @@ -78,6 +84,11 @@ describe('retryIfBulkEditConflicts', () => { expect(result).toEqual({ apiKeysToInvalidate: [], errors: [], + skipped: [ + { id: 'skip-1', name: 'skip-1', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-2', name: 'skip-2', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-5', name: 'skip-5', skip_reason: 'RULE_NOT_MODIFIED' }, + ], results: [ { attributes: {}, @@ -130,7 +141,7 @@ describe('retryIfBulkEditConflicts', () => { expect(mockLogger.warn).toBeCalledWith(`${mockOperationName} conflicts, exceeded retries`); }); - for (let i = 1; i <= RETRY_IF_CONFLICTS_ATTEMPTS; i++) { + for (let i = 1; i <= RETRY_IF_CONFLICTS_ATTEMPTS + 1; i++) { test(`should work when operation conflicts ${i} times`, async () => { const result = await retryIfBulkEditConflicts( mockLogger, @@ -141,4 +152,30 @@ describe('retryIfBulkEditConflicts', () => { expect(result).toBe(result); }); } + + test('should not return duplicated skip results when the underlying bulkEditOperation is retried multiple times and returns the same skip results on every attempt', async () => { + const result = await retryIfBulkEditConflicts( + mockLogger, + mockOperationName, + getOperationConflictsTimes(3), + mockFilter, + undefined, + ['apikey1', 'apikey2'], + [], + [], + [ + { id: 'skip-1', name: 'skip-1', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-2', name: 'skip-2', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-2', name: 'skip-2', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-3', name: 'skip-3', skip_reason: 'RULE_NOT_MODIFIED' }, + ] + ); + + expect(result.skipped).toEqual([ + { id: 'skip-1', name: 'skip-1', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-2', name: 'skip-2', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-3', name: 'skip-3', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skip-5', name: 'skip-5', skip_reason: 'RULE_NOT_MODIFIED' }, + ]); + }); }); diff --git a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.ts b/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.ts index 984ce17d149e0..8d03d01df5af4 100644 --- a/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.ts +++ b/x-pack/plugins/alerting/server/rules_client/common/retry_if_bulk_edit_conflicts.ts @@ -9,6 +9,7 @@ import pMap from 'p-map'; import { chunk } from 'lodash'; import { KueryNode } from '@kbn/es-query'; import { Logger, SavedObjectsBulkUpdateObject, SavedObjectsUpdateResponse } from '@kbn/core/server'; +import { BulkActionSkipResult } from '../../../common/bulk_edit'; import { convertRuleIdsToKueryNode } from '../../lib'; import { BulkOperationError } from '../types'; import { RawRule } from '../../types'; @@ -22,12 +23,14 @@ type BulkEditOperation = (filter: KueryNode | null) => Promise<{ rules: Array>; resultSavedObjects: Array>; errors: BulkOperationError[]; + skipped: BulkActionSkipResult[]; }>; interface ReturnRetry { apiKeysToInvalidate: string[]; results: Array>; errors: BulkOperationError[]; + skipped: BulkActionSkipResult[]; } /** @@ -52,7 +55,8 @@ export const retryIfBulkEditConflicts = async ( retries: number = RETRY_IF_CONFLICTS_ATTEMPTS, accApiKeysToInvalidate: string[] = [], accResults: Array> = [], - accErrors: BulkOperationError[] = [] + accErrors: BulkOperationError[] = [], + accSkipped: BulkActionSkipResult[] = [] ): Promise => { // run the operation, return if no errors or throw if not a conflict error try { @@ -61,6 +65,7 @@ export const retryIfBulkEditConflicts = async ( resultSavedObjects, errors: localErrors, rules: localRules, + skipped: localSkipped, } = await bulkEditOperation(filter); const conflictErrorMap = resultSavedObjects.reduce>( @@ -76,12 +81,17 @@ export const retryIfBulkEditConflicts = async ( const results = [...accResults, ...resultSavedObjects.filter((res) => res.error === undefined)]; const apiKeysToInvalidate = [...accApiKeysToInvalidate, ...localApiKeysToInvalidate]; const errors = [...accErrors, ...localErrors]; + // Create array of unique skipped rules by id + const skipped = [ + ...new Map([...accSkipped, ...localSkipped].map((item) => [item.id, item])).values(), + ]; if (conflictErrorMap.size === 0) { return { apiKeysToInvalidate, results, errors, + skipped, }; } @@ -102,6 +112,7 @@ export const retryIfBulkEditConflicts = async ( apiKeysToInvalidate, results, errors: [...errors, ...conflictErrors], + skipped, }; } @@ -126,7 +137,8 @@ export const retryIfBulkEditConflicts = async ( retries - 1, apiKeysToInvalidate, results, - errors + errors, + skipped ), { concurrency: 1, @@ -138,9 +150,10 @@ export const retryIfBulkEditConflicts = async ( results: [...acc.results, ...item.results], apiKeysToInvalidate: [...acc.apiKeysToInvalidate, ...item.apiKeysToInvalidate], errors: [...acc.errors, ...item.errors], + skipped: [...acc.skipped, ...item.skipped], }; }, - { results: [], apiKeysToInvalidate: [], errors: [] } + { results: [], apiKeysToInvalidate: [], errors: [], skipped: [] } ); } catch (err) { throw err; diff --git a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts index cae2ebc2406bb..dc97258fa994d 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts @@ -10,8 +10,22 @@ import Boom from '@hapi/boom'; import { cloneDeep } from 'lodash'; import { AlertConsumers } from '@kbn/rule-data-utils'; import { KueryNode, nodeBuilder } from '@kbn/es-query'; -import { SavedObjectsBulkUpdateObject, SavedObjectsUpdateResponse } from '@kbn/core/server'; -import { RawRule, SanitizedRule, RuleTypeParams, Rule, RuleSnoozeSchedule } from '../../types'; +import { + SavedObjectsBulkUpdateObject, + SavedObjectsFindResult, + SavedObjectsUpdateResponse, +} from '@kbn/core/server'; +import { BulkActionSkipResult } from '../../../common/bulk_edit'; +import { + RawRule, + SanitizedRule, + RuleTypeParams, + Rule, + RuleSnoozeSchedule, + RuleWithLegacyId, + RuleTypeRegistry, + RawRuleAction, +} from '../../types'; import { validateRuleTypeParams, getRuleNotifyWhenType, @@ -46,6 +60,7 @@ import { BulkOperationError, RuleBulkOperationAggregation, RulesClientContext, + CreateAPIKeyResult, } from '../types'; export type BulkEditFields = keyof Pick< @@ -95,7 +110,20 @@ export type BulkEditOperation = value?: undefined; }; -type RuleParamsModifier = (params: Params) => Promise; +type ApiKeysMap = Map; + +type ApiKeyAttributes = Pick; + +type RuleType = ReturnType; + +export interface RuleParamsModifierResult { + modifiedParams: Params; + isParamsUpdateSkipped: boolean; +} + +export type RuleParamsModifier = ( + params: Params +) => Promise>; export interface BulkEditOptionsFilter { filter?: string | KueryNode; @@ -118,6 +146,7 @@ export async function bulkEdit( options: BulkEditOptions ): Promise<{ rules: Array>; + skipped: BulkActionSkipResult[]; errors: BulkOperationError[]; total: number; }> { @@ -210,7 +239,7 @@ export async function bulkEdit( { concurrency: RULE_TYPE_CHECKS_CONCURRENCY } ); - const { apiKeysToInvalidate, results, errors } = await retryIfBulkEditConflicts( + const { apiKeysToInvalidate, results, errors, skipped } = await retryIfBulkEditConflicts( context.logger, `rulesClient.update('operations=${JSON.stringify(options.operations)}, paramsModifier=${ options.paramsModifier ? '[Function]' : undefined @@ -224,11 +253,13 @@ export async function bulkEdit( qNodeFilterWithAuth ); - await bulkMarkApiKeysForInvalidation( - { apiKeys: apiKeysToInvalidate }, - context.logger, - context.unsecuredSavedObjectsClient - ); + if (apiKeysToInvalidate.length > 0) { + await bulkMarkApiKeysForInvalidation( + { apiKeys: apiKeysToInvalidate }, + context.logger, + context.unsecuredSavedObjectsClient + ); + } const updatedRules = results.map(({ id, attributes, references }) => { return getAlertFromRaw( @@ -241,37 +272,9 @@ export async function bulkEdit( ); }); - // update schedules only if schedule operation is present - const scheduleOperation = options.operations.find( - ( - operation - ): operation is Extract }> => - operation.field === 'schedule' - ); - - if (scheduleOperation?.value) { - const taskIds = updatedRules.reduce((acc, rule) => { - if (rule.scheduledTaskId) { - acc.push(rule.scheduledTaskId); - } - return acc; - }, []); - - try { - await context.taskManager.bulkUpdateSchedules(taskIds, scheduleOperation.value); - context.logger.debug( - `Successfully updated schedules for underlying tasks: ${taskIds.join(', ')}` - ); - } catch (error) { - context.logger.error( - `Failure to update schedules for underlying tasks: ${taskIds.join( - ', ' - )}. TaskManager bulkUpdateSchedules failed with Error: ${error.message}` - ); - } - } + await bulkUpdateSchedules(context, options.operations, updatedRules); - return { rules: updatedRules, errors, total }; + return { rules: updatedRules, skipped, errors, total }; } async function bulkEditOcc( @@ -290,6 +293,7 @@ async function bulkEditOcc( rules: Array>; resultSavedObjects: Array>; errors: BulkOperationError[]; + skipped: BulkActionSkipResult[]; }> { const rulesFinder = await context.encryptedSavedObjectsClient.createPointInTimeFinderDecryptedAsInternalUser( @@ -302,210 +306,420 @@ async function bulkEditOcc( ); const rules: Array> = []; + const skipped: BulkActionSkipResult[] = []; const errors: BulkOperationError[] = []; - const apiKeysToInvalidate: string[] = []; - const apiKeysMap = new Map(); + const apiKeysMap: ApiKeysMap = new Map(); const username = await context.getUserName(); for await (const response of rulesFinder.find()) { await pMap( response.saved_objects, - async (rule) => { - try { - if (rule.attributes.apiKey) { - apiKeysMap.set(rule.id, { oldApiKey: rule.attributes.apiKey }); - } + async (rule: SavedObjectsFindResult) => + updateRuleAttributesAndParamsInMemory({ + context, + rule, + operations, + paramsModifier, + apiKeysMap, + rules, + skipped, + errors, + username, + }), + { concurrency: API_KEY_GENERATE_CONCURRENCY } + ); + } + await rulesFinder.close(); - const ruleType = context.ruleTypeRegistry.get(rule.attributes.alertTypeId); + const { result, apiKeysToInvalidate } = + rules.length > 0 + ? await saveBulkUpdatedRules(context, rules, apiKeysMap) + : { + result: { saved_objects: [] }, + apiKeysToInvalidate: [], + }; + + return { + apiKeysToInvalidate, + resultSavedObjects: result.saved_objects, + errors, + rules, + skipped, + }; +} - let attributes = cloneDeep(rule.attributes); - let ruleActions = { - actions: injectReferencesIntoActions( - rule.id, - rule.attributes.actions, - rule.references || [] - ), - }; +async function bulkUpdateSchedules( + context: RulesClientContext, + operations: BulkEditOperation[], + updatedRules: Array +): Promise { + const scheduleOperation = operations.find( + ( + operation + ): operation is Extract }> => + operation.field === 'schedule' + ); - for (const operation of operations) { - const { field } = operation; - if (field === 'snoozeSchedule' || field === 'apiKey') { - if (rule.attributes.actions.length) { - try { - await context.actionsAuthorization.ensureAuthorized('execute'); - } catch (error) { - throw Error(`Rule not authorized for bulk ${field} update - ${error.message}`); - } - } - } - } + if (!scheduleOperation?.value) { + return; + } + const taskIds = updatedRules.reduce((acc, rule) => { + if (rule.scheduledTaskId) { + acc.push(rule.scheduledTaskId); + } + return acc; + }, []); - let hasUpdateApiKeyOperation = false; - - for (const operation of operations) { - switch (operation.field) { - case 'actions': - await validateActions(context, ruleType, { - ...attributes, - actions: operation.value, - }); - ruleActions = applyBulkEditOperation(operation, ruleActions); - break; - case 'snoozeSchedule': - // Silently skip adding snooze or snooze schedules on security - // rules until we implement snoozing of their rules - if (attributes.consumer === AlertConsumers.SIEM) { - break; - } - if (operation.operation === 'set') { - const snoozeAttributes = getBulkSnoozeAttributes(attributes, operation.value); - try { - verifySnoozeScheduleLimit(snoozeAttributes); - } catch (error) { - throw Error(`Error updating rule: could not add snooze - ${error.message}`); - } - attributes = { - ...attributes, - ...snoozeAttributes, - }; - } - if (operation.operation === 'delete') { - const idsToDelete = operation.value && [...operation.value]; - if (idsToDelete?.length === 0) { - attributes.snoozeSchedule?.forEach((schedule) => { - if (schedule.id) { - idsToDelete.push(schedule.id); - } - }); - } - attributes = { - ...attributes, - ...getBulkUnsnoozeAttributes(attributes, idsToDelete), - }; - } - break; - case 'apiKey': { - hasUpdateApiKeyOperation = true; - break; - } - default: - attributes = applyBulkEditOperation(operation, attributes); - } - } + try { + await context.taskManager.bulkUpdateSchedules(taskIds, scheduleOperation.value); + context.logger.debug( + `Successfully updated schedules for underlying tasks: ${taskIds.join(', ')}` + ); + } catch (error) { + context.logger.error( + `Failure to update schedules for underlying tasks: ${taskIds.join( + ', ' + )}. TaskManager bulkUpdateSchedules failed with Error: ${error.message}` + ); + } +} - // validate schedule interval - if (attributes.schedule.interval) { - const isIntervalInvalid = - parseDuration(attributes.schedule.interval as string) < - context.minimumScheduleIntervalInMs; - if (isIntervalInvalid && context.minimumScheduleInterval.enforce) { - throw Error( - `Error updating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` - ); - } else if (isIntervalInvalid && !context.minimumScheduleInterval.enforce) { - context.logger.warn( - `Rule schedule interval (${attributes.schedule.interval}) for "${ruleType.id}" rule type with ID "${attributes.id}" is less than the minimum value (${context.minimumScheduleInterval.value}). Running rules at this interval may impact alerting performance. Set "xpack.alerting.rules.minimumScheduleInterval.enforce" to true to prevent such changes.` - ); - } - } +async function updateRuleAttributesAndParamsInMemory({ + context, + rule, + operations, + paramsModifier, + apiKeysMap, + rules, + skipped, + errors, + username, +}: { + context: RulesClientContext; + rule: SavedObjectsFindResult; + operations: BulkEditOptions['operations']; + paramsModifier: BulkEditOptions['paramsModifier']; + apiKeysMap: ApiKeysMap; + rules: Array>; + skipped: BulkActionSkipResult[]; + errors: BulkOperationError[]; + username: string | null; +}): Promise { + try { + if (rule.attributes.apiKey) { + apiKeysMap.set(rule.id, { oldApiKey: rule.attributes.apiKey }); + } - const ruleParams = paramsModifier - ? await paramsModifier(attributes.params as Params) - : attributes.params; - - // validate rule params - const validatedAlertTypeParams = validateRuleTypeParams( - ruleParams, - ruleType.validate?.params - ); - const validatedMutatedAlertTypeParams = validateMutatedRuleTypeParams( - validatedAlertTypeParams, - rule.attributes.params, - ruleType.validate?.params - ); - - const { - actions: rawAlertActions, - references, - params: updatedParams, - } = await extractReferences( - context, - ruleType, - ruleActions.actions, - validatedMutatedAlertTypeParams - ); - - const shouldUpdateApiKey = attributes.enabled || hasUpdateApiKeyOperation; - - // create API key - let createdAPIKey = null; - try { - createdAPIKey = shouldUpdateApiKey - ? await context.createAPIKey(generateAPIKeyName(ruleType.id, attributes.name)) - : null; - } catch (error) { - throw Error(`Error updating rule: could not create API key - ${error.message}`); - } + const ruleType = context.ruleTypeRegistry.get(rule.attributes.alertTypeId); - const apiKeyAttributes = apiKeyAsAlertAttributes(createdAPIKey, username); + await ensureAuthorizationForBulkUpdate(context, operations, rule); - // collect generated API keys - if (apiKeyAttributes.apiKey) { - apiKeysMap.set(rule.id, { - ...apiKeysMap.get(rule.id), - newApiKey: apiKeyAttributes.apiKey, - }); - } + const { attributes, ruleActions, hasUpdateApiKeyOperation, isAttributesUpdateSkipped } = + await getUpdatedAttributesFromOperations(context, operations, rule, ruleType); + + validateScheduleInterval(context, attributes.schedule.interval, ruleType.id, rule.id); + + const { modifiedParams: ruleParams, isParamsUpdateSkipped } = paramsModifier + ? await paramsModifier(attributes.params as Params) + : { + modifiedParams: attributes.params as Params, + isParamsUpdateSkipped: true, + }; + + // If neither attributes nor parameters were updated, mark + // the rule as skipped and continue to the next rule. + if (isAttributesUpdateSkipped && isParamsUpdateSkipped) { + skipped.push({ + id: rule.id, + name: rule.attributes.name, + skip_reason: 'RULE_NOT_MODIFIED', + }); + return; + } + + // validate rule params + const validatedAlertTypeParams = validateRuleTypeParams(ruleParams, ruleType.validate?.params); + const validatedMutatedAlertTypeParams = validateMutatedRuleTypeParams( + validatedAlertTypeParams, + rule.attributes.params, + ruleType.validate?.params + ); + + const { + actions: rawAlertActions, + references, + params: updatedParams, + } = await extractReferences( + context, + ruleType, + ruleActions.actions, + validatedMutatedAlertTypeParams + ); + + const { apiKeyAttributes } = await prepareApiKeys( + context, + rule, + ruleType, + apiKeysMap, + attributes, + hasUpdateApiKeyOperation, + username + ); + + const { updatedAttributes } = updateAttributes( + context, + attributes, + apiKeyAttributes, + updatedParams, + rawAlertActions, + username + ); - // get notifyWhen - const notifyWhen = getRuleNotifyWhenType( - attributes.notifyWhen ?? null, - attributes.throttle ?? null - ); + rules.push({ + ...rule, + references, + attributes: updatedAttributes, + }); + } catch (error) { + errors.push({ + message: error.message, + rule: { + id: rule.id, + name: rule.attributes?.name, + }, + }); + context.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.BULK_EDIT, + error, + }) + ); + } +} + +async function ensureAuthorizationForBulkUpdate( + context: RulesClientContext, + operations: BulkEditOperation[], + rule: SavedObjectsFindResult +): Promise { + if (rule.attributes.actions.length === 0) { + return; + } + + for (const operation of operations) { + const { field } = operation; + if (field === 'snoozeSchedule' || field === 'apiKey') { + try { + await context.actionsAuthorization.ensureAuthorized('execute'); + break; + } catch (error) { + throw Error(`Rule not authorized for bulk ${field} update - ${error.message}`); + } + } + } +} - const updatedAttributes = updateMeta(context, { +async function getUpdatedAttributesFromOperations( + context: RulesClientContext, + operations: BulkEditOperation[], + rule: SavedObjectsFindResult, + ruleType: RuleType +) { + let attributes = cloneDeep(rule.attributes); + let ruleActions = { + actions: injectReferencesIntoActions(rule.id, rule.attributes.actions, rule.references || []), + }; + + let hasUpdateApiKeyOperation = false; + let isAttributesUpdateSkipped = true; + + for (const operation of operations) { + // Check if the update should be skipped for the current action. + // If it should, save the skip reasons in attributesUpdateSkipReasons + // and continue to the next operation before without + // the `isAttributesUpdateSkipped` flag to false. + switch (operation.field) { + case 'actions': { + await validateActions(context, ruleType, { ...attributes, actions: operation.value }); + + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + operation, + ruleActions + ); + if (isAttributeModified) { + ruleActions = modifiedAttributes; + isAttributesUpdateSkipped = false; + } + break; + } + case 'snoozeSchedule': { + // Silently skip adding snooze or snooze schedules on security + // rules until we implement snoozing of their rules + if (rule.attributes.consumer === AlertConsumers.SIEM) { + // While the rule is technically not updated, we are still marking + // the rule as updated in case of snoozing, until support + // for snoozing is added. + isAttributesUpdateSkipped = false; + break; + } + if (operation.operation === 'set') { + const snoozeAttributes = getBulkSnoozeAttributes(rule.attributes, operation.value); + try { + verifySnoozeScheduleLimit(snoozeAttributes); + } catch (error) { + throw Error(`Error updating rule: could not add snooze - ${error.message}`); + } + attributes = { ...attributes, - ...apiKeyAttributes, - params: updatedParams as RawRule['params'], - actions: rawAlertActions, - notifyWhen, - updatedBy: username, - updatedAt: new Date().toISOString(), - }); - - // add mapped_params - const mappedParams = getMappedParams(updatedParams); - - if (Object.keys(mappedParams).length) { - updatedAttributes.mapped_params = mappedParams; + ...snoozeAttributes, + }; + } + if (operation.operation === 'delete') { + const idsToDelete = operation.value && [...operation.value]; + if (idsToDelete?.length === 0) { + attributes.snoozeSchedule?.forEach((schedule) => { + if (schedule.id) { + idsToDelete.push(schedule.id); + } + }); } + attributes = { + ...attributes, + ...getBulkUnsnoozeAttributes(attributes, idsToDelete), + }; + } + isAttributesUpdateSkipped = false; + break; + } + case 'apiKey': { + hasUpdateApiKeyOperation = true; + isAttributesUpdateSkipped = false; + break; + } + default: { + const { modifiedAttributes, isAttributeModified } = applyBulkEditOperation( + operation, + rule.attributes + ); - rules.push({ - ...rule, - references, - attributes: updatedAttributes, - }); - } catch (error) { - errors.push({ - message: error.message, - rule: { - id: rule.id, - name: rule.attributes?.name, - }, - }); - context.auditLogger?.log( - ruleAuditEvent({ - action: RuleAuditAction.BULK_EDIT, - error, - }) - ); + if (isAttributeModified) { + attributes = { + ...attributes, + ...modifiedAttributes, + }; + isAttributesUpdateSkipped = false; } - }, - { concurrency: API_KEY_GENERATE_CONCURRENCY } + } + } + } + return { + attributes, + ruleActions, + hasUpdateApiKeyOperation, + isAttributesUpdateSkipped, + }; +} + +function validateScheduleInterval( + context: RulesClientContext, + scheduleInterval: string, + ruleTypeId: string, + ruleId: string +): void { + if (!scheduleInterval) { + return; + } + const isIntervalInvalid = + parseDuration(scheduleInterval as string) < context.minimumScheduleIntervalInMs; + if (isIntervalInvalid && context.minimumScheduleInterval.enforce) { + throw Error( + `Error updating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` + ); + } else if (isIntervalInvalid && !context.minimumScheduleInterval.enforce) { + context.logger.warn( + `Rule schedule interval (${scheduleInterval}) for "${ruleTypeId}" rule type with ID "${ruleId}" is less than the minimum value (${context.minimumScheduleInterval.value}). Running rules at this interval may impact alerting performance. Set "xpack.alerting.rules.minimumScheduleInterval.enforce" to true to prevent such changes.` ); } - await rulesFinder.close(); +} +async function prepareApiKeys( + context: RulesClientContext, + rule: SavedObjectsFindResult, + ruleType: RuleType, + apiKeysMap: ApiKeysMap, + attributes: RawRule, + hasUpdateApiKeyOperation: boolean, + username: string | null +): Promise<{ apiKeyAttributes: ApiKeyAttributes }> { + const shouldUpdateApiKey = attributes.enabled || hasUpdateApiKeyOperation; + + let createdAPIKey: CreateAPIKeyResult | null = null; + try { + createdAPIKey = shouldUpdateApiKey + ? await context.createAPIKey(generateAPIKeyName(ruleType.id, attributes.name)) + : null; + } catch (error) { + throw Error(`Error updating rule: could not create API key - ${error.message}`); + } + + const apiKeyAttributes = apiKeyAsAlertAttributes(createdAPIKey, username); + // collect generated API keys + if (apiKeyAttributes.apiKey) { + apiKeysMap.set(rule.id, { + ...apiKeysMap.get(rule.id), + newApiKey: apiKeyAttributes.apiKey, + }); + } + + return { + apiKeyAttributes, + }; +} + +function updateAttributes( + context: RulesClientContext, + attributes: RawRule, + apiKeyAttributes: ApiKeyAttributes, + updatedParams: RuleTypeParams, + rawAlertActions: RawRuleAction[], + username: string | null +): { + updatedAttributes: RawRule; +} { + // get notifyWhen + const notifyWhen = getRuleNotifyWhenType( + attributes.notifyWhen ?? null, + attributes.throttle ?? null + ); + + const updatedAttributes = updateMeta(context, { + ...attributes, + ...apiKeyAttributes, + params: updatedParams as RawRule['params'], + actions: rawAlertActions, + notifyWhen, + updatedBy: username, + updatedAt: new Date().toISOString(), + }); + + // add mapped_params + const mappedParams = getMappedParams(updatedParams); + + if (Object.keys(mappedParams).length) { + updatedAttributes.mapped_params = mappedParams; + } + + return { + updatedAttributes, + }; +} + +async function saveBulkUpdatedRules( + context: RulesClientContext, + rules: Array>, + apiKeysMap: ApiKeysMap +) { + const apiKeysToInvalidate: string[] = []; let result; try { result = await context.unsecuredSavedObjectsClient.bulkCreate(rules, { overwrite: true }); @@ -514,12 +728,9 @@ async function bulkEditOcc( if (apiKeysMap.size > 0) { await bulkMarkApiKeysForInvalidation( { - apiKeys: Array.from(apiKeysMap.values()).reduce((acc, value) => { - if (value.newApiKey) { - acc.push(value.newApiKey); - } - return acc; - }, []), + apiKeys: Array.from(apiKeysMap.values()) + .filter((value) => value.newApiKey) + .map((value) => value.newApiKey as string), }, context.logger, context.unsecuredSavedObjectsClient @@ -541,5 +752,5 @@ async function bulkEditOcc( } }); - return { apiKeysToInvalidate, resultSavedObjects: result.saved_objects, errors, rules }; + return { result, apiKeysToInvalidate }; } diff --git a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts index 5e440d2e6b6d7..652f243060c82 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/bulk_edit.test.ts @@ -59,6 +59,9 @@ const rulesClientParams: jest.Mocked = { auditLogger, minimumScheduleInterval: { value: '1m', enforce: false }, }; +const paramsModifier = jest.fn(); + +const MOCK_API_KEY = Buffer.from('123:abc').toString('base64'); beforeEach(() => { getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry); @@ -93,7 +96,7 @@ describe('bulkEdit()', () => { ...existingRule, attributes: { ...existingRule.attributes, - apiKey: Buffer.from('123:abc').toString('base64'), + apiKey: MOCK_API_KEY, }, }; @@ -129,7 +132,14 @@ describe('bulkEdit()', () => { total: 1, }); - mockCreatePointInTimeFinderAsInternalUser(); + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { ...existingDecryptedRule.attributes, enabled: true }, + }, + ], + }); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [existingRule], @@ -150,7 +160,19 @@ describe('bulkEdit()', () => { producer: 'alerts', }); }); + describe('tags operations', () => { + beforeEach(() => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { ...existingDecryptedRule.attributes, tags: ['foo'] }, + }, + ], + }); + }); + test('should add new tag', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ @@ -229,7 +251,6 @@ describe('bulkEdit()', () => { }, ], }); - const result = await rulesClient.bulkEdit({ filter: '', operations: [ @@ -309,6 +330,256 @@ describe('bulkEdit()', () => { { overwrite: true } ); }); + + test('should skip operation when adding already existing tags', async () => { + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'add', + value: ['foo'], + }, + ], + }); + + expect(result.total).toBe(1); + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(0); + expect(result.skipped).toHaveLength(1); + expect(result.skipped[0]).toHaveProperty('id', existingRule.id); + expect(result.skipped[0]).toHaveProperty('skip_reason', 'RULE_NOT_MODIFIED'); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + }); + + test('should skip operation when adding no tags', async () => { + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'add', + value: [], + }, + ], + }); + + expect(result.total).toBe(1); + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(0); + expect(result.skipped).toHaveLength(1); + expect(result.skipped[0]).toHaveProperty('id', existingRule.id); + expect(result.skipped[0]).toHaveProperty('skip_reason', 'RULE_NOT_MODIFIED'); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + }); + + test('should skip operation when deleting non existing tags', async () => { + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'delete', + value: ['bar'], + }, + ], + }); + + expect(result.total).toBe(1); + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(0); + expect(result.skipped).toHaveLength(1); + expect(result.skipped[0]).toHaveProperty('id', existingRule.id); + expect(result.skipped[0]).toHaveProperty('skip_reason', 'RULE_NOT_MODIFIED'); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + }); + + test('should skip operation when deleting no tags', async () => { + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'delete', + value: [], + }, + ], + }); + + expect(result.total).toBe(1); + expect(result.errors).toHaveLength(0); + expect(result.rules).toHaveLength(0); + expect(result.skipped).toHaveLength(1); + expect(result.skipped[0]).toHaveProperty('id', existingRule.id); + expect(result.skipped[0]).toHaveProperty('skip_reason', 'RULE_NOT_MODIFIED'); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + }); + }); + + describe('index pattern operations', () => { + beforeEach(() => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { + ...existingDecryptedRule.attributes, + params: { index: ['index-1', 'index-2'] }, + }, + }, + ], + }); + }); + + test('should add index patterns', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + id: '1', + type: 'alert', + attributes: { + enabled: true, + tags: ['foo'], + alertTypeId: 'myType', + schedule: { interval: '1m' }, + consumer: 'myApp', + scheduledTaskId: 'task-123', + params: { + index: ['test-1', 'test-2', 'test-4', 'test-5'], + }, + throttle: null, + notifyWhen: null, + actions: [], + }, + references: [], + version: '123', + }, + ], + }); + + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['test-1', 'test-2', 'test-4', 'test-5'], + }, + isParamsUpdateSkipped: false, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [], + paramsModifier, + }); + + expect(result.rules[0].params).toHaveProperty('index', [ + 'test-1', + 'test-2', + 'test-4', + 'test-5', + ]); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + [ + expect.objectContaining({ + id: '1', + type: 'alert', + attributes: expect.objectContaining({ + params: expect.objectContaining({ + index: ['test-1', 'test-2', 'test-4', 'test-5'], + }), + }), + }), + ], + { overwrite: true } + ); + }); + + test('should delete index patterns', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + id: '1', + type: 'alert', + attributes: { + enabled: true, + tags: ['foo'], + alertTypeId: 'myType', + schedule: { interval: '1m' }, + consumer: 'myApp', + scheduledTaskId: 'task-123', + params: { + index: ['test-1'], + }, + throttle: null, + notifyWhen: null, + actions: [], + }, + references: [], + version: '123', + }, + ], + }); + + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['test-1'], + }, + isParamsUpdateSkipped: false, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [], + paramsModifier, + }); + + expect(result.rules[0].params).toHaveProperty('index', ['test-1']); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + [ + expect.objectContaining({ + id: '1', + type: 'alert', + attributes: expect.objectContaining({ + params: expect.objectContaining({ + index: ['test-1'], + }), + }), + }), + ], + { overwrite: true } + ); + }); + + test('should skip operation when params modifiers does not modify index pattern array', async () => { + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['test-1', 'test-2'], + }, + isParamsUpdateSkipped: true, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [], + paramsModifier, + }); + + expect(result.rules).toHaveLength(0); + expect(result.skipped[0].id).toBe(existingRule.id); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + }); }); describe('snoozeSchedule operations', () => { @@ -714,6 +985,16 @@ describe('bulkEdit()', () => { }); describe('apiKey operations', () => { + beforeEach(() => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { ...existingDecryptedRule.attributes, tags: ['foo'] }, + }, + ], + }); + }); test('should bulk update API key', async () => { // Does not generate API key for disabled rules await rulesClient.bulkEdit({ @@ -744,6 +1025,252 @@ describe('bulkEdit()', () => { }); }); + describe('mixed operations', () => { + beforeEach(() => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { + ...existingDecryptedRule.attributes, + tags: ['foo'], + params: { index: ['index-1', 'index-2'] }, + }, + }, + ], + }); + }); + + it('should succesfully update tags and index patterns and return updated rule', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + id: '1', + type: 'alert', + attributes: { + enabled: true, + tags: ['foo', 'test-1'], + alertTypeId: 'myType', + schedule: { interval: '1m' }, + consumer: 'myApp', + scheduledTaskId: 'task-123', + params: { + index: ['index-1', 'index-2', 'index-3'], + }, + throttle: null, + notifyWhen: null, + actions: [], + }, + references: [], + version: '123', + }, + ], + }); + + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['index-1', 'index-2', 'index-3'], + }, + isParamsUpdateSkipped: false, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-1'], + }, + ], + paramsModifier, + }); + + expect(result.rules[0]).toHaveProperty('tags', ['foo', 'test-1']); + expect(result.rules[0]).toHaveProperty('params.index', ['index-1', 'index-2', 'index-3']); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + [ + expect.objectContaining({ + id: '1', + type: 'alert', + attributes: expect.objectContaining({ + tags: ['foo', 'test-1'], + params: { + index: ['index-1', 'index-2', 'index-3'], + }, + }), + }), + ], + { overwrite: true } + ); + }); + + it('should succesfully update rule if tags are updated but index patterns are not', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + id: '1', + type: 'alert', + attributes: { + enabled: true, + tags: ['foo', 'test-1'], + alertTypeId: 'myType', + schedule: { interval: '1m' }, + consumer: 'myApp', + scheduledTaskId: 'task-123', + params: { + index: ['index-1', 'index-2'], + }, + throttle: null, + notifyWhen: null, + actions: [], + }, + references: [], + version: '123', + }, + ], + }); + + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['index-1', 'index-2'], + }, + isParamsUpdateSkipped: true, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'add', + value: ['test-1'], + }, + ], + paramsModifier, + }); + + expect(result.rules[0]).toHaveProperty('tags', ['foo', 'test-1']); + expect(result.rules[0]).toHaveProperty('params.index', ['index-1', 'index-2']); + expect(result.skipped).toHaveLength(0); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + [ + expect.objectContaining({ + id: '1', + type: 'alert', + attributes: expect.objectContaining({ + tags: ['foo', 'test-1'], + params: { + index: ['index-1', 'index-2'], + }, + }), + }), + ], + { overwrite: true } + ); + }); + + it('should succesfully update rule if index patterns are updated but tags are not', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [ + { + id: '1', + type: 'alert', + attributes: { + enabled: true, + tags: ['foo'], + alertTypeId: 'myType', + schedule: { interval: '1m' }, + consumer: 'myApp', + scheduledTaskId: 'task-123', + params: { + index: ['index-1', 'index-2', 'index-3'], + }, + throttle: null, + notifyWhen: null, + actions: [], + }, + references: [], + version: '123', + }, + ], + }); + + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['index-1', 'index-2', 'index-3'], + }, + isParamsUpdateSkipped: false, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'add', + value: ['foo'], + }, + ], + paramsModifier, + }); + + expect(result.rules[0]).toHaveProperty('tags', ['foo']); + expect(result.rules[0]).toHaveProperty('params.index', ['index-1', 'index-2', 'index-3']); + expect(result.skipped).toHaveLength(0); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + [ + expect.objectContaining({ + id: '1', + type: 'alert', + attributes: expect.objectContaining({ + tags: ['foo'], + params: { + index: ['index-1', 'index-2', 'index-3'], + }, + }), + }), + ], + { overwrite: true } + ); + }); + + it('should skip rule update if neither index patterns nor tags are updated', async () => { + paramsModifier.mockResolvedValue({ + modifiedParams: { + index: ['index-1', 'index-2'], + }, + isParamsUpdateSkipped: true, + }); + + const result = await rulesClient.bulkEdit({ + filter: '', + operations: [ + { + field: 'tags', + operation: 'add', + value: ['foo'], + }, + ], + paramsModifier, + }); + + expect(result.skipped[0]).toHaveProperty('id', existingRule.id); + expect(result.skipped[0]).toHaveProperty('skip_reason', 'RULE_NOT_MODIFIED'); + + expect(result.rules).toHaveLength(0); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(0); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(0); + }); + }); + describe('ruleTypes aggregation and validation', () => { test('should call unsecuredSavedObjectsClient.find for aggregations by alertTypeId and consumer', async () => { await rulesClient.bulkEdit({ @@ -964,6 +1491,18 @@ describe('bulkEdit()', () => { }); describe('apiKeys', () => { + beforeEach(() => { + createAPIKeyMock.mockResolvedValueOnce({ apiKeysEnabled: true, result: { api_key: '111' } }); + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [ + { + ...existingDecryptedRule, + attributes: { ...existingDecryptedRule.attributes, enabled: true }, + }, + ], + }); + }); + test('should call createPointInTimeFinderDecryptedAsInternalUser that returns api Keys', async () => { await rulesClient.bulkEdit({ filter: 'alert.attributes.tags: "APM"', @@ -1021,16 +1560,6 @@ describe('bulkEdit()', () => { }); test('should call bulkMarkApiKeysForInvalidation to invalidate unused keys if bulkCreate failed', async () => { - createAPIKeyMock.mockReturnValue({ apiKeysEnabled: true, result: { api_key: '111' } }); - mockCreatePointInTimeFinderAsInternalUser({ - saved_objects: [ - { - ...existingDecryptedRule, - attributes: { ...existingDecryptedRule.attributes, enabled: true }, - }, - ], - }); - unsecuredSavedObjectsClient.bulkCreate.mockImplementation(() => { throw new Error('Fail'); }); @@ -1057,16 +1586,6 @@ describe('bulkEdit()', () => { }); test('should call bulkMarkApiKeysForInvalidation to invalidate unused keys if SO update failed', async () => { - createAPIKeyMock.mockReturnValue({ apiKeysEnabled: true, result: { api_key: '111' } }); - mockCreatePointInTimeFinderAsInternalUser({ - saved_objects: [ - { - ...existingDecryptedRule, - attributes: { ...existingDecryptedRule.attributes, enabled: true }, - }, - ], - }); - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [ { @@ -1128,15 +1647,6 @@ describe('bulkEdit()', () => { }); test('should return error in rule errors if key is not generated', async () => { - mockCreatePointInTimeFinderAsInternalUser({ - saved_objects: [ - { - ...existingDecryptedRule, - attributes: { ...existingDecryptedRule.attributes, enabled: true }, - }, - ], - }); - await rulesClient.bulkEdit({ filter: 'alert.attributes.tags: "APM"', operations: [ @@ -1152,6 +1662,12 @@ describe('bulkEdit()', () => { }); describe('params validation', () => { + beforeEach(() => { + mockCreatePointInTimeFinderAsInternalUser({ + saved_objects: [existingDecryptedRule], + }); + }); + test('should return error for rule that failed params validation', async () => { ruleTypeRegistry.get.mockReturnValue({ id: '123', @@ -1218,7 +1734,7 @@ describe('bulkEdit()', () => { { field: 'tags', operation: 'add', - value: ['test-1'], + value: ['test-1', 'another-tag'], }, ], }); @@ -1251,7 +1767,7 @@ describe('bulkEdit()', () => { paramsModifier: async (params) => { params.index = ['test-index-*']; - return params; + return { modifiedParams: params, isParamsUpdateSkipped: false, skipReasons: [] }; }, }); @@ -1294,7 +1810,7 @@ describe('bulkEdit()', () => { paramsModifier: async (params) => { params.index = ['test-index-*']; - return params; + return { modifiedParams: params, isParamsUpdateSkipped: false, skipReasons: [] }; }, }); diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/request_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/request_schema.ts index d595dc88441cc..b0860c55ebd5a 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/request_schema.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/request_schema.ts @@ -66,7 +66,9 @@ const BulkActionEditPayloadTags = t.type({ value: RuleTagArray, }); -type BulkActionEditPayloadIndexPatterns = t.TypeOf; +export type BulkActionEditPayloadIndexPatterns = t.TypeOf< + typeof BulkActionEditPayloadIndexPatterns +>; const BulkActionEditPayloadIndexPatterns = t.intersection([ t.type({ type: t.union([ diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/response_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/response_schema.ts new file mode 100644 index 0000000000000..e9b4ad6bbc1e5 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/bulk_actions/response_schema.ts @@ -0,0 +1,53 @@ +/* + * 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 type { BulkActionSkipResult } from '@kbn/alerting-plugin/common'; +import type { RuleResponse } from '../../../../rule_schema'; +import type { BulkActionsDryRunErrCode } from '../../../../../constants'; + +export interface RuleDetailsInError { + id: string; + name?: string; +} +export interface NormalizedRuleError { + message: string; + status_code: number; + err_code?: BulkActionsDryRunErrCode; + rules: RuleDetailsInError[]; +} +export interface BulkEditActionResults { + updated: RuleResponse[]; + created: RuleResponse[]; + deleted: RuleResponse[]; + skipped: BulkActionSkipResult[]; +} + +export interface BulkEditActionSummary { + failed: number; + skipped: number; + succeeded: number; + total: number; +} +export interface BulkEditActionSuccessResponse { + success: boolean; + rules_count: number; + attributes: { + results: BulkEditActionResults; + summary: BulkEditActionSummary; + }; +} +export interface BulkEditActionErrorResponse { + status_code: number; + message: string; + attributes: { + results: BulkEditActionResults; + summary: BulkEditActionSummary; + errors?: NormalizedRuleError[]; + }; +} + +export type BulkEditActionResponse = BulkEditActionSuccessResponse | BulkEditActionErrorResponse; diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts index 701aed857a7ea..7aa90997a8101 100644 --- a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts +++ b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts @@ -10,7 +10,6 @@ import { MODAL_CONFIRMATION_BODY, RULE_CHECKBOX, RULES_TAGS_POPOVER_BTN, - TOASTER_BODY, MODAL_ERROR_BODY, } from '../../screens/alerts_detection_rules'; @@ -203,7 +202,7 @@ describe('Detection rules, bulk edit', () => { // action should finish typeTags(['test-tag']); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); }); it('Prebuilt and custom rules selected: user cancels action', () => { @@ -230,9 +229,9 @@ describe('Detection rules, bulk edit', () => { // open add tags form and add 2 new tags openBulkEditAddTagsForm(); - typeTags(prePopulatedTags); + typeTags(['new-tag-1']); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount }); + waitForBulkEditActionToFinish({ updatedCount: rulesCount }); testMultipleSelectedRulesLabel(rulesCount); // check if first four(rulesCount) rules still selected and tags are updated @@ -241,7 +240,7 @@ describe('Detection rules, bulk edit', () => { cy.get(RULES_TAGS_POPOVER_BTN) .eq(i) .each(($el) => { - testTagsBadge($el, prePopulatedTags); + testTagsBadge($el, prePopulatedTags.concat(['new-tag-1'])); }); } }); @@ -280,7 +279,7 @@ describe('Detection rules, bulk edit', () => { openBulkEditAddTagsForm(); typeTags(tagsToBeAdded); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if all rules have been updated with new tags testAllTagsBadges(resultingTags); @@ -309,12 +308,7 @@ describe('Detection rules, bulk edit', () => { openBulkEditAddTagsForm(); typeTags(tagsToBeAdded); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); - - cy.get(TOASTER_BODY).should( - 'have.text', - `You've successfully updated ${expectedNumberOfCustomRulesToBeEdited} rules` - ); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); }); it('Overwrite tags in custom rules', () => { @@ -336,7 +330,7 @@ describe('Detection rules, bulk edit', () => { typeTags(tagsToOverwrite); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if all rules have been updated with new tags testAllTagsBadges(tagsToOverwrite); @@ -358,7 +352,7 @@ describe('Detection rules, bulk edit', () => { openBulkEditDeleteTagsForm(); typeTags(tagsToDelete); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check tags has been removed from all rules testAllTagsBadges(resultingTags); @@ -383,7 +377,7 @@ describe('Detection rules, bulk edit', () => { typeIndexPatterns(indexPattersToBeAdded); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfNotMLRules }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfNotMLRules }); // check if rule has been updated goToTheRuleDetailsOf(RULE_NAME); @@ -413,7 +407,7 @@ describe('Detection rules, bulk edit', () => { typeIndexPatterns(indexPattersToBeAdded); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfNotMLRules }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfNotMLRules }); // check if rule has been updated goToTheRuleDetailsOf(RULE_NAME); @@ -431,12 +425,9 @@ describe('Detection rules, bulk edit', () => { typeIndexPatterns(indexPattersToBeAdded); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfNotMLRules }); - - cy.get(TOASTER_BODY).should( - 'have.text', - `You've successfully updated ${expectedNumberOfNotMLRules} rules. If you did not select to apply changes to rules using Kibana data views, those rules were not updated and will continue using data views.` - ); + waitForBulkEditActionToFinish({ + updatedCount: expectedNumberOfNotMLRules, + }); }); it('Overwrite index patterns in custom rules', () => { @@ -458,7 +449,7 @@ describe('Detection rules, bulk edit', () => { typeIndexPatterns(indexPattersToWrite); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfNotMLRules }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfNotMLRules }); // check if rule has been updated goToTheRuleDetailsOf(RULE_NAME); @@ -477,7 +468,7 @@ describe('Detection rules, bulk edit', () => { typeIndexPatterns(indexPatternsToDelete); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfNotMLRules }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfNotMLRules }); // check if rule has been updated goToTheRuleDetailsOf(RULE_NAME); @@ -494,7 +485,7 @@ describe('Detection rules, bulk edit', () => { submitBulkEditForm(); // error toast should be displayed that that rules edit failed - cy.contains(TOASTER_BODY, `${expectedNumberOfNotMLRules} rules failed to update.`); + waitForBulkEditActionToFinish({ failedCount: expectedNumberOfNotMLRules }); // on error toast button click display error that index patterns can't be empty clickErrorToastBtn(); @@ -520,7 +511,7 @@ describe('Detection rules, bulk edit', () => { selectTimelineTemplate(timelineTemplateName); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if timeline template has been updated to selected one goToTheRuleDetailsOf(RULE_NAME); @@ -536,7 +527,7 @@ describe('Detection rules, bulk edit', () => { clickApplyTimelineTemplatesMenuItem(); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if timeline template has been updated to selected one, by opening rule that have had timeline prior to editing goToTheRuleDetailsOf(RULE_NAME); @@ -570,7 +561,7 @@ describe('Detection rules, bulk edit', () => { setScheduleLookbackTimeUnit('Minutes'); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); goToTheRuleDetailsOf(RULE_NAME); @@ -592,7 +583,7 @@ describe('Detection rules, bulk edit', () => { setScheduleLookbackTimeUnit('Seconds'); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); goToTheRuleDetailsOf(RULE_NAME); diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts index b897aa3024cda..8de71890326d6 100644 --- a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts +++ b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts @@ -150,7 +150,7 @@ describe('Detection rules, bulk edit of rule actions', () => { addSlackRuleAction(expectedSlackMessage); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfRulesToBeEdited }); // check if rule has been updated goToEditRuleActionsSettingsOf(ruleNameToAssert); @@ -181,7 +181,7 @@ describe('Detection rules, bulk edit of rule actions', () => { ); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfRulesToBeEdited }); // check if rule has been updated goToEditRuleActionsSettingsOf(ruleNameToAssert); @@ -204,7 +204,7 @@ describe('Detection rules, bulk edit of rule actions', () => { addEmailConnectorAndRuleAction(expectedEmail, expectedSubject); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if rule has been updated goToEditRuleActionsSettingsOf(ruleNameToAssert); diff --git a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_data_view.cy.ts b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_data_view.cy.ts index 766c8d9483f0a..4644ed8a436d4 100644 --- a/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_data_view.cy.ts +++ b/x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_data_view.cy.ts @@ -58,13 +58,13 @@ const expectedIndexPatterns = ['index-1-*', 'index-2-*']; const expectedNumberOfCustomRulesToBeEdited = 6; -const indexDataSource = { dataView: DATA_VIEW_ID, type: 'dataView' } as const; +const dataViewDataSource = { dataView: DATA_VIEW_ID, type: 'dataView' } as const; -const defaultRuleData = { - dataSource: indexDataSource, +const dataViewRuleData = { + dataSource: dataViewDataSource, }; -describe('Detection rules, bulk edit, data view', () => { +describe('Bulk editing index patterns of rules with a data view only', () => { before(() => { cleanKibana(); login(); @@ -75,26 +75,29 @@ describe('Detection rules, bulk edit, data view', () => { postDataView(DATA_VIEW_ID); - createCustomRule({ ...getNewRule(), ...defaultRuleData }, '1'); - createEventCorrelationRule({ ...getEqlRule(), ...defaultRuleData }, '2'); - createCustomIndicatorRule({ ...getNewThreatIndicatorRule(), ...defaultRuleData }, '3'); - createThresholdRule({ ...getNewThresholdRule(), ...defaultRuleData }, '4'); - createNewTermsRule({ ...getNewTermsRule(), ...defaultRuleData }, '5'); - createSavedQueryRule({ ...getNewRule(), ...defaultRuleData, savedId: 'mocked' }, '6'); + createCustomRule({ ...getNewRule(), ...dataViewRuleData }, '1'); + createEventCorrelationRule({ ...getEqlRule(), ...dataViewRuleData }, '2'); + createCustomIndicatorRule({ ...getNewThreatIndicatorRule(), ...dataViewRuleData }, '3'); + createThresholdRule({ ...getNewThresholdRule(), ...dataViewRuleData }, '4'); + createNewTermsRule({ ...getNewTermsRule(), ...dataViewRuleData }, '5'); + createSavedQueryRule({ ...getNewRule(), ...dataViewRuleData, savedId: 'mocked' }, '6'); visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL); waitForRulesTableToBeLoaded(); }); - it('Add index patterns to custom rules with configured data view', () => { + it('Add index patterns to custom rules with configured data view: all rules are skipped', () => { selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); openBulkEditAddIndexPatternsForm(); typeIndexPatterns(expectedIndexPatterns); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ + skippedCount: expectedNumberOfCustomRulesToBeEdited, + showDataViewsWarning: true, + }); // check if rule still has data view and index patterns field does not exist goToRuleDetails(); @@ -102,7 +105,7 @@ describe('Detection rules, bulk edit, data view', () => { assertDetailsNotExist(INDEX_PATTERNS_DETAILS); }); - it('Add index patterns to custom rules with configured data view when data view checkbox is checked', () => { + it('Add index patterns to custom rules with configured data view when data view checkbox is checked: rules are updated', () => { selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); openBulkEditAddIndexPatternsForm(); @@ -115,7 +118,7 @@ describe('Detection rules, bulk edit, data view', () => { submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if rule has been updated with index patterns and data view does not exist goToRuleDetails(); @@ -123,7 +126,7 @@ describe('Detection rules, bulk edit, data view', () => { assertDetailsNotExist(DATA_VIEW_DETAILS); }); - it('Overwrite index patterns in custom rules with configured data view', () => { + it('Overwrite index patterns in custom rules with configured data view when overwrite data view checkbox is NOT checked:: rules are skipped', () => { selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); openBulkEditAddIndexPatternsForm(); @@ -131,7 +134,10 @@ describe('Detection rules, bulk edit, data view', () => { checkOverwriteIndexPatternsCheckbox(); submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ + skippedCount: expectedNumberOfCustomRulesToBeEdited, + showDataViewsWarning: true, + }); // check if rule still has data view and index patterns field does not exist goToRuleDetails(); @@ -139,7 +145,7 @@ describe('Detection rules, bulk edit, data view', () => { assertDetailsNotExist(INDEX_PATTERNS_DETAILS); }); - it('Overwrite index patterns in custom rules with configured data view when data view checkbox is checked', () => { + it('Overwrite index patterns in custom rules with configured data view when overwrite data view checkbox is checked: rules are updated', () => { selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); openBulkEditAddIndexPatternsForm(); @@ -149,7 +155,7 @@ describe('Detection rules, bulk edit, data view', () => { submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ updatedCount: expectedNumberOfCustomRulesToBeEdited }); // check if rule has been overwritten with index patterns and data view does not exist goToRuleDetails(); @@ -157,7 +163,7 @@ describe('Detection rules, bulk edit, data view', () => { assertDetailsNotExist(DATA_VIEW_DETAILS); }); - it('Delete index patterns in custom rules with configured data view', () => { + it('Delete index patterns in custom rules with configured data view: rules are skipped', () => { selectNumberOfRules(expectedNumberOfCustomRulesToBeEdited); openBulkEditDeleteIndexPatternsForm(); @@ -168,10 +174,79 @@ describe('Detection rules, bulk edit, data view', () => { submitBulkEditForm(); - waitForBulkEditActionToFinish({ rulesCount: expectedNumberOfCustomRulesToBeEdited }); + waitForBulkEditActionToFinish({ + skippedCount: expectedNumberOfCustomRulesToBeEdited, + showDataViewsWarning: true, + }); // check if rule still has data view and index patterns field does not exist goToRuleDetails(); getDetails(DATA_VIEW_DETAILS).contains(DATA_VIEW_ID); }); }); + +describe('Bulk editing index patterns of rules with index patterns and rules with a data view', () => { + const customRulesNumber = 2; + before(() => { + cleanKibana(); + login(); + }); + beforeEach(() => { + deleteAlertsAndRules(); + esArchiverResetKibana(); + + postDataView(DATA_VIEW_ID); + + createCustomRule({ ...getNewRule(), ...dataViewRuleData }, '1'); + createCustomRule( + { + ...getNewRule(), + dataSource: { + type: 'indexPatterns', + index: ['test-index-1-*'], + }, + }, + '2' + ); + + visitWithoutDateRange(SECURITY_DETECTIONS_RULES_URL); + + waitForRulesTableToBeLoaded(); + }); + + it('Add index patterns to custom rules: one rule is updated, one rule is skipped', () => { + selectNumberOfRules(customRulesNumber); + + openBulkEditAddIndexPatternsForm(); + typeIndexPatterns(expectedIndexPatterns); + submitBulkEditForm(); + + waitForBulkEditActionToFinish({ + updatedCount: 1, + skippedCount: 1, + showDataViewsWarning: true, + }); + + // check if rule still has data view and index patterns field does not exist + goToRuleDetails(); + getDetails(DATA_VIEW_DETAILS).contains(DATA_VIEW_ID); + assertDetailsNotExist(INDEX_PATTERNS_DETAILS); + }); + + it('Add index patterns to custom rules when overwrite data view checkbox is checked: all rules are updated', () => { + selectNumberOfRules(customRulesNumber); + + openBulkEditAddIndexPatternsForm(); + typeIndexPatterns(expectedIndexPatterns); + checkOverwriteDataViewCheckbox(); + submitBulkEditForm(); + + waitForBulkEditActionToFinish({ + updatedCount: 2, + }); + + // check if rule still has data view and index patterns field does not exist + goToRuleDetails(); + assertDetailsNotExist(DATA_VIEW_DETAILS); + }); +}); diff --git a/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts b/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts index 64f0e88849356..e01e72fe140db 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/rules_bulk_edit.ts @@ -125,9 +125,42 @@ export const openTagsSelect = () => { export const submitBulkEditForm = () => cy.get(RULES_BULK_EDIT_FORM_CONFIRM_BTN).click(); -export const waitForBulkEditActionToFinish = ({ rulesCount }: { rulesCount: number }) => { +export const waitForBulkEditActionToFinish = ({ + updatedCount, + skippedCount, + failedCount, + showDataViewsWarning = false, +}: { + updatedCount?: number; + skippedCount?: number; + failedCount?: number; + showDataViewsWarning?: boolean; +}) => { cy.get(BULK_ACTIONS_PROGRESS_BTN).should('be.disabled'); - cy.contains(TOASTER_BODY, `You've successfully updated ${rulesCount} rule`); + + if (updatedCount !== undefined) { + cy.contains(TOASTER_BODY, `You've successfully updated ${updatedCount} rule`); + } + if (failedCount !== undefined) { + if (failedCount === 1) { + cy.contains(TOASTER_BODY, `${failedCount} rule failed to update`); + } else { + cy.contains(TOASTER_BODY, `${failedCount} rules failed to update`); + } + } + if (skippedCount !== undefined) { + if (skippedCount === 1) { + cy.contains(TOASTER_BODY, `${skippedCount} rule was skipped`); + } else { + cy.contains(TOASTER_BODY, `${skippedCount} rules were skipped`); + } + if (showDataViewsWarning) { + cy.contains( + TOASTER_BODY, + 'If you did not select to apply changes to rules using Kibana data views, those rules were not updated and will continue using data views.' + ); + } + } }; export const checkPrebuiltRulesCannotBeModified = (rulesCount: number) => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts index 9e4f22484a00d..c867b6b3fd792 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts @@ -189,6 +189,7 @@ export const fetchRuleById = async ({ id, signal }: FetchRuleProps): Promise 0 ? ` ${i18n.RULES_BULK_EDIT_SUCCESS_DATA_VIEW_RULES_SKIPPED_DETAIL}` : null; if ( editPayload.some( (x) => @@ -70,12 +72,13 @@ export function explainBulkEditSuccess( x.type === BulkActionEditType.delete_index_patterns ) ) { - return `${i18n.RULES_BULK_EDIT_SUCCESS_DESCRIPTION(summary.succeeded)}. ${ - i18n.RULES_BULK_EDIT_SUCCESS_INDEX_EDIT_DESCRIPTION - }`; + return `${i18n.RULES_BULK_EDIT_SUCCESS_DESCRIPTION( + summary.succeeded, + summary.skipped + )}${dataViewSkipDetail}`; } - return i18n.RULES_BULK_EDIT_SUCCESS_DESCRIPTION(summary.succeeded); + return i18n.RULES_BULK_EDIT_SUCCESS_DESCRIPTION(summary.succeeded, summary.skipped); } export function summarizeBulkError(action: BulkActionType): string { @@ -125,7 +128,7 @@ export function explainBulkError(action: BulkActionType, error: HTTPError): stri return i18n.RULES_BULK_DISABLE_FAILURE_DESCRIPTION(summary.failed); case BulkActionType.edit: - return i18n.RULES_BULK_EDIT_FAILURE_DESCRIPTION(summary.failed); + return i18n.RULES_BULK_EDIT_FAILURE_DESCRIPTION(summary.failed, summary.skipped); } } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/helpers.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/helpers.ts index 13666f514b0be..49d29fc34da76 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/helpers.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/helpers.ts @@ -69,6 +69,7 @@ export const getExportedRulesCounts = async (blob: Blob): Promise +export const RULES_BULK_EDIT_SUCCESS_DESCRIPTION = ( + succeededRulesCount: number, + skippedRulesCount: number +) => i18n.translate( 'xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.successToastDescription', { - values: { rulesCount }, - defaultMessage: - "You've successfully updated {rulesCount, plural, =1 {# rule} other {# rules}}", + values: { succeededRulesCount, skippedRulesCount }, + defaultMessage: `{succeededRulesCount, plural, =0 {} =1 {You've successfully updated # rule. } other {You've successfully updated # rules. }} + {skippedRulesCount, plural, =0 {} =1 { # rule was skipped.} other { # rules were skipped.}} + `, } ); -export const RULES_BULK_EDIT_SUCCESS_INDEX_EDIT_DESCRIPTION = i18n.translate( +export const RULES_BULK_EDIT_SUCCESS_DATA_VIEW_RULES_SKIPPED_DETAIL = i18n.translate( 'xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.successIndexEditToastDescription', { defaultMessage: @@ -1090,12 +1094,16 @@ export const RULES_BULK_EDIT_FAILURE = i18n.translate( } ); -export const RULES_BULK_EDIT_FAILURE_DESCRIPTION = (rulesCount: number) => +export const RULES_BULK_EDIT_FAILURE_DESCRIPTION = ( + failedRulesCount: number, + skippedRulesCount: number +) => i18n.translate( 'xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.errorToastDescription', { - values: { rulesCount }, - defaultMessage: '{rulesCount, plural, =1 {# rule} other {# rules}} failed to update.', + values: { failedRulesCount, skippedRulesCount }, + defaultMessage: + '{failedRulesCount, plural, =0 {} =1 {# rule} other {# rules}} failed to update. {skippedRulesCount, plural, =0 {} =1 { # rule was skipped.} other { # rules were skipped.}}', } ); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts index 9da1573738d82..ed15fd08c9428 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts @@ -33,7 +33,10 @@ const buildResponses = (method: Method, calls: MockCall[]): ResponseCall[] => { case 'custom': return calls.map(([call]) => ({ status: call.statusCode, - body: JSON.parse(call.body), + body: + Buffer.isBuffer(call.body) || typeof call.body === 'string' + ? JSON.parse(call.body) + : call.body, })); case 'customError': return calls.map(([call]) => ({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.test.ts index f64de099dfcbe..a7034123e7a96 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.test.ts @@ -75,6 +75,7 @@ describe('Perform bulk action route', () => { results: someBulkActionResults(), summary: { failed: 0, + skipped: 0, succeeded: 1, total: 1, }, @@ -96,6 +97,7 @@ describe('Perform bulk action route', () => { results: someBulkActionResults(), summary: { failed: 0, + skipped: 0, succeeded: 0, total: 0, }, @@ -148,6 +150,7 @@ describe('Perform bulk action route', () => { results: someBulkActionResults(), summary: { failed: 1, + skipped: 0, succeeded: 0, total: 1, }, @@ -185,6 +188,7 @@ describe('Perform bulk action route', () => { results: someBulkActionResults(), summary: { failed: 1, + skipped: 0, succeeded: 0, total: 1, }, @@ -224,6 +228,7 @@ describe('Perform bulk action route', () => { results: someBulkActionResults(), summary: { failed: 1, + skipped: 0, succeeded: 0, total: 1, }, @@ -236,6 +241,7 @@ describe('Perform bulk action route', () => { it('returns partial failure error if update of few rules fail', async () => { clients.rulesClient.bulkEdit.mockResolvedValue({ rules: [mockRule, mockRule], + skipped: [], errors: [ { message: 'mocked validation message', @@ -264,6 +270,7 @@ describe('Perform bulk action route', () => { summary: { failed: 3, succeeded: 2, + skipped: 0, total: 5, }, errors: [ @@ -333,6 +340,7 @@ describe('Perform bulk action route', () => { attributes: { summary: { failed: 1, + skipped: 0, succeeded: 1, total: 2, }, @@ -355,6 +363,133 @@ describe('Perform bulk action route', () => { }); }); + describe('rule skipping', () => { + it('returns partial failure error with skipped rules if some rule updates fail and others are skipped', async () => { + clients.rulesClient.bulkEdit.mockResolvedValue({ + rules: [mockRule, mockRule], + skipped: [ + { id: 'skipped-rule-id-1', name: 'Skipped Rule 1', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skipped-rule-id-2', name: 'Skipped Rule 2', skip_reason: 'RULE_NOT_MODIFIED' }, + ], + errors: [ + { + message: 'test failure', + rule: { id: 'failed-rule-id-3', name: 'Detect Root/Admin Users' }, + }, + ], + total: 5, + }); + + const response = await server.inject( + getBulkActionEditRequest(), + requestContextMock.convertContext(context) + ); + + expect(response.status).toEqual(500); + expect(response.body).toEqual({ + attributes: { + summary: { + failed: 1, + skipped: 2, + succeeded: 2, + total: 5, + }, + errors: [ + { + message: 'test failure', + rules: [ + { + id: 'failed-rule-id-3', + name: 'Detect Root/Admin Users', + }, + ], + status_code: 500, + }, + ], + results: someBulkActionResults(), + }, + message: 'Bulk edit partially failed', + status_code: 500, + }); + }); + + it('returns success with skipped rules if some rules are skipped, but no errors are reported', async () => { + clients.rulesClient.bulkEdit.mockResolvedValue({ + rules: [mockRule, mockRule], + skipped: [ + { id: 'skipped-rule-id-1', name: 'Skipped Rule 1', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skipped-rule-id-2', name: 'Skipped Rule 2', skip_reason: 'RULE_NOT_MODIFIED' }, + ], + errors: [], + total: 4, + }); + + const response = await server.inject( + getBulkActionEditRequest(), + requestContextMock.convertContext(context) + ); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + attributes: { + summary: { + failed: 0, + skipped: 2, + succeeded: 2, + total: 4, + }, + results: someBulkActionResults(), + }, + rules_count: 4, + success: true, + }); + }); + + it('returns 500 with skipped rules if some rules are skipped, but some errors are reported', async () => { + clients.rulesClient.bulkEdit.mockResolvedValue({ + rules: [mockRule, mockRule], + skipped: [ + { id: 'skipped-rule-id-1', name: 'Skipped Rule 1', skip_reason: 'RULE_NOT_MODIFIED' }, + { id: 'skipped-rule-id-2', name: 'Skipped Rule 2', skip_reason: 'RULE_NOT_MODIFIED' }, + ], + errors: [ + { + message: 'test failure', + rule: { id: 'failed-rule-id-3', name: 'Detect Root/Admin Users' }, + }, + ], + total: 5, + }); + + const response = await server.inject( + getBulkActionEditRequest(), + requestContextMock.convertContext(context) + ); + + expect(response.status).toEqual(500); + expect(response.body).toEqual({ + attributes: { + summary: { + failed: 1, + skipped: 2, + succeeded: 2, + total: 5, + }, + results: someBulkActionResults(), + errors: [ + { + message: 'test failure', + rules: [{ id: 'failed-rule-id-3', name: 'Detect Root/Admin Users' }], + status_code: 500, + }, + ], + }, + message: 'Bulk edit partially failed', + status_code: 500, + }); + }); + }); + describe('request validation', () => { it('rejects payloads with no action', async () => { const request = requestMock.create({ @@ -504,7 +639,7 @@ describe('Perform bulk action route', () => { success: true, rules_count: rulesNumber, attributes: { - summary: { failed: 0, succeeded: rulesNumber, total: rulesNumber }, + summary: { failed: 0, skipped: 0, succeeded: rulesNumber, total: rulesNumber }, results: someBulkActionResults(), }, }) @@ -517,5 +652,6 @@ function someBulkActionResults() { created: expect.any(Array), deleted: expect.any(Array), updated: expect.any(Array), + skipped: expect.any(Array), }; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts index 3e884113f2c84..82660cba2a870 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts @@ -8,13 +8,17 @@ import { truncate } from 'lodash'; import moment from 'moment'; import { BadRequestError, transformError } from '@kbn/securitysolution-es-utils'; -import type { KibanaResponseFactory, Logger, SavedObjectsClientContract } from '@kbn/core/server'; +import type { + IKibanaResponse, + KibanaResponseFactory, + Logger, + SavedObjectsClientContract, +} from '@kbn/core/server'; import type { RulesClient, BulkOperationError } from '@kbn/alerting-plugin/server'; -import type { SanitizedRule } from '@kbn/alerting-plugin/common'; +import type { SanitizedRule, BulkActionSkipResult } from '@kbn/alerting-plugin/common'; import { AbortError } from '@kbn/kibana-utils-plugin/common'; import type { RuleAlertType, RuleParams } from '../../../../rule_schema'; - import type { BulkActionsDryRunErrCode } from '../../../../../../../common/constants'; import { DETECTION_ENGINE_RULES_BULK_ACTION, @@ -26,6 +30,13 @@ import { PerformBulkActionRequestBody, PerformBulkActionRequestQuery, } from '../../../../../../../common/detection_engine/rule_management/api/rules/bulk_actions/request_schema'; +import type { + NormalizedRuleError, + RuleDetailsInError, + BulkEditActionResponse, + BulkEditActionResults, + BulkEditActionSummary, +} from '../../../../../../../common/detection_engine/rule_management/api/rules/bulk_actions/response_schema'; import type { SetupPlugins } from '../../../../../../plugin'; import type { SecuritySolutionPluginRouter } from '../../../../../../types'; import { buildRouteValidation } from '../../../../../../utils/build_validation/route_validation'; @@ -56,18 +67,7 @@ const MAX_RULES_TO_PROCESS_TOTAL = 10000; const MAX_ERROR_MESSAGE_LENGTH = 1000; const MAX_ROUTE_CONCURRENCY = 5; -interface RuleDetailsInError { - id: string; - name?: string; -} -interface NormalizedRuleError { - message: string; - status_code: number; - err_code?: BulkActionsDryRunErrCode; - rules: RuleDetailsInError[]; -} - -type BulkActionError = +export type BulkActionError = | PromisePoolError | PromisePoolError | BulkOperationError; @@ -124,61 +124,66 @@ const buildBulkResponse = ( updated = [], created = [], deleted = [], + skipped = [], }: { isDryRun?: boolean; errors?: BulkActionError[]; updated?: RuleAlertType[]; created?: RuleAlertType[]; deleted?: RuleAlertType[]; + skipped?: BulkActionSkipResult[]; } -) => { +): IKibanaResponse => { const numSucceeded = updated.length + created.length + deleted.length; + const numSkipped = skipped.length; const numFailed = errors.length; - const summary = { + + const summary: BulkEditActionSummary = { failed: numFailed, succeeded: numSucceeded, - total: numSucceeded + numFailed, + skipped: numSkipped, + total: numSucceeded + numFailed + numSkipped, }; // if response is for dry_run, empty lists of rules returned, as rules are not actually updated and stored within ES // thus, it's impossible to return reliably updated/duplicated/deleted rules - const results = isDryRun + const results: BulkEditActionResults = isDryRun ? { updated: [], created: [], deleted: [], + skipped: [], } : { updated: updated.map((rule) => internalRuleToAPIResponse(rule)), created: created.map((rule) => internalRuleToAPIResponse(rule)), deleted: deleted.map((rule) => internalRuleToAPIResponse(rule)), + skipped, }; if (numFailed > 0) { - return response.custom({ + return response.custom({ headers: { 'content-type': 'application/json' }, - body: Buffer.from( - JSON.stringify({ - message: summary.succeeded > 0 ? 'Bulk edit partially failed' : 'Bulk edit failed', - status_code: 500, - attributes: { - errors: normalizeErrorResponse(errors), - results, - summary, - }, - }) - ), + body: { + message: summary.succeeded > 0 ? 'Bulk edit partially failed' : 'Bulk edit failed', + status_code: 500, + attributes: { + errors: normalizeErrorResponse(errors), + results, + summary, + }, + }, statusCode: 500, }); } - return response.ok({ - body: { - success: true, - rules_count: summary.total, - attributes: { results, summary }, - }, - }); + const responseBody: BulkEditActionResponse = { + success: true, + rules_count: summary.total, + attributes: { results, summary }, + }; + + return response.ok({ body: responseBody }); }; const fetchRulesByQueryOrIds = async ({ @@ -340,7 +345,7 @@ export const performBulkActionRoute = ( // handling this action before switch statement as bulkEditRules fetch rules within // rulesClient method, hence there is no need to use fetchRulesByQueryOrIds utility if (body.action === BulkActionType.edit && !isDryRun) { - const { rules, errors } = await bulkEditRules({ + const { rules, errors, skipped } = await bulkEditRules({ rulesClient, filter: query, ids: body.ids, @@ -371,6 +376,7 @@ export const performBulkActionRoute = ( updated: migrationOutcome.results .filter(({ result }) => result) .map(({ result }) => result), + skipped, errors: [...errors, ...migrationOutcome.errors], }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts index 89abe357bb58b..6660ce730f84b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts @@ -7,7 +7,6 @@ import type { BulkOperationError, RulesClient } from '@kbn/alerting-plugin/server'; import pMap from 'p-map'; - import { MAX_RULES_TO_UPDATE_IN_PARALLEL, NOTIFICATION_THROTTLE_NO_ACTIONS, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.test.ts index ee4b19f12d81b..6d65665713ade 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.test.ts @@ -45,23 +45,23 @@ describe('ruleParamsModifier', () => { } as RuleAlertType['params']; test('should increment version if rule is custom (immutable === false)', () => { - const editedRuleParams = ruleParamsModifier(ruleParamsMock, [ + const { modifiedParams } = ruleParamsModifier(ruleParamsMock, [ { type: BulkActionEditType.add_index_patterns, value: ['my-index-*'], }, ]); - expect(editedRuleParams).toHaveProperty('version', ruleParamsMock.version + 1); + expect(modifiedParams).toHaveProperty('version', ruleParamsMock.version + 1); }); test('should not increment version if rule is prebuilt (immutable === true)', () => { - const editedRuleParams = ruleParamsModifier({ ...ruleParamsMock, immutable: true }, [ + const { modifiedParams } = ruleParamsModifier({ ...ruleParamsMock, immutable: true }, [ { type: BulkActionEditType.add_index_patterns, value: ['my-index-*'], }, ]); - expect(editedRuleParams).toHaveProperty('version', ruleParamsMock.version); + expect(modifiedParams).toHaveProperty('version', ruleParamsMock.version); }); describe('index_patterns', () => { @@ -73,6 +73,7 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToAdd: ['index-2-*', 'index-3-*'], resultingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], + isParamsUpdateSkipped: true, }, ], [ @@ -87,6 +88,7 @@ describe('ruleParamsModifier', () => { 'index-4-*', 'index-5-*', ], + isParamsUpdateSkipped: false, }, ], [ @@ -101,6 +103,7 @@ describe('ruleParamsModifier', () => { 'index-4-*', 'index-5-*', ], + isParamsUpdateSkipped: false, }, ], [ @@ -109,12 +112,21 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToAdd: [], resultingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], + isParamsUpdateSkipped: true, }, ], ])( 'should add index patterns to rule, case:"%s"', - (caseName, { existingIndexPatterns, indexPatternsToAdd, resultingIndexPatterns }) => { - const editedRuleParams = ruleParamsModifier( + ( + caseName, + { + existingIndexPatterns, + indexPatternsToAdd, + resultingIndexPatterns, + isParamsUpdateSkipped, + } + ) => { + const { modifiedParams, isParamsUpdateSkipped: isUpdateSkipped } = ruleParamsModifier( { ...ruleParamsMock, index: existingIndexPatterns } as RuleAlertType['params'], [ { @@ -123,7 +135,8 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('index', resultingIndexPatterns); + expect(modifiedParams).toHaveProperty('index', resultingIndexPatterns); + expect(isParamsUpdateSkipped).toBe(isUpdateSkipped); } ); }); @@ -136,6 +149,7 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToDelete: ['index-2-*', 'index-3-*'], resultingIndexPatterns: ['index-1-*'], + isParamsUpdateSkipped: false, }, ], [ @@ -144,6 +158,7 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToDelete: ['index-4-*', 'index-5-*'], resultingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], + isParamsUpdateSkipped: true, }, ], [ @@ -152,6 +167,7 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToDelete: ['index-3-*', 'index-4-*', 'index-5-*'], resultingIndexPatterns: ['index-1-*', 'index-2-*'], + isParamsUpdateSkipped: false, }, ], [ @@ -160,12 +176,21 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToDelete: [], resultingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], + isParamsUpdateSkipped: true, }, ], ])( 'should delete index patterns from rule, case:"%s"', - (caseName, { existingIndexPatterns, indexPatternsToDelete, resultingIndexPatterns }) => { - const editedRuleParams = ruleParamsModifier( + ( + caseName, + { + existingIndexPatterns, + indexPatternsToDelete, + resultingIndexPatterns, + isParamsUpdateSkipped, + } + ) => { + const { modifiedParams, isParamsUpdateSkipped: isUpdateSkipped } = ruleParamsModifier( { ...ruleParamsMock, index: existingIndexPatterns } as RuleAlertType['params'], [ { @@ -174,7 +199,8 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('index', resultingIndexPatterns); + expect(modifiedParams).toHaveProperty('index', resultingIndexPatterns); + expect(isParamsUpdateSkipped).toBe(isUpdateSkipped); } ); }); @@ -187,6 +213,7 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToOverwrite: ['index-2-*', 'index-3-*'], resultingIndexPatterns: ['index-2-*', 'index-3-*'], + isParamsUpdateSkipped: false, }, ], [ @@ -195,6 +222,7 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToOverwrite: ['index-4-*', 'index-5-*'], resultingIndexPatterns: ['index-4-*', 'index-5-*'], + isParamsUpdateSkipped: false, }, ], [ @@ -203,12 +231,21 @@ describe('ruleParamsModifier', () => { existingIndexPatterns: ['index-1-*', 'index-2-*', 'index-3-*'], indexPatternsToOverwrite: ['index-3-*', 'index-4-*', 'index-5-*'], resultingIndexPatterns: ['index-3-*', 'index-4-*', 'index-5-*'], + isParamsUpdateSkipped: false, }, ], ])( 'should overwrite index patterns in rule, case:"%s"', - (caseName, { existingIndexPatterns, indexPatternsToOverwrite, resultingIndexPatterns }) => { - const editedRuleParams = ruleParamsModifier( + ( + caseName, + { + existingIndexPatterns, + indexPatternsToOverwrite, + resultingIndexPatterns, + isParamsUpdateSkipped, + } + ) => { + const { modifiedParams, isParamsUpdateSkipped: isUpdateSkipped } = ruleParamsModifier( { ...ruleParamsMock, index: existingIndexPatterns } as RuleAlertType['params'], [ { @@ -217,14 +254,16 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('index', resultingIndexPatterns); + expect(modifiedParams).toHaveProperty('index', resultingIndexPatterns); + expect(isParamsUpdateSkipped).toBe(isUpdateSkipped); } ); }); test('should return undefined index patterns on remove action if rule has dataViewId only', () => { const testDataViewId = 'test-data-view-id'; - const editedRuleParams = ruleParamsModifier( + + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier( { dataViewId: testDataViewId } as RuleAlertType['params'], [ { @@ -233,12 +272,12 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).not.toHaveProperty('index'); - expect(editedRuleParams).toHaveProperty('dataViewId', testDataViewId); + expect(modifiedParams).not.toHaveProperty('index'); + expect(isParamsUpdateSkipped).toBe(true); }); test('should set dataViewId to undefined if overwrite_data_views=true on set_index_patterns action', () => { - const editedRuleParams = ruleParamsModifier( + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier( { dataViewId: 'test-data-view', index: ['test-*'] } as RuleAlertType['params'], [ { @@ -248,11 +287,12 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('dataViewId', undefined); + expect(modifiedParams).toHaveProperty('dataViewId', undefined); + expect(isParamsUpdateSkipped).toBe(false); }); test('should set dataViewId to undefined if overwrite_data_views=true on add_index_patterns action', () => { - const editedRuleParams = ruleParamsModifier( + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier( { dataViewId: 'test-data-view', index: ['test-*'] } as RuleAlertType['params'], [ { @@ -262,11 +302,12 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('dataViewId', undefined); + expect(modifiedParams).toHaveProperty('dataViewId', undefined); + expect(isParamsUpdateSkipped).toBe(false); }); test('should set dataViewId to undefined if overwrite_data_views=true on delete_index_patterns action', () => { - const editedRuleParams = ruleParamsModifier( + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier( { dataViewId: 'test-data-view', index: ['test-*', 'index'] } as RuleAlertType['params'], [ { @@ -276,12 +317,13 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('dataViewId', undefined); - expect(editedRuleParams).toHaveProperty('index', ['test-*']); + expect(modifiedParams).toHaveProperty('dataViewId', undefined); + expect(modifiedParams).toHaveProperty('index', ['test-*']); + expect(isParamsUpdateSkipped).toBe(false); }); test('should set dataViewId to undefined and index to undefined if overwrite_data_views=true on delete_index_patterns action and rule had no index patterns to begin with', () => { - const editedRuleParams = ruleParamsModifier( + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier( { dataViewId: 'test-data-view', index: undefined } as RuleAlertType['params'], [ { @@ -291,8 +333,9 @@ describe('ruleParamsModifier', () => { }, ] ); - expect(editedRuleParams).toHaveProperty('dataViewId', undefined); - expect(editedRuleParams).toHaveProperty('index', undefined); + expect(modifiedParams).toHaveProperty('dataViewId', undefined); + expect(modifiedParams).toHaveProperty('index', undefined); + expect(isParamsUpdateSkipped).toBe(false); }); test('should throw error on adding index pattern if rule is of machine learning type', () => { @@ -337,7 +380,7 @@ describe('ruleParamsModifier', () => { describe('timeline', () => { test('should set timeline', () => { - const editedRuleParams = ruleParamsModifier(ruleParamsMock, [ + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier(ruleParamsMock, [ { type: BulkActionEditType.set_timeline, value: { @@ -347,8 +390,9 @@ describe('ruleParamsModifier', () => { }, ]); - expect(editedRuleParams.timelineId).toBe('91832785-286d-4ebe-b884-1a208d111a70'); - expect(editedRuleParams.timelineTitle).toBe('Test timeline'); + expect(modifiedParams.timelineId).toBe('91832785-286d-4ebe-b884-1a208d111a70'); + expect(modifiedParams.timelineTitle).toBe('Test timeline'); + expect(isParamsUpdateSkipped).toBe(false); }); }); @@ -357,7 +401,7 @@ describe('ruleParamsModifier', () => { const INTERVAL_IN_MINUTES = 5; const LOOKBACK_IN_MINUTES = 1; const FROM_IN_SECONDS = (INTERVAL_IN_MINUTES + LOOKBACK_IN_MINUTES) * 60; - const editedRuleParams = ruleParamsModifier(ruleParamsMock, [ + const { modifiedParams, isParamsUpdateSkipped } = ruleParamsModifier(ruleParamsMock, [ { type: BulkActionEditType.set_schedule, value: { @@ -368,11 +412,12 @@ describe('ruleParamsModifier', () => { ]); // eslint-disable-next-line @typescript-eslint/no-explicit-any - expect((editedRuleParams as any).interval).toBeUndefined(); - expect(editedRuleParams.meta).toStrictEqual({ + expect((modifiedParams as any).interval).toBeUndefined(); + expect(modifiedParams.meta).toStrictEqual({ from: '1m', }); - expect(editedRuleParams.from).toBe(`now-${FROM_IN_SECONDS}s`); + expect(modifiedParams.from).toBe(`now-${FROM_IN_SECONDS}s`); + expect(isParamsUpdateSkipped).toBe(false); }); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts index 9f5f252b9fd86..e92b6e6ab0163 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/rule_params_modifier.ts @@ -5,12 +5,14 @@ * 2.0. */ -/* eslint-disable complexity */ - import moment from 'moment'; import { parseInterval } from '@kbn/data-plugin/common/search/aggs/utils/date_interval_utils'; +import type { RuleParamsModifierResult } from '@kbn/alerting-plugin/server/rules_client/methods/bulk_edit'; import type { RuleAlertType } from '../../../rule_schema'; -import type { BulkActionEditForRuleParams } from '../../../../../../common/detection_engine/rule_management/api/rules/bulk_actions/request_schema'; +import type { + BulkActionEditForRuleParams, + BulkActionEditPayloadIndexPatterns, +} from '../../../../../../common/detection_engine/rule_management/api/rules/bulk_actions/request_schema'; import { BulkActionEditType } from '../../../../../../common/detection_engine/rule_management/api/rules/bulk_actions/request_schema'; import { invariant } from '../../../../../../common/utils/invariant'; @@ -22,22 +24,70 @@ export const deleteItemsFromArray = (arr: T[], items: T[]): T[] => { return arr.filter((item) => !itemsSet.has(item)); }; +// Check if current params have a configured data view id +// and the action is not set to overwrite data views +const isDataViewExistsAndNotOverriden = ( + dataViewId: string | undefined, + action: BulkActionEditPayloadIndexPatterns +) => dataViewId != null && !action.overwrite_data_views; + +// Check if the index patterns added to the rule already exist in it +const hasIndexPatterns = ( + indexPatterns: string[] | undefined, + action: BulkActionEditPayloadIndexPatterns +) => action.value.every((indexPattern) => indexPatterns?.includes(indexPattern)); + +// Check if the index patterns to be deleted don't exist in the rule +const hasNotIndexPattern = ( + indexPatterns: string[] | undefined, + action: BulkActionEditPayloadIndexPatterns +) => action.value.every((indexPattern) => !indexPatterns?.includes(indexPattern)); + +const shouldSkipIndexPatternsBulkAction = ( + indexPatterns: string[] | undefined, + dataViewId: string | undefined, + action: BulkActionEditPayloadIndexPatterns +) => { + if (isDataViewExistsAndNotOverriden(dataViewId, action)) { + return true; + } + + if (action.type === BulkActionEditType.add_index_patterns) { + return hasIndexPatterns(indexPatterns, action); + } + + if (action.type === BulkActionEditType.delete_index_patterns) { + return hasNotIndexPattern(indexPatterns, action); + } + + return false; +}; + const applyBulkActionEditToRuleParams = ( existingRuleParams: RuleAlertType['params'], action: BulkActionEditForRuleParams -): RuleAlertType['params'] => { +): { + ruleParams: RuleAlertType['params']; + isActionSkipped: boolean; +} => { let ruleParams = { ...existingRuleParams }; + // If the action is succesfully applied and the rule params are modified, + // we update the following flag to false. As soon as the current function + // returns this flag as false, at least once, for any action, we know that + // the rule needs to be marked as having its params updated. + let isActionSkipped = false; switch (action.type) { // index_patterns actions // index pattern is not present in machine learning rule type, so we throw error on it - case BulkActionEditType.add_index_patterns: + case BulkActionEditType.add_index_patterns: { invariant( ruleParams.type !== 'machine_learning', "Index patterns can't be added. Machine learning rule doesn't have index patterns property" ); - if (ruleParams.dataViewId != null && !action.overwrite_data_views) { + if (shouldSkipIndexPatternsBulkAction(ruleParams.index, ruleParams.dataViewId, action)) { + isActionSkipped = true; break; } @@ -47,14 +97,18 @@ const applyBulkActionEditToRuleParams = ( ruleParams.index = addItemsToArray(ruleParams.index ?? [], action.value); break; - - case BulkActionEditType.delete_index_patterns: + } + case BulkActionEditType.delete_index_patterns: { invariant( ruleParams.type !== 'machine_learning', "Index patterns can't be deleted. Machine learning rule doesn't have index patterns property" ); - if (ruleParams.dataViewId != null && !action.overwrite_data_views) { + if ( + !action.overwrite_data_views && + shouldSkipIndexPatternsBulkAction(ruleParams.index, ruleParams.dataViewId, action) + ) { + isActionSkipped = true; break; } @@ -66,14 +120,15 @@ const applyBulkActionEditToRuleParams = ( ruleParams.index = deleteItemsFromArray(ruleParams.index, action.value); } break; - - case BulkActionEditType.set_index_patterns: + } + case BulkActionEditType.set_index_patterns: { invariant( ruleParams.type !== 'machine_learning', "Index patterns can't be overwritten. Machine learning rule doesn't have index patterns property" ); - if (ruleParams.dataViewId != null && !action.overwrite_data_views) { + if (shouldSkipIndexPatternsBulkAction(ruleParams.index, ruleParams.dataViewId, action)) { + isActionSkipped = true; break; } @@ -83,16 +138,17 @@ const applyBulkActionEditToRuleParams = ( ruleParams.index = action.value; break; - + } // timeline actions - case BulkActionEditType.set_timeline: + case BulkActionEditType.set_timeline: { ruleParams = { ...ruleParams, timelineId: action.value.timeline_id || undefined, timelineTitle: action.value.timeline_title || undefined, }; - break; + break; + } // update look-back period in from and meta.from fields case BulkActionEditType.set_schedule: { const interval = parseInterval(action.value.interval) ?? moment.duration(0); @@ -108,26 +164,35 @@ const applyBulkActionEditToRuleParams = ( }, from: `now-${from}s`, }; + + break; } } - return ruleParams; + return { ruleParams, isActionSkipped }; }; /** * takes list of bulkEdit actions and apply them to rule.params by mutating it * @param existingRuleParams * @param actions - * @returns mutated params + * @returns mutated params, isParamsUpdateSkipped flag */ export const ruleParamsModifier = ( existingRuleParams: RuleAlertType['params'], actions: BulkActionEditForRuleParams[] -) => { - const modifiedParams = actions.reduce( - (acc, action) => ({ ...acc, ...applyBulkActionEditToRuleParams(acc, action) }), - existingRuleParams - ); +): RuleParamsModifierResult => { + let isParamsUpdateSkipped = true; + + const modifiedParams = actions.reduce((acc, action) => { + const { ruleParams, isActionSkipped } = applyBulkActionEditToRuleParams(acc, action); + + // The rule was updated with at least one action, so mark our rule as updated + if (!isActionSkipped) { + isParamsUpdateSkipped = false; + } + return { ...acc, ...ruleParams }; + }, existingRuleParams); // increment version even if actions are empty, as attributes can be modified as well outside of ruleParamsModifier // version must not be modified for immutable rule. Otherwise prebuilt rules upgrade flow will be broken @@ -135,5 +200,5 @@ export const ruleParamsModifier = ( modifiedParams.version += 1; } - return modifiedParams; + return { modifiedParams, isParamsUpdateSkipped }; }; diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index f9c4359d9285f..c6c3752bfc724 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -26923,9 +26923,7 @@ "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.addRuleActions.infoCalloutTitle": "Configurer les actions pour {rulesCount, plural, one {# règle que vous avez sélectionnée} other {# règles que vous avez sélectionnées}}", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.addRuleActions.warningCalloutMessage": "Vous êtes sur le point d'écraser les actions de règle pour {rulesCount, plural, one {# règle sélectionnée} other {# règles sélectionnées}}. Cliquez sur {saveButton} pour appliquer les modifications.", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.applyTimelineTemplate.warningCalloutMessage": "Vous êtes sur le point d'appliquer des modifications à {rulesCount, plural, one {# règle sélectionnée} other {# règles sélectionnées}}. Si vous avez déjà appliqué des modèles de chronologie à ces règles, ils seront remplacés ou (si vous sélectionnez \"Aucun\") réinitialisés.", - "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.errorToastDescription": "Impossible de mettre à jour {rulesCount, plural, =1 {# règle} other {# règles}}.", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.setSchedule.warningCalloutMessage": "Vous êtes sur le point d'appliquer des modifications à {rulesCount, plural, one {# règle sélectionnée} other {# règles sélectionnées}}. Les modifications que vous effectuez écraseront les planifications existantes de la règle et le temps de récupération supplémentaire (le cas échéant).", - "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.successToastDescription": "Vous avez correctement mis à jour {rulesCount, plural, =1 {# règle} other {# règles}}", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.elasticRulesEditDescription": "{rulesCount, plural, =1 {# règle Elastic prédéfinie} other {# règles Elastic prédéfinies}} (modification des règles prédéfinies non prise en charge)", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.elasticRulesExportDescription": "{rulesCount, plural, =1 {# règle Elastic prédéfinie} other {# règles Elastic prédéfinies}} (exportation des règles prédéfinies non prise en charge)", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.enable.errorToastDescription": "Impossible d'activer {rulesCount, plural, =1 {# règle} other {# règles}}.", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 2925afe9e3a1f..95aef8c8ef76a 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -26898,9 +26898,7 @@ "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.addRuleActions.infoCalloutTitle": "選択した{rulesCount, plural, other {# ルール}}のアクションを設定", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.addRuleActions.warningCalloutMessage": "{rulesCount, plural, other {# 個の選択したルール}}のルールを上書きしようとしています。{saveButton}をクリックすると、変更が適用されます。", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.applyTimelineTemplate.warningCalloutMessage": "{rulesCount, plural, other {# 個の選択したルール}}に変更を適用しようとしています。以前にタイムラインテンプレートをこれらのルールに適用している場合は、上書きされるか、（［なし］を選択した場合は）［なし］にリセットされます。", - "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.errorToastDescription": "{rulesCount, plural, other {#個のルール}}を更新できませんでした。", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.setSchedule.warningCalloutMessage": "{rulesCount, plural, other {# 個の選択したルール}}に変更を適用しようとしています。行った変更は、既存のルールスケジュールと追加の振り返り時間（該当する場合）を上書きします。", - "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.successToastDescription": "{rulesCount, plural, other {#個のルール}}を正常に更新しました", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.elasticRulesEditDescription": "{rulesCount, plural, other {# 個の既製のElasticルール}}（既製のルールは編集できません）", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.elasticRulesExportDescription": "{rulesCount, plural, other {# 個の既製のElasticルール}}（既製のルールはエクスポートできません）", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.enable.errorToastDescription": "{rulesCount, plural, other {#個のルール}}を有効にできませんでした。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index aac0c566a6732..8acf63ed7d3e8 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -26931,9 +26931,7 @@ "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.addRuleActions.infoCalloutTitle": "为您选择的 {rulesCount, plural, other {# 个规则}}配置操作", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.addRuleActions.warningCalloutMessage": "您即将覆盖 {rulesCount, plural, other {# 个选定规则}}的规则操作。单击 {saveButton} 以应用更改。", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.applyTimelineTemplate.warningCalloutMessage": "您即将对 {rulesCount, plural, other {# 个选定规则}}应用更改。如果之前已将时间线模板应用于这些规则，则会将其覆盖或重置为无（如果您选择了“无”）。", - "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.errorToastDescription": "无法更新 {rulesCount, plural, other {# 个规则}}。", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.setSchedule.warningCalloutMessage": "您即将对 {rulesCount, plural, other {# 个选定规则}}应用更改。您所做的更改将覆盖现有规则计划和其他回查时间（如有）。", - "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.edit.successToastDescription": "您已成功更新 {rulesCount, plural, other {# 个规则}}", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.elasticRulesEditDescription": "{rulesCount, plural, other {# 个预构建的 Elastic 规则}}（不支持编辑预构建的规则）", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.elasticRulesExportDescription": "{rulesCount, plural, other {# 个预构建的 Elastic 规则}}（不支持导出预构建的规则）", "xpack.securitySolution.detectionEngine.rules.allRules.bulkActions.enable.errorToastDescription": "无法启用 {rulesCount, plural, other {# 个规则}}。", diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_edit.ts b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_edit.ts index 53ee50a27012c..df53acd6acdbd 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_edit.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/group3/tests/alerting/bulk_edit.ts @@ -102,6 +102,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { }, ], rules: [], + skipped: [], total: 1, }); expect(response.statusCode).to.eql(200); @@ -241,7 +242,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { break; case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': - expect(response.body).to.eql({ errors: [], rules: [], total: 0 }); + expect(response.body).to.eql({ errors: [], rules: [], skipped: [], total: 0 }); expect(response.statusCode).to.eql(200); break; case 'global_read at space1': @@ -382,7 +383,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { break; case 'space_1_all at space1': case 'space_1_all_alerts_none_actions at space1': - expect(response.body).to.eql({ errors: [], rules: [], total: 0 }); + expect(response.body).to.eql({ errors: [], rules: [], skipped: [], total: 0 }); expect(response.statusCode).to.eql(200); break; case 'global_read at space1': @@ -587,7 +588,7 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { switch (scenario.id) { case 'superuser at space1': case 'global_read at space1': - expect(response.body).to.eql({ rules: [], errors: [], total: 0 }); + expect(response.body).to.eql({ rules: [], skipped: [], errors: [], total: 0 }); expect(response.statusCode).to.eql(200); break; case 'no_kibana_privileges at space1': diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts index 48117cc08ecf6..06e55fdfdda1b 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts @@ -118,7 +118,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkActionType.delete }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the deleted rule is returned with the response expect(body.attributes.results.deleted[0].name).to.eql(testRule.name); @@ -153,7 +153,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkActionType.delete }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the deleted rule is returned with the response expect(body.attributes.results.deleted[0].name).to.eql(rule1.name); @@ -174,7 +174,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkActionType.enable }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].enabled).to.eql(true); @@ -210,7 +210,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkActionType.enable }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].enabled).to.eql(true); @@ -243,7 +243,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkActionType.disable }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].enabled).to.eql(false); @@ -279,7 +279,7 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkActionType.disable }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].enabled).to.eql(false); @@ -317,7 +317,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the duplicated rule is returned with the response expect(body.attributes.results.created[0].name).to.eql(`${ruleToDuplicate.name} [Duplicate]`); @@ -363,7 +363,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the duplicated rule is returned with the response expect(body.attributes.results.created[0].name).to.eql(`${ruleToDuplicate.name} [Duplicate]`); @@ -452,6 +452,7 @@ export default ({ getService }: FtrProviderContext): void => { expect(bulkEditResponse.attributes.summary).to.eql({ failed: 0, + skipped: 0, succeeded: 1, total: 1, }); @@ -473,30 +474,12 @@ export default ({ getService }: FtrProviderContext): void => { tagsToDelete: ['tag1', 'tag2'], resultingTags: ['tag3'], }, - { - caseName: '3 existing tags - 2 other tags(none of them) = 3 tags', - existingTags: ['tag1', 'tag2', 'tag3'], - tagsToDelete: ['tag4', 'tag5'], - resultingTags: ['tag1', 'tag2', 'tag3'], - }, { caseName: '3 existing tags - 1 of them - 2 other tags(none of them) = 2 tags', existingTags: ['tag1', 'tag2', 'tag3'], tagsToDelete: ['tag3', 'tag4', 'tag5'], resultingTags: ['tag1', 'tag2'], }, - { - caseName: '3 existing tags - 0 tags = 3 tags', - existingTags: ['tag1', 'tag2', 'tag3'], - tagsToDelete: [], - resultingTags: ['tag1', 'tag2', 'tag3'], - }, - { - caseName: '0 existing tags - 2 tags = 0 tags', - existingTags: [], - tagsToDelete: ['tag4', 'tag5'], - resultingTags: [], - }, { caseName: '3 existing tags - 3 of them = 0 tags', existingTags: ['tag1', 'tag2', 'tag3'], @@ -526,6 +509,7 @@ export default ({ getService }: FtrProviderContext): void => { expect(bulkEditResponse.attributes.summary).to.eql({ failed: 0, + skipped: 0, succeeded: 1, total: 1, }); @@ -541,12 +525,6 @@ export default ({ getService }: FtrProviderContext): void => { }); const addTagsCases = [ - { - caseName: '3 existing tags + 2 of them = 3 tags', - existingTags: ['tag1', 'tag2', 'tag3'], - addedTags: ['tag1', 'tag2'], - resultingTags: ['tag1', 'tag2', 'tag3'], - }, { caseName: '3 existing tags + 2 other tags(none of them) = 5 tags', existingTags: ['tag1', 'tag2', 'tag3'], @@ -565,12 +543,6 @@ export default ({ getService }: FtrProviderContext): void => { addedTags: ['tag4', 'tag5'], resultingTags: ['tag4', 'tag5'], }, - { - caseName: '3 existing tags + 0 tags = 3 tags', - existingTags: ['tag1', 'tag2', 'tag3'], - addedTags: [], - resultingTags: ['tag1', 'tag2', 'tag3'], - }, ]; addTagsCases.forEach(({ caseName, existingTags, addedTags, resultingTags }) => { @@ -593,6 +565,7 @@ export default ({ getService }: FtrProviderContext): void => { expect(bulkEditResponse.attributes.summary).to.eql({ failed: 0, + skipped: 0, succeeded: 1, total: 1, }); @@ -606,6 +579,86 @@ export default ({ getService }: FtrProviderContext): void => { expect(updatedRule.tags).to.eql(resultingTags); }); }); + + const skipTagsUpdateCases = [ + // Delete no-ops + { + caseName: '3 existing tags - 0 tags = 3 tags', + existingTags: ['tag1', 'tag2', 'tag3'], + tagsToUpdate: [], + resultingTags: ['tag1', 'tag2', 'tag3'], + operation: BulkActionEditType.delete_tags, + }, + { + caseName: '0 existing tags - 2 tags = 0 tags', + existingTags: [], + tagsToUpdate: ['tag4', 'tag5'], + resultingTags: [], + operation: BulkActionEditType.delete_tags, + }, + { + caseName: '3 existing tags - 2 other tags (none of them) = 3 tags', + existingTags: ['tag1', 'tag2', 'tag3'], + tagsToUpdate: ['tag4', 'tag5'], + resultingTags: ['tag1', 'tag2', 'tag3'], + operation: BulkActionEditType.delete_tags, + }, + // Add no-ops + { + caseName: '3 existing tags + 2 of them = 3 tags', + existingTags: ['tag1', 'tag2', 'tag3'], + tagsToUpdate: ['tag1', 'tag2'], + resultingTags: ['tag1', 'tag2', 'tag3'], + operation: BulkActionEditType.add_tags, + }, + { + caseName: '3 existing tags + 0 tags = 3 tags', + existingTags: ['tag1', 'tag2', 'tag3'], + tagsToUpdate: [], + resultingTags: ['tag1', 'tag2', 'tag3'], + operation: BulkActionEditType.add_tags, + }, + ]; + + skipTagsUpdateCases.forEach( + ({ caseName, existingTags, tagsToUpdate, resultingTags, operation }) => { + it(`should skip rule updated for tags, case: "${caseName}"`, async () => { + const ruleId = 'ruleId'; + + await createRule(supertest, log, { ...getSimpleRule(ruleId), tags: existingTags }); + + const { body: bulkEditResponse } = await postBulkAction() + .send({ + query: '', + action: BulkActionType.edit, + [BulkActionType.edit]: [ + { + type: operation, + value: tagsToUpdate, + }, + ], + }) + .expect(200); + + expect(bulkEditResponse.attributes.summary).to.eql({ + failed: 0, + skipped: 1, + succeeded: 0, + total: 1, + }); + + // Check that the rules is returned as skipped with expected skip reason + expect(bulkEditResponse.attributes.results.skipped[0].skip_reason).to.eql( + 'RULE_NOT_MODIFIED' + ); + + // Check that the no changes have been persisted + const { body: updatedRule } = await fetchRule(ruleId).expect(200); + + expect(updatedRule.tags).to.eql(resultingTags); + }); + } + ); }); describe('index patterns actions', () => { @@ -626,7 +679,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(bulkEditResponse.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(bulkEditResponse.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updated rule is returned with the response expect(bulkEditResponse.attributes.results.updated[0].index).to.eql(['initial-index-*']); @@ -656,7 +714,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(bulkEditResponse.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(bulkEditResponse.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updated rule is returned with the response expect(bulkEditResponse.attributes.results.updated[0].index).to.eql( @@ -688,7 +751,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(bulkEditResponse.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(bulkEditResponse.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updated rule is returned with the response expect(bulkEditResponse.attributes.results.updated[0].index).to.eql( @@ -717,7 +785,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Index patterns can't be added. Machine learning rule doesn't have index patterns property", @@ -750,7 +818,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Mutated params invalid: Index patterns can't be empty", status_code: 500, @@ -783,7 +851,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Mutated params invalid: Index patterns can't be empty", status_code: 500, @@ -800,6 +868,95 @@ export default ({ getService }: FtrProviderContext): void => { expect(reFetchedRule.index).to.eql(['simple-index-*']); }); + + const skipIndexPatternsUpdateCases = [ + // Delete no-ops + { + caseName: '3 existing indeces - 0 indeces = 3 indeces', + existingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + indexPatternsToUpdate: [], + resultingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + operation: BulkActionEditType.delete_index_patterns, + }, + { + caseName: '0 existing indeces - 2 indeces = 0 indeces', + existingIndexPatterns: [], + indexPatternsToUpdate: ['index1-*', 'index2-*'], + resultingIndexPatterns: [], + operation: BulkActionEditType.delete_index_patterns, + }, + { + caseName: '3 existing indeces - 2 other indeces (none of them) = 3 indeces', + existingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + indexPatternsToUpdate: ['index8-*', 'index9-*'], + resultingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + operation: BulkActionEditType.delete_index_patterns, + }, + // Add no-ops + { + caseName: '3 existing indeces + 2 exisiting indeces= 3 indeces', + existingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + indexPatternsToUpdate: ['index1-*', 'index2-*'], + resultingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + operation: BulkActionEditType.add_index_patterns, + }, + { + caseName: '3 existing indeces + 0 indeces = 3 indeces', + existingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + indexPatternsToUpdate: [], + resultingIndexPatterns: ['index1-*', 'index2-*', 'index3-*'], + operation: BulkActionEditType.add_index_patterns, + }, + ]; + + skipIndexPatternsUpdateCases.forEach( + ({ + caseName, + existingIndexPatterns, + indexPatternsToUpdate, + resultingIndexPatterns, + operation, + }) => { + it(`should skip rule updated for tags, case: "${caseName}"`, async () => { + const ruleId = 'ruleId'; + + await createRule(supertest, log, { + ...getSimpleRule(ruleId), + index: existingIndexPatterns, + }); + + const { body: bulkEditResponse } = await postBulkAction() + .send({ + query: '', + action: BulkActionType.edit, + [BulkActionType.edit]: [ + { + type: operation, + value: indexPatternsToUpdate, + }, + ], + }) + .expect(200); + + expect(bulkEditResponse.attributes.summary).to.eql({ + failed: 0, + skipped: 1, + succeeded: 0, + total: 1, + }); + + // Check that the rules is returned as skipped with expected skip reason + expect(bulkEditResponse.attributes.results.skipped[0].skip_reason).to.eql( + 'RULE_NOT_MODIFIED' + ); + + // Check that the no changes have been persisted + const { body: updatedRule } = await fetchRule(ruleId).expect(200); + + expect(updatedRule.index).to.eql(resultingIndexPatterns); + }); + } + ); }); it('should migrate legacy actions on edit', async () => { @@ -837,7 +994,12 @@ export default ({ getService }: FtrProviderContext): void => { ], }); - expect(setTagsBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(setTagsBody.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updates have been persisted const { body: setTagsRule } = await fetchRule(ruleId).expect(200); @@ -883,7 +1045,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].timeline_id).to.eql(timelineId); @@ -926,7 +1088,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].timeline_id).to.be(undefined); @@ -955,7 +1117,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Index patterns can't be added. Machine learning rule doesn't have index patterns property", @@ -988,7 +1150,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Mutated params invalid: Index patterns can't be empty", status_code: 500, @@ -1078,7 +1240,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ + failed: 1, + skipped: 0, + succeeded: 0, + total: 1, + }); expect(body.attributes.errors[0]).to.eql({ message: "Elastic rule can't be edited", status_code: 500, @@ -1606,7 +1773,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ + failed: 1, + skipped: 0, + succeeded: 0, + total: 1, + }); expect(body.attributes.errors[0]).to.eql({ message: "Elastic rule can't be edited", status_code: 500, @@ -1834,7 +2006,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); expect(body.attributes.results.updated[0].interval).to.eql(interval); expect(body.attributes.results.updated[0].meta).to.eql({ from: `${lookbackMinutes}m` }); @@ -1870,7 +2042,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(setIndexBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(setIndexBody.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updated rule is returned with the response expect(setIndexBody.attributes.results.updated[0].index).to.eql(['initial-index-*']); @@ -1882,15 +2059,15 @@ export default ({ getService }: FtrProviderContext): void => { expect(setIndexRule.index).to.eql(['initial-index-*']); }); - it('should NOT add an index pattern to a rule and overwrite the data view when overwrite_data_views is false', async () => { + it('should return skipped rule and NOT add an index pattern to a rule or overwrite the data view when overwrite_data_views is false', async () => { const ruleId = 'ruleId'; const dataViewId = 'index1-*'; - const simpleRule = { + + const simpleRule = await createRule(supertest, log, { ...getSimpleRule(ruleId), index: undefined, data_view_id: dataViewId, - }; - await createRule(supertest, log, simpleRule); + }); const { body: setIndexBody } = await postBulkAction() .send({ @@ -1906,13 +2083,21 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(setIndexBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(setIndexBody.attributes.summary).to.eql({ + failed: 0, + skipped: 1, + succeeded: 0, + total: 1, + }); - // Check that the updated rule is returned with the response - expect(setIndexBody.attributes.results.updated[0].index).to.eql(undefined); - expect(setIndexBody.attributes.results.updated[0].data_view_id).to.eql(dataViewId); + expect(setIndexBody.attributes.errors).to.be(undefined); - // Check that the updates have been persisted + // Check that the skipped rule is returned with the response + expect(setIndexBody.attributes.results.skipped[0].id).to.eql(simpleRule.id); + expect(setIndexBody.attributes.results.skipped[0].name).to.eql(simpleRule.name); + expect(setIndexBody.attributes.results.skipped[0].skip_reason).to.eql('RULE_NOT_MODIFIED'); + + // Check that the rule has not been updated const { body: setIndexRule } = await fetchRule(ruleId).expect(200); expect(setIndexRule.index).to.eql(undefined); @@ -1943,7 +2128,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(setIndexBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(setIndexBody.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updated rule is returned with the response expect(setIndexBody.attributes.results.updated[0].index).to.eql(['initial-index-*']); @@ -1979,7 +2169,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Mutated params invalid: Index patterns can't be empty", status_code: 500, @@ -1992,15 +2182,14 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should NOT set an index pattern to a rule and overwrite the data view when overwrite_data_views is false', async () => { + it('should return skipped rule and NOT set an index pattern to a rule or overwrite the data view when overwrite_data_views is false', async () => { const ruleId = 'ruleId'; const dataViewId = 'index1-*'; - const simpleRule = { + const simpleRule = await createRule(supertest, log, { ...getSimpleRule(ruleId), index: undefined, data_view_id: dataViewId, - }; - await createRule(supertest, log, simpleRule); + }); const { body: setIndexBody } = await postBulkAction() .send({ @@ -2016,13 +2205,21 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(setIndexBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(setIndexBody.attributes.summary).to.eql({ + failed: 0, + skipped: 1, + succeeded: 0, + total: 1, + }); - // Check that the updated rule is returned with the response - expect(setIndexBody.attributes.results.updated[0].index).to.eql(undefined); - expect(setIndexBody.attributes.results.updated[0].data_view_id).to.eql(dataViewId); + expect(setIndexBody.attributes.errors).to.be(undefined); - // Check that the updates have been persisted + // Check that the skipped rule is returned with the response + expect(setIndexBody.attributes.results.skipped[0].id).to.eql(simpleRule.id); + expect(setIndexBody.attributes.results.skipped[0].name).to.eql(simpleRule.name); + expect(setIndexBody.attributes.results.skipped[0].skip_reason).to.eql('RULE_NOT_MODIFIED'); + + // Check that the rule has not been updated const { body: setIndexRule } = await fetchRule(ruleId).expect(200); expect(setIndexRule.index).to.eql(undefined); @@ -2031,7 +2228,7 @@ export default ({ getService }: FtrProviderContext): void => { // This rule will now not have a source defined - as has been the behavior of rules since the beginning // this rule will use the default index patterns on rule run - it('should NOT error if all index patterns removed from a rule with data views when no index patterns exist on the rule and overwrite_data_views is true', async () => { + it('should be successful on an attempt to remove index patterns from a rule with only a dataView (no index patterns exist on the rule), if overwrite_data_views is true', async () => { const dataViewId = 'index1-*'; const ruleId = 'ruleId'; const rule = await createRule(supertest, log, { @@ -2054,7 +2251,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ + failed: 0, + skipped: 0, + succeeded: 1, + total: 1, + }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].index).to.eql(undefined); @@ -2090,7 +2292,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).to.eql({ failed: 1, succeeded: 0, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); expect(body.attributes.errors[0]).to.eql({ message: "Mutated params invalid: Index patterns can't be empty", status_code: 500, @@ -2103,7 +2305,7 @@ export default ({ getService }: FtrProviderContext): void => { }); }); - it('should NOT return error if all index patterns removed from a rule with data views and overwrite_data_views is false', async () => { + it('should return a skipped rule if all index patterns removed from a rule with data views and overwrite_data_views is false', async () => { const dataViewId = 'index1-*'; const ruleId = 'ruleId'; const rule = await createRule(supertest, log, { @@ -2126,17 +2328,134 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 1, succeeded: 0, total: 1 }); + expect(body.attributes.errors).to.be(undefined); + + // Check that the skipped rule is returned with the response + expect(body.attributes.results.skipped[0].id).to.eql(rule.id); + expect(body.attributes.results.skipped[0].name).to.eql(rule.name); + expect(body.attributes.results.skipped[0].skip_reason).to.eql('RULE_NOT_MODIFIED'); + }); + }); + + describe('multiple_actions', () => { + it('should return one updated rule when applying two valid operations on a rule', async () => { + const ruleId = 'ruleId'; + const rule = await createRule(supertest, log, { + ...getSimpleRule(ruleId), + index: ['index1-*'], + tags: ['tag1', 'tag2'], + }); + + const { body } = await postBulkAction() + .send({ + ids: [rule.id], + action: BulkActionType.edit, + [BulkActionType.edit]: [ + { + type: BulkActionEditType.add_index_patterns, + value: ['initial-index-*'], + }, + { + type: BulkActionEditType.add_tags, + value: ['tag3'], + }, + ], + }) + .expect(200); + + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response - expect(body.attributes.results.updated[0].index).to.eql(['simple-index-*']); - expect(body.attributes.results.updated[0].data_view_id).to.eql(dataViewId); + expect(body.attributes.results.updated[0].tags).to.eql(['tag1', 'tag2', 'tag3']); + expect(body.attributes.results.updated[0].index).to.eql(['index1-*', 'initial-index-*']); - // Check that the updates have been persisted - const { body: setIndexRule } = await fetchRule(ruleId).expect(200); + // Check that the rule has been persisted + const { body: updatedRule } = await fetchRule(ruleId).expect(200); - expect(setIndexRule.index).to.eql(['simple-index-*']); - expect(setIndexRule.data_view_id).to.eql(dataViewId); + expect(updatedRule.index).to.eql(['index1-*', 'initial-index-*']); + expect(updatedRule.tags).to.eql(['tag1', 'tag2', 'tag3']); + }); + + it('should return one updated rule when applying one valid operation and one operation to be skipped on a rule', async () => { + const ruleId = 'ruleId'; + const rule = await createRule(supertest, log, { + ...getSimpleRule(ruleId), + index: ['index1-*'], + tags: ['tag1', 'tag2'], + }); + + const { body } = await postBulkAction() + .send({ + ids: [rule.id], + action: BulkActionType.edit, + [BulkActionType.edit]: [ + // Valid operation + { + type: BulkActionEditType.add_index_patterns, + value: ['initial-index-*'], + }, + // Operation to be skipped + { + type: BulkActionEditType.add_tags, + value: ['tag1'], + }, + ], + }) + .expect(200); + + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); + + // Check that the updated rule is returned with the response + expect(body.attributes.results.updated[0].tags).to.eql(['tag1', 'tag2']); + expect(body.attributes.results.updated[0].index).to.eql(['index1-*', 'initial-index-*']); + + // Check that the rule has been persisted + const { body: updatedRule } = await fetchRule(ruleId).expect(200); + + expect(updatedRule.index).to.eql(['index1-*', 'initial-index-*']); + expect(updatedRule.tags).to.eql(['tag1', 'tag2']); + }); + + it('should return one skipped rule when two (all) operations result in a no-op', async () => { + const ruleId = 'ruleId'; + const rule = await createRule(supertest, log, { + ...getSimpleRule(ruleId), + index: ['index1-*'], + tags: ['tag1', 'tag2'], + }); + + const { body } = await postBulkAction() + .send({ + ids: [rule.id], + action: BulkActionType.edit, + [BulkActionType.edit]: [ + // Operation to be skipped + { + type: BulkActionEditType.add_index_patterns, + value: ['index1-*'], + }, + // Operation to be skipped + { + type: BulkActionEditType.add_tags, + value: ['tag1'], + }, + ], + }) + .expect(200); + + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 1, succeeded: 0, total: 1 }); + + // Check that the skipped rule is returned with the response + expect(body.attributes.results.skipped[0].name).to.eql(rule.name); + expect(body.attributes.results.skipped[0].id).to.eql(rule.id); + expect(body.attributes.results.skipped[0].skip_reason).to.eql('RULE_NOT_MODIFIED'); + + // Check that no change to the rule have been persisted + const { body: skippedRule } = await fetchRule(ruleId).expect(200); + + expect(skippedRule.index).to.eql(['index1-*']); + expect(skippedRule.tags).to.eql(['tag1', 'tag2']); }); }); @@ -2192,7 +2511,7 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // Check that the updated rule is returned with the response expect(body.attributes.results.updated[0].timeline_id).to.eql(timelineId); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action_dry_run.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action_dry_run.ts index 7914edf10247a..1996dc2438580 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action_dry_run.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action_dry_run.ts @@ -26,9 +26,9 @@ import { // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { - const es = getService('es'); const supertest = getService('supertest'); const log = getService('log'); + const es = getService('es'); const postDryRunBulkAction = () => supertest @@ -74,9 +74,14 @@ export default ({ getService }: FtrProviderContext): void => { .send({ action: BulkActionType.delete }) .expect(200); - expect(body.attributes.summary).toEqual({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).toEqual({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // dry_run mode shouldn't return any rules in results - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); // Check that rule wasn't deleted await fetchRule(ruleId).expect(200); @@ -90,9 +95,14 @@ export default ({ getService }: FtrProviderContext): void => { .send({ action: BulkActionType.enable }) .expect(200); - expect(body.attributes.summary).toEqual({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).toEqual({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // dry_run mode shouldn't return any rules in results - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); // Check that the updates have not been persisted const { body: ruleBody } = await fetchRule(ruleId).expect(200); @@ -107,9 +117,14 @@ export default ({ getService }: FtrProviderContext): void => { .send({ action: BulkActionType.disable }) .expect(200); - expect(body.attributes.summary).toEqual({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).toEqual({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // dry_run mode shouldn't return any rules in results - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); // Check that the updates have not been persisted const { body: ruleBody } = await fetchRule(ruleId).expect(200); @@ -125,9 +140,14 @@ export default ({ getService }: FtrProviderContext): void => { .send({ action: BulkActionType.disable }) .expect(200); - expect(body.attributes.summary).toEqual({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).toEqual({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // dry_run mode shouldn't return any rules in results - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); // Check that the rule wasn't duplicated const { body: rulesResponse } = await findRules().expect(200); @@ -153,9 +173,14 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body.attributes.summary).toEqual({ failed: 0, succeeded: 1, total: 1 }); + expect(body.attributes.summary).toEqual({ failed: 0, skipped: 0, succeeded: 1, total: 1 }); // dry_run mode shouldn't return any rules in results - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); // Check that the updates have not been persisted const { body: ruleBody } = await fetchRule(ruleId).expect(200); @@ -184,8 +209,13 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).toEqual({ failed: 1, succeeded: 0, total: 1 }); - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.summary).toEqual({ failed: 1, skipped: 0, succeeded: 0, total: 1 }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); expect(body.attributes.errors).toHaveLength(1); expect(body.attributes.errors[0]).toEqual({ @@ -225,8 +255,18 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(500); - expect(body.attributes.summary).toEqual({ failed: 1, succeeded: 0, total: 1 }); - expect(body.attributes.results).toEqual({ updated: [], created: [], deleted: [] }); + expect(body.attributes.summary).toEqual({ + failed: 1, + skipped: 0, + succeeded: 0, + total: 1, + }); + expect(body.attributes.results).toEqual({ + updated: [], + skipped: [], + created: [], + deleted: [], + }); expect(body.attributes.errors).toHaveLength(1); expect(body.attributes.errors[0]).toEqual({