Add 'inventory' item to Security navigation menu #204373

albertoblaz · 2024-12-16T12:11:12Z

Summary

Closes #201706.

Add 'inventory' item to Security navigation menu (either when the sidebar is expanded and the full navigation menu is shown or when the sidebar is collapsed and only the Security menu is visible).

Changeset details

Render 'inventory' item and enable /app/security/asset_inventory route both conditionally based on feature flag
Async loading/rendering of AssetInventory main page from within SecuritySolution plugin
- Delete unnecessary boilerplate existing in AssetInventory

Out of scope

AssetInventory nav sub-menu is skipped until more concrete requirements are defined on what to do with them

How to test

Activate the feature flag by adding this line to your local kibana.dev.yml:

xpack.securitySolution.enableExperimental: ['assetInventoryStoreEnabled']

Screenshots

Full menu (expanded mode)

Only Security menu (collapsed mode)

AssetInventory loaded async from within Security Solution

Checklist

If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Risks

No risks. Navigation item will be added if and only if feature flag is enabled, which shouldn't happen for end users until development is completed.

elasticmachine · 2024-12-17T09:36:38Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/index.tsx

x-pack/solutions/security/plugins/asset_inventory/public/components/app.tsx

opauloh

LGTM

kapral18

LGTM

albertoblaz · 2024-12-23T17:31:51Z

@elasticmachine merge upstream

angorayc · 2024-12-23T17:26:22Z

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/index.tsx

+
+  return (
+    <SecuritySolutionPageWrapper noPadding>
+      {assetInventory.getAssetInventoryPage({})}


Would it make more sense to make it just {assetInventory.getAssetInventoryPage()} here?

Are we able to assembled the param AppPluginStartDependencies the function needs in x-pack/solutions/security/plugins/asset_inventory/public/plugin.ts, so we don't need to provided it as a param?

I agree with you, though this is just temporary because we are not passing in any dependency for now

angorayc · 2024-12-23T17:29:18Z

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/routes.tsx

+
+export const routes: SecuritySubPluginRoutes = [
+  {
+    path: ExperimentalFeaturesService.get().assetInventoryStoreEnabled ? ASSET_INVENTORY_PATH : [],


I think we can just put path: ASSET_INVENTORY_PATH, here,
I believe the feature flag should have been taken into account via experimentalKey in x-pack/solutions/security/plugins/security_solution/public/asset_inventory/links.ts

@angorayc Doesn't experimentalKey only affect the link's visibility based on the flag? Here we wanted to prevent users from accessing the plugin's page by manually entering the URL. If you confirm we still don't need to do that, I can remove it ofc

Yes, SecurityRoutePageWrapper takes care of that. You can test the page stays unaccessible without this check when the flag is off

@semd I forgot auto-merge was enabled, but will take care of that for sure!

angorayc · 2024-12-23T17:34:01Z

x-pack/solutions/security/plugins/asset_inventory/public/components/app.tsx

+          <EuiPageTemplate.Header>
+            <EuiTitle size="l">
+              <h1>
+                <FormattedMessage id="assetInventory.allAssets" defaultMessage="All Assets" />


We have an existing assets page on the serverless version:

I guess this is a temporary place holder title, but I'd recommend making this header more specific to avoid confusion in the future:

Hmm I can bring it up in the next UX weekly meeting. I'm just following this Figma mockup.

elasticmachine · 2024-12-23T19:06:00Z

💚 Build Succeeded

Buildkite Build
Commit: 37c90a5

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`assetInventory`	17	6	-11
`securitySolution`	6479	6483	+4
total			-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
`assetInventory`	6	8	+2

Async chunks

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

id	before	after	diff
`assetInventory`	1.2KB	817.0B	-382.0B
`securitySolution`	21.4MB	21.4MB	+1.0KB
total			+657.0B

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
`assetInventory`	0	1	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`assetInventory`	3.1KB	3.3KB	+179.0B
`securitySolution`	88.0KB	88.2KB	+169.0B
`securitySolutionEss`	11.8KB	11.8KB	+35.0B
`securitySolutionServerless`	26.4KB	26.4KB	+35.0B
total			+418.0B

Unknown metric groups

API count

id	before	after	diff
`assetInventory`	6	8	+2

References to deprecated APIs

id	before	after	diff
`assetInventory`	1	0	-1

History

💚 Build #261919 succeeded c20c974
💔 Build #261471 failed 05e75f5
💔 Build #261342 failed 1b24077
💔 Build #261273 failed 22bc87d

cc @albertoblaz

semd · 2024-12-23T19:10:45Z

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/index.tsx

+  return (
+    <SecuritySolutionPageWrapper noPadding>
+      {assetInventory.getAssetInventoryPage({})}
+      <SpyRoute pageName={SecurityPageName.assetInventory} />


That's good, but SecurityRoutePageWrapper already adds the SpyRoute implicitly, no need to add it again :)

@semd Didn't know that either, thanks a lot. Will take care of that in a follow-up PR

semd · 2024-12-23T19:14:03Z

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/routes.tsx

+
+export const routes: SecuritySubPluginRoutes = [
+  {
+    path: ExperimentalFeaturesService.get().assetInventoryStoreEnabled ? ASSET_INVENTORY_PATH : [],


Yes, SecurityRoutePageWrapper takes care of that. You can test the page stays unaccessible without this check when the flag is off

## Summary Add 'inventory' item to Security navigation menu (either when the sidebar is expanded and the full navigation menu is shown or when the sidebar is collapsed and only the Security menu is visible). ### Changeset details - Render 'inventory' item and enable `/app/security/asset_inventory` route both conditionally based on feature flag - Async loading/rendering of AssetInventory main page from within SecuritySolution plugin - Delete unnecessary boilerplate existing in AssetInventory ### Out of scope - AssetInventory nav sub-menu is skipped until more concrete requirements are defined on what to do with them ### How to test Activate the feature flag by adding this line to your local `kibana.dev.yml`: ```yml xpack.securitySolution.enableExperimental: ['assetInventoryStoreEnabled'] ``` ### Screenshots <details><summary>Full menu (expanded mode)</summary> <img width="240" alt="Screenshot 2024-12-16 at 13 12 45" src="https://github.com/user-attachments/assets/f0939f38-5be6-481b-ace1-07f46f3622ae" /> </details> <details><summary>Only Security menu (collapsed mode)</summary> <img width="256" alt="Screenshot 2024-12-16 at 13 12 33" src="https://github.com/user-attachments/assets/b0bd62f0-5cea-4b7b-a731-3a53be362192" /> </details> <details><summary>AssetInventory loaded async from within Security Solution</summary> <img width="1640" alt="Screenshot 2024-12-16 at 17 23 01" src="https://github.com/user-attachments/assets/b84716c9-6b18-4225-bf71-62c8ef07b302" /> </details> ### Checklist - [x] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Risks No risks. Navigation item will be added if and only if feature flag is enabled, which shouldn't happen for end users until development is completed.

albertoblaz added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related labels Dec 16, 2024

albertoblaz self-assigned this Dec 16, 2024

albertoblaz force-pushed the asset-inv-nav-menu branch 3 times, most recently from 83dc7b1 to 73fe3bd Compare December 17, 2024 09:36

albertoblaz marked this pull request as ready for review December 17, 2024 09:36

albertoblaz requested review from a team as code owners December 17, 2024 09:36

albertoblaz requested review from ashokaditya and szwarckonrad December 17, 2024 09:36

ashokaditya requested review from dasansol92 and removed request for ashokaditya December 17, 2024 09:54

albertoblaz added the backport:skip This commit does not require backporting label Dec 17, 2024

albertoblaz marked this pull request as draft December 17, 2024 11:00

albertoblaz force-pushed the asset-inv-nav-menu branch 2 times, most recently from 346c865 to 05e75f5 Compare December 17, 2024 15:56

opauloh reviewed Dec 17, 2024

View reviewed changes

x-pack/solutions/security/plugins/security_solution/public/asset_inventory/pages/index.tsx Outdated Show resolved Hide resolved

x-pack/solutions/security/plugins/asset_inventory/public/components/app.tsx Outdated Show resolved Hide resolved

albertoblaz force-pushed the asset-inv-nav-menu branch from 179afb6 to 5c28360 Compare December 18, 2024 15:26

Add 'inventory' to Security navigation menu

c20c974

albertoblaz force-pushed the asset-inv-nav-menu branch from 5c28360 to c20c974 Compare December 18, 2024 15:26

albertoblaz marked this pull request as ready for review December 18, 2024 15:27

opauloh approved these changes Dec 18, 2024

View reviewed changes

albertoblaz enabled auto-merge (squash) December 18, 2024 17:22

joeypoon approved these changes Dec 18, 2024

View reviewed changes

szwarckonrad approved these changes Dec 19, 2024

View reviewed changes

kapral18 approved these changes Dec 23, 2024

View reviewed changes

Merge branch 'main' into asset-inv-nav-menu

37c90a5

angorayc reviewed Dec 23, 2024

View reviewed changes

albertoblaz merged commit 6d5be74 into elastic:main Dec 23, 2024
8 checks passed

kibanamachine added the v9.0.0 label Dec 23, 2024

semd reviewed Dec 23, 2024

View reviewed changes

albertoblaz mentioned this pull request Dec 24, 2024

Add New Security Navigation Menu "Inventory" #201706

Closed

5 tasks

albertoblaz deleted the asset-inv-nav-menu branch December 30, 2024 11:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'inventory' item to Security navigation menu #204373

Add 'inventory' item to Security navigation menu #204373

albertoblaz commented Dec 16, 2024 •

edited

Loading

elasticmachine commented Dec 17, 2024

opauloh left a comment

kapral18 left a comment

albertoblaz commented Dec 23, 2024

angorayc Dec 23, 2024 •

edited

Loading

albertoblaz Dec 23, 2024

angorayc Dec 23, 2024

albertoblaz Dec 23, 2024

semd Dec 23, 2024

albertoblaz Dec 23, 2024

angorayc Dec 23, 2024 •

edited

Loading

albertoblaz Dec 23, 2024

elasticmachine commented Dec 23, 2024

API count

References to deprecated APIs

semd Dec 23, 2024

albertoblaz Dec 23, 2024

semd Dec 23, 2024

Add 'inventory' item to Security navigation menu #204373

Add 'inventory' item to Security navigation menu #204373

Conversation

albertoblaz commented Dec 16, 2024 • edited Loading

Summary

Changeset details

Out of scope

How to test

Screenshots

Checklist

Risks

elasticmachine commented Dec 17, 2024

opauloh left a comment

Choose a reason for hiding this comment

kapral18 left a comment

Choose a reason for hiding this comment

albertoblaz commented Dec 23, 2024

angorayc Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angorayc Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Dec 23, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

References to deprecated APIs

History

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albertoblaz commented Dec 16, 2024 •

edited

Loading

angorayc Dec 23, 2024 •

edited

Loading

angorayc Dec 23, 2024 •

edited

Loading