elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.65k stars 8.23k forks source link

Create stable API for accessing safe Elasticsearch config values in plugins #119862

Open joshdover opened 2 years ago

joshdover commented 2 years ago

Blocked by #117496

In #117496, we added the ability for plugins to read CA fingerprints from the Elasticsearch configuration, a feature we plan to support for the long-term to allow plugins to integrate directly with the new security-on-by-default feature.

In order to minimize scope for the 8.0 release, this change was added to the existing legacy.globalConfig$() API. We should move this and any other necessary ES config values to a stable, undeprecated API directly on the Elasticsearch service.

It should still be accessible on the PluginInitializerContext interface so that it can be accessed in the same way as all other configuration values.

Scope:

elasticmachine commented 2 years ago

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

pgayvallet commented 2 years ago

It should still be accessible on the PluginInitializerContext interface so that it can be accessed in the same way as all other configuration values.

@joshdover any reason to want this exposed via the initializer context instead of an API from the elasticsearch service's setup contract?

joshdover commented 2 years ago

It should still be accessible on the PluginInitializerContext interface so that it can be accessed in the same way as all other configuration values.

This was my only reason: consistency with other config values. Though tbh, I've always felt that this constructor being separate from setup was really unnecessary and just added complexity rather than made anything simpler. Maybe we should move away from PluginInitializerContext entirely?

pgayvallet commented 2 years ago

This was my only reason: consistency with other config values.

Ok, thanks, just wanted to confirm using a core service's contract wouldn't be an issue for you.

FWIW, the config exposed via PluginInitializerContext was only meant to expose a plugin's specific config properties (and then we added this ugly legacyConfig workaround). If a plugin was to share parts of its config to some consumers, it would do so with a setup and/or start contract API. I think following the same logic here would make sense, therefor exposing this via the elasticsearch setup contract.

Identify config values in use that are in legacy & should be available on a stable API

Note that in addition to the legacyConfig we're exposing to plugins

https://github.com/elastic/kibana/blob/106183551a69399d93d1a6686640ab1b1640f249/src/core/server/plugins/types.ts#L374-L375

We're also exposing the whole config via ElasticsearchServiceSetup.legacy.config$:

https://github.com/elastic/kibana/blob/d2bd7f848793ffed6ff52717abcad8f35a64b140/src/core/server/elasticsearch/elasticsearch_service.ts#L94-L96

which is (at least) used by the console proxy:

https://github.com/elastic/kibana/blob/f6a9afea6165c6072bd0c3fdf00439b7a98de1fa/src/plugins/console/server/plugin.ts#L50

Not sure if this one should be considered as part of the scope of this issue?

joshdover commented 2 years ago

We're also exposing the whole config via ElasticsearchServiceSetup.legacy.config$:

Ah interesting, didn't realize we had that as well. FWIW our main use case for this has been eliminated and we no longer plan to need to read any ES config values from the Fleet UI side of things. I could see this changing in the future, but no plans currently.

Other than Console, I'm not sure what the requirements are for other plugins for config values. We definitely should remove legacy anything though and have a long-term option.

TinaHeiligers commented 2 years ago

I'm enabling Jira sync to get this issue on the team's roadmap. We're long past the platform migration initiative and should be removing anything legacy-related.

richkuz commented 2 years ago

Enterprise Search has a use case for fetching the external Elasticsearch URL: https://github.com/elastic/kibana/issues/77464#issuecomment-1216604754

lukeelmers commented 2 years ago

@richkuz Based on Alex's comment (https://github.com/elastic/kibana/issues/77464#issuecomment-692684190), it sounds like simply exposing the ES URL that Kibana is configured with isn't going to solve the problem for Enterprise Search since y'all need the external URL.

Would you mind opening a separate issue for that use case (exposing external ES URL to Kibana plugins?). It would be a blocker for https://github.com/elastic/kibana/issues/77464 and for the Enterprise Search use case.

richkuz commented 2 years ago

👍 Submitted https://github.com/elastic/kibana/issues/139405