From b7cfe02f93195e3cf0492cd8a5d799446c6e9230 Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Wed, 13 May 2026 09:51:22 +0200 Subject: [PATCH 1/8] Add rulesClient.bulkCreate(), optionally delaying rulesClient.bulkEnableTasks() --- .../rule_circuit_breaker_error_message.ts | 11 +- .../bulk_create/bulk_create_rules.test.ts | 639 ++++++++++++++++++ .../methods/bulk_create/bulk_create_rules.ts | 308 +++++++++ .../rule/methods/bulk_create/index.ts | 9 + .../rule/methods/bulk_create/types.ts | 71 ++ .../rule/methods/bulk_create/utils.ts | 386 +++++++++++ .../methods/bulk_enable/bulk_enable_rules.ts | 55 +- .../bulk_enable_tasks/bulk_enable_tasks.ts | 57 ++ .../rule/methods/bulk_enable_tasks/index.ts | 10 + .../types/bulk_enable_tasks_types.ts | 14 + .../methods/bulk_enable_tasks/types/index.ts | 8 + .../plugins/shared/alerting/server/index.ts | 5 + .../alerting/server/rules_client.mock.ts | 4 + .../server/rules_client/rules_client.ts | 8 + 14 files changed, 1532 insertions(+), 53 deletions(-) create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/index.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts create mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts diff --git a/x-pack/platform/plugins/shared/alerting/common/rule_circuit_breaker_error_message.ts b/x-pack/platform/plugins/shared/alerting/common/rule_circuit_breaker_error_message.ts index 7779b1a5dd60b..729dd5cdb6ead 100644 --- a/x-pack/platform/plugins/shared/alerting/common/rule_circuit_breaker_error_message.ts +++ b/x-pack/platform/plugins/shared/alerting/common/rule_circuit_breaker_error_message.ts @@ -47,6 +47,12 @@ const getBulkEnableRuleErrorSummary = () => { }); }; +const getBulkCreateRuleErrorSummary = () => { + return i18n.translate('xpack.alerting.ruleCircuitBreaker.error.bulkCreateSummary', { + defaultMessage: `Rules cannot be bulk created. The maximum number of runs per minute would be exceeded.`, + }); +}; + const getRuleCircuitBreakerErrorDetail = ({ interval, intervalAvailable, @@ -84,7 +90,7 @@ export const getRuleCircuitBreakerErrorMessage = ({ name?: string; interval: number; intervalAvailable: number; - action: 'update' | 'create' | 'enable' | 'bulkEdit' | 'bulkEnable'; + action: 'update' | 'create' | 'enable' | 'bulkEdit' | 'bulkEnable' | 'bulkCreate'; rules?: number; }) => { let errorMessageSummary: string; @@ -105,6 +111,9 @@ export const getRuleCircuitBreakerErrorMessage = ({ case 'bulkEnable': errorMessageSummary = getBulkEnableRuleErrorSummary(); break; + case 'bulkCreate': + errorMessageSummary = getBulkCreateRuleErrorSummary(); + break; } return `Error validating circuit breaker - ${errorMessageSummary} - ${getRuleCircuitBreakerErrorDetail( diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts new file mode 100644 index 0000000000000..ea631aad4cd6f --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts @@ -0,0 +1,639 @@ +/* + * 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 { + coreFeatureFlagsMock, + loggingSystemMock, + savedObjectsClientMock, + savedObjectsRepositoryMock, + uiSettingsServiceMock, +} from '@kbn/core/server/mocks'; +import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks'; +import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks'; +import { actionsAuthorizationMock } from '@kbn/actions-plugin/server/mocks'; +import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks'; +import type { ActionsAuthorization, ActionsClient } from '@kbn/actions-plugin/server'; +import { createMockConnector } from '@kbn/actions-plugin/server/application/connector/mocks'; +import { ruleTypeRegistryMock } from '../../../../rule_type_registry.mock'; +import { alertingAuthorizationMock } from '../../../../authorization/alerting_authorization.mock'; +import type { AlertingAuthorization } from '../../../../authorization/alerting_authorization'; +import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry'; +import { backfillClientMock } from '../../../../backfill_client/backfill_client.mock'; +import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; +import { getBeforeSetup, setGlobalDate } from '../../../../rules_client/tests/lib'; +import type { ConstructorOptions } from '../../../../rules_client/rules_client'; +import { RulesClient } from '../../../../rules_client/rules_client'; +import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; +import { validateScheduleLimit } from '../get_schedule_frequency'; +import { RuleAuditAction } from '../../../../rules_client/common/audit_events'; + +const BULK_CREATE_AS_DISABLED_PREFIX = 'Rule created in a disabled state: '; + +jest.mock('../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({ + bulkMarkApiKeysForInvalidation: jest.fn(), +})); + +jest.mock('../get_schedule_frequency', () => ({ + validateScheduleLimit: jest.fn(), +})); + +jest.mock('@kbn/core-saved-objects-utils-server', () => { + const actual = jest.requireActual('@kbn/core-saved-objects-utils-server'); + return { + ...actual, + SavedObjectsUtils: { + generateId: jest.fn(), + }, + }; +}); + +import { SavedObjectsUtils } from '@kbn/core-saved-objects-utils-server'; + +const taskManager = taskManagerMock.createStart(); +const ruleTypeRegistry = ruleTypeRegistryMock.create(); +const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); +const encryptedSavedObjects = encryptedSavedObjectsMock.createClient(); +const authorization = alertingAuthorizationMock.create(); +const actionsAuthorization = actionsAuthorizationMock.create(); +const auditLogger = auditLoggerMock.create(); +const internalSavedObjectsRepository = savedObjectsRepositoryMock.create(); + +const rulesClientParams: jest.Mocked = { + taskManager, + ruleTypeRegistry, + unsecuredSavedObjectsClient, + authorization: authorization as unknown as AlertingAuthorization, + actionsAuthorization: actionsAuthorization as unknown as ActionsAuthorization, + spaceId: 'default', + namespace: 'default', + getUserName: jest.fn(), + createAPIKey: jest.fn(), + logger: loggingSystemMock.create().get(), + internalSavedObjectsRepository, + encryptedSavedObjectsClient: encryptedSavedObjects, + getActionsClient: jest.fn(), + getEventLogClient: jest.fn(), + kibanaVersion: 'v8.0.0', + auditLogger, + maxScheduledPerMinute: 10000, + minimumScheduleInterval: { value: '1m', enforce: false }, + isAuthenticationTypeAPIKey: jest.fn(), + getAuthenticationAPIKey: jest.fn(), + getAlertIndicesAlias: jest.fn(), + alertsService: null, + backfillClient: backfillClientMock.create(), + connectorAdapterRegistry: new ConnectorAdapterRegistry(), + isSystemAction: jest.fn(), + uiSettings: uiSettingsServiceMock.createStartContract(), + featureFlags: coreFeatureFlagsMock.createStart(), + isServerless: false, +}; + +setGlobalDate(); + +const baseRule = (overrides: Record = {}) => ({ + enabled: false, + name: 'r', + tags: [], + alertTypeId: '123', + consumer: 'bar', + schedule: { interval: '1m' }, + throttle: null, + notifyWhen: null, + params: { foo: true }, + actions: [], + ...overrides, +}); + +const buildBulkResponse = ( + rules: Array<{ id: string; error?: { message: string; statusCode: number } }> +) => ({ + saved_objects: rules.map((r) => ({ + id: r.id, + type: RULE_SAVED_OBJECT_TYPE, + references: [], + ...(r.error + ? { error: { ...r.error, error: 'Conflict' } } + : { + attributes: { + alertTypeId: '123', + name: `name-${r.id}`, + enabled: false, + consumer: 'bar', + schedule: { interval: '1m' }, + params: { foo: true }, + actions: [], + createdBy: 'elastic', + updatedBy: 'elastic', + createdAt: '2019-02-12T21:01:22.479Z', + updatedAt: '2019-02-12T21:01:22.479Z', + snoozeSchedule: [], + muteAll: false, + mutedInstanceIds: [], + executionStatus: { status: 'pending', lastExecutionDate: '2019-02-12T21:01:22.479Z' }, + revision: 0, + running: false, + apiKey: null, + apiKeyOwner: null, + apiKeyCreatedByUser: null, + }, + }), + })) as never, +}); + +describe('bulkCreateRules', () => { + let rulesClient: RulesClient; + let actionsClient: jest.Mocked; + + beforeEach(async () => { + getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry); + (auditLogger.log as jest.Mock).mockClear(); + (bulkMarkApiKeysForInvalidation as jest.Mock).mockReset(); + (validateScheduleLimit as jest.Mock).mockReset(); + let counter = 0; + (SavedObjectsUtils.generateId as jest.Mock).mockImplementation(() => `mock-id-${++counter}`); + rulesClient = new RulesClient(rulesClientParams); + actionsClient = (await rulesClientParams.getActionsClient()) as jest.Mocked; + actionsClient.getBulk.mockResolvedValue([ + createMockConnector({ id: '1', actionTypeId: 'test', name: 'a' }), + ]); + actionsClient.listTypes.mockResolvedValue([]); + actionsClient.isSystemAction.mockReturnValue(false); + rulesClientParams.getActionsClient.mockResolvedValue(actionsClient); + rulesClientParams.createAPIKey.mockResolvedValue({ + apiKeysEnabled: true, + result: { id: 'key-id', name: 'key', api_key: 'key-value' } as never, + }); + rulesClientParams.isAuthenticationTypeAPIKey.mockReturnValue(false); + taskManager.bulkSchedule.mockImplementation(async (tasks) => tasks as never); + taskManager.bulkEnable.mockResolvedValue({ tasks: [], errors: [] } as never); + }); + + test('returns empty result for empty input without touching SO/TM/key clients', async () => { + const result = await rulesClient.bulkCreateRules({ rules: [] }); + expect(result).toEqual({ rules: [], errors: [], total: 0, taskIdsFailedToBeEnabled: [] }); + expect(unsecuredSavedObjectsClient.bulkCreate).not.toHaveBeenCalled(); + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + expect(rulesClientParams.createAPIKey).not.toHaveBeenCalled(); + }); + + test('all-disabled happy path: single bulkCreate, no TM, no API keys', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }, { data: baseRule({ name: 'b' }) }], + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); + expect(rulesClientParams.createAPIKey).not.toHaveBeenCalled(); + expect(result.errors).toEqual([]); + expect(result.total).toBe(2); + expect(result.rules).toHaveLength(2); + expect(result.taskIdsFailedToBeEnabled).toEqual([]); + }); + + test('all-enabled happy path: API keys, bulkSchedule, bulkCreate, bulkEnable', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b', enabled: true }) }, + ], + }); + + expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(2); + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); + const scheduledIds = (taskManager.bulkSchedule.mock.calls[0][0] as Array<{ id: string }>).map( + (t) => t.id + ); + expect(scheduledIds).toEqual(['mock-id-1', 'mock-id-2']); + // tasks are scheduled disabled; bulkEnable flips them on + expect( + (taskManager.bulkSchedule.mock.calls[0][0] as Array<{ enabled: boolean }>).every( + (t) => t.enabled === false + ) + ).toBe(true); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(taskManager.bulkEnable).toHaveBeenCalledWith(['mock-id-1', 'mock-id-2']); + expect(result.errors).toEqual([]); + expect(result.total).toBe(2); + expect(result.rules).toHaveLength(2); + expect(result.taskIdsFailedToBeEnabled).toEqual([]); + }); + + describe('skipTaskEnabling', () => { + test('skips taskManager.bulkEnable and returns enabled task ids in taskIdsFailedToBeEnabled', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b', enabled: true }) }, + ], + skipTaskEnabling: true, + }); + + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); + expect(result.taskIdsFailedToBeEnabled).toEqual(['mock-id-1', 'mock-id-2']); + }); + + test('returns empty taskIdsFailedToBeEnabled when no rule is enabled', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }], + skipTaskEnabling: true, + }); + + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); + expect(result.taskIdsFailedToBeEnabled).toEqual([]); + }); + + test('excludes per-row SO failure ids from taskIdsFailedToBeEnabled', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([ + { id: 'mock-id-1' }, + { id: 'mock-id-2', error: { message: '409', statusCode: 409 } }, + ]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b', enabled: true }) }, + ], + skipTaskEnabling: true, + }); + + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); + expect(result.taskIdsFailedToBeEnabled).toEqual(['mock-id-1']); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.id).toBe('mock-id-2'); + }); + + test('empty-input early return returns standard shape', async () => { + const result = await rulesClient.bulkCreateRules({ rules: [], skipTaskEnabling: true }); + expect(result).toEqual({ + rules: [], + errors: [], + total: 0, + taskIdsFailedToBeEnabled: [], + }); + }); + }); + + test('mixed enabled+disabled happy path', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a', enabled: true }) }, { data: baseRule({ name: 'b' }) }], + }); + + expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(1); + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); + expect(taskManager.bulkSchedule.mock.calls[0][0]).toHaveLength(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(taskManager.bulkEnable).toHaveBeenCalledWith(['mock-id-1']); + expect(result.errors).toEqual([]); + expect(result.total).toBe(2); + }); + + test('Phase 1 per-rule throw is isolated', async () => { + let calls = 0; + ruleTypeRegistry.get.mockImplementation((typeId: string) => { + calls += 1; + if (calls === 1) throw new Error('rule type not found'); + return { + id: typeId, + name: 'Test', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + async executor() { + return { state: {} }; + }, + category: 'test', + producer: 'alerts', + solution: 'stack', + validate: { params: { validate: (p: unknown) => p } }, + validLegacyConsumers: [], + } as never; + }); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'fails' }) }, { data: baseRule({ name: 'ok' }) }], + }); + + expect(result.total).toBe(2); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.name).toBe('fails'); + expect(result.rules).toHaveLength(1); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(1); + }); + + test('Phase 1 API key creation failure: enabled rule degrades to disabled, no key minted, SO still written, no scheduling', async () => { + rulesClientParams.createAPIKey.mockRejectedValueOnce(new Error('keys disabled')); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'enabled-keyfail', enabled: true }) }, + { data: baseRule({ name: 'enabled-ok', enabled: true }) }, + ], + }); + + // Both rules persisted in a single SO bulkCreate. The first one is demoted + // to disabled because its key mint failed; the second is scheduled normally. + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); + const persistedAttrsByName = new Map( + ( + unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0] as Array<{ + attributes: { name: string; enabled: boolean; apiKey: string | null }; + }> + ).map((o) => [o.attributes.name, o.attributes]) + ); + expect(persistedAttrsByName.get('enabled-keyfail')?.enabled).toBe(false); + expect(persistedAttrsByName.get('enabled-keyfail')?.apiKey).toBeNull(); + expect(persistedAttrsByName.get('enabled-ok')?.enabled).toBe(true); + // Only the surviving enabled rule was scheduled / enabled. + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); + expect(taskManager.bulkSchedule.mock.calls[0][0]).toHaveLength(1); + expect(taskManager.bulkEnable).toHaveBeenCalledWith(['mock-id-2']); + // No key mint succeeded for the demoted rule, so nothing to invalidate. + expect(bulkMarkApiKeysForInvalidation).not.toHaveBeenCalled(); + expect(result.rules).toHaveLength(2); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toEqual( + expect.objectContaining({ + disabledReason: 'api_key_creation_failed', + rule: expect.objectContaining({ name: 'enabled-keyfail' }), + }) + ); + expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); + expect(result.errors[0].message).toContain('keys disabled'); + }); + + test('Phase 2 schedule-limit trip: enabled rule degrades to disabled, key invalidated, SO still written for both', async () => { + (validateScheduleLimit as jest.Mock).mockResolvedValue({ + interval: 100, + intervalAvailable: 50, + }); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'enabled', enabled: true }) }, + { data: baseRule({ name: 'disabled' }) }, + ], + }); + + // The enabled rule contributed an API key → must be invalidated + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalled(); + // Both rules persisted in a single SO bulkCreate; the originally-enabled + // one is degraded to disabled (no scheduledTaskId, no API key fields). + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); + const persistedAttrsByName = new Map( + ( + unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0] as Array<{ + attributes: { name: string; enabled: boolean; scheduledTaskId?: string | null }; + }> + ).map((o) => [o.attributes.name, o.attributes]) + ); + expect(persistedAttrsByName.get('enabled')?.enabled).toBe(false); + expect(persistedAttrsByName.get('enabled')?.scheduledTaskId).toBeUndefined(); + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + expect(result.rules).toHaveLength(2); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toEqual( + expect.objectContaining({ + disabledReason: 'schedule_limit_exceeded', + rule: expect.objectContaining({ name: 'enabled' }), + }) + ); + expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); + }); + + test('Phase 3 whole TM throw: enabled subset degrades to disabled, disabled subset unaffected', async () => { + taskManager.bulkSchedule.mockRejectedValueOnce(new Error('cluster unavailable')); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'enabled', enabled: true }) }, + { data: baseRule({ name: 'disabled' }) }, + ], + }); + + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); + expect(result.rules).toHaveLength(2); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toEqual( + expect.objectContaining({ + disabledReason: 'task_schedule_failed', + rule: expect.objectContaining({ name: 'enabled' }), + }) + ); + expect(result.errors[0].message).toContain('cluster unavailable'); + expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); + }); + + test('Phase 3 silent per-task drop: dropped rule degrades to disabled and SO is still written', async () => { + taskManager.bulkSchedule.mockImplementationOnce(async (tasks) => { + return [tasks[0]] as never; + }); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'kept', enabled: true }) }, + { data: baseRule({ name: 'dropped', enabled: true }) }, + ], + }); + + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); + expect(result.rules).toHaveLength(2); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toEqual( + expect.objectContaining({ + disabledReason: 'task_validation_failed', + rule: expect.objectContaining({ name: 'dropped' }), + }) + ); + expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); + }); + + test('Phase 4 per-row error on Phase-3-scheduled id: per-rule key invalidated, bulkRemove called', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1', error: { message: 'conflict', statusCode: 409 } }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'enabled', enabled: true }) }], + }); + + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); + // Per-row error path now batches TM cleanup into a single bulkRemove. + expect(taskManager.bulkRemove).toHaveBeenCalledWith(['mock-id-1']); + expect(taskManager.removeIfExists).not.toHaveBeenCalled(); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].status).toBe(409); + expect(result.errors[0].disabledReason).toBeUndefined(); + expect(result.rules).toHaveLength(0); + }); + + test('Phase 4 per-row error on caller-supplied id NOT in newlyScheduledTaskIds: NO TM cleanup', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'caller-id', error: { message: 'conflict', statusCode: 409 } }]) + ); + + await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'disabled-collision' }), options: { id: 'caller-id' } }], + }); + + expect(taskManager.removeIfExists).not.toHaveBeenCalled(); + expect(taskManager.bulkRemove).not.toHaveBeenCalled(); + }); + + test('Phase 4 whole-call throw: invalidates every newly-minted key and best-effort bulkRemove of scheduled ids, then rethrows', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockRejectedValueOnce(new Error('SO down')); + + await expect( + rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b', enabled: true }) }, + ], + }) + ).rejects.toThrow('SO down'); + + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); + expect(taskManager.bulkRemove).toHaveBeenCalledWith(['mock-id-1', 'mock-id-2']); + }); + + test('Phase 5 per-task enable error: surfaced in taskIdsFailedToBeEnabled, no SO rollback', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + taskManager.bulkEnable.mockResolvedValueOnce({ + tasks: [], + errors: [{ id: 'mock-id-1', error: { type: 'foo', message: 'no' } }], + } as never); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a', enabled: true }) }], + }); + + expect(result.taskIdsFailedToBeEnabled).toEqual(['mock-id-1']); + expect(result.rules).toHaveLength(1); + }); + + test('every demotion path stamps a machine-readable disabledReason on the error', async () => { + // schedule_limit_exceeded + (validateScheduleLimit as jest.Mock).mockResolvedValueOnce({ + interval: 100, + intervalAvailable: 50, + }); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + const phase2 = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a', enabled: true }) }], + }); + expect(phase2.errors[0]).toEqual( + expect.objectContaining({ disabledReason: 'schedule_limit_exceeded' }) + ); + + // task_schedule_failed + taskManager.bulkSchedule.mockRejectedValueOnce(new Error('tm boom')); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( + buildBulkResponse([{ id: 'mock-id-2' }]) + ); + const phase3a = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'b', enabled: true }) }], + }); + expect(phase3a.errors[0]).toEqual( + expect.objectContaining({ disabledReason: 'task_schedule_failed' }) + ); + + // task_validation_failed (silent drop) + taskManager.bulkSchedule.mockImplementationOnce(async () => [] as never); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( + buildBulkResponse([{ id: 'mock-id-3' }]) + ); + const phase3b = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'c', enabled: true }) }], + }); + expect(phase3b.errors[0]).toEqual( + expect.objectContaining({ disabledReason: 'task_validation_failed' }) + ); + + // api_key_creation_failed + rulesClientParams.createAPIKey.mockRejectedValueOnce(new Error('no keys')); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( + buildBulkResponse([{ id: 'mock-id-4' }]) + ); + const phase1 = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'd', enabled: true }) }], + }); + expect(phase1.errors[0]).toEqual( + expect.objectContaining({ disabledReason: 'api_key_creation_failed' }) + ); + }); + + // Caller-side chunking is covered by import_rules.test.ts; bulkCreateRules handles one batch per call. + + test('emits per-rule CREATE audit event for surviving rules and ENABLE for the enabled subset', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'enabled', enabled: true }) }, + { data: baseRule({ name: 'disabled' }) }, + ], + }); + + const actions = (auditLogger.log as jest.Mock).mock.calls + .map(([event]) => event?.event?.action) + .filter(Boolean); + expect(actions.filter((a) => a === RuleAuditAction.CREATE)).toHaveLength(2); + expect(actions.filter((a) => a === RuleAuditAction.ENABLE)).toHaveLength(1); + }); +}); diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts new file mode 100644 index 0000000000000..63ce9ce803a1e --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -0,0 +1,308 @@ +/* + * 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 pMap from 'p-map'; +import { withSpan } from '@kbn/apm-utils'; +import type { SavedObject, SavedObjectsBulkCreateObject } from '@kbn/core/server'; +import { SavedObjectsUtils } from '@kbn/core/server'; +import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; +import { getRuleCircuitBreakerErrorMessage } from '../../../../../common'; +import { updateMeta } from '../../../../rules_client/lib'; +import { API_KEY_GENERATE_CONCURRENCY } from '../../../../rules_client/common/constants'; +import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; +import { bulkEnableTasks } from '../bulk_enable_tasks'; +import { bulkCreateRulesSo } from '../../../../data/rule'; +import type { RawRule, SanitizedRule } from '../../../../types'; +import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; +import type { RuleParams } from '../../types'; +import { validateScheduleLimit } from '../get_schedule_frequency'; +import type { + ApiKeyEntry, + PreparedRule, + BulkCreateRulesParams, + BulkCreateRulesResult, +} from './types'; +import { + buildTaskInstance, + collectNewKeysToInvalidate, + demotePreparedRules, + flushKeysToInvalidate, + prepareRule, + toSanitizedRule, +} from './utils'; + +export async function bulkCreateRules( + context: RulesClientContext, + params: BulkCreateRulesParams +): Promise> { + const { rules } = params; + const { logger } = context; + const total = rules.length; + + if (total === 0) { + return { rules: [], errors: [], total: 0, taskIdsFailedToBeEnabled: [] }; + } + + const username = await context.getUserName(); + const actionsClient = await context.getActionsClient(); + + const inputsWithIds = rules.map((rule) => ({ + id: rule.options?.id ?? SavedObjectsUtils.generateId(), + rule, + })); + + logger.debug(`Bulk creating batch of ${total} rules`); + + // Phase 1: per-rule prepare (validation + api key generation). + // NOTE: in order to minimise external calls, the values below get mutated + // at different stages in the process (ie if we fail schedule creation), + // so that we invalidate keys and create rule SOs in a single batch at the end. + const preparedRules = new Map(); + const keysToInvalidate = new Set(); + const apiKeysMap = new Map(); + const errors: BulkOperationError[] = []; + const authzCache = new Map>(); + + await pMap( + inputsWithIds, + async ({ id, rule }) => { + const { prepared, error } = await prepareRule({ + context, + actionsClient, + username, + id, + rule, + authzCache, + errors, + apiKeysMap, + }); + if (prepared) preparedRules.set(id, prepared); + else if (error) errors.push(error); + }, + { concurrency: API_KEY_GENERATE_CONCURRENCY } + ); + + // No survivors? Flush out any keys created and return + if (preparedRules.size === 0) { + await flushKeysToInvalidate(keysToInvalidate, context); + return { rules: [], errors, total, taskIdsFailedToBeEnabled: [] }; + } + + // Phase 2: validate schedule-limits, enabled subset only. + const enabled = [...preparedRules.values()].filter((p) => p.enabled); + + if (enabled.length > 0) { + const updatedInterval = enabled.map((r) => r.schedule.interval); + const validationPayload = await validateScheduleLimit({ context, updatedInterval }); + const enabledIds = enabled.map((p) => p.id); + if (validationPayload) { + const reasonMessage = getRuleCircuitBreakerErrorMessage({ + interval: validationPayload.interval, + intervalAvailable: validationPayload.intervalAvailable, + action: 'bulkCreate', + rules: enabledIds.length, + }); + logger.debug(`Demoting ${enabledIds.length} rules -> disabled, schedule limit exceeded.`); + demotePreparedRules({ + ids: enabledIds, + reason: 'schedule_limit_exceeded', + message: reasonMessage, + preparedRules, + apiKeysMap, + keysToInvalidate, + errors, + username, + }); + } + } + + // Phase 3: schedule tasks for the surviving enabled subset. + const survivingEnabled = [...preparedRules.values()].filter((p) => p.enabled); + const newlyScheduledTaskIds = new Set(); + + if (survivingEnabled.length > 0) { + const tasksToSchedule = survivingEnabled.map((preparedRule) => + buildTaskInstance(context, preparedRule) + ); + + let scheduledIds: string[] = []; + const survivingEnabledIds = survivingEnabled.map((p) => p.id); + try { + const scheduledTasks = await withSpan( + { name: 'taskManager.bulkSchedule', type: 'tasks' }, + () => context.taskManager.bulkSchedule(tasksToSchedule) + ); + scheduledIds = scheduledTasks.map((task) => task.id); + } catch (error) { + // Whole-call TM throw: demote enabled subset to disabled, continue. + logger.warn( + `Demoting ${survivingEnabledIds.length} rules -> disabled, task scheduling failed.` + ); + demotePreparedRules({ + ids: survivingEnabledIds, + reason: 'task_schedule_failed', + message: `Failed to schedule tasks: ${error.message}`, + preparedRules, + apiKeysMap, + keysToInvalidate, + errors, + username, + }); + } + + if (scheduledIds.length > 0) { + scheduledIds.forEach((id) => newlyScheduledTaskIds.add(id)); + } + + // Silent per-task drops: bulkSchedule's `taskInstanceToAttributes` validation + // logs+skips invalid instances. Diff requested vs returned and demote the missing ones. + if (preparedRules.size > 0 && scheduledIds.length < survivingEnabledIds.length) { + const returned = new Set(scheduledIds); + const dropped = survivingEnabledIds.filter((id) => !returned.has(id)); + if (dropped.length > 0) { + logger.warn(`Demoting ${dropped.length} rules -> disabled, task validation failed.`); + demotePreparedRules({ + ids: dropped, + reason: 'task_validation_failed', + message: 'Task scheduling silently dropped this rule (validation failure in task store)', + preparedRules, + apiKeysMap, + keysToInvalidate, + errors, + username, + }); + } + } + } + + // Audit per-rule CREATE event before persistence (mirrors createRuleSavedObject). + for (const prepared of preparedRules.values()) { + context.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.CREATE, + outcome: 'unknown', + savedObject: { type: RULE_SAVED_OBJECT_TYPE, id: prepared.id, name: prepared.name }, + }) + ); + } + + // Phase 4: bulk SO create (no overwrite — id collisions surface as per-row 409) + const bulkObjects: Array> = [...preparedRules.values()].map( + (prepared) => ({ + type: RULE_SAVED_OBJECT_TYPE, + id: prepared.id, + attributes: updateMeta(context, prepared.rawRule), + references: prepared.references, + }) + ); + + let bulkResponse; + try { + bulkResponse = await withSpan( + { name: 'unsecuredSavedObjectsClient.bulkCreate', type: 'rules' }, + () => + bulkCreateRulesSo({ + savedObjectsClient: context.unsecuredSavedObjectsClient, + bulkCreateRuleAttributes: bulkObjects, + }) + ); + } catch (error) { + // Whole-call SO failure (auth, timeout, etc): invalidate every newly-minted + // key, best-effort orphan-task cleanup, then rethrow. + for (const k of collectNewKeysToInvalidate(apiKeysMap.values())) keysToInvalidate.add(k); + await flushKeysToInvalidate(keysToInvalidate, context); + if (newlyScheduledTaskIds.size > 0) { + try { + await context.taskManager.bulkRemove([...newlyScheduledTaskIds]); + } catch (cleanupError) { + logger.error( + `bulkCreateRules: failed to clean up tasks ${[...newlyScheduledTaskIds].join( + ', ' + )} after SO bulkCreate threw: ${cleanupError.message}` + ); + } + } + throw error; + } + + // Phase 4 per-row outcomes. + const successfulSos: Array> = []; + const taskIdsToEnable: string[] = []; + const taskIdsToCleanUp: string[] = []; + + for (const so of bulkResponse.saved_objects) { + if (so.error) { + errors.push({ + message: so.error.message ?? 'n/a', + status: so.error.statusCode, + rule: { id: so.id, name: preparedRules.get(so.id)?.name ?? 'n/a' }, + }); + + const apiKey = apiKeysMap.get(so.id); + if (apiKey) { + for (const k of collectNewKeysToInvalidate([apiKey])) keysToInvalidate.add(k); + } + + // Only ids we scheduled in Phase 3. Skipping caller-supplied id collisions + // avoids nuking a pre-existing rule's task on a 409. + if (newlyScheduledTaskIds.has(so.id)) { + taskIdsToCleanUp.push(so.id); + } + } else { + successfulSos.push(so as SavedObject); + if (newlyScheduledTaskIds.has(so.id)) { + taskIdsToEnable.push(so.id); + // Audit per-rule ENABLE for the enabled subset (mirrors single-rule semantics). + context.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.ENABLE, + outcome: 'unknown', + savedObject: { + type: RULE_SAVED_OBJECT_TYPE, + id: so.id, + name: preparedRules.get(so.id)?.name, + }, + }) + ); + } + } + } + + // Single batched TM cleanup for per-row failures. + if (taskIdsToCleanUp.length > 0) { + try { + logger.warn(`Cleaning up ${taskIdsToCleanUp.length} tasks where SO creation failed.`); + await context.taskManager.bulkRemove(taskIdsToCleanUp); + } catch (cleanupError) { + logger.error( + `bulkCreateRules: failed to clean up tasks ${taskIdsToCleanUp.join( + ', ' + )} after SO per-row errors: ${cleanupError.message}` + ); + } + } + + // Phase 5: enable scheduled tasks. If skipTaskEnabling, caller enables them later. + const taskIdsFailedToBeEnabled = params.skipTaskEnabling + ? [...taskIdsToEnable] + : (await bulkEnableTasks(context, { taskIds: taskIdsToEnable })).taskIdsFailedToBeEnabled; + + // Single end-of-function flush for all collected key invalidations. + await flushKeysToInvalidate(keysToInvalidate, context); + + // Phase 6: domain transform + return. + const sanitizedRules: Array> = successfulSos.map((so) => + toSanitizedRule(context, so, context.ruleTypeRegistry) + ); + + return { + rules: sanitizedRules, + errors, + total, + taskIdsFailedToBeEnabled, + }; +} diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/index.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/index.ts new file mode 100644 index 0000000000000..33a030bc4338f --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/index.ts @@ -0,0 +1,9 @@ +/* + * 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. + */ + +export type { BulkCreateRulesItem, BulkCreateRulesParams, BulkCreateRulesResult } from './types'; +export { bulkCreateRules } from './bulk_create_rules'; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts new file mode 100644 index 0000000000000..9f6c04878e737 --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts @@ -0,0 +1,71 @@ +/* + * 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 { SavedObjectReference } from '@kbn/core/server'; +import type { IntervalSchedule } from '../../../../../common'; +import type { RuleParams } from '../../types'; +import type { CreateRuleData } from '../create/types'; +import type { CreateRuleOptions } from '../create/create_rule'; +import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; +import type { RawRule, SanitizedRule } from '../../../../types'; + +export interface PreparedRule { + id: string; + name: string; + enabled: boolean; + rawRule: RawRule; + references: SavedObjectReference[]; + schedule: IntervalSchedule; + consumer: string; + ruleTypeId: string; +} + +export interface ApiKeyEntry { + apiKey: string | null; + uiamApiKey: string | null; + apiKeyCreatedByUser: boolean | null; +} + +export interface PrepareRuleArgs { + context: RulesClientContext; + actionsClient: Awaited>; + username: string | null; + id: string; + rule: BulkCreateRulesItem; + authzCache: Map>; + errors: BulkCreateOperationError[]; + apiKeysMap: Map; +} + +export interface BulkCreateRulesItem { + data: CreateRuleData; + options?: CreateRuleOptions; + allowMissingConnectorSecrets?: boolean; +} + +export interface BulkCreateRulesParams { + rules: Array>; + // If true, skip Phase 5 (taskManager.bulkEnable); + skipTaskEnabling?: boolean; +} + +export type BulkCreateDisabledReason = + | 'api_key_creation_failed' + | 'schedule_limit_exceeded' + | 'task_schedule_failed' + | 'task_validation_failed'; + +export interface BulkCreateOperationError extends BulkOperationError { + disabledReason?: BulkCreateDisabledReason; +} + +export interface BulkCreateRulesResult { + rules: Array>; + errors: BulkCreateOperationError[]; + total: number; + taskIdsFailedToBeEnabled: string[]; +} diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts new file mode 100644 index 0000000000000..5adc4ed38fe4f --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts @@ -0,0 +1,386 @@ +/* + * 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 Boom from '@hapi/boom'; +import Semver from 'semver'; +import { i18n } from '@kbn/i18n'; +import type { SavedObject } from '@kbn/core/server'; +import type { TaskInstanceWithDeprecatedFields } from '@kbn/task-manager-plugin/server/task'; + +import { validateAndAuthorizeSystemActions } from '../../../../lib/validate_authorize_system_actions'; +import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; +import { parseDuration } from '../../../../../common'; +import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; +import { + validateRuleTypeParams, + getRuleNotifyWhenType, + getDefaultMonitoringRuleDomainProperties, +} from '../../../../lib'; +import { getRuleExecutionStatusPending } from '../../../../lib/rule_execution_status'; +import { + addGeneratedActionValues, + createNewAPIKeySet, + extractReferences, + validateActions, +} from '../../../../rules_client/lib'; +import { + addMissingUiamKeyTagIfNeeded, + apiKeyAsAlertAttributes, + apiKeyAsRuleDomainProperties, +} from '../../../../rules_client/common'; +import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; +import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; +import type { RawRule, RuleTypeRegistry, SanitizedRule } from '../../../../types'; +import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; +import type { RuleDomain, RuleParams } from '../../types'; +import { + transformRuleAttributesToRuleDomain, + transformRuleDomainToRule, + transformRuleDomainToRuleAttributes, +} from '../../transforms'; +import { ruleDomainSchema } from '../../schemas'; +import { createRuleDataSchema } from '../create/schemas'; +import type { + PreparedRule, + PrepareRuleArgs, + ApiKeyEntry, + BulkCreateOperationError, + BulkCreateDisabledReason, +} from './types'; + +export const getBulkCreateAsDisabledMessage = (message: string): string => + i18n.translate('xpack.alerting.rulesClient.bulkCreate.ruleCreatedDisabledErrorMessage', { + defaultMessage: 'Rule created in a disabled state: {message}', + values: { message }, + }); + +export const collectNewKeysToInvalidate = (entries: Iterable): string[] => { + const keys: string[] = []; + for (const { apiKey, uiamApiKey, apiKeyCreatedByUser } of entries) { + if (apiKey && !apiKeyCreatedByUser) keys.push(apiKey); + if (uiamApiKey && !apiKeyCreatedByUser) keys.push(uiamApiKey); + } + return keys; +}; + +export const buildTaskInstance = ( + context: RulesClientContext, + prepared: PreparedRule +): TaskInstanceWithDeprecatedFields => ({ + id: prepared.id, + taskType: `alerting:${prepared.ruleTypeId}`, + schedule: prepared.schedule, + params: { + alertId: prepared.id, + spaceId: context.spaceId, + consumer: prepared.consumer, + }, + state: { + previousStartedAt: null, + alertTypeState: {}, + alertInstances: {}, + }, + scope: ['alerting'], + // Tasks are scheduled disabled. Phase 5 enables them via taskManager.bulkEnable + // so per-task validation drops never produce a running task without a rule SO, + // and the activation can randomise schedule datetimes across the batch. + enabled: false, +}); + +export const toSanitizedRule = ( + context: RulesClientContext, + so: SavedObject, + ruleTypeRegistry: RuleTypeRegistry +): SanitizedRule => { + const ruleType = ruleTypeRegistry.get(so.attributes.alertTypeId); + const ruleDomain: RuleDomain = transformRuleAttributesToRuleDomain( + so.attributes, + { + id: so.id, + logger: context.logger, + ruleType, + references: so.references, + omitGeneratedValues: false, + }, + context.isSystemAction + ); + + try { + ruleDomainSchema.validate(ruleDomain); + } catch (e) { + context.logger.warn(`Error validating bulk-created rule domain object for id: ${so.id}, ${e}`); + } + + return transformRuleDomainToRule(ruleDomain, { isPublic: true }) as SanitizedRule; +}; + +export const prepareRule = async ({ + context, + actionsClient, + username, + id, + rule, + authzCache, + errors, + apiKeysMap, +}: PrepareRuleArgs): Promise<{ prepared?: PreparedRule; error?: BulkOperationError }> => { + const { allowMissingConnectorSecrets } = rule; + + try { + const { actions: genActions, systemActions: genSystemActions } = await addGeneratedActionValues( + rule.data.actions, + rule.data.systemActions, + context + ); + const data = { ...rule.data, actions: genActions, systemActions: genSystemActions }; + + try { + createRuleDataSchema.validate(data); + } catch (validationError) { + throw Boom.badRequest(`Error validating create data - ${validationError.message}`); + } + + // ruleTypeRegistry.get throws 400 if not registered. + context.ruleTypeRegistry.get(data.alertTypeId); + + const authzKey = `${data.alertTypeId}::${data.consumer}`; + if (!authzCache.has(authzKey)) { + authzCache.set( + authzKey, + context.authorization.ensureAuthorized({ + ruleTypeId: data.alertTypeId, + consumer: data.consumer, + operation: WriteOperations.Create, + entity: AlertingAuthorizationEntity.Rule, + }) + ); + } + try { + await authzCache.get(authzKey)!; + } catch (authzError) { + context.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.CREATE, + savedObject: { type: RULE_SAVED_OBJECT_TYPE, id, name: data.name }, + error: authzError, + }) + ); + throw authzError; + } + + context.ruleTypeRegistry.ensureRuleTypeEnabled(data.alertTypeId); + const ruleType = context.ruleTypeRegistry.get(data.alertTypeId); + const validatedRuleTypeParams = validateRuleTypeParams(data.params, ruleType.validate.params); + + await validateActions(context, ruleType, data, allowMissingConnectorSecrets); + await validateAndAuthorizeSystemActions({ + actionsClient, + actionsAuthorization: context.actionsAuthorization, + connectorAdapterRegistry: context.connectorAdapterRegistry, + systemActions: data.systemActions ?? [], + rule: { consumer: data.consumer, producer: ruleType.producer }, + }); + + const intervalInMs = parseDuration(data.schedule.interval); + if ( + intervalInMs < context.minimumScheduleIntervalInMs && + context.minimumScheduleInterval.enforce + ) { + throw Boom.badRequest( + `Error creating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` + ); + } + if ( + intervalInMs < context.minimumScheduleIntervalInMs && + !context.minimumScheduleInterval.enforce + ) { + context.logger.warn( + `Rule schedule interval (${data.schedule.interval}) for "${ruleType.id}" rule type with ID "${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 creation of these rules.` + ); + } + + // Mint API key for enabled rules. + // Soft-fail: a key-mint failure does NOT reject the rule. We persist it as + // disabled, push a degraded error so the caller knows, and let downstream + // phases treat it as a never-enabled rule. Re-enabling later will re-mint. + let effectiveEnabled = data.enabled; + let apiKeyProps: + | ReturnType + | Awaited> = apiKeyAsRuleDomainProperties( + null, + username, + false + ); + if (data.enabled) { + try { + apiKeyProps = await createNewAPIKeySet(context, { + id: ruleType.id, + ruleName: data.name, + username, + shouldUpdateApiKey: true, + errorMessage: 'Error creating rule: could not create API key', + }); + apiKeysMap.set(id, { + apiKey: apiKeyProps.apiKey ?? null, + uiamApiKey: apiKeyProps.uiamApiKey ?? null, + apiKeyCreatedByUser: apiKeyProps.apiKeyCreatedByUser ?? null, + }); + } catch (apiKeyErr) { + effectiveEnabled = false; + apiKeyProps = apiKeyAsRuleDomainProperties(null, username, false); + errors.push({ + message: getBulkCreateAsDisabledMessage(apiKeyErr.message), + status: apiKeyErr.output?.statusCode, + rule: { id, name: data.name }, + disabledReason: 'api_key_creation_failed', + }); + } + } + + const allActions = [...data.actions, ...(data.systemActions ?? [])]; + const artifacts = data.artifacts ?? {}; + const { + references, + params: updatedParams, + actions: actionsWithRefs, + artifacts: artifactsWithRefs, + } = await extractReferences(context, ruleType, allActions, validatedRuleTypeParams, artifacts); + + const createTime = Date.now(); + const lastRunTimestamp = new Date(); + const legacyId = Semver.lt(context.kibanaVersion, '8.0.0') ? id : null; + const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null); + const throttle = data.throttle ?? null; + const { systemActions: _sa, actions: _a, ...restData } = data; + + const tagsWithUiamCheck = await addMissingUiamKeyTagIfNeeded( + data.tags, + apiKeyProps.uiamApiKey, + apiKeyProps.apiKeyCreatedByUser, + context.isServerless, + context.featureFlags + ); + + const ruleAttributes = transformRuleDomainToRuleAttributes({ + actionsWithRefs, + artifactsWithRefs, + rule: { + ...restData, + tags: tagsWithUiamCheck, + ...apiKeyProps, + enabled: effectiveEnabled, + id, + createdBy: username, + updatedBy: username, + createdAt: new Date(createTime), + updatedAt: new Date(createTime), + snoozeSchedule: [], + muteAll: false, + mutedInstanceIds: [], + notifyWhen, + throttle, + executionStatus: getRuleExecutionStatusPending(lastRunTimestamp.toISOString()), + monitoring: getDefaultMonitoringRuleDomainProperties(lastRunTimestamp.toISOString()), + revision: 0, + running: false, + } as unknown as RuleDomain, + params: { legacyId, paramsWithRefs: updatedParams }, + }); + + if (effectiveEnabled) { + ruleAttributes.lastEnabledAt = new Date(createTime).toISOString(); + ruleAttributes.scheduledTaskId = id; + } + + const prepared = { + id, + name: data.name, + enabled: effectiveEnabled, + rawRule: ruleAttributes, + references, + schedule: data.schedule, + consumer: data.consumer, + ruleTypeId: data.alertTypeId, + }; + return { prepared }; + } catch (err) { + const error = { + message: err.message, + status: err.output?.statusCode, + rule: { id, name: rule.data?.name ?? 'n/a' }, + }; + return { error }; + } +}; + +/** + * Demote in-memory (enabled -> disabled): flips a set of currently-enabled + * prepared rules to disabled, queues their API keys for invalidation, records a degraded + * error so the caller can surface "rule was created in a disabled state". + */ +export const demotePreparedRules = ({ + ids, + reason, + message, + preparedRules, + apiKeysMap, + keysToInvalidate, + errors, + username, +}: { + ids: string[]; + reason: BulkCreateDisabledReason; + message: string; + preparedRules: Map; + apiKeysMap: Map; + keysToInvalidate: Set; + errors: BulkCreateOperationError[]; + username: string | null; +}): void => { + for (const id of ids) { + const prepared = preparedRules.get(id); + if (!prepared || !prepared.enabled) continue; + + const apiKey = apiKeysMap.get(id); + if (apiKey) { + for (const k of collectNewKeysToInvalidate([apiKey])) keysToInvalidate.add(k); + apiKeysMap.delete(id); + } + + // Re-shape `rawRule` to the disabled-rule form. + const nullKey = apiKeyAsAlertAttributes(null, username, false); + prepared.rawRule = { + ...prepared.rawRule, + ...nullKey, + uiamApiKey: null, + enabled: false, + }; + delete prepared.rawRule.scheduledTaskId; + delete prepared.rawRule.lastEnabledAt; + prepared.enabled = false; + + errors.push({ + message: getBulkCreateAsDisabledMessage(message), + rule: { id, name: prepared.name }, + disabledReason: reason, + }); + } +}; + +export const flushKeysToInvalidate = async ( + keysToInvalidate: Set, + context: RulesClientContext +): Promise => { + if (keysToInvalidate.size === 0) return; + // Note: ES Call via savedObjectsClient.bulkCreate() under the hood + await bulkMarkApiKeysForInvalidation( + { apiKeys: [...keysToInvalidate] }, + context.logger, + context.unsecuredSavedObjectsClient + ); + keysToInvalidate.clear(); +}; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts index 17dc427a704ee..37c3aead471c0 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts @@ -16,8 +16,6 @@ import type { SavedObjectsFindResult, } from '@kbn/core/server'; import { withSpan } from '@kbn/apm-utils'; -import type { Logger } from '@kbn/core/server'; -import type { TaskManagerStartContract } from '@kbn/task-manager-plugin/server'; import { TaskStatus } from '@kbn/task-manager-plugin/server'; import type { TaskInstanceWithDeprecatedFields } from '@kbn/task-manager-plugin/server/task'; import { RuleChangeTrackingAction } from '@kbn/alerting-types'; @@ -46,6 +44,7 @@ import type { RulesClientContext, BulkOperationError } from '../../../../rules_c import { validateScheduleLimit } from '../get_schedule_frequency'; import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; import type { BulkEnableRulesParams, BulkEnableRulesResult } from './types'; +import { bulkEnableTasks } from '../bulk_enable_tasks'; import { bulkEnableRulesParamsSchema } from './schemas'; import { transformRuleAttributesToRuleDomain, transformRuleDomainToRule } from '../../transforms'; import { ruleDomainSchema } from '../../schemas'; @@ -117,10 +116,8 @@ export const bulkEnableRules = async ( const [taskIdsToEnable] = accListSpecificForBulkOperation; - const taskIdsFailedToBeEnabled = await tryToEnableTasks({ - taskIdsToEnable, - logger: context.logger, - taskManager: context.taskManager, + const { taskIdsFailedToBeEnabled } = await bulkEnableTasks(context, { + taskIds: taskIdsToEnable, }); const updatedRules = rules.map(({ id, attributes, references }) => { @@ -442,49 +439,3 @@ const bulkEnableRulesWithOCC = async ( accListSpecificForBulkOperation: [taskIdsToEnable], }; }; - -export const tryToEnableTasks = async ({ - taskIdsToEnable, - logger, - taskManager, -}: { - taskIdsToEnable: string[]; - logger: Logger; - taskManager: TaskManagerStartContract; -}) => { - const taskIdsFailedToBeEnabled: string[] = []; - - if (taskIdsToEnable.length > 0) { - try { - const resultFromEnablingTasks = await withSpan( - { name: 'taskManager.bulkEnable', type: 'rules' }, - async () => taskManager.bulkEnable(taskIdsToEnable) - ); - resultFromEnablingTasks?.errors?.forEach((error) => { - taskIdsFailedToBeEnabled.push(error.id); - }); - if (resultFromEnablingTasks.tasks.length) { - logger.debug( - `Successfully enabled schedules for underlying tasks: ${resultFromEnablingTasks.tasks - .map((task) => task.id) - .join(', ')}` - ); - } - if (resultFromEnablingTasks.errors.length) { - logger.error( - `Failure to enable schedules for underlying tasks: ${resultFromEnablingTasks.errors - .map((error) => error.id) - .join(', ')}` - ); - } - } catch (error) { - taskIdsFailedToBeEnabled.push(...taskIdsToEnable); - logger.error( - `Failure to enable schedules for underlying tasks: ${taskIdsToEnable.join( - ', ' - )}. TaskManager bulkEnable failed with Error: ${error.message}` - ); - } - } - return taskIdsFailedToBeEnabled; -}; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts new file mode 100644 index 0000000000000..5f0bf05db4d79 --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts @@ -0,0 +1,57 @@ +/* + * 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 { withSpan } from '@kbn/apm-utils'; +import type { RulesClientContext } from '../../../../rules_client/types'; +import type { BulkEnableTasksParams, BulkEnableTasksResult } from './types'; + +// Enable previously scheduled task manager tasks; returns ids whose enable failed. +export const bulkEnableTasks = async ( + context: RulesClientContext, + params: BulkEnableTasksParams +): Promise => { + const { taskIds } = params; + const { logger, taskManager } = context; + const taskIdsFailedToBeEnabled: string[] = []; + + if (taskIds.length === 0) { + return { taskIdsFailedToBeEnabled }; + } + + try { + const resultFromEnablingTasks = await withSpan( + { name: 'taskManager.bulkEnable', type: 'rules' }, + async () => taskManager.bulkEnable(taskIds) + ); + resultFromEnablingTasks?.errors?.forEach((error) => { + taskIdsFailedToBeEnabled.push(error.id); + }); + if (resultFromEnablingTasks.tasks.length) { + logger.debug( + `Successfully enabled schedules for underlying tasks: ${resultFromEnablingTasks.tasks + .map((task) => task.id) + .join(', ')}` + ); + } + if (resultFromEnablingTasks.errors.length) { + logger.error( + `Failure to enable schedules for underlying tasks: ${resultFromEnablingTasks.errors + .map((error) => error.id) + .join(', ')}` + ); + } + } catch (error) { + taskIdsFailedToBeEnabled.push(...taskIds); + logger.error( + `Failure to enable schedules for underlying tasks: ${taskIds.join( + ', ' + )}. TaskManager bulkEnable failed with Error: ${error.message}` + ); + } + + return { taskIdsFailedToBeEnabled }; +}; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts new file mode 100644 index 0000000000000..89c0c7b818ebc --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts @@ -0,0 +1,10 @@ +/* + * 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. + */ + +export type { BulkEnableTasksParams, BulkEnableTasksResult } from './types'; + +export { bulkEnableTasks } from './bulk_enable_tasks'; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts new file mode 100644 index 0000000000000..bee29dc832f7a --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts @@ -0,0 +1,14 @@ +/* + * 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. + */ + +export interface BulkEnableTasksParams { + taskIds: string[]; +} + +export interface BulkEnableTasksResult { + taskIdsFailedToBeEnabled: string[]; +} diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts new file mode 100644 index 0000000000000..a3a0bb77a2785 --- /dev/null +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts @@ -0,0 +1,8 @@ +/* + * 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. + */ + +export type * from './bulk_enable_tasks_types'; diff --git a/x-pack/platform/plugins/shared/alerting/server/index.ts b/x-pack/platform/plugins/shared/alerting/server/index.ts index d7157469f2e3c..d92c1e7b64e35 100644 --- a/x-pack/platform/plugins/shared/alerting/server/index.ts +++ b/x-pack/platform/plugins/shared/alerting/server/index.ts @@ -55,6 +55,11 @@ export type { GetRuleHistoryResult, RuleChangeTrackingDisabledError, } from './rules_client'; +export type { + BulkCreateRulesItem, + BulkCreateRulesParams, + BulkCreateRulesResult, +} from './application/rule/methods/bulk_create'; export type { Rule } from './application/rule/types'; export type { PublicAlert as Alert } from './alert'; export { parseDuration, isRuleSnoozed } from './lib'; diff --git a/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts b/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts index 696eaac7f1301..7f10921c2c50f 100644 --- a/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts +++ b/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts @@ -53,8 +53,12 @@ const createRulesClientMock = () => { bulkGetRules: jest.fn(), bulkEdit: jest.fn(), bulkEditRuleParamsWithReadAuth: jest.fn(), + bulkCreateRules: jest + .fn() + .mockResolvedValue({ rules: [], errors: [], total: 0, taskIdsFailedToBeEnabled: [] }), bulkDeleteRules: jest.fn(), bulkEnableRules: jest.fn(), + bulkEnableTasks: jest.fn().mockResolvedValue({ taskIdsFailedToBeEnabled: [] }), bulkDisableRules: jest.fn(), snooze: jest.fn(), unsnooze: jest.fn(), diff --git a/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts b/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts index 19714f7760f1a..5a66e7ff01084 100644 --- a/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts +++ b/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts @@ -62,6 +62,10 @@ import type { BulkEditRuleParamsOptions } from '../application/rule/methods/bulk import { bulkEditRuleParamsWithReadAuth } from '../application/rule/methods/bulk_edit_params/bulk_edit_rule_params'; import type { BulkEnableRulesParams } from '../application/rule/methods/bulk_enable'; import { bulkEnableRules } from '../application/rule/methods/bulk_enable'; +import type { BulkCreateRulesParams } from '../application/rule/methods/bulk_create'; +import { bulkCreateRules } from '../application/rule/methods/bulk_create'; +import type { BulkEnableTasksParams } from '../application/rule/methods/bulk_enable_tasks'; +import { bulkEnableTasks } from '../application/rule/methods/bulk_enable_tasks'; import { enableRule } from '../application/rule/methods/enable_rule/enable_rule'; import { updateRuleApiKey } from '../application/rule/methods/update_api_key/update_rule_api_key'; import { disableRule } from '../application/rule/methods/disable/disable_rule'; @@ -193,6 +197,9 @@ export class RulesClient { public bulkGetRules = (params: BulkGetRulesParams) => bulkGetRules(this.context, params); + public bulkCreateRules = ( + params: BulkCreateRulesParams + ) => bulkCreateRules(this.context, params); public bulkDeleteRules = (options: BulkDeleteRulesParams) => bulkDeleteRules(this.context, options); public bulkEdit = (options: BulkEditOptions) => @@ -202,6 +209,7 @@ export class RulesClient { options: BulkEditRuleParamsOptions ) => bulkEditRuleParamsWithReadAuth(this.context, options); public bulkEnableRules = (params: BulkEnableRulesParams) => bulkEnableRules(this.context, params); + public bulkEnableTasks = (params: BulkEnableTasksParams) => bulkEnableTasks(this.context, params); public bulkDisableRules = (options: BulkDisableRulesRequestBody) => bulkDisableRules(this.context, options); From b9e3b737796b24236a8f95da39d3736005c18507 Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Thu, 14 May 2026 17:12:48 +0200 Subject: [PATCH 2/8] Incorporate feedback from alerting framework team --- .../bulk_create/bulk_create_rules.test.ts | 372 +++++++++++------- .../methods/bulk_create/bulk_create_rules.ts | 142 +++++-- .../rule/methods/bulk_create/types.ts | 21 +- .../rule/methods/bulk_create/utils.ts | 75 ++-- .../methods/bulk_enable/bulk_enable_rules.ts | 55 ++- .../bulk_enable_tasks/bulk_enable_tasks.ts | 57 --- .../rule/methods/bulk_enable_tasks/index.ts | 10 - .../types/bulk_enable_tasks_types.ts | 14 - .../methods/bulk_enable_tasks/types/index.ts | 8 - .../alerting/server/rules_client.mock.ts | 5 +- .../server/rules_client/common/constants.ts | 4 + .../server/rules_client/rules_client.ts | 3 - 12 files changed, 436 insertions(+), 330 deletions(-) delete mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts delete mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts delete mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts delete mode 100644 x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts index ea631aad4cd6f..abeaf5ebf7ef9 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts @@ -30,6 +30,12 @@ import { RulesClient } from '../../../../rules_client/rules_client'; import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import { validateScheduleLimit } from '../get_schedule_frequency'; import { RuleAuditAction } from '../../../../rules_client/common/audit_events'; +import { + BULK_TM_SCHEDULE_DELAY, + DEFAULT_BULK_CREATE_BATCH_SIZE, + MAX_BULK_CREATE_BATCH_SIZE, + MAX_RULES_NUMBER_FOR_BULK_OPERATION, +} from '../../../../rules_client/common/constants'; const BULK_CREATE_AS_DISABLED_PREFIX = 'Rule created in a disabled state: '; @@ -72,6 +78,7 @@ const rulesClientParams: jest.Mocked = { namespace: 'default', getUserName: jest.fn(), createAPIKey: jest.fn(), + cloneAPIKey: jest.fn(), logger: loggingSystemMock.create().get(), internalSavedObjectsRepository, encryptedSavedObjectsClient: encryptedSavedObjects, @@ -148,14 +155,15 @@ const buildBulkResponse = ( describe('bulkCreateRules', () => { let rulesClient: RulesClient; let actionsClient: jest.Mocked; + let idCounter = 0; beforeEach(async () => { getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry); (auditLogger.log as jest.Mock).mockClear(); (bulkMarkApiKeysForInvalidation as jest.Mock).mockReset(); (validateScheduleLimit as jest.Mock).mockReset(); - let counter = 0; - (SavedObjectsUtils.generateId as jest.Mock).mockImplementation(() => `mock-id-${++counter}`); + idCounter = 0; + (SavedObjectsUtils.generateId as jest.Mock).mockImplementation(() => `mock-id-${++idCounter}`); rulesClient = new RulesClient(rulesClientParams); actionsClient = (await rulesClientParams.getActionsClient()) as jest.Mocked; actionsClient.getBulk.mockResolvedValue([ @@ -170,12 +178,11 @@ describe('bulkCreateRules', () => { }); rulesClientParams.isAuthenticationTypeAPIKey.mockReturnValue(false); taskManager.bulkSchedule.mockImplementation(async (tasks) => tasks as never); - taskManager.bulkEnable.mockResolvedValue({ tasks: [], errors: [] } as never); }); test('returns empty result for empty input without touching SO/TM/key clients', async () => { const result = await rulesClient.bulkCreateRules({ rules: [] }); - expect(result).toEqual({ rules: [], errors: [], total: 0, taskIdsFailedToBeEnabled: [] }); + expect(result).toEqual({ successfulIds: [], errors: [], total: 0 }); expect(unsecuredSavedObjectsClient.bulkCreate).not.toHaveBeenCalled(); expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); expect(rulesClientParams.createAPIKey).not.toHaveBeenCalled(); @@ -196,44 +203,13 @@ describe('bulkCreateRules', () => { expect(rulesClientParams.createAPIKey).not.toHaveBeenCalled(); expect(result.errors).toEqual([]); expect(result.total).toBe(2); - expect(result.rules).toHaveLength(2); - expect(result.taskIdsFailedToBeEnabled).toEqual([]); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); }); - test('all-enabled happy path: API keys, bulkSchedule, bulkCreate, bulkEnable', async () => { - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( - buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) - ); - - const result = await rulesClient.bulkCreateRules({ - rules: [ - { data: baseRule({ name: 'a', enabled: true }) }, - { data: baseRule({ name: 'b', enabled: true }) }, - ], - }); - - expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(2); - expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); - const scheduledIds = (taskManager.bulkSchedule.mock.calls[0][0] as Array<{ id: string }>).map( - (t) => t.id - ); - expect(scheduledIds).toEqual(['mock-id-1', 'mock-id-2']); - // tasks are scheduled disabled; bulkEnable flips them on - expect( - (taskManager.bulkSchedule.mock.calls[0][0] as Array<{ enabled: boolean }>).every( - (t) => t.enabled === false - ) - ).toBe(true); - expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); - expect(taskManager.bulkEnable).toHaveBeenCalledWith(['mock-id-1', 'mock-id-2']); - expect(result.errors).toEqual([]); - expect(result.total).toBe(2); - expect(result.rules).toHaveLength(2); - expect(result.taskIdsFailedToBeEnabled).toEqual([]); - }); - - describe('skipTaskEnabling', () => { - test('skips taskManager.bulkEnable and returns enabled task ids in taskIdsFailedToBeEnabled', async () => { + test('all-enabled happy path: API keys, bulkSchedule with enabled=true and future runAt, bulkCreate, no bulkEnable', async () => { + const now = Date.parse('2024-01-01T00:00:00.000Z'); + jest.useFakeTimers().setSystemTime(now); + try { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) ); @@ -243,60 +219,31 @@ describe('bulkCreateRules', () => { { data: baseRule({ name: 'a', enabled: true }) }, { data: baseRule({ name: 'b', enabled: true }) }, ], - skipTaskEnabling: true, }); + expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(2); expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); + const scheduled = taskManager.bulkSchedule.mock.calls[0][0] as Array<{ + id: string; + enabled: boolean; + runAt: Date; + scheduledAt: Date; + }>; + expect(scheduled.map((t) => t.id)).toEqual(['mock-id-1', 'mock-id-2']); + expect(scheduled.every((t) => t.enabled === true)).toBe(true); + const minRunAt = now + BULK_TM_SCHEDULE_DELAY; + for (const task of scheduled) { + expect(task.runAt.getTime()).toBeGreaterThanOrEqual(minRunAt); + expect(task.scheduledAt.getTime()).toBe(task.runAt.getTime()); + } + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); expect(taskManager.bulkEnable).not.toHaveBeenCalled(); - expect(result.taskIdsFailedToBeEnabled).toEqual(['mock-id-1', 'mock-id-2']); - }); - - test('returns empty taskIdsFailedToBeEnabled when no rule is enabled', async () => { - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( - buildBulkResponse([{ id: 'mock-id-1' }]) - ); - - const result = await rulesClient.bulkCreateRules({ - rules: [{ data: baseRule({ name: 'a' }) }], - skipTaskEnabling: true, - }); - - expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); - expect(taskManager.bulkEnable).not.toHaveBeenCalled(); - expect(result.taskIdsFailedToBeEnabled).toEqual([]); - }); - - test('excludes per-row SO failure ids from taskIdsFailedToBeEnabled', async () => { - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( - buildBulkResponse([ - { id: 'mock-id-1' }, - { id: 'mock-id-2', error: { message: '409', statusCode: 409 } }, - ]) - ); - - const result = await rulesClient.bulkCreateRules({ - rules: [ - { data: baseRule({ name: 'a', enabled: true }) }, - { data: baseRule({ name: 'b', enabled: true }) }, - ], - skipTaskEnabling: true, - }); - - expect(taskManager.bulkEnable).not.toHaveBeenCalled(); - expect(result.taskIdsFailedToBeEnabled).toEqual(['mock-id-1']); - expect(result.errors).toHaveLength(1); - expect(result.errors[0].rule.id).toBe('mock-id-2'); - }); - - test('empty-input early return returns standard shape', async () => { - const result = await rulesClient.bulkCreateRules({ rules: [], skipTaskEnabling: true }); - expect(result).toEqual({ - rules: [], - errors: [], - total: 0, - taskIdsFailedToBeEnabled: [], - }); - }); + expect(result.errors).toEqual([]); + expect(result.total).toBe(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); + } finally { + jest.useRealTimers(); + } }); test('mixed enabled+disabled happy path', async () => { @@ -312,9 +259,10 @@ describe('bulkCreateRules', () => { expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); expect(taskManager.bulkSchedule.mock.calls[0][0]).toHaveLength(1); expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); - expect(taskManager.bulkEnable).toHaveBeenCalledWith(['mock-id-1']); + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); expect(result.errors).toEqual([]); expect(result.total).toBe(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); }); test('Phase 1 per-rule throw is isolated', async () => { @@ -351,7 +299,7 @@ describe('bulkCreateRules', () => { expect(result.total).toBe(2); expect(result.errors).toHaveLength(1); expect(result.errors[0].rule.name).toBe('fails'); - expect(result.rules).toHaveLength(1); + expect(result.successfulIds).toEqual(['mock-id-2']); expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(1); }); @@ -368,8 +316,6 @@ describe('bulkCreateRules', () => { ], }); - // Both rules persisted in a single SO bulkCreate. The first one is demoted - // to disabled because its key mint failed; the second is scheduled normally. expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); const persistedAttrsByName = new Map( @@ -382,13 +328,11 @@ describe('bulkCreateRules', () => { expect(persistedAttrsByName.get('enabled-keyfail')?.enabled).toBe(false); expect(persistedAttrsByName.get('enabled-keyfail')?.apiKey).toBeNull(); expect(persistedAttrsByName.get('enabled-ok')?.enabled).toBe(true); - // Only the surviving enabled rule was scheduled / enabled. expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); expect(taskManager.bulkSchedule.mock.calls[0][0]).toHaveLength(1); - expect(taskManager.bulkEnable).toHaveBeenCalledWith(['mock-id-2']); - // No key mint succeeded for the demoted rule, so nothing to invalidate. + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); expect(bulkMarkApiKeysForInvalidation).not.toHaveBeenCalled(); - expect(result.rules).toHaveLength(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); expect(result.errors).toHaveLength(1); expect(result.errors[0]).toEqual( expect.objectContaining({ @@ -416,10 +360,7 @@ describe('bulkCreateRules', () => { ], }); - // The enabled rule contributed an API key → must be invalidated expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalled(); - // Both rules persisted in a single SO bulkCreate; the originally-enabled - // one is degraded to disabled (no scheduledTaskId, no API key fields). expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); const persistedAttrsByName = new Map( @@ -432,7 +373,7 @@ describe('bulkCreateRules', () => { expect(persistedAttrsByName.get('enabled')?.enabled).toBe(false); expect(persistedAttrsByName.get('enabled')?.scheduledTaskId).toBeUndefined(); expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); - expect(result.rules).toHaveLength(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); expect(result.errors).toHaveLength(1); expect(result.errors[0]).toEqual( expect.objectContaining({ @@ -459,7 +400,7 @@ describe('bulkCreateRules', () => { expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalled(); expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); - expect(result.rules).toHaveLength(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); expect(result.errors).toHaveLength(1); expect(result.errors[0]).toEqual( expect.objectContaining({ @@ -487,7 +428,7 @@ describe('bulkCreateRules', () => { }); expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); - expect(result.rules).toHaveLength(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); expect(result.errors).toHaveLength(1); expect(result.errors[0]).toEqual( expect.objectContaining({ @@ -508,13 +449,12 @@ describe('bulkCreateRules', () => { }); expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); - // Per-row error path now batches TM cleanup into a single bulkRemove. expect(taskManager.bulkRemove).toHaveBeenCalledWith(['mock-id-1']); expect(taskManager.removeIfExists).not.toHaveBeenCalled(); expect(result.errors).toHaveLength(1); expect(result.errors[0].status).toBe(409); expect(result.errors[0].disabledReason).toBeUndefined(); - expect(result.rules).toHaveLength(0); + expect(result.successfulIds).toEqual([]); }); test('Phase 4 per-row error on caller-supplied id NOT in newlyScheduledTaskIds: NO TM cleanup', async () => { @@ -530,37 +470,20 @@ describe('bulkCreateRules', () => { expect(taskManager.bulkRemove).not.toHaveBeenCalled(); }); - test('Phase 4 whole-call throw: invalidates every newly-minted key and best-effort bulkRemove of scheduled ids, then rethrows', async () => { + test('Phase 4 whole-call throw: invalidates keys, bulkRemoves scheduled ids, surfaces SO failure in errors (does NOT rethrow)', async () => { unsecuredSavedObjectsClient.bulkCreate.mockRejectedValueOnce(new Error('SO down')); - await expect( - rulesClient.bulkCreateRules({ - rules: [ - { data: baseRule({ name: 'a', enabled: true }) }, - { data: baseRule({ name: 'b', enabled: true }) }, - ], - }) - ).rejects.toThrow('SO down'); - - expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); - expect(taskManager.bulkRemove).toHaveBeenCalledWith(['mock-id-1', 'mock-id-2']); - }); - - test('Phase 5 per-task enable error: surfaced in taskIdsFailedToBeEnabled, no SO rollback', async () => { - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( - buildBulkResponse([{ id: 'mock-id-1' }]) - ); - taskManager.bulkEnable.mockResolvedValueOnce({ - tasks: [], - errors: [{ id: 'mock-id-1', error: { type: 'foo', message: 'no' } }], - } as never); - const result = await rulesClient.bulkCreateRules({ - rules: [{ data: baseRule({ name: 'a', enabled: true }) }], + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b', enabled: true }) }, + ], }); - expect(result.taskIdsFailedToBeEnabled).toEqual(['mock-id-1']); - expect(result.rules).toHaveLength(1); + expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1); + expect(taskManager.bulkRemove).toHaveBeenCalledWith(['mock-id-1', 'mock-id-2']); + expect(result.successfulIds).toEqual([]); + expect(result.errors.some((e) => e.message.includes('SO down'))).toBe(true); }); test('every demotion path stamps a machine-readable disabledReason on the error', async () => { @@ -616,8 +539,6 @@ describe('bulkCreateRules', () => { ); }); - // Caller-side chunking is covered by import_rules.test.ts; bulkCreateRules handles one batch per call. - test('emits per-rule CREATE audit event for surviving rules and ENABLE for the enabled subset', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) @@ -636,4 +557,187 @@ describe('bulkCreateRules', () => { expect(actions.filter((a) => a === RuleAuditAction.CREATE)).toHaveLength(2); expect(actions.filter((a) => a === RuleAuditAction.ENABLE)).toHaveLength(1); }); + + describe('batching', () => { + test('splits rules across batches and concatenates results', async () => { + const ruleCount = DEFAULT_BULK_CREATE_BATCH_SIZE * 2 + 1; // forces 3 batches with default batch size + unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => + buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: Array.from({ length: ruleCount }, (_, i) => ({ + data: baseRule({ name: `r-${i}` }), + })), + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(3); + expect(result.total).toBe(ruleCount); + expect(result.successfulIds).toHaveLength(ruleCount); + expect(result.errors).toEqual([]); + }); + + test('honours caller-supplied batchSize', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => + buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) + ); + + await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 10 }, (_, i) => ({ data: baseRule({ name: `r-${i}` }) })), + batchSize: 4, + }); + + // 10 rules, batchSize 4 → 3 batches (4, 4, 2) + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(3); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(4); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[1][0]).toHaveLength(4); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[2][0]).toHaveLength(2); + }); + + test('clamps batchSize above hard cap and logs a warn', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => + buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) + ); + + // 1 rule with absurd batchSize; we just verify the warn + that the batch still runs. + await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }], + batchSize: 999_999, + }); + + const warnCalls = (rulesClientParams.logger.warn as jest.Mock).mock.calls + .map((c) => c[0]) + .filter((m: string) => + m?.includes?.(`batchSize 999999 clamped to ${MAX_BULK_CREATE_BATCH_SIZE}`) + ); + expect(warnCalls).toHaveLength(1); + }); + + test(`rejects with 400 when rules.length exceeds MAX_RULES_NUMBER_FOR_BULK_OPERATION (${MAX_RULES_NUMBER_FOR_BULK_OPERATION})`, async () => { + const over = MAX_RULES_NUMBER_FOR_BULK_OPERATION + 1; + await expect( + rulesClient.bulkCreateRules({ + rules: Array.from({ length: over }, (_, i) => ({ data: baseRule({ name: `r-${i}` }) })), + }) + ).rejects.toThrow( + `${over} rules exceeds the hard limit of ${MAX_RULES_NUMBER_FOR_BULK_OPERATION}` + ); + expect(unsecuredSavedObjectsClient.bulkCreate).not.toHaveBeenCalled(); + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + }); + + test('per-batch runAt is at least the buffer beyond now', async () => { + const now = Date.parse('2024-01-01T00:00:00.000Z'); + jest.useFakeTimers().setSystemTime(now); + try { + unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => + buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) + ); + + await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 4 }, (_, i) => ({ + data: baseRule({ name: `r-${i}`, enabled: true }), + })), + batchSize: 2, + }); + + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(2); + const minRunAt = now + BULK_TM_SCHEDULE_DELAY; + for (const call of taskManager.bulkSchedule.mock.calls) { + for (const t of call[0] as Array<{ runAt: Date }>) { + expect(t.runAt.getTime()).toBeGreaterThanOrEqual(minRunAt); + } + } + } finally { + jest.useRealTimers(); + } + }); + }); + + describe('exitEarlyOnError', () => { + test('default (false): keeps processing subsequent batches even after an SO per-row error', async () => { + unsecuredSavedObjectsClient.bulkCreate + .mockResolvedValueOnce( + buildBulkResponse([ + { id: 'mock-id-1', error: { message: 'conflict', statusCode: 409 } }, + { id: 'mock-id-2' }, + ]) + ) + .mockResolvedValueOnce(buildBulkResponse([{ id: 'mock-id-3' }, { id: 'mock-id-4' }])); + + const result = await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 4 }, (_, i) => ({ data: baseRule({ name: `r-${i}` }) })), + batchSize: 2, + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(2); + expect(result.successfulIds).toEqual(['mock-id-2', 'mock-id-3', 'mock-id-4']); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.id).toBe('mock-id-1'); + }); + + test('true: stops at the first SO per-row failure, returns prior successes', async () => { + unsecuredSavedObjectsClient.bulkCreate + .mockResolvedValueOnce(buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }])) + .mockResolvedValueOnce( + buildBulkResponse([ + { id: 'mock-id-3', error: { message: 'conflict', statusCode: 409 } }, + { id: 'mock-id-4' }, + ]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 6 }, (_, i) => ({ data: baseRule({ name: `r-${i}` }) })), + batchSize: 2, + exitEarlyOnError: true, + }); + + // Only batches 1 and 2 should have executed; batch 3 is skipped. + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2', 'mock-id-4']); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.id).toBe('mock-id-3'); + }); + + test('true: stops on whole-call SO throw and surfaces SO-failure error', async () => { + unsecuredSavedObjectsClient.bulkCreate + .mockResolvedValueOnce(buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }])) + .mockRejectedValueOnce(new Error('SO down')); + + const result = await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 6 }, (_, i) => ({ data: baseRule({ name: `r-${i}` }) })), + batchSize: 2, + exitEarlyOnError: true, + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); + expect(result.errors.some((e) => e.message.includes('SO down'))).toBe(true); + }); + + test('true: does NOT trigger on Phase-1/2/3 demotion errors (only on SO failures)', async () => { + // Batch 1: schedule limit trips on the enabled rule (Phase 2), SO succeeds. + // Batch 2: should still execute. + (validateScheduleLimit as jest.Mock).mockResolvedValueOnce({ + interval: 100, + intervalAvailable: 50, + }); + unsecuredSavedObjectsClient.bulkCreate + .mockResolvedValueOnce(buildBulkResponse([{ id: 'mock-id-1' }])) + .mockResolvedValueOnce(buildBulkResponse([{ id: 'mock-id-2' }])); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b' }) }, + ], + batchSize: 1, + exitEarlyOnError: true, + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); + expect(result.errors.some((e) => e.disabledReason === 'schedule_limit_exceeded')).toBe(true); + }); + }); }); diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts index 63ce9ce803a1e..420b10f4c9d36 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -5,26 +5,34 @@ * 2.0. */ +import Boom from '@hapi/boom'; import pMap from 'p-map'; import { withSpan } from '@kbn/apm-utils'; -import type { SavedObject, SavedObjectsBulkCreateObject } from '@kbn/core/server'; +import type { SavedObjectsBulkCreateObject } from '@kbn/core/server'; import { SavedObjectsUtils } from '@kbn/core/server'; import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; import { getRuleCircuitBreakerErrorMessage } from '../../../../../common'; import { updateMeta } from '../../../../rules_client/lib'; -import { API_KEY_GENERATE_CONCURRENCY } from '../../../../rules_client/common/constants'; +import { + API_KEY_GENERATE_CONCURRENCY, + DEFAULT_BULK_CREATE_BATCH_SIZE, + MAX_BULK_CREATE_BATCH_SIZE, + MAX_RULES_NUMBER_FOR_BULK_OPERATION, +} from '../../../../rules_client/common/constants'; import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; -import { bulkEnableTasks } from '../bulk_enable_tasks'; import { bulkCreateRulesSo } from '../../../../data/rule'; -import type { RawRule, SanitizedRule } from '../../../../types'; -import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; +import type { RawRule } from '../../../../types'; +import type { RulesClientContext } from '../../../../rules_client/types'; import type { RuleParams } from '../../types'; import { validateScheduleLimit } from '../get_schedule_frequency'; import type { ApiKeyEntry, - PreparedRule, + BatchResult, + BulkCreateOperationError, + BulkCreateRulesItem, BulkCreateRulesParams, BulkCreateRulesResult, + PreparedRule, } from './types'; import { buildTaskInstance, @@ -32,31 +40,95 @@ import { demotePreparedRules, flushKeysToInvalidate, prepareRule, - toSanitizedRule, } from './utils'; export async function bulkCreateRules( context: RulesClientContext, params: BulkCreateRulesParams -): Promise> { - const { rules } = params; +): Promise { + const { rules, exitEarlyOnError = false } = params; const { logger } = context; const total = rules.length; if (total === 0) { - return { rules: [], errors: [], total: 0, taskIdsFailedToBeEnabled: [] }; + return { successfulIds: [], errors: [], total: 0 }; + } + + if (total > MAX_RULES_NUMBER_FOR_BULK_OPERATION) { + throw Boom.badRequest( + `bulkCreateRules: ${total} rules exceeds the hard limit of ${MAX_RULES_NUMBER_FOR_BULK_OPERATION}. ` + + `Callers should enforce request-level limits before invoking this method.` + ); + } + + const requestedBatchSize = params.batchSize ?? DEFAULT_BULK_CREATE_BATCH_SIZE; + const batchSize = Math.max(1, Math.min(MAX_BULK_CREATE_BATCH_SIZE, requestedBatchSize)); + + if (requestedBatchSize !== batchSize) { + logger.warn( + `bulkCreateRules: batchSize ${requestedBatchSize} clamped to ${batchSize} (hard cap ${MAX_BULK_CREATE_BATCH_SIZE}).` + ); } const username = await context.getUserName(); const actionsClient = await context.getActionsClient(); - const inputsWithIds = rules.map((rule) => ({ + const successfulIds: string[] = []; + const errors: BulkCreateOperationError[] = []; + + const totalBatches = Math.ceil(total / batchSize); + logger.debug(`bulkCreateRules: ${total} rules, ${totalBatches}x batches of ${batchSize} rules.`); + + for (let batchIndex = 0; batchIndex < totalBatches; batchIndex++) { + const start = batchIndex * batchSize; + const batch = rules.slice(start, start + batchSize); + + const result = await runBatch({ + context, + username, + actionsClient, + batch, + }); + + successfulIds.push(...result.successfulIds); + errors.push(...result.errors); + + if (exitEarlyOnError && result.soFailureOccurred) { + logger.warn( + `bulkCreateRules: exiting early on SO failure at batch ${ + batchIndex + 1 + }/${totalBatches}. ` + + `${successfulIds.length} rule(s) created, ${ + total - start - batch.length + } rule(s) skipped.` + ); + break; + } + } + + return { successfulIds, errors, total }; +} + +interface RunBatchArgs { + context: RulesClientContext; + username: string | null; + actionsClient: Awaited>; + batch: Array>; +} + +async function runBatch({ + context, + username, + actionsClient, + batch, +}: RunBatchArgs): Promise { + const { logger } = context; + + const inputsWithIds = batch.map((rule) => ({ id: rule.options?.id ?? SavedObjectsUtils.generateId(), rule, })); - logger.debug(`Bulk creating batch of ${total} rules`); - // Phase 1: per-rule prepare (validation + api key generation). // NOTE: in order to minimise external calls, the values below get mutated // at different stages in the process (ie if we fail schedule creation), @@ -64,7 +136,7 @@ export async function bulkCreateRules( const preparedRules = new Map(); const keysToInvalidate = new Set(); const apiKeysMap = new Map(); - const errors: BulkOperationError[] = []; + const errors: BulkCreateOperationError[] = []; const authzCache = new Map>(); await pMap( @@ -86,10 +158,10 @@ export async function bulkCreateRules( { concurrency: API_KEY_GENERATE_CONCURRENCY } ); - // No survivors? Flush out any keys created and return + // No survivors? Flush any keys created and return. if (preparedRules.size === 0) { await flushKeysToInvalidate(keysToInvalidate, context); - return { rules: [], errors, total, taskIdsFailedToBeEnabled: [] }; + return { successfulIds: [], errors, soFailureOccurred: false }; } // Phase 2: validate schedule-limits, enabled subset only. @@ -211,10 +283,9 @@ export async function bulkCreateRules( }) ); } catch (error) { - // Whole-call SO failure (auth, timeout, etc): invalidate every newly-minted - // key, best-effort orphan-task cleanup, then rethrow. + // Whole-call SO failure: invalidate keys, best-effort task cleanup. + // Surface as batch-wide SO failure so exitEarlyOnError can honour it. for (const k of collectNewKeysToInvalidate(apiKeysMap.values())) keysToInvalidate.add(k); - await flushKeysToInvalidate(keysToInvalidate, context); if (newlyScheduledTaskIds.size > 0) { try { await context.taskManager.bulkRemove([...newlyScheduledTaskIds]); @@ -226,16 +297,23 @@ export async function bulkCreateRules( ); } } - throw error; + await flushKeysToInvalidate(keysToInvalidate, context); + errors.push({ + message: `Failed to bulk create rule saved objects: ${error.message}`, + status: error.output?.statusCode, + rule: { id: 'n/a', name: 'n/a' }, + }); + return { successfulIds: [], errors, soFailureOccurred: true }; } // Phase 4 per-row outcomes. - const successfulSos: Array> = []; - const taskIdsToEnable: string[] = []; + const batchSuccessfulIds: string[] = []; const taskIdsToCleanUp: string[] = []; + let perRowFailureOccurred = false; for (const so of bulkResponse.saved_objects) { if (so.error) { + perRowFailureOccurred = true; errors.push({ message: so.error.message ?? 'n/a', status: so.error.statusCode, @@ -253,9 +331,8 @@ export async function bulkCreateRules( taskIdsToCleanUp.push(so.id); } } else { - successfulSos.push(so as SavedObject); + batchSuccessfulIds.push(so.id); if (newlyScheduledTaskIds.has(so.id)) { - taskIdsToEnable.push(so.id); // Audit per-rule ENABLE for the enabled subset (mirrors single-rule semantics). context.auditLogger?.log( ruleAuditEvent({ @@ -286,23 +363,12 @@ export async function bulkCreateRules( } } - // Phase 5: enable scheduled tasks. If skipTaskEnabling, caller enables them later. - const taskIdsFailedToBeEnabled = params.skipTaskEnabling - ? [...taskIdsToEnable] - : (await bulkEnableTasks(context, { taskIds: taskIdsToEnable })).taskIdsFailedToBeEnabled; - - // Single end-of-function flush for all collected key invalidations. + // Single per-batch flush for all collected key invalidations. await flushKeysToInvalidate(keysToInvalidate, context); - // Phase 6: domain transform + return. - const sanitizedRules: Array> = successfulSos.map((so) => - toSanitizedRule(context, so, context.ruleTypeRegistry) - ); - return { - rules: sanitizedRules, + successfulIds: batchSuccessfulIds, errors, - total, - taskIdsFailedToBeEnabled, + soFailureOccurred: perRowFailureOccurred, }; } diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts index 9f6c04878e737..26101e9d247db 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts @@ -11,7 +11,7 @@ import type { RuleParams } from '../../types'; import type { CreateRuleData } from '../create/types'; import type { CreateRuleOptions } from '../create/create_rule'; import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; -import type { RawRule, SanitizedRule } from '../../../../types'; +import type { RawRule } from '../../../../types'; export interface PreparedRule { id: string; @@ -49,8 +49,10 @@ export interface BulkCreateRulesItem { export interface BulkCreateRulesParams { rules: Array>; - // If true, skip Phase 5 (taskManager.bulkEnable); - skipTaskEnabling?: boolean; + /** Per-batch size; clamped to [1, MAX_BULK_CREATE_BATCH_SIZE], defaults to DEFAULT_BULK_CREATE_BATCH_SIZE. Total is bounded by MAX_RULES_NUMBER_FOR_BULK_OPERATION (callers should enforce request-level limits). */ + batchSize?: number; + /** If true, stop further batches on SO-level failure (whole-call throw or any per-row SO error). Phase 1/2/3 demotions never halt the loop. Defaults to false. */ + exitEarlyOnError?: boolean; } export type BulkCreateDisabledReason = @@ -63,9 +65,16 @@ export interface BulkCreateOperationError extends BulkOperationError { disabledReason?: BulkCreateDisabledReason; } -export interface BulkCreateRulesResult { - rules: Array>; +export interface BulkCreateRulesResult { + /** IDs of rules whose SO was successfully persisted. */ + successfulIds: string[]; errors: BulkCreateOperationError[]; total: number; - taskIdsFailedToBeEnabled: string[]; +} + +export interface BatchResult { + successfulIds: string[]; + errors: BulkCreateOperationError[]; + /** True if SO bulkCreate threw or any per-row SO error was returned; drives the exitEarlyOnError short-circuit. */ + soFailureOccurred: boolean; } diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts index 5adc4ed38fe4f..d232bcf35f851 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts @@ -8,7 +8,6 @@ import Boom from '@hapi/boom'; import Semver from 'semver'; import { i18n } from '@kbn/i18n'; -import type { SavedObject } from '@kbn/core/server'; import type { TaskInstanceWithDeprecatedFields } from '@kbn/task-manager-plugin/server/task'; import { validateAndAuthorizeSystemActions } from '../../../../lib/validate_authorize_system_actions'; @@ -32,17 +31,12 @@ import { apiKeyAsAlertAttributes, apiKeyAsRuleDomainProperties, } from '../../../../rules_client/common'; +// import { BULK_TM_SCHEDULE_DELAY } from '../../../../rules_client/common/constants'; import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; -import type { RawRule, RuleTypeRegistry, SanitizedRule } from '../../../../types'; import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; import type { RuleDomain, RuleParams } from '../../types'; -import { - transformRuleAttributesToRuleDomain, - transformRuleDomainToRule, - transformRuleDomainToRuleAttributes, -} from '../../transforms'; -import { ruleDomainSchema } from '../../schemas'; +import { transformRuleDomainToRuleAttributes } from '../../transforms'; import { createRuleDataSchema } from '../create/schemas'; import type { PreparedRule, @@ -67,55 +61,30 @@ export const collectNewKeysToInvalidate = (entries: Iterable): stri return keys; }; +// Task is scheduled `enabled: true` in the future. export const buildTaskInstance = ( context: RulesClientContext, prepared: PreparedRule -): TaskInstanceWithDeprecatedFields => ({ - id: prepared.id, - taskType: `alerting:${prepared.ruleTypeId}`, - schedule: prepared.schedule, - params: { - alertId: prepared.id, - spaceId: context.spaceId, - consumer: prepared.consumer, - }, - state: { - previousStartedAt: null, - alertTypeState: {}, - alertInstances: {}, - }, - scope: ['alerting'], - // Tasks are scheduled disabled. Phase 5 enables them via taskManager.bulkEnable - // so per-task validation drops never produce a running task without a rule SO, - // and the activation can randomise schedule datetimes across the batch. - enabled: false, -}); - -export const toSanitizedRule = ( - context: RulesClientContext, - so: SavedObject, - ruleTypeRegistry: RuleTypeRegistry -): SanitizedRule => { - const ruleType = ruleTypeRegistry.get(so.attributes.alertTypeId); - const ruleDomain: RuleDomain = transformRuleAttributesToRuleDomain( - so.attributes, - { - id: so.id, - logger: context.logger, - ruleType, - references: so.references, - omitGeneratedValues: false, +): TaskInstanceWithDeprecatedFields => { + return { + id: prepared.id, + taskType: `alerting:${prepared.ruleTypeId}`, + schedule: prepared.schedule, + params: { + alertId: prepared.id, + spaceId: context.spaceId, + consumer: prepared.consumer, }, - context.isSystemAction - ); - - try { - ruleDomainSchema.validate(ruleDomain); - } catch (e) { - context.logger.warn(`Error validating bulk-created rule domain object for id: ${so.id}, ${e}`); - } - - return transformRuleDomainToRule(ruleDomain, { isPublic: true }) as SanitizedRule; + state: { + previousStartedAt: null, + alertTypeState: {}, + alertInstances: {}, + }, + scope: ['alerting'], + enabled: true, + runAt: new Date(), + scheduledAt: new Date(), + }; }; export const prepareRule = async ({ diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts index 37c3aead471c0..17dc427a704ee 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable/bulk_enable_rules.ts @@ -16,6 +16,8 @@ import type { SavedObjectsFindResult, } from '@kbn/core/server'; import { withSpan } from '@kbn/apm-utils'; +import type { Logger } from '@kbn/core/server'; +import type { TaskManagerStartContract } from '@kbn/task-manager-plugin/server'; import { TaskStatus } from '@kbn/task-manager-plugin/server'; import type { TaskInstanceWithDeprecatedFields } from '@kbn/task-manager-plugin/server/task'; import { RuleChangeTrackingAction } from '@kbn/alerting-types'; @@ -44,7 +46,6 @@ import type { RulesClientContext, BulkOperationError } from '../../../../rules_c import { validateScheduleLimit } from '../get_schedule_frequency'; import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; import type { BulkEnableRulesParams, BulkEnableRulesResult } from './types'; -import { bulkEnableTasks } from '../bulk_enable_tasks'; import { bulkEnableRulesParamsSchema } from './schemas'; import { transformRuleAttributesToRuleDomain, transformRuleDomainToRule } from '../../transforms'; import { ruleDomainSchema } from '../../schemas'; @@ -116,8 +117,10 @@ export const bulkEnableRules = async ( const [taskIdsToEnable] = accListSpecificForBulkOperation; - const { taskIdsFailedToBeEnabled } = await bulkEnableTasks(context, { - taskIds: taskIdsToEnable, + const taskIdsFailedToBeEnabled = await tryToEnableTasks({ + taskIdsToEnable, + logger: context.logger, + taskManager: context.taskManager, }); const updatedRules = rules.map(({ id, attributes, references }) => { @@ -439,3 +442,49 @@ const bulkEnableRulesWithOCC = async ( accListSpecificForBulkOperation: [taskIdsToEnable], }; }; + +export const tryToEnableTasks = async ({ + taskIdsToEnable, + logger, + taskManager, +}: { + taskIdsToEnable: string[]; + logger: Logger; + taskManager: TaskManagerStartContract; +}) => { + const taskIdsFailedToBeEnabled: string[] = []; + + if (taskIdsToEnable.length > 0) { + try { + const resultFromEnablingTasks = await withSpan( + { name: 'taskManager.bulkEnable', type: 'rules' }, + async () => taskManager.bulkEnable(taskIdsToEnable) + ); + resultFromEnablingTasks?.errors?.forEach((error) => { + taskIdsFailedToBeEnabled.push(error.id); + }); + if (resultFromEnablingTasks.tasks.length) { + logger.debug( + `Successfully enabled schedules for underlying tasks: ${resultFromEnablingTasks.tasks + .map((task) => task.id) + .join(', ')}` + ); + } + if (resultFromEnablingTasks.errors.length) { + logger.error( + `Failure to enable schedules for underlying tasks: ${resultFromEnablingTasks.errors + .map((error) => error.id) + .join(', ')}` + ); + } + } catch (error) { + taskIdsFailedToBeEnabled.push(...taskIdsToEnable); + logger.error( + `Failure to enable schedules for underlying tasks: ${taskIdsToEnable.join( + ', ' + )}. TaskManager bulkEnable failed with Error: ${error.message}` + ); + } + } + return taskIdsFailedToBeEnabled; +}; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts deleted file mode 100644 index 5f0bf05db4d79..0000000000000 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/bulk_enable_tasks.ts +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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 { withSpan } from '@kbn/apm-utils'; -import type { RulesClientContext } from '../../../../rules_client/types'; -import type { BulkEnableTasksParams, BulkEnableTasksResult } from './types'; - -// Enable previously scheduled task manager tasks; returns ids whose enable failed. -export const bulkEnableTasks = async ( - context: RulesClientContext, - params: BulkEnableTasksParams -): Promise => { - const { taskIds } = params; - const { logger, taskManager } = context; - const taskIdsFailedToBeEnabled: string[] = []; - - if (taskIds.length === 0) { - return { taskIdsFailedToBeEnabled }; - } - - try { - const resultFromEnablingTasks = await withSpan( - { name: 'taskManager.bulkEnable', type: 'rules' }, - async () => taskManager.bulkEnable(taskIds) - ); - resultFromEnablingTasks?.errors?.forEach((error) => { - taskIdsFailedToBeEnabled.push(error.id); - }); - if (resultFromEnablingTasks.tasks.length) { - logger.debug( - `Successfully enabled schedules for underlying tasks: ${resultFromEnablingTasks.tasks - .map((task) => task.id) - .join(', ')}` - ); - } - if (resultFromEnablingTasks.errors.length) { - logger.error( - `Failure to enable schedules for underlying tasks: ${resultFromEnablingTasks.errors - .map((error) => error.id) - .join(', ')}` - ); - } - } catch (error) { - taskIdsFailedToBeEnabled.push(...taskIds); - logger.error( - `Failure to enable schedules for underlying tasks: ${taskIds.join( - ', ' - )}. TaskManager bulkEnable failed with Error: ${error.message}` - ); - } - - return { taskIdsFailedToBeEnabled }; -}; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts deleted file mode 100644 index 89c0c7b818ebc..0000000000000 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/index.ts +++ /dev/null @@ -1,10 +0,0 @@ -/* - * 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. - */ - -export type { BulkEnableTasksParams, BulkEnableTasksResult } from './types'; - -export { bulkEnableTasks } from './bulk_enable_tasks'; diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts deleted file mode 100644 index bee29dc832f7a..0000000000000 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/bulk_enable_tasks_types.ts +++ /dev/null @@ -1,14 +0,0 @@ -/* - * 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. - */ - -export interface BulkEnableTasksParams { - taskIds: string[]; -} - -export interface BulkEnableTasksResult { - taskIdsFailedToBeEnabled: string[]; -} diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts deleted file mode 100644 index a3a0bb77a2785..0000000000000 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_enable_tasks/types/index.ts +++ /dev/null @@ -1,8 +0,0 @@ -/* - * 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. - */ - -export type * from './bulk_enable_tasks_types'; diff --git a/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts b/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts index 7f10921c2c50f..c0032d6e8be7c 100644 --- a/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts +++ b/x-pack/platform/plugins/shared/alerting/server/rules_client.mock.ts @@ -53,12 +53,9 @@ const createRulesClientMock = () => { bulkGetRules: jest.fn(), bulkEdit: jest.fn(), bulkEditRuleParamsWithReadAuth: jest.fn(), - bulkCreateRules: jest - .fn() - .mockResolvedValue({ rules: [], errors: [], total: 0, taskIdsFailedToBeEnabled: [] }), + bulkCreateRules: jest.fn().mockResolvedValue({ successfulIds: [], errors: [], total: 0 }), bulkDeleteRules: jest.fn(), bulkEnableRules: jest.fn(), - bulkEnableTasks: jest.fn().mockResolvedValue({ taskIdsFailedToBeEnabled: [] }), bulkDisableRules: jest.fn(), snooze: jest.fn(), unsnooze: jest.fn(), diff --git a/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts b/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts index 321bb2eb18029..af0ffd06b860b 100644 --- a/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts +++ b/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts @@ -24,3 +24,7 @@ export const alertingAuthorizationFilterOpts: AlertingAuthorizationFilterOpts = export const MAX_RULES_NUMBER_FOR_BULK_OPERATION = 10000; export const API_KEY_GENERATE_CONCURRENCY = 50; export const RULE_TYPE_CHECKS_CONCURRENCY = 50; + +export const DEFAULT_BULK_CREATE_BATCH_SIZE = 100; +export const MAX_BULK_CREATE_BATCH_SIZE = 500; +export const BULK_TM_SCHEDULE_DELAY = 30_000; diff --git a/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts b/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts index 5a66e7ff01084..c033d7e8c7b37 100644 --- a/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts +++ b/x-pack/platform/plugins/shared/alerting/server/rules_client/rules_client.ts @@ -64,8 +64,6 @@ import type { BulkEnableRulesParams } from '../application/rule/methods/bulk_ena import { bulkEnableRules } from '../application/rule/methods/bulk_enable'; import type { BulkCreateRulesParams } from '../application/rule/methods/bulk_create'; import { bulkCreateRules } from '../application/rule/methods/bulk_create'; -import type { BulkEnableTasksParams } from '../application/rule/methods/bulk_enable_tasks'; -import { bulkEnableTasks } from '../application/rule/methods/bulk_enable_tasks'; import { enableRule } from '../application/rule/methods/enable_rule/enable_rule'; import { updateRuleApiKey } from '../application/rule/methods/update_api_key/update_rule_api_key'; import { disableRule } from '../application/rule/methods/disable/disable_rule'; @@ -209,7 +207,6 @@ export class RulesClient { options: BulkEditRuleParamsOptions ) => bulkEditRuleParamsWithReadAuth(this.context, options); public bulkEnableRules = (params: BulkEnableRulesParams) => bulkEnableRules(this.context, params); - public bulkEnableTasks = (params: BulkEnableTasksParams) => bulkEnableTasks(this.context, params); public bulkDisableRules = (options: BulkDisableRulesRequestBody) => bulkDisableRules(this.context, options); From 0074370c52dd520a352c87084108be598bf8dd31 Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Thu, 28 May 2026 13:08:33 +0200 Subject: [PATCH 3/8] Additional round of feedback. Move in-memory validation up-front before making ES calls --- .../bulk_create/bulk_create_rules.test.ts | 344 ++++++++++++++---- .../methods/bulk_create/bulk_create_rules.ts | 66 ++-- .../rule/methods/bulk_create/types.ts | 1 - .../rule/methods/bulk_create/utils.ts | 211 +++++++---- .../server/rules_client/common/constants.ts | 1 - 5 files changed, 443 insertions(+), 180 deletions(-) diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts index abeaf5ebf7ef9..e7f2ef6c4228b 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts @@ -31,7 +31,6 @@ import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_a import { validateScheduleLimit } from '../get_schedule_frequency'; import { RuleAuditAction } from '../../../../rules_client/common/audit_events'; import { - BULK_TM_SCHEDULE_DELAY, DEFAULT_BULK_CREATE_BATCH_SIZE, MAX_BULK_CREATE_BATCH_SIZE, MAX_RULES_NUMBER_FOR_BULK_OPERATION, @@ -206,44 +205,35 @@ describe('bulkCreateRules', () => { expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); }); - test('all-enabled happy path: API keys, bulkSchedule with enabled=true and future runAt, bulkCreate, no bulkEnable', async () => { - const now = Date.parse('2024-01-01T00:00:00.000Z'); - jest.useFakeTimers().setSystemTime(now); - try { - unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( - buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) - ); + test('all-enabled happy path: API keys, bulkSchedule with enabled=true (no runAt/scheduledAt), bulkCreate, no bulkEnable', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); - const result = await rulesClient.bulkCreateRules({ - rules: [ - { data: baseRule({ name: 'a', enabled: true }) }, - { data: baseRule({ name: 'b', enabled: true }) }, - ], - }); + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', enabled: true }) }, + { data: baseRule({ name: 'b', enabled: true }) }, + ], + }); - expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(2); - expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); - const scheduled = taskManager.bulkSchedule.mock.calls[0][0] as Array<{ - id: string; - enabled: boolean; - runAt: Date; - scheduledAt: Date; - }>; - expect(scheduled.map((t) => t.id)).toEqual(['mock-id-1', 'mock-id-2']); - expect(scheduled.every((t) => t.enabled === true)).toBe(true); - const minRunAt = now + BULK_TM_SCHEDULE_DELAY; - for (const task of scheduled) { - expect(task.runAt.getTime()).toBeGreaterThanOrEqual(minRunAt); - expect(task.scheduledAt.getTime()).toBe(task.runAt.getTime()); - } - expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); - expect(taskManager.bulkEnable).not.toHaveBeenCalled(); - expect(result.errors).toEqual([]); - expect(result.total).toBe(2); - expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); - } finally { - jest.useRealTimers(); + expect(rulesClientParams.createAPIKey).toHaveBeenCalledTimes(2); + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(1); + const scheduled = taskManager.bulkSchedule.mock.calls[0][0] as Array<{ + id: string; + enabled: boolean; + }>; + expect(scheduled.map((t) => t.id)).toEqual(['mock-id-1', 'mock-id-2']); + expect(scheduled.every((t) => t.enabled === true)).toBe(true); + for (const task of scheduled) { + expect(task).not.toHaveProperty('runAt'); + expect(task).not.toHaveProperty('scheduledAt'); } + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(taskManager.bulkEnable).not.toHaveBeenCalled(); + expect(result.errors).toEqual([]); + expect(result.total).toBe(2); + expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); }); test('mixed enabled+disabled happy path', async () => { @@ -265,7 +255,7 @@ describe('bulkCreateRules', () => { expect(result.successfulIds).toEqual(['mock-id-1', 'mock-id-2']); }); - test('Phase 1 per-rule throw is isolated', async () => { + test('Phase B1 per-rule throw is isolated', async () => { let calls = 0; ruleTypeRegistry.get.mockImplementation((typeId: string) => { calls += 1; @@ -303,7 +293,7 @@ describe('bulkCreateRules', () => { expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(1); }); - test('Phase 1 API key creation failure: enabled rule degrades to disabled, no key minted, SO still written, no scheduling', async () => { + test('Phase B1 API key creation failure: enabled rule degrades to disabled, no key minted, SO still written, no scheduling', async () => { rulesClientParams.createAPIKey.mockRejectedValueOnce(new Error('keys disabled')); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) @@ -344,7 +334,7 @@ describe('bulkCreateRules', () => { expect(result.errors[0].message).toContain('keys disabled'); }); - test('Phase 2 schedule-limit trip: enabled rule degrades to disabled, key invalidated, SO still written for both', async () => { + test('Phase B2 schedule-limit trip: enabled rule degrades to disabled, key invalidated, SO still written for both', async () => { (validateScheduleLimit as jest.Mock).mockResolvedValue({ interval: 100, intervalAvailable: 50, @@ -384,7 +374,22 @@ describe('bulkCreateRules', () => { expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); }); - test('Phase 3 whole TM throw: enabled subset degrades to disabled, disabled subset unaffected', async () => { + test('Phase B2: validateScheduleLimit called exactly once per batch', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => + buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) + ); + + await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 4 }, (_, i) => ({ + data: baseRule({ name: `r-${i}`, enabled: true }), + })), + batchSize: 2, + }); + + expect(validateScheduleLimit).toHaveBeenCalledTimes(2); + }); + + test('Phase B3 whole TM throw: enabled subset degrades to disabled, disabled subset unaffected', async () => { taskManager.bulkSchedule.mockRejectedValueOnce(new Error('cluster unavailable')); unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) @@ -412,7 +417,7 @@ describe('bulkCreateRules', () => { expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); }); - test('Phase 3 silent per-task drop: dropped rule degrades to disabled and SO is still written', async () => { + test('Phase B3 silent per-task drop: dropped rule degrades to disabled and SO is still written', async () => { taskManager.bulkSchedule.mockImplementationOnce(async (tasks) => { return [tasks[0]] as never; }); @@ -439,7 +444,7 @@ describe('bulkCreateRules', () => { expect(result.errors[0].message.startsWith(BULK_CREATE_AS_DISABLED_PREFIX)).toBe(true); }); - test('Phase 4 per-row error on Phase-3-scheduled id: per-rule key invalidated, bulkRemove called', async () => { + test('Phase B4 per-row error on Phase-B3-scheduled id: per-rule key invalidated, bulkRemove called', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( buildBulkResponse([{ id: 'mock-id-1', error: { message: 'conflict', statusCode: 409 } }]) ); @@ -457,7 +462,7 @@ describe('bulkCreateRules', () => { expect(result.successfulIds).toEqual([]); }); - test('Phase 4 per-row error on caller-supplied id NOT in newlyScheduledTaskIds: NO TM cleanup', async () => { + test('Phase B4 per-row error on caller-supplied id NOT in newlyScheduledTaskIds: NO TM cleanup', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( buildBulkResponse([{ id: 'caller-id', error: { message: 'conflict', statusCode: 409 } }]) ); @@ -470,7 +475,7 @@ describe('bulkCreateRules', () => { expect(taskManager.bulkRemove).not.toHaveBeenCalled(); }); - test('Phase 4 whole-call throw: invalidates keys, bulkRemoves scheduled ids, surfaces SO failure in errors (does NOT rethrow)', async () => { + test('Phase B4 whole-call throw: invalidates keys, bulkRemoves scheduled ids, surfaces SO failure in errors (does NOT rethrow)', async () => { unsecuredSavedObjectsClient.bulkCreate.mockRejectedValueOnce(new Error('SO down')); const result = await rulesClient.bulkCreateRules({ @@ -487,7 +492,7 @@ describe('bulkCreateRules', () => { }); test('every demotion path stamps a machine-readable disabledReason on the error', async () => { - // schedule_limit_exceeded + // schedule_limit_exceeded (Phase B2) (validateScheduleLimit as jest.Mock).mockResolvedValueOnce({ interval: 100, intervalAvailable: 50, @@ -495,10 +500,10 @@ describe('bulkCreateRules', () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( buildBulkResponse([{ id: 'mock-id-1' }]) ); - const phase2 = await rulesClient.bulkCreateRules({ + const phase0 = await rulesClient.bulkCreateRules({ rules: [{ data: baseRule({ name: 'a', enabled: true }) }], }); - expect(phase2.errors[0]).toEqual( + expect(phase0.errors[0]).toEqual( expect.objectContaining({ disabledReason: 'schedule_limit_exceeded' }) ); @@ -507,10 +512,10 @@ describe('bulkCreateRules', () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( buildBulkResponse([{ id: 'mock-id-2' }]) ); - const phase3a = await rulesClient.bulkCreateRules({ + const phase2a = await rulesClient.bulkCreateRules({ rules: [{ data: baseRule({ name: 'b', enabled: true }) }], }); - expect(phase3a.errors[0]).toEqual( + expect(phase2a.errors[0]).toEqual( expect.objectContaining({ disabledReason: 'task_schedule_failed' }) ); @@ -519,10 +524,10 @@ describe('bulkCreateRules', () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValueOnce( buildBulkResponse([{ id: 'mock-id-3' }]) ); - const phase3b = await rulesClient.bulkCreateRules({ + const phase2b = await rulesClient.bulkCreateRules({ rules: [{ data: baseRule({ name: 'c', enabled: true }) }], }); - expect(phase3b.errors[0]).toEqual( + expect(phase2b.errors[0]).toEqual( expect.objectContaining({ disabledReason: 'task_validation_failed' }) ); @@ -558,6 +563,201 @@ describe('bulkCreateRules', () => { expect(actions.filter((a) => a === RuleAuditAction.ENABLE)).toHaveLength(1); }); + describe('preValidate', () => { + test('per-rule isolation: one registry-throw, two valid → one error, two survive to runBatch', async () => { + let calls = 0; + ruleTypeRegistry.get.mockImplementation((typeId: string) => { + calls += 1; + if (calls === 1) throw new Error('unregistered type'); + return { + id: typeId, + name: 'Test', + actionGroups: [{ id: 'default', name: 'Default' }], + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + async executor() { + return { state: {} }; + }, + category: 'test', + producer: 'alerts', + solution: 'stack', + validate: { params: { validate: (p: unknown) => p } }, + validLegacyConsumers: [], + } as never; + }); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-2' }, { id: 'mock-id-3' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'invalid' }) }, + { data: baseRule({ name: 'ok-1' }) }, + { data: baseRule({ name: 'ok-2' }) }, + ], + }); + + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.name).toBe('invalid'); + expect(result.successfulIds).toEqual(['mock-id-2', 'mock-id-3']); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(2); + }); + + test('all inputs fail preValidate: zero calls to authorization.ensureAuthorized, bulkSchedule, bulkCreate, createAPIKey', async () => { + ruleTypeRegistry.get.mockImplementation(() => { + throw new Error('unregistered'); + }); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }, { data: baseRule({ name: 'b' }) }], + }); + + expect(result.errors).toHaveLength(2); + expect(result.successfulIds).toEqual([]); + expect(authorization.ensureAuthorized).not.toHaveBeenCalled(); + expect(rulesClientParams.createAPIKey).not.toHaveBeenCalled(); + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + expect(unsecuredSavedObjectsClient.bulkCreate).not.toHaveBeenCalled(); + }); + + test('exitEarlyOnError=true + preValidate error → returns immediately, zero ES writes', async () => { + ruleTypeRegistry.get.mockImplementationOnce(() => { + throw new Error('unregistered'); + }); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'bad' }) }, { data: baseRule({ name: 'good' }) }], + exitEarlyOnError: true, + }); + + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.name).toBe('bad'); + expect(result.successfulIds).toEqual([]); + expect(unsecuredSavedObjectsClient.bulkCreate).not.toHaveBeenCalled(); + expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); + }); + + test('parseDuration throws on malformed interval: per-rule error, other rules unaffected', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-2' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'bad-interval', schedule: { interval: 'NOT_VALID' } }) }, + { data: baseRule({ name: 'ok' }) }, + ], + }); + + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.name).toBe('bad-interval'); + expect(result.successfulIds).toEqual(['mock-id-2']); + }); + + test('minimum-interval enforce=true: rule removed with 400 error', async () => { + const enforceClient = new RulesClient({ + ...rulesClientParams, + minimumScheduleInterval: { value: '1m', enforce: true }, + }); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-2' }]) + ); + + const result = await enforceClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'too-fast', schedule: { interval: '30s' } }) }, + { data: baseRule({ name: 'ok' }) }, + ], + }); + + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.name).toBe('too-fast'); + expect(result.errors[0].message).toContain('less than the allowed minimum interval'); + expect(result.successfulIds).toEqual(['mock-id-2']); + }); + + test('minimum-interval enforce=false: logger.warn called, rule retained and forwarded to runBatch', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'fast', schedule: { interval: '30s' } }) }], + }); + + const warnCalls = (rulesClientParams.logger.warn as jest.Mock).mock.calls + .map((c) => c[0]) + .filter((m: string) => m?.includes?.('less than the minimum value')); + expect(warnCalls).toHaveLength(1); + expect(result.errors).toEqual([]); + expect(result.successfulIds).toEqual(['mock-id-1']); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + }); + + test('deduped per-pair authz: two rules same pair, rejected → ensureAuthorized called once, both get per-rule audit+error', async () => { + (authorization.ensureAuthorized as jest.Mock).mockRejectedValueOnce( + new Error('not authorized') + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a', alertTypeId: '123', consumer: 'bar' }) }, + { data: baseRule({ name: 'b', alertTypeId: '123', consumer: 'bar' }) }, + ], + }); + + expect(authorization.ensureAuthorized).toHaveBeenCalledTimes(1); + expect(result.errors).toHaveLength(2); + expect(result.successfulIds).toEqual([]); + expect(unsecuredSavedObjectsClient.bulkCreate).not.toHaveBeenCalled(); + + const failAudits = (auditLogger.log as jest.Mock).mock.calls + .map(([event]) => event) + .filter( + (e: { event?: { action: string; outcome?: string } }) => + e?.event?.action === RuleAuditAction.CREATE && e?.event?.outcome === 'failure' + ); + expect(failAudits).toHaveLength(2); + }); + + test('partial authz: pair A authorized, pair B rejected → pair A survives, pair B gets audit+error', async () => { + (authorization.ensureAuthorized as jest.Mock).mockImplementation( + async ({ consumer }: { consumer: string }) => { + if (consumer === 'other') throw new Error('not authorized for other'); + } + ); + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'allowed', alertTypeId: '123', consumer: 'bar' }) }, + { data: baseRule({ name: 'blocked', alertTypeId: '123', consumer: 'other' }) }, + ], + }); + + expect(result.errors).toHaveLength(1); + expect(result.errors[0].rule.name).toBe('blocked'); + expect(result.successfulIds).toEqual(['mock-id-1']); + expect(unsecuredSavedObjectsClient.bulkCreate.mock.calls[0][0]).toHaveLength(1); + }); + + test('ids are assigned before preValidate (caller-supplied id passed through)', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'my-custom-id' }]) + ); + + const result = await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'custom' }), options: { id: 'my-custom-id' } }], + }); + + expect(result.successfulIds).toEqual(['my-custom-id']); + }); + }); + describe('batching', () => { test('splits rules across batches and concatenates results', async () => { const ruleCount = DEFAULT_BULK_CREATE_BATCH_SIZE * 2 + 1; // forces 3 batches with default batch size @@ -626,30 +826,24 @@ describe('bulkCreateRules', () => { expect(taskManager.bulkSchedule).not.toHaveBeenCalled(); }); - test('per-batch runAt is at least the buffer beyond now', async () => { - const now = Date.parse('2024-01-01T00:00:00.000Z'); - jest.useFakeTimers().setSystemTime(now); - try { - unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => - buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) - ); + test('task instances passed to bulkSchedule do not contain runAt or scheduledAt', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockImplementation(async (objects) => + buildBulkResponse((objects as Array<{ id: string }>).map((o) => ({ id: o.id }))) + ); + + await rulesClient.bulkCreateRules({ + rules: Array.from({ length: 4 }, (_, i) => ({ + data: baseRule({ name: `r-${i}`, enabled: true }), + })), + batchSize: 2, + }); - await rulesClient.bulkCreateRules({ - rules: Array.from({ length: 4 }, (_, i) => ({ - data: baseRule({ name: `r-${i}`, enabled: true }), - })), - batchSize: 2, - }); - - expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(2); - const minRunAt = now + BULK_TM_SCHEDULE_DELAY; - for (const call of taskManager.bulkSchedule.mock.calls) { - for (const t of call[0] as Array<{ runAt: Date }>) { - expect(t.runAt.getTime()).toBeGreaterThanOrEqual(minRunAt); - } + expect(taskManager.bulkSchedule).toHaveBeenCalledTimes(2); + for (const call of taskManager.bulkSchedule.mock.calls) { + for (const t of call[0] as unknown as Array>) { + expect(t).not.toHaveProperty('runAt'); + expect(t).not.toHaveProperty('scheduledAt'); } - } finally { - jest.useRealTimers(); } }); }); @@ -715,8 +909,8 @@ describe('bulkCreateRules', () => { expect(result.errors.some((e) => e.message.includes('SO down'))).toBe(true); }); - test('true: does NOT trigger on Phase-1/2/3 demotion errors (only on SO failures)', async () => { - // Batch 1: schedule limit trips on the enabled rule (Phase 2), SO succeeds. + test('true: does NOT trigger on Phase-B1/B2/B3 demotion errors (only on SO failures)', async () => { + // Batch 1: schedule limit trips on the enabled rule (Phase B2), SO succeeds. // Batch 2: should still execute. (validateScheduleLimit as jest.Mock).mockResolvedValueOnce({ interval: 100, diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts index 420b10f4c9d36..9719e938f1953 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -40,6 +40,7 @@ import { demotePreparedRules, flushKeysToInvalidate, prepareRule, + preValidate, } from './utils'; export async function bulkCreateRules( @@ -72,17 +73,38 @@ export async function bulkCreateRules( const username = await context.getUserName(); const actionsClient = await context.getActionsClient(); - const successfulIds: string[] = []; const errors: BulkCreateOperationError[] = []; - const totalBatches = Math.ceil(total / batchSize); - logger.debug(`bulkCreateRules: ${total} rules, ${totalBatches}x batches of ${batchSize} rules.`); + const inputs = rules.map((rule) => ({ + id: rule.options?.id ?? SavedObjectsUtils.generateId(), + rule, + })); + + // Phase A: Validate in-memory (schema, rule type enabled, check params, etc...). Then authorize consumer/alertTypeId pairs. + const { validated, errors: validationErrors } = await preValidate({ context, inputs }); + errors.push(...validationErrors); + + if (validationErrors.length > 0 && exitEarlyOnError) { + logger.warn( + `bulkCreateRules: exiting early on preValidate; ${validationErrors.length} rule(s) failed pre-flight, zero ES writes.` + ); + return { successfulIds, errors, total }; + } + if (validated.length === 0) { + return { successfulIds, errors, total }; + } + + const totalBatches = Math.ceil(validated.length / batchSize); + logger.debug( + `bulkCreateRules: ${total} input(s), ${validated.length} validated after preValidate, ${totalBatches}x batches of ${batchSize}.` + ); for (let batchIndex = 0; batchIndex < totalBatches; batchIndex++) { const start = batchIndex * batchSize; - const batch = rules.slice(start, start + batchSize); + const batch = validated.slice(start, start + batchSize); + // Phase B: per-batch ES writes. const result = await runBatch({ context, username, @@ -99,7 +121,7 @@ export async function bulkCreateRules( batchIndex + 1 }/${totalBatches}. ` + `${successfulIds.length} rule(s) created, ${ - total - start - batch.length + validated.length - start - batch.length } rule(s) skipped.` ); break; @@ -113,7 +135,7 @@ interface RunBatchArgs { context: RulesClientContext; username: string | null; actionsClient: Awaited>; - batch: Array>; + batch: Array<{ id: string; rule: BulkCreateRulesItem }>; } async function runBatch({ @@ -124,23 +146,16 @@ async function runBatch({ }: RunBatchArgs): Promise { const { logger } = context; - const inputsWithIds = batch.map((rule) => ({ - id: rule.options?.id ?? SavedObjectsUtils.generateId(), - rule, - })); - - // Phase 1: per-rule prepare (validation + api key generation). // NOTE: in order to minimise external calls, the values below get mutated - // at different stages in the process (ie if we fail schedule creation), - // so that we invalidate keys and create rule SOs in a single batch at the end. + // at different stages in the process (ie if we fail schedule creation). const preparedRules = new Map(); const keysToInvalidate = new Set(); const apiKeysMap = new Map(); const errors: BulkCreateOperationError[] = []; - const authzCache = new Map>(); + // Phase B1: per-rule prepare (high latency validation + API key generation). await pMap( - inputsWithIds, + batch, async ({ id, rule }) => { const { prepared, error } = await prepareRule({ context, @@ -148,7 +163,6 @@ async function runBatch({ username, id, rule, - authzCache, errors, apiKeysMap, }); @@ -164,14 +178,16 @@ async function runBatch({ return { successfulIds: [], errors, soFailureOccurred: false }; } - // Phase 2: validate schedule-limits, enabled subset only. + // Phase B2: check schedule-limit on the enabled subset; demote on overflow. const enabled = [...preparedRules.values()].filter((p) => p.enabled); if (enabled.length > 0) { - const updatedInterval = enabled.map((r) => r.schedule.interval); - const validationPayload = await validateScheduleLimit({ context, updatedInterval }); - const enabledIds = enabled.map((p) => p.id); + const validationPayload = await validateScheduleLimit({ + context, + updatedInterval: enabled.map((r) => r.schedule.interval), + }); if (validationPayload) { + const enabledIds = enabled.map((p) => p.id); const reasonMessage = getRuleCircuitBreakerErrorMessage({ interval: validationPayload.interval, intervalAvailable: validationPayload.intervalAvailable, @@ -192,7 +208,7 @@ async function runBatch({ } } - // Phase 3: schedule tasks for the surviving enabled subset. + // Phase B3: schedule tasks for the surviving enabled subset. const survivingEnabled = [...preparedRules.values()].filter((p) => p.enabled); const newlyScheduledTaskIds = new Set(); @@ -262,7 +278,7 @@ async function runBatch({ ); } - // Phase 4: bulk SO create (no overwrite — id collisions surface as per-row 409) + // Phase B4: bulk SO create (no overwrite — id collisions surface as per-row 409). const bulkObjects: Array> = [...preparedRules.values()].map( (prepared) => ({ type: RULE_SAVED_OBJECT_TYPE, @@ -306,7 +322,7 @@ async function runBatch({ return { successfulIds: [], errors, soFailureOccurred: true }; } - // Phase 4 per-row outcomes. + // Phase B4 per-row outcomes. const batchSuccessfulIds: string[] = []; const taskIdsToCleanUp: string[] = []; let perRowFailureOccurred = false; @@ -325,7 +341,7 @@ async function runBatch({ for (const k of collectNewKeysToInvalidate([apiKey])) keysToInvalidate.add(k); } - // Only ids we scheduled in Phase 3. Skipping caller-supplied id collisions + // Only ids we scheduled in Phase B3. Skipping caller-supplied id collisions // avoids nuking a pre-existing rule's task on a 409. if (newlyScheduledTaskIds.has(so.id)) { taskIdsToCleanUp.push(so.id); diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts index 26101e9d247db..a651953d34eed 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts @@ -36,7 +36,6 @@ export interface PrepareRuleArgs { username: string | null; id: string; rule: BulkCreateRulesItem; - authzCache: Map>; errors: BulkCreateOperationError[]; apiKeysMap: Map; } diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts index d232bcf35f851..c079cfce84a00 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts @@ -31,7 +31,6 @@ import { apiKeyAsAlertAttributes, apiKeyAsRuleDomainProperties, } from '../../../../rules_client/common'; -// import { BULK_TM_SCHEDULE_DELAY } from '../../../../rules_client/common/constants'; import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; @@ -44,6 +43,7 @@ import type { ApiKeyEntry, BulkCreateOperationError, BulkCreateDisabledReason, + BulkCreateRulesItem, } from './types'; export const getBulkCreateAsDisabledMessage = (message: string): string => @@ -61,30 +61,140 @@ export const collectNewKeysToInvalidate = (entries: Iterable): stri return keys; }; -// Task is scheduled `enabled: true` in the future. +// Create tasks `enabled:true`, runAt / scheduledAt intentionally omitted. +// TM addJitter (PR# 269991) applies on bulkSchedule(), significantly speeding up the process. export const buildTaskInstance = ( context: RulesClientContext, prepared: PreparedRule -): TaskInstanceWithDeprecatedFields => { - return { - id: prepared.id, - taskType: `alerting:${prepared.ruleTypeId}`, - schedule: prepared.schedule, - params: { - alertId: prepared.id, - spaceId: context.spaceId, - consumer: prepared.consumer, - }, - state: { - previousStartedAt: null, - alertTypeState: {}, - alertInstances: {}, - }, - scope: ['alerting'], - enabled: true, - runAt: new Date(), - scheduledAt: new Date(), - }; +): TaskInstanceWithDeprecatedFields => ({ + id: prepared.id, + taskType: `alerting:${prepared.ruleTypeId}`, + schedule: prepared.schedule, + params: { + alertId: prepared.id, + spaceId: context.spaceId, + consumer: prepared.consumer, + }, + state: { + previousStartedAt: null, + alertTypeState: {}, + alertInstances: {}, + }, + scope: ['alerting'], + enabled: true, +}); + +export const preValidate = async ({ + context, + inputs, +}: { + context: RulesClientContext; + inputs: Array<{ id: string; rule: BulkCreateRulesItem }>; +}): Promise<{ + validated: Array<{ id: string; rule: BulkCreateRulesItem }>; + errors: BulkCreateOperationError[]; +}> => { + const validated = new Map }>(); + const errors: BulkCreateOperationError[] = []; + const authPairs = new Map< + string, + { ruleTypeId: string; consumer: string; ids: string[]; names: Map } + >(); + + // Phase A1: per-rule in-memory checks, sequential, cheapest-first. + for (const { id, rule } of inputs) { + try { + const { actions: genActions, systemActions: genSystemActions } = + await addGeneratedActionValues(rule.data.actions, rule.data.systemActions, context); + const data = { ...rule.data, actions: genActions, systemActions: genSystemActions }; + + try { + createRuleDataSchema.validate(data); + } catch (err) { + throw Boom.badRequest(`Error validating create data - ${err.message}`); + } + + // ruleTypeRegistry.get throws 400 if not registered. + const ruleType = context.ruleTypeRegistry.get(data.alertTypeId); + context.ruleTypeRegistry.ensureRuleTypeEnabled(data.alertTypeId); + validateRuleTypeParams(data.params, ruleType.validate.params); + + const intervalInMs = parseDuration(data.schedule.interval); + if ( + intervalInMs < context.minimumScheduleIntervalInMs && + context.minimumScheduleInterval.enforce + ) { + throw Boom.badRequest( + `Error creating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` + ); + } + if ( + intervalInMs < context.minimumScheduleIntervalInMs && + !context.minimumScheduleInterval.enforce + ) { + context.logger.warn( + `Rule schedule interval (${data.schedule.interval}) for "${ruleType.id}" rule type with ID "${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 creation of these rules.` + ); + } + + const authzKey = `${data.alertTypeId}::${data.consumer}`; + const pair = authPairs.get(authzKey); + if (pair) { + pair.ids.push(id); + pair.names.set(id, data.name); + } else { + authPairs.set(authzKey, { + ruleTypeId: data.alertTypeId, + consumer: data.consumer, + ids: [id], + names: new Map([[id, data.name]]), + }); + } + + validated.set(id, { id, rule }); + } catch (err) { + errors.push({ + message: err.message, + status: err.output?.statusCode, + rule: { id, name: rule.data?.name ?? 'n/a' }, + }); + } + } + + if (validated.size === 0) { + return { validated: [], errors }; + } + + // Phase A2: deduped per-pair authorization. + for (const { ruleTypeId, consumer, ids, names } of authPairs.values()) { + try { + await context.authorization.ensureAuthorized({ + ruleTypeId, + consumer, + operation: WriteOperations.Create, + entity: AlertingAuthorizationEntity.Rule, + }); + } catch (authzError) { + // One audit per rule in failing set. Following single rule `create()`. + for (const ruleId of ids) { + context.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.CREATE, + savedObject: { type: RULE_SAVED_OBJECT_TYPE, id: ruleId, name: names.get(ruleId)! }, + error: authzError, + }) + ); + errors.push({ + message: authzError.message, + status: authzError.output?.statusCode, + rule: { id: ruleId, name: names.get(ruleId) ?? 'n/a' }, + }); + validated.delete(ruleId); + } + } + } + + return { validated: [...validated.values()], errors }; }; export const prepareRule = async ({ @@ -93,7 +203,6 @@ export const prepareRule = async ({ username, id, rule, - authzCache, errors, apiKeysMap, }: PrepareRuleArgs): Promise<{ prepared?: PreparedRule; error?: BulkOperationError }> => { @@ -107,41 +216,6 @@ export const prepareRule = async ({ ); const data = { ...rule.data, actions: genActions, systemActions: genSystemActions }; - try { - createRuleDataSchema.validate(data); - } catch (validationError) { - throw Boom.badRequest(`Error validating create data - ${validationError.message}`); - } - - // ruleTypeRegistry.get throws 400 if not registered. - context.ruleTypeRegistry.get(data.alertTypeId); - - const authzKey = `${data.alertTypeId}::${data.consumer}`; - if (!authzCache.has(authzKey)) { - authzCache.set( - authzKey, - context.authorization.ensureAuthorized({ - ruleTypeId: data.alertTypeId, - consumer: data.consumer, - operation: WriteOperations.Create, - entity: AlertingAuthorizationEntity.Rule, - }) - ); - } - try { - await authzCache.get(authzKey)!; - } catch (authzError) { - context.auditLogger?.log( - ruleAuditEvent({ - action: RuleAuditAction.CREATE, - savedObject: { type: RULE_SAVED_OBJECT_TYPE, id, name: data.name }, - error: authzError, - }) - ); - throw authzError; - } - - context.ruleTypeRegistry.ensureRuleTypeEnabled(data.alertTypeId); const ruleType = context.ruleTypeRegistry.get(data.alertTypeId); const validatedRuleTypeParams = validateRuleTypeParams(data.params, ruleType.validate.params); @@ -154,28 +228,9 @@ export const prepareRule = async ({ rule: { consumer: data.consumer, producer: ruleType.producer }, }); - const intervalInMs = parseDuration(data.schedule.interval); - if ( - intervalInMs < context.minimumScheduleIntervalInMs && - context.minimumScheduleInterval.enforce - ) { - throw Boom.badRequest( - `Error creating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` - ); - } - if ( - intervalInMs < context.minimumScheduleIntervalInMs && - !context.minimumScheduleInterval.enforce - ) { - context.logger.warn( - `Rule schedule interval (${data.schedule.interval}) for "${ruleType.id}" rule type with ID "${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 creation of these rules.` - ); - } - // Mint API key for enabled rules. // Soft-fail: a key-mint failure does NOT reject the rule. We persist it as - // disabled, push a degraded error so the caller knows, and let downstream - // phases treat it as a never-enabled rule. Re-enabling later will re-mint. + // disabled, push a degraded error so the caller knows. let effectiveEnabled = data.enabled; let apiKeyProps: | ReturnType diff --git a/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts b/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts index af0ffd06b860b..7234d86539b4e 100644 --- a/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts +++ b/x-pack/platform/plugins/shared/alerting/server/rules_client/common/constants.ts @@ -27,4 +27,3 @@ export const RULE_TYPE_CHECKS_CONCURRENCY = 50; export const DEFAULT_BULK_CREATE_BATCH_SIZE = 100; export const MAX_BULK_CREATE_BATCH_SIZE = 500; -export const BULK_TM_SCHEDULE_DELAY = 30_000; From d6d1463549d88be4562fabce6b43f4481edfe04e Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Thu, 28 May 2026 13:45:23 +0200 Subject: [PATCH 4/8] Review APM, move preValidate to bulk_create_rules for visibility in key steps. --- .../methods/bulk_create/bulk_create_rules.ts | 175 +++++++++++++++--- .../rule/methods/bulk_create/utils.ts | 120 ------------ 2 files changed, 150 insertions(+), 145 deletions(-) diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts index 9719e938f1953..8ec7a26e3deda 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -11,8 +11,10 @@ import { withSpan } from '@kbn/apm-utils'; import type { SavedObjectsBulkCreateObject } from '@kbn/core/server'; import { SavedObjectsUtils } from '@kbn/core/server'; import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; -import { getRuleCircuitBreakerErrorMessage } from '../../../../../common'; -import { updateMeta } from '../../../../rules_client/lib'; +import { getRuleCircuitBreakerErrorMessage, parseDuration } from '../../../../../common'; +import { addGeneratedActionValues, updateMeta } from '../../../../rules_client/lib'; +import { validateRuleTypeParams } from '../../../../lib'; +import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; import { API_KEY_GENERATE_CONCURRENCY, DEFAULT_BULK_CREATE_BATCH_SIZE, @@ -24,6 +26,7 @@ import { bulkCreateRulesSo } from '../../../../data/rule'; import type { RawRule } from '../../../../types'; import type { RulesClientContext } from '../../../../rules_client/types'; import type { RuleParams } from '../../types'; +import { createRuleDataSchema } from '../create/schemas'; import { validateScheduleLimit } from '../get_schedule_frequency'; import type { ApiKeyEntry, @@ -40,7 +43,6 @@ import { demotePreparedRules, flushKeysToInvalidate, prepareRule, - preValidate, } from './utils'; export async function bulkCreateRules( @@ -131,6 +133,123 @@ export async function bulkCreateRules( return { successfulIds, errors, total }; } +async function preValidate({ + context, + inputs, +}: { + context: RulesClientContext; + inputs: Array<{ id: string; rule: BulkCreateRulesItem }>; +}): Promise<{ + validated: Array<{ id: string; rule: BulkCreateRulesItem }>; + errors: BulkCreateOperationError[]; +}> { + const validated = new Map }>(); + const errors: BulkCreateOperationError[] = []; + const authPairs = new Map< + string, + { ruleTypeId: string; consumer: string; ids: string[]; names: Map } + >(); + + // Phase A1: per-rule in-memory checks, sequential, cheapest-first. + await withSpan({ name: 'preValidate.checkInMemory', type: 'rules' }, async () => { + for (const { id, rule } of inputs) { + try { + const { actions: genActions, systemActions: genSystemActions } = + await addGeneratedActionValues(rule.data.actions, rule.data.systemActions, context); + const data = { ...rule.data, actions: genActions, systemActions: genSystemActions }; + + try { + createRuleDataSchema.validate(data); + } catch (err) { + throw Boom.badRequest(`Error validating create data - ${err.message}`); + } + + // ruleTypeRegistry.get throws 400 if not registered. + const ruleType = context.ruleTypeRegistry.get(data.alertTypeId); + context.ruleTypeRegistry.ensureRuleTypeEnabled(data.alertTypeId); + validateRuleTypeParams(data.params, ruleType.validate.params); + + const intervalInMs = parseDuration(data.schedule.interval); + if ( + intervalInMs < context.minimumScheduleIntervalInMs && + context.minimumScheduleInterval.enforce + ) { + throw Boom.badRequest( + `Error creating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` + ); + } + if ( + intervalInMs < context.minimumScheduleIntervalInMs && + !context.minimumScheduleInterval.enforce + ) { + context.logger.warn( + `Rule schedule interval (${data.schedule.interval}) for "${ruleType.id}" rule type with ID "${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 creation of these rules.` + ); + } + + const authzKey = `${data.alertTypeId}::${data.consumer}`; + const pair = authPairs.get(authzKey); + if (pair) { + pair.ids.push(id); + pair.names.set(id, data.name); + } else { + authPairs.set(authzKey, { + ruleTypeId: data.alertTypeId, + consumer: data.consumer, + ids: [id], + names: new Map([[id, data.name]]), + }); + } + + validated.set(id, { id, rule }); + } catch (err) { + errors.push({ + message: err.message, + status: err.output?.statusCode, + rule: { id, name: rule.data?.name ?? 'n/a' }, + }); + } + } + }); + + if (validated.size === 0) { + return { validated: [], errors }; + } + + // Phase A2: deduped per-pair authorization. + await withSpan({ name: 'preValidate.ensureAuthorized', type: 'rules' }, async () => { + for (const { ruleTypeId, consumer, ids, names } of authPairs.values()) { + try { + await context.authorization.ensureAuthorized({ + ruleTypeId, + consumer, + operation: WriteOperations.Create, + entity: AlertingAuthorizationEntity.Rule, + }); + } catch (authzError) { + // One audit per rule in failing set. Following single rule `create()`. + for (const ruleId of ids) { + context.auditLogger?.log( + ruleAuditEvent({ + action: RuleAuditAction.CREATE, + savedObject: { type: RULE_SAVED_OBJECT_TYPE, id: ruleId, name: names.get(ruleId)! }, + error: authzError, + }) + ); + errors.push({ + message: authzError.message, + status: authzError.output?.statusCode, + rule: { id: ruleId, name: names.get(ruleId) ?? 'n/a' }, + }); + validated.delete(ruleId); + } + } + } + }); + + return { validated: [...validated.values()], errors }; +} + interface RunBatchArgs { context: RulesClientContext; username: string | null; @@ -154,22 +273,24 @@ async function runBatch({ const errors: BulkCreateOperationError[] = []; // Phase B1: per-rule prepare (high latency validation + API key generation). - await pMap( - batch, - async ({ id, rule }) => { - const { prepared, error } = await prepareRule({ - context, - actionsClient, - username, - id, - rule, - errors, - apiKeysMap, - }); - if (prepared) preparedRules.set(id, prepared); - else if (error) errors.push(error); - }, - { concurrency: API_KEY_GENERATE_CONCURRENCY } + await withSpan({ name: 'runBatch.pMap.prepareRule', type: 'rules' }, () => + pMap( + batch, + async ({ id, rule }) => { + const { prepared, error } = await prepareRule({ + context, + actionsClient, + username, + id, + rule, + errors, + apiKeysMap, + }); + if (prepared) preparedRules.set(id, prepared); + else if (error) errors.push(error); + }, + { concurrency: API_KEY_GENERATE_CONCURRENCY } + ) ); // No survivors? Flush any keys created and return. @@ -182,10 +303,14 @@ async function runBatch({ const enabled = [...preparedRules.values()].filter((p) => p.enabled); if (enabled.length > 0) { - const validationPayload = await validateScheduleLimit({ - context, - updatedInterval: enabled.map((r) => r.schedule.interval), - }); + const validationPayload = await withSpan( + { name: 'runBatch.validateScheduleLimit', type: 'rules' }, + () => + validateScheduleLimit({ + context, + updatedInterval: enabled.map((r) => r.schedule.interval), + }) + ); if (validationPayload) { const enabledIds = enabled.map((p) => p.id); const reasonMessage = getRuleCircuitBreakerErrorMessage({ @@ -221,7 +346,7 @@ async function runBatch({ const survivingEnabledIds = survivingEnabled.map((p) => p.id); try { const scheduledTasks = await withSpan( - { name: 'taskManager.bulkSchedule', type: 'tasks' }, + { name: 'runBatch.bulkSchedule', type: 'tasks' }, () => context.taskManager.bulkSchedule(tasksToSchedule) ); scheduledIds = scheduledTasks.map((task) => task.id); @@ -291,7 +416,7 @@ async function runBatch({ let bulkResponse; try { bulkResponse = await withSpan( - { name: 'unsecuredSavedObjectsClient.bulkCreate', type: 'rules' }, + { name: 'runBatch.bulkCreateRulesSo', type: 'rules' }, () => bulkCreateRulesSo({ savedObjectsClient: context.unsecuredSavedObjectsClient, diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts index c079cfce84a00..e7dd994f0f725 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/utils.ts @@ -5,15 +5,11 @@ * 2.0. */ -import Boom from '@hapi/boom'; import Semver from 'semver'; import { i18n } from '@kbn/i18n'; import type { TaskInstanceWithDeprecatedFields } from '@kbn/task-manager-plugin/server/task'; import { validateAndAuthorizeSystemActions } from '../../../../lib/validate_authorize_system_actions'; -import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; -import { parseDuration } from '../../../../../common'; -import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization'; import { validateRuleTypeParams, getRuleNotifyWhenType, @@ -31,19 +27,16 @@ import { apiKeyAsAlertAttributes, apiKeyAsRuleDomainProperties, } from '../../../../rules_client/common'; -import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events'; import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation'; import type { BulkOperationError, RulesClientContext } from '../../../../rules_client/types'; import type { RuleDomain, RuleParams } from '../../types'; import { transformRuleDomainToRuleAttributes } from '../../transforms'; -import { createRuleDataSchema } from '../create/schemas'; import type { PreparedRule, PrepareRuleArgs, ApiKeyEntry, BulkCreateOperationError, BulkCreateDisabledReason, - BulkCreateRulesItem, } from './types'; export const getBulkCreateAsDisabledMessage = (message: string): string => @@ -84,119 +77,6 @@ export const buildTaskInstance = ( enabled: true, }); -export const preValidate = async ({ - context, - inputs, -}: { - context: RulesClientContext; - inputs: Array<{ id: string; rule: BulkCreateRulesItem }>; -}): Promise<{ - validated: Array<{ id: string; rule: BulkCreateRulesItem }>; - errors: BulkCreateOperationError[]; -}> => { - const validated = new Map }>(); - const errors: BulkCreateOperationError[] = []; - const authPairs = new Map< - string, - { ruleTypeId: string; consumer: string; ids: string[]; names: Map } - >(); - - // Phase A1: per-rule in-memory checks, sequential, cheapest-first. - for (const { id, rule } of inputs) { - try { - const { actions: genActions, systemActions: genSystemActions } = - await addGeneratedActionValues(rule.data.actions, rule.data.systemActions, context); - const data = { ...rule.data, actions: genActions, systemActions: genSystemActions }; - - try { - createRuleDataSchema.validate(data); - } catch (err) { - throw Boom.badRequest(`Error validating create data - ${err.message}`); - } - - // ruleTypeRegistry.get throws 400 if not registered. - const ruleType = context.ruleTypeRegistry.get(data.alertTypeId); - context.ruleTypeRegistry.ensureRuleTypeEnabled(data.alertTypeId); - validateRuleTypeParams(data.params, ruleType.validate.params); - - const intervalInMs = parseDuration(data.schedule.interval); - if ( - intervalInMs < context.minimumScheduleIntervalInMs && - context.minimumScheduleInterval.enforce - ) { - throw Boom.badRequest( - `Error creating rule: the interval is less than the allowed minimum interval of ${context.minimumScheduleInterval.value}` - ); - } - if ( - intervalInMs < context.minimumScheduleIntervalInMs && - !context.minimumScheduleInterval.enforce - ) { - context.logger.warn( - `Rule schedule interval (${data.schedule.interval}) for "${ruleType.id}" rule type with ID "${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 creation of these rules.` - ); - } - - const authzKey = `${data.alertTypeId}::${data.consumer}`; - const pair = authPairs.get(authzKey); - if (pair) { - pair.ids.push(id); - pair.names.set(id, data.name); - } else { - authPairs.set(authzKey, { - ruleTypeId: data.alertTypeId, - consumer: data.consumer, - ids: [id], - names: new Map([[id, data.name]]), - }); - } - - validated.set(id, { id, rule }); - } catch (err) { - errors.push({ - message: err.message, - status: err.output?.statusCode, - rule: { id, name: rule.data?.name ?? 'n/a' }, - }); - } - } - - if (validated.size === 0) { - return { validated: [], errors }; - } - - // Phase A2: deduped per-pair authorization. - for (const { ruleTypeId, consumer, ids, names } of authPairs.values()) { - try { - await context.authorization.ensureAuthorized({ - ruleTypeId, - consumer, - operation: WriteOperations.Create, - entity: AlertingAuthorizationEntity.Rule, - }); - } catch (authzError) { - // One audit per rule in failing set. Following single rule `create()`. - for (const ruleId of ids) { - context.auditLogger?.log( - ruleAuditEvent({ - action: RuleAuditAction.CREATE, - savedObject: { type: RULE_SAVED_OBJECT_TYPE, id: ruleId, name: names.get(ruleId)! }, - error: authzError, - }) - ); - errors.push({ - message: authzError.message, - status: authzError.output?.statusCode, - rule: { id: ruleId, name: names.get(ruleId) ?? 'n/a' }, - }); - validated.delete(ruleId); - } - } - } - - return { validated: [...validated.values()], errors }; -}; - export const prepareRule = async ({ context, actionsClient, From 32cd9de3a6ac21df944a3f496728ffdcea2aafcc Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Thu, 28 May 2026 14:09:14 +0200 Subject: [PATCH 5/8] Optimize bulk task scheduling. Single task runs now, multiple tasks are jittered. --- .../server/task_scheduling.test.ts | 142 ++++++++++-------- .../task_manager/server/task_scheduling.ts | 16 +- 2 files changed, 84 insertions(+), 74 deletions(-) diff --git a/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.test.ts b/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.test.ts index 9a6b0339fd83b..1e8fbf7f3f706 100644 --- a/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.test.ts +++ b/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.test.ts @@ -529,24 +529,21 @@ describe('TaskScheduling', () => { const bulkUpdatePayload = mockTaskStore.bulkUpdate.mock.calls[0][0]; expect(bulkUpdatePayload.length).toBe(2); - expect(bulkUpdatePayload[0]).toEqual({ - ...task, - enabled: true, - runAt: new Date('1970-01-01T00:00:00.000Z'), - scheduledAt: new Date('1970-01-01T00:00:00.000Z'), - }); - - expect(omit(bulkUpdatePayload[1], 'runAt', 'scheduledAt')).toEqual({ - ...omit(task2, 'runAt', 'scheduledAt'), - enabled: true, - }); - const { runAt, scheduledAt } = bulkUpdatePayload[1]; - expect(runAt.getTime()).toEqual(scheduledAt.getTime()); - expect(runAt.getTime() - bulkUpdatePayload[0].runAt.getTime()).toBeLessThanOrEqual( - 5 * 60 * 1000 - ); + // When more than one task is enabled, every task is jittered (no task runs immediately). + for (const payload of bulkUpdatePayload) { + const sourceTask = payload.id === task.id ? task : task2; + expect(omit(payload, 'runAt', 'scheduledAt')).toEqual({ + ...omit(sourceTask, 'runAt', 'scheduledAt'), + enabled: true, + }); + const { runAt, scheduledAt } = payload; + expect(runAt.getTime()).toEqual(scheduledAt.getTime()); + expect(runAt.getTime()).toBeGreaterThanOrEqual(1); + expect(runAt.getTime()).toBeLessThanOrEqual(5 * 60 * 1000); + } }); + test('should call store bulk update with request when provided', async () => { const task = taskManagerMock.createTask({ id, @@ -1383,7 +1380,31 @@ describe('TaskScheduling', () => { ); }); - test('leaves the first task untouched and jitters subsequent recurring tasks within min(interval, 5m)', async () => { + test('runs a single recurring task immediately', async () => { + const taskScheduling = new TaskScheduling(taskSchedulingOpts); + const task = { + taskType: 'foo', + params: {}, + state: {}, + schedule: { interval: '1m' }, + }; + await taskScheduling.bulkSchedule([task]); + + const bulkSchedulePayload = mockTaskStore.bulkSchedule.mock.calls[0][0]; + + expect(bulkSchedulePayload).toEqual([ + { + ...task, + id: undefined, + traceparent: 'parent', + enabled: true, + runAt: new Date(), + scheduledAt: new Date(), + }, + ]); + }); + + test('jitters every recurring task within min(interval, 5m) when more than one task is scheduled', async () => { const taskScheduling = new TaskScheduling(taskSchedulingOpts); const task0 = { taskType: 'foo', @@ -1409,40 +1430,24 @@ describe('TaskScheduling', () => { expect(bulkSchedulePayload.length).toBe(3); - expect(bulkSchedulePayload[0]).toEqual({ - ...task0, - id: undefined, - traceparent: 'parent', - enabled: true, - runAt: new Date(), - scheduledAt: new Date(), - }); + const inputs = [task0, task1, task2]; + const expectedUpperBounds = [60 * 1000, 60 * 1000, 5 * 60 * 1000]; - expect(omit(bulkSchedulePayload[1], 'runAt', 'scheduledAt')).toEqual({ - ...task1, - id: undefined, - traceparent: 'parent', - enabled: true, - }); - expect(omit(bulkSchedulePayload[2], 'runAt', 'scheduledAt')).toEqual({ - ...task2, - id: undefined, - traceparent: 'parent', - enabled: true, + bulkSchedulePayload.forEach((payload, idx) => { + expect(omit(payload, 'runAt', 'scheduledAt')).toEqual({ + ...inputs[idx], + id: undefined, + traceparent: 'parent', + enabled: true, + }); + const runAt = payload.runAt!.getTime(); + expect(payload.scheduledAt!.getTime()).toBe(runAt); + expect(runAt).toBeGreaterThanOrEqual(1); + expect(runAt).toBeLessThanOrEqual(expectedUpperBounds[idx]); }); - - const t1RunAt = bulkSchedulePayload[1].runAt!.getTime(); - expect(bulkSchedulePayload[1].scheduledAt!.getTime()).toBe(t1RunAt); - expect(t1RunAt).toBeGreaterThanOrEqual(1); - expect(t1RunAt).toBeLessThanOrEqual(60 * 1000); - - const t2RunAt = bulkSchedulePayload[2].runAt!.getTime(); - expect(bulkSchedulePayload[2].scheduledAt!.getTime()).toBe(t2RunAt); - expect(t2RunAt).toBeGreaterThanOrEqual(1); - expect(t2RunAt).toBeLessThanOrEqual(5 * 60 * 1000); }); - test('runs ad-hoc tasks immediately without jitter, even at i > 0', async () => { + test('runs ad-hoc tasks immediately without jitter even when scheduled alongside other tasks', async () => { const taskScheduling = new TaskScheduling(taskSchedulingOpts); const recurringTask = { taskType: 'foo', @@ -1459,25 +1464,30 @@ describe('TaskScheduling', () => { const bulkSchedulePayload = mockTaskStore.bulkSchedule.mock.calls[0][0]; - expect(bulkSchedulePayload).toEqual([ - { - ...recurringTask, - id: undefined, - traceparent: 'parent', - enabled: true, - runAt: new Date(), - scheduledAt: new Date(), - }, - { - ...adHocTask, - id: undefined, - schedule: undefined, - traceparent: 'parent', - enabled: true, - runAt: new Date(), - scheduledAt: new Date(), - }, - ]); + expect(bulkSchedulePayload.length).toBe(2); + + // Recurring task is now jittered since there is more than one task. + expect(omit(bulkSchedulePayload[0], 'runAt', 'scheduledAt')).toEqual({ + ...recurringTask, + id: undefined, + traceparent: 'parent', + enabled: true, + }); + const recurringRunAt = bulkSchedulePayload[0].runAt!.getTime(); + expect(bulkSchedulePayload[0].scheduledAt!.getTime()).toBe(recurringRunAt); + expect(recurringRunAt).toBeGreaterThanOrEqual(1); + expect(recurringRunAt).toBeLessThanOrEqual(60 * 1000); + + // Ad-hoc task still runs immediately because addJitter returns "now" when there is no interval. + expect(bulkSchedulePayload[1]).toEqual({ + ...adHocTask, + id: undefined, + schedule: undefined, + traceparent: 'parent', + enabled: true, + runAt: new Date(), + scheduledAt: new Date(), + }); }); test('does not jitter disabled tasks even if they have an interval schedule', async () => { diff --git a/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.ts b/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.ts index 0677c39c39d47..fde163706803e 100644 --- a/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.ts +++ b/x-pack/platform/plugins/shared/task_manager/server/task_scheduling.ts @@ -157,7 +157,7 @@ export class TaskScheduling { ? agent.currentTraceparent : ''; const modifiedTasks = await Promise.all( - taskInstances.map(async (taskInstance, i) => { + taskInstances.map(async (taskInstance, i, arr) => { const { taskInstance: modifiedTask } = await this.middleware.beforeSave({ ...omit(options, 'apiKey', 'request'), taskInstance: ensureDeprecatedFieldsAreCorrected(taskInstance, this.logger), @@ -165,10 +165,10 @@ export class TaskScheduling { const enabled = modifiedTask.enabled ?? true; let scheduling: Partial<{ runAt: Date; scheduledAt: Date }> = {}; if (enabled) { - // Run the first task now. Run all other tasks a random number of ms in the future, - // with a maximum of 5 minutes or the task interval, whichever is smaller. + // Run now if there is only a single task. + // Otherwise add jitter to avoid them firing together. scheduling = - i === 0 + arr.length === 1 ? { runAt: new Date(), scheduledAt: new Date() } : addJitter(modifiedTask.schedule?.interval) ?? {}; } @@ -216,11 +216,11 @@ export class TaskScheduling { store: this.store, getTasks: async (ids) => await this.bulkGetTasksHelper(ids), filter: (task) => !task.enabled, - map: (task, i) => { + map: (task, i, arr) => { if (runSoon) { - // Run the first task now. Run all other tasks a random number of ms in the future, - // with a maximum of 5 minutes or the task interval, whichever is smaller. - return i === 0 + // Run now if there is only a single task. + // Otherwise add jitter to avoid them firing together. + return arr.length === 1 ? { ...task, enabled: true, runAt: new Date(), scheduledAt: new Date() } : { ...task, enabled: true, ...addJitter(task.schedule?.interval ?? '0s') }; } From d9badae1988869c100fad385d00c4d9cf6e1731d Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Thu, 28 May 2026 14:59:36 +0200 Subject: [PATCH 6/8] Add recent changeTracking support --- .../bulk_create/bulk_create_rules.test.ts | 132 ++++++++++++++++++ .../methods/bulk_create/bulk_create_rules.ts | 25 +++- .../rule/methods/bulk_create/types.ts | 3 + 3 files changed, 158 insertions(+), 2 deletions(-) diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts index e7f2ef6c4228b..dccb9c719a505 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.test.ts @@ -934,4 +934,136 @@ describe('bulkCreateRules', () => { expect(result.errors.some((e) => e.disabledReason === 'schedule_limit_exceeded')).toBe(true); }); }); + + describe('change tracking', () => { + const createChangeTrackingService = () => ({ + log: jest.fn().mockResolvedValue(undefined), + logBulk: jest.fn().mockResolvedValue(undefined), + getHistory: jest.fn().mockResolvedValue({ items: [], total: 0 }), + }); + + const setRuleType = (overrides: { trackChanges?: boolean } = {}) => { + ruleTypeRegistry.get.mockReturnValue({ + id: '123', + name: 'Test', + actionGroups: [{ id: 'default', name: 'Default' }], + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + recoveryActionGroup: { id: 'recovered', name: 'Recovered' }, + executor: jest.fn(), + category: 'test', + validate: { params: { validate: (params: unknown) => params } }, + solution: 'security', + validLegacyConsumers: [], + trackChanges: true, + ...overrides, + } as never); + }; + + test('logs every successfully created rule in a single bulk call with the requested action', async () => { + const changeTrackingService = createChangeTrackingService(); + const trackingClient = new RulesClient({ ...rulesClientParams, changeTrackingService }); + setRuleType(); + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }, { id: 'mock-id-2' }]) + ); + + await trackingClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }, { data: baseRule({ name: 'b' }) }], + changeTracking: { action: 'rule_install', metadata: { bulkCount: 2 } }, + }); + + expect(changeTrackingService.logBulk).toHaveBeenCalledTimes(1); + expect(changeTrackingService.logBulk).toHaveBeenCalledWith( + [ + expect.objectContaining({ objectId: 'mock-id-1' }), + expect.objectContaining({ objectId: 'mock-id-2' }), + ], + expect.objectContaining({ + action: 'rule_install', + data: { metadata: { bulkCount: 2 } }, + }) + ); + }); + + test('defaults to ruleCreate when no action is provided', async () => { + const changeTrackingService = createChangeTrackingService(); + const trackingClient = new RulesClient({ ...rulesClientParams, changeTrackingService }); + setRuleType(); + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + + await trackingClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }], + }); + + expect(changeTrackingService.logBulk).toHaveBeenCalledWith( + expect.any(Array), + expect.objectContaining({ action: 'rule_create' }) + ); + }); + + test('only logs successfully persisted SOs when bulk create has partial failures', async () => { + const changeTrackingService = createChangeTrackingService(); + const trackingClient = new RulesClient({ ...rulesClientParams, changeTrackingService }); + setRuleType(); + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([ + { id: 'mock-id-1' }, + { id: 'mock-id-2', error: { message: 'conflict', statusCode: 409 } }, + { id: 'mock-id-3' }, + ]) + ); + + await trackingClient.bulkCreateRules({ + rules: [ + { data: baseRule({ name: 'a' }) }, + { data: baseRule({ name: 'b' }) }, + { data: baseRule({ name: 'c' }) }, + ], + }); + + expect(changeTrackingService.logBulk).toHaveBeenCalledTimes(1); + expect(changeTrackingService.logBulk).toHaveBeenCalledWith( + [ + expect.objectContaining({ objectId: 'mock-id-1' }), + expect.objectContaining({ objectId: 'mock-id-3' }), + ], + expect.any(Object) + ); + }); + + test('does not call logBulk when the rule type opts out of tracking', async () => { + const changeTrackingService = createChangeTrackingService(); + const trackingClient = new RulesClient({ ...rulesClientParams, changeTrackingService }); + setRuleType({ trackChanges: false }); + + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + + await trackingClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }], + }); + + expect(changeTrackingService.logBulk).not.toHaveBeenCalled(); + }); + + test('does not throw when no change tracking service is configured', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue( + buildBulkResponse([{ id: 'mock-id-1' }]) + ); + + await rulesClient.bulkCreateRules({ + rules: [{ data: baseRule({ name: 'a' }) }], + }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalled(); + }); + }); }); diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts index 8ec7a26e3deda..bf312a4a70296 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -8,8 +8,9 @@ import Boom from '@hapi/boom'; import pMap from 'p-map'; import { withSpan } from '@kbn/apm-utils'; -import type { SavedObjectsBulkCreateObject } from '@kbn/core/server'; +import type { SavedObject, SavedObjectsBulkCreateObject } from '@kbn/core/server'; import { SavedObjectsUtils } from '@kbn/core/server'; +import { RuleChangeTrackingAction, type RuleChangeTracking } from '@kbn/alerting-types'; import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects'; import { getRuleCircuitBreakerErrorMessage, parseDuration } from '../../../../../common'; import { addGeneratedActionValues, updateMeta } from '../../../../rules_client/lib'; @@ -44,12 +45,13 @@ import { flushKeysToInvalidate, prepareRule, } from './utils'; +import { logRuleChanges } from '../common_utils/log_rule_changes'; export async function bulkCreateRules( context: RulesClientContext, params: BulkCreateRulesParams ): Promise { - const { rules, exitEarlyOnError = false } = params; + const { rules, exitEarlyOnError = false, changeTracking } = params; const { logger } = context; const total = rules.length; @@ -112,6 +114,7 @@ export async function bulkCreateRules( username, actionsClient, batch, + changeTracking, }); successfulIds.push(...result.successfulIds); @@ -255,6 +258,7 @@ interface RunBatchArgs { username: string | null; actionsClient: Awaited>; batch: Array<{ id: string; rule: BulkCreateRulesItem }>; + changeTracking?: RuleChangeTracking; } async function runBatch({ @@ -262,6 +266,7 @@ async function runBatch({ username, actionsClient, batch, + changeTracking, }: RunBatchArgs): Promise { const { logger } = context; @@ -413,6 +418,7 @@ async function runBatch({ }) ); + const createTime = Date.now(); let bulkResponse; try { bulkResponse = await withSpan( @@ -450,6 +456,7 @@ async function runBatch({ // Phase B4 per-row outcomes. const batchSuccessfulIds: string[] = []; const taskIdsToCleanUp: string[] = []; + const successfulSavedObjects: Array> = []; let perRowFailureOccurred = false; for (const so of bulkResponse.saved_objects) { @@ -473,6 +480,7 @@ async function runBatch({ } } else { batchSuccessfulIds.push(so.id); + successfulSavedObjects.push(so as SavedObject); if (newlyScheduledTaskIds.has(so.id)) { // Audit per-rule ENABLE for the enabled subset (mirrors single-rule semantics). context.auditLogger?.log( @@ -507,6 +515,19 @@ async function runBatch({ // Single per-batch flush for all collected key invalidations. await flushKeysToInvalidate(keysToInvalidate, context); + // Per-rule change-history entries for SOs that actually persisted. + if (successfulSavedObjects.length > 0) { + await logRuleChanges({ + ruleSOs: successfulSavedObjects, + rulesClientContext: context, + changesContext: { + action: changeTracking?.action ?? RuleChangeTrackingAction.ruleCreate, + timestamp: createTime, + metadata: changeTracking?.metadata, + }, + }); + } + return { successfulIds: batchSuccessfulIds, errors, diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts index a651953d34eed..45e6ad5b81df5 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/types.ts @@ -6,6 +6,7 @@ */ import type { SavedObjectReference } from '@kbn/core/server'; +import type { RuleChangeTracking } from '@kbn/alerting-types'; import type { IntervalSchedule } from '../../../../../common'; import type { RuleParams } from '../../types'; import type { CreateRuleData } from '../create/types'; @@ -52,6 +53,8 @@ export interface BulkCreateRulesParams { batchSize?: number; /** If true, stop further batches on SO-level failure (whole-call throw or any per-row SO error). Phase 1/2/3 demotions never halt the loop. Defaults to false. */ exitEarlyOnError?: boolean; + /** Rule change tracking context. `action` defaults to `RuleChangeTrackingAction.ruleCreate`; consumers can override. */ + changeTracking?: RuleChangeTracking; } export type BulkCreateDisabledReason = From 9c80944ba46ef9a732316352c4e61faf2b85e57b Mon Sep 17 00:00:00 2001 From: Steven de Salas Date: Thu, 28 May 2026 16:21:19 +0200 Subject: [PATCH 7/8] Add wiring for security detection rules --- .../methods/bulk_create/bulk_create_rules.ts | 2 +- .../common/experimental_features.ts | 10 + .../perform_rule_installation_handler.ts | 43 ++- .../api/{timeouts.ts => constants.ts} | 3 + .../api/rules/bulk_actions/route.ts | 2 +- .../api/rules/export_rules/route.ts | 2 +- .../api/rules/import_rules/route.ts | 9 +- .../__mocks__/detection_rules_client.ts | 2 + ..._client.bulk_create_prebuilt_rules.test.ts | 163 +++++++++ ...ion_rules_client.bulk_import_rules.test.ts | 270 ++++++++++++++ ...ction_rules_client.change_tracking.test.ts | 92 +++++ .../detection_rules_client.ts | 23 ++ .../detection_rules_client_interface.ts | 12 + .../methods/bulk_create_prebuilt_rules.ts | 127 +++++++ .../methods/bulk_import_rules.ts | 345 ++++++++++++++++++ .../logic/import/import_rules.test.ts | 60 +++ .../logic/import/import_rules.ts | 70 ++-- 17 files changed, 1189 insertions(+), 46 deletions(-) rename x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/{timeouts.ts => constants.ts} (77%) create mode 100644 x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_create_prebuilt_rules.test.ts create mode 100644 x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_import_rules.test.ts create mode 100644 x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_create_prebuilt_rules.ts create mode 100644 x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_import_rules.ts diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts index bf312a4a70296..02e06356d63a2 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -418,7 +418,6 @@ async function runBatch({ }) ); - const createTime = Date.now(); let bulkResponse; try { bulkResponse = await withSpan( @@ -456,6 +455,7 @@ async function runBatch({ // Phase B4 per-row outcomes. const batchSuccessfulIds: string[] = []; const taskIdsToCleanUp: string[] = []; + const createTime = Date.now(); const successfulSavedObjects: Array> = []; let perRowFailureOccurred = false; diff --git a/x-pack/solutions/security/plugins/security_solution/common/experimental_features.ts b/x-pack/solutions/security/plugins/security_solution/common/experimental_features.ts index c16ee3c983421..36a19112477e7 100644 --- a/x-pack/solutions/security/plugins/security_solution/common/experimental_features.ts +++ b/x-pack/solutions/security/plugins/security_solution/common/experimental_features.ts @@ -22,6 +22,16 @@ export const allowedExperimentalValues = Object.freeze({ */ previewTelemetryUrlEnabled: false, + /** + * When enabled, prebuilt rule installation (POST .../prebuilt_rules/installation/_perform) + * and rule import (POST .../rules/_import) use the new alerting `rulesClient.bulkCreateRules` + * path, which handles both disabled and enabled rules (API key minting + task scheduling) + * in a single bulk call instead of the per-rule create loop. + * + * Release: TBD + */ + bulkCreateRulesEnabled: false, + /** * Enables extended rule execution logging to Event Log. When this setting is enabled: * - Rules write their console error, info, debug, and trace messages to Event Log, diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_installation/perform_rule_installation_handler.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_installation/perform_rule_installation_handler.ts index 36ece0b2f841b..281d0518e671d 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_installation/perform_rule_installation_handler.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/api/perform_rule_installation/perform_rule_installation_handler.ts @@ -109,31 +109,40 @@ export const performRuleInstallationHandler = async ( ); ruleInstallQueue.push(...(await excludeLicenseRestrictedRules(allInstallableRules, mlAuthz))); } - const changeTracking = { metadata: { bulkCount: ruleInstallQueue.length, }, }; - const BATCH_SIZE = 100; - while (ruleInstallQueue.length > 0) { - const rulesToInstall = ruleInstallQueue.splice(0, BATCH_SIZE); - const ruleAssets = await ruleAssetsClient.fetchAssetsByVersion(rulesToInstall); - - const { results, errors } = await createPrebuiltRules( - detectionRulesClient, - ruleAssets, - changeTracking, - logger - ); - - const batchInstalledRules = results.map(({ result: rule }) => - pick(rule, ['id', 'rule_id', 'version']) + const { bulkCreateRulesEnabled } = ctx.securitySolution.getConfig().experimentalFeatures; + if (bulkCreateRulesEnabled) { + const ruleAssets = await ruleAssetsClient.fetchAssetsByVersion(ruleInstallQueue); + const { results, errors } = await detectionRulesClient.bulkCreatePrebuiltRules({ + rules: ruleAssets, + }); + installedRules.push( + ...results.map(({ result: rule }) => pick(rule, ['id', 'rule_id', 'version'])) ); - - installedRules.push(...batchInstalledRules); ruleErrors.push(...errors); + } else { + const BATCH_SIZE = 100; + while (ruleInstallQueue.length > 0) { + const rulesToInstall = ruleInstallQueue.splice(0, BATCH_SIZE); + const ruleAssets = await ruleAssetsClient.fetchAssetsByVersion(rulesToInstall); + + const { results, errors } = await createPrebuiltRules( + detectionRulesClient, + ruleAssets, + changeTracking, + logger + ); + + installedRules.push( + ...results.map(({ result: rule }) => pick(rule, ['id', 'rule_id', 'version'])) + ); + ruleErrors.push(...errors); + } } const { error: timelineInstallationError } = await performTimelinesInstallation( diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/timeouts.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/constants.ts similarity index 77% rename from x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/timeouts.ts rename to x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/constants.ts index 24bf61ddc2773..08236d1f4ccda 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/timeouts.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/constants.ts @@ -13,3 +13,6 @@ export const RULE_MANAGEMENT_BULK_ACTION_SOCKET_TIMEOUT_MS = 3600000 as const; * 1 hour = 3600000 ms = 60 minutes * 60 seconds * 1000 ms */ export const RULE_MANAGEMENT_IMPORT_EXPORT_SOCKET_TIMEOUT_MS = 3600000 as const; + +/** Cap concurrent rule imports at 1 to bound heap; mirrors PREBUILT_RULES_OPERATION_CONCURRENCY. */ +export const RULE_MANAGEMENT_IMPORT_CONCURRENCY = 1; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts index 0a7adce22d097..0e740d5edb1ae 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts @@ -44,7 +44,7 @@ import { validateBulkDuplicateRule, } from '../../../logic/bulk_actions/validations'; import { getExportByObjectIds } from '../../../logic/export/get_export_by_object_ids'; -import { RULE_MANAGEMENT_BULK_ACTION_SOCKET_TIMEOUT_MS } from '../../timeouts'; +import { RULE_MANAGEMENT_BULK_ACTION_SOCKET_TIMEOUT_MS } from '../../constants'; import type { BulkActionError } from './bulk_actions_response'; import { buildBulkResponse } from './bulk_actions_response'; import { bulkEnableDisableRules } from './bulk_enable_disable_rules'; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts index 148c9b6414819..e2fb93f468bcd 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts @@ -20,7 +20,7 @@ import { getRulesCount } from '../../../logic/search/get_existing_prepackaged_ru import { getExportByObjectIds } from '../../../logic/export/get_export_by_object_ids'; import { getExportAll } from '../../../logic/export/get_export_all'; import { buildSiemResponse } from '../../../../routes/utils'; -import { RULE_MANAGEMENT_IMPORT_EXPORT_SOCKET_TIMEOUT_MS } from '../../timeouts'; +import { RULE_MANAGEMENT_IMPORT_EXPORT_SOCKET_TIMEOUT_MS } from '../../constants'; export const exportRulesRoute = ( router: SecuritySolutionPluginRouter, diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index 1ab568c3242bf..004173bde7522 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -39,7 +39,11 @@ import { getTupleDuplicateErrorsAndUniqueRules, migrateLegacyActionsIds, } from '../../../utils/utils'; -import { RULE_MANAGEMENT_IMPORT_EXPORT_SOCKET_TIMEOUT_MS } from '../../timeouts'; +import { + RULE_MANAGEMENT_IMPORT_CONCURRENCY, + RULE_MANAGEMENT_IMPORT_EXPORT_SOCKET_TIMEOUT_MS, +} from '../../constants'; +import { routeLimitedConcurrencyTag } from '../../../../../../utils/route_limited_concurrency_tag'; import { createPrebuiltRuleObjectsClient } from '../../../../prebuilt_rules/logic/rule_objects/prebuilt_rule_objects_client'; const CHUNK_PARSED_OBJECT_SIZE = 50; @@ -63,6 +67,7 @@ export const importRulesRoute = ( maxBytes: config.maxRuleImportPayloadBytes, output: 'stream', }, + tags: [routeLimitedConcurrencyTag(RULE_MANAGEMENT_IMPORT_CONCURRENCY)], timeout: { idleSocket: RULE_MANAGEMENT_IMPORT_EXPORT_SOCKET_TIMEOUT_MS, }, @@ -187,6 +192,7 @@ export const importRulesRoute = ( ctx.securitySolution.getCheckOsqueryResponseActionAuthz(), }); + const experimentalFeatures = ctx.securitySolution.getConfig().experimentalFeatures; const ruleChunks = chunk(CHUNK_PARSED_OBJECT_SIZE, validatedResponseActionsRules); const importRuleResponse = await importRules({ @@ -200,6 +206,7 @@ export const importRulesRoute = ( allowMissingConnectorSecrets: !!actionConnectors.length, ruleSourceImporter, detectionRulesClient, + experimentalFeatures, }); const parseErrors = parsedRuleErrors.map((error) => diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/__mocks__/detection_rules_client.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/__mocks__/detection_rules_client.ts index bb6796e60735b..8f9642f4f659c 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/__mocks__/detection_rules_client.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/__mocks__/detection_rules_client.ts @@ -13,6 +13,7 @@ const createDetectionRulesClientMock = () => { const mocked: DetectionRulesClientMock = { createCustomRule: jest.fn(), createPrebuiltRule: jest.fn(), + bulkCreatePrebuiltRules: jest.fn().mockResolvedValue({ results: [], errors: [] }), updateRule: jest.fn(), patchRule: jest.fn(), deleteRule: jest.fn(), @@ -21,6 +22,7 @@ const createDetectionRulesClientMock = () => { revertPrebuiltRule: jest.fn(), importRule: jest.fn(), importRules: jest.fn(), + bulkImportRules: jest.fn().mockResolvedValue({ responses: [] }), getRuleCustomizationStatus: jest.fn(), getHistoryForRule: jest.fn(), }; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_create_prebuilt_rules.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_create_prebuilt_rules.test.ts new file mode 100644 index 0000000000000..5a36e0b006a52 --- /dev/null +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_create_prebuilt_rules.test.ts @@ -0,0 +1,163 @@ +/* + * 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 { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; +import { savedObjectsClientMock } from '@kbn/core/server/mocks'; +import { licenseMock } from '@kbn/licensing-plugin/common/licensing.mock'; + +import { SecurityRuleChangeTrackingAction } from '../../../../../../common/detection_engine/rule_management/rule_change_tracking'; +import { getCreateRulesSchemaMock } from '../../../../../../common/api/detection_engine/model/rule_schema/mocks'; +import { buildMlAuthz } from '../../../../machine_learning/authz'; +import { throwAuthzError } from '../../../../machine_learning/validation'; +import { createDetectionRulesClient } from './detection_rules_client'; +import type { IDetectionRulesClient } from './detection_rules_client_interface'; +import { createProductFeaturesServiceMock } from '../../../../product_features_service/mocks'; +import { getMockRulesAuthz } from '../../__mocks__/authz'; + +jest.mock('../../../../machine_learning/authz'); +jest.mock('../../../../machine_learning/validation'); + +describe('DetectionRulesClient.bulkCreatePrebuiltRules', () => { + let rulesClient: ReturnType; + let detectionRulesClient: IDetectionRulesClient; + + const mlAuthz = (buildMlAuthz as jest.Mock)(); + const rulesAuthz = getMockRulesAuthz(); + const actionsClient: jest.Mocked = { + isSystemAction: jest.fn(), + } as unknown as jest.Mocked; + + beforeEach(() => { + rulesClient = rulesClientMock.create(); + (throwAuthzError as jest.Mock).mockReset(); + detectionRulesClient = createDetectionRulesClient({ + actionsClient, + rulesClient, + mlAuthz, + rulesAuthz, + savedObjectsClient: savedObjectsClientMock.create(), + license: licenseMock.createLicenseMock(), + productFeaturesService: createProductFeaturesServiceMock(), + }); + }); + + it('issues a single bulkCreateRules call with disabled, immutable rules and emits { id, rule_id, version } pairs', async () => { + const asset1 = { ...getCreateRulesSchemaMock(), version: 1, rule_id: 'rule-1' }; + const asset2 = { ...getCreateRulesSchemaMock(), version: 2, rule_id: 'rule-2' }; + + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => { + const ids = args.rules.map((r) => (r.options as { id: string }).id); + return { successfulIds: ids, errors: [], total: ids.length }; + }); + + const { results, errors } = await detectionRulesClient.bulkCreatePrebuiltRules({ + rules: [asset1, asset2], + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledTimes(1); + const callArgs = rulesClient.bulkCreateRules.mock.calls[0][0]; + expect(callArgs.rules).toHaveLength(2); + expect(callArgs.rules.every((r) => r.data.enabled === false)).toBe(true); + expect(callArgs.rules.every((r) => r.data.params.immutable === true)).toBe(true); + expect(errors).toEqual([]); + expect(results).toHaveLength(2); + const callIds = callArgs.rules.map((r) => (r.options as { id: string }).id); + expect(results[0].result).toEqual({ id: callIds[0], rule_id: 'rule-1', version: 1 }); + expect(results[1].result).toEqual({ id: callIds[1], rule_id: 'rule-2', version: 2 }); + }); + + it('issues a single bulkCreateRules call regardless of input size (alerting batches internally)', async () => { + const assets = Array.from({ length: 250 }, (_, i) => ({ + ...getCreateRulesSchemaMock(), + version: 1, + rule_id: `rule-${i}`, + })); + + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => ({ + successfulIds: args.rules.map((r) => (r.options as { id: string }).id), + errors: [], + total: args.rules.length, + })); + + const { results, errors } = await detectionRulesClient.bulkCreatePrebuiltRules({ + rules: assets, + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledTimes(1); + expect(results).toHaveLength(250); + expect(errors).toEqual([]); + }); + + it('reports per-rule errors from the alerting layer', async () => { + const asset = { ...getCreateRulesSchemaMock(), version: 1, rule_id: 'rule-1' }; + + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => { + const id = (args.rules[0].options as { id: string }).id; + return { + successfulIds: [], + errors: [{ message: 'boom', status: 500, rule: { id, name: asset.name } }], + total: 1, + }; + }); + + const { results, errors } = await detectionRulesClient.bulkCreatePrebuiltRules({ + rules: [asset], + }); + + expect(results).toEqual([]); + expect(errors).toHaveLength(1); + expect(errors[0].item).toBe(asset); + expect(errors[0].error.message).toBe('boom'); + }); + + it('returns ML-auth failures as per-rule errors without calling bulkCreateRules', async () => { + const asset = { ...getCreateRulesSchemaMock(), version: 1, rule_id: 'rule-1' }; + (throwAuthzError as jest.Mock).mockImplementation(() => { + throw new Error('ML auth denied'); + }); + + const { results, errors } = await detectionRulesClient.bulkCreatePrebuiltRules({ + rules: [asset], + }); + + expect(rulesClient.bulkCreateRules).not.toHaveBeenCalled(); + expect(results).toEqual([]); + expect(errors[0].error.message).toBe('ML auth denied'); + }); + + it('returns empty result for empty input', async () => { + const result = await detectionRulesClient.bulkCreatePrebuiltRules({ rules: [] }); + expect(result).toEqual({ results: [], errors: [] }); + expect(rulesClient.bulkCreateRules).not.toHaveBeenCalled(); + }); + + it('forwards ruleInstall action and rules.length as bulkCount to rulesClient.bulkCreateRules', async () => { + const assets = Array.from({ length: 3 }, (_, i) => ({ + ...getCreateRulesSchemaMock(), + version: 1, + rule_id: `rule-${i}`, + })); + + rulesClient.bulkCreateRules.mockResolvedValueOnce({ + successfulIds: [], + errors: [], + total: 0, + }); + + await detectionRulesClient.bulkCreatePrebuiltRules({ rules: assets }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledWith( + expect.objectContaining({ + changeTracking: { + action: SecurityRuleChangeTrackingAction.ruleInstall, + metadata: { bulkCount: assets.length }, + }, + }) + ); + }); +}); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_import_rules.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_import_rules.test.ts new file mode 100644 index 0000000000000..8d49df5b79a5b --- /dev/null +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.bulk_import_rules.test.ts @@ -0,0 +1,270 @@ +/* + * 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 { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import { actionsClientMock } from '@kbn/actions-plugin/server/actions_client/actions_client.mock'; +import { savedObjectsClientMock } from '@kbn/core/server/mocks'; +import { licenseMock } from '@kbn/licensing-plugin/common/licensing.mock'; + +import { buildMlAuthz } from '../../../../machine_learning/__mocks__/authz'; +import { SecurityRuleChangeTrackingAction } from '../../../../../../common/detection_engine/rule_management/rule_change_tracking'; +import { getImportRulesSchemaMock } from '../../../../../../common/api/detection_engine/rule_management/mocks'; +import { getRulesSchemaMock } from '../../../../../../common/api/detection_engine/model/rule_schema/mocks'; +import { getRuleMock } from '../../../routes/__mocks__/request_responses'; +import { getQueryRuleParams } from '../../../rule_schema/mocks'; +import { ruleSourceImporterMock } from '../import/rule_source_importer/rule_source_importer.mock'; +import { createDetectionRulesClient } from './detection_rules_client'; +import { importRule } from './methods/import_rule'; +import { checkRuleExceptionReferences } from '../import/check_rule_exception_references'; +import { findRules } from '../search/find_rules'; +import { createProductFeaturesServiceMock } from '../../../../product_features_service/mocks'; +import { getMockRulesAuthz } from '../../__mocks__/authz'; +import { __testing_escapeKql } from './methods/bulk_import_rules'; +import { isRuleImportError } from '../import/errors'; + +jest.mock('./methods/import_rule'); +jest.mock('../import/check_rule_exception_references'); +jest.mock('../search/find_rules'); + +describe('detectionRulesClient.bulkImportRules', () => { + let rulesClient: ReturnType; + let subject: ReturnType; + let mockRuleSourceImporter: ReturnType; + const rulesAuthz = getMockRulesAuthz(); + + beforeEach(() => { + (findRules as jest.Mock).mockReset(); + (importRule as jest.Mock).mockReset(); + rulesClient = rulesClientMock.create(); + subject = createDetectionRulesClient({ + actionsClient: actionsClientMock.create(), + rulesClient, + mlAuthz: buildMlAuthz(), + rulesAuthz, + savedObjectsClient: savedObjectsClientMock.create(), + license: licenseMock.createLicenseMock(), + productFeaturesService: createProductFeaturesServiceMock(), + }); + + (checkRuleExceptionReferences as jest.Mock).mockReturnValue([[], []]); + (findRules as jest.Mock).mockResolvedValue({ data: [] }); + (importRule as jest.Mock).mockResolvedValue(getRulesSchemaMock()); + rulesClient.bulkCreateRules.mockResolvedValue({ + successfulIds: [], + errors: [], + total: 0, + }); + + mockRuleSourceImporter = ruleSourceImporterMock.create(); + mockRuleSourceImporter.calculateRuleSource.mockReturnValue({ + ruleSource: { type: 'internal' }, + immutable: false, + }); + }); + + it('all-new disabled rules: single bulkCreateRules call, no findRules conflicts', async () => { + const ruleToImport = { ...getImportRulesSchemaMock(), enabled: false }; + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => ({ + successfulIds: args.rules.map((r) => (r.options as { id: string }).id), + errors: [], + total: args.rules.length, + })); + + const { responses } = await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules: [ruleToImport], + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledTimes(1); + const args = rulesClient.bulkCreateRules.mock.calls[0][0]; + expect(args.rules[0].data.enabled).toBe(false); + expect(importRule).not.toHaveBeenCalled(); + expect(responses).toEqual([{ rule_id: ruleToImport.rule_id }]); + }); + + it('all-new enabled rules: preserves enabled flag in single bulk call', async () => { + const ruleToImport = { ...getImportRulesSchemaMock(), enabled: true }; + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => ({ + successfulIds: args.rules.map((r) => (r.options as { id: string }).id), + errors: [], + total: args.rules.length, + })); + + await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules: [ruleToImport], + }); + + const args = rulesClient.bulkCreateRules.mock.calls[0][0]; + expect(args.rules[0].data.enabled).toBe(true); + }); + + it('issues a single bulkCreateRules call regardless of input size (alerting batches internally)', async () => { + const rules = Array.from({ length: 250 }, (_, i) => ({ + ...getImportRulesSchemaMock(), + rule_id: `rule-${i}`, + })); + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => ({ + successfulIds: args.rules.map((r) => (r.options as { id: string }).id), + errors: [], + total: args.rules.length, + })); + + const { responses } = await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules, + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledTimes(1); + expect(responses).toHaveLength(250); + }); + + it('mixed new+existing with overwriteRules:false reports conflict for existing', async () => { + const r1 = { ...getImportRulesSchemaMock(), rule_id: 'new-rule' }; + const r2 = { ...getImportRulesSchemaMock(), rule_id: 'existing-rule' }; + + (findRules as jest.Mock).mockResolvedValueOnce({ + data: [getRuleMock({ ...getQueryRuleParams(), ruleId: 'existing-rule' })], + }); + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => ({ + successfulIds: args.rules.map((r) => (r.options as { id: string }).id), + errors: [], + total: args.rules.length, + })); + + const { responses } = await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules: [r1, r2], + }); + + const conflicts = responses.filter((r) => isRuleImportError(r) && r.error.type === 'conflict'); + expect(conflicts).toHaveLength(1); + expect(isRuleImportError(conflicts[0]) && conflicts[0].error.ruleId).toBe('existing-rule'); + expect(rulesClient.bulkCreateRules.mock.calls[0][0].rules).toHaveLength(1); + expect(importRule).not.toHaveBeenCalled(); + }); + + it('mixed new+existing with overwriteRules:true: existing fall through to per-rule importRule and surface as { rule_id }', async () => { + const r1 = { ...getImportRulesSchemaMock(), rule_id: 'new-rule' }; + const r2 = { ...getImportRulesSchemaMock(), rule_id: 'existing-rule' }; + + (findRules as jest.Mock).mockResolvedValueOnce({ + data: [getRuleMock({ ...getQueryRuleParams(), ruleId: 'existing-rule' })], + }); + (importRule as jest.Mock).mockResolvedValueOnce({ + ...getRulesSchemaMock(), + rule_id: 'existing-rule', + }); + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => ({ + successfulIds: args.rules.map((r) => (r.options as { id: string }).id), + errors: [], + total: args.rules.length, + })); + + const { responses } = await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: true, + ruleSourceImporter: mockRuleSourceImporter, + rules: [r1, r2], + }); + + expect(importRule).toHaveBeenCalledTimes(1); + expect((importRule as jest.Mock).mock.calls[0][0].importRulePayload.ruleToImport.rule_id).toBe( + 'existing-rule' + ); + expect(rulesClient.bulkCreateRules.mock.calls[0][0].rules).toHaveLength(1); + expect(responses).toEqual( + expect.arrayContaining([{ rule_id: 'existing-rule' }, { rule_id: 'new-rule' }]) + ); + }); + + it('per-row bulk error is re-paired to its source rule_id via uuid', async () => { + const ruleToImport = getImportRulesSchemaMock(); + rulesClient.bulkCreateRules.mockImplementationOnce(async (args) => { + const id = (args.rules[0].options as { id: string }).id; + return { + successfulIds: [], + errors: [{ message: 'boom', status: 500, rule: { id, name: ruleToImport.name } }], + total: 1, + }; + }); + + const { responses } = await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules: [ruleToImport], + }); + + const errors = responses.filter(isRuleImportError); + expect(errors).toHaveLength(1); + expect(errors[0].error.ruleId).toBe(ruleToImport.rule_id); + expect(errors[0].error.message).toBe('boom'); + }); + + it('forwards ruleImport action and rules.length as bulkCount to rulesClient.bulkCreateRules', async () => { + const rules = [ + { ...getImportRulesSchemaMock(), rule_id: 'rule-1' }, + { ...getImportRulesSchemaMock(), rule_id: 'rule-2' }, + { ...getImportRulesSchemaMock(), rule_id: 'rule-3' }, + ]; + rulesClient.bulkCreateRules.mockResolvedValueOnce({ + successfulIds: [], + errors: [], + total: 0, + }); + + await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules, + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledWith( + expect.objectContaining({ + changeTracking: { + action: SecurityRuleChangeTrackingAction.ruleImport, + metadata: { bulkCount: rules.length }, + }, + }) + ); + }); + + it('returns empty result for empty input without calling alerting/findRules', async () => { + const result = await subject.bulkImportRules({ + allowMissingConnectorSecrets: false, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + rules: [], + }); + + expect(result.responses).toEqual([]); + expect(rulesClient.bulkCreateRules).not.toHaveBeenCalled(); + expect(findRules).not.toHaveBeenCalled(); + }); +}); + +describe('bulk_import_rules KQL escape', () => { + it('escapes backslashes', () => { + expect(__testing_escapeKql('foo\\bar')).toBe('foo\\\\bar'); + }); + it('escapes double quotes', () => { + expect(__testing_escapeKql('a"b')).toBe('a\\"b'); + }); + it('escapes both', () => { + expect(__testing_escapeKql('a\\"b')).toBe('a\\\\\\"b'); + }); +}); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.change_tracking.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.change_tracking.test.ts index e310f703ddb55..f15cf124cb916 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.change_tracking.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.change_tracking.test.ts @@ -32,11 +32,13 @@ import { getRuleByRuleId } from './methods/get_rule_by_rule_id'; import { checkRuleExceptionReferences } from '../import/check_rule_exception_references'; import { ruleSourceImporterMock } from '../import/rule_source_importer/rule_source_importer.mock'; import { getMockRulesAuthz } from '../../__mocks__/authz'; +import { findRules } from '../search/find_rules'; jest.mock('../../../../machine_learning/authz'); jest.mock('../../../../machine_learning/validation'); jest.mock('./methods/get_rule_by_rule_id'); jest.mock('../import/check_rule_exception_references'); +jest.mock('../search/find_rules'); describe('DetectionRulesClient change tracking', () => { let rulesClient: ReturnType; @@ -58,9 +60,15 @@ describe('DetectionRulesClient change tracking', () => { total: 1, taskIdsFailedToBeDeleted: [], }); + rulesClient.bulkCreateRules.mockResolvedValue({ + successfulIds: [], + errors: [], + total: 0, + }); (getRuleByRuleId as jest.Mock).mockResolvedValue(null); (checkRuleExceptionReferences as jest.Mock).mockReturnValue([[], []]); + (findRules as jest.Mock).mockResolvedValue({ data: [] }); detectionRulesClient = createDetectionRulesClient({ actionsClient, @@ -210,6 +218,46 @@ describe('DetectionRulesClient change tracking', () => { }); }); + it('bulkCreatePrebuiltRules uses ruleInstall action', async () => { + const ruleAsset: PrebuiltRuleAsset = { + ...getCreateRulesSchemaMock(), + version: 1, + rule_id: 'rule-1', + }; + + await detectionRulesClient.bulkCreatePrebuiltRules({ rules: [ruleAsset] }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledWith( + expect.objectContaining({ + changeTracking: expect.objectContaining({ + action: SecurityRuleChangeTrackingAction.ruleInstall, + }), + }) + ); + }); + + it('bulkImportRules uses ruleImport action on the bulk-create branch', async () => { + const mockRuleSourceImporter = ruleSourceImporterMock.create(); + mockRuleSourceImporter.calculateRuleSource.mockReturnValue({ + ruleSource: { type: 'internal' }, + immutable: false, + }); + + await detectionRulesClient.bulkImportRules({ + rules: [getImportRulesSchemaMock()], + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledWith( + expect.objectContaining({ + changeTracking: expect.objectContaining({ + action: SecurityRuleChangeTrackingAction.ruleImport, + }), + }) + ); + }); + it('revertPrebuiltRule uses ruleRevert action', async () => { const existingRule = getRulesEqlSchemaMock(); const ruleAsset: PrebuiltRuleAsset = { @@ -260,6 +308,50 @@ describe('DetectionRulesClient change tracking', () => { ); }); + it('bulkCreatePrebuiltRules computes bulkCount from rules.length', async () => { + const ruleAssets: PrebuiltRuleAsset[] = [ + { ...getCreateRulesSchemaMock(), version: 1, rule_id: 'rule-1' }, + { ...getCreateRulesSchemaMock(), version: 1, rule_id: 'rule-2' }, + { ...getCreateRulesSchemaMock(), version: 1, rule_id: 'rule-3' }, + ]; + + await detectionRulesClient.bulkCreatePrebuiltRules({ rules: ruleAssets }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledWith( + expect.objectContaining({ + changeTracking: expect.objectContaining({ + metadata: { bulkCount: ruleAssets.length }, + }), + }) + ); + }); + + it('bulkImportRules computes bulkCount from rules.length on the bulk-create branch', async () => { + const mockRuleSourceImporter = ruleSourceImporterMock.create(); + mockRuleSourceImporter.calculateRuleSource.mockReturnValue({ + ruleSource: { type: 'internal' }, + immutable: false, + }); + const rules = [ + { ...getImportRulesSchemaMock(), rule_id: 'rule-1' }, + { ...getImportRulesSchemaMock(), rule_id: 'rule-2' }, + ]; + + await detectionRulesClient.bulkImportRules({ + rules, + overwriteRules: false, + ruleSourceImporter: mockRuleSourceImporter, + }); + + expect(rulesClient.bulkCreateRules).toHaveBeenCalledWith( + expect.objectContaining({ + changeTracking: expect.objectContaining({ + metadata: { bulkCount: rules.length }, + }), + }) + ); + }); + it('importRules passes bulkCount through to importRule', async () => { const importRuleSpy = jest .spyOn(detectionRulesClient, 'importRule') diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts index d07c515b93a68..8fcbd15c479b7 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts @@ -19,9 +19,12 @@ import type { MlAuthz } from '../../../../machine_learning/authz'; import type { ProductFeaturesService } from '../../../../product_features_service'; import { createPrebuiltRuleAssetsClient } from '../../../prebuilt_rules/logic/rule_assets/prebuilt_rule_assets_client'; import type { RuleImportErrorObject } from '../import/errors'; +import type { BulkImportRulesResult } from './methods/bulk_import_rules'; import type { + BulkCreatePrebuiltRulesArgs, BulkDeleteRulesArgs, BulkDeleteRulesReturn, + BulkImportRulesArgs, CreateCustomRuleArgs, CreatePrebuiltRuleArgs, DeleteRuleArgs, @@ -35,7 +38,9 @@ import type { UpgradePrebuiltRuleArgs, } from './detection_rules_client_interface'; import { createRule } from './methods/create_rule'; +import { bulkCreatePrebuiltRules } from './methods/bulk_create_prebuilt_rules'; import { bulkDeleteRules } from './methods/bulk_delete_rules'; +import { bulkImportRules } from './methods/bulk_import_rules'; import { deleteRule } from './methods/delete_rule'; import { importRule } from './methods/import_rule'; import { importRules } from './methods/import_rules'; @@ -123,6 +128,12 @@ export const createDetectionRulesClient = ({ }); }, + async bulkCreatePrebuiltRules(args: BulkCreatePrebuiltRulesArgs) { + return withSecuritySpan('DetectionRulesClient.bulkCreatePrebuiltRules', async () => { + return bulkCreatePrebuiltRules({ actionsClient, rulesClient, mlAuthz, args }); + }); + }, + async updateRule({ ruleUpdate, changeTracking }: UpdateRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.updateRule', async () => { return updateRule({ @@ -223,6 +234,18 @@ export const createDetectionRulesClient = ({ }); }, + async bulkImportRules(args: BulkImportRulesArgs): Promise { + return withSecuritySpan('DetectionRulesClient.bulkImportRules', async () => { + return bulkImportRules({ + actionsClient, + rulesClient, + savedObjectsClient, + mlAuthz, + args, + }); + }); + }, + async getHistoryForRule(args: GetHistoryForRuleArgs) { return withSecuritySpan('DetectionRulesClient.getHistoryForRule', async () => { return getHistoryForRule({ rulesClient, ...args }); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client_interface.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client_interface.ts index a463b26dbc53f..f5aa6b0ae935d 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client_interface.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client_interface.ts @@ -22,11 +22,16 @@ import type { RuleImportErrorObject } from '../import/errors'; import type { PrebuiltRuleAsset } from '../../../prebuilt_rules'; import type { PrebuiltRulesCustomizationStatus } from '../../../../../../common/detection_engine/prebuilt_rules/prebuilt_rule_customization_status'; import type { RuleAlertType } from '../../../rule_schema'; +import type { BulkCreatePrebuiltRulesResult } from './methods/bulk_create_prebuilt_rules'; +import type { BulkImportRulesResult } from './methods/bulk_import_rules'; export interface IDetectionRulesClient { getRuleCustomizationStatus: () => PrebuiltRulesCustomizationStatus; createCustomRule: (args: CreateCustomRuleArgs) => Promise; createPrebuiltRule: (args: CreatePrebuiltRuleArgs) => Promise; + bulkCreatePrebuiltRules: ( + args: BulkCreatePrebuiltRulesArgs + ) => Promise; updateRule: (args: UpdateRuleArgs) => Promise; patchRule: (args: PatchRuleArgs) => Promise; deleteRule: (args: DeleteRuleArgs) => Promise; @@ -35,6 +40,7 @@ export interface IDetectionRulesClient { revertPrebuiltRule: (args: RevertPrebuiltRuleArgs) => Promise; importRule: (args: ImportRuleArgs) => Promise; importRules: (args: ImportRulesArgs) => Promise>; + bulkImportRules: (args: BulkImportRulesArgs) => Promise; getHistoryForRule: (args: GetHistoryForRuleArgs) => Promise; } @@ -48,6 +54,10 @@ export interface CreatePrebuiltRuleArgs { changeTracking?: SecurityRuleChangeTracking; } +export interface BulkCreatePrebuiltRulesArgs { + rules: PrebuiltRuleAsset[]; +} + export interface UpdateRuleArgs { ruleUpdate: RuleUpdateProps; changeTracking?: SecurityRuleChangeTracking; @@ -104,3 +114,5 @@ export interface GetHistoryForRuleArgs { page?: number; perPage?: number; } + +export type BulkImportRulesArgs = ImportRulesArgs; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_create_prebuilt_rules.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_create_prebuilt_rules.ts new file mode 100644 index 0000000000000..140fc1ebb781a --- /dev/null +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_create_prebuilt_rules.ts @@ -0,0 +1,127 @@ +/* + * 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 { v4 as uuidv4 } from 'uuid'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; +import type { RulesClient } from '@kbn/alerting-plugin/server'; +import { ruleTypeMappings } from '@kbn/securitysolution-rules'; +import { SERVER_APP_ID } from '../../../../../../../common'; +import { SecurityRuleChangeTrackingAction } from '../../../../../../../common/detection_engine/rule_management/rule_change_tracking'; +import type { PrebuiltRuleAsset } from '../../../../prebuilt_rules'; +import type { MlAuthz } from '../../../../../machine_learning/authz'; +import type { RuleParams } from '../../../../rule_schema'; +import { convertRuleResponseToAlertingRule } from '../converters/convert_rule_response_to_alerting_rule'; +import { applyRuleDefaults } from '../mergers/apply_rule_defaults'; +import { validateMlAuth } from '../utils'; + +export interface BulkCreatePrebuiltRulesArgs { + rules: PrebuiltRuleAsset[]; +} + +export interface BulkCreatePrebuiltRulesResultItem { + id: string; + rule_id: string; + version: number; +} + +export interface BulkCreatePrebuiltRulesResult { + results: Array<{ result: BulkCreatePrebuiltRulesResultItem }>; + errors: Array<{ item: PrebuiltRuleAsset; error: Error }>; +} + +interface BulkCreatePrebuiltRulesOptions { + actionsClient: ActionsClient; + rulesClient: RulesClient; + mlAuthz: MlAuthz; + args: BulkCreatePrebuiltRulesArgs; +} + +// Bulk-installs prebuilt rules in a single `rulesClient.bulkCreateRules` call; alerting handles batching internally and only returns `successfulIds`, so we re-pair them with the input assets via a uuid map to derive `{ id, rule_id, version }`. +export const bulkCreatePrebuiltRules = async ({ + actionsClient, + rulesClient, + mlAuthz, + args, +}: BulkCreatePrebuiltRulesOptions): Promise => { + const { rules } = args; + const results: BulkCreatePrebuiltRulesResult['results'] = []; + const errors: BulkCreatePrebuiltRulesResult['errors'] = []; + + if (rules.length === 0) return { results, errors }; + + // Per-type ML auth dedupe: each unique rule type is checked once. + const checkedTypes = new Set(); + const mlAuthErrorByType = new Map(); + for (const rule of rules) { + if (!checkedTypes.has(rule.type)) { + checkedTypes.add(rule.type); + try { + await validateMlAuth(mlAuthz, rule.type); + } catch (e) { + mlAuthErrorByType.set(rule.type, e instanceof Error ? e : new Error(String(e))); + } + } + } + + // Pre-assign uuids so per-rule successes/failures from the alerting bulk call + // can be re-paired with their input PrebuiltRuleAsset by id. + const itemById = new Map(); + const bulkInputs: Array<{ + data: ReturnType & { + alertTypeId: string; + consumer: string; + enabled: boolean; + }; + options: { id: string }; + }> = []; + + for (const rule of rules) { + const mlError = mlAuthErrorByType.get(rule.type); + if (mlError) { + errors.push({ item: rule, error: mlError }); + } else { + const id = uuidv4(); + itemById.set(id, rule); + const ruleWithDefaults = applyRuleDefaults({ ...rule, immutable: true }); + const data = { + ...convertRuleResponseToAlertingRule(ruleWithDefaults, actionsClient), + alertTypeId: ruleTypeMappings[rule.type], + consumer: SERVER_APP_ID, + enabled: false, + }; + bulkInputs.push({ data, options: { id } }); + } + } + + if (bulkInputs.length === 0) return { results, errors }; + + const { successfulIds, errors: bulkErrors } = await rulesClient.bulkCreateRules({ + rules: bulkInputs, + changeTracking: { + action: SecurityRuleChangeTrackingAction.ruleInstall, + metadata: { bulkCount: rules.length }, + }, + }); + + for (const id of successfulIds) { + const asset = itemById.get(id); + if (asset) { + results.push({ result: { id, rule_id: asset.rule_id, version: asset.version } }); + } + } + + bulkErrors.forEach((err) => { + const item = itemById.get(err.rule.id); + if (!item) return; + errors.push({ + item, + error: Object.assign(new Error(err.message), { statusCode: err.status }), + }); + }); + + return { results, errors }; +}; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_import_rules.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_import_rules.ts new file mode 100644 index 0000000000000..522d4bd0ab6db --- /dev/null +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/bulk_import_rules.ts @@ -0,0 +1,345 @@ +/* + * 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 pMap from 'p-map'; +import { i18n } from '@kbn/i18n'; +import { v4 as uuidv4 } from 'uuid'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; +import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { SavedObjectsClientContract } from '@kbn/core/server'; +import { ruleTypeMappings } from '@kbn/securitysolution-rules'; +import { SERVER_APP_ID } from '../../../../../../../common'; +import { SecurityRuleChangeTrackingAction } from '../../../../../../../common/detection_engine/rule_management/rule_change_tracking'; +import type { RuleResponse, RuleToImport } from '../../../../../../../common/api/detection_engine'; +import { ruleToImportHasVersion } from '../../../../../../../common/api/detection_engine/rule_management'; +import type { MlAuthz } from '../../../../../machine_learning/authz'; +import type { RuleParams } from '../../../../rule_schema'; +import { findRules } from '../../search/find_rules'; +import { convertAlertingRuleToRuleResponse } from '../converters/convert_alerting_rule_to_rule_response'; +import { convertRuleResponseToAlertingRule } from '../converters/convert_rule_response_to_alerting_rule'; +import { applyRuleDefaults } from '../mergers/apply_rule_defaults'; +import { validateMlAuth } from '../utils'; +import { + type RuleImportErrorObject, + createRuleImportErrorObject, + isRuleImportError, +} from '../../import/errors'; +import { checkRuleExceptionReferences } from '../../import/check_rule_exception_references'; +import { getReferencedExceptionLists } from '../../import/gather_referenced_exceptions'; +import type { IRuleSourceImporter } from '../../import/rule_source_importer'; +import { importRule as importRuleSingle } from './import_rule'; +import { createPrebuiltRuleAssetsClient } from '../../../../prebuilt_rules/logic/rule_assets/prebuilt_rule_assets_client'; + +const OVERWRITE_FALLBACK_CONCURRENCY = 10; + +interface BulkImportRulesOptions { + actionsClient: ActionsClient; + rulesClient: RulesClient; + savedObjectsClient: SavedObjectsClientContract; + mlAuthz: MlAuthz; + args: { + rules: RuleToImport[]; + overwriteRules: boolean; + ruleSourceImporter: IRuleSourceImporter; + allowMissingConnectorSecrets?: boolean; + }; +} + +export interface BulkImportRuleSuccess { + rule_id: string; +} + +export interface BulkImportRulesResult { + responses: Array; +} + +// Per-rule pre-checks (in-process, isolated try/catch). Output: only rules +// that pass all checks proceed to classification. +interface PreparedImport { + rule: RuleToImport; + immutable: boolean; + ruleSource: ReturnType['ruleSource']; + exceptionsList: RuleToImport['exceptions_list']; +} + +// rule_id is free-form; escape backslashes and quotes for safe KQL interpolation. +const escapeKql = (id: string) => id.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); + +const emptyResult = (): BulkImportRulesResult => ({ responses: [] }); + +interface PrepareRuleArgs { + rule: RuleToImport; + mlAuthz: MlAuthz; + ruleSourceImporter: IRuleSourceImporter; + existingLists: Awaited>; + checkedTypes: Set; + mlAuthErrorByType: Map; +} + +interface PrepareRuleResult { + prepared?: PreparedImport; + errors: RuleImportErrorObject[]; +} + +const missingVersionError = (ruleId: string): RuleImportErrorObject => + createRuleImportErrorObject({ + ruleId, + message: i18n.translate( + 'xpack.securitySolution.detectionEngine.rules.cannotImportPrebuiltRuleWithoutVersion', + { + defaultMessage: + 'Prebuilt rules must specify a "version" to be imported. [rule_id: {ruleId}]', + values: { ruleId }, + } + ), + }); + +const cacheMlAuthError = async ({ + rule, + mlAuthz, + checkedTypes, + mlAuthErrorByType, +}: Pick): Promise< + Error | undefined +> => { + if (!checkedTypes.has(rule.type)) { + checkedTypes.add(rule.type); + try { + await validateMlAuth(mlAuthz, rule.type); + } catch (e) { + mlAuthErrorByType.set(rule.type, e instanceof Error ? e : new Error(String(e))); + } + } + return mlAuthErrorByType.get(rule.type); +}; + +const prepareRuleForImport = async ({ + rule, + mlAuthz, + ruleSourceImporter, + existingLists, + checkedTypes, + mlAuthErrorByType, +}: PrepareRuleArgs): Promise => { + if (!ruleSourceImporter.isPrebuiltRule(rule)) { + rule.version = rule.version ?? 1; + } + if (!ruleToImportHasVersion(rule)) { + return { errors: [missingVersionError(rule.rule_id)] }; + } + + const mlError = await cacheMlAuthError({ rule, mlAuthz, checkedTypes, mlAuthErrorByType }); + if (mlError) { + return { + errors: [createRuleImportErrorObject({ ruleId: rule.rule_id, message: mlError.message })], + }; + } + + const [exceptionErrors, exceptionsList] = checkRuleExceptionReferences({ rule, existingLists }); + const errors: RuleImportErrorObject[] = [...exceptionErrors]; + + let ruleSourceResult; + try { + ruleSourceResult = ruleSourceImporter.calculateRuleSource(rule); + } catch (e) { + errors.push( + createRuleImportErrorObject({ + ruleId: rule.rule_id, + message: e instanceof Error ? e.message : String(e), + }) + ); + } + + if (!ruleSourceResult) { + return { errors }; + } + + return { + errors, + prepared: { + rule, + immutable: ruleSourceResult.immutable, + ruleSource: ruleSourceResult.ruleSource, + exceptionsList, + }, + }; +}; + +export const bulkImportRules = async ({ + actionsClient, + rulesClient, + savedObjectsClient, + mlAuthz, + args, +}: BulkImportRulesOptions): Promise => { + const { rules, overwriteRules, ruleSourceImporter, allowMissingConnectorSecrets } = args; + if (rules.length === 0) return emptyResult(); + + const responses: Array = []; + + const existingLists = await getReferencedExceptionLists({ rules, savedObjectsClient }); + await ruleSourceImporter.setup(rules); + + const prepared: PreparedImport[] = []; + const checkedTypes = new Set(); + const mlAuthErrorByType = new Map(); + + for (const rule of rules) { + const result = await prepareRuleForImport({ + rule, + mlAuthz, + ruleSourceImporter, + existingLists, + checkedTypes, + mlAuthErrorByType, + }); + responses.push(...result.errors); + if (result.prepared) { + prepared.push(result.prepared); + } + } + + if (prepared.length === 0) { + return { responses }; + } + + // Bulk lookup for `rule_id` conflicts: single KQL parenthesized OR-list. + const ruleIds = prepared.map((p) => p.rule.rule_id); + const filter = `alert.attributes.params.ruleId: (${ruleIds + .map((id) => `"${escapeKql(id)}"`) + .join(' OR ')})`; + const found = await findRules({ + rulesClient, + filter, + page: 1, + perPage: ruleIds.length, + fields: undefined, + sortField: undefined, + sortOrder: undefined, + }); + const existingByRuleId = new Map(); + for (const alertingRule of found.data) { + const ruleResponse = convertAlertingRuleToRuleResponse(alertingRule); + existingByRuleId.set(ruleResponse.rule_id, ruleResponse); + } + + // Classify: conflict | overwrite-fallback | bulk-create. + const conflicts: PreparedImport[] = []; + const toOverwrite: PreparedImport[] = []; + const toBulkCreate: PreparedImport[] = []; + for (const p of prepared) { + if (existingByRuleId.has(p.rule.rule_id)) { + if (overwriteRules) toOverwrite.push(p); + else conflicts.push(p); + } else { + toBulkCreate.push(p); + } + } + + conflicts.forEach((p) => { + responses.push( + createRuleImportErrorObject({ + ruleId: p.rule.rule_id, + type: 'conflict', + message: 'Rule with this rule_id already exists', + }) + ); + }); + + // Overwrite branch: stays per-rule via existing single-rule importRule. The + // resulting full RuleResponse is collapsed to { rule_id } for a uniform + // success shape across the overwrite and bulk-create branches. + if (toOverwrite.length > 0) { + const prebuiltRuleAssetClient = createPrebuiltRuleAssetsClient(savedObjectsClient); + const overwriteResults = await pMap( + toOverwrite, + async (p): Promise => { + try { + const updated = (await importRuleSingle({ + actionsClient, + rulesClient, + mlAuthz, + prebuiltRuleAssetClient, + importRulePayload: { + ruleToImport: { ...p.rule, exceptions_list: [...(p.exceptionsList ?? [])] }, + overrideFields: { rule_source: p.ruleSource, immutable: p.immutable }, + overwriteRules: true, + allowMissingConnectorSecrets, + }, + })) as RuleResponse | RuleImportErrorObject; + if (isRuleImportError(updated)) return updated; + return { rule_id: updated.rule_id }; + } catch (err) { + if (isRuleImportError(err)) return err; + return createRuleImportErrorObject({ + ruleId: p.rule.rule_id, + message: err?.message ?? 'unknown error', + }); + } + }, + { concurrency: OVERWRITE_FALLBACK_CONCURRENCY } + ); + responses.push(...overwriteResults); + } + + if (toBulkCreate.length === 0) { + return { responses }; + } + + // Bulk-create new rules in a single alerting call. Pre-assign uuids so we + // can re-pair successes/failures back to the source `rule_id`. + const inputById = new Map(); + const bulkInputs = toBulkCreate.map((p) => { + const id = uuidv4(); + inputById.set(id, p); + const ruleResponse = applyRuleDefaults({ + ...p.rule, + exceptions_list: [...(p.exceptionsList ?? [])], + immutable: p.immutable, + rule_source: p.ruleSource, + }); + const data = { + ...convertRuleResponseToAlertingRule(ruleResponse, actionsClient), + alertTypeId: ruleTypeMappings[p.rule.type], + consumer: SERVER_APP_ID, + // Preserve the user-requested enabled flag — alerting handles enabled + // rules natively (API key + task scheduling) in this single call. + enabled: p.rule.enabled ?? false, + }; + return { data, options: { id }, allowMissingConnectorSecrets }; + }); + + const { successfulIds, errors: bulkErrors } = await rulesClient.bulkCreateRules({ + rules: bulkInputs, + changeTracking: { + action: SecurityRuleChangeTrackingAction.ruleImport, + metadata: { bulkCount: rules.length }, + }, + }); + + for (const id of successfulIds) { + const source = inputById.get(id); + if (source) { + responses.push({ rule_id: source.rule.rule_id }); + } + } + + bulkErrors.forEach((err) => { + const source = inputById.get(err.rule.id); + if (!source) return; + responses.push( + createRuleImportErrorObject({ + ruleId: source.rule.rule_id, + message: err.message, + }) + ); + }); + + return { responses }; +}; + +export { escapeKql as __testing_escapeKql }; diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.test.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.test.ts index a6e4e8bad297d..d9fdc06df7116 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.test.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.test.ts @@ -215,4 +215,64 @@ describe('importRules', () => { { rule_id: successfulRuleId, status_code: 200 }, ]); }); + + describe('bulk path (bulkCreateRulesEnabled)', () => { + const experimentalFeatures = { bulkCreateRulesEnabled: true } as never; + + it('flattens all chunks into a single bulkImportRules call (no skipTaskEnabling, no bulkEnableTasks follow-up)', async () => { + const r1 = { ...ruleToImport, rule_id: 'r1' }; + const r2 = { ...ruleToImport, rule_id: 'r2' }; + const r3 = { ...ruleToImport, rule_id: 'r3' }; + + detectionRulesClient.bulkImportRules.mockResolvedValueOnce({ + responses: [{ rule_id: 'r1' }, { rule_id: 'r2' }, { rule_id: 'r3' }], + }); + + const result = await importRules({ + ruleChunks: [[r1, r2], [r3]], + overwriteRules: false, + detectionRulesClient, + ruleSourceImporter: mockRuleSourceImporter, + experimentalFeatures, + }); + + expect(detectionRulesClient.bulkImportRules).toHaveBeenCalledTimes(1); + const args = detectionRulesClient.bulkImportRules.mock.calls[0][0]; + expect(args).not.toHaveProperty('skipTaskEnabling'); + expect(args.rules.map((r) => r.rule_id)).toEqual(['r1', 'r2', 'r3']); + expect(result).toEqual([ + { rule_id: 'r1', status_code: 200 }, + { rule_id: 'r2', status_code: 200 }, + { rule_id: 'r3', status_code: 200 }, + ]); + }); + + it('maps per-rule errors from bulkImportRules to 4xx import responses', async () => { + detectionRulesClient.bulkImportRules.mockResolvedValueOnce({ + responses: [ + createRuleImportErrorObject({ ruleId: 'rule-a', message: 'boom' }), + { rule_id: 'rule-b' }, + createRuleImportErrorObject({ + ruleId: 'rule-c', + message: 'conflict', + type: 'conflict', + }), + ], + }); + + const result = await importRules({ + ruleChunks: [[ruleToImport]], + overwriteRules: false, + detectionRulesClient, + ruleSourceImporter: mockRuleSourceImporter, + experimentalFeatures, + }); + + expect(result).toEqual([ + { error: { message: 'boom', status_code: 400 }, rule_id: 'rule-a' }, + { rule_id: 'rule-b', status_code: 200 }, + { error: { message: 'conflict', status_code: 409 }, rule_id: 'rule-c' }, + ]); + }); + }); }); diff --git a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.ts b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.ts index b1fda8646b390..e09be42fd54d8 100644 --- a/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.ts +++ b/x-pack/solutions/security/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules.ts @@ -6,11 +6,13 @@ */ import type { SecurityRuleChangeTracking } from '../../../../../../common/detection_engine/rule_management/rule_change_tracking'; +import type { ExperimentalFeatures } from '../../../../../../common'; import type { RuleToImport } from '../../../../../../common/api/detection_engine'; import { type ImportRuleResponse, createBulkErrorObject } from '../../../routes/utils'; +import type { BulkImportRuleSuccess } from '../detection_rules_client/methods/bulk_import_rules'; import type { IRuleSourceImporter } from './rule_source_importer'; import type { IDetectionRulesClient } from '../detection_rules_client/detection_rules_client_interface'; -import { isRuleConflictError, isRuleImportError } from './errors'; +import { isRuleConflictError, isRuleImportError, type RuleImportErrorObject } from './errors'; /** * Takes a stream of rules to be imported and either creates or updates rules @@ -19,6 +21,7 @@ import { isRuleConflictError, isRuleImportError } from './errors'; * @param overwriteRules {boolean} - whether to overwrite existing rules * with imported rules if their rule_id matches * @param detectionRulesClient {object} + * @param experimentalFeatures - feature flags; in particular `bulkCreateRulesEnabled` * @returns {Promise} an array of error and success messages from import */ export const importRules = async ({ @@ -28,6 +31,7 @@ export const importRules = async ({ detectionRulesClient, ruleSourceImporter, allowMissingConnectorSecrets, + experimentalFeatures, }: { ruleChunks: RuleToImport[][]; changeTracking?: SecurityRuleChangeTracking; @@ -35,6 +39,7 @@ export const importRules = async ({ detectionRulesClient: IDetectionRulesClient; ruleSourceImporter: IRuleSourceImporter; allowMissingConnectorSecrets?: boolean; + experimentalFeatures?: ExperimentalFeatures; }): Promise => { const response: ImportRuleResponse[] = []; @@ -42,32 +47,47 @@ export const importRules = async ({ return response; } - for (const rules of ruleChunks) { - const importedRulesResponse = await detectionRulesClient.importRules({ - allowMissingConnectorSecrets, - overwriteRules, - ruleSourceImporter, - rules, - changeTracking, - }); - - const importResponses = importedRulesResponse.map((rule) => { - if (isRuleImportError(rule)) { - return createBulkErrorObject({ - message: rule.error.message, - statusCode: isRuleConflictError(rule) ? 409 : 400, - ruleId: rule.error.ruleId, - }); - } + const useBulk = experimentalFeatures?.bulkCreateRulesEnabled ?? false; - return { - rule_id: rule.rule_id, - status_code: 200, - }; - }); - - response.push(...importResponses); + if (!useBulk) { + for (const rules of ruleChunks) { + const importedRulesResponse = await detectionRulesClient.importRules({ + allowMissingConnectorSecrets, + overwriteRules, + ruleSourceImporter, + rules, + changeTracking, + }); + response.push(...importedRulesResponse.map(toImportRuleResponse)); + } + return response; } + const allRules = ruleChunks.flat(); + const { responses } = await detectionRulesClient.bulkImportRules({ + allowMissingConnectorSecrets, + overwriteRules, + ruleSourceImporter, + rules: allRules, + changeTracking, + }); + response.push(...responses.map(toImportRuleResponse)); + return response; }; + +const toImportRuleResponse = ( + rule: BulkImportRuleSuccess | RuleImportErrorObject +): ImportRuleResponse => { + if (isRuleImportError(rule)) { + return createBulkErrorObject({ + message: rule.error.message, + statusCode: isRuleConflictError(rule) ? 409 : 400, + ruleId: rule.error.ruleId, + }); + } + return { + rule_id: rule.rule_id, + status_code: 200, + }; +}; From 5c3e2bea70e3f43ea0d2afca6564c35eba843df0 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 28 May 2026 14:55:24 +0000 Subject: [PATCH 8/8] Changes from node scripts/eslint_all_files --no-cache --fix --- .../methods/bulk_create/bulk_create_rules.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts index 02e06356d63a2..33bdbf6259835 100644 --- a/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts +++ b/x-pack/platform/plugins/shared/alerting/server/application/rule/methods/bulk_create/bulk_create_rules.ts @@ -350,9 +350,8 @@ async function runBatch({ let scheduledIds: string[] = []; const survivingEnabledIds = survivingEnabled.map((p) => p.id); try { - const scheduledTasks = await withSpan( - { name: 'runBatch.bulkSchedule', type: 'tasks' }, - () => context.taskManager.bulkSchedule(tasksToSchedule) + const scheduledTasks = await withSpan({ name: 'runBatch.bulkSchedule', type: 'tasks' }, () => + context.taskManager.bulkSchedule(tasksToSchedule) ); scheduledIds = scheduledTasks.map((task) => task.id); } catch (error) { @@ -420,13 +419,11 @@ async function runBatch({ let bulkResponse; try { - bulkResponse = await withSpan( - { name: 'runBatch.bulkCreateRulesSo', type: 'rules' }, - () => - bulkCreateRulesSo({ - savedObjectsClient: context.unsecuredSavedObjectsClient, - bulkCreateRuleAttributes: bulkObjects, - }) + bulkResponse = await withSpan({ name: 'runBatch.bulkCreateRulesSo', type: 'rules' }, () => + bulkCreateRulesSo({ + savedObjectsClient: context.unsecuredSavedObjectsClient, + bulkCreateRuleAttributes: bulkObjects, + }) ); } catch (error) { // Whole-call SO failure: invalidate keys, best-effort task cleanup.