Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [Cases] Performance and RBAC improvements (#101465) #101652

Merged
merged 1 commit into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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