Skip to content

Commit

Permalink
[Cases] Performance and RBAC improvements (#101465) (#101652)
Browse files Browse the repository at this point in the history
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
cnasikas and kibanamachine authored Jun 8, 2021
1 parent 11ecb4e commit f2a5c47
Show file tree
Hide file tree
Showing 15 changed files with 182 additions and 197 deletions.
20 changes: 10 additions & 10 deletions x-pack/plugins/cases/common/api/cases/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ import * as rt from 'io-ts';

import { UserRT } from '../user';
import { CaseConnectorRt, ConnectorMappingsRt, ESCaseConnector } from '../connectors';
import { OmitProp } from '../runtime_types';
import { OWNER_FIELD } from './constants';

// TODO: we will need to add this type rt.literal('close-by-third-party')
const ClosureTypeRT = rt.union([rt.literal('close-by-user'), rt.literal('close-by-pushing')]);

const CasesConfigureBasicRt = rt.type({
const CasesConfigureBasicWithoutOwnerRt = rt.type({
/**
* The external connector
*/
Expand All @@ -24,15 +22,17 @@ const CasesConfigureBasicRt = rt.type({
* Whether to close the case after it has been synced with the external system
*/
closure_type: ClosureTypeRT,
/**
* The plugin owner that manages this configuration
*/
owner: rt.string,
});

const CasesConfigureBasicWithoutOwnerRt = rt.type(
OmitProp(CasesConfigureBasicRt.props, OWNER_FIELD)
);
const CasesConfigureBasicRt = rt.intersection([
CasesConfigureBasicWithoutOwnerRt,
rt.type({
/**
* The plugin owner that manages this configuration
*/
owner: rt.string,
}),
]);

export const CasesConfigureRequestRt = CasesConfigureBasicRt;
export const CasesConfigurePatchRt = rt.intersection([
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/cases/common/api/runtime_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { omit } from 'lodash';
import { either, fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';
import { pipe } from 'fp-ts/lib/pipeable';
Expand All @@ -14,9 +13,6 @@ import { isObject } from 'lodash/fp';

type ErrorFactory = (message: string) => Error;

export const OmitProp = <O extends rt.Props, K extends keyof O>(o: O, k: K): Omit<O, K> =>
omit(o, k);

/**
* @deprecated Use packages/kbn-securitysolution-io-ts-utils/src/format_errors/index.ts
* Bug fix for the TODO is in the format_errors package
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/cases/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,6 @@ export const ENABLE_CASE_CONNECTOR = false;
if (ENABLE_CASE_CONNECTOR) {
SAVED_OBJECT_TYPES.push(SUB_CASE_SAVED_OBJECT);
}

export const MAX_DOCS_PER_PAGE = 10000;
export const MAX_CONCURRENT_SEARCHES = 10;
30 changes: 19 additions & 11 deletions x-pack/plugins/cases/server/client/attachments/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@
*/

import Boom from '@hapi/boom';
import { CASE_SAVED_OBJECT, SUB_CASE_SAVED_OBJECT } from '../../../common/constants';

import { AssociationType } from '../../../common/api';
import pMap from 'p-map';

import { SavedObject } from 'kibana/public';
import {
CASE_SAVED_OBJECT,
MAX_CONCURRENT_SEARCHES,
SUB_CASE_SAVED_OBJECT,
} from '../../../common/constants';
import { AssociationType, CommentAttributes } from '../../../common/api';
import { CasesClientArgs } from '../types';
import { buildCommentUserActionItem } from '../../services/user_actions/helpers';
import { createCaseError } from '../../common/error';
Expand Down Expand Up @@ -88,14 +94,16 @@ export async function deleteAll(
})),
});

await Promise.all(
comments.saved_objects.map((comment) =>
attachmentService.delete({
unsecuredSavedObjectsClient,
attachmentId: comment.id,
})
)
);
const mapper = async (comment: SavedObject<CommentAttributes>) =>
attachmentService.delete({
unsecuredSavedObjectsClient,
attachmentId: comment.id,
});

// Ensuring we don't too many concurrent deletions running.
await pMap(comments.saved_objects, mapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});

const deleteDate = new Date().toISOString();

Expand Down
102 changes: 58 additions & 44 deletions x-pack/plugins/cases/server/client/cases/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
* 2.0.
*/

import pMap from 'p-map';
import { Boom } from '@hapi/boom';
import { SavedObjectsClientContract } from 'kibana/server';
import { ENABLE_CASE_CONNECTOR } from '../../../common/constants';
import { SavedObject, SavedObjectsClientContract, SavedObjectsFindResponse } from 'kibana/server';
import { ENABLE_CASE_CONNECTOR, MAX_CONCURRENT_SEARCHES } from '../../../common/constants';
import { CasesClientArgs } from '..';
import { createCaseError } from '../../common/error';
import { AttachmentService, CasesService } from '../../services';
import { buildCaseUserActionItem } from '../../services/user_actions/helpers';
import { Operations, OwnerEntity } from '../../authorization';
import { OWNER_FIELD } from '../../../common/api';
import { OWNER_FIELD, SubCaseAttributes, CommentAttributes } from '../../../common/api';

async function deleteSubCases({
attachmentService,
Expand All @@ -37,19 +38,24 @@ async function deleteSubCases({
id: subCaseIDs,
});

// This shouldn't actually delete anything because all the comments should be deleted when comments are deleted
// per case ID
await Promise.all(
commentsForSubCases.saved_objects.map((commentSO) =>
attachmentService.delete({ unsecuredSavedObjectsClient, attachmentId: commentSO.id })
)
);

await Promise.all(
subCasesForCaseIds.saved_objects.map((subCaseSO) =>
caseService.deleteSubCase(unsecuredSavedObjectsClient, subCaseSO.id)
)
);
const commentMapper = (commentSO: SavedObject<CommentAttributes>) =>
attachmentService.delete({ unsecuredSavedObjectsClient, attachmentId: commentSO.id });

const subCasesMapper = (subCaseSO: SavedObject<SubCaseAttributes>) =>
caseService.deleteSubCase(unsecuredSavedObjectsClient, subCaseSO.id);

/**
* This shouldn't actually delete anything because
* all the comments should be deleted when comments are deleted
* per case ID. We also ensure that we don't too many concurrent deletions running.
*/
await pMap(commentsForSubCases.saved_objects, commentMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});

await pMap(subCasesForCaseIds.saved_objects, subCasesMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});
}

/**
Expand Down Expand Up @@ -88,38 +94,46 @@ export async function deleteCases(ids: string[], clientArgs: CasesClientArgs): P
entities: Array.from(entities.values()),
});

await Promise.all(
ids.map((id) =>
caseService.deleteCase({
unsecuredSavedObjectsClient,
id,
})
)
);
const deleteCasesMapper = async (id: string) =>
caseService.deleteCase({
unsecuredSavedObjectsClient,
id,
});

const comments = await Promise.all(
ids.map((id) =>
caseService.getAllCaseComments({
// Ensuring we don't too many concurrent deletions running.
await pMap(ids, deleteCasesMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});

const getCommentsMapper = async (id: string) =>
caseService.getAllCaseComments({
unsecuredSavedObjectsClient,
id,
});

// Ensuring we don't too many concurrent get running.
const comments = await pMap(ids, getCommentsMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});

/**
* This is a nested pMap.Mapper.
* Each element of the comments array contains all comments of a particular case.
* For that reason we need first to create a map that iterate over all cases
* and return a pMap that deletes the comments for that case
*/
const deleteCommentsMapper = async (commentRes: SavedObjectsFindResponse<CommentAttributes>) =>
pMap(commentRes.saved_objects, (comment) =>
attachmentService.delete({
unsecuredSavedObjectsClient,
id,
attachmentId: comment.id,
})
)
);

if (comments.some((c) => c.saved_objects.length > 0)) {
await Promise.all(
comments.map((c) =>
Promise.all(
c.saved_objects.map(({ id }) =>
attachmentService.delete({
unsecuredSavedObjectsClient,
attachmentId: id,
})
)
)
)
);
}

// Ensuring we don't too many concurrent deletions running.
await pMap(comments, deleteCommentsMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});

if (ENABLE_CASE_CONNECTOR) {
await deleteSubCases({
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/server/client/cases/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export const find = async (

ensureSavedObjectsAreAuthorized([...cases.casesMap.values()]);

// casesStatuses are bounded by us. No need to limit concurrent calls.
const [openCases, inProgressCases, closedCases] = await Promise.all([
...caseStatuses.map((status) => {
const statusQuery = constructQueryOptions({ ...queryArgs, status, authorizationFilter });
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/cases/server/client/cases/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import pMap from 'p-map';
import Boom from '@hapi/boom';
import { pipe } from 'fp-ts/lib/pipeable';
import { fold } from 'fp-ts/lib/Either';
Expand Down Expand Up @@ -42,6 +43,7 @@ import { CasesService } from '../../services';
import {
CASE_COMMENT_SAVED_OBJECT,
CASE_SAVED_OBJECT,
MAX_CONCURRENT_SEARCHES,
SUB_CASE_SAVED_OBJECT,
} from '../../../common/constants';
import {
Expand Down Expand Up @@ -162,9 +164,11 @@ async function throwIfInvalidUpdateOfTypeWithAlerts({
};

const requestsUpdatingTypeField = requests.filter((req) => req.type === CaseType.collection);
const casesAlertTotals = await Promise.all(
requestsUpdatingTypeField.map((caseToUpdate) => getAlertsForID(caseToUpdate))
);
const getAlertsMapper = async (caseToUpdate: ESCasePatchRequest) => getAlertsForID(caseToUpdate);
// Ensuring we don't too many concurrent get running.
const casesAlertTotals = await pMap(requestsUpdatingTypeField, getAlertsMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});

// grab the cases that have at least one alert comment attached to them
const typeUpdateWithAlerts = casesAlertTotals.filter((caseInfo) => caseInfo.alerts.total > 0);
Expand Down
30 changes: 20 additions & 10 deletions x-pack/plugins/cases/server/client/configure/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import pMap from 'p-map';
import Boom from '@hapi/boom';
import { pipe } from 'fp-ts/lib/pipeable';
import { fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';

import { SavedObjectsFindResponse, SavedObjectsUtils } from '../../../../../../src/core/server';
import { SUPPORTED_CONNECTORS } from '../../../common/constants';
import {
SavedObject,
SavedObjectsFindResponse,
SavedObjectsUtils,
} from '../../../../../../src/core/server';
import { MAX_CONCURRENT_SEARCHES, SUPPORTED_CONNECTORS } from '../../../common/constants';
import {
CaseConfigureResponseRt,
CasesConfigurePatch,
Expand All @@ -26,6 +32,7 @@ import {
CaseConfigurationsResponseRt,
CasesConfigurePatchRt,
ConnectorMappings,
ESCasesConfigureAttributes,
} from '../../../common/api';
import { createCaseError } from '../../common/error';
import {
Expand Down Expand Up @@ -175,8 +182,9 @@ async function get(
}))
);

const configurations = await Promise.all(
myCaseConfigure.saved_objects.map(async (configuration) => {
const configurations = await pMap(
myCaseConfigure.saved_objects,
async (configuration: SavedObject<ESCasesConfigureAttributes>) => {
const { connector, ...caseConfigureWithoutConnector } = configuration?.attributes ?? {
connector: null,
};
Expand Down Expand Up @@ -204,7 +212,7 @@ async function get(
error,
id: configuration.id,
};
})
}
);

return CaseConfigurationsResponseRt.encode(configurations);
Expand Down Expand Up @@ -400,11 +408,13 @@ async function create(
);

if (myCaseConfigure.saved_objects.length > 0) {
await Promise.all(
myCaseConfigure.saved_objects.map((cc) =>
caseConfigureService.delete({ unsecuredSavedObjectsClient, configurationId: cc.id })
)
);
const deleteConfigurationMapper = async (c: SavedObject<ESCasesConfigureAttributes>) =>
caseConfigureService.delete({ unsecuredSavedObjectsClient, configurationId: c.id });

// Ensuring we don't too many concurrent deletions running.
await pMap(myCaseConfigure.saved_objects, deleteConfigurationMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
});
}

const savedObjectID = SavedObjectsUtils.generateId();
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/cases/server/client/stats/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ async function getStatusTotalsByType(
ensureSavedObjectsAreAuthorized,
} = await authorization.getAuthorizationFilter(Operations.getCaseStatuses);

// casesStatuses are bounded by us. No need to limit concurrent calls.
const [openCases, inProgressCases, closedCases] = await Promise.all([
...caseStatuses.map((status) => {
const statusQuery = constructQueryOptions({
Expand Down
Loading

0 comments on commit f2a5c47

Please sign in to comment.