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

[Cases] Migrate connector ID to references #104221

Merged
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e6dc819
Starting configure migration
jonathan-buttner Jul 1, 2021
a62c282
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Jul 1, 2021
ab586b0
Initial refactor of configuration connector id
jonathan-buttner Jul 1, 2021
6a0d9e2
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Jul 1, 2021
d3be649
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Jul 12, 2021
dbf84dd
Additional clean up and tests
jonathan-buttner Jul 13, 2021
adbb016
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Jul 14, 2021
6c00c6b
Adding some tests
jonathan-buttner Jul 14, 2021
54cd2d2
Finishing configure tests
jonathan-buttner Jul 15, 2021
aed1b30
Starting case attributes transformation refactor
jonathan-buttner Jul 16, 2021
636c586
adding more tests for the cases service
jonathan-buttner Jul 26, 2021
37ef451
Adding more functionality and tests for cases migration
jonathan-buttner Jul 27, 2021
48a449d
Finished unit tests for cases transition
jonathan-buttner Jul 28, 2021
68137ea
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Jul 29, 2021
2029b4e
Finished tests and moved types
jonathan-buttner Jul 29, 2021
3b530ba
Cleaning up type names
jonathan-buttner Jul 29, 2021
f653634
Fixing types and renaming
jonathan-buttner Jul 29, 2021
a1a01d2
Adding more tests directly for the transformations
jonathan-buttner Jul 29, 2021
3affb65
Fixing tests and renaming some functions
jonathan-buttner Jul 30, 2021
b27d7b8
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Jul 30, 2021
3be7768
Adding transformation helper tests
jonathan-buttner Jul 30, 2021
d418f91
Adding migration utility tests and some clean up
jonathan-buttner Jul 30, 2021
13d8ffd
Begining logic to remove references when it is the none connector
jonathan-buttner Jul 30, 2021
17cb102
Fixing merge reference bug
jonathan-buttner Aug 2, 2021
7689a83
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Aug 2, 2021
290ec82
Addressing feedback
jonathan-buttner Aug 2, 2021
7f7a35b
Changing test name and creating constants file
jonathan-buttner Aug 3, 2021
eb94ffd
Merge branch 'master' of github.com:elastic/kibana into migrate-conne…
jonathan-buttner Aug 3, 2021
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
28 changes: 10 additions & 18 deletions x-pack/plugins/cases/common/api/cases/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { NumberFromString } from '../saved_object';
import { UserRT } from '../user';
import { CommentResponseRt } from './comment';
import { CasesStatusResponseRt, CaseStatusRt } from './status';
import { CaseConnectorRt, ESCaseConnector } from '../connectors';
import { CaseConnectorRt } from '../connectors';
import { SubCaseResponseRt } from './sub_case';

const BucketsAggs = rt.array(
Expand Down Expand Up @@ -87,24 +87,21 @@ const CaseBasicRt = rt.type({
owner: rt.string,
});

const CaseExternalServiceBasicRt = rt.type({
connector_id: rt.string,
/**
* The external service fields. Exporting here for use in the service transformation code so I can define
* a type without the connector_id field.
*/
export const CaseExternalServiceBasicRt = rt.type({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exporting here for use in the service transformation code so I can define a type without the connector_id field.

connector_id: rt.union([rt.string, rt.null]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connector_id will be null if the reference cannot be found with deserializing from the saved object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the user cannot set the connector_id I think it's fine to have it null.

connector_name: rt.string,
external_id: rt.string,
external_title: rt.string,
external_url: rt.string,
pushed_at: rt.string,
pushed_by: UserRT,
});

const CaseFullExternalServiceRt = rt.union([
rt.intersection([
CaseExternalServiceBasicRt,
rt.type({
pushed_at: rt.string,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved these up into the basic definition above.

pushed_by: UserRT,
}),
]),
rt.null,
]);
const CaseFullExternalServiceRt = rt.union([CaseExternalServiceBasicRt, rt.null]);

export const CaseAttributesRt = rt.intersection([
CaseBasicRt,
Expand Down Expand Up @@ -326,11 +323,6 @@ export type CaseFullExternalService = rt.TypeOf<typeof CaseFullExternalServiceRt
export type CaseSettings = rt.TypeOf<typeof SettingsRt>;
export type ExternalServiceResponse = rt.TypeOf<typeof ExternalServiceResponseRt>;

export type ESCaseAttributes = Omit<CaseAttributes, 'connector'> & { connector: ESCaseConnector };
export type ESCasePatchRequest = Omit<CasePatchRequest, 'connector'> & {
connector?: ESCaseConnector;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These have been moved into the cases service because they are isolated to the service layer. Other parts of cases no longer need to reference the types. Instead they will always reference the CaseAttributes or equivalent type.

export type AllTagsFindRequest = rt.TypeOf<typeof AllTagsFindRequestRt>;
export type AllReportersFindRequest = AllTagsFindRequest;

Expand Down
6 changes: 1 addition & 5 deletions x-pack/plugins/cases/common/api/cases/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import * as rt from 'io-ts';

import { UserRT } from '../user';
import { CaseConnectorRt, ConnectorMappingsRt, ESCaseConnector } from '../connectors';
import { CaseConnectorRt, ConnectorMappingsRt } from '../connectors';

// 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')]);
Expand Down Expand Up @@ -83,8 +83,4 @@ export type CasesConfigureAttributes = rt.TypeOf<typeof CaseConfigureAttributesR
export type CasesConfigureResponse = rt.TypeOf<typeof CaseConfigureResponseRt>;
export type CasesConfigurationsResponse = rt.TypeOf<typeof CaseConfigurationsResponseRt>;

export type ESCasesConfigureAttributes = Omit<CasesConfigureAttributes, 'connector'> & {
connector: ESCaseConnector;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been moved into the configure service because it is isolated to the service layer. Other parts of the cases plugin no longer need to reference the type. Instead they will reference the CasesConfigureAttributes type directly.

export type GetConfigureFindRequest = rt.TypeOf<typeof GetConfigureFindRequestRt>;
15 changes: 2 additions & 13 deletions x-pack/plugins/cases/common/api/connectors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ const ConnectorNoneTypeFieldsRt = rt.type({
fields: rt.null,
});

export const noneConnectorId: string = 'none';

export const ConnectorTypeFieldsRt = rt.union([
ConnectorJiraTypeFieldsRt,
ConnectorNoneTypeFieldsRt,
Expand Down Expand Up @@ -102,16 +104,3 @@ export type ConnectorServiceNowSIRTypeFields = rt.TypeOf<typeof ConnectorService

// we need to change these types back and forth for storing in ES (arrays overwrite, objects merge)
export type ConnectorFields = rt.TypeOf<typeof ConnectorFieldsRt>;

export type ESConnectorFields = Array<{
key: string;
value: unknown;
}>;

export type ESCaseConnectorTypes = ConnectorTypes;
export interface ESCaseConnector {
id: string;
name: string;
type: ESCaseConnectorTypes;
fields: ESConnectorFields | null;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were moved into the service layer as well since they are only referenced by service layer functionality.

Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,11 @@ export const getPushedServiceLabelTitle = (action: CaseUserActions, firstPush: b

export const getPushInfo = (
caseServices: CaseServices,
parsedValue: { connector_id: string; connector_name: string },
// a JSON parse failure will result in null for parsedValue
parsedValue: { connector_id: string | null; connector_name: string } | null,
index: number
) =>
parsedValue != null
parsedValue != null && parsedValue.connector_id != null
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connector_id will be null if the reference cannot be found within the saved object. I did that instead of making the entire external_service field null to preserve the other fields in external_service.

? {
firstPush: caseServices[parsedValue.connector_id]?.firstPushIndex === index,
parsedConnectorId: parsedValue.connector_id,
Expand Down
15 changes: 2 additions & 13 deletions x-pack/plugins/cases/server/client/cases/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,9 @@ import {
MAX_TITLE_LENGTH,
} from '../../../common';
import { buildCaseUserActionItem } from '../../services/user_actions/helpers';
import { getConnectorFromConfiguration } from '../utils';

import { Operations } from '../../authorization';
import {
createCaseError,
flattenCaseSavedObject,
transformCaseConnectorToEsConnector,
transformNewCase,
} from '../../common';
import { createCaseError, flattenCaseSavedObject, transformNewCase } from '../../common';
import { CasesClientArgs } from '..';

/**
Expand All @@ -48,7 +42,6 @@ export const create = async (
const {
unsecuredSavedObjectsClient,
caseService,
caseConfigureService,
userActionService,
user,
logger,
Expand Down Expand Up @@ -90,10 +83,6 @@ export const create = async (
// eslint-disable-next-line @typescript-eslint/naming-convention
const { username, full_name, email } = user;
const createdDate = new Date().toISOString();
const myCaseConfigure = await caseConfigureService.find({
unsecuredSavedObjectsClient,
});
const caseConfigureConnector = getConnectorFromConfiguration(myCaseConfigure);

const newCase = await caseService.postNewCase({
unsecuredSavedObjectsClient,
Expand All @@ -103,7 +92,7 @@ export const create = async (
username,
full_name,
email,
connector: transformCaseConnectorToEsConnector(query.connector ?? caseConfigureConnector),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transformCaseConnectorToEsConnector has been moved into the service layer (postNewCase) and will occur directly before the saved object client is called to create the case.

connector: query.connector,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The connector is a required field for the creation so it will always be defined.

}),
id: savedObjectID,
});
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/cases/server/client/cases/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import { SavedObject } from 'kibana/server';
import {
CaseResponseRt,
CaseResponse,
ESCaseAttributes,
User,
UsersRt,
AllTagsFindRequest,
Expand All @@ -27,6 +26,7 @@ import {
ENABLE_CASE_CONNECTOR,
CasesByAlertId,
CasesByAlertIdRt,
CaseAttributes,
} from '../../../common';
import { countAlertsForID, createCaseError, flattenCaseSavedObject } from '../../common';
import { CasesClientArgs } from '..';
Expand Down Expand Up @@ -171,7 +171,7 @@ export const get = async (
);
}

let theCase: SavedObject<ESCaseAttributes>;
let theCase: SavedObject<CaseAttributes>;
let subCaseIds: string[] = [];
if (ENABLE_CASE_CONNECTOR) {
const [caseInfo, subCasesForCaseId] = await Promise.all([
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/cases/server/client/cases/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import {
CaseResponse,
CaseStatuses,
ExternalServiceResponse,
ESCaseAttributes,
ESCasesConfigureAttributes,
CaseType,
ENABLE_CASE_CONNECTOR,
CasesConfigureAttributes,
CaseAttributes,
} from '../../../common';
import { buildCaseUserActionItem } from '../../services/user_actions/helpers';

Expand All @@ -33,8 +33,8 @@ import { casesConnectors } from '../../connectors';
* In the future we could allow push to close all the sub cases of a collection but that's not currently supported.
*/
function shouldCloseByPush(
configureSettings: SavedObjectsFindResponse<ESCasesConfigureAttributes>,
caseInfo: SavedObject<ESCaseAttributes>
configureSettings: SavedObjectsFindResponse<CasesConfigureAttributes>,
caseInfo: SavedObject<CaseAttributes>
): boolean {
return (
configureSettings.total > 0 &&
Expand Down
49 changes: 21 additions & 28 deletions x-pack/plugins/cases/server/client/cases/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,12 @@ import {
CommentAttributes,
CommentType,
ENABLE_CASE_CONNECTOR,
ESCaseAttributes,
ESCasePatchRequest,
excess,
MAX_CONCURRENT_SEARCHES,
SUB_CASE_SAVED_OBJECT,
throwErrors,
MAX_TITLE_LENGTH,
CaseAttributes,
} from '../../../common';
import { buildCaseUserActions } from '../../services/user_actions/helpers';
import { getCaseToUpdate } from '../utils';
Expand All @@ -51,7 +50,6 @@ import {
createCaseError,
flattenCaseSavedObject,
isCommentRequestTypeAlertOrGenAlert,
transformCaseConnectorToEsConnector,
} from '../../common';
import { UpdateAlertRequest } from '../alerts/types';
import { CasesClientInternal } from '../client_internal';
Expand All @@ -62,8 +60,8 @@ import { Operations, OwnerEntity } from '../../authorization';
* Throws an error if any of the requests attempt to update a collection style cases' status field.
*/
function throwIfUpdateStatusOfCollection(
requests: ESCasePatchRequest[],
casesMap: Map<string, SavedObject<ESCaseAttributes>>
requests: CasePatchRequest[],
casesMap: Map<string, SavedObject<CaseAttributes>>
) {
const requestsUpdatingStatusOfCollection = requests.filter(
(req) =>
Expand All @@ -82,8 +80,8 @@ function throwIfUpdateStatusOfCollection(
* Throws an error if any of the requests attempt to update a collection style case to an individual one.
*/
function throwIfUpdateTypeCollectionToIndividual(
requests: ESCasePatchRequest[],
casesMap: Map<string, SavedObject<ESCaseAttributes>>
requests: CasePatchRequest[],
casesMap: Map<string, SavedObject<CaseAttributes>>
) {
const requestsUpdatingTypeCollectionToInd = requests.filter(
(req) =>
Expand All @@ -102,7 +100,7 @@ function throwIfUpdateTypeCollectionToIndividual(
/**
* Throws an error if any of the requests attempt to update the type of a case.
*/
function throwIfUpdateType(requests: ESCasePatchRequest[]) {
function throwIfUpdateType(requests: CasePatchRequest[]) {
const requestsUpdatingType = requests.filter((req) => req.type !== undefined);

if (requestsUpdatingType.length > 0) {
Expand All @@ -118,7 +116,7 @@ function throwIfUpdateType(requests: ESCasePatchRequest[]) {
/**
* Throws an error if any of the requests attempt to update the owner of a case.
*/
function throwIfUpdateOwner(requests: ESCasePatchRequest[]) {
function throwIfUpdateOwner(requests: CasePatchRequest[]) {
const requestsUpdatingOwner = requests.filter((req) => req.owner !== undefined);

if (requestsUpdatingOwner.length > 0) {
Expand All @@ -136,11 +134,11 @@ async function throwIfInvalidUpdateOfTypeWithAlerts({
caseService,
unsecuredSavedObjectsClient,
}: {
requests: ESCasePatchRequest[];
requests: CasePatchRequest[];
caseService: CasesService;
unsecuredSavedObjectsClient: SavedObjectsClientContract;
}) {
const getAlertsForID = async (caseToUpdate: ESCasePatchRequest) => {
const getAlertsForID = async (caseToUpdate: CasePatchRequest) => {
const alerts = await caseService.getAllCaseComments({
unsecuredSavedObjectsClient,
id: caseToUpdate.id,
Expand All @@ -163,7 +161,7 @@ async function throwIfInvalidUpdateOfTypeWithAlerts({
};

const requestsUpdatingTypeField = requests.filter((req) => req.type === CaseType.collection);
const getAlertsMapper = async (caseToUpdate: ESCasePatchRequest) => getAlertsForID(caseToUpdate);
const getAlertsMapper = async (caseToUpdate: CasePatchRequest) => getAlertsForID(caseToUpdate);
// Ensuring we don't too many concurrent get running.
const casesAlertTotals = await pMap(requestsUpdatingTypeField, getAlertsMapper, {
concurrency: MAX_CONCURRENT_SEARCHES,
Expand All @@ -185,7 +183,7 @@ async function throwIfInvalidUpdateOfTypeWithAlerts({
/**
* Throws an error if any of the requests updates a title and the length is over MAX_TITLE_LENGTH.
*/
function throwIfTitleIsInvalid(requests: ESCasePatchRequest[]) {
function throwIfTitleIsInvalid(requests: CasePatchRequest[]) {
const requestsInvalidTitle = requests.filter(
(req) => req.title !== undefined && req.title.length > MAX_TITLE_LENGTH
);
Expand Down Expand Up @@ -218,7 +216,7 @@ async function getAlertComments({
caseService,
unsecuredSavedObjectsClient,
}: {
casesToSync: ESCasePatchRequest[];
casesToSync: CasePatchRequest[];
caseService: CasesService;
unsecuredSavedObjectsClient: SavedObjectsClientContract;
}): Promise<SavedObjectsFindResponse<CommentAttributes>> {
Expand Down Expand Up @@ -315,9 +313,9 @@ async function updateAlerts({
unsecuredSavedObjectsClient,
casesClientInternal,
}: {
casesWithSyncSettingChangedToOn: ESCasePatchRequest[];
casesWithStatusChangedAndSynced: ESCasePatchRequest[];
casesMap: Map<string, SavedObject<ESCaseAttributes>>;
casesWithSyncSettingChangedToOn: CasePatchRequest[];
casesWithStatusChangedAndSynced: CasePatchRequest[];
casesMap: Map<string, SavedObject<CaseAttributes>>;
caseService: CasesService;
unsecuredSavedObjectsClient: SavedObjectsClientContract;
casesClientInternal: CasesClientInternal;
Expand Down Expand Up @@ -376,7 +374,7 @@ async function updateAlerts({
}

function partitionPatchRequest(
casesMap: Map<string, SavedObject<ESCaseAttributes>>,
casesMap: Map<string, SavedObject<CaseAttributes>>,
patchReqCases: CasePatchRequest[]
): {
nonExistingCases: CasePatchRequest[];
Expand Down Expand Up @@ -441,7 +439,7 @@ export const update = async (
const casesMap = myCases.saved_objects.reduce((acc, so) => {
acc.set(so.id, so);
return acc;
}, new Map<string, SavedObject<ESCaseAttributes>>());
}, new Map<string, SavedObject<CaseAttributes>>());

const { nonExistingCases, conflictedCases, casesToAuthorize } = partitionPatchRequest(
casesMap,
Expand Down Expand Up @@ -469,17 +467,12 @@ export const update = async (
);
}

const updateCases: ESCasePatchRequest[] = query.cases.map((updateCase) => {
const updateCases: CasePatchRequest[] = query.cases.map((updateCase) => {
const currentCase = myCases.saved_objects.find((c) => c.id === updateCase.id);
const { connector, ...thisCase } = updateCase;
const { id, version } = updateCase;
return currentCase != null
? getCaseToUpdate(currentCase.attributes, {
...thisCase,
...(connector != null
? { connector: transformCaseConnectorToEsConnector(connector) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need to do the transformation here because that functionality/responsibility has been moved to the service layer.

: {}),
})
: { id: thisCase.id, version: thisCase.version };
? getCaseToUpdate(currentCase.attributes, updateCase)
: { id, version };
});

const updateFilterCases = updateCases.filter((updateCase) => {
Expand Down
Loading