[SecuritySolution][Detections] Adds SavedObject persistence to Signal…

…s Migrations (elastic#85690)

* Adds new SO type for persisting our signals migrations

* WIP: Migration status SO client

Trying to forge a patterrn using io-ts to validate at runtime. I think
I've got it working but I want to refactor the pipeline out into a
reusable function(s).

* Implements our SavedObjects service for signals migrations

* Defines a simple client that delegates to the base SO client with
our SO type
* Defines a service that consumes the simpler client, adding validations
  and data transforms on top.

* Refactoring migration code to work with saved objects

As opposed to the previous ephemeral, encoded tokens, we now retrieve migration
info from saved objects.

At the API level, this means that both the create and finalize endpoints
receive a list of concrete indices. No more passing around tokens.

As both endpoints are idempotent, users can hammer them as much as they
want with the same lists of indices. Redundant creates and finalizes
will be met with inline 400 messages, and as one continues to poll the
finalize endpoint they should see more and more indices respond with
"completed: true"

* Fixing integration tests first, and anything upstream breaking them

* Clean up API integration tests

* standardize assignment of responses (with types)
* deletes migration SOs as test cleanup

* Split API tests into separate files

This was getting big and unwieldy; this splits these into one file per
endpoint.

* Refactor: split existing migration service functionality into atomic functions

This will allow us to repurpose the service to compose more
functionality and be more specifically useful, while keeping the
component logic separate.

* WIP: moving logic into migrationService.create

* Splitting get_migration_status into component functions

getMigrationStatus was really two separate aggregations, so I split them
out and we recompose them in the necessary routes.

* Move finalization logic into function

* migrationService exposes this as .finalize()
* adds an error field to our migration SO
  * We currently only have one error that we persist there, but it would
    be very time-consuming to track down that information were it not
    there.

* Adds function for migration "deletion" logic

* migrationService leverages this function
* adds new boolean to our savedObject
* deletes unused function (deleteMigrationSavedObject)

* Adds route for soft-deletion of migrations

* Updating tests related to migration status

* Adding/updating mocks/unit tests necessary to satisfy the things I
  need to test
* I mainly wanted to test that the the status endpoint filtered out the
  deleted migrations; this was accomplished with a unit test after
  fleshing out some mocks/sample data.

* Move old migration service tests to the relevant function tests

This logic was previously moved out into component functions; this moves
the tests accordingly.

* Add some unit tests around our reindex call

* Fix create migration route tests

Mocks out our migration functions, rather than stubbing ES calls
directly.

* Updates finalize route unit tests

Addresses functionality that hasn't been moved to finalizeMigration()

* Unit tests our finalization logic

Fixes a bug where we weren't accounting for soft-deleted migrations.
ALso updates our test migration SO to have a status of 'pending' as
that's a more useful default.

* Fixes finalization integration tests

These were failing due:
* a change in the migration status API response
* a bug I introduced in the finalize route

* Adds tests for our migration deletion endpoint

* unit tests
* API integration tests
* Caught/fixed bug with deleting a successful migration

* Fixes types

Removes unused code.

* Prevent race condition due to template rollover during migration

If a user has an out of date index (v1) relative to the template (v2), but the
template itself is out of date (newest is v3), then it's possible that
the template is rolled over to v3 after the v1-v2 migration has been
created but before the new index has been created.

In such a case, the new index would receive the v3 mappings but would
incorrectl be marked as v2. This shouldn't necessarily be an issue, but
it's an unnecessary state that can easily be prevented with the guard
introduced here.

* Add real usernames to migration savedObjects

In addition to the SOs themselves giving us observability into what
migration actions were performed, this gives us the additional info of
_who_ performed the action.

* Index minimal migration SO fields needed for current functionality

* Add additional migration info to status endpoint

This will allow users to finalize a migration if they've lost the
response to their POST call.

* Finalize endpoint receives an array of migration IDs, not indices

This disambiguates _which_ migrations we were finalizing if you passed
an index (which was previously: the most recent migration).

* Fix type errors in tests after we threaded through username

* Update responsibilities of migration finalize/delete endpoints

Discussions with @marshallmain lead to the following refactor:

* finalize does not delete tasks
* finalize only applies cleanup policy to a failed migration
* delete takes an array of migration ids (like finalize)
* delete hard-deletes the SavedObject of a completed (failed or
  successful) migration

This gives a bit more flexibility with the endpoints, as well as
disambiguates the semantics: it just deletes migrations!

* Fix tests that were broken during refactoring

* Fix type errors

I removed some logic here but forgot the imports :(

* Move outdated integration test

In the case of a successful migration, application of the cleanup policy
is done by the deletion endpoint. In the interest of data preservation,
we do not delete a sourceIndex unless it is explicitly deleted.

Co-authored-by: Kibana Machine <[email protected]>

x-pack/plugins/lists/public/lists/api.ts

@@ -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 };

x-pack/plugins/security_solution/common/detection_engine/schemas/request/delete_signals_migration_schema.ts

@@ -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>;

x-pack/plugins/security_solution/common/detection_engine/schemas/request/finalize_signals_migration_schema.mock.ts

@@ -7,6 +7,5 @@
 import { FinalizeSignalsMigrationSchema } from './finalize_signals_migration_schema';
 
 export const getFinalizeSignalsMigrationSchemaMock = (): FinalizeSignalsMigrationSchema => ({
-  migration_token:
-    'eyJkZXN0aW5hdGlvbkluZGV4IjoiZGVzdGluYXRpb25JbmRleCIsInNvdXJjZUluZGV4Ijoic291cmNlSW5kZXgiLCJ0YXNrSWQiOiJteS10YXNrLWlkIn0=',
+  migration_ids: ['migrationSOIdentifier'],
 });

x-pack/plugins/security_solution/common/detection_engine/schemas/request/finalize_signals_migration_schema.ts

@@ -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),
   })
 );
 

x-pack/plugins/security_solution/common/detection_engine/schemas/request/get_signals_migration_status_schema.mock.ts

@@ -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',
+});

x-pack/plugins/security_solution/common/detection_engine/schemas/request/get_migration_status_schema.ts → x-pack/plugins/security_solution/common/detection_engine/schemas/request/get_signals_migration_status_schema.ts

@@ -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>;

x-pack/plugins/security_solution/common/validate.ts

@@ -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';
@@ -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));

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration.test.ts

@@ -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,
+    });
+  });
+});

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/migrate_signals.ts → x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration.ts

@@ -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
@@ -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,
@@ -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,
@@ -67,5 +73,6 @@ export const migrateSignals = async ({
     destinationIndex: migrationIndex,
     sourceIndex: index,
     taskId: response.body.task,
+    version,
   };
 };

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_signals_migration_index.test.ts → x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration_index.test.ts

@@ -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' })

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_signals_migration_index.ts → x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration_index.ts

@@ -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,

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration_saved_object.test.ts

@@ -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"');
+  });
+});

x-pack/plugins/security_solution/server/lib/detection_engine/migrations/create_migration_saved_object.ts

@@ -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
+  );
+};