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

[uiSettings] upgrade old config on read #24108

Merged
merged 9 commits into from
Oct 20, 2018

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Oct 16, 2018

Fixes #22541
Introduced by #14164

Kibana's uiSettings are stored in a saved object with an ID based on the version number so that each Kibana version installed has its own config document. When a new version of Kibana is installed it looks for the most recent config document (from a non-snapshot/beta version) and uses it as the basis for its own uiSettings. Before #14164 the config document was being upgraded in the elasticsearch plugin health check, which meant that in nearly every case the old settings would be preserved, but since then settings were only migrated when uiSettings were written.

This PR updates the uiSettings#get() method (and therefore the API that exposes it) to attempt a config document upgrade if the config document is not found. This upgrade does the same procedure as we have been doing on write except that it does not fail if the write fails due to either user permissions or the saved object index being read only. In these situations the unwritten config upgrade is returned.

todo:
@elastic/kibana-platform now that we have migrations, do we still need to keep a different config document for each Kibana version? We're migrating all document to a new index on each upgrade right? Seems like that should supersede this version-specific-config stuff.

release note:
We've fixed a bug introduced in Kibana 6.1 that prevented "advanced config" settings like default index pattern from being migrated from older Kibana installations until the first advanced config update was made.

@spalger spalger added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:fix labels Oct 16, 2018
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the fix/upgrade-ui-settings-on-get branch from 7aae593 to b68a9bf Compare October 17, 2018 03:35
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the fix/upgrade-ui-settings-on-get branch from b68a9bf to 007dc93 Compare October 17, 2018 04:02
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger added the review label Oct 17, 2018
@spalger spalger requested review from legrego and azasypkin October 17, 2018 14:14
Copy link
Member

@legrego legrego left a comment

Choose a reason for hiding this comment

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

LGTM with optional nit. Great test coverage too ❤️
I focused mostly on the code changes, and I smoke tested locally with and w/o spaces

src/ui/ui_settings/ui_settings_service.js Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger merged commit 9904bd9 into elastic:master Oct 20, 2018
spalger pushed a commit to spalger/kibana that referenced this pull request Oct 22, 2018
* doing config migration during config get if necessary

* fixing issue with writing new config when user does not have write privilege

* [uiSettings] only log about config upgrade on success

* [uiSettings/createOrUpgradeSavedConfig] add onWriteError option

* [uiSettings] return the upgradeAttributes if reader is unable to write upgrade

* [uiSettings] update route tests to cover upgrade on get

* [spaces/integration-tests] add config doc to expected objects

* [uiSettings] avoid shadowed variable name
spalger pushed a commit that referenced this pull request Oct 23, 2018
* doing config migration during config get if necessary

* fixing issue with writing new config when user does not have write privilege

* [uiSettings] only log about config upgrade on success

* [uiSettings/createOrUpgradeSavedConfig] add onWriteError option

* [uiSettings] return the upgradeAttributes if reader is unable to write upgrade

* [uiSettings] update route tests to cover upgrade on get

* [spaces/integration-tests] add config doc to expected objects

* [uiSettings] avoid shadowed variable name
@spalger
Copy link
Contributor Author

spalger commented Oct 23, 2018

6.x/6.5: 437f1e3

@spalger spalger deleted the fix/upgrade-ui-settings-on-get branch October 23, 2018 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:fix review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default index pattern not being preserved across upgrades
4 participants