[Fleet] Improve Functionality around Managed Package Policies (elasti…

…c#114526) * Enabled auto policy upgrades for APM and Synthetics * fixup! Enabled auto policy upgrades for APM and Synthetics * Rework preconfiguration policy upgrade flow + report errors * Fix type error in test * Fix type errors + tests * wip * Remove keep policies up to date checks * Remove references to KEEP_POLICIES_UP_TO_DATE_PACKAGES * Move package policy upgrade results to nonFatalErrors * Fix types * Fix type error Co-authored-by: Kibana Machine <[email protected]>
kibanamachine · Oct 14, 2021 · c87b599 · c87b599
1 parent 18fc38e
commit c87b599
Show file tree

Hide file tree

Showing 6 changed files with 186 additions and 35 deletions.
diff --git a/x-pack/plugins/fleet/server/routes/setup/handlers.test.ts b/x-pack/plugins/fleet/server/routes/setup/handlers.test.ts
@@ -53,7 +53,10 @@ describe('FleetSetupHandler', () => {
     );
     await fleetSetupHandler(context, request, response);
 
-    const expectedBody: PostFleetSetupResponse = { isInitialized: true, nonFatalErrors: [] };
+    const expectedBody: PostFleetSetupResponse = {
+      isInitialized: true,
+      nonFatalErrors: [],
+    };
     expect(response.customError).toHaveBeenCalledTimes(0);
     expect(response.ok).toHaveBeenCalledWith({ body: expectedBody });
   });

diff --git a/x-pack/plugins/fleet/server/routes/setup/handlers.ts b/x-pack/plugins/fleet/server/routes/setup/handlers.ts
@@ -49,13 +49,21 @@ export const fleetSetupHandler: RequestHandler = async (context, request, respon
     const setupStatus = await setupFleet(soClient, esClient);
     const body: PostFleetSetupResponse = {
       ...setupStatus,
-      nonFatalErrors: setupStatus.nonFatalErrors.map((e) => {
+      nonFatalErrors: setupStatus.nonFatalErrors.flatMap((e) => {
         // JSONify the error object so it can be displayed properly in the UI
-        const error = e.error ?? e;
-        return {
-          name: error.name,
-          message: error.message,
-        };
+        if ('error' in e) {
+          return {
+            name: e.error.name,
+            message: e.error.message,
+          };
+        } else {
+          return e.errors.map((upgradePackagePolicyError: any) => {
+            return {
+              name: upgradePackagePolicyError.key,
+              message: upgradePackagePolicyError.message,
+            };
+          });
+        }
       }),
     };
 

diff --git a/x-pack/plugins/fleet/server/services/managed_package_policies.test.ts b/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 { debug: jest.fn() };
+        return { error: jest.fn() };
       }),
     },
   };
@@ -27,7 +27,9 @@ jest.mock('./app_context', () => {
 describe('managed package policies', () => {
   afterEach(() => {
     (packagePolicyService.get as jest.Mock).mockReset();
+    (packagePolicyService.getUpgradeDryRunDiff as jest.Mock).mockReset();
     (getPackageInfo as jest.Mock).mockReset();
+    (packagePolicyService.upgrade as jest.Mock).mockReset();
   });
 
   it('should not upgrade policies for non-managed package', async () => {
@@ -54,6 +56,16 @@ describe('managed package policies', () => {
       }
     );
 
+    (packagePolicyService.getUpgradeDryRunDiff as jest.Mock).mockImplementationOnce(
+      (savedObjectsClient: any, id: string) => {
+        return {
+          name: 'non-managed-package-policy',
+          diff: [{ id: 'foo' }, { id: 'bar' }],
+          hasErrors: false,
+        };
+      }
+    );
+
     (getPackageInfo as jest.Mock).mockImplementationOnce(
       ({ savedObjectsClient, pkgName, pkgVersion }) => ({
         name: pkgName,
@@ -91,6 +103,16 @@ describe('managed package policies', () => {
       }
     );
 
+    (packagePolicyService.getUpgradeDryRunDiff as jest.Mock).mockImplementationOnce(
+      (savedObjectsClient: any, id: string) => {
+        return {
+          name: 'non-managed-package-policy',
+          diff: [{ id: 'foo' }, { id: 'bar' }],
+          hasErrors: false,
+        };
+      }
+    );
+
     (getPackageInfo as jest.Mock).mockImplementationOnce(
       ({ savedObjectsClient, pkgName, pkgVersion }) => ({
         name: pkgName,
@@ -103,4 +125,84 @@ describe('managed package policies', () => {
 
     expect(packagePolicyService.upgrade).toBeCalledWith(soClient, esClient, ['managed-package-id']);
   });
+
+  describe('when dry run reports conflicts', () => {
+    it('should return errors + diff without performing upgrade', async () => {
+      const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;
+      const soClient = savedObjectsClientMock.create();
+
+      (packagePolicyService.get as jest.Mock).mockImplementationOnce(
+        (savedObjectsClient: any, id: string) => {
+          return {
+            id,
+            inputs: {},
+            version: '',
+            revision: 1,
+            updated_at: '',
+            updated_by: '',
+            created_at: '',
+            created_by: '',
+            package: {
+              name: 'conflicting-package',
+              title: 'Conflicting Package',
+              version: '0.0.1',
+            },
+          };
+        }
+      );
+
+      (packagePolicyService.getUpgradeDryRunDiff as jest.Mock).mockImplementationOnce(
+        (savedObjectsClient: any, id: string) => {
+          return {
+            name: 'conflicting-package-policy',
+            diff: [
+              { id: 'foo' },
+              { id: 'bar', errors: [{ key: 'some.test.value', message: 'Conflict detected' }] },
+            ],
+            hasErrors: true,
+          };
+        }
+      );
+
+      (getPackageInfo as jest.Mock).mockImplementationOnce(
+        ({ savedObjectsClient, pkgName, pkgVersion }) => ({
+          name: pkgName,
+          version: pkgVersion,
+          keepPoliciesUpToDate: true,
+        })
+      );
+
+      const result = await upgradeManagedPackagePolicies(soClient, esClient, [
+        'conflicting-package-policy',
+      ]);
+
+      expect(result).toEqual([
+        {
+          packagePolicyId: 'conflicting-package-policy',
+          diff: [
+            {
+              id: 'foo',
+            },
+            {
+              id: 'bar',
+              errors: [
+                {
+                  key: 'some.test.value',
+                  message: 'Conflict detected',
+                },
+              ],
+            },
+          ],
+          errors: [
+            {
+              key: 'some.test.value',
+              message: 'Conflict detected',
+            },
+          ],
+        },
+      ]);
+
+      expect(packagePolicyService.upgrade).not.toBeCalled();
+    });
+  });
 });
diff --git a/x-pack/plugins/fleet/server/services/managed_package_policies.ts b/x-pack/plugins/fleet/server/services/managed_package_policies.ts
@@ -7,12 +7,19 @@
 
 import type { ElasticsearchClient, SavedObjectsClientContract } from 'src/core/server';
 
+import type { UpgradePackagePolicyDryRunResponseItem } from '../../common';
 import { AUTO_UPDATE_PACKAGES } from '../../common';
 
 import { appContextService } from './app_context';
-import { getPackageInfo } from './epm/packages';
+import { getInstallation, getPackageInfo } from './epm/packages';
 import { packagePolicyService } from './package_policy';
 
+export interface UpgradeManagedPackagePoliciesResult {
+  packagePolicyId: string;
+  diff: UpgradePackagePolicyDryRunResponseItem['diff'];
+  errors: any;
+}
+
 /**
  * Upgrade any package policies for packages installed through setup that are denoted as `AUTO_UPGRADE` packages
  * or have the `keep_policies_up_to_date` flag set to `true`
@@ -21,8 +28,8 @@ export const upgradeManagedPackagePolicies = async (
   soClient: SavedObjectsClientContract,
   esClient: ElasticsearchClient,
   packagePolicyIds: string[]
-) => {
-  const policyIdsToUpgrade: string[] = [];
+): Promise<UpgradeManagedPackagePoliciesResult[]> => {
+  const results: UpgradeManagedPackagePoliciesResult[] = [];
 
   for (const packagePolicyId of packagePolicyIds) {
     const packagePolicy = await packagePolicyService.get(soClient, packagePolicyId);
@@ -37,22 +44,50 @@ export const upgradeManagedPackagePolicies = async (
       pkgVersion: packagePolicy.package.version,
     });
 
+    const installedPackage = await getInstallation({
+      savedObjectsClient: soClient,
+      pkgName: packagePolicy.package.name,
+    });
+
+    const isPolicyVersionAlignedWithInstalledVersion =
+      packageInfo.version === installedPackage?.version;
+
     const shouldUpgradePolicies =
-      AUTO_UPDATE_PACKAGES.some((pkg) => pkg.name === packageInfo.name) ||
-      packageInfo.keepPoliciesUpToDate;
+      !isPolicyVersionAlignedWithInstalledVersion &&
+      (AUTO_UPDATE_PACKAGES.some((pkg) => pkg.name === packageInfo.name) ||
+        packageInfo.keepPoliciesUpToDate);
 
     if (shouldUpgradePolicies) {
-      policyIdsToUpgrade.push(packagePolicy.id);
-    }
-  }
-
-  if (policyIdsToUpgrade.length) {
-    appContextService
-      .getLogger()
-      .debug(
-        `Upgrading ${policyIdsToUpgrade.length} package policies: ${policyIdsToUpgrade.join(', ')}`
+      // Since upgrades don't report diffs/errors, we need to perform a dry run first in order
+      // to notify the user of any granular policy upgrade errors that occur during Fleet's
+      // preconfiguration check
+      const dryRunResults = await packagePolicyService.getUpgradeDryRunDiff(
+        soClient,
+        packagePolicyId
       );
 
-    await packagePolicyService.upgrade(soClient, esClient, policyIdsToUpgrade);
+      if (dryRunResults.hasErrors) {
+        const errors = dryRunResults.diff?.[1].errors;
+        appContextService
+          .getLogger()
+          .error(
+            new Error(
+              `Error upgrading package policy ${packagePolicyId}: ${JSON.stringify(errors)}`
+            )
+          );
+
+        results.push({ packagePolicyId, diff: dryRunResults.diff, errors });
+        continue;
+      }
+
+      try {
+        await packagePolicyService.upgrade(soClient, esClient, [packagePolicyId]);
+        results.push({ packagePolicyId, diff: dryRunResults.diff, errors: [] });
+      } catch (error) {
+        results.push({ packagePolicyId, diff: dryRunResults.diff, errors: [error] });
+      }
+    }
   }
+
+  return results;
 };
diff --git a/x-pack/plugins/fleet/server/services/preconfiguration.ts b/x-pack/plugins/fleet/server/services/preconfiguration.ts
@@ -35,13 +35,14 @@ import { agentPolicyService, addPackageToAgentPolicy } from './agent_policy';
 import type { InputsOverride } from './package_policy';
 import { overridePackageInputs } from './package_policy';
 import { appContextService } from './app_context';
+import type { UpgradeManagedPackagePoliciesResult } from './managed_package_policies';
 import { upgradeManagedPackagePolicies } from './managed_package_policies';
 import { outputService } from './output';
 
 interface PreconfigurationResult {
   policies: Array<{ id: string; updated_at: string }>;
   packages: string[];
-  nonFatalErrors: PreconfigurationError[];
+  nonFatalErrors: Array<PreconfigurationError | UpgradeManagedPackagePoliciesResult>;
 }
 
 function isPreconfiguredOutputDifferentFromCurrent(
@@ -326,16 +327,15 @@ export async function ensurePreconfiguredPackagesAndPolicies(
     }
   }
 
-  try {
-    const fulfilledPolicyPackagePolicyIds = fulfilledPolicies.flatMap<string>(
-      ({ policy }) => policy?.package_policies as string[]
-    );
+  const fulfilledPolicyPackagePolicyIds = fulfilledPolicies
+    .filter(({ policy }) => policy?.package_policies)
+    .flatMap<string>(({ policy }) => policy?.package_policies as string[]);
 
-    await upgradeManagedPackagePolicies(soClient, esClient, fulfilledPolicyPackagePolicyIds);
-    // Swallow errors that occur when upgrading
-  } catch (error) {
-    appContextService.getLogger().error(error);
-  }
+  const packagePolicyUpgradeResults = await upgradeManagedPackagePolicies(
+    soClient,
+    esClient,
+    fulfilledPolicyPackagePolicyIds
+  );
 
   return {
     policies: fulfilledPolicies.map((p) =>
@@ -353,7 +353,7 @@ export async function ensurePreconfiguredPackagesAndPolicies(
           }
     ),
     packages: fulfilledPackages.map((pkg) => pkgToPkgKey(pkg)),
-    nonFatalErrors: [...rejectedPackages, ...rejectedPolicies],
+    nonFatalErrors: [...rejectedPackages, ...rejectedPolicies, ...packagePolicyUpgradeResults],
   };
 }
 

diff --git a/x-pack/plugins/fleet/server/services/setup.ts b/x-pack/plugins/fleet/server/services/setup.ts
@@ -32,10 +32,13 @@ import { ensureDefaultComponentTemplate } from './epm/elasticsearch/template/ins
 import { getInstallations, installPackage } from './epm/packages';
 import { isPackageInstalled } from './epm/packages/install';
 import { pkgToPkgKey } from './epm/registry';
+import type { UpgradeManagedPackagePoliciesResult } from './managed_package_policies';
 
 export interface SetupStatus {
   isInitialized: boolean;
-  nonFatalErrors: Array<PreconfigurationError | DefaultPackagesInstallationError>;
+  nonFatalErrors: Array<
+    PreconfigurationError | DefaultPackagesInstallationError | UpgradeManagedPackagePoliciesResult
+  >;
 }
 
 export async function setupFleet(