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

[Security Solution] [Platform] Migrate pre-8.0 action connector ids in rule params on import to post-8.0 _id #120975

Merged
merged 23 commits into from
Jan 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6f29df4
WIP
dhurley14 Dec 8, 2021
ee3e7d1
WIP
dhurley14 Dec 8, 2021
fa8b09f
working with actions client resolve
dhurley14 Dec 8, 2021
4b43eb5
use elasticsearch instead of saved objects client resolve api
dhurley14 Dec 8, 2021
8c40aad
cleanup
dhurley14 Dec 9, 2021
45e40bd
more cleanup
dhurley14 Dec 9, 2021
9adc3db
updates comments and a little bit of cleanup
dhurley14 Dec 9, 2021
14f2d95
undo change
dhurley14 Dec 9, 2021
a8ac8be
cleanup
dhurley14 Dec 9, 2021
f21d717
merge main into branch
dhurley14 Dec 9, 2021
67836fa
fix typecheck failure
dhurley14 Dec 10, 2021
83b7d70
some error handling and added jest tests for swapActionIds and migrat…
dhurley14 Dec 15, 2021
33dffbb
only find origin id for action saved objects
dhurley14 Dec 15, 2021
19b1862
Merge branch 'main' into migrate-imported-actions
kibanamachine Dec 15, 2021
50a2b40
replacd es client with scoped SO client including hidden objects
dhurley14 Dec 16, 2021
9e0be05
fix jest tests to use mocked SO client
dhurley14 Dec 17, 2021
af7de37
Merge remote-tracking branch 'upstream/main' into migrate-imported-ac…
dhurley14 Dec 28, 2021
2ff74da
add more integration tests
dhurley14 Dec 29, 2021
ced8410
fixes comments
dhurley14 Dec 29, 2021
0262eb4
updates from PR comments
dhurley14 Jan 4, 2022
e80a271
Merge remote-tracking branch 'upstream/main' into migrate-imported-ac…
dhurley14 Jan 4, 2022
8ee2dc4
updates expected error message in jest test
dhurley14 Jan 4, 2022
a90ec5a
nest pMaps - pMap over rule.actions in case the number of promises is…
dhurley14 Jan 4, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export const action = t.exact(
})
);

export type Action = t.TypeOf<typeof action>;

export const actions = t.array(action);
export type Actions = t.TypeOf<typeof actions>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ import {
buildSiemResponse,
} from '../utils';

import { getTupleDuplicateErrorsAndUniqueRules, getInvalidConnectors } from './utils';
import {
getTupleDuplicateErrorsAndUniqueRules,
getInvalidConnectors,
migrateLegacyActionsIds,
} from './utils';
import { createRulesAndExceptionsStreamFromNdJson } from '../../rules/create_rules_stream_from_ndjson';
import { buildRouteValidation } from '../../../../utils/build_validation/route_validation';
import { HapiReadableStream } from '../../rules/types';
Expand Down Expand Up @@ -74,6 +78,9 @@ export const importRulesRoute = (
const rulesClient = context.alerting.getRulesClient();
const actionsClient = context.actions.getActionsClient();
const esClient = context.core.elasticsearch.client;
const actionSOClient = context.core.savedObjects.getClient({
includedHiddenTypes: ['action'],
});
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.securitySolution.getAppClient();
const exceptionsClient = context.lists?.getExceptionListClient();
Expand Down Expand Up @@ -127,8 +134,13 @@ export const importRulesRoute = (
const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] =
getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite);

const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors(
const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds(
Copy link
Contributor

Choose a reason for hiding this comment

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

Action here in migrateLegacyActionIds refers to connectors right? Wondering if we should have the same language between migrateLegacyActionIds and getInvalidConnectors. Or is one dealing with the rule action objects and he other dealing with the connectors?

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 is a similar issue to when rules were known as "alerts" because they were alerting SO's. The security solution calls SO actions as actions but they are known to kibana alerting as action connectors. My thinking is to follow the same guidance we used between "rules" and "alerts" back when they meant the same thing ('alert' SO types) where we called them rules in the security solution. I believe since the precedent is for connectors to be called 'actions' in the security solution we should continue to use that terminology in the code.

parsedObjectsWithoutDuplicateErrors,
actionSOClient
);

const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors(
migratedParsedObjectsWithoutDuplicateErrors,
actionsClient
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { Readable } from 'stream';
import { createPromiseFromStreams } from '@kbn/utils';
import { Action, ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types';

import {
transformAlertToRule,
Expand All @@ -19,6 +20,8 @@ import {
getDuplicates,
getTupleDuplicateErrorsAndUniqueRules,
getInvalidConnectors,
swapActionIds,
migrateLegacyActionsIds,
} from './utils';
import { getAlertMock } from '../__mocks__/request_responses';
import { INTERNAL_IDENTIFIER } from '../../../../../common/constants';
Expand All @@ -30,7 +33,6 @@ import { createRulesAndExceptionsStreamFromNdJson } from '../../rules/create_rul
import { RuleAlertType } from '../../rules/types';
import { ImportRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/import_rules_schema';
import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock';
import { ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types';
import { CreateRulesBulkSchema } from '../../../../../common/detection_engine/schemas/request';
import {
getMlRuleParams,
Expand Down Expand Up @@ -652,6 +654,208 @@ describe.each([
});
});

describe('swapActionIds', () => {
const mockAction: Action = {
group: 'group string',
id: 'some-7.x-id',
action_type_id: '.slack',
params: {},
};
const soClient = clients.core.savedObjects.getClient();
beforeEach(() => {
soClient.find.mockReset();
soClient.find.mockClear();
});

test('returns original action if Elasticsearch query fails', async () => {
clients.core.savedObjects
.getClient()
.find.mockRejectedValueOnce(new Error('failed to query'));
const result = await swapActionIds(mockAction, soClient);
expect(result).toEqual(mockAction);
});

test('returns original action if Elasticsearch query returns no hits', async () => {
soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [],
}));
const result = await swapActionIds(mockAction, soClient);
expect(result).toEqual(mockAction);
});

test('returns error if conflicting action connectors are found -> two hits found with same originId', async () => {
soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'fake id 1', type: 'action', attributes: {}, references: [] },
{ score: 0, id: 'fake id 2', type: 'action', attributes: {}, references: [] },
],
}));
const result = await swapActionIds(mockAction, soClient);
expect(result instanceof Error).toBeTruthy();
expect((result as unknown as Error).message).toEqual(
'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details'
);
});

test('returns action with new migrated _id if a single hit is found when querying by action connector originId', async () => {
soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] },
],
}));
const result = await swapActionIds(mockAction, soClient);
expect(result).toEqual({ ...mockAction, id: 'new-post-8.0-id' });
});
});

describe('migrateLegacyActionsIds', () => {
const mockAction: Action = {
group: 'group string',
id: 'some-7.x-id',
action_type_id: '.slack',
params: {},
};
const soClient = clients.core.savedObjects.getClient();
beforeEach(() => {
soClient.find.mockReset();
soClient.find.mockClear();
});
test('returns import rules schemas + migrated action', async () => {
const rule: ReturnType<typeof getCreateRulesSchemaMock> = {
...getCreateRulesSchemaMock('rule-1'),
actions: [mockAction],
};
soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] },
],
}));

const res = await migrateLegacyActionsIds(
// @ts-expect-error
[rule],
soClient
);
expect(res).toEqual([{ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }]);
});

test('returns import rules schemas + multiple migrated action', async () => {
const rule: ReturnType<typeof getCreateRulesSchemaMock> = {
...getCreateRulesSchemaMock('rule-1'),
actions: [mockAction, { ...mockAction, id: 'different-id' }],
};
soClient.find.mockImplementation(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] },
],
}));

const res = await migrateLegacyActionsIds(
// @ts-expect-error
[rule],
soClient
);
expect(res).toEqual([
{
...rule,
actions: [
{ ...mockAction, id: 'new-post-8.0-id' },
{ ...mockAction, id: 'new-post-8.0-id' },
],
},
]);
});

test('returns import rules schemas + migrated action resulting in error', async () => {
const rule: ReturnType<typeof getCreateRulesSchemaMock> = {
...getCreateRulesSchemaMock('rule-1'),
actions: [mockAction],
};
soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] },
{ score: 0, id: 'new-post-8.0-id-2', type: 'action', attributes: {}, references: [] },
],
}));

const res = await migrateLegacyActionsIds(
// @ts-expect-error
[rule],
soClient
);
expect(res[0] instanceof Error).toBeTruthy();
expect((res[0] as unknown as Error).message).toEqual(
JSON.stringify({
rule_id: 'rule-1',
error: {
status_code: 409,
message:
'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details',
},
})
);
});
test('returns import multiple rules schemas + migrated action, one success and one error', async () => {
const rule: ReturnType<typeof getCreateRulesSchemaMock> = {
...getCreateRulesSchemaMock('rule-1'),
actions: [mockAction],
};

soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] },
],
}));
soClient.find.mockImplementationOnce(async () => ({
total: 0,
per_page: 0,
page: 1,
saved_objects: [
{ score: 0, id: 'new-post-8.0-id', type: 'action', attributes: {}, references: [] },
{ score: 0, id: 'new-post-8.0-id-2', type: 'action', attributes: {}, references: [] },
],
}));

const res = await migrateLegacyActionsIds(
// @ts-expect-error
[rule, rule],
soClient
);
expect(res[0]).toEqual({ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] });
expect(res[1] instanceof Error).toBeTruthy();
expect((res[1] as unknown as Error).message).toEqual(
JSON.stringify({
rule_id: 'rule-1',
error: {
status_code: 409,
message:
'Found two action connectors with originId or _id: some-7.x-id The upload cannot be completed unless the _id or the originId of the action connector is changed. See https://www.elastic.co/guide/en/kibana/current/sharing-saved-objects.html for more details',
},
})
);
});
});
describe('getInvalidConnectors', () => {
beforeEach(() => {
clients.actionsClient.getAll.mockReset();
Expand Down
Loading