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

[Core][WIP] SavedObjects v2 #40380

Closed
rudolf opened this issue Jul 4, 2019 · 2 comments
Closed

[Core][WIP] SavedObjects v2 #40380

rudolf opened this issue Jul 4, 2019 · 2 comments
Labels
Feature:Saved Objects stale Used to mark issues that were closed for being stale Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@rudolf
Copy link
Contributor

rudolf commented Jul 4, 2019

This is a WIP meta-issue to collect all the known shortcomings related to SavedObjects

1. Validation

Unless data (or attributes) is validated across the entire SavedObjects service it's nearly impossible to write reliable migrations and business logic. Having to add defensive checks in your migrations and business logic means we're not achieving the benefit of having a migration system: always having a consistent schema/shape for your data and being able to easily reason about the range of values your code has to deal with.

To be effective validation has to be applied to the following areas:

  1. loading data from elasticsearch (since we don't control what data can be pushed directly into es)
  2. writing data to elasticsearch
  3. accepting data from the HTTP API
  4. Running migrations Improve handling of invalid data during migrations #38669

Note: validation should not enforce business rules or constraints, these should be applied at the API level (see 2.1).

2. SavedObjects HTTP API

  1. Kibana automatically exposes the saved objects API for all plugins which makes it impossible for Plugin authors to apply business logic constraints on incoming data. Plugins can define their own HTTP API's in order to implement these constraints, but there's no way to disable the SavedObjects HTTP API. In addition to placing constraints on data, plugins might want to augment the data with derived fields or metadata and require strong guarantees that this derived data will always be present Pre-save modification hooks for Saved Objects #27380 Add date created, date modified, other metadata to Saved Objects #9202.
  2. Some plugins consume their SavedObjects from the client-side by exposing their own GraphQL endpoint with custom resolvers that use the SavedObjects client. These plugins therefore don't need an HTTP API.
  3. Given the above, is it worth not exposing an HTTP API for all Saved Objects, but rather providing utilities so that plugins could easily mount the "standard" Saved Objects API if they wish. (How would this work with spaces, security?)

3. SavedObjectsClient

1. Common

  1. Typed errors for all methods. Don't reject on known error conditions but return these errors instead so that errors can be typed. Having a discriminated union of typed results or errors gives us type guarantees that all conditions are handled. Having types in place for errors also makes it easier to change error signatures.
  2. Change method signatures to take advantage of object destructing create({ type, attributes, overwrite, references })

2. Client-side

  1. Exposed data is different to that of the HTTP API and server-side clients:
    client {savedObjects: []}
    server: {saved_objects: [...]}

3. server-side

  1. Spaces and security plugins rely on the ability to extend or compose functionality into the SavedObjectsClient. We currently depend on a combination of "specialized" SavedObjectsClients which use composition to extend the underlying SavedObjectsRepository and "wrappers". The current architecture makes it hard to reason about the resulting behaviour. It's also hard to build features into spaces without bleading x-pack implementation details into OSS Security - Reorder Saved Objects Client wrappers #38444.

5. KibanaMigrator

  1. Allow migrations to asynchronously load state before running each of the synchronous migration functions Saved object migrations to allow pre-migration state collection #34996
  2. Don't import data if migrations failed. Unlike migrations on an index, there is no potential for data loss so we shouldn't accept imported data when migrations fail.
@rudolf rudolf added Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Jul 4, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@mikecote
Copy link
Contributor

mikecote commented Jul 17, 2019

One thing I could see being added is to change the saved objects client function signatures. It could be different and take advantage of object destructuring.

Example:

// From
create(type, attributes, { overwrite, references })

// To
create({ type, attributes, overwrite, references })

It seemed odd since the beginning to put references within the options when it really is a sibling field to attributes.

@joshdover joshdover added the stale Used to mark issues that were closed for being stale label Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Saved Objects stale Used to mark issues that were closed for being stale 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