Kibana: Whitelist 'enabled' config for Platform plugins

[rudolf]

If a plugin doesn't explicitly define an enabled config schema, then disabling the plugin in kibana.yml will throw a validation error.

//kibana.yml
apm_oss.enabled: false

// throws
log   [12:21:46.564] [fatal][root] { Error: [config validation of [apm_oss].enabled]: definition for this key is missing

[elasticmachine]

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

[restrry]

to note: In LP we require a plugin to define it explicitly, fallback to the deault schema otherwise https://github.com/elastic/kibana/blob/8e9a8a84dccfa7965ce8a22362885e6cdef8b51f/src/legacy/plugin_discovery/plugin_config/schema.js#L28-L32

[joshdover]

On second thought, should enabled be allowed for every plugin? I think we may have some plugins that we don't ever want to be disabled, for example, the data plugin.

@elastic/kibana-app-arch do you have any thoughts on this? Is there any valid reason we would want to disable some of the more "core" plugins? The only thing I can think of is for testing a slimmer Kibana for some reason.

[lukeelmers]

Kibana would be pretty useless without the data plugin enabled, although data does have a few required dependencies that must remain enabled as well:

• data
  
  
  
  • uiActions
  
  
  • expressions
  
  
  • bfetch
  
  
  • inspector
  
  

The only thing I can think of is for testing a slimmer Kibana for some reason.

This is the only valid reason I can think of as well, and I believe it is an approach APM uses in testing. But I would argue that it might actually make life easier for folks in this scenario, because it would prevent them from accidentally disabling a plugin that was somewhere in data's dependency tree. Most likely they would need all of these enabled for their tests anyway.

My vote would be to stick with the legacy platform approach of requiring all plugins to have an enabled option (or just set it behind the scenes for them), and possibly make an exception for items in the list above. It feels like it'd be asking for trouble if any plugin had the ability to make themselves "mandatory".

[joshdover]

My vote would be to stick with the legacy platform approach of requiring all plugins to have an enabled option (or just set it behind the scenes for them), and possibly make an exception for items in the list above. It feels like it'd be asking for trouble if any plugin had the ability to make themselves "mandatory".

This is where I'm leaning as well, though I'd like to make it a bit less confusing than legacy and not provide a default schema at all if none is specified.

The only downside I can think of to this approach is documenting the enabled config option and making that discoverable.

If we do go with this approach, there's really no work to do here in Core itself. It is up to plugins to make sure their config schema includes enabled.

---

Alternatively, we could add the enabled key to a plugin's config schema _only if it is not already specified_ which would still allow plugins like data to specify that enabled must be true:

export const config = {
  schema: schema.object({
    enabled: schema.oneOf([true], { defaultValue: true })
  })
}

The code in Core would look something like (some of these APIs don't currently exist):

if (!plugin.config.schema.includes('enabled')) {
  plugin.config.schema.extend({
    enabled: schema.boolean({ defaultValue: true })
  });
}

[pgayvallet]

My vote would be to stick with the legacy platform approach of requiring all plugins to have an enabled option

+1 for that.

[joshdover]

Let's do this: go with what we have now (require plugins to specify this option explicitly) and later work on improving the DX here in the future (by implementing a bit more magic as proposed above).

[joshdover]

I want to note that the NP actually does mirror the legacy behavior here: if no config schema is specified at all, plugins still have an implicit <configPrefix>.enabled config that is allowed and respected by the plugin discovery process.

However this value is not exposed to plugins (not particularly useful anyways).