[Index management] Server-side NP ready #56829

sebelga · 2020-02-05T09:01:05Z

This is the first PR of 3 to migrate Index management to the new platform to make it easier to test and review.

This PR migrates the server-side code and update all the API routes to

Use the new platform router (with schema validation)
Use the "licensing" new platform plugin to guard our API route with license check

How to test

Navigate through the app and make sure that all requests are fulfilled correctly.
Open the flyout details and make sure all tabs display the correct data
Test all index drop-down actions from the list of indices (not on .kibana-1 index! 😄 )
Create / edit / delete an index template
Make sure the extension points from ILM, Rollup and CCR are correctly added. You can see it on the request reponse, each index should have the metadata added

And for follower indices a badge should appear in the table

Code structure proposal

I am making the proposal to put the logic inside the plugin setup() phase inside dedicated services. The end result should be something like this

As you can see both the route registering and the license are encapsulated inside their own classes. Which I think is cleaner than what we currently have with watcher.

Once the whole migration of Index management is done (moved to the "plugins" folder), I think it would be good to move the License.ts service out to our "es-ui-shared" folder and make it available to all our apps (through the np dependencies mechanism).

cc @cjcenizal @jloleysens @alisonelizabeth

elasticmachine · 2020-02-05T09:32:58Z

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

jloleysens

Code

No major blockers identified here, the plugin looks quite ready for NP!

Left a comment regarding the new pattern you proposed. Perhaps we can move the conversation to a separate issue?

One thing that we can do to be absolutely sure we have weeded out all remaining NP dependencies is place the application inside an np_ready folder which will make ES lint check for illegal import paths too - next time perhaps!

UI/UX

I saw this error appear while editing the JSON settings of an index:

I was just wondering whether more helpful information is not being returned from ES?

jloleysens · 2020-02-06T10:27:38Z

x-pack/legacy/plugins/index_management/server/plugin.ts

+  setup({ http }: CoreSetup, { licensing }: Dependencies): IndexMgmtSetup {
+    const router = http.createRouter();
+
+    this.license.setup(


Overall I like this idea.

It's nice to see how the different plugin phases can be tagged in services so we build on the established plugin pattern.

My question is; what benefits do we get from have instances of the services live on the plugin as opposed to constructing them at setup time and passing them into our other components? Is it mainly for naming or do we see these services often also having start phase concerns?

It seemed more logical to me to instantiate them outside, no real benefit. But as you said, this patterns allows us to call .start() and .stop() if needed.

sebelga · 2020-02-06T14:06:02Z

Thanks for the review @jloleysens !

One thing that we can do to be absolutely sure we have weeded out all remaining NP dependencies is place the application inside an np_ready folder which will make ES lint check for illegal import paths too - next time perhaps!

I had it there, but then decided to put it back at root level so all relative imports are correct (to "common" folder mainly) and ready to by copied as is in the "plugins" folder.
I guess that eslint will complain in the next PR when actually having the plugin inside the "plugins" folder right?

I saw this error appear while editing the JSON settings of an index

I will look if I changed something in the response. Do you remember what you changed to get that error?

jloleysens · 2020-02-06T15:45:06Z

Do you remember what you changed to get that error?

Per the picture, I just added "blah": 1 to the bottom of settings. 😆

sebelga · 2020-02-07T08:28:04Z

Per the picture, I just added "blah": 1 to the bottom of settings.

This is usually a good way to start breaking things 😄

…x-management

kibanamachine · 2020-02-11T06:01:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1e902eb

History

💔 Build #25500 failed fc96fed
💔 Build #24527 failed 7f8d305

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

* master: (34 commits) [Index management] Server-side NP ready (elastic#56829) Webhook action - make user and password secrets optional (elastic#56823) [DOCS] Removes reference to IRC (elastic#57245) [Monitoring] NP migration: Local angular module (elastic#51823) [SIEM] Adds ECS link to help menu (elastic#57104) Ensure http interceptors are shares across lifecycle methods (elastic#57150) [Remote clusters] Migrate server code out of legacy (elastic#56781) fixes render bug in alert list (elastic#57152) siem 7.6 updates (elastic#57169) Make the update alert API key API work when AAD is out of sync (elastic#56640) fix(NA): MaxListenersExceededWarning on getLoggerStream (elastic#57133) [Metrics UI] Setup commonly used time ranges in timepicker (elastic#56701) [Maps] set filter.meta.key to geoFieldName so query passes filterMatchesIndex when ignoreFilterIfFieldNotInIndex is true (elastic#56692) Create plugin mock for event log plugin (elastic#57048) fix ts error on master (elastic#57236) Don't create API key for disabled alerts when calling create API (elastic#57041) Fix enable and disable API to still work when AAD is out of sync (elastic#56634) [DOCS] Canvas embed objects (elastic#57156) Delete autocomplete namespace (elastic#57187) Security - Inject logout url (elastic#57201) ...

sebelga added 26 commits February 5, 2020 14:04

Move server code to "np_ready" folder

b22e9ab

Add reload() api route

ee474e8

Add indexClose() & indexOpen() api route

88192c5

Add indexRefresh() api route

65f1ec2

Add freeze() & unfreeze() api route

bd74ed2

Add flush() & forceMerge() api route

a69e7f8

Add deleteIndex() api route

ba2c389

Add indexMapping() api route

55a1c78

Add index settings Api routes

7e53791

Add indexStats() api route

09542fe

Add get template(s) api routes

8425911

Clean-up server plugin dependencies

c212847

Add create & update template api routes

59ac5da

Use data client instead of admin client

d5e6177

Add delete template api routes and bring in wrapEsError helper

3f7ffe5

Use class to declare ApiRoutes

d4616bb

Create License service to check license on routes

38e038c

Add route guard to License service

12c3723

Guard all api route with license checker

81410f1

Remove constructor from License and move to setup() call

77188c8

Move IndexDataEnricher to its own class

3aa9d3b

Add missing license route guard

a6e227e

Add missing "rollover_alias" prop to ilmPolicy schema object

fec69db

Refactor var name plugin public api

78340e6

Remove "np_ready" folder for server side

5e1b5e7

Add readonly to Server plugin class

7f8d305

sebelga changed the title ~~[Index management] NP migration~~ [Index management] Server-side NP ready Feb 5, 2020

sebelga added Feature:Index Management Index and index templates UI Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes labels Feb 5, 2020

sebelga added the Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more label Feb 5, 2020

sebelga removed the Feature:New Platform label Feb 5, 2020

sebelga requested a review from jloleysens February 5, 2020 09:33

sebelga marked this pull request as ready for review February 5, 2020 09:34

jloleysens approved these changes Feb 6, 2020

View reviewed changes

sebelga added 4 commits February 7, 2020 14:26

Add lib helpers in route dependencies

5861ab0

Wrap all server api response in try catch and return ES errors

5da8c81

Fix API integration tests

30a7494

Merge branch 'master' into np-migration/index-management

fc96fed

sebelga mentioned this pull request Feb 10, 2020

[Index management] Client-side NP ready #57186

Closed

sebelga added 2 commits February 11, 2020 10:02

Fix API integration test (2)

167f075

Merge remote-tracking branch 'upstream/master' into np-migration/inde…

1e902eb

…x-management

sebelga added v7.7.0 v8.0.0 labels Feb 11, 2020

sebelga merged commit 7e9d797 into elastic:master Feb 11, 2020

sebelga deleted the np-migration/index-management branch February 11, 2020 06:06

cjcenizal added the Feature:NP Migration label Feb 12, 2020

sebelga mentioned this pull request Feb 20, 2020

[7.x] [Index management] Server-side NP ready (#56829) #58085

Merged

sebelga added a commit that referenced this pull request Feb 20, 2020

[Index management] Server-side NP ready (#56829) (#58085)

4c6a15c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Index management] Server-side NP ready #56829

[Index management] Server-side NP ready #56829

sebelga commented Feb 5, 2020 •

edited

Loading

elasticmachine commented Feb 5, 2020

jloleysens left a comment

jloleysens Feb 6, 2020

sebelga Feb 6, 2020

sebelga commented Feb 6, 2020

jloleysens commented Feb 6, 2020 •

edited

Loading

sebelga commented Feb 7, 2020

kibanamachine commented Feb 11, 2020

[Index management] Server-side NP ready #56829

[Index management] Server-side NP ready #56829

Conversation

sebelga commented Feb 5, 2020 • edited Loading

How to test

Code structure proposal

elasticmachine commented Feb 5, 2020

jloleysens left a comment

Choose a reason for hiding this comment

Code

UI/UX

jloleysens Feb 6, 2020

Choose a reason for hiding this comment

sebelga Feb 6, 2020

Choose a reason for hiding this comment

sebelga commented Feb 6, 2020

jloleysens commented Feb 6, 2020 • edited Loading

sebelga commented Feb 7, 2020

kibanamachine commented Feb 11, 2020

💚 Build Succeeded

History

sebelga commented Feb 5, 2020 •

edited

Loading

jloleysens commented Feb 6, 2020 •

edited

Loading