[Fleet] When removing a inputs type package policy, clean up assets by criamico · Pull Request #218582 · elastic/kibana

criamico · 2025-04-17T15:44:45Z

Fixes #209789

Summary

For inputs type integrations (docs), we install all the assets when creating the integration policy and not at integration install time (like for "regular": integrations).

However the clean up of assets doesn't happen when removing the integration policy and this leaves around orphaned assets that are not working anymore.

This PR adds a new endpoint that removes the datastream assets

DELETE kbn:/api/fleet/epm/packages/{pkgName}/{pkgVersion}/datastream_assets?packagePolicyId={Id}

The new endpoint is called by the UI when removing the integration policy;
Only the datastream assets that match exactly the dataset name are removed; assets that are common across the integration are kept and can only be deleted when the whole integration is uninstalled.

Additional changes:

I did some light refactoring of the functions for inputs-type integrations to make the code more readable
Updated the dev_docs about input-type integrations that haven't been touched for long time

Testing

Install an input-type package, for instance "custom logs"
Check the assets created under the tab assets
Check that the package has only this integration policy
Remove the integration policy for the package - a warning is shown:

Verify that the assets related to the package are cleaned up as well
Try again but with several integration policies
In this case the clean up doesn't happen

Delete assets when there are two integration policies with different dataset names

Dataset names areudp.generic and udp.test - in the video I deleted policy udp-2 having dataset name udp.test and the relative assets are no longer present:

Screen.Recording.2025-05-14.at.16.29.26.mov

Delete assets when there are two integration policies with same dataset names

In this case there are two different policies having the same dataset name udp.generic, when deleting the policy there is no warning to remove the assets. In fact they can be deleted only when there is only one remaining policy using them:

Screen.Recording.2025-05-14.at.16.36.47.mov

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

criamico · 2025-04-18T09:38:47Z

x-pack/platform/plugins/shared/fleet/server/services/epm/packages/install.ts

  return installingPackages;
 }

-export async function installAssetsForInputPackagePolicy(opts: {


I moved this function to a new file that groups the utilities needed for input-type packages

criamico · 2025-04-18T14:18:29Z

@elasticmachine merge upstream

elasticmachine · 2025-04-18T14:30:41Z

Pinging @elastic/fleet (Team:Fleet)

x-pack/platform/plugins/shared/fleet/server/services/epm/packages/input_type_packages.ts

criamico · 2025-04-28T12:15:24Z

@elasticmachine merge upstream

nchaulet · 2025-04-28T18:03:14Z

x-pack/platform/plugins/shared/fleet/server/services/epm/packages/input_type_packages.ts

+      if (!installation) {
+        throw new FleetError(`${packageInfo.name} is not installed`);
+      }
+      await cleanupAssets(installation, esClient, soClient);


I think here we probably need a little more logic, it seems to me we only want to delete the assets for a the specific datastream defined by that package policy. An input integration could have been used for multiple package policy, also we should check there are no other package policy for that integration defining the same package policy. wdyt?

we should check there are no other package policy for that integration defining the same package policy

Yes @jillguyonnet added a check in here: https://github.com/criamico/kibana/blob/1db6c23088f2f45fc5c9ed227fc369fe02b6ec4f/x-pack/platform/plugins/shared/fleet/server/services/package_policy.ts#L1686

The current logic is removing all package assets if we are removing the last policy for this package, so assuming 2 package policies system-1 and system-2:

remove system-1 -> assets are left untouched

remove system-2 (last policy) -> all assets are removed

Note that the package itself isn't uninstalled.

it seems to me we only want to delete the assets for a the specific datastream defined by that package policy

Is there a scenario where creating a package policy can create assets that are only relevant to this policy? e.g. with a custom dataset? If that's the case, then maybe we want additional logic to the above to clean up these specific assets, something like:

remove system-1 -> only assets specific to system-1 are removed

remove system-2 (last policy) -> all assets are removed

WDYT?

Thanks @jillguyonnet, I checked and the second scenario you described seems correct.

I'll remove only the assets related to the package policy dataset name, according with @nchaulet comment:

it seems to me we only want to delete the assets for a the specific datastream defined by that package policy

Is there a scenario where creating a package policy can create assets that are only relevant to this policy? e.g. with a custom dataset? If that's the case, then maybe we want additional logic to the above to clean up these specific assets, something like:

Yes this is the case with input package policies (they are a little confusing to understand), for those policies we create if they do not exists the index template, pipeline for the provided dataset.

criamico · 2025-05-23T10:17:26Z

@elasticmachine merge upstream

criamico · 2025-05-26T13:52:36Z

@elasticmachine merge upstream

criamico · 2025-05-27T09:02:58Z

@elasticmachine merge upstream

criamico · 2025-05-27T13:31:08Z

x-pack/platform/plugins/shared/fleet/server/services/epm/packages/input_type_packages.ts

+  const dataStreamType =
+    packagePolicy.inputs[0].streams[0].vars?.[DATA_STREAM_TYPE_VAR_NAME]?.value ||
+    packagePolicy.inputs[0].streams[0].data_stream?.type ||
+    'logs';


I moved here the changes from #214216

jillguyonnet · 2025-05-28T15:20:30Z

This might not be related to this change, but I'm trying with Custom UDP Logs and it looks it adds 2 asset references that don't actually exist and then are left hanging when the package policy is removed. Is that known?

Before adding the integration:

After adding it:

logs@custom and udp@custom component templates don't exist:

After deleting the package policy:

nchaulet · 2025-05-28T15:38:36Z

...t/public/applications/integrations/sections/epm/screens/detail/policies/package_policies.tsx

+    setAgentBasedPackageAndAgentPolicies(mappedPoliciesData);
  }, [agentBasedData, mapPoliciesData]);

+  const isInputPackageDatasetUsedByMultiplePolicies = useCallback(


I think this will probably not work with space awareness enabled, as the dataset could be used in policies in a different space, I think here one solution could be to put the responsability on the user and ask him if he want to delete the related assets and explain if other policies use that template it will be problematic wdyt?

otherwise we could do that check in the API and throw if there is pacakge policy using that dataset and the call is done without a force flag

Probably having that check in the API is better, and we can handle this nicely in the UI, with a warning assets for that package policy has not been deleted as they are used by other package policy ...

Is the plan to have space awareness enabled for all users in the long run, or will it remain an optional feature? Depending on this we might pick different options.

Probably having that check in the API is better, and we can handle this nicely in the UI, with a warning assets for that package policy has not been deleted as they are used by other package policy ...

it looks like a sensible way to handle it. I'll check this solution and see if there are any caveats.

I just did a test with two spaces where I added Custom UDP Logs both times with udp.generic dataset. When I removed the package policy in the second space, it deleted the assets. So I think you're right, the logic needs to be space aware. I agree with the solution of making the API space aware and warning the user that assets were not deleted because they are used by another package policy in another space (which mightn't be obvious).

Is the plan to have space awareness enabled for all users in the long run, or will it remain an optional feature? Depending on this we might pick different options.

Yes space aware will be the default starting from 9.1

criamico · 2025-05-28T15:46:18Z

Custom UDP Logs adds 2 asset references that don't actually exist and then are left hanging when the package policy is removed

@jillguyonnet so the fact that the two assets are left there should be correct, as they don't match the dataset name; however I'm not sure why those two assets don't exist, I noticed it but forgot to open a ticket for it.
I'm going to open it in the integrations repo.

nchaulet

code LGTM 🚀

criamico · 2025-05-29T14:52:59Z

@elasticmachine merge upstream

criamico · 2025-05-29T14:56:18Z

x-pack/platform/plugins/shared/fleet/server/routes/epm/package_datastream_assets_handler.ts

+    const { items: allPackagePolicies } = await packagePolicyService.list(allSpacesSoClient, {
+      kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name:${pkgName}`,
+      spaceId: '*',
+    });


@nchaulet I moved the logic to find if the dataset is used by other policies directly in the endpoint and I made it space aware; it will fail directly in the api response, so I removed almost all the logic from the UI.

elasticmachine · 2025-05-29T16:33:38Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: c835c0e

Failed CI Steps

Test Failures

[job] [logs] Fleet Cypress Tests #2 / View agents list Agent status filter should filter on healthy and unhealthy
[job] [logs] Fleet Cypress Tests #2 / View agents list Bulk actions should allow to bulk upgrade agents and cancel that upgrade

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
`fleet`	1418	1424	+6

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.7MB	1.7MB	+735.0B

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.0KB	165.4KB	+381.0B

Unknown metric groups

API count

id	before	after	diff
`fleet`	1547	1553	+6

History

💔 Build #303412 failed 8bba3d0
💛 Build #302577 was flaky 9b02913
💛 Build #302409 was flaky faea016
💛 Build #302088 was flaky 68044fd
💔 Build #301997 failed b531c9e
💔 Build #301401 failed e9f683e

cc @criamico @jillguyonnet

…lastic#218582) Fixes elastic#209789 ## Summary For `inputs` type integrations ([docs](https://github.com/elastic/kibana/blob/main/x-pack/platform/plugins/shared/fleet/dev_docs/input_packages.md)), we install all the assets when creating the integration policy and not at integration install time (like for "regular": integrations). However the clean up of assets doesn't happen when removing the integration policy and this leaves around orphaned assets that are not working anymore. - This PR adds a new endpoint that removes the datastream assets ``` DELETE kbn:/api/fleet/epm/packages/{pkgName}/{pkgVersion}/datastream_assets?packagePolicyId={Id} ``` - The new endpoint is called by the UI when removing the integration policy; - Only the datastream assets that match exactly the dataset name are removed; assets that are common across the integration are kept and can only be deleted when the whole integration is uninstalled. Additional changes: - I did some light refactoring of the functions for inputs-type integrations to make the code more readable - Updated the dev_docs about input-type integrations that haven't been touched for long time ### Testing - Install an input-type package, for instance "custom logs" - Check the assets created under the tab `assets` - Check that the package has only this integration policy - Remove the integration policy for the package - a warning is shown: <img width="937" alt="Screenshot 2025-05-09 at 16 58 51" src="https://github.com/user-attachments/assets/0f86ab38-e0a9-47f5-91f5-71b83e17f2e3" /> - Verify that the assets related to the package are cleaned up as well - Try again but with several integration policies - In this case the clean up doesn't happen ### Delete assets when there are two integration policies with different dataset names Dataset names are`udp.generic` and `udp.test` - in the video I deleted policy `udp-2` having dataset name `udp.test` and the relative assets are no longer present: https://github.com/user-attachments/assets/23350051-1b26-4e52-914d-62f784809c80 ### Delete assets when there are two integration policies with same dataset names In this case there are two different policies having the same dataset name `udp.generic`, when deleting the policy there is no warning to remove the assets. In fact they can be deleted only when there is only one remaining policy using them: https://github.com/user-attachments/assets/f75668dd-a4ce-4f5a-ba5d-c99911278dfc ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: jillguyonnet <jill.guyonnet@gmail.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

criamico added 2 commits April 17, 2025 17:30

[Fleet] When removing a inputs type package policy, clean up assets

ae2c7ae

Split tests

5d4af8d

criamico self-assigned this Apr 17, 2025

criamico added the Team:Fleet Team label for Observability Data Collection Fleet team label Apr 17, 2025

criamico added 2 commits April 18, 2025 10:44

Expand dev doc about input packages

9eedc05

Add unit tests

6525a2e

criamico commented Apr 18, 2025

View reviewed changes

Add constant instead of magic number

288def9

criamico added v9.0.1 release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Apr 18, 2025

Merge branch 'main' into 209789_inputs_package_assets_cleanup

7f31853

criamico marked this pull request as ready for review April 18, 2025 14:30

criamico requested a review from a team as a code owner April 18, 2025 14:30

fix missing await

0c568ef

nchaulet reviewed Apr 22, 2025

View reviewed changes

x-pack/platform/plugins/shared/fleet/server/services/epm/packages/input_type_packages.ts Outdated Show resolved Hide resolved

jillguyonnet self-assigned this Apr 23, 2025

Remove assets if last policy for package

426fc36

jillguyonnet marked this pull request as draft April 25, 2025 09:51

elasticmachine and others added 4 commits April 28, 2025 14:15

Merge branch 'main' into 209789_inputs_package_assets_cleanup

11e0d0a

update tests

0f6df81

Add test to packagePolicyService

722c3b8

Add unit test

1db6c23

criamico marked this pull request as ready for review April 28, 2025 16:00

nchaulet reviewed Apr 28, 2025

View reviewed changes

criamico marked this pull request as draft April 30, 2025 07:49

Create new endpoint and move logic to handler

cf5492d

elasticmachine and others added 2 commits May 23, 2025 12:17

Merge branch 'main' into 209789_inputs_package_assets_cleanup

b531c9e

fix runtime error

68044fd

Merge branch 'main' into 209789_inputs_package_assets_cleanup

faea016

criamico requested a review from nchaulet May 26, 2025 14:30

criamico added 2 commits May 27, 2025 10:41

Merge main and fix conflicts with 214216

75f5b2d

apply changes from 214216

417f6ee

Merge branch 'main' into 209789_inputs_package_assets_cleanup

9b02913

criamico commented May 27, 2025

View reviewed changes

nchaulet reviewed May 28, 2025

View reviewed changes

Move all logic to backend per code review and fix tests

8bba3d0

nchaulet approved these changes May 29, 2025

View reviewed changes

add unit test

e68fb62

Merge branch 'main' into 209789_inputs_package_assets_cleanup

147e0a8

criamico commented May 29, 2025

View reviewed changes

Revert changes not needed anymore

c835c0e

criamico merged commit 0ba35cb into elastic:main May 30, 2025
10 checks passed

kibanamachine added the v9.1.0 label May 30, 2025

kibanamachine mentioned this pull request Jun 3, 2025

Update AssetCriticalityBadge color map to EUI Borealis #222024

Merged

7 tasks

kibanamachine mentioned this pull request Jun 25, 2025

[Fleet] Replace call to registry when deleting kibana assets for custom packages #224886

Merged

1 task

kibanamachine mentioned this pull request Jul 28, 2025

[Security Solution] Improve bulk actions API reference docs #228712

Merged

criamico mentioned this pull request Sep 23, 2025

[Fleet] Leftover component templates remain when uninstalling Otel input packages' integration policies #236047

Closed

Conversation

criamico commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Delete assets when there are two integration policies with different dataset names

Delete assets when there are two integration policies with same dataset names

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criamico commented Apr 18, 2025

Uh oh!

elasticmachine commented Apr 18, 2025

Uh oh!

Uh oh!

criamico commented Apr 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criamico commented May 23, 2025

Uh oh!

criamico commented May 26, 2025

Uh oh!

criamico commented May 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jillguyonnet commented May 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

criamico commented May 28, 2025

Uh oh!

nchaulet left a comment

Choose a reason for hiding this comment

Uh oh!

criamico commented May 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented May 29, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

criamico commented Apr 17, 2025 •

edited

Loading