Add config value support in frontend to New Platform

[joshdover]

Currently in the New Platform, we allow frontend plugins to read injected vars from legacy plugins, but there is no mechanism for adding injected vars from the server side.

Because the New Platform frontend is a single-page application, I believe we no longer have a strong case for adding the injected var mechanism and instead should allow plugins to whitelist configuration values that should be available in the frontend.

Reasons not to add injected vars to the new platform:

• Injected vars can be calculated based on data that changes.
  • In the New Platform, it's entirely possible for a user to have a single browser session that lasts months or days. This creates a problem if the underlying data for an injected var changed but the user has not refreshed the browser. If we only expose config values that can only change when Kibana is restarted, we can easily detect that the config changed (use a hash of the config) and require the user to refresh their browser.
• We have implicit dependencies between plugins and the injected vars they expose and use in the legacy platform.
  • This creates problems where plugin A uses injected vars from plugin B but may not depend explicitly on plugin B. If plugin B is disabled, plugin A breaks.

Reasons we may still want injected vars:

• There are values the frontend needs that depend on sensitive data that should not be exposed to the browser.
  • Without injected vars, plugins would need to make HTTP requests to the backend on startup, which could slow down the boot time of the UI. I don't think this is a huge problem because the UI will be bootstrapping much less frequently in a SPA.

Implementation Proposal

I believe we could add a new exposeToBrowser: boolean field to the TypeOptions interface passed to @kbn/config-schema types.

Core could then use this to expose any fields marked as true to the browser-side plugin.

[elasticmachine]

Pinging @elastic/kibana-platform

[joshdover]

cc @legrego @elastic/kibana-app-arch

[legrego]

I believe we no longer have a strong case for adding the injected var mechanism and instead should allow plugins to whitelist configuration values that should be available in the frontend.

Without injected vars, plugins would need to make HTTP requests to the backend on startup, which could slow down the boot time of the UI. I don't this is a huge problem because the UI will be bootstrapping much less in a SPA.

Given that the NP will not have as many full bootstrap events, I tend to agree with your assessment.
I wouldn't want to see the bootstrap get to the point where Kibana requires a dozen or more individual API requests just so plugins have enough information to complete their setup/start phases though (especially since some plugins won't be able to start until their dependencies finish). If it turns into a noticeable performance issue, then we can always come up with a solution at that point...it's likely not worth engineering for that scenario just yet.

[legrego]

Tying together a couple of different conversations here:

• The spaces plugin needs client-side access to the server.basePath setting which is specified via kibana.yml. Note that this is different from the base path that the http service exposes today, because that one is a space-aware path, but server.basePath is not space aware. (Spaces - Client NP Migration, Phase 1 #40856 (comment))
• The security plugin needs client-side access to its xpack.security.sessionTimeout setting which is specified via kibana.yml in order to effectively warn about session expirations (NP Security HTTP Interceptors #39477).

Given these two needs, and potentially others, I think Josh's proposal of whitelisting configuration values makes sense here.

[mshustov]

Problems to discuss:

• Data consistency. During migration, we have NP and LP config services. They might have different values due to inconsistency in validation rules. Having a warning in docs is enough?
• Rehydration. Config might have fields with not serializable data structures - ByteSize, Duration and we need to rehydrate them on the client. Can be solved via serialization/de-serialization wrapper by request.
• Declaration format

I believe we could add a new exposeToBrowser: boolean field to the TypeOptions interface passed to @kbn/config-schema types.

@kbn/config-schema is used not only for config, but for other runtime validations: request params/query/body, action secrets, etc.
it knows nothing about an environment it's run.
From a technical point of view, it's not right to tight a validation library to a specific environment.
As an alternative, each plugin may declare fields to expose to a browser as a part of config declaration

export const config = {
  exposeToBrowser: ['key1'], 
  schema: ...
}

pros:
Consistent with the config schema declaration.
An audit is not bound to runtime, can be written as a separate CLI tool.
On Kibana start validates that only known fields are declared.
cons:
Requires manual sync with a schema.
API maybe not obvious.

• Runtime updates
  Config service on the server-side provides a config as a stream of values. Even though we support only logging config updates ATM, we may have to support this functionality later on the client-side as well. For now, we can provide only a static value to simplify consumers logic.

• Server & Client API consistency.
  We pass a config as a part of PluginInitializerContext on the server and should do so on the client as well.

[legrego]

w/r/t Declaration format, I agree we shouldn't couple it to @kbn/config-schema, and I think your exposeToBrowser approach would work well. I think it's located close enough to the schema to be easy enough to maintain, and having the validation to only specify known keys will help with that as well.

I started writing an alternative proposal, but then I realized that @joshdover essentially stated the same in the issue description above:

I believe we could add a new exposeToBrowser: boolean field to the TypeOptions interface passed to @kbn/config-schema types.

We could consider the TypeOptions approach while still decoupling the schema from this enhancement by introducing a generic metadata property to TypeOptions, which can hold arbitrary information. The service which supplies config to the client could look for a specific entry in that field to determine which properties to send to the UI.

One downside to this approach is that the metadata field won't make sense everywhere. Consider the following:

schema.object({
  cookieName: schema.string({ defaultValue: 'sid' }),
  sessionTimeout: schema.oneOf([
    schema.number(), 
    schema.literal(null, { metadata: { exposeToBrowser: true } })
  ], { defaultValue: null }),
  }),
});

Here, we are saying that the sessionTimeout property is exposed to the UI in the case that it's the null literal, but not when it's declared as a number. I don't think this is a scenario we'd want to support.

[joshdover]

Great points about not coupling @kbn/config-schema to any particular environment 👍

In terms of the alternatives raised, I think I'm leaning towards @restrry's solution. My thinking here is that:

• Easier to read
• Sidesteps any of the nested problems mentioned above
• Requires zero changes to @kbn/config-schema which should make adding this a bit easier & less likely to break existing usages of this library.

• Data consistency. During migration, we have NP and LP config services. They might have different values due to inconsistency in validation rules. Having a warning in docs is enough?

I don't think this problem is specific to the issue of accessing config on the client. This type of discrepancy can exist today. IMO, documentation about this is good enough. This is a temporary situation which should be able to be resolved in the coming months as more plugins are migrated to the New Platform.

• Rehydration. Config might have fields with not serializable data structures - ByteSize, Duration and we need to rehydrate them on the client. Can be solved via serialization/de-serialization wrapper by request.

I imagine that to do this rehydration process, we'll also need to serialize the schema itself (or import it into browser bundles?). This sounds non-trivial to me. We could pare down the work here by only supporting a handful of primitive types in the initial version and falling back to strings for the other types.

• Runtime updates
  Config service on the server-side provides a config as a stream of values. Even though we support only logging config updates ATM, we may have to support this functionality later on the client-side as well. For now, we can provide only a static value to simplify consumers logic.

+1 on pushing runtime updates to a future change.

• Server & Client API consistency.
  We pass a config as a part of PluginInitializerContext on the server and should do so on the client as well.

+1

[mshustov]

also

export const config = {
  exposeToBrowser: ['key1'], 
}

should support shared config types for server & client sides via Pick<Config, 'key'>

[pgayvallet]

Started to look at this one.

A few questions comes to mind:

• How do we manage nested configuration values?

export const ConfigSchema = schema.object(
  {
    ui: schema.object({ enabled: schema.boolean({ defaultValue: false }) }),
  },
);

should we allow something like

export const config = {
    exposeToBrowser: ['ui.enabled'], 
    schema: ConfigSchema,
}

or do we simply only allow to expose root-level values?

• Technically, what options do we have to actually inject the values from the server to the client side without performing a request?

[mshustov]

How do we manage nested configuration values?

We can start with the simplest option and expose only root keys. Config schema can be non-trivial and contain conditional types, maybe types, union types, etc. It can make it harder to access nested fields.

Technically, what options do we have to actually inject the values from the server to the client side without performing a request?

We can inject data in the HTML page. As we do in the legacy platform https://github.com/elastic/kibana/blob/master/src/legacy/ui/ui_render/ui_render_mixin.js#L235

Another problem: there could be client-side only plugins that want to leverage the config mechanism. We need to decide how they should declare schema and browser keys.

[pgayvallet]

Another problem: there could be client-side only plugins that want to leverage the config mechanism. We need to decide how they should declare schema and browser keys.

Hum, that's seems like more than one problem actually:

1. How do we manage plugin schema validation without proper associated server-side plugin?
2. (follow up of 1.) In Kibana should crash immediately when misconfigured #49378 we want the app to crash in case of unknown keys. We will need to have the backend aware of the client-only plugin configuration one way or another
3. In term of consistency, that seems like server+client plugins would register the client-exposed keys in the server plugin's config, but that client only plugin would use another mechanism?

Is kbn-config-schema and it's dependencies supposed to work client-side? if that's the case, I guess we could have the client-side plugin expose it's config schema and having the discovery system load it if the plugin is client-side only? but I still don't like the behaviour difference between a client-only and a server or server/client plugin.

• How was this working in legacy? was basically the whole config exposed client-side?
• Do we know for sure we have an actual need about this?

[pgayvallet]

Another option to actually use the same mecanism between client-only plugins and other kind of plugin would be to actually expose the config from the plugin root folder plugin instead of the server folder plugin/server ?
That way discovery could load and validates the config the same way it currently does, client plugin will not need to creates a server-side plugin, and the behaviour would be the same for every kind of plugins?

We could even keep the old config path loading as fallback to preserve compatibility with plugin already exposing a config in plugin/server/index.ts ?

As an alternative, we could also allow client-only plugin to expose a config object from plugin/server without an actual server-plugin declaration, but I think I prefer the first option?

WDYT @restrry ?

[joshdover]

Is kbn-config-schema and it's dependencies supposed to work client-side?

It does not currently work client-side due to the version of Joi we're using. I believe once #48026 is merged, we could upgrade our Joi version to the latest one, which does support the browser environment.

How was this working in legacy? was basically the whole config exposed client-side?

In legacy, plugins could define a injectVars function on their server-side plugin definition which would allow them to pass arbitrary data to be serialized to the client when the frontend boots. Problem with this pattern:

• It can't be used for client-side only plugins
• The way it's designed in legacy has lead to a lot of inter-plugin dependencies on this data (probably a fixable problem, but the issue above is a non-starter)

Do we know for sure we have an actual need about this?

A lot of legacy plugins use the injectedVars pattern today, so I do think there is a need. Another option would be to require plugins to make a HTTP request to the backend when they start, but I think that would lead to a lot of requests happening at once to the backend when the frontend app is first bootstrapped. This would also not work for client-side only plugins

That way discovery could load and validates the config the same way it currently does, client plugin will not need to creates a server-side plugin, and the behaviour would be the same for every kind of plugins?

We could even keep the old config path loading as fallback to preserve compatibility with plugin already exposing a config in plugin/server/index.ts ?

As an alternative, we could also allow client-only plugin to expose a config object from plugin/server without an actual server-plugin declaration, but I think I prefer the first option?

This option seems attractive from a consistency standpoint, but results in a plugin being able to execute code on the server. Though, so does the original proposal in this issue.

I think the only option that would not lead to client-side plugins being able to execute code on the server would be a static json file format, which is definitely outside the scope of what we need to solve now. Given that, I think we will need to punt on pure client-side only plugins until we have time to tackle that.

The issue with exposing the same config object to client and server is that some config values are sensitive and should not be exposed to the client at all (eg. encryption keys, Elasticsearch credentials, etc.). We absolutely need the developer to explicitly whitelist which options are available to the client.

[pgayvallet]

we could upgrade our Joi version to the latest one, which does support the browser environment.

We don't really need joi client-side, it's just that importing the config type (we only need the type here, and only for generating the client config type from the server's using something like restrry suggested) will result in probably importing joi as the config type is based on kbn-config-schema
If we are striping types (or if tree shaking is actually enabled in webpack) this is probably not even an issue

I think the only option that would not lead to client-side plugins being able to execute code on the server would be a static json file format

As long as our validation library doesnt support static json for validation, this is not an option as the client-side only props will not be known by the server validation resulting on kibana not starting

The issue with exposing the same config object to client and server is that some config values are sensitive and should not be exposed to the client at all (eg. encryption keys, Elasticsearch credentials, etc.). We absolutely need the developer to explicitly whitelist which options are available to the client

Thats not was I was thinking of. Even with a shared config in plugin root folder, I was thinking of keeping the exposeToBrowser property to stick with explicit whitelisting. It's just a way to allow client-only plugin to actually be able to use configuration properties without a server folder/plugin.

This option seems attractive from a consistency standpoint, but results in a plugin being able to execute code on the server. Though, so does the original proposal in this issue.

If a malevolent plugin wants to execute server-side code, it just have to add a server-side part to their plugin, and voila, so I'm not sure that's a bigger issue than overall plugin sandboxing which is widely out of the scope of the issue?

But WDYT ? should we just forget about client-only plugins for now ?

[joshdover]

Thats not was I was thinking of. Even with a shared config in plugin root folder, I was thinking of keeping the exposeToBrowser property to stick with explicit whitelisting. It's just a way to allow client-only plugin to actually be able to use configuration properties without a server folder/plugin.

Got it, this makes sense to me.

If a malevolent plugin wants to execute server-side code, it just have to add a server-side part to their plugin, and voila, so I'm not sure that's a bigger issue than overall plugin sandboxing which is widely out of the scope of the issue?

In order to execute server-side code that accesses Core APIs, yes they'd need to create a server side plugin. But if there's a config.ts file that we import, then the plugin gets a chance to execute any code they want, introducing an attack vector on the underlying host.

For context here, Cloud wants to be able to allow customers to install 3rd party plugins that cannot execute any code on their infrastructure at all. An alternative would be sandboxing, but that still carries some risks.

I don't think we necessarily need to solve this case now, we should just think about how it would be done in the future and make sure we don't code ourselves into a corner.

I think what you're proposing is a viable option, and we can address true client-side only plugins in the future, possibly with allowing schema to be defined in a json format, just with limited functionality.

[pgayvallet]

Ok, let's drop the client-only plugins config support from the issue then.