mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 01:38:56 -04:00
[RAM] replaces savedObjectClient.bulkUpdate with bulkCreate method in RulesClient.bulkEdit (#137593) (#137976)
## Summary
`savedObjectClient.bulkUpdate` method uses ES _bulk API with `update` action, which doesn’t allow to delete fields from Saved Objects(SO), i.e. overwrite them with undefined
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html
Instead, in this PR it replaced by `savedObjectClient.bulkCreate` with `overwrite=true`, that allows to remove existing fields by complete overwrite of SOs. It results in _bulk API query `index` action
### 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
(cherry picked from commit ff062cea07)
Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com>
This commit is contained in:
Kibana Machine
GitHub
parent
88971dde15
commit
c01a9c9609
3 changed files with 127 additions and 56 deletions
|
|
@ -1796,7 +1796,7 @@ export class RulesClient {
|
|||
|
||||
let result;
|
||||
try {
|
||||
result = await this.unsecuredSavedObjectsClient.bulkUpdate(rules);
|
||||
result = await this.unsecuredSavedObjectsClient.bulkCreate(rules, { overwrite: true });
|
||||
} catch (e) {
|
||||
// avoid unused newly generated API keys
|
||||
if (apiKeysMap.size > 0) {
|
||||
|
|
|
|||
|
|
@ -124,7 +124,7 @@ describe('bulkEdit()', () => {
|
|||
|
||||
mockCreatePointInTimeFinderAsInternalUser();
|
||||
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [existingRule],
|
||||
});
|
||||
|
||||
|
|
@ -145,7 +145,7 @@ describe('bulkEdit()', () => {
|
|||
});
|
||||
describe('tags operations', () => {
|
||||
test('should add new tag', async () => {
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
@ -184,21 +184,23 @@ describe('bulkEdit()', () => {
|
|||
expect(result.rules).toHaveLength(1);
|
||||
expect(result.rules[0]).toHaveProperty('tags', ['foo', 'test-1']);
|
||||
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0]).toHaveLength(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0][0]).toEqual([
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
tags: ['foo', 'test-1'],
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
|
||||
[
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
tags: ['foo', 'test-1'],
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
]);
|
||||
],
|
||||
{ overwrite: true }
|
||||
);
|
||||
});
|
||||
|
||||
test('should delete tag', async () => {
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
@ -234,21 +236,23 @@ describe('bulkEdit()', () => {
|
|||
|
||||
expect(result.rules[0]).toHaveProperty('tags', []);
|
||||
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0]).toHaveLength(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0][0]).toEqual([
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
tags: [],
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
|
||||
[
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
tags: [],
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
]);
|
||||
],
|
||||
{ overwrite: true }
|
||||
);
|
||||
});
|
||||
|
||||
test('should set tags', async () => {
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
@ -284,17 +288,19 @@ describe('bulkEdit()', () => {
|
|||
|
||||
expect(result.rules[0]).toHaveProperty('tags', ['test-1', 'test-2']);
|
||||
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0]).toHaveLength(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0][0]).toEqual([
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
tags: ['test-1', 'test-2'],
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
|
||||
[
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
tags: ['test-1', 'test-2'],
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
]);
|
||||
],
|
||||
{ overwrite: true }
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -574,7 +580,7 @@ describe('bulkEdit()', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('should call bulkMarkApiKeysForInvalidation to invalidate unused keys if bulkUpdate failed', async () => {
|
||||
test('should call bulkMarkApiKeysForInvalidation to invalidate unused keys if bulkCreate failed', async () => {
|
||||
createAPIKeyMock.mockReturnValue({ apiKeysEnabled: true, result: { api_key: '111' } });
|
||||
mockCreatePointInTimeFinderAsInternalUser({
|
||||
saved_objects: [
|
||||
|
|
@ -585,7 +591,7 @@ describe('bulkEdit()', () => {
|
|||
],
|
||||
});
|
||||
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockImplementation(() => {
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockImplementation(() => {
|
||||
throw new Error('Fail');
|
||||
});
|
||||
|
||||
|
|
@ -621,7 +627,7 @@ describe('bulkEdit()', () => {
|
|||
],
|
||||
});
|
||||
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
@ -795,7 +801,7 @@ describe('bulkEdit()', () => {
|
|||
minimumScheduleInterval: { value: '3m', enforce: true },
|
||||
});
|
||||
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [],
|
||||
});
|
||||
|
||||
|
|
@ -819,7 +825,7 @@ describe('bulkEdit()', () => {
|
|||
|
||||
describe('paramsModifier', () => {
|
||||
test('should update index pattern params', async () => {
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
@ -856,19 +862,21 @@ describe('bulkEdit()', () => {
|
|||
expect(result.rules).toHaveLength(1);
|
||||
expect(result.rules[0]).toHaveProperty('params.index', ['test-index-*']);
|
||||
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0]).toHaveLength(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkUpdate.mock.calls[0][0]).toEqual([
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
params: expect.objectContaining({
|
||||
index: ['test-index-*'],
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1);
|
||||
expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith(
|
||||
[
|
||||
expect.objectContaining({
|
||||
id: '1',
|
||||
type: 'alert',
|
||||
attributes: expect.objectContaining({
|
||||
params: expect.objectContaining({
|
||||
index: ['test-index-*'],
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
]);
|
||||
],
|
||||
{ overwrite: true }
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -893,8 +901,8 @@ describe('bulkEdit()', () => {
|
|||
});
|
||||
|
||||
describe('task manager', () => {
|
||||
test('should call task manager method bulkUpdateSchedules if operation set new schedules', async () => {
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
test('should call task manager method bulkCreateSchedules if operation set new schedules', async () => {
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
@ -932,8 +940,8 @@ describe('bulkEdit()', () => {
|
|||
});
|
||||
});
|
||||
|
||||
test('should not call task manager method bulkUpdateSchedules if operation is not set schedule', async () => {
|
||||
unsecuredSavedObjectsClient.bulkUpdate.mockResolvedValue({
|
||||
test('should not call task manager method bulkCreateSchedules if operation is not set schedule', async () => {
|
||||
unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({
|
||||
saved_objects: [
|
||||
{
|
||||
id: '1',
|
||||
|
|
|
|||
|
|
@ -8,12 +8,20 @@
|
|||
import expect from '@kbn/expect';
|
||||
import type { SanitizedRule } from '@kbn/alerting-plugin/common';
|
||||
import { Spaces } from '../../scenarios';
|
||||
import { checkAAD, getUrlPrefix, getTestRuleData, ObjectRemover } from '../../../common/lib';
|
||||
import {
|
||||
checkAAD,
|
||||
getUrlPrefix,
|
||||
getTestRuleData,
|
||||
ObjectRemover,
|
||||
createWaitForExecutionCount,
|
||||
} from '../../../common/lib';
|
||||
import { FtrProviderContext } from '../../../common/ftr_provider_context';
|
||||
|
||||
// eslint-disable-next-line import/no-default-export
|
||||
export default function createUpdateTests({ getService }: FtrProviderContext) {
|
||||
const supertest = getService('supertest');
|
||||
const retry = getService('retry');
|
||||
const waitForExecutionCount = createWaitForExecutionCount(supertest, Spaces.space1.id);
|
||||
|
||||
describe('bulkEdit', () => {
|
||||
const objectRemover = new ObjectRemover(supertest);
|
||||
|
|
@ -310,5 +318,60 @@ export default function createUpdateTests({ getService }: FtrProviderContext) {
|
|||
severity: '40-medium',
|
||||
});
|
||||
});
|
||||
|
||||
it('should not overwrite internal field monitoring', async () => {
|
||||
const { body: createdRule } = await supertest
|
||||
.post(`${getUrlPrefix(Spaces.space1.id)}/api/alerting/rule`)
|
||||
.set('kbn-xsrf', 'foo')
|
||||
.send(
|
||||
getTestRuleData({
|
||||
enabled: true,
|
||||
tags: ['default'],
|
||||
params: { risk_score: 40, severity: 'medium' },
|
||||
})
|
||||
);
|
||||
|
||||
objectRemover.add(Spaces.space1.id, createdRule.id, 'rule', 'alerting');
|
||||
|
||||
await waitForExecutionCount(1, createdRule.id);
|
||||
|
||||
const monitoringData = (
|
||||
await supertest.get(
|
||||
`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`
|
||||
)
|
||||
).body.monitoring;
|
||||
|
||||
// single rule execution is recorded in monitoring history
|
||||
expect(monitoringData.execution.history).to.have.length(1);
|
||||
|
||||
const payload = {
|
||||
ids: [createdRule.id],
|
||||
operations: [
|
||||
{
|
||||
operation: 'add',
|
||||
field: 'tags',
|
||||
value: ['tag-1'],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const bulkEditResponse = await retry.try(async () =>
|
||||
supertest
|
||||
.post(`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rules/_bulk_edit`)
|
||||
.set('kbn-xsrf', 'foo')
|
||||
.send(payload)
|
||||
.expect(200)
|
||||
);
|
||||
|
||||
// after applying bulk edit action monitoring still available
|
||||
expect(bulkEditResponse.body.rules[0].monitoring).to.eql(monitoringData);
|
||||
|
||||
// test if monitoring data persistent
|
||||
const getRuleResponse = await supertest.get(
|
||||
`${getUrlPrefix(Spaces.space1.id)}/internal/alerting/rule/${createdRule.id}`
|
||||
);
|
||||
|
||||
expect(getRuleResponse.body.monitoring).to.eql(monitoringData);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue