Skip to content

Enable Asset Inventory page via Kibana Advanced Settings#211884

Merged
albertoblaz merged 8 commits intoelastic:mainfrom
albertoblaz:asset-inv-settings
Feb 28, 2025
Merged

Enable Asset Inventory page via Kibana Advanced Settings#211884
albertoblaz merged 8 commits intoelastic:mainfrom
albertoblaz:asset-inv-settings

Conversation

@albertoblaz
Copy link
Contributor

@albertoblaz albertoblaz commented Feb 20, 2025

Summary

Closes https://github.com/elastic/security-team/issues/11683.

Adds an advanced setting to enable/disable the Asset Inventory page. Replaces the old assetInventoryUXEnabled feature flag. The placement of the setting is right below "Enable graph visualization", within the Security Solution group.

Screenshots

Setting off Screenshot 2025-02-21 at 09 38 43
Setting on Screenshot 2025-02-21 at 09 38 55
Overriden setting - activated via kibana.dev.yml Screenshot 2025-02-21 at 09 38 14

How to test

Follow the instructions provided in the README.md file committed in this PR.

Definition of Done

  • Advanced Settings Integration

    • Add a new setting under Kibana Advanced Settings for enabling the Asset Inventory feature:
      • Setting Name: Enable Asset Inventory
      • Setting Key: securitySolution:enableAssetInventory
      • Description: "Enable the Asset Inventory feature to view and manage assets in the Security Solution plugin."
      • Type: Toggle (On/Off).
      • Default Value: Off.
    • Ensure the setting reflects the current status of the Asset Inventory feature (On/Off).
    • Group the setting logically under the Security Solution in the Kibana Advanced Settings page.
    • Ensure the toggle is discoverable and adheres to Kibana’s design guidelines.
  • Implementation

    • Update the Asset Inventory initialization logic to check the new Kibana setting (securitySolution:enableAssetInventory) instead of relying on the assetInventoryUXEnabled feature flag in kibana.dev.yml. For now we don't need to worry about initialization
    • Provide backward compatibility by allowing the kibana.dev.yml flag (xpack.securitySolution.assetInventoryUXEnabled) to override the setting in development environments.
    • The toggle should dynamically enable or disable the Asset Inventory feature without requiring a Kibana restart.
  • Testing

    • Add unit tests to verify:
      • The toggle updates the setting value correctly.
      • The Asset Inventory feature respects the toggle status (enabled/disabled).
    • Add functional tests to validate the toggle’s behavior in the Advanced Settings page.
  • Documentation

    • Update the documentation to explain how to enable/disable the Asset Inventory feature using Kibana Advanced Settings.
    • Provide details about the fallback behavior when using the kibana.dev.yml flag.

Checklist

  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Feature will get exposed to end users if combination of setting and feature flag is not set up correctly.

@albertoblaz albertoblaz added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Cloud Security Cloud Security team related labels Feb 20, 2025
@albertoblaz albertoblaz self-assigned this Feb 20, 2025
@albertoblaz albertoblaz force-pushed the asset-inv-settings branch 10 times, most recently from 63b9fd1 to 9aa0ed0 Compare February 21, 2025 17:36
@albertoblaz albertoblaz marked this pull request as ready for review February 21, 2025 17:36
@albertoblaz albertoblaz requested review from a team as code owners February 21, 2025 17:36
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

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

Looks good to me, just some text changes

@albertoblaz
Copy link
Contributor Author

@opauloh Updated copy as per suggested :)

@albertoblaz albertoblaz enabled auto-merge (squash) February 24, 2025 15:30
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is now an advanced setting, instead of forcing the environment to have the value by default is better to enable it first in the before hook, you can check how to do it in: x-pack/test/security_solution_cypress/cypress/tasks/api_calls/kibana_advanced_settings.ts

Is this functionality going to be available in serverless as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this functionality going to be available in serverless as well?

Hi @MadameSheema, yes the idea of shifting from Kibana Feature Flag to Advanced Settings is to have Asset Inventory available on serverless as well, it does seem that enabling the advanced settings from API would be better in that case as it would allow us to test both environments by removing the @ess tag .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@opauloh @MadameSheema I replaced the setting override with the API calls, which is indeed much better :)

However, removing { tags: ['@ess'] } from the describe block did not work and tests were skipped, so I kept it and added '@serverless' to the list. With that, tests seem to run and pass

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @opauloh @albertoblaz lots of thanks for the clarification and the changes :)

You can learn more about how tags work for Cypress Security Solution tests here.

Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

kbn management changes lgtm

Copy link
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

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

Code review only. Shared UX changes LGTM

Copy link
Contributor

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

code changes LGTM (telemetry, and overall file changes)

@albertoblaz albertoblaz force-pushed the asset-inv-settings branch 2 times, most recently from 75211c3 to 5d4ecfd Compare February 28, 2025 15:16
@albertoblaz albertoblaz requested review from a team as code owners February 28, 2025 15:16
@albertoblaz albertoblaz requested a review from rylnd February 28, 2025 15:16
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

LGTM for the Threat Hunting Investigations team

Copy link
Contributor

@MadameSheema MadameSheema left a comment

Choose a reason for hiding this comment

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

Security Engineering Productivity changes LGTM!

@albertoblaz albertoblaz requested a review from opauloh February 28, 2025 16:03
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

Detection Engine changes LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

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/management-settings-ids 135 136 +1

Async chunks

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

id before after diff
securitySolution 8.9MB 8.9MB -18.0B

Page load bundle

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

id before after diff
securitySolution 86.0KB 86.0KB +27.0B
Unknown metric groups

API count

id before after diff
@kbn/management-settings-ids 136 137 +1

History

  • 💛 Build #278887 was flaky aea3790caa6781bb7178e6a26bd98b6afba30849
  • 💚 Build #278588 succeeded 67ccfe254c88257aed38a86044f68c61bb51f668
  • 💔 Build #278536 failed 221eca6f9cfc6c5b6d639d408fac1087fb90bc17
  • 💔 Build #278473 failed 5dc2a8a07d4be879b3f5a3648ead264227e7fba4
  • 💔 Build #278452 failed 8f1c7b9029f6eb46e67dd13928139231ed51b4ad

cc @albertoblaz

Copy link
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

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

💯

@albertoblaz albertoblaz merged commit 0a56262 into elastic:main Feb 28, 2025
9 checks passed
@albertoblaz albertoblaz deleted the asset-inv-settings branch February 28, 2025 18:12
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
)

## Summary

Closes elastic/security-team#11683.

Adds an advanced setting to enable/disable the Asset Inventory page.
Replaces the old `assetInventoryUXEnabled` feature flag. The placement
of the setting is right below "Enable graph visualization", within the
Security Solution group.

### Screenshots

<details><summary>Setting off</summary>
<img width="943" alt="Screenshot 2025-02-21 at 09 38 43"
src="https://github.com/user-attachments/assets/c3b561cd-7dfa-4218-9004-cc89c5768551"
/>
</details>

<details><summary>Setting on</summary>
<img width="735" alt="Screenshot 2025-02-21 at 09 38 55"
src="https://github.com/user-attachments/assets/7a9ebf17-9339-49f2-820e-e26087f1c17c"
/>
</details>

<details><summary>Overriden setting - activated via
kibana.dev.yml</summary>
<img width="943" alt="Screenshot 2025-02-21 at 09 38 14"
src="https://github.com/user-attachments/assets/6ebb1e73-cffb-4bfd-ab21-631955574ce1"
/>
</details>

### How to test

Follow the instructions provided in the *README.md* file committed in
this PR.

### Definition of Done

- **Advanced Settings Integration**
- [x] Add a new setting under **Kibana Advanced Settings** for enabling
the Asset Inventory feature:
     - **Setting Name**: `Enable Asset Inventory`
     - **Setting Key**: `securitySolution:enableAssetInventory`
- **Description**: "Enable the Asset Inventory feature to view and
manage assets in the Security Solution plugin."
     - **Type**: Toggle (On/Off).
     - **Default Value**: Off.
- [x] Ensure the setting reflects the current status of the Asset
Inventory feature (On/Off).
- [x] Group the setting logically under the **Security Solution** in the
Kibana Advanced Settings page.
- [x] Ensure the toggle is discoverable and adheres to Kibana’s design
guidelines.

- **Implementation**
- [ ] ~~Update the `Asset Inventory` initialization logic to check the
new Kibana setting (`securitySolution:enableAssetInventory`) instead of
relying on the `assetInventoryUXEnabled` feature flag in
`kibana.dev.yml`.~~ For now we don't need to worry about initialization
- [ ] ~~Provide backward compatibility by allowing the `kibana.dev.yml`
flag (`xpack.securitySolution.assetInventoryUXEnabled`) to override the
setting in development environments.~~
- [x] The toggle should dynamically enable or disable the Asset
Inventory feature without requiring a Kibana restart.

- **Testing**
   - [x] Add unit tests to verify:
     - The toggle updates the setting value correctly.
- The Asset Inventory feature respects the toggle status
(enabled/disabled).
- [x] Add functional tests to validate the toggle’s behavior in the
Advanced Settings page.

- **Documentation**
- [x] Update the documentation to explain how to enable/disable the
Asset Inventory feature using Kibana Advanced Settings.
- [ ] ~~Provide details about the fallback behavior when using the
`kibana.dev.yml` flag.~~

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [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] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [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)

### Identify risks

Feature will get exposed to end users if combination of setting and
feature flag is not set up correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants