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

Schema library for new platform: Joi vs @kbn/schema? #18818

Closed
azasypkin opened this issue May 4, 2018 · 10 comments
Closed

Schema library for new platform: Joi vs @kbn/schema? #18818

azasypkin opened this issue May 4, 2018 · 10 comments
Assignees
Labels
discuss Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@azasypkin
Copy link
Member

I tried to go through all places in Kibana where we use Joi to see what features we rely on exactly and compared them to the feature set provided by @kbn/schema (currently @kbn/utils). Here is the summary:

Joi features that we use, but currently aren't supported by @kbn/schema

  • Sibling references - we use them quite often, it may take some time to replicate this functionality in @kbn/schema correctly
  • Context references ($dev, $buildSha etc.) - it's just a set of key/value pairs that are provided at run-time and can be used by any schema validator within the tree. The concept is easy to implement.
  • Conditional schema - easy to implement assuming context and sibling references are implemented.
  • String validators
    • string.insensitive - easy to implement
    • string.guid - may take some time to implement properly
    • string.uri - may take some time to implement properly
    • string.hostname - may take some time to implement properly
  • Number validators
    • number.integer - easy to implement
  • Date validators
    • date - may take some time to implement properly
    • date.iso - one-liner regex, easy to implement
  • Array validators
    • array.single - easy to implement
  • Object validators
    • object.unknown - easy to implement
    • object.pattern - easy to implement, but very weird thing...
  • Few other validators like any.notes, any.allow and any.func - all are easy to implement

Joi doesn't have Duration or ByteSize validators out of the box like @kbn/schema has, but it'd be straightforward to add them via Joi.extend. What I don't like in Joi is a huuuuuge API surface and absence of a way to define custom validator functions right in the schema without going through Joi.extend unless I missed that (example from new platform).

So right now I can think of three ways to move forward:

  1. Switch over to Joi in new platform and get rid of @kbn/schema - it will require some time, but not much. The major downside is that we'll have Joi in our "public" interface with its entire huge API
  2. Keep using @kbn/schema and implement missing features one by one, only when particular feature blocks someone from migrating to new platform. The major downside is amount of required "reinvent-the-wheel" work
  3. Keep using @kbn/schema, but rely on Joi under the hood. With this we'll still have power of Joi but won't spend too much time replicating it in @kbn/schema and will have full control over our public API without exposing Joi there. The major downside I see here is kind of weird undecided temporal solution for schema management.

@elastic/kibana-platform and @epixa please vote or offer alternative solution :) Let me know if there is not enough data for you to decide, I'll try get more.

@azasypkin azasypkin added notabug When the issue is closed this label Indicates that it wasn't a bug feedback_needed Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc non-issue Indicates to automation that a pull request should not appear in the release notes Feature:New Platform discussion labels May 4, 2018
@epixa
Copy link
Contributor

epixa commented May 4, 2018

We don't have to provide a comprehensive generic schema library on our own. We can choose to expose certain third party dependencies to plugins directly, if we feel those dependencies are valuable, stable, and not worth implementing ourselves. With a public plugin API though, we'll need to keep those dependencies locked on their major version throughout our major version. This is no different than Elasticsearch exposing log4j, for example.

That said, we need far more control over our config validations than we get by relying on semver declarations of a third party plugin. At the moment, we make no distinction between generic schema validation and our config schemas, which is a little strange. For example, it may be appropriate for a generic schema library to support twenty different ways to specify a date, but it doesn't make as much sense for date values in kibana.yml to support twenty different formats. We may not even want plugins to have control over what date format(s) they support in their configuration in order to keep the overall kibana.yml consistent.

So I see our config schema as a subset of the overall generic schema capabilities we have at our disposal, even though right now they are one in the same.

For that reason, I don't think option 2 is the right move here. Schema validation is a tricky business, and if we can avoid getting bogged down in it, we'll probably be happier. Instead, we should probably adopt a library like joi internally and then work on limiting the feature set through a config-specific facade.

@spalger
Copy link
Contributor

spalger commented May 4, 2018

I agree with @epixa, but it sounds like he's suggesting we go for option 3, not 2, which is what I think I'd prefer.

Basically, expose methods for configuring Joi that allows us to fully control the version exposed and enable things like adhoc extensions.

It still feels like a lot of work to define the new interface and document it completely, but it's almost certainly work worth doing.

@azasypkin
Copy link
Member Author

Thanks for the input @epixa and @spalger! Could you please clarify a few points for me?

should probably adopt a library like joi internally

By internally you mean within something like @kbn/schema package or within platform (and only platform, core plugins as any other plugins will work with facade(s) platform will provide)?

work on limiting the feature set through a config-specific facade.

Does config-specific facade include both config schema and route validation schema? It feels like route validation and config validation should rely on different "facades" (even if they are similar)...

Basically, expose methods for configuring Joi that allows us to fully control the version exposed and enable things like adhoc extensions.

I'm not sure I get the idea exactly, can you provide a quick example (exposing from where to where)?

It still feels like a lot of work to define the new interface and document it completely, but it's almost certainly work worth doing.

I'm not scared :) But do you think this new interface will differ drastically from the schema-related stuff we have already in new platform?

@rhoboat
Copy link

rhoboat commented May 4, 2018

I vote 3.

@rhoboat
Copy link

rhoboat commented May 4, 2018

I think I get what Court is saying, so I can clarify too

By internally you mean within something like @kbn/schema package or within platform (and only platform, core plugins as any other plugins will work with facade(s) platform will provide)?

I think he means inside of @kbn/schema, which is basically a wrapper for Joi. It follows that then all this functionality is available across Kibana.


Does config-specific facade include both config schema and route validation schema? It feels like route validation and config validation should rely on different "facades" (even if they are similar)...

For now we're talking about just config, right? If it makes sense to group things together here, like route validation, then fine. I think we should start with what we need for config schema.


Basically, expose methods for configuring Joi that allows us to fully control the version exposed and enable things like adhoc extensions.

I'm not sure I get the idea exactly, can you provide a quick example (exposing from where to where)?

The end user doesn't have to care about what version of Joi we're employing behind the scenes. So we can keep updating Joi or not based on our timeline. If we didn't create a wrapper around joi, we couldn't do this.

Enabling ad hoc extensions is also possible because we're not tied to the particular things Joi exposes in its API--we can create our own extra methods we might need. Or say Joi has a method that doesn't accept certain arguments, we could wrap that in our own, allow passing in new or additional arguments, and only expose that. We never have to expose the methods of the core Joi lib (or any other lib we might use).

@epixa epixa removed non-issue Indicates to automation that a pull request should not appear in the release notes notabug When the issue is closed this label Indicates that it wasn't a bug labels May 5, 2018
@azasypkin
Copy link
Member Author

Thanks @archanid! Okay, it seems I confused you this time, sorry for that. I'm probably being too picky about wording, but better safe than sorry 🙈

So everything what @archanid is saying is already implied by proposed solution №3 and I'm glad we're on the same page, but that's exactly the source of my confusion:

  • @epixa, you didn't mention option 3 explicitly, that's why I assume that proposed isolation of Joi inside of something like @kbn/schema with exposing of a limited, controlled and 3rd-party-lib-agnostic set of API is not exactly what you suggest, right?

  • @spalger, you're saying __expose methods for configuring Joi__ that allows us to fully control the __version exposed__, but none of the consumers of @kbn/schema should know anything about Joi (its version or specific configuration options) as it's going to be internal implementation detail of @kbn/schema. You also mention enable things like adhoc extensions, do you mean @kbn/schema should provide extension points to outside? Because internally we'll obviously be able to extend Joi as we want, but again it's purely @kbn/schema internal "business" (unless we alter public API exposed by @kbn/schema) or I misunderstanding something?

@epixa
Copy link
Contributor

epixa commented May 5, 2018

@epixa, you didn't mention option 3 explicitly, that's why I assume that proposed isolation of Joi inside of something like @kbn/schema with exposing of a limited, controlled and 3rd-party-lib-agnostic set of API is not exactly what you suggest, right?

I didn't mention option 3 explicitly because while it's close to what I'm proposing, it's not quite the same thing. At the moment, @kbn/schema is a public generic schema library, and I'm proposing that we may not even want our own generic schema library. What we really need right now is a config schema library, and that we need to own the public interface for, even if it's powered by joi behind the scenes. We should also choose to be as restrictive as possible in the feature set of that interface based on our known needs from kibana.yml.

I didn't mention this part specifically, but we also need some sort of schema system for route validation, but does it need to be a public package at all? Route configuration happens during the lifecycle of plugins, so why not just expose the validation interface directly from that service? The config service validation stuff happens before the lifecycle begins (before start) for plugins, so that kind of needs to be exposed as a static library, but that doesn't mean all possible types of schema validation should be static.

If at some point we decided that we did want to expose a generic schema library to plugins for some other purposes (note, not as an end-user facing feature, just a generic library), we don't have to write it ourselves, and we don't even need to maintain a facade necessarily. We could choose to expose Joi as a supported dependency and lock its major version to ours. But let's not bother with that now because it's not even clear if we need a common, generic schema library.

@spalger
Copy link
Contributor

spalger commented May 7, 2018

none of the consumers of @kbn/schema should know anything about Joi (its version or specific configuration options) as it's going to be internal implementation detail of @kbn/schema

Yeah, the language I used is weird but that was my expectation as well.

do you mean @kbn/schema should provide extension points to outside?

By this I just meant that we can enable features like what you linked to here. Again, I think we're on the same page.

@azasypkin
Copy link
Member Author

Okay, great, thank you all. I believe I have enough info to move forward with the solution for config schema validation in new platform. We'll see how it goes.

@azasypkin
Copy link
Member Author

Handled in #19556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

4 participants