-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[RAM] bulk enable rules api (#144216)
Connected #144082 Summary I am working on getting alive bulk enable for rules. I this PR I've created a new internal endpoint _bulk_enable. This endpoint is getting rules ids or filter as parameters and return object like this: { errors: [], total, }. Decide to stick with only enable in this PR to keep it smaller. In a next PR I'll decide if I should create disable endpoint or change enable to handle disable as well. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
- Loading branch information
1 parent
1566897
commit e76f15c
Showing
13 changed files
with
1,670 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
126 changes: 126 additions & 0 deletions
126
x-pack/plugins/alerting/server/routes/bulk_enable_rules.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
/* | ||
* 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 { httpServiceMock } from '@kbn/core/server/mocks'; | ||
|
||
import { bulkEnableRulesRoute } from './bulk_enable_rules'; | ||
import { licenseStateMock } from '../lib/license_state.mock'; | ||
import { mockHandlerArguments } from './_mock_handler_arguments'; | ||
import { rulesClientMock } from '../rules_client.mock'; | ||
import { RuleTypeDisabledError } from '../lib/errors/rule_type_disabled'; | ||
import { verifyApiAccess } from '../lib/license_api_access'; | ||
|
||
const rulesClient = rulesClientMock.create(); | ||
|
||
jest.mock('../lib/license_api_access', () => ({ | ||
verifyApiAccess: jest.fn(), | ||
})); | ||
|
||
beforeEach(() => { | ||
jest.resetAllMocks(); | ||
}); | ||
|
||
describe('bulkEnableRulesRoute', () => { | ||
const bulkEnableRequest = { filter: '' }; | ||
const bulkEnableResult = { errors: [], total: 1, taskIdsFailedToBeEnabled: [] }; | ||
|
||
it('should enable rules with proper parameters', async () => { | ||
const licenseState = licenseStateMock.create(); | ||
const router = httpServiceMock.createRouter(); | ||
|
||
bulkEnableRulesRoute({ router, licenseState }); | ||
|
||
const [config, handler] = router.patch.mock.calls[0]; | ||
|
||
expect(config.path).toBe('/internal/alerting/rules/_bulk_enable'); | ||
|
||
rulesClient.bulkEnableRules.mockResolvedValueOnce(bulkEnableResult); | ||
|
||
const [context, req, res] = mockHandlerArguments( | ||
{ rulesClient }, | ||
{ | ||
body: bulkEnableRequest, | ||
}, | ||
['ok'] | ||
); | ||
|
||
expect(await handler(context, req, res)).toEqual({ | ||
body: bulkEnableResult, | ||
}); | ||
|
||
expect(rulesClient.bulkEnableRules).toHaveBeenCalledTimes(1); | ||
expect(rulesClient.bulkEnableRules.mock.calls[0]).toEqual([bulkEnableRequest]); | ||
|
||
expect(res.ok).toHaveBeenCalled(); | ||
}); | ||
|
||
it('ensures the license allows bulk enabling rules', async () => { | ||
const licenseState = licenseStateMock.create(); | ||
const router = httpServiceMock.createRouter(); | ||
|
||
rulesClient.bulkEnableRules.mockResolvedValueOnce(bulkEnableResult); | ||
|
||
bulkEnableRulesRoute({ router, licenseState }); | ||
|
||
const [, handler] = router.patch.mock.calls[0]; | ||
|
||
const [context, req, res] = mockHandlerArguments( | ||
{ rulesClient }, | ||
{ | ||
body: bulkEnableRequest, | ||
} | ||
); | ||
|
||
await handler(context, req, res); | ||
|
||
expect(verifyApiAccess).toHaveBeenCalledWith(licenseState); | ||
}); | ||
|
||
it('ensures the license check prevents bulk enabling rules', async () => { | ||
const licenseState = licenseStateMock.create(); | ||
const router = httpServiceMock.createRouter(); | ||
|
||
(verifyApiAccess as jest.Mock).mockImplementation(() => { | ||
throw new Error('Failure'); | ||
}); | ||
|
||
bulkEnableRulesRoute({ router, licenseState }); | ||
|
||
const [, handler] = router.patch.mock.calls[0]; | ||
|
||
const [context, req, res] = mockHandlerArguments( | ||
{ rulesClient }, | ||
{ | ||
body: bulkEnableRequest, | ||
} | ||
); | ||
|
||
expect(handler(context, req, res)).rejects.toMatchInlineSnapshot(`[Error: Failure]`); | ||
}); | ||
|
||
it('ensures the rule type gets validated for the license', async () => { | ||
const licenseState = licenseStateMock.create(); | ||
const router = httpServiceMock.createRouter(); | ||
|
||
bulkEnableRulesRoute({ router, licenseState }); | ||
|
||
const [, handler] = router.patch.mock.calls[0]; | ||
|
||
rulesClient.bulkEnableRules.mockRejectedValue( | ||
new RuleTypeDisabledError('Fail', 'license_invalid') | ||
); | ||
|
||
const [context, req, res] = mockHandlerArguments({ rulesClient }, { params: {}, body: {} }, [ | ||
'ok', | ||
'forbidden', | ||
]); | ||
|
||
await handler(context, req, res); | ||
|
||
expect(res.forbidden).toHaveBeenCalledWith({ body: { message: 'Fail' } }); | ||
}); | ||
}); |
50 changes: 50 additions & 0 deletions
50
x-pack/plugins/alerting/server/routes/bulk_enable_rules.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* 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 { schema } from '@kbn/config-schema'; | ||
import { IRouter } from '@kbn/core/server'; | ||
import { verifyAccessAndContext, handleDisabledApiKeysError } from './lib'; | ||
import { ILicenseState, RuleTypeDisabledError } from '../lib'; | ||
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types'; | ||
|
||
export const bulkEnableRulesRoute = ({ | ||
router, | ||
licenseState, | ||
}: { | ||
router: IRouter<AlertingRequestHandlerContext>; | ||
licenseState: ILicenseState; | ||
}) => { | ||
router.patch( | ||
{ | ||
path: `${INTERNAL_BASE_ALERTING_API_PATH}/rules/_bulk_enable`, | ||
validate: { | ||
body: schema.object({ | ||
filter: schema.maybe(schema.string()), | ||
ids: schema.maybe(schema.arrayOf(schema.string(), { minSize: 1, maxSize: 1000 })), | ||
}), | ||
}, | ||
}, | ||
handleDisabledApiKeysError( | ||
router.handleLegacyErrors( | ||
verifyAccessAndContext(licenseState, async (context, req, res) => { | ||
const rulesClient = (await context.alerting).getRulesClient(); | ||
const { filter, ids } = req.body; | ||
|
||
try { | ||
const result = await rulesClient.bulkEnableRules({ filter, ids }); | ||
return res.ok({ body: result }); | ||
} catch (e) { | ||
if (e instanceof RuleTypeDisabledError) { | ||
return e.sendResponse(res); | ||
} | ||
throw e; | ||
} | ||
}) | ||
) | ||
) | ||
); | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
119 changes: 119 additions & 0 deletions
119
x-pack/plugins/alerting/server/rules_client/lib/retry_if_bulk_enable_conflicts.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
/* | ||
* 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 { chunk } from 'lodash'; | ||
import { KueryNode } from '@kbn/es-query'; | ||
import { Logger } from '@kbn/core/server'; | ||
import { convertRuleIdsToKueryNode } from '../../lib'; | ||
import { BulkOperationError } from '../rules_client'; | ||
import { waitBeforeNextRetry, RETRY_IF_CONFLICTS_ATTEMPTS } from './wait_before_next_retry'; | ||
|
||
const MAX_RULES_IDS_IN_RETRY = 1000; | ||
|
||
export type BulkEnableOperation = (filter: KueryNode | null) => Promise<{ | ||
errors: BulkOperationError[]; | ||
taskIdsToEnable: string[]; | ||
}>; | ||
|
||
interface ReturnRetry { | ||
errors: BulkOperationError[]; | ||
taskIdsToEnable: string[]; | ||
} | ||
|
||
/** | ||
* Retries BulkEnable requests | ||
* If in response are presents conflicted savedObjects(409 statusCode), this util constructs filter with failed SO ids and retries bulkEnable operation until | ||
* all SO updated or number of retries exceeded | ||
* @param logger | ||
* @param bulkEnableOperation | ||
* @param filter - KueryNode filter | ||
* @param retries - number of retries left | ||
* @param accErrors - accumulated conflict errors | ||
* @param accTaskIdsToEnable - accumulated task ids | ||
* @returns Promise<ReturnRetry> | ||
*/ | ||
|
||
export const retryIfBulkEnableConflicts = async ( | ||
logger: Logger, | ||
bulkEnableOperation: BulkEnableOperation, | ||
filter: KueryNode | null, | ||
retries: number = RETRY_IF_CONFLICTS_ATTEMPTS, | ||
accErrors: BulkOperationError[] = [], | ||
accTaskIdsToEnable: string[] = [] | ||
): Promise<ReturnRetry> => { | ||
try { | ||
const { errors: currentErrors, taskIdsToEnable: currentTaskIdsToEnable } = | ||
await bulkEnableOperation(filter); | ||
|
||
const taskIdsToEnable = [...accTaskIdsToEnable, ...currentTaskIdsToEnable]; | ||
const errors = | ||
retries <= 0 | ||
? [...accErrors, ...currentErrors] | ||
: [...accErrors, ...currentErrors.filter((error) => error.status !== 409)]; | ||
|
||
const ruleIdsWithConflictError = currentErrors.reduce<string[]>((acc, error) => { | ||
if (error.status === 409) { | ||
return [...acc, error.rule.id]; | ||
} | ||
return acc; | ||
}, []); | ||
|
||
if (ruleIdsWithConflictError.length === 0) { | ||
return { | ||
errors, | ||
taskIdsToEnable, | ||
}; | ||
} | ||
|
||
if (retries <= 0) { | ||
logger.warn('Bulk enable rules conflicts, exceeded retries'); | ||
|
||
return { | ||
errors, | ||
taskIdsToEnable, | ||
}; | ||
} | ||
|
||
logger.debug( | ||
`Bulk enable rules conflicts, retrying ..., ${ruleIdsWithConflictError.length} saved objects conflicted` | ||
); | ||
|
||
await waitBeforeNextRetry(retries); | ||
|
||
// here, we construct filter query with ids. But, due to a fact that number of conflicted saved objects can exceed few thousands we can encounter following error: | ||
// "all shards failed: search_phase_execution_exception: [query_shard_exception] Reason: failed to create query: maxClauseCount is set to 2621" | ||
// That's why we chunk processing ids into pieces by size equals to MAX_RULES_IDS_IN_RETRY | ||
return ( | ||
await pMap( | ||
chunk(ruleIdsWithConflictError, MAX_RULES_IDS_IN_RETRY), | ||
async (queryIds) => | ||
retryIfBulkEnableConflicts( | ||
logger, | ||
bulkEnableOperation, | ||
convertRuleIdsToKueryNode(queryIds), | ||
retries - 1, | ||
errors, | ||
taskIdsToEnable | ||
), | ||
{ | ||
concurrency: 1, | ||
} | ||
) | ||
).reduce<ReturnRetry>( | ||
(acc, item) => { | ||
return { | ||
errors: [...acc.errors, ...item.errors], | ||
taskIdsToEnable: [...acc.taskIdsToEnable, ...item.taskIdsToEnable], | ||
}; | ||
}, | ||
{ errors: [], taskIdsToEnable: [] } | ||
); | ||
} catch (err) { | ||
throw err; | ||
} | ||
}; |
Oops, something went wrong.