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 user actions connector ID #108272

Merged

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Aug 11, 2021

Background: #105677

Initial PR: #104221

This is a follow on PR for the work to migrate references to saved objects that are stored in fields outside of the references field for user actions. The connector ID can be stored in a few types of user actions:

Creation of a case user action

{
...
  connector: {
    id: <connector id to be migrated>,
    ...
  }
}

Updating the connector within a case

{
  id: <id to be migrated>
  ...
}

Pushing a case

{
  connector_id: <id to be migrated>
  ...
}

User actions holds a JSON encoded version of the above objects in the fields named oldValue and newValue to indicate what the original value was and what it is being changed to.

Performance Concern

The easiest solution would be to have the backend decode the user actions oldValue and newValue fields and place the connector ID back into them and re-encode the fields before they are returned to the UI. This would have avoiding needing to change any code in the UI. We had some performance concerns with doing that approach for every user action when the case detail view is requested.

Instead the approach we took was to move the ids to new_val_connector_id and old_val_connector_id. This required a number of changes in the UI to look for that field instead of using the encoded values for the id.

Notable Changes

  • Backend
    • Migration for user actions saved objects to extract the connector id and move it to the references field
    • The json encoding work was moved down into the service layer
    • The service layer handles remove the connector id field and moving it to the references field
    • When returning the user actions the connector id is placed in the new_val_connector_id and old_val_connector_id fields to correspond with the newValue and oldValue
  • Frontend
    • Refactored the places where we were accessing the connector id from the encoded value to leverage both the encoded value and the appropriate *_val_connector_id
    • Added more type checking for places where we called parseString on the encoded value

Testing

I added unit tests for the transformation and migration code in the backend and frontend where I could. I also added integration tests that run the migration on older 7.13 data.

To test this PR:

  • Create a case
  • Add/remove/update the connector
  • Push the case
  • Add comments and other attachments
  • Ensure that the user actions are all displaying the type of action taken by the user correctly

@jonathan-buttner jonathan-buttner added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Feature:Cases Cases feature auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Aug 11, 2021
@jonathan-buttner
Copy link
Contributor Author

@elasticmachine merge upstream

@jonathan-buttner
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cases 331 334 +3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
cases 412 425 +13

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cases 308.4KB 308.8KB +424.0B

Page load bundle

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

id before after diff
cases 79.6KB 80.2KB +621.0B
Unknown metric groups

API count

id before after diff
cases 454 469 +15

History

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

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Tested migration and functionality locally! LGTM! 🚀

@jonathan-buttner jonathan-buttner merged commit 10ac814 into elastic:master Sep 20, 2021
@jonathan-buttner jonathan-buttner deleted the migrate-ua-connector-id branch September 20, 2021 19:29
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Sep 20, 2021
* Making progress

* Fleshing out the extraction logic

* Finishing migration logic and starting more tests

* Finishing migration unit tests

* Making progress on services

* Finishing transform to es schema

* Finishing transform functionality and unit tests

* reverting migration data updates

* Cleaning up type errors

* fixing test error

* Working migration tests

* Refactoring retrieval of connector fields

* Refactoring connector id in and tests in frontend

* Fixing tests and finished refactoring parse string

* Fixing integration test

* Fixing integration tests

* Removing some duplicate code and updating test name

* Fixing create connector user action bug

* Addressing feedback and logging error

* Moving parsing function to common

* Fixing type errors

* Fixing type errors

* Addressing feedback

* Fixing lint errors

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

tylersmalley pushed a commit that referenced this pull request Sep 20, 2021
@tylersmalley
Copy link
Contributor

@jonathan-buttner, apologies, this PR needed to be reverted as it failed when it hit master. I have also closed the backport.

14:25:24         │1)    cases security and spaces enabled: basic
14:25:24         │       Common
14:25:24         │         import and export cases
14:25:24         │           imports a case with a connector:
14:25:24         │
14:25:24         │      Error: expected undefined to sort of equal '1cd34740-06ad-11ec-babc-0b08808e8e01'
14:25:24         │       at Assertion.assert (/dev/shm/workspace/parallel/6/kibana/node_modules/@kbn/expect/expect.js:100:11)
14:25:24         │       at Assertion.eql (/dev/shm/workspace/parallel/6/kibana/node_modules/@kbn/expect/expect.js:244:8)
14:25:24         │       at Context.<anonymous> (test/case_api_integration/security_and_spaces/tests/common/cases/import_export.ts:156:50)
14:25:24         │       at runMicrotasks (<anonymous>)
14:25:24         │       at processTicksAndRejections (internal/process/task_queues.js:95:5)
14:25:24         │       at Object.apply (/dev/shm/workspace/parallel/6/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Cases Cases feature release_note:skip Skip the PR/issue when compiling release notes reverted Team:Threat Hunting Security Solution Threat Hunting Team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cases] Migrate Connector ID Fields to References
6 participants