[App Arch] Saved object migrations from kibana plugin to new platform #59044

alexwizp · 2020-03-02T15:47:37Z

Summary

Currently most saved object migrations live in the src/legacy/core_plugins/kibana plugin. SO migrations should now be supported in the NP as of #57430.

We need to split up the migrations so each respective type is owned by a new platform plugin:

index-pattern type -> data plugin
search type -> data plugin
visualization type -> visualizations plugin
src/legacy/core_plugins/data/mappings-> np

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 support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

Part of elastic#55946

elasticmachine · 2020-03-03T10:42:40Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

alexwizp · 2020-03-03T10:44:26Z

@elasticmachine merge upstream

src/core/server/saved_objects/saved_objects_service.ts

lukeelmers

Reviewed app arch items -- code changes and structure LGTM!

Tested one of the index pattern migrations locally (chrome) & verified migration is still running as expected.

alexwizp · 2020-03-04T08:36:13Z

@elasticmachine merge upstream

pgayvallet

See comment, this is blocked by #59291. Appart from that, this looks good for now.

src/core/server/saved_objects/saved_objects_service.ts

alexwizp · 2020-03-11T08:32:49Z

ping @elastic/kibana-platform

alexwizp · 2020-03-11T09:02:16Z

@elasticmachine merge upstream

alexwizp · 2020-03-11T12:47:05Z

@elasticmachine merge upstream

alexwizp · 2020-03-11T14:47:43Z

@elasticmachine merge upstream

alexwizp · 2020-03-11T19:30:40Z

@pgayvallet could you please review that PR?

pgayvallet

Would it be possible to follow the convention in src/core/CONVENTIONS.md regarding SO types?

Saved object type definitions should be defined in their own `server/saved_objects` directory.

The folder should contain a file per type, named after the snake_case name of the type, and an `index.ts` file exporting all the types.

Appart from that, LGTM

# Conflicts: # src/plugins/data/server/server.api.md

…platform (elastic#59044) * [App Arch] Saved object migrations from kibana plugin to new platform Part of elastic#55946 * search type -> data plugin * remove migrations folder * visualization type -> visualizations plugin * src/legacy/core_plugins/data/mappings-> np * savedObjectsManagement -> management section * move code into save_objects folder Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (elastic#59044) adds new test (elastic#60064) [Uptime] Index Status API to Rest (elastic#59657)

kibanamachine · 2020-03-16T13:51:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5c4a309

History

💚 Build #33357 succeeded 5c4a309
💚 Build #32439 succeeded ff3e192
💔 Build #32387 failed d486f66
💚 Build #32084 succeeded a646fd0
💔 Build #32068 failed 2fa7cff

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

* master: (40 commits) skips 'config_open.ts' files from linter check (elastic#60248) [Searchprofiler] Spacing between rendered shards (elastic#60238) Add UiSettings validation & Kibana default route redirection (elastic#59694) [SIEM][CASE] Change configuration button (elastic#60229) [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (elastic#59044) adds new test (elastic#60064) [Uptime] Index Status API to Rest (elastic#59657) [SIEM] Adds 'Closes and opens signals' Cypress test (elastic#59950) [NP] Graph migration (elastic#59409) [ML] Clone analytics job (elastic#59791) Move VALUE_CLICK_TRIGGER and APPLY_FILTER_TRIGGER to ui_action… (elastic#60202) Handle improperly defined Watcher Logging Action text parameter. (elastic#60169) Move select range trigger to uiActions (elastic#60168) [SIEM][CASES] Configure cases: Final (elastic#59358) Closes elastic#59784. Sets xpack.apm.serviceMapEnabled default value true. (elastic#60153) [siem/cypress] update junit filename to be picked up by runbld (elastic#60156) OSS logic added to test environment (elastic#60134) Move canvas setup into app mount (elastic#59926) enable triggers_actions_ui plugin by default (elastic#60137) Expose Elasticsearch from start and deprecate from setup (elastic#59886) ...

…platform (#59044) (#60247) * [App Arch] Saved object migrations from kibana plugin to new platform Part of #55946 * search type -> data plugin * remove migrations folder * visualization type -> visualizations plugin * src/legacy/core_plugins/data/mappings-> np * savedObjectsManagement -> management section * move code into save_objects folder Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

tylersmalley · 2020-04-15T21:03:02Z

src/plugins/data/server/saved_objects/search_migrations.ts

+    return doc;
+  }
+
+  if (searchSource.index && Array.isArray(doc.references)) {


I am curious why we decided to protect against doc.references not being an array this way. Should existing references not existing prevent us from migrating the searchSource?

For reference, here is what we are migrating from:

https://github.com/elastic/kibana/blob/master/src/legacy/core_plugins/kibana/migrations/migrations.js#L38-L62

@tylersmalley not sure that your case is possible cause in version 7.0.0 we added references for all all saved object types. Code you can find in that file.

But why it was added:
Please open src/core/server/saved_objects/serialization/types.ts file and scroll to line:
export type SavedObjectUnsanitizedDoc = SavedObjectDoc & Partial<Referencable>;

As you see we add references to SavedObjectUnsanitizedDoc using Partial utility type.

This means that if I remove the check in my migration script, I will encounter the following ts error:

I am not sure if there is an actual issue here, but I wanted to also bring it up so we can avoid this in the future. When we have a type issue like the one seen here where Object is possibly 'undefined'. We shouldn't change the behavior of the function in order to resolve the type error. In this case, if someone did call migrateIndexPattern where doc.references was not an array it would result in an undesired behavior and behavior that differed from the original implementation (with migrations it's also important that we don't change the behavior). In this case, I feel the correct resolution would be so set doc.references in the method (as done in setNewReferences before calling migrateIndexPattern, or casting the type to something which does not have references as optional.

@tylersmalley I also thought about it today, and agree with you it should be fixed. I'll do it soon.

lukeelmers mentioned this pull request Mar 2, 2020

[App Arch] Saved object migrations from kibana plugin to new platform #55946

Closed

6 tasks

[App Arch] Saved object migrations from kibana plugin to new platform

869907e

Part of elastic#55946

alexwizp force-pushed the 55946-1 branch from fb8e59f to 869907e Compare March 3, 2020 10:36

alexwizp requested review from lukeelmers, lizozom and ppisljar March 3, 2020 10:41

alexwizp self-assigned this Mar 3, 2020

alexwizp added Feature:NP Migration v7.7.0 v8.0.0 Feature:Data Views Data Views code and UI - index patterns before 8.0 release_note:skip Skip the PR/issue when compiling release notes labels Mar 3, 2020

alexwizp added the Team:AppArch label Mar 3, 2020

alexwizp requested review from pgayvallet and VladLasitsa March 3, 2020 10:43

Merge branch 'master' into 55946-1

3e56323

alexwizp commented Mar 3, 2020

View reviewed changes

src/core/server/saved_objects/saved_objects_service.ts Outdated Show resolved Hide resolved

alexwizp marked this pull request as ready for review March 3, 2020 11:12

alexwizp requested review from a team as code owners March 3, 2020 11:12

alexwizp added 2 commits March 3, 2020 17:40

search type -> data plugin

1b5014a

remove migrations folder

50675dd

lukeelmers approved these changes Mar 4, 2020

View reviewed changes

Merge branch 'master' into 55946-1

9724bd0

pgayvallet reviewed Mar 4, 2020

View reviewed changes

src/core/server/saved_objects/saved_objects_service.ts Outdated Show resolved Hide resolved

visualization type -> visualizations plugin

8aa9938

alexwizp requested a review from pgayvallet March 10, 2020 14:11

Merge remote-tracking branch 'upstream/master' into 55946-1

a646fd0

elasticmachine and others added 2 commits March 11, 2020 05:02

Merge branch 'master' into 55946-1

fd7849a

Merge remote-tracking branch 'upstream/master' into 55946-1

edc8e46

Merge branch 'master' into 55946-1

d486f66

Merge branch 'master' into 55946-1

ff3e192

pgayvallet approved these changes Mar 16, 2020

View reviewed changes

alexwizp added 3 commits March 16, 2020 10:45

Merge remote-tracking branch 'upstream/master' into 55946-1

a2b8ac3

# Conflicts: # src/plugins/data/server/server.api.md

move code into save_objects folder

b02e9fe

Merge remote-tracking branch 'upstream/master' into 55946-1

5c4a309

alexwizp merged commit 4be60e5 into elastic:master Mar 16, 2020

alexwizp mentioned this pull request Mar 16, 2020

[7.x] [Step 1][App Arch] Saved object migrations from kibana plugin to new platform (#59044) #60247

Merged

alexwizp changed the title ~~[Step 1][App Arch] Saved object migrations from kibana plugin to new platform~~ [App Arch] Saved object migrations from kibana plugin to new platform Mar 16, 2020

lukeelmers mentioned this pull request Mar 17, 2020

Goodbye, legacy data plugin 👋 #60449

Merged

alexwizp mentioned this pull request Mar 26, 2020

move visualizations plugin to new platform #60403

Merged

tylersmalley reviewed Apr 15, 2020

View reviewed changes

alexwizp mentioned this pull request Apr 23, 2020

[App Arch] [Cleanup] Saved object migrations from kibana plugin to new platform #64289

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Arch] Saved object migrations from kibana plugin to new platform #59044

[App Arch] Saved object migrations from kibana plugin to new platform #59044

alexwizp commented Mar 2, 2020 •

edited

Loading

elasticmachine commented Mar 3, 2020

alexwizp commented Mar 3, 2020

lukeelmers left a comment

alexwizp commented Mar 4, 2020

pgayvallet left a comment

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

pgayvallet left a comment

kibanamachine commented Mar 16, 2020

tylersmalley Apr 15, 2020

tylersmalley Apr 15, 2020

alexwizp Apr 17, 2020

tylersmalley Apr 22, 2020

alexwizp Apr 22, 2020

[App Arch] Saved object migrations from kibana plugin to new platform #59044

[App Arch] Saved object migrations from kibana plugin to new platform #59044

Conversation

alexwizp commented Mar 2, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Mar 3, 2020

alexwizp commented Mar 3, 2020

lukeelmers left a comment

Choose a reason for hiding this comment

alexwizp commented Mar 4, 2020

pgayvallet left a comment

Choose a reason for hiding this comment

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

alexwizp commented Mar 11, 2020

pgayvallet left a comment

Choose a reason for hiding this comment

kibanamachine commented Mar 16, 2020

💚 Build Succeeded

History

tylersmalley Apr 15, 2020

Choose a reason for hiding this comment

tylersmalley Apr 15, 2020

Choose a reason for hiding this comment

alexwizp Apr 17, 2020

Choose a reason for hiding this comment

tylersmalley Apr 22, 2020

Choose a reason for hiding this comment

alexwizp Apr 22, 2020

Choose a reason for hiding this comment

alexwizp commented Mar 2, 2020 •

edited

Loading