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

Adds developer docs for uiSettings #124648

Merged
merged 6 commits into from
Feb 5, 2022

Conversation

TinaHeiligers
Copy link
Contributor

Resolves #123243
This PR fleshes out the uiSettings developer documentation adding sections for:

  • overview
  • client-side usage
  • server side usage
  • migrations
  • security concerns

@TinaHeiligers TinaHeiligers added enhancement New value added to drive a business result technical debt Improvement of the software architecture and operational architecture backport:skip This commit does not require backporting docs DevDocs Feature:uiSettings labels Feb 4, 2022
@TinaHeiligers TinaHeiligers marked this pull request as ready for review February 4, 2022 01:38
@TinaHeiligers TinaHeiligers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 4, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@TinaHeiligers TinaHeiligers added release_note:skip Skip the PR/issue when compiling release notes v8.1.0 v8.2.0 labels Feb 4, 2022
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Thank you @TinaHeiligers for writing this up! I love the thorough explanation of this service.

I added a few comments as I was using this document as learning material for myself 😇

Also, do you think it's worth asking the Docs team to review this?

=== Server side usage
=== Overview

UI settings are configurable from the Advanced Settings page in Management and control the behavior of {kib}. `uiSettings` are stored in a `config` saved object and, as such, conform to the same conditions as other saved objects.
Copy link
Member

Choose a reason for hiding this comment

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

conform to the same conditions as other saved objects

Can we highlight the limitations/conditions that may affect this use case of SOs? Is it space-scoped? Migrations? Anything else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've linked the text to the saved objects service.

@@ -38,3 +92,66 @@ export class MyPlugin implements Plugin {
}

----

Registeration only requires a schema for validation on read and write, all {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md[other parameters] are optional.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Registeration only requires a schema for validation on read and write, all {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md[other parameters] are optional.
Registration only requires a schema for validation on read and write, all {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md[other parameters] are optional.


Registeration only requires a schema for validation on read and write, all {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsparams.md[other parameters] are optional.

See:
Copy link
Member

Choose a reason for hiding this comment

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

I read somewhere that we use Refer instead of See for accessibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better yet, I added links to the API docs directly from the relevant text 😉


There are several ways to configure an advanced setting:

- Through the <<advanced-options, Advanced Settings UI>> (security restrictions apply)
Copy link
Member

Choose a reason for hiding this comment

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

nit: Can we add a link to the section where we explain the restrictions?

- {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsservicesetup.register.md[UI settings service Setup API docs]

==== Migrations
Because `uiSettings` are persisted in the `config` saved object, changing or removing a setting requires a migration of the whole saved object. For example, if we wanted to remove the `custom` setting in the example above and then also rename `my_setting:fourtyTwo` to `my_other_setting:fourtyTwo`, we'd have two migration entries, one for each change targeting the required versions:
Copy link
Member

Choose a reason for hiding this comment

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

As I'm reading this I'm wondering: do we have a deprecation policy/service for the UI Settings?

I'm thinking about the use case when a UI setting is globally overridden in kibana.yml, and then it's removed/renamed. Would it crash Kibana? How do we notify the user about the new name?

Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Feb 4, 2022

Choose a reason for hiding this comment

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

Plugins can leverage the optional deprecation parameter on registration for handling deprecation notices and renames etc. The deprecation warnings are rendered in Advanced Settings and should also be mirrored in the Configure Kibana guide (as we do in the docs for 7.17).

Renames and removals are all handled through config migrations, which take care of settings that have been changed in Advanced settings. I added examples of how to do both: remove a setting and rename a setting. These "sub-migrations" are all in core right now. There is an open issue to discuss potentially exposing a deprecation API that will allow settings-owners to declare their migration(s) on registration.

Overrides are validated at start-up and Kibana will throw a configuration error if there's an issue. I tried to explain that in this sentence:

If an override is misconfigured, it will fail config validation and will prevent Kibana from starting up.

However, at the moment, unrecognized settings don't cause issues. We still need to handle that. This is a good and a bad thing though, if someone has an override for a deprecated setting, it won't prevent Kibana from starting up 😉

Copy link
Member

Choose a reason for hiding this comment

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

@TinaHeiligers On this topic, maybe we should explicitly call out that 3rd party plugins cannot currently register migrations for their advanced settings? (i.e. at the moment, if a 3rd party plugin registers an advanced setting, it's essentially permanent and cannot be fixed without a manual migration, as we can't merge their migrations into core)

Copy link
Member

Choose a reason for hiding this comment

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

One other idea 🙂 maybe we can link to the docs on registering migrations from here.

Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Feb 4, 2022

Choose a reason for hiding this comment

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

Thank you both! I've added a tip about the deprecations parameter and updating the docs.

=== Server side usage
=== Overview

UI settings are configurable from the Advanced Settings page in Management and control the behavior of {kib}. `uiSettings` are stored in a `config` saved object and, as such, conform to the same conditions as other saved objects.
Copy link
Member

Choose a reason for hiding this comment

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

nits/questions:

  • Should we link to the saved objects docs from here?
  • Do we capitalize Saved Objects in our docs? I'm never sure the right thing to do here, and I see a mix of both in the docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it depends on the context? Here I'm referring to Saved Objects as entities (proper nouns?), so I guess we should capitalize here too. 🤷‍♀️ . Lets go with that and see.

Comment on lines 13 to 15
- Locked via kibana.yml's {kib-repo}blob/{branch}/src/core/server/ui_settings/ui_settings_defaults_client.ts[uiSettings.overrides.<key>]
- Through the client-side `uiSettings` Service
- Through the server-side `uiSettings` Service
Copy link
Member

Choose a reason for hiding this comment

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

nit: In terms of structure, I'd consider treating these bullets as a sort of table of contents for the rest of the page, and make each entry an anchor link to a dedicated section on that topic. (You already have dedicated sections for client/server side, so it would mean making the overrides a dedicated section).

IMO a structure like this would help with the overall flow. Maybe something along the lines of:


Overview

UI settings are configurable from the Advanced Settings page in Management and control the behavior of Kibana. uiSettings are stored in a config saved object and, as such, conform to the same conditions as other saved objects.

There are several ways to configure an advanced setting:

Keep in mind that once you add a new advanced setting, you cannot change or remove it without registering a migration in core.

Configuration with Advanced Settings UI

The uiSettings service is the programmatic interface to Kibana's Advanced Settings UI. Kibana plugins use the service to extend Kibana UI Settings Management with custom settings for their plugin.

Configuration through the Advanced settings UI is restricted to users authorised to access the Advanced Settings page. Users who don’t have permissions to change these values default to using the settings configured to the space they are in. Config saved objects can be shared between spaces.

Configuration with UI settings overrides

When a setting is configured as an override in kibana.yml, it will override any other value stored in the config saved object. If an override is misconfigured, it will fail config validation and will prevent Kibana from starting up. The override applies to Kibana as a whole for all spaces and users and the option will be disabled in the Advanced Settings page. We refer to these as "global" overrides.

Configuration on the client side

blahblah

Configuration on the server side

blahblahblah

Migrations

blahblahblah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

----
{kib-repo}blob/{branch}/src/core/server/ui_settings/saved_objects/migrations.ts[`uiSettings` migrations] are declared directly in the `uiSettings` service.

==== Security-related consideration
Copy link
Member

Choose a reason for hiding this comment

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

I'd consider merging this with a dedicated section on advanced settings ui (see my example above)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsservicesetup.register.md[UI settings service Setup API docs]

==== Migrations
Because `uiSettings` are persisted in the `config` saved object, changing or removing a setting requires a migration of the whole saved object. For example, if we wanted to remove the `custom` setting in the example above and then also rename `my_setting:fourtyTwo` to `my_other_setting:fourtyTwo`, we'd have two migration entries, one for each change targeting the required versions:
Copy link
Member

Choose a reason for hiding this comment

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

@TinaHeiligers On this topic, maybe we should explicitly call out that 3rd party plugins cannot currently register migrations for their advanced settings? (i.e. at the moment, if a 3rd party plugin registers an advanced setting, it's essentially permanent and cannot be fixed without a manual migration, as we can't merge their migrations into core)


The program interface to <<advanced-options, UI settings>>.
It makes it possible for Kibana plugins to extend Kibana UI Settings Management with custom settings.
The `uiSettings` service is the programming interface to {kib} <<advanced-options, Advanced Settings UI>>. {kib} plugins use the service to extend Kibana UI Settings Management with custom settings for their plugin.
Copy link
Member

Choose a reason for hiding this comment

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

nit: programmatic interface? could probably go either way


On the client, the `uiSettings` service is exposed directly from `core`.

Users can adjust the settings via Management Advanced Settings UI. The client-side {kib-repo}blob/{branch}/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.md[`uiSettings` client] provides plugins access to the `config` entries stored in elasticsearch.
Copy link
Member

Choose a reason for hiding this comment

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

stored in {es}.


Users can adjust the settings via Management Advanced Settings UI. The client-side {kib-repo}blob/{branch}/docs/development/core/public/kibana-plugin-core-public.iuisettingsclient.md[`uiSettings` client] provides plugins access to the `config` entries stored in elasticsearch.

In the interest of performance, `uiSettings` are cached. Any changes that require cache refreshes (or for whatever other reason) should register an instruction to reload the page when settings are configured in Advanced Settings using the `requiresPageReload` parameter.
Copy link
Member

Choose a reason for hiding this comment

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

nit: could probably drop the (or for whatever other reason) parenthetical

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😆 good catch! That was a "bloop" from the rough draft!

- {kib-repo}blob/{branch}/docs/development/core/server/kibana-plugin-core-server.uisettingsservicesetup.register.md[UI settings service Setup API docs]

==== Migrations
Because `uiSettings` are persisted in the `config` saved object, changing or removing a setting requires a migration of the whole saved object. For example, if we wanted to remove the `custom` setting in the example above and then also rename `my_setting:fourtyTwo` to `my_other_setting:fourtyTwo`, we'd have two migration entries, one for each change targeting the required versions:
Copy link
Member

Choose a reason for hiding this comment

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

One other idea 🙂 maybe we can link to the docs on registering migrations from here.

@KOTungseth
Copy link
Contributor

@TinaHeiligers thank you for opening this PR! Is the plan to also update the Dev docs in the new docs system?

@TinaHeiligers
Copy link
Contributor Author

TinaHeiligers commented Feb 4, 2022

@TinaHeiligers thank you for opening this PR! Is the plan to also update the Dev docs in the new docs system?

@KOTungseth I went hunting for these in the new docs system but couldn't find them 😱 . If you have a link, please send it my way, otherwise, we'll need new docs for those.

If you have a chance, I could use a hand with the language here too. I'm doing a second pass right now.


[IMPORTANT]
==============================================
Migrations for 3rd party plugin advanced settings are not currently supported. If a 3rd party plugin registers an advanced setting, the setting is essentially permanent and cannot be fixed without manual intervention.
Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Feb 4, 2022

Choose a reason for hiding this comment

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

Explicitly point out that migrations for 3rd party plugin advanced settings aren't supported right now. cc @lukeelmers

Copy link
Member

Choose a reason for hiding this comment

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

👍 LGTM. Could consider linking to the GH issue, but I don't think it's critical.

----

[TIP]
==============================================
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This tip mentions how we warn users of future changes.


UI settings are configurable from the Advanced Settings page in Management and control the behavior of {kib}. uiSettings are stored in a config saved object and, as such, conform to the same conditions as other <<saved-objects-service, Saved Objects>>.

There are several ways to configure an advanced setting:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've converted the bullet points into a sub-index for guiding readers through the contents on this page.

Configuration through the Advanced Settings UI is restricted to users authorised to acces the Advanced Settings page. Users who don't have permissions to change these values default to using the settings configured to the space they are in. Config saved objects can be shared between spaces.

[[uisettings-overrides]]
=== Configuration with UI settings overrides
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was in two minds about explicitly mentioning overrides. We hadn't documented these in the past on purpose to limit their use. Folks generally discovered these by going through the code and if they had and something went wrong, we have to dig through and figure out where the issue is.

Explicitly documenting overrides here gives us the opportunity to warn about misconfiguration issues, limited validation, and how setting an override affects Kibana's behavior.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we add an experimental[] flag to this as a middle ground?

When a setting is configured as an override in kibana.yml, it will override any other value stored in the config saved object. If an override is misconfigured, it will fail config validation and prevent Kibana from starting up. The override applies to Kibana as a whole for all spaces and users and the option will be disabled in the Advanced Settings page. We refer to these as "global" overrides.

[[client-side-usage]]
=== Client side usage
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't currently mention anything about the client-side service and how it's used. Now at least we do!

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

🚀 Thanks for adding these!

Configuration through the Advanced Settings UI is restricted to users authorised to acces the Advanced Settings page. Users who don't have permissions to change these values default to using the settings configured to the space they are in. Config saved objects can be shared between spaces.

[[uisettings-overrides]]
=== Configuration with UI settings overrides
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we add an experimental[] flag to this as a middle ground?


[IMPORTANT]
==============================================
Migrations for 3rd party plugin advanced settings are not currently supported. If a 3rd party plugin registers an advanced setting, the setting is essentially permanent and cannot be fixed without manual intervention.
Copy link
Member

Choose a reason for hiding this comment

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

👍 LGTM. Could consider linking to the GH issue, but I don't think it's critical.


[TIP]
==============================================
Plugins can leverage the optional deprecation parameter on registration for handling deprecation notices and renames. The deprecation warnings are rendered in the Advanced Settings UI and should also be added to the Configure Kibana guide.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we link to Configure Kibana here?

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

History

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

@TinaHeiligers TinaHeiligers merged commit 483483e into elastic:main Feb 5, 2022
@TinaHeiligers TinaHeiligers deleted the uiSettings-dev-docs branch February 5, 2022 00:30
@TinaHeiligers TinaHeiligers added v8.0.0 v8.0.1 auto-backport Deprecated - use backport:version if exact versions are needed and removed backport:skip This commit does not require backporting labels Feb 14, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 14, 2022
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Feb 14, 2022
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.0
8.1

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Feb 14, 2022
(cherry picked from commit 483483e)

Co-authored-by: Christiane (Tina) Heiligers <[email protected]>
kibanamachine added a commit that referenced this pull request Feb 14, 2022
(cherry picked from commit 483483e)

Co-authored-by: Christiane (Tina) Heiligers <[email protected]>
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 DevDocs docs enhancement New value added to drive a business result Feature:uiSettings release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc technical debt Improvement of the software architecture and operational architecture v8.0.0 v8.0.1 v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add dev docs for uiSettings
7 participants