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

[SecuritySolution][Detections] Adds SavedObject persistence to Signals Migrations #85690

Merged
merged 33 commits into from
Dec 15, 2020
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b579f54
Adds new SO type for persisting our signals migrations
rylnd Dec 9, 2020
a405c29
WIP: Migration status SO client
rylnd Dec 9, 2020
c2d14b7
Implements our SavedObjects service for signals migrations
rylnd Dec 10, 2020
4648a72
Refactoring migration code to work with saved objects
rylnd Dec 10, 2020
45f4341
Fixing integration tests first, and anything upstream breaking them
rylnd Dec 10, 2020
f5f1d28
Clean up API integration tests
rylnd Dec 10, 2020
cdf3a04
Split API tests into separate files
rylnd Dec 10, 2020
e9d1dde
Refactor: split existing migration service functionality into atomic …
rylnd Dec 10, 2020
7581900
WIP: moving logic into migrationService.create
rylnd Dec 10, 2020
a16fe2c
Splitting get_migration_status into component functions
rylnd Dec 11, 2020
20daf30
Move finalization logic into function
rylnd Dec 11, 2020
ca2a521
Adds function for migration "deletion" logic
rylnd Dec 11, 2020
e8a362c
Adds route for soft-deletion of migrations
rylnd Dec 11, 2020
047ba5d
Updating tests related to migration status
rylnd Dec 11, 2020
3cda7a6
Move old migration service tests to the relevant function tests
rylnd Dec 12, 2020
da11ee6
Add some unit tests around our reindex call
rylnd Dec 12, 2020
31b8d6c
Fix create migration route tests
rylnd Dec 12, 2020
f82485a
Updates finalize route unit tests
rylnd Dec 12, 2020
25a7842
Unit tests our finalization logic
rylnd Dec 12, 2020
518c911
Fixes finalization integration tests
rylnd Dec 12, 2020
d519c2f
Adds tests for our migration deletion endpoint
rylnd Dec 14, 2020
1ca0d24
Fixes types
rylnd Dec 14, 2020
d387f26
Prevent race condition due to template rollover during migration
rylnd Dec 14, 2020
2bb03bc
Add real usernames to migration savedObjects
rylnd Dec 14, 2020
74745b2
Index minimal migration SO fields needed for current functionality
rylnd Dec 14, 2020
822f5d3
Add additional migration info to status endpoint
rylnd Dec 14, 2020
787fd93
Finalize endpoint receives an array of migration IDs, not indices
rylnd Dec 14, 2020
082ec2c
Fix type errors in tests after we threaded through username
rylnd Dec 14, 2020
a1d2d6a
Update responsibilities of migration finalize/delete endpoints
rylnd Dec 15, 2020
677d444
Fix tests that were broken during refactoring
rylnd Dec 15, 2020
728122d
Fix type errors
rylnd Dec 15, 2020
a03d54d
Move outdated integration test
rylnd Dec 15, 2020
65b0339
Merge branch 'master' into signals_migration_so
kibanamachine Dec 15, 2020
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
2 changes: 1 addition & 1 deletion x-pack/plugins/lists/public/lists/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const importListWithValidation = async ({
),
chain((payload) => tryCatch(() => importList({ http, signal, ...payload }), toError)),
chain((response) => fromEither(validateEither(listSchema, response))),
flow(toPromise)
toPromise
);

export { importListWithValidation as importList };
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import * as t from 'io-ts';

export const deleteSignalsMigrationSchema = t.exact(
t.type({
migration_ids: t.array(t.string),
})
);

export type DeleteSignalsMigrationSchema = t.TypeOf<typeof deleteSignalsMigrationSchema>;
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@
import { FinalizeSignalsMigrationSchema } from './finalize_signals_migration_schema';

export const getFinalizeSignalsMigrationSchemaMock = (): FinalizeSignalsMigrationSchema => ({
migration_token:
'eyJkZXN0aW5hdGlvbkluZGV4IjoiZGVzdGluYXRpb25JbmRleCIsInNvdXJjZUluZGV4Ijoic291cmNlSW5kZXgiLCJ0YXNrSWQiOiJteS10YXNrLWlkIn0=',
migration_ids: ['migrationSOIdentifier'],
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,9 @@

import * as t from 'io-ts';

import { NonEmptyString } from '../types';

const migrationToken = NonEmptyString;

export const finalizeSignalsMigrationSchema = t.exact(
t.type({
migration_token: migrationToken,
migration_ids: t.array(t.string),
})
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { GetSignalsMigrationStatusSchema } from './get_signals_migration_status_schema';

export const getSignalsMigrationStatusSchemaMock = (): GetSignalsMigrationStatusSchema => ({
from: 'now-30d',
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import * as t from 'io-ts';

import { from } from '../common/schemas';

export const getMigrationStatusSchema = t.exact(
export const getSignalsMigrationStatusSchema = t.exact(
t.type({
from,
})
);

export type GetMigrationStatusSchema = t.TypeOf<typeof getMigrationStatusSchema>;
export type GetSignalsMigrationStatusSchema = t.TypeOf<typeof getSignalsMigrationStatusSchema>;
6 changes: 6 additions & 0 deletions x-pack/plugins/security_solution/common/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { fold, Either, mapLeft } from 'fp-ts/lib/Either';
import { pipe } from 'fp-ts/lib/pipeable';
import { fromEither, TaskEither } from 'fp-ts/lib/TaskEither';
import * as t from 'io-ts';
import { exactCheck } from './exact_check';
import { formatErrors } from './format_errors';
Expand Down Expand Up @@ -33,3 +34,8 @@ export const validateEither = <T extends t.Mixed, A extends unknown>(
(a) => schema.validate(a, t.getDefaultContext(schema.asDecoder())),
mapLeft((errors) => new Error(formatErrors(errors).join(',')))
);

export const validateTaskEither = <T extends t.Mixed, A extends unknown>(
schema: T,
obj: A
): TaskEither<Error, A> => fromEither(validateEither(schema, obj));
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { elasticsearchServiceMock } from 'src/core/server/mocks';
import { createMigrationIndex } from './create_migration_index';
import { createMigration } from './create_migration';

jest.mock('./create_migration_index');

describe('createMigration', () => {
let esClient: ReturnType<typeof elasticsearchServiceMock.createElasticsearchClient>;

beforeEach(() => {
esClient = elasticsearchServiceMock.createElasticsearchClient();
});

it('passes reindex options to the reindex call', async () => {
const reindexOptions = {
requests_per_second: 3,
size: 10,
slices: 2,
};
await createMigration({
esClient,
index: 'my-signals-index',
reindexOptions,
version: 12,
});

expect(esClient.reindex).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({
source: {
index: 'my-signals-index',
size: reindexOptions.size,
},
}),
requests_per_second: reindexOptions.requests_per_second,
slices: reindexOptions.slices,
})
);
});

it('returns info about the created migration', async () => {
(createMigrationIndex as jest.Mock).mockResolvedValueOnce('destinationIndex');
// @ts-expect-error minimum stub for our reindex response
esClient.reindex.mockResolvedValueOnce({ body: { task: 'reindexTaskId' } });

const migration = await createMigration({
esClient,
index: 'my-signals-index',
reindexOptions: {},
version: 12,
});

expect(migration).toEqual({
destinationIndex: 'destinationIndex',
sourceIndex: 'my-signals-index',
taskId: 'reindexTaskId',
version: 12,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@

import { ElasticsearchClient } from 'src/core/server';
import { SignalsReindexOptions } from '../../../../common/detection_engine/schemas/request/create_signals_migration_schema';
import { createSignalsMigrationIndex } from './create_signals_migration_index';
import { MigrationDetails } from './types';
import { createMigrationIndex } from './create_migration_index';

export interface CreatedMigration {
destinationIndex: string;
sourceIndex: string;
taskId: string;
version: number;
}

/**
* Migrates signals for a given concrete index. Signals are reindexed into a
Expand All @@ -19,10 +25,10 @@ import { MigrationDetails } from './types';
* @param version version of the current signals template/mappings
* @param reindexOptions object containing reindex options {@link SignalsReindexOptions}
*
* @returns identifying information representing the {@link MigrationDetails}
* @returns identifying information representing the {@link MigrationInfo}
* @throws if elasticsearch returns an error
*/
export const migrateSignals = async ({
export const createMigration = async ({
esClient,
index,
reindexOptions,
Expand All @@ -32,8 +38,8 @@ export const migrateSignals = async ({
index: string;
reindexOptions: SignalsReindexOptions;
version: number;
}): Promise<MigrationDetails> => {
const migrationIndex = await createSignalsMigrationIndex({
}): Promise<CreatedMigration> => {
const migrationIndex = await createMigrationIndex({
esClient,
index,
version,
Expand Down Expand Up @@ -67,5 +73,6 @@ export const migrateSignals = async ({
destinationIndex: migrationIndex,
sourceIndex: index,
taskId: response.body.task,
version,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,18 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { ElasticsearchClient } from 'src/core/server';
import { elasticsearchServiceMock } from 'src/core/server/mocks';
import { createSignalsMigrationIndex } from './create_signals_migration_index';
import { createMigrationIndex } from './create_migration_index';

describe('getMigrationStatus', () => {
let esClient: ElasticsearchClient;
describe('createMigrationIndex', () => {
let esClient: ReturnType<typeof elasticsearchServiceMock.createElasticsearchClient>;

beforeEach(() => {
esClient = elasticsearchServiceMock.createElasticsearchClient();
});

it('creates an index suffixed with the template version', async () => {
await createSignalsMigrationIndex({ esClient, index: 'my-signals-index', version: 4 });
await createMigrationIndex({ esClient, index: 'my-signals-index', version: 4 });

expect(esClient.indices.create).toHaveBeenCalledWith(
expect.objectContaining({ index: 'my-signals-index-r000004' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { ElasticsearchClient } from 'src/core/server';
*
* @returns the name of the created index
*/
export const createSignalsMigrationIndex = async ({
export const createMigrationIndex = async ({
esClient,
index,
version,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { savedObjectsClientMock } from 'src/core/server/mocks';
import { getSignalsMigrationSavedObjectMock } from './saved_objects_schema.mock';
import { createMigrationSavedObject } from './create_migration_saved_object';

describe('createMigrationSavedObjects', () => {
let soClient: ReturnType<typeof savedObjectsClientMock.create>;

beforeEach(() => {
soClient = savedObjectsClientMock.create();
});

it('returns the SavedObject if valid', () => {
// @ts-expect-error response mock is missing a few fields
soClient.create.mockResolvedValue(getSignalsMigrationSavedObjectMock());
const { attributes } = getSignalsMigrationSavedObjectMock();

return expect(
createMigrationSavedObject({ attributes, soClient, username: 'username' })
).resolves.toEqual(getSignalsMigrationSavedObjectMock());
});

it('rejects if response is invalid', () => {
const { attributes } = getSignalsMigrationSavedObjectMock();
// @ts-expect-error stubbing our SO creation
soClient.create.mockResolvedValue({ ...getSignalsMigrationSavedObjectMock(), id: null });

return expect(
createMigrationSavedObject({ attributes, soClient, username: 'username' })
).rejects.toThrow('Invalid value "null" supplied to "id"');
});

it('does not pass excess fields', async () => {
// @ts-expect-error response mock is missing a few fields
soClient.create.mockResolvedValue(getSignalsMigrationSavedObjectMock());
const { attributes } = getSignalsMigrationSavedObjectMock();
const attributesWithExtra = { ...attributes, extra: true };

const result = await createMigrationSavedObject({
attributes: attributesWithExtra,
soClient,
username: 'username',
});
expect(result).toEqual(getSignalsMigrationSavedObjectMock());

const [call] = soClient.create.mock.calls;
const attrs = call[1] as Record<string, unknown>;

expect(Object.keys(attrs)).not.toContain('extra');
});

it('rejects if attributes are invalid', () => {
const { attributes } = getSignalsMigrationSavedObjectMock();
// @ts-expect-error intentionally breaking the type
attributes.destinationIndex = null;

return expect(
createMigrationSavedObject({ attributes, soClient, username: 'username' })
).rejects.toThrow('Invalid value "null" supplied to "destinationIndex"');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { chain, tryCatch } from 'fp-ts/lib/TaskEither';
import { pipe } from 'fp-ts/lib/pipeable';

import { SavedObjectsClientContract } from 'src/core/server';
import { validateTaskEither } from '../../../../common/validate';
import { signalsMigrationSOClient } from './saved_objects_client';
import {
signalsMigrationSO,
SignalsMigrationSO,
signalsMigrationSOCreateAttributes,
SignalsMigrationSOCreateAttributes,
} from './saved_objects_schema';
import { getIsoDateString, toError, toPromise } from './helpers';

const generateAttributes = (username: string) => {
const now = getIsoDateString();
return { created: now, createdBy: username, updated: now, updatedBy: username };
};

export const createMigrationSavedObject = async ({
attributes,
soClient,
username,
}: {
attributes: SignalsMigrationSOCreateAttributes;
soClient: SavedObjectsClientContract;
username: string;
}): Promise<SignalsMigrationSO> => {
const client = signalsMigrationSOClient(soClient);

return pipe(
attributes,
(attrs) => validateTaskEither(signalsMigrationSOCreateAttributes, attrs),
chain((validAttrs) =>
tryCatch(() => client.create({ ...validAttrs, ...generateAttributes(username) }), toError)
),
chain((so) => validateTaskEither(signalsMigrationSO, so)),
toPromise
);
};
Loading