review improvements

x-pack/plugins/fleet/server/services/managed_package_policies.test.ts

@@ -18,7 +18,7 @@ jest.mock('./app_context', () => {
     ...jest.requireActual('./app_context'),
     appContextService: {
       getLogger: jest.fn(() => {
-        return { error: jest.fn() };
+        return { error: jest.fn(), debug: jest.fn() };
       }),
     },
   };
@@ -34,15 +34,7 @@ describe('upgradeManagedPackagePolicies', () => {
     const soClient = savedObjectsClientMock.create();
 
     (getInstallations as jest.Mock).mockResolvedValueOnce({
-      saved_objects: [
-        {
-          attributes: {
-            id: 'test-installation',
-            version: '0.0.1',
-            keep_policies_up_to_date: false,
-          },
-        },
-      ],
+      saved_objects: [],
     });
 
     await upgradeManagedPackagePolicies(soClient, esClient);

x-pack/plugins/fleet/server/services/managed_package_policies.ts

@@ -32,24 +32,25 @@ export const upgradeManagedPackagePolicies = async (
   soClient: SavedObjectsClientContract,
   esClient: ElasticsearchClient
 ): Promise<UpgradeManagedPackagePoliciesResult[]> => {
+  appContextService
+    .getLogger()
+    .debug('Running required package policies upgrades for managed policies');
   const results: UpgradeManagedPackagePoliciesResult[] = [];
 
   const installedPackages = await getInstallations(soClient, {
-    filter: `${PACKAGES_SAVED_OBJECT_TYPE}.attributes.install_status:installed`,
+    filter: `${PACKAGES_SAVED_OBJECT_TYPE}.attributes.install_status:installed AND ${PACKAGES_SAVED_OBJECT_TYPE}.attributes.keep_policies_up_to_date:true`,
   });
 
   for (const { attributes: installedPackage } of installedPackages.saved_objects) {
-    if (installedPackage.keep_policies_up_to_date) {
-      const packagePolicies = await getPackagePoliciesNotMatchingVersion(
-        soClient,
-        installedPackage.name,
-        installedPackage.version
-      );
-
-      for (const packagePolicy of packagePolicies) {
-        if (isPolicyVersionLtInstalledVersion(packagePolicy, installedPackage)) {
-          await upgradePackagePolicy(soClient, esClient, packagePolicy.id, results);
-        }
+    const packagePolicies = await getPackagePoliciesNotMatchingVersion(
+      soClient,
+      installedPackage.name,
+      installedPackage.version
+    );
+
+    for (const packagePolicy of packagePolicies) {
+      if (isPolicyVersionLtInstalledVersion(packagePolicy, installedPackage)) {
+        await upgradePackagePolicy(soClient, esClient, packagePolicy.id, results);
       }
     }
   }

x-pack/plugins/fleet/server/services/setup.test.ts

@@ -5,29 +5,56 @@
  * 2.0.
  */
 
+import type { SavedObjectsClientContract } from 'kibana/server';
+import type { ElasticsearchClientMock } from 'src/core/server/mocks';
+
 import { createAppContextStartContractMock, xpackMocks } from '../mocks';
 
+import { ensurePreconfiguredPackagesAndPolicies } from '.';
+
 import { appContextService } from './app_context';
+import { getInstallations } from './epm/packages';
+import { upgradeManagedPackagePolicies } from './managed_package_policies';
 import { setupFleet } from './setup';
 
-const mockedMethodThrowsError = () =>
-  jest.fn().mockImplementation(() => {
+jest.mock('./preconfiguration');
+jest.mock('./settings');
+jest.mock('./output');
+jest.mock('./epm/packages');
+jest.mock('./managed_package_policies');
+
+const mockedMethodThrowsError = (mockFn: jest.Mock) =>
+  mockFn.mockImplementation(() => {
     throw new Error('SO method mocked to throw');
   });
 
 class CustomTestError extends Error {}
-const mockedMethodThrowsCustom = () =>
-  jest.fn().mockImplementation(() => {
+const mockedMethodThrowsCustom = (mockFn: jest.Mock) =>
+  mockFn.mockImplementation(() => {
     throw new CustomTestError('method mocked to throw');
   });
 
 describe('setupFleet', () => {
   let context: ReturnType<typeof xpackMocks.createRequestHandlerContext>;
+  let soClient: jest.Mocked<SavedObjectsClientContract>;
+  let esClient: ElasticsearchClientMock;
 
   beforeEach(async () => {
     context = xpackMocks.createRequestHandlerContext();
     // prevents `Logger not set.` and other appContext errors
     appContextService.start(createAppContextStartContractMock());
+    soClient = context.core.savedObjects.client;
+    esClient = context.core.elasticsearch.client.asInternalUser;
+
+    (getInstallations as jest.Mock).mockResolvedValueOnce({
+      saved_objects: [],
+    });
+
+    (ensurePreconfiguredPackagesAndPolicies as jest.Mock).mockResolvedValue({
+      nonFatalErrors: [],
+    });
+
+    (upgradeManagedPackagePolicies as jest.Mock).mockResolvedValue([]);
   });
 
   afterEach(async () => {
@@ -37,29 +64,61 @@ describe('setupFleet', () => {
 
   describe('should reject with any error thrown underneath', () => {
     it('SO client throws plain Error', async () => {
-      const soClient = context.core.savedObjects.client;
-      soClient.create = mockedMethodThrowsError();
-      soClient.find = mockedMethodThrowsError();
-      soClient.get = mockedMethodThrowsError();
-      soClient.update = mockedMethodThrowsError();
-      const esClient = context.core.elasticsearch.client.asInternalUser;
+      mockedMethodThrowsError(upgradeManagedPackagePolicies as jest.Mock);
 
       const setupPromise = setupFleet(soClient, esClient);
       await expect(setupPromise).rejects.toThrow('SO method mocked to throw');
       await expect(setupPromise).rejects.toThrow(Error);
     });
 
     it('SO client throws other error', async () => {
-      const soClient = context.core.savedObjects.client;
-      soClient.create = mockedMethodThrowsCustom();
-      soClient.find = mockedMethodThrowsCustom();
-      soClient.get = mockedMethodThrowsCustom();
-      soClient.update = mockedMethodThrowsCustom();
-      const esClient = context.core.elasticsearch.client.asInternalUser;
+      mockedMethodThrowsCustom(upgradeManagedPackagePolicies as jest.Mock);
 
       const setupPromise = setupFleet(soClient, esClient);
       await expect(setupPromise).rejects.toThrow('method mocked to throw');
       await expect(setupPromise).rejects.toThrow(CustomTestError);
     });
   });
+
+  it('should not return non fatal errors when upgrade result has no errors', async () => {
+    (upgradeManagedPackagePolicies as jest.Mock).mockResolvedValue([
+      {
+        errors: [],
+        packagePolicyId: '1',
+      },
+    ]);
+
+    const result = await setupFleet(soClient, esClient);
+
+    expect(result).toEqual({
+      isInitialized: true,
+      nonFatalErrors: [],
+    });
+  });
+
+  it('should return non fatal errors when upgrade result has errors', async () => {
+    (upgradeManagedPackagePolicies as jest.Mock).mockResolvedValue([
+      {
+        errors: [{ key: 'key', message: 'message' }],
+        packagePolicyId: '1',
+      },
+    ]);
+
+    const result = await setupFleet(soClient, esClient);
+
+    expect(result).toEqual({
+      isInitialized: true,
+      nonFatalErrors: [
+        {
+          errors: [
+            {
+              key: 'key',
+              message: 'message',
+            },
+          ],
+          packagePolicyId: '1',
+        },
+      ],
+    });
+  });
 });

x-pack/plugins/fleet/server/services/setup.ts

@@ -109,7 +109,6 @@ async function createSetupSideEffects(
       DEFAULT_SPACE_ID
     );
 
-  logger.debug('Ensure managed package policies are upgraded');
   const packagePolicyUpgradeErrors = (
     await upgradeManagedPackagePolicies(soClient, esClient)
   ).filter((result) => (result.errors ?? []).length > 0);