[Infrastructure UI] Implement inventory views CRUD endpoints by tonyghiani · Pull Request #154900 · elastic/kibana

tonyghiani · 2023-04-13T10:31:32Z

📓 Summary

This PR implements the CRUD endpoints for the inventory views.
Following the approach used for the LogViews service, it exposes a client that abstracts all the logic concerned to the inventory-view saved objects.

It also follows the guideline provided for Versioning interfaces and Versioning HTTP APIs, preparing for the serverless.

🤓 Tips for the reviewer

You can open the Kibana dev tools and play with the following snippet to test the create APIs, or you can perform the same requests with your preferred client:

// Get all
GET kbn:/api/infra/inventory_views

// Create one
POST kbn:/api/infra/inventory_views
{
  "attributes": {
    "name": "My inventory view"
  }
}

// Get one
GET kbn:/api/infra/inventory_views/<switch-with-id>

// Update one
PUT kbn:/api/infra/inventory_views/<switch-with-id>
{
  "attributes": {
    "name": "My inventory view 2"
  }
}

// Delete one
DELETE kbn:/api/infra/inventory_views/<switch-with-id>

👣 Next steps

Replicate the same logic for the metrics explorer saved object
Create a client-side abstraction to consume the service
Update the existing react custom hooks to consume the endpoint

ghost · 2023-04-13T10:31:48Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…-ref HEAD~1..HEAD --fix'

…ctored

elasticmachine · 2023-04-17T12:52:01Z

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

yngrdyn

Changes in kbn-io-ts-utils package LGTM

tonyghiani · 2023-04-19T15:51:59Z

x-pack/plugins/infra/common/http_api/inventory_views/v1/common.ts

+
+export type InventoryViewRequestQuery = rt.TypeOf<typeof inventoryViewRequestQueryRT>;
+
+const inventoryViewAttributesResponseRT = rt.intersection([


note: for the attributes of this saved object, the configuration has always been dynamic and not strictly typed. As they can vary depending on the user usage on the page, I left it open to all the attributes as it was before, marking only those that I'm sure will be present in the response.

This applies to any other attributes definition you'll find for this attribute's saved object.

tonyghiani · 2023-04-19T15:54:56Z

x-pack/plugins/infra/common/http_api/inventory_views/v1/common.ts

+    isDefault: rt.boolean,
+    isStatic: rt.boolean,


note:

isDefault represent whether a saved view is the default one by user preference.

isStatic is used to distinguish the static saved view that will always be present and served, now hardcoded in the server and not configurable.

Can those values change using the API? For example, using the update endpoint to set another default view?

As those values are derived as:

isDefault: match with the inventoryDefaultView configuration saved in the source configuration

isStatic is a statically set value to false

They could insert those values triggering a manual request to the endpoint, but I guess is better to handle the case, I'll change that!

tonyghiani · 2023-04-19T16:02:32Z

x-pack/plugins/infra/server/saved_objects/inventory_view/inventory_view_saved_object.ts

note: moved the inventory-view saved object type here among the others, with #155117 I'll clean up the legacy usage of this SO defined in common/saved_objects. (Same applies for the metrics-explorer-view SO)

tonyghiani · 2023-04-19T16:05:57Z

x-pack/plugins/infra/server/services/inventory_views/inventory_views_client.ts

+    private readonly infraSources: IInfraSources
+  ) {}
+
+  static STATIC_VIEW_ID = '0';


note: keep the previous id for the static view always present.
I initially wanted to change it to the static string, but as it relies on a value saved in the infrastructure-ui-source SO, changing it would imply also writing a migration for that SO, resulting in overengineering a simple rename.

tonyghiani · 2023-04-19T16:09:53Z

x-pack/plugins/infra/server/services/inventory_views/inventory_views_client.ts

+    const defaultView = InventoryViewsClient.createStaticView(
+      sourceConfiguration.configuration.inventoryDefaultView
+    );
+    const views = inventoryViewSavedObject.saved_objects.map((savedObject) =>
+      this.mapSavedObjectToInventoryView(
+        savedObject,
+        sourceConfiguration.configuration.inventoryDefaultView
+      )
+    );
+
+    const inventoryViews = [defaultView, ...views];
+
+    const sortedInventoryViews = this.moveDefaultViewOnTop(inventoryViews);


note: When getting all the available inventory views, we want them to:

Always include the hardcoded static inventory view.

Map into a reduced version to avoid large payloads.

Sorted by creation date, but having on top the default view selected by the user.

To calculate which one is the default view and not delegate this to the client, we retrieve in parallel the source configuration to compute in runtime the isDefault attribute for each inventory view.

tonyghiani · 2023-04-19T16:12:40Z

x-pack/plugins/infra/server/services/inventory_views/inventory_views_client.ts

+  private async assertNameConflict(name: string, whitelist: string[] = []) {
+    const results = await this.savedObjectsClient.find<InventoryViewAttributes>({
+      type: inventoryViewSavedObjectName,
+      perPage: 1000,
+    });
+
+    const hasConflict = [InventoryViewsClient.createStaticView(), ...results.saved_objects].some(
+      (obj) => !whitelist.includes(obj.id) && obj.attributes.name === name
+    );
+
+    if (hasConflict) {
+      throw Boom.conflict('A view with that name already exists.');
+    }
+  }


note: we can't delegate this search to the savedObjectsClient.find method since, being the attributes mapping set to dynamic: false, searching for the name won't return results.

jennypavlova

Great work 👏

Tried the requests, and everything worked 🎉 I have several questions regarding the update endpoint (it should be PUT not POST right? - the PR description can be updated in that case)

I tried to update isStatic or isDefault and didn't work - that is correct, right? Only the name can be changed?
The default view can't be updated/deleted right? Because the API returns 404 if I try to (using 0 as id).

jennypavlova · 2023-04-20T16:24:55Z

x-pack/plugins/infra/common/http_api/inventory_views/v1/common.ts

+    isDefault: rt.boolean,
+    isStatic: rt.boolean,


Can those values change using the API? For example, using the update endpoint to set another default view?

jennypavlova · 2023-04-20T16:39:00Z

x-pack/plugins/infra/server/routes/inventory_views/delete_inventory_view.ts

+
+        return response.noContent();
+      } catch (error) {
+        if (isBoom(error)) {


Nit: If the id is set to 0 (Default view id) the response is 404, should we have a message explaining that the default view can't be deleted or something like that - because the view is returned from the GET and maybe this can be confusing 🤔

In the client will not be possible to delete the static view, and in case they try to do it manually, they'll get the 404 value, but I agree it could be confusing, I'll introduce an error for this case to handle it

jennypavlova · 2023-04-20T16:40:19Z

x-pack/plugins/infra/server/routes/inventory_views/README.md

+}
+```
+
+## Update one: `PUT /api/infra/inventory_views/{inventoryViewId}`


Should we mention which properties can be updated?

tonyghiani · 2023-04-20T17:57:29Z

Thanks for the review Jenny!

Tried the requests, and everything worked 🎉 I have several questions regarding the update endpoint (it should be PUT not POST right? - the PR description can be updated in that case)

Correct, it is implemented as a PUT, it was my mistake on the PR description, corrected 👍

I tried to update isStatic or isDefault and didn't work - that is correct, right? Only the name can be changed?

Exactly, those values are not writable, I'll take care of updating the validation against those two values!

The default view can't be updated/deleted right? Because the API returns 404 if I try to (using 0 as id).

The static view (I called it like this to distinguish it from the default view, which is a preference setting chosen by the user) is not configurable/deletable, I will add proper error handling in case the user attempt to change it.

…-fix'

jennypavlova

LGTM 🚀 Thank you for adding the changes!

kibana-ci · 2023-04-24T09:54:00Z

💚 Build Succeeded

Buildkite Build
Commit: 88466a4

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1345	1352	+7

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/io-ts-utils`	38	39	+1
`infra`	41	42	+1
total			+2

Async chunks

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

id	before	after	diff
`infra`	2.0MB	2.0MB	+1.0KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`infra`	9	10	+1

Unknown metric groups

API count

id	before	after	diff
`@kbn/io-ts-utils`	38	39	+1
`infra`	44	45	+1
total			+2

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	17	19	+2
`securitySolution`	395	398	+3
total			+5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	18	20	+2
`securitySolution`	475	478	+3
total			+5

History

💔 Build #122298 failed 0b71493
💚 Build #121770 succeeded 72086f1
💚 Build #121213 succeeded 5120167
💔 Build #121150 failed 547963f
💚 Build #121071 succeeded 8ba830c

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

…ient (#155126) ## 📓 Summary Depends on #154900 Closes #155110 This PR implements the `InventoryViewsService` and `InventoryViewsClient`, injecting an instance of the client in the KibanaContextForPlugin and exposing so a set of utilities to retrieve/update inventory views: - `findInventoryViews` - `getInventoryView` - `createInventoryView` - `updateInventoryView` - `deleteInventoryView` ## 👣 Next steps - Implement #154725 to consume the service --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…lorerViewsClient (#155878) ## 📓 Summary Depends on #154900 Closes #155112 This PR implements the `InventoryViewsService` and `InventoryViewsClient`, injecting an instance of the client in the KibanaContextForPlugin and exposing so a set of utilities to retrieve/update inventory views: - `findMetricsExplorerViews` - `getMetricsExplorerView` - `createMetricsExplorerView` - `updateMetricsExplorerView` - `deleteMetricsExplorerView` ## 👣 Next steps - Implement #154725 to consume the service --------- Co-authored-by: Marco Antonio Ghiani <marcoantonio.ghiani@elastic.co> Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com>

Marco Antonio Ghiani added 5 commits April 11, 2023 11:19

feat(infra): wip on inventory views

3ec7402

refactor(infra): move saved objects into server

10e34e8

feat(infra): add inventory view to common types with versioning

a7c9330

feat(infra): wip endpoints

c3d9c8e

feat(infra): implement inventory view endpoints

bab4c46

tonyghiani added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services release_note:skip Skip the PR/issue when compiling release notes labels Apr 13, 2023

kibanamachine and others added 8 commits April 13, 2023 10:36

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

6df39b1

…-ref HEAD~1..HEAD --fix'

fix(infra): update saved object runtime type

a6d9262

feat(infra): add static view

70a6f5f

docs(infra): add doc about endpoints

e71d868

Merge branch 'main' into 152617-saved-views-http-endpoints

15b88a1

refactor(infra): restore legacy saved view files until client is refa…

d21947a

…ctored

tests(infra): add inventoryViews test

85881d3

feat(infra): init metrics explorer view common

418b3b1

tonyghiani marked this pull request as ready for review April 17, 2023 12:51

Merge branch 'main' into 152617-saved-views-http-endpoints

bfa86b9

tonyghiani requested a review from a team April 17, 2023 12:51

tonyghiani requested a review from a team as a code owner April 17, 2023 12:51

tonyghiani requested a review from a team April 17, 2023 12:51

tonyghiani requested a review from a team as a code owner April 17, 2023 12:51

tonyghiani and others added 4 commits April 17, 2023 16:28

Merge branch 'main' into 152617-saved-views-http-endpoints

6a18272

Merge branch 'main' into 152617-saved-views-http-endpoints

59e6e33

feat(infra): update type imports

1af7672

fix(infra): typo

426f924

tonyghiani mentioned this pull request Apr 18, 2023

[Infrastructure UI] Create InventoryViewsService and InventoryViewsClient #155126

Merged

yngrdyn approved these changes Apr 18, 2023

View reviewed changes

dgieselaar approved these changes Apr 18, 2023

View reviewed changes

tonyghiani and others added 4 commits April 19, 2023 14:28

Merge branch 'main' into 152617-saved-views-http-endpoints

8ba830c

refactor(infra): sort inventory views with default on top

2e3e551

refactor(infra): switch back to old default id

547963f

test(infra): update broken test

5120167

tonyghiani commented Apr 19, 2023

View reviewed changes

jennypavlova self-requested a review April 20, 2023 15:39

jennypavlova reviewed Apr 20, 2023

View reviewed changes

tonyghiani and others added 4 commits April 20, 2023 22:14

Merge branch 'main' into 152617-saved-views-http-endpoints

8efe997

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

72086f1

…-fix'

refactor(infra): request changes

ead0118

Merge branch 'main' into 152617-saved-views-http-endpoints

0b71493

jennypavlova approved these changes Apr 24, 2023

View reviewed changes

fix(infra): update test types

88466a4

tonyghiani merged commit 6ac4e19 into elastic:main Apr 24, 2023

tonyghiani deleted the 152617-saved-views-http-endpoints branch April 24, 2023 09:54

kibanamachine added v8.8.0 backport:skip This PR does not require backporting labels Apr 24, 2023

tonyghiani mentioned this pull request Apr 25, 2023

[Infrastructure UI] Implement Metrics explorer views CRUD endpoints #155621

Merged

tonyghiani mentioned this pull request Apr 26, 2023

[Infrastructure UI] Create MetricsExplorerViewsService and MetricsExplorerViewsClient #155878

Merged


		export type InventoryViewRequestQuery = rt.TypeOf<typeof inventoryViewRequestQueryRT>;

		const inventoryViewAttributesResponseRT = rt.intersection([

Conversation

tonyghiani commented Apr 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📓 Summary

🤓 Tips for the reviewer

👣 Next steps

Uh oh!

ghost commented Apr 13, 2023

🤖 GitHub comments

Uh oh!

elasticmachine commented Apr 17, 2023

Uh oh!

yngrdyn left a comment

Choose a reason for hiding this comment

Uh oh!

tonyghiani Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonyghiani Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jennypavlova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonyghiani commented Apr 20, 2023

Uh oh!

jennypavlova left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Apr 24, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

API count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tonyghiani commented Apr 13, 2023 •

edited

Loading

tonyghiani Apr 19, 2023 •

edited

Loading

tonyghiani Apr 19, 2023 •

edited

Loading