Skip to content

[7.x] [Security Solution][Detections] Signals Migration API (#84721)#85624

Merged
rylnd merged 1 commit intoelastic:7.xfrom
rylnd:backport/7.x/pr-84721
Dec 10, 2020
Merged

[7.x] [Security Solution][Detections] Signals Migration API (#84721)#85624
rylnd merged 1 commit intoelastic:7.xfrom
rylnd:backport/7.x/pr-84721

Conversation

@rylnd
Copy link
Copy Markdown
Contributor

@rylnd rylnd commented Dec 10, 2020

Backports the following commits to 7.x:

* WIP: basic reindexing works, lots of edge cases and TODOs to tackle

* Add note

* Add version metadata to signals documents

* WIP: Starting over from the ground up

* Removes obsolete endpoints/functions
* Adds endpoint for checking the migration status of signals indices
* Adds helper functions to represent the logical pieces of answering
  that question

* Fleshing out upgrade of signals

* triggers reindex for each index
* starts implementing followup endpoint to "finalize" after reindexing
  is finished

* Fleshing out more of the upgrade path

Still moving logic around a bunch.

* Pad the version number of our destination migration index

Instead of e.g. `.siem-signals-default-000001-r5`, this will generate
`.siem-signals-default-000001-r000005`.

This shouldn't matter much, but it may make it easier for users at a
glance to see the story of each index.

* Fleshing out more upgrade finalization

* Verifies that task matches the specified parameters
* Verifies that document counts are the same
* updates aliases
* finalization endpoint requires both source/dest indexes since we can't
  determine that from the task itself.

* Ensure that new signals are generated with an appropriate schema_version

* Apply migration cleanup policy to obsolete signals indexes

After upgrading a particular signals index, we're left with both the old
and new copies of the index. While the former is unlinked, it's still
taking up disk space; this ensures that it will eventually be deleted,
but gives users enough time to recover data if necessary.

This also ensures that, as with the normal signals ILM policy, it is
present during our normal sanity checks.

* Move more logic into component functions

* Fix type errors

* Refactor to make things a little more organized

* Moves migration-related routes under signals/ to match their routing
* Generalizes migration-agnostic helpers, moves them to appropriate
  folders (namely index/)
* Inlined getMigrationStatusInRange, a hyper-specific function with
  limited utility elsewhere

* Add some JSDoc comments around our new functions

This is as much to get my thoughts in order as it is for posterity.

Next: tests!

* Adds integration tests around migration status route

* Adds io-ts schema for route params
* Adds es_archiver data to represent an outdated signals index

* Adds API integration tests for our signals upgrade endpoint

* Adds io-ts schema for route params
* Adds second signals index archive, updates docs
* Adds test helper to wait for a given index to have documents
* Adds test helper to retrieve the relevant index name from a call to
  esArchive.load

* WIP: Fleshing out finalization tests

* Consolidate terminalogy around a migration

We're no longer making a distinction between an upgrade vs. an update
vs. a migration vs. a reindex: a migration is the concept that
encompasses this work. Both an index and individual documents can
require a migration, but both follow the same code path to migrate.

* Implement encoding of migration details

This will be a slightly better API: rather than having to pass all three
fields to finalize the migration, API users can instead send the token.

* Better transformation of errors thrown from the elasticsearch client

These often contain detailed information that we were previously
dropping. This will give better info on the migration finalization
endpoint, but should give more information across all detection_engine
endpoints in the case of an es client error.

* Finishing integration tests around finalization endpoint

This lead to a few changes in the responses from our different
endpoints; mainly, we pass both the migration token AND its constituent
parts to aid in debugging.

* Test an error case due to a reindexing failure

This would be really hard to reproduce with an integration test since
we'd need to generate a specific reindex failure. Much easier to stub
some ES calls to exercise that code in a unit test.

* Remove unnecessary version info from signals documents

We now record a single document-level version field. This represents the
version of the document's _source, which is generated by our rule
execution.

When either a mapping _or_ a transformation is added, this version will
be bumped such that new signals will contain the newest version, while
the index itself may still contain the old mappings.

The transformation pipeline will use the signal version to short-circuit
unnecessary transformations.

* Migrate an index relative to the ACTUAL template version

This handles the case where a user is attempting to migrate, but has not
yet rolled over to the newest template. Running rules may insert "new"
signals into an "old" index, but from the perspective of the app no
migration is necessary in that case.

If/when they roll over, the aforementioned index (and possibly older
ones) will be qualified as outdated, and can be migrated.

* Enrich our migration_status endpoint with an is_outdated qualification

This can be determined programatically, but for users manually
interpreting this response, the qualification will help.

* Update migration scripts

* More uniform version checking

* getIndexVersion always returns a number
* version comparisons use isOutdated

* Fix signal generation unit tests

We now generate a version field to indicate the version under which the
signal was created/migrated.

* Support reindex options to be sent to create_migration endpoint

Rather than having to perform a manual reindex, this should give API
users some control over the performance of their automated migration.

* Fix signal generation integration tests

These were failing on our new signal field.

* Add unit tests for getMigrationStatus

* Add a basic test for getSignalsIndicesInRange

Since this is ultimately just an aggregation query there's not much else
to test.

* Add unit test for the naming of our destination migration index

* Handle write indices in our migration logic

* Treat write indices as any other index in migration status endpoint
* Migration API rejects requests containing write indices
* Migration API rejects requests containing unknown/non-signals indices

* Add original hot phase to migration cleanup policy

Without this phase, ILM gets confused as it tries to move to the delete
phase and fails.

* Update old comment

The referenced field has changed.

* Delete task document as part of finalization

* Accurately report recoverable errors on create_signals_migration route

If we have a recoverable error: e.g. the destination index already
exists, or a specified index is a write index, we now report those
errors as part of the normal 200 response as these do not preclude other
specified indices from being migrated.

However, if non-signals indices are specified, we do continue to reject
the entire request, as that's indicative of misuse of the endpoint.
@rylnd rylnd added the backport This PR is a backport of another PR label Dec 10, 2020
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 47341 48121 +780

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 210.8KB 211.1KB +326.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@rylnd rylnd merged commit 0073c8e into elastic:7.x Dec 10, 2020
@rylnd rylnd deleted the backport/7.x/pr-84721 branch December 11, 2020 04:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants