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.

Sign up for GitHub

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

Jump to bottom

Implement feature flag to toggle Asset Inventory plugin #203844

Merged
merged 1 commit into from
Dec 17, 2024
Merged

Implement feature flag to toggle Asset Inventory plugin #203844

merged 1 commit into from
Dec 17, 2024
+13 −1

Conversation

albertoblaz
Copy link
Contributor

@albertoblaz albertoblaz commented Dec 11, 2024 •
edited
Loading

Summary

Closes #201705.

Reuse security_solution's config to control visibility of Asset Inventory plugin.

Definition of done

  • Use the existing xpack.securitySolution.enableExperimental configuration to add a new feature flag
    • Done in x-pack/plugins/security_solution/common/experimental_features.ts
  • Implement the feature flag xpack.securitySolution.enableExperimental: ['assetInventoryStoreEnabled'].
    • This can only be done locally in kibana.dev.yml, but I can add it to kibana.yml instead.
  • Update any relevant documentation to explain how to enable the feature.
    • Updated plugin's README file.

How to test

In your kibana.yml file, add the following entry at the bottom:

xpack.securitySolution.enableExperimental: ['assetInventoryStoreEnabled']

Checklist

  • Documentation was added for features that require explanation or tutorials
  • 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

@albertoblaz albertoblaz added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Dec 11, 2024
@albertoblaz albertoblaz self-assigned this Dec 11, 2024
@albertoblaz albertoblaz added v9.0.0 backport:version Backport to applied version labels v8.18.0 labels Dec 11, 2024
@albertoblaz albertoblaz marked this pull request as ready for review December 11, 2024 17:47
@albertoblaz albertoblaz requested review from a team as code owners December 11, 2024 17:47
@elasticmachine
Copy link
Contributor

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

@opauloh
Copy link
Contributor

opauloh commented Dec 13, 2024

Hi @albertoblaz,

Following up on our discussion today on the Asset Inventory sync, we’ve reviewed the implementation of the Asset Inventory Feature Flags. Initially, we planned to have a separate Feature Flag (FF) specifically for toggling the Asset Inventory navigation (assetInventoryEnabled). However, since this is closely interconnected with the Universal Entity engine FF ( assetInventoryStoreEnabled), we’ve decided that maintaining two separate Feature Flags (one for the universal entity engine and another for the asset inventory page) may be redundant as both Feature Flags woud be required for the Asset Inventory feature.

@tiansivive would it still make sense to leverage this PR to update it to include the assetInventoryStoreEnabled instead of assetInventoryEnabled? By merging the Feature Flag in a separate PR it would help unblocking others UX Asset Inventory tasks while the Universal Entity engine PR waits for the review.

@tiansivive
Copy link
Contributor

@opauloh
If I understand correctly, you want to change the flag in this PR to the one I introduced in mine?
I think that makes sense! I'm hoping my PR gets merged today, it's already been (mostly) reviewed so 🤞🏽

@opauloh
Copy link
Contributor

opauloh commented Dec 13, 2024 •
edited
Loading

@opauloh If I understand correctly, you want to change the flag in this PR to the one I introduced in mine? I think that makes sense! I'm hoping my PR gets merged today, it's already been (mostly) reviewed so 🤞🏽

Yes, my suggestion was to change this PR to point to your feature flag assetInventoryStoreEnabled

@opauloh opauloh self-requested a review December 13, 2024 16:47
@albertoblaz albertoblaz force-pushed the asset-inv-feature-flag branch from d1f2b2d to 3cc8559 Compare December 16, 2024 12:17
@albertoblaz
Copy link
Contributor Author

@opauloh @tiansivive Changed from assetInventoryEnabled -> assetInventoryStoreEnabled as per your suggestions 👍

@albertoblaz albertoblaz force-pushed the asset-inv-feature-flag branch from 3cc8559 to 459464a Compare December 17, 2024 09:11
@albertoblaz albertoblaz added backport:skip This commit does not require backporting and removed v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) backport:version Backport to applied version labels v8.18.0 labels Dec 17, 2024
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
securitySolution 88.0KB 88.0KB +30.0B

History

cc @albertoblaz

@albertoblaz albertoblaz merged commit 15dc9c7 into elastic:main Dec 17, 2024
14 checks passed
@albertoblaz albertoblaz deleted the asset-inv-feature-flag branch December 17, 2024 18:39
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
@albertoblaz @JoseLuisGJ
Implement feature flag to toggle Asset Inventory plugin (elastic#203844)
0a4f362 
## Summary

Closes elastic#201705.

Reuse security_solution's config to control visibility of Asset
Inventory plugin.

### Definition of done

- [x] Use the existing `xpack.securitySolution.enableExperimental`
configuration to add a new feature flag
- Done in
`x-pack/plugins/security_solution/common/experimental_features.ts`
- [x] Implement the feature flag
`xpack.securitySolution.enableExperimental:
['assetInventoryStoreEnabled']`.
- This can only be done locally in `kibana.dev.yml`, but I can add it to
`kibana.yml` instead.
- [x] Update any relevant documentation to explain how to enable the
feature.
  - Updated plugin's `README` file.

### How to test

In your
[kibana.yml](https://github.com/elastic/kibana/blob/main/config/kibana.yml)
file, add the following entry at the bottom:

```
xpack.securitySolution.enableExperimental: ['assetInventoryStoreEnabled']
```

### Checklist

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JordanSh JordanSh JordanSh approved these changes

@opauloh opauloh opauloh approved these changes

Assignees

@albertoblaz albertoblaz

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement Feature Flag for Enabling/Disabling the Plugin
6 participants
@albertoblaz @elasticmachine @opauloh @tiansivive @JordanSh @kibanamachine