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 management] Prepare support Index template V2 format #61588

Merged
merged 36 commits into from
Apr 2, 2020

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Mar 27, 2020

This PR prepares the index_management plugin to support the upcoming Index Template V2 (elastic/elasticsearch#53101).

Context

In v7.8 Elasticsearch is going to release a new API to read/create/update/delete index templates. Those index templates will have a different format (V2) that the current index template (V1).

The main difference being:

// V1 format

{
  "templateName": {
    "aliases: {},
    "mappings": {}.
    "settings": {},
    ...
  }
}

// V2 format
{
  "templateName: {
    "template": { // wrapped under _template_ parameter
      "aliases: {},
      "mappings": {}.
      "settings": {},
    },
    // new parameter. An array of template components (upcoming new API for 7.8),
    "composed_of": []
    // new parameter. An integer to set the priority of the index template to apply in case 2 index patterns matches an index name
    "priority: 1,
    ... // remaining parameters are the same
  }
}

There will be 2 different APIs to manage index templates:

  • PUT _template for the V1 format (supported until the end of 8.x cycle)
  • PUT _index_template for the V2 format (supported from 7.8)

This means that 2 index templates can have the same name with a different version.

Changes in this PR

This PR is a preparation work to support both formats in the UI. The client code has been refactored to work only with V2 index template format (it expects V2 format to be thrown at it) as it will be the future of index template. A _kbnMeta object is added from the server to let the client know the format version of the template.

Screenshot 2020-03-27 at 16 15 09

Knowing the format version is important for the client as, when we will fetch or delete an index template, we need to tell our API server route the format version so we can proxy the call to the correct Elasticsearch API.

We will also, in a future PR, enhance the table view to clearly indicate to the user the format of the index template. And probably add a deprecation warning for v1 template and a button to convert V1 index templates to the V2 format.

This PR still creates V1 index templates but our goal for 7.8 is to create V2 index templates by default.

How to review

  • Create an index template and verify that the Request preview (in Review step) is the V1 format

Screenshot 2020-03-30 at 15 42 30

  • Edit the index template (from the pencil icon in the table, the context menu in the table and the context menu in the details flyout), make sure it saves correctly
  • Clone an index template (from the context menu in the table and the context menu in the details flyout), make sure it saves correctly

Fixes #57288

@sebelga sebelga force-pushed the index-template-v2/table-view branch from d805d58 to ac7efa0 Compare March 27, 2020 13:13
@sebelga sebelga force-pushed the index-template-v2/table-view branch from c2f7104 to ca0f0ec Compare March 30, 2020 14:07
@sebelga sebelga marked this pull request as ready for review March 30, 2020 14:53
@sebelga sebelga added Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8 v8.0.0 labels Mar 30, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@sebelga
Copy link
Contributor Author

sebelga commented Mar 31, 2020

@elasticmachine merge upstream

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Great start with this @sebelga! Code LGTM. Tested locally and everything worked as expected. I did notice a test that was previously skipped failed on the latest run; there might be some more investigation needed there.


/**
* Up until the end of the 8.x release cycle we need to support both
* V1 and V2 index template formats. This constant keeps track of wether
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* V1 and V2 index template formats. This constant keeps track of wether
* V1 and V2 index template formats. This constant keeps track of whether

* you may not use this file except in compliance with the Elastic License.
*/

// TODO: Move mappings type from Mappings editor here
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you plan to address this as part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't plan on touching the mappings editor in this PR. As I plan on adding test coverage to the mappings editor I will do it then.

const isValid = validateContent();
const value = isValid && content.trim() !== '' ? JSON.parse(content) : {};
const data = { [prop]: value };
return Promise.resolve({ isValid, data });
// If no key has been added to the JSON object, we strip it out so an empty objecct is not sent in the request
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// If no key has been added to the JSON object, we strip it out so an empty objecct is not sent in the request
// If no key has been added to the JSON object, we strip it out so an empty object is not sent in the request

import { Location } from 'history';

export const getFormatVersionFromQueryparams = (location: Location): 1 | 2 | undefined => {
const { v } = parse(location.search.substring(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this might be more clear as const { v: version } = ...

@@ -128,8 +130,7 @@ describe('<TemplateEdit />', () => {
expect(nameInput.props().disabled).toEqual(true);
});

// TODO: Flakey test
describe.skip('form payload', () => {
describe('form payload', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this failed on the latest run. Might be more work that needs to be done. Related issue: #57288.

@sebelga
Copy link
Contributor Author

sebelga commented Apr 1, 2020

Thanks for the review @alisonelizabeth! Indeed the flakiness was still there. I added a new helper in the Testbed utils: waitFor('myTestSubject') which should help us for this type of flakiness related to hooks cycle update. (no need to add an estimated guess in await nextTick(43); // wait 43 millisecond anymore 😄)

I will run the CI on this PR a few times to make sure.

@sebelga
Copy link
Contributor Author

sebelga commented Apr 1, 2020

@elasticmachine merge upstream

@cjcenizal
Copy link
Contributor

@elasticmachine merge upstream

@shahzad31 shahzad31 requested a review from a team as a code owner April 2, 2020 06:32
@sebelga
Copy link
Contributor Author

sebelga commented Apr 2, 2020

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented Apr 2, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@sebelga sebelga merged commit 74d8413 into elastic:master Apr 2, 2020
@sebelga sebelga deleted the index-template-v2/table-view branch April 2, 2020 18:14
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 2, 2020
* master:
  Switch to embeddable factory interface with optional override (elastic#61165)
  fix text error in diagrams (elastic#62101)
  [Index management] Prepare support Index template V2 format (elastic#61588)
  Updates dashboard images (elastic#62011)
  [Maps] remove MapBounds type (elastic#62332)
  [Uptime] Convert anomaly Jobs name to lowercase to comply with… (elastic#62293)
  Make d3 place nicely with object values (elastic#62004)
  EMT-287: update schema with elastic agent id (elastic#62252)
  [Maps] fix replaceLayerList to handle case where map is not intialized (elastic#62202)
  Remove support for deprecated xpack.telemetry configurations (elastic#51142)
  [Uptime] Remove static constant for index name completely (elastic#62256)
  [APM] E2E: install dependencies for vanilla workspaces (elastic#62178)
  [backport] Bump to 5.1.3 (elastic#62286)
  Show server name in Remote Cluster detail panel (elastic#62250)
  Rename some alert types (elastic#61693)
  changing duration type to ms, s, m (elastic#62265)
  [ML] Clear Kibana index pattern cache on creation or form reset. (elastic#62184)
  Move `src/legacy/server/index_patterns` to data plugin (server) (Remove step) (elastic#61618)
  [NP] Remove IndexedArray usage in Schemas (elastic#61410)
tsullivan added a commit to tsullivan/kibana that referenced this pull request Apr 3, 2020
commit 5107ad7
Merge: 41881bc 408baf2
Author: Elastic Machine <elasticmachine@users.noreply.github.com>
Date:   Fri Apr 3 00:29:11 2020 -0400

    Merge branch 'master' into reporting/np-migration-server-config

commit 408baf2
Author: Nathan L Smith <nathan.smith@elastic.co>
Date:   Thu Apr 2 16:44:29 2020 -0500

    Allow Enterprise license for service map (elastic#62371)

    We were previously only allowing "platinum" and "trial". Change this to allow `hasAtLeast('platinum')` which includes "platinum", "enterprise", and "trial".

    Fixes elastic#62243.

commit d10889a
Author: Brandon Morelli <brandon.morelli@elastic.co>
Date:   Thu Apr 2 14:19:40 2020 -0700

    docs: updates to apm agent config (elastic#61893)

commit 2ae566e
Author: Jen Huang <its.jenetic@gmail.com>
Date:   Thu Apr 2 13:04:32 2020 -0700

    [Ingest] Fix package info request returning 500 (elastic#61712)

    * Fix 500 package info request error

    * Fix installed packages page not loading

    Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

commit 4cb96ee
Author: Mikhail Shustov <restrry@gmail.com>
Date:   Thu Apr 2 21:41:56 2020 +0200

    move crypto to server utils (elastic#62344)

    * move crypto to server utils

    * fix mocks

commit 7039aba
Author: Mike Côté <mikecote@users.noreply.github.com>
Date:   Thu Apr 2 15:25:03 2020 -0400

    Start indexing documents by default (elastic#62159)

commit 6d8bdf6
Author: Davis Plumlee <56367316+dplumlee@users.noreply.github.com>
Date:   Thu Apr 2 14:59:50 2020 -0400

    [Endpoint] Update host field accordion (elastic#61878)

commit d639432
Author: Nicolas Ruflin <spam@ruflin.com>
Date:   Thu Apr 2 20:46:52 2020 +0200

    Add more definitions about Ingest Management (elastic#62049)

    This should help to explain the different terms.

    The docs directory was also renamed from epm to ingest_manager.

commit bb747ab
Author: Stacey Gammon <gammon@elastic.co>
Date:   Thu Apr 2 14:27:51 2020 -0400

    Switch to embeddable factory interface with optional override (elastic#61165)

    * wip

    * typescript map embeddable

    * More updates

    * Address code review comments and update some usages in SIEM and uptime to the new types

    * More clean up - carry over some of the SIEM types to maps for render tool tip

    * fixes

    * fixes

    * Address more review comments

    * fixes

    * fixes

    * fix jest test

    * Fix visualize embeddable

    * fixes after master merge

    * Fixes

    * Prefix variable with name "custom" to make it more obvious

    * Remove layerList from input state

    * fixes

    * Update src/plugins/dashboard/public/embeddable/dashboard_container_factory.tsx

    Co-Authored-By: Vadim Dalecky <streamich@users.noreply.github.com>

    * review updates

    * fixes

    * update maps readme

    Co-authored-by: Vadim Dalecky <streamich@users.noreply.github.com>

commit 09f1bae
Author: Peter Schretlen <peter.schretlen@elastic.co>
Date:   Thu Apr 2 14:23:09 2020 -0400

    fix text error in diagrams (elastic#62101)

commit 74d8413
Author: Sébastien Loix <sabee77@gmail.com>
Date:   Thu Apr 2 20:14:46 2020 +0200

    [Index management] Prepare support Index template V2 format (elastic#61588)

commit d6587d7
Author: Kaarina Tungseth <kaarina.tungseth@elastic.co>
Date:   Thu Apr 2 13:03:50 2020 -0500

    Updates dashboard images (elastic#62011)

commit 41881bc
Merge: b965d81 8b31ce0
Author: Elastic Machine <elasticmachine@users.noreply.github.com>
Date:   Mon Mar 30 18:51:56 2020 -0400

    Merge branch 'master' into reporting/np-migration-server-config

commit b965d81
Merge: 3997034 9ff8be6
Author: Elastic Machine <elasticmachine@users.noreply.github.com>
Date:   Mon Mar 30 17:19:48 2020 -0400

    Merge branch 'master' into reporting/np-migration-server-config

commit 3997034
Merge: 0414532 8d539aa
Author: Elastic Machine <elasticmachine@users.noreply.github.com>
Date:   Fri Mar 27 18:58:21 2020 -0400

    Merge branch 'master' into reporting/np-migration-server-config

commit 0414532
Merge: 412635e 878ab20
Author: Joel Griffith <joel.griffith@elastic.co>
Date:   Fri Mar 27 12:48:10 2020 -0700

    Merge remote-tracking branch 'upstream/master' into reporting/np-migration-server-config

commit 412635e
Merge: b1b2bfa 5a537d1
Author: Joel Griffith <joel.griffith@elastic.co>
Date:   Fri Mar 27 11:32:11 2020 -0700

    Merge remote-tracking branch 'upstream/master' into reporting/np-migration-server-config

commit b1b2bfa
Merge: 60220fb 9a53c08
Author: Joel Griffith <joel.griffith@elastic.co>
Date:   Thu Mar 26 09:22:52 2020 -0700

    Merge remote-tracking branch 'upstream/master' into reporting/np-migration-server-config

commit 60220fb
Author: Joel Griffith <joel.griffith@elastic.co>
Date:   Wed Mar 25 10:10:24 2020 -0700

    Revert "Revert "[Reporting/New Platform Migration] Use a new config service on server-side (elastic#55882)""

    This reverts commit 5c04d9e.
@rayafratkina rayafratkina added v7.8.0 and removed v7.8 labels Apr 6, 2020
@cjcenizal
Copy link
Contributor

@sebelga Can you confirm that I understand the changes introduced by this PR correctly?

  • The GET /templates route currently fetches only v1 templates. In the future, we intend to update it to fetch both v1 and v2 index templates.
  • This route has been updated to normalize all fetched templates to the v2 shape before returning them to the client. As part of this normalization step, we add the _kbnMeta property to index templates to differentiate between those fetched from the v1 API and those fetched from the v2 API.
  • The client has been updated to accommodate the new v2 shape, and no longer supports the v1 shape.
  • The PUT /templates/{name} and POST /templates/{name} endpoints for creating and updating index templates have been updated to check the version stored in the _kbnMeta property, and either convert to v1 shape or throw an error. In the future, we intend to update it to save both v1 and v2 index templates using their respective APIs.

@sebelga
Copy link
Contributor Author

sebelga commented Apr 22, 2020

@cjcenizal this is correct. Apart from the PUT and POST, the DELETE also will need to be updated to support both APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Index Management Index and index templates UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix flaky Index Templates component integration tests
6 participants