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

[Index Template] Fix previewing data streams in template editor #140189

Merged

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Sep 7, 2022

Summary

fix #139925

Currently, when you create a data stream template and try to use the "preview template" functionality, you'll simulate an index template without a data stream configuration because the data stream is configured using an empty object {} and existing code strips empty objects from the template during simulation:

const indexTemplate = serializeTemplate(stripEmptyFields(template) as TemplateDeserialized);

You can see this that by starting creating a data stream:
Screenshot 2022-09-07 at 15 14 18
And then use "preview" on the next step. See there is no data_stream:{} in the simulate request:
Screenshot 2022-09-07 at 15 14 54

This is problematic in cases where a configurion is valid for a data stream template, but invalid for a regular index template. I bumped into this while testing new time series streams in #138748

The fix makes simulation code to remove only empty strings, but leave empty objects untouched.
Please check if this is OK. Looks like all other places which pre-process template are already removing only empty strings. For example:

stripEmptyFields(template, {
types: ['string'],
}) as TemplateDeserialized

A safer alternative could be to make handling specific for data_stream key

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
indexManagement 522.7KB 522.7KB +59.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@Dosant Dosant added release_note:fix Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Team:AppServicesUx v8.5.0 labels Sep 7, 2022
@Dosant Dosant marked this pull request as ready for review September 7, 2022 14:52
@Dosant Dosant requested a review from a team as a code owner September 7, 2022 14:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

Copy link
Contributor

@yuliacech yuliacech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for fixing this, @Dosant!
Changes LGTM, tested locally 👍

@Dosant Dosant merged commit 1a48a06 into elastic:main Sep 8, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Index Management Index and index templates UI release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Index Template] Preview strips dataStream prop making it impossible to preview a data stream
5 participants