[8.6] Catching saved object not found error, do not delete referenced package assets (#146274) (#146411)

# Backport This will backport the following commits from `main` to `8.6`: - [Catching saved object not found error, do not delete referenced package assets (#146274)](https://github.com/elastic/kibana/pull/146274)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com>
2025-04-24 09:48:58 -04:00 · 2022-11-30 12:38:24 -05:00 · 2022-11-30 12:38:24 -05:00 · 89882ccca9
commit 89882ccca9
parent b46c0f71ff
8 changed files with 230 additions and 38 deletions
--- a/x-pack/plugins/fleet/server/services/agent_policies/package_policies_to_agent_permissions.ts
+++ b/x-pack/plugins/fleet/server/services/agent_policies/package_policies_to_agent_permissions.ts
@ -37,7 +37,7 @@ export async function storedPackagePoliciesToAgentPermissions(
  const permissionEntries = (packagePolicies as PackagePolicy[]).map<Promise<[string, any]>>(
    async (packagePolicy) => {
      if (!packagePolicy.package) {
-        throw new Error(`No package for package policy ${packagePolicy.name}`);
+        throw new Error(`No package for package policy ${packagePolicy.name ?? packagePolicy.id}`);
      }

      const pkg = packageInfoCache.get(pkgToPkgKey(packagePolicy.package))!;
--- a/x-pack/plugins/fleet/server/services/epm/archive/storage.test.ts
+++ b/x-pack/plugins/fleet/server/services/epm/archive/storage.test.ts
@ -0,0 +1,33 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-utils-server';
+
+import { getAsset } from './storage';
+
+jest.mock('../../app_context', () => {
+  return {
+    appContextService: {
+      getLogger: jest.fn().mockReturnValue({
+        warn: jest.fn(),
+      }),
+    },
+  };
+});
+
+describe('getAsset', () => {
+  it('should not throw error if saved object not found', async () => {
+    const soClientMock = {
+      get: jest.fn().mockRejectedValue(SavedObjectsErrorHelpers.createGenericNotFoundError()),
+    } as any;
+    const result = await getAsset({
+      savedObjectsClient: soClientMock,
+      path: 'path',
+    });
+    expect(result).toBeUndefined();
+  });
+});
--- a/x-pack/plugins/fleet/server/services/epm/archive/storage.ts
+++ b/x-pack/plugins/fleet/server/services/epm/archive/storage.ts
@ -13,6 +13,7 @@ import { isBinaryFile } from 'isbinaryfile';
 import mime from 'mime-types';
 import uuidv5 from 'uuid/v5';
 import type { SavedObjectsClientContract, SavedObjectsBulkCreateObject } from '@kbn/core/server';
+import { SavedObjectsErrorHelpers } from '@kbn/core/server';

 import { ASSETS_SAVED_OBJECT_TYPE } from '../../../../common';
 import type {
@ -157,16 +158,24 @@ export async function getAsset(opts: {
  path: string;
 }) {
  const { savedObjectsClient, path } = opts;
-  const assetSavedObject = await savedObjectsClient.get<PackageAsset>(
-    ASSETS_SAVED_OBJECT_TYPE,
-    assetPathToObjectId(path)
-  );
-  const storedAsset = assetSavedObject?.attributes;
-  if (!storedAsset) {
-    return;
-  }
+  try {
+    const assetSavedObject = await savedObjectsClient.get<PackageAsset>(
+      ASSETS_SAVED_OBJECT_TYPE,
+      assetPathToObjectId(path)
+    );
+    const storedAsset = assetSavedObject?.attributes;
+    if (!storedAsset) {
+      return;
+    }

-  return storedAsset;
+    return storedAsset;
+  } catch (error) {
+    if (SavedObjectsErrorHelpers.isNotFoundError(error)) {
+      appContextService.getLogger().warn(error.message);
+      return;
+    }
+    throw error;
+  }
 }

 export const getEsPackage = async (
--- a/x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.ts
+++ b/x-pack/plugins/fleet/server/services/epm/kibana/assets/tag_assets.ts
@ -9,6 +9,7 @@ import { taggableTypes } from '@kbn/saved-objects-tagging-plugin/common/constant
 import type { IAssignmentService, ITagsClient } from '@kbn/saved-objects-tagging-plugin/server';

 import type { KibanaAssetType } from '../../../../../common';
+import { appContextService } from '../../../app_context';

 import type { ArchiveAsset } from './install';
 import { KibanaSavedObjectTypeMapping } from './install';
@ -44,12 +45,20 @@ export async function tagKibanaAssets(opts: TagAssetsParams) {
    ensurePackageTag(opts),
  ]);

-  await savedObjectTagAssignmentService.updateTagAssignments({
-    tags: [managedTagId, packageTagId],
-    assign: taggableAssets,
-    unassign: [],
-    refresh: false,
-  });
+  try {
+    await savedObjectTagAssignmentService.updateTagAssignments({
+      tags: [managedTagId, packageTagId],
+      assign: taggableAssets,
+      unassign: [],
+      refresh: false,
+    });
+  } catch (error) {
+    if (error.status === 404) {
+      appContextService.getLogger().warn(error.message);
+      return;
+    }
+    throw error;
+  }
 }

 function getTaggableAssets(kibanaAssets: TagAssetsParams['kibanaAssets']) {
--- a/x-pack/plugins/fleet/server/services/epm/packages/cleanup.test.ts
+++ b/x-pack/plugins/fleet/server/services/epm/packages/cleanup.test.ts
@ -30,16 +30,35 @@ describe(' Cleanup old assets', () => {
  let removeArchiveEntriesMock: jest.MockedFunction<typeof storage.removeArchiveEntries>;

  function mockFindVersions(versions: string[]) {
-    soClient.find.mockResolvedValue({
-      page: 0,
-      per_page: 0,
-      total: 0,
-      saved_objects: [],
-      aggregations: {
-        versions: {
-          buckets: versions.map((v) => ({ key: '0.3.3' })),
-        },
-      },
+    soClient.find.mockImplementation((options: any): Promise<any> => {
+      if (options.type === 'epm-packages-assets') {
+        return Promise.resolve({
+          page: 0,
+          per_page: 0,
+          total: 0,
+          saved_objects: [],
+          aggregations: {
+            versions: {
+              buckets: versions.map((v) => ({ key: '0.3.3' })),
+            },
+          },
+        });
+      } else if (options.type === 'epm-packages') {
+        return Promise.resolve({
+          saved_objects: [
+            {
+              attributes: {
+                package_assets: [
+                  {
+                    id: 'asset1',
+                  },
+                ],
+              },
+            },
+          ],
+        });
+      }
+      return Promise.resolve({});
    });
  }

@ -80,6 +99,24 @@ describe(' Cleanup old assets', () => {
    expect(removeArchiveEntriesMock).not.toHaveBeenCalled();
  });

+  it('should not remove asset referened by epm-packages', async () => {
+    mockFindVersions(['0.3.3']);
+    packagePolicyServiceMock.list.mockResolvedValue({ total: 0, items: [], page: 0, perPage: 0 });
+    soClient.createPointInTimeFinder = jest.fn().mockResolvedValue({
+      close: jest.fn(),
+      find: function* asyncGenerator() {
+        yield { saved_objects: [{ id: 'asset1' }, { id: '2' }] };
+      },
+    });
+
+    await removeOldAssets({ soClient, pkgName: 'apache', currentVersion: '1.0.0' });
+
+    expect(removeArchiveEntriesMock).toHaveBeenCalledWith({
+      savedObjectsClient: soClient,
+      refs: [{ id: '2', type: 'epm-packages-assets' }],
+    });
+  });
+
  it('should remove old assets from all pages', async () => {
    mockFindVersions(['0.3.3']);
    packagePolicyServiceMock.list.mockResolvedValue({ total: 0, items: [], page: 0, perPage: 0 });
--- a/x-pack/plugins/fleet/server/services/epm/packages/cleanup.ts
+++ b/x-pack/plugins/fleet/server/services/epm/packages/cleanup.ts
@ -9,7 +9,11 @@ import type { SavedObjectsClientContract } from '@kbn/core/server';

 import { removeArchiveEntries } from '../archive/storage';

-import { ASSETS_SAVED_OBJECT_TYPE, PACKAGE_POLICY_SAVED_OBJECT_TYPE } from '../../../../common';
+import {
+  ASSETS_SAVED_OBJECT_TYPE,
+  PACKAGE_POLICY_SAVED_OBJECT_TYPE,
+  PACKAGES_SAVED_OBJECT_TYPE,
+} from '../../../../common';
 import type { PackageAssetReference } from '../../../../common/types';
 import { packagePolicyService } from '../../package_policy';
 import { appContextService } from '../..';
@ -37,15 +41,26 @@ export async function removeOldAssets(options: {
    (obj: { key: string }) => obj.key
  );

+  const packageAssetRefsRes = await soClient.find({
+    type: PACKAGES_SAVED_OBJECT_TYPE,
+    filter: `${PACKAGES_SAVED_OBJECT_TYPE}.attributes.name:${pkgName}`,
+    fields: [`${PACKAGES_SAVED_OBJECT_TYPE}.package_assets`],
+  });
+
+  const packageAssetRefs = (
+    (packageAssetRefsRes.saved_objects?.[0]?.attributes as any)?.package_assets ?? []
+  ).map((ref: any) => ref.id);
+
  for (const oldVersion of oldVersions) {
-    await removeAssetsFromVersion(soClient, pkgName, oldVersion);
+    await removeAssetsFromVersion(soClient, pkgName, oldVersion, packageAssetRefs);
  }
 }

 async function removeAssetsFromVersion(
  soClient: SavedObjectsClientContract,
  pkgName: string,
-  oldVersion: string
+  oldVersion: string,
+  packageAssetRefs: string[]
 ) {
  // check if any policies are using this package version
  const { total } = await packagePolicyService.list(soClient, {
@ -73,8 +88,10 @@ async function removeAssetsFromVersion(
    const refs = assets.saved_objects.map(
      (obj) => ({ id: obj.id, type: ASSETS_SAVED_OBJECT_TYPE } as PackageAssetReference)
    );
+    // only delete epm-packages-assets that are not referenced by epm-packages
+    const unusedRefs = refs.filter((ref) => !packageAssetRefs.includes(ref.id));

-    await removeArchiveEntries({ savedObjectsClient: soClient, refs });
+    await removeArchiveEntries({ savedObjectsClient: soClient, refs: unusedRefs });
  }
  await finder.close();
 }
--- a/x-pack/plugins/fleet/server/services/epm/packages/remove.test.ts
+++ b/x-pack/plugins/fleet/server/services/epm/packages/remove.test.ts
@ -0,0 +1,69 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { packagePolicyService } from '../..';
+
+import { removeInstallation } from './remove';
+
+jest.mock('../..', () => {
+  return {
+    appContextService: {
+      getLogger: jest.fn().mockReturnValue({
+        info: jest.fn(),
+        error: jest.fn(),
+      }),
+    },
+    packagePolicyService: {
+      list: jest.fn().mockResolvedValue({ total: 1, items: [{ id: 'system-1' }] }),
+      delete: jest.fn(),
+    },
+  };
+});
+
+const mockPackagePolicyService = packagePolicyService as jest.Mocked<typeof packagePolicyService>;
+
+describe('removeInstallation', () => {
+  let soClientMock: any;
+  const esClientMock = {} as any;
+  beforeEach(() => {
+    soClientMock = {
+      get: jest.fn().mockResolvedValue({ attributes: { installed_kibana: [], installed_es: [] } }),
+      delete: jest.fn(),
+      find: jest.fn().mockResolvedValue({ saved_objects: [] }),
+      bulkResolve: jest.fn().mockResolvedValue({ resolved_objects: [] }),
+    } as any;
+  });
+  it('should remove package policies when force', async () => {
+    await removeInstallation({
+      savedObjectsClient: soClientMock,
+      pkgName: 'system',
+      pkgVersion: '1.0.0',
+      esClient: esClientMock,
+      force: true,
+    });
+    expect(mockPackagePolicyService.delete).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.anything(),
+      ['system-1'],
+      { force: true }
+    );
+  });
+
+  it('should throw when trying to remove package with package policies when not force', async () => {
+    await expect(
+      removeInstallation({
+        savedObjectsClient: soClientMock,
+        pkgName: 'system',
+        pkgVersion: '1.0.0',
+        esClient: esClientMock,
+        force: false,
+      })
+    ).rejects.toThrowError(
+      `unable to remove package with existing package policy(s) in use by agent(s)`
+    );
+  });
+});
--- a/x-pack/plugins/fleet/server/services/epm/packages/remove.ts
+++ b/x-pack/plugins/fleet/server/services/epm/packages/remove.ts
@ -17,7 +17,11 @@ import { DEFAULT_SPACE_ID } from '@kbn/spaces-plugin/common/constants';

 import { SavedObjectsUtils, SavedObjectsErrorHelpers } from '@kbn/core/server';

-import { PACKAGE_POLICY_SAVED_OBJECT_TYPE, PACKAGES_SAVED_OBJECT_TYPE } from '../../../constants';
+import {
+  PACKAGE_POLICY_SAVED_OBJECT_TYPE,
+  PACKAGES_SAVED_OBJECT_TYPE,
+  SO_SEARCH_LIMIT,
+} from '../../../constants';
 import { ElasticsearchAssetType } from '../../../types';
 import type {
  AssetReference,
@ -48,16 +52,30 @@ export async function removeInstallation(options: {
  const installation = await getInstallation({ savedObjectsClient, pkgName });
  if (!installation) throw Boom.badRequest(`${pkgName} is not installed`);

-  const { total } = await packagePolicyService.list(savedObjectsClient, {
+  const { total, items } = await packagePolicyService.list(savedObjectsClient, {
    kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name:${pkgName}`,
-    page: 0,
-    perPage: 0,
+    page: 1,
+    perPage: options.force ? SO_SEARCH_LIMIT : 0,
  });

-  if (total > 0)
-    throw Boom.badRequest(
-      `unable to remove package with existing package policy(s) in use by agent(s)`
-    );
+  if (total > 0) {
+    if (options.force) {
+      // delete package policies
+      const ids = items.map((item) => item.id);
+      appContextService
+        .getLogger()
+        .info(
+          `deleting package policies of ${pkgName} package because force flag was enabled: ${ids}`
+        );
+      await packagePolicyService.delete(savedObjectsClient, esClient, ids, {
+        force: options.force,
+      });
+    } else {
+      throw Boom.badRequest(
+        `unable to remove package with existing package policy(s) in use by agent(s)`
+      );
+    }
+  }

  // Delete the installed assets. Don't include installation.package_assets. Those are irrelevant to users
  const installedAssets = [...installation.installed_kibana, ...installation.installed_es];