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

allows plugins to define validation schema for "enabled" flag #50286

Merged
merged 4 commits into from
Nov 18, 2019

Conversation

mshustov
Copy link
Contributor

@mshustov mshustov commented Nov 12, 2019

Summary

the first commit closes: #49012
validation error message gives a hint about source of the error: "[config validation of [key]]:...

the second commit closes: #49289
plugins are able to define config schema with default value for enabled flag

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

Dev docs

If you want your plugin to be disabled by default you can specify it via config:

export const config = {
  schema: schema.object({ enabled: schema.boolean({ defaultValue: true }) })
}

@mshustov mshustov added review chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. v7.6.0 labels Nov 12, 2019
@mshustov mshustov requested a review from a team as a code owner November 12, 2019 10:16
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

if (!config.has(enabledPath)) {

// if plugin hasn't got a config schema, we try to read "enabled" directly
const isEnabled =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As an alternative solution, we can always extend the plugin config with a schema for the enabled flag. Although, I'd prefer to do not touch the user schema.

@mshustov mshustov added this to Code review in kibana-core [DEPRECATED] via automation Nov 12, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@paul-tavares
Copy link
Contributor

Hi @restrry
I tried this out for the Endpoint Security plugin and it works well - thank you for working on it.
Do you know where I can find more information about config (what's available and how to use it)?

@mshustov
Copy link
Contributor Author

@paul-tavares right now you can only specify config schema. More details about config https://github.com/restrry/kibana/blob/b9a43a1788486c0d064cbe7aaff1fc7e987f7dfa/src/core/MIGRATION.md#configure-plugin
A schema is defined with @kbn/config-schema package. Docs for the package are in progress #50307
Let me know if you see something missed in the docs.

@mshustov
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@paul-tavares
Copy link
Contributor

@restrry thanks for that. I would not have thought of looking in the Migration docs because we're a brand new plugin that will be using only the new platform - but we're probably the exception. I will make a mental note to also review migration type of docs when looking for info. 😄

Re: Docs
I did not see an area that detail what configuration attributes are available from the platform side - for example: enabled. Knowing that list would help to ensure that plugins don't try to use "reserved" settings. The example does provide some other values, but not clear if those are used by the platform or if specific to the plugin.

One observation I would like to share, which perhaps is more specific to the disablement of a plugin, is the config is added to the server/index, which causes the actual sever code to be loaded (require()). Although the exported plugin function is never called, depending on what the plugin server code does that might acutely cause it to "run" (example: any type of setup outside of Plugin setup, start and stop lifecycle hooks)

@oatkiller oatkiller mentioned this pull request Nov 15, 2019
9 tasks
@mshustov
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mshustov mshustov merged commit 583500e into elastic:master Nov 18, 2019
kibana-core [DEPRECATED] automation moved this from Code review to Needs Backport Nov 18, 2019
@mshustov mshustov deleted the issue-49289-read-schema-for-enabled branch November 18, 2019 09:25
mshustov added a commit to mshustov/kibana that referenced this pull request Nov 18, 2019
…c#50286)

* validation error message gives a hint about error source

* allows plugins to define validation schema for "enabled" flag
@mshustov mshustov removed the review label Nov 18, 2019
mshustov added a commit that referenced this pull request Nov 18, 2019
#50885)

* validation error message gives a hint about error source

* allows plugins to define validation schema for "enabled" flag
jloleysens added a commit to jloleysens/kibana that referenced this pull request Nov 18, 2019
…her [skip ci]

* upstream/master: (54 commits)
  allows plugins to define validation schema for "enabled" flag (elastic#50286)
  Add retry to find.existsByDisplayedByCssSelector (elastic#48734)
  [i18n] integrate latest translations (elastic#50864)
  ui/resize_checker 👉 src/plugins/kibana_utils (elastic#44750)
  Fix @reach/router types (elastic#50863)
  [ML] Adding ML node warning to overview and analytics pages (elastic#50766)
  Bump storybook dependencies (elastic#50752)
  [APM Replace usage of idx with optional chaining (elastic#50849)
  [SIEM] Fix eslint errors (elastic#49713)
  Improve "Browser client is out of date" error message (elastic#50296)
  [SIEM][Detection Engine] REST API improvements and changes from UI/UX feedback (elastic#50797)
  Move @kbn/es-query into data plugin - es-query folder (elastic#50182)
  Index Management new platform migration (elastic#49359)
  Increase retry for cloud snapshot to finish (elastic#50781)
  Removing EuiCode from inside EuiPanel (elastic#50683)
  [SIEM] Tests for search_after and bulk index (elastic#50129)
  Make babel understand TypeScript 3.7 syntax (elastic#50772)
  Fixing mocha tests and broken password change status codes (elastic#50704)
  [Canvas] Use compressed forms in sidebar (elastic#49419)
  Add labels to shell scripts in Jenkins (elastic#49657)
  ...
@joshdover joshdover moved this from Needs Backport to Done (7.6) in kibana-core [DEPRECATED] Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported chore Feature:New Platform release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugins can declare disability be default Better reporting of config errors
4 participants