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.

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

Merged

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Jun 21, 2024

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

POST kbn:/api/fleet/epm/packages/{pkgName}/{pkgVersion}/kibana_assets

I also introduce an API to be able to delete those newly created assets.

DELETE kbn:/api/fleet/epm/packages/{pkgName}/{pkgVersion}/kibana_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

{
  additional_spaces_installed_kibana?: {
   [spaceId: string]: KibanaAssetsReference[]
   }
}

I added a new

UI Changes

With nginx installed in the default space:

Screenshot 2024-06-21 at 10 05 19 AM

In the test space:

Assets not installed
Screenshot 2024-06-26 at 8 32 21 AM

Assets installed
Screenshot 2024-06-21 at 10 06 45 AM

Open questions/Issues found during implementation

  • Delete API it is needed? I think it could help for future supportability.
  • IDs in markdown visualization are not working => should we rewrite them?

Todo

  • Put those new APIs/UIs behind a feature flag
  • API integration tests
  • Create a follow issue for markdown in dashboard IDs rewrite POC.

@nchaulet nchaulet added release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team labels Jun 21, 2024
@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@nchaulet
Copy link
Member Author

@kpollich Before I start writing test and polishing that one I would love to get your initial thought on the implemented behavior

@kpollich
Copy link
Member

Delete API it is needed? I think it could help for future supportability.

+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.

IDs in markdown visualization are not working => should we rewrite them?

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.

@nchaulet nchaulet force-pushed the feature-kibana-install-differents-space branch from 60adb25 to 80320de Compare June 21, 2024 18:13
@nchaulet
Copy link
Member Author

/ci

@nchaulet
Copy link
Member Author

/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<
Copy link
Member Author

@nchaulet nchaulet Jun 21, 2024

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

kibanamachine and others added 3 commits June 21, 2024 19:57
… src/core/server/integration_tests/ci_checks'
…nchaulet/kibana into feature-kibana-install-differents-space
@nchaulet
Copy link
Member Author

/ci

@drewdaemon
Copy link
Contributor

@kpollich

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?

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.

@nchaulet nchaulet marked this pull request as ready for review June 25, 2024 13:46
@nchaulet nchaulet requested review from a team as code owners June 25, 2024 13:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@@ -291,6 +291,7 @@
],
"enterprise_search_telemetry": [],
"epm-packages": [
"additionnal_spaces_installed_kibana",
Copy link
Member

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 😇

Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

Additive mappings LGTM

@jen-huang
Copy link
Contributor

UX suggestion:

For the case where assets are not yet installed in this space, why don't we offer the Install Kibana assets button directly in this callout?:

image

This way, we can remove it from the Settings tab. The current addition to the settings tab is confusing IMO with the existing reinstall option.

Btw, what happens if the user chooses to Uninstall or Reinstall while in the secondary space? Will all assets across all spaces be un/re-installed?

@nchaulet
Copy link
Member Author

For the case where assets are not yet installed in this space, why don't we offer the Install Kibana assets button directly in this callout?:

Yes that's a great idea, and will simplify the settings page

Btw, what happens if the user chooses to Uninstall or Reinstall while in the secondary space? Will all assets across all spaces be un/re-installed?

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)

@nchaulet
Copy link
Member Author

@jen-huang Just updated my PR to have the install button in the assets page, makes a lot more sense

Screenshot 2024-06-26 at 8 32 21 AM

@jen-huang jen-huang self-requested a review June 26, 2024 16:33
Copy link
Contributor

@jen-huang jen-huang left a 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.

@@ -231,6 +241,13 @@ export const AssetsPage = ({ packageInfo, refetchPackageInfo }: AssetsPanelProps
}}
Copy link
Contributor

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

Copy link
Member Author

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.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #42 / Actions and Triggers app Stack alerts page Loads the page Disables the SIEM solution filter when any other is applied

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 1036 1037 +1

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
fleet 1219 1226 +7

Async chunks

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

id before after diff
fleet 1.4MB 1.4MB +1.4KB

Page load bundle

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

id before after diff
fleet 165.9KB 166.5KB +623.0B
Unknown metric groups

API count

id before after diff
fleet 1341 1348 +7

History

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

@nchaulet nchaulet merged commit ace2996 into elastic:main Jun 26, 2024
24 checks passed
@nchaulet nchaulet deleted the feature-kibana-install-differents-space branch June 26, 2024 20:12
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fleet] Fleet should allow an integration to be installed/reinstalled into another space
9 participants