-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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.
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
[Fleet] Allow to install Kibana assets in multiple spaces #186620
[Fleet] Allow to install Kibana assets in multiple spaces #186620
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
@kpollich Before I start writing test and polishing that one I would love to get your initial thought on the implemented behavior |
+1 I think we should have an API for deleting these assets. It will almost certainly come up in SDH's as a troubleshooting step.
If it's possible to rewrite the ID's on the fly while imported the visualization, that could be an intriguing option. This would also help with mitigating #176552, and could be a workaround for that issue specific to Fleet-managed assets. @drewdaemon I wonder if you have thoughts on this. Does it make sense for Fleet to try and rewrite ID's within these markdown panels on-the-fly during the package installation process, or does that sound like a concerning workflow? I could see it being brittle or expensive to parse large dashboard JSON objects and running a regex or something to do string replacement for the proper ID's. |
60adb25
to
80320de
Compare
/ci |
/ci |
* A list of allowed values that can be used in `xpack.fleet.enableExperimental`. | ||
* This object is then used to validate and parse the value entered. | ||
*/ | ||
export const allowedExperimentalValues = Object.freeze< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really related to that feature, but it allow to add some type safety to allowedExperimentalValues
, and fix the autocompletion that was bothering me
… src/core/server/integration_tests/ci_checks'
…nchaulet/kibana into feature-kibana-install-differents-space
/ci |
It could be worth a PoC. I have performed this operation manually before to help particular customers. It could be safe actually because we're talking about find-and-replace on UUIDs. |
…install-differents-space
Pinging @elastic/fleet (Team:Fleet) |
@@ -291,6 +291,7 @@ | |||
], | |||
"enterprise_search_telemetry": [], | |||
"epm-packages": [ | |||
"additionnal_spaces_installed_kibana", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: additional
has one n
😇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additive mappings LGTM
… src/core/server/integration_tests/ci_checks'
UX suggestion: For the case where assets are not yet installed in this space, why don't we offer the This way, we can remove it from the Btw, what happens if the user chooses to |
Yes that's a great idea, and will simplify the settings page
Yes uninstall will uninstall the package and his assets in all space (if no integration policy use it), and reinstall will reinstall assets across all spaces (similar to an update) |
@jen-huang Just updated my PR to have the install button in the assets page, makes a lot more sense |
…install-differents-space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small copy fixes, otherwise LGTM. Tested locally.
...pplications/integrations/sections/epm/screens/detail/assets/install_kibana_assets_button.tsx
Outdated
Show resolved
Hide resolved
...pplications/integrations/sections/epm/screens/detail/assets/install_kibana_assets_button.tsx
Outdated
Show resolved
Hide resolved
@@ -231,6 +241,13 @@ export const AssetsPage = ({ packageInfo, refetchPackageInfo }: AssetsPanelProps | |||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there already a docs issue to update https://www.elastic.co/guide/en/fleet/current/install-uninstall-integration-assets.html once this feature is enabled by default? if not, let's go ahead and create a placeholder issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No there is not, I will create one for the whole space awareness project as there is probably more than this to document.
Co-authored-by: Jen Huang <its.jenetic@gmail.com>
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Resolve #172963
The package saved object is agnostic, but some of the installed assets (Kibana one are space aware), because of this we currently only allow to install kibana assets in one space.
That PR allow to install kibana assets in multiple spaces by introducing a new API
I also introduce an API to be able to delete those newly created assets.
Instead of introducing a new property to the package saved object, we could eventually introduce a new saved object space aware
installation_kibana_assets
that contains just the kibana assets, but this introduce more complexity.On package reinstall/upgrade it will upgrade assets through all spaces
The feature is behind a feature flag
useSpaceAwareness
Implementation details
Saved object change
I added a new property
additional_spaces_installed_kibana
on the installation saved object to keep track of the installed assets in different spaces:It looks like this
I added a new
UI Changes
With nginx installed in the default space:
In the test space:
Assets not installed
Assets installed
Open questions/Issues found during implementation
Todo