-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Encrypted Saved Objects] Adds support for migrations in ESO #69513
[Encrypted Saved Objects] Adds support for migrations in ESO #69513
Conversation
* master: (82 commits) Drilldown docs 2 (elastic#69375) [APM] Replace ML index queries with searching via mlAnomalySearch API (elastic#69099) [Ingest Manager][Endpoint] Add Endpoint Create Policy flow with Ingest (elastic#68955) [DOCS] Updates titles in Maps docs (elastic#68703) [SIEM][Timeline] Minor timeline improvement (elastic#69386) Update dependency @elastic/charts to v19.5.2 (elastic#69126) [ML] Functional tests - Reduce DFA job model memory (elastic#69295) [ML] Functional tests - add more recognize and setup module API tests (elastic#69251) feat: 🎸 don't show drilldown action in "edit" mode (elastic#69371) [SIEM][Timeline] Persist timeline to localStorage (elastic#67156) Replaces the Custom Color Picker on TSVB with the EuiColorPicker (elastic#68888) [APM] Only add decimals for numbers below 10 (elastic#69334) Explore underlying data (elastic#68496) [SIEM] Adds example unit test to convert KQL using a nested query [Component template] Details flyout (elastic#68732) [DOCS] Fixes license management links (elastic#69347) [BundleRefPlugin] resolve imports to files too (elastic#69241) [APM] Fix service maps not loading when there are no APM ML jobs (elastic#69240) [Reporting] Prepare export type definitions for Task Manager (elastic#65213) [kbn/pm] only count cached project (elastic#69113) ...
Pinging @elastic/kibana-security (Team:Security) |
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
@elasticmachine merge upstream |
* master: (63 commits) Bump jest related packages (elastic#58095) [SECURITY] Introduce kibana nav (elastic#68862) disable pageLoadMetrics job, it's gotten really flaky [Endpoint] Fix flaky endpoints list unit test (elastic#69591) skip failing suite (elastic#69595) [Security_Solution][Endpoint] Resolver leverage ancestry array for queries (elastic#69264) Fixing resolver alert generation (elastic#69587) [Endpoint] add policy empty state (elastic#69449) [APM] Add support for dark mode (elastic#69362) [ML] Data Grid Histograms (elastic#68359) Resolving conflicts (elastic#69597) [DOCS] Add related link to the ingest management docs (elastic#69467) Remove endpoint alert fields from signal mapping (elastic#68934) [ftr] add support for docker servers (elastic#68173) Merge/restyle nodes table (elastic#69098) skip tests using hostDetailsPolicyResponseActionBadge [DOCS] Adds kibana-pull attribute for release docs (elastic#69554) [SIEM][Detection Engine] Fixes 7.8 and 7.9 upgrade issue within rules where you can get the error "params invalid: [lists]: definition for this key is missing" Document authentication settings. (elastic#69284) [CCR] Fix follower indices table not updating after pausing (elastic#69228) ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few nits but otherwise Platform changes look good to me
src/core/server/saved_objects/migrations/core/migrate_raw_docs.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewing for alerting - no alerting changes here (perhaps in a previous commit)
LGTM - shape looks like it will work for migrating our alerting ESOs
ACK: Will start reviewing tomorrow morning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I have a bunch of minor nits and one concern. I proposed one option, but happy to discuss any other options as well. Let me know what you think.
x-pack/plugins/encrypted_saved_objects/server/create_migration.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/create_migration.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/create_migration.ts
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,370 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: do we need all these objects here? Can we just have config
, space
and saved-object-with-migration
or even only saved-object-with-migration
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know 😬
I just used what was dumped by esarchiver, I'll take a look 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you manage to figure that out? Not super important, mostly curiosity and desire to keep this file as free of unnecessary stuff as possible. I actually have a feeling that we just need one ESO object and the rest will be created automatically if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, oops, I misread your correction 🤦
x-pack/test/encrypted_saved_objects_api_integration/tests/encrypted_saved_objects_api.ts
Show resolved
Hide resolved
* master: (36 commits) [APM] Storybook theme fixes (elastic#69730) remove top-nav. it is not rendered in KP (elastic#69488) expose DocLinks API from start only (elastic#68745) Properly redirect legacy URLs (elastic#68284) Redirect to Logged Out UI on SAML Logout Response. Prefer Login Selector UI to Logged Out UI whenever possible. (elastic#69676) Fixes elastic#69344: Don't allow empty string for server.basePath config (elastic#69377) Migrate legacy import/export endpoints (elastic#69474) [ML] Fixes anomaly chart and validation for one week bucket span (elastic#69671) Support deep links inside of `RelayState` for SAML IdP initiated login. (elastic#69401) [Obs] Update Observability landing page text (elastic#69727) [Metrics UI] Add inventory alert preview (elastic#68909) remove scroll in drag & drop context (elastic#69710) [SECURITY] Add endpoint alerts url (elastic#69707) [Index Management] Fix API Integration Test and use of `timestamp_field` (elastic#69666) [Maps] Remove extra layer of telemetry nesting under "attributes" (elastic#66137) Add plugin API for customizing the logging configuration (elastic#68704) adds kibana navigation tests (elastic#69733) fix link to analytics results from management (elastic#69550) [ML] Transform: Enable force delete if one of the transforms failed (elastic#69472) [ML] Transform: Table enhancements (elastic#69307) ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks for making these adjustments! I finished code review, only skipped tests for create_migration
for now since, as we discussed over Slack, we want to first figure out migration behavior in case of changed encryption key (currently it seems we block/crash Kibana).
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,370 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you manage to figure that out? Not super important, mostly curiosity and desire to keep this file as free of unnecessary stuff as possible. I actually have a feeling that we just need one ESO object and the rest will be created automatically if needed.
import { EncryptedSavedObjectsMigrationService } from './crypto/encrypted_saved_objects_migration_service'; | ||
|
||
type SavedObjectOptionalMigrationFn<InputAttributes, MigratedAttributes> = ( | ||
doc: SavedObjectUnsanitizedDoc<InputAttributes> | SavedObjectUnsanitizedDoc<InputAttributes>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Soo, same suggestion here, type is redundantly duplicated ...
doc: SavedObjectUnsanitizedDoc<InputAttributes> | SavedObjectUnsanitizedDoc<InputAttributes>, | |
doc: SavedObjectUnsanitizedDoc<InputAttributes>, |
...ck/test/encrypted_saved_objects_api_integration/fixtures/api_consumer_plugin/server/index.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/create_migration.test.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/encrypted_saved_objects/server/create_migration.test.ts
Outdated
Show resolved
Hide resolved
…_saved_objects_service.ts Co-authored-by: Aleh Zasypkin <[email protected]>
…gmmorris/kibana into alerting/migrate-old-alerting-consumere * 'alerting/migrate-old-alerting-consumere' of github.com:gmmorris/kibana: Update x-pack/plugins/encrypted_saved_objects/server/crypto/encrypted_saved_objects_service.ts
Co-authored-by: Aleh Zasypkin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
…#69513) Introduces migrations into Encrypted Saved Objects. The two main changes here are: 1. The addition of a createMigration api on the EncryptedSavedObjectsPluginSetup. 2. A change in SavedObjects migration to ensure they don't block the event loop.
* master: (90 commits) [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513) [SIEM] Replace WithSource with useWithSource hook (elastic#68722) [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708) rename old siem kibana config to securitySolution (elastic#69874) Remove unused Resolver code (elastic#69914) [Observability] Fixing dynamic return type based on the appName (elastic#69894) [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419) Fixes special clicks and 3rd party icon sizes in nav (elastic#69767) [APM] Catch annotations index permission error and log warning (elastic#69881) [Endpoint][Ingest Manager] minor code cleanup (elastic#69844) [Logs UI] Logs ui context menu (elastic#69915) Index pattern serialize and de-serialize (elastic#68844) [QA] Unskip functional tests (elastic#69760) [SIEM][Detection Engine] - Update DE to work with new exceptions schema (elastic#69715) Fixes elastic#69639: Ignore url.url fields above 2048 characters (elastic#69863) PR: Provide limit warnings to user when API limits are reached. (elastic#69590) [Maps] Remove broken button (elastic#69853) Makes usage collection methods available on start (elastic#69836) [SIEM][CASE] Improve Jira's labelling (elastic#69892) [Logs UI] Access ML via the programmatic plugin API (elastic#68905) ...
…#69977) Introduces migrations into Encrypted Saved Objects. The two main changes here are: 1. The addition of a createMigration api on the EncryptedSavedObjectsPluginSetup. 2. A change in SavedObjects migration to ensure they don't block the event loop.
* master: [Encrypted Saved Objects] Adds support for migrations in ESO (elastic#69513) [SIEM] Replace WithSource with useWithSource hook (elastic#68722) [Endpoint]EMT-451: add ability to filter endpoint metadata based on presence of unenrolled events (elastic#69708) rename old siem kibana config to securitySolution (elastic#69874) Remove unused Resolver code (elastic#69914) [Observability] Fixing dynamic return type based on the appName (elastic#69894) [SECURITY SOLUTION][INGEST] Task/endpoint list tests (elastic#69419) Fixes special clicks and 3rd party icon sizes in nav (elastic#69767) [APM] Catch annotations index permission error and log warning (elastic#69881) [Endpoint][Ingest Manager] minor code cleanup (elastic#69844) [Logs UI] Logs ui context menu (elastic#69915) Index pattern serialize and de-serialize (elastic#68844)
Summary
This PR provides the ground work needed to address #68994 and #67157.
The two main changes here are:
createMigration
api on theEncryptedSavedObjectsPluginSetup
.Important Note:
This change introduces a potential breaking step in the migration step.
As we now try to migrated these Encrypted Saved Objects, a failure to decrypt these objects will result in Kibana failing to start up. In fact, this is the expected behaviour when Kibana is using an ephemeral encryption key which is automatically regenerated at startup.
This has been discussed with @legrego and @rudolf and we came to the conclusion that this is reasonable as:
Dev Docs
We now have an
createMigration
api on theEncryptedSavedObjectsPluginSetup
which facilitates defining a migration for an EncryptedSavedObject type.Defining migrations
EncryptedSavedObjects rely on standard SavedObject migrations, but due to the additional complexity introduced by the need to decrypt and reencrypt the migrated document, there are some caveats to how we support this.
The good news is, most of this complexity is abstracted away by the plugin and all you need to do is leverage our api.
The
EncryptedSavedObjects
Plugin SetupContract exposes ancreateMigration
api which facilitates defining a migration for your EncryptedSavedObject type.The
createMigration
function takes four arguments:EncryptedSavedObjectTypeRegistration
which describes the ESOType of the input (the document prior to migration). If this type isn't provided, we'll assume the input doc follows the registered type.EncryptedSavedObjectTypeRegistration
which describes the ESOType of the output (the document after migration). If this type isn't provided, we'll assume the migrated doc follows the registered type.Example: Migrating a Value
In the above example you can see thwe following:
shouldBeMigrated
we limit the migrated alerts to those whoseconsumer
field equalsalerting
or is undefined.consumer
to the value we want (alerts
orunknown
, depending on the current value). In this function we can assume that only documents with aconsumer
ofalerting
orundefined
will be passed in, but it's still safest not to, and so we use the currentconsumer
as the default when needed.As we said above, an EncryptedSavedObject migration is a normal SavedObjects migration, and so we can plug it into the underlying SavedObject just like any other kind of migration:
Example: Migating a Type
If your migration needs to change the type by, for example, removing an encrypted field, you will have to specify the legacy type for the input.
As you can see in this example how we provide a legacy type which describes the input which needs to be decrypted.
The migration function will default to using the registered type to encrypt the migrated document after the migration is applied.
If you need to migrate between two legacy types, you can specify both types at once:
Checklist
Delete any items that are not applicable to this PR.
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportThis was checked for keyboard-only and screenreader accessibilityThis renders correctly on smaller devices using a responsive layout. (You can test this in your browserThis was checked for cross-browser compatibility, including a check against IE11For maintainers