diff --git a/x-pack/plugins/fleet/server/routes/setup/handlers.test.ts b/x-pack/plugins/fleet/server/routes/setup/handlers.test.ts index bd82989a9e8287..c196054faf08cc 100644 --- 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 index 6311b9d970d35d..d24db96667d527 100644 --- 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 index a53b1fe6489055..52c1c71446d641 100644 --- 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 index 73f85525f4c607..25e24828927120 100644 --- 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 => { + 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 index a878af64aa05e1..3b322e1112d6ae 100644 --- 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; } function isPreconfiguredOutputDifferentFromCurrent( @@ -326,16 +327,15 @@ export async function ensurePreconfiguredPackagesAndPolicies( } } - try { - const fulfilledPolicyPackagePolicyIds = fulfilledPolicies.flatMap( - ({ policy }) => policy?.package_policies as string[] - ); + const fulfilledPolicyPackagePolicyIds = fulfilledPolicies + .filter(({ policy }) => policy?.package_policies) + .flatMap(({ 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 index 08c580d80c804d..37d79c1bb691d2 100644 --- 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; + nonFatalErrors: Array< + PreconfigurationError | DefaultPackagesInstallationError | UpgradeManagedPackagePoliciesResult + >; } export async function setupFleet(