diff --git a/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts index 77e7a2c4ede1aa..58463bfa5569d2 100644 --- a/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts +++ b/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts @@ -146,7 +146,8 @@ export const updatePackagePolicyHandler: RequestHandler< esClient, request.params.packagePolicyId, { ...newData, package: pkg, inputs }, - { user } + { user }, + packagePolicy.package?.version ); return response.ok({ body: { item: updatedPackagePolicy }, diff --git a/x-pack/plugins/fleet/server/services/package_policy.test.ts b/x-pack/plugins/fleet/server/services/package_policy.test.ts index c25a1db753c73c..a141203bfc81a4 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.test.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.test.ts @@ -134,6 +134,12 @@ jest.mock('./epm/packages/cleanup', () => { }; }); +jest.mock('./upgrade_usage', () => { + return { + sendTelemetryEvents: jest.fn(), + }; +}); + const mockedFetchInfo = fetchInfo as jest.Mock>; type CombinedExternalCallback = PutPackagePolicyUpdateCallback | PostPackagePolicyCreateCallback; @@ -578,6 +584,12 @@ describe('Package policy service', () => { }); describe('update', () => { + beforeEach(() => { + appContextService.start(createAppContextStartContractMock()); + }); + afterEach(() => { + appContextService.stop(); + }); it('should fail to update on version conflict', async () => { const savedObjectsClient = savedObjectsClientMock.create(); savedObjectsClient.get.mockResolvedValue({ @@ -601,7 +613,8 @@ describe('Package policy service', () => { savedObjectsClient, elasticsearchClient, 'the-package-policy-id', - createPackagePolicyMock() + createPackagePolicyMock(), + 'current-version' ) ).rejects.toThrow('Saved object [abc/123] conflict'); }); @@ -721,7 +734,8 @@ describe('Package policy service', () => { savedObjectsClient, elasticsearchClient, 'the-package-policy-id', - { ...mockPackagePolicy, inputs: inputsUpdate } + { ...mockPackagePolicy, inputs: inputsUpdate }, + 'current-version' ); const [modifiedInput] = result.inputs; @@ -844,7 +858,8 @@ describe('Package policy service', () => { savedObjectsClient, elasticsearchClient, 'the-package-policy-id', - { ...mockPackagePolicy, inputs: inputsUpdate } + { ...mockPackagePolicy, inputs: inputsUpdate }, + 'current-version' ); const [modifiedInput] = result.inputs; @@ -903,7 +918,8 @@ describe('Package policy service', () => { savedObjectsClient, elasticsearchClient, 'the-package-policy-id', - { ...mockPackagePolicy, inputs: [] } + { ...mockPackagePolicy, inputs: [] }, + 'current-version' ); expect(result.elasticsearch).toMatchObject({ privileges: { cluster: ['monitor'] } }); diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index b0cbfb08090e61..5b2cce5417bc2f 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -359,7 +359,8 @@ class PackagePolicyService { esClient: ElasticsearchClient, id: string, packagePolicy: UpdatePackagePolicy, - options?: { user?: AuthenticatedUser } + options?: { user?: AuthenticatedUser }, + currentVersion?: string ): Promise { const oldPackagePolicy = await this.get(soClient, id); const { version, ...restOfPackagePolicy } = packagePolicy; @@ -433,6 +434,20 @@ class PackagePolicyService { pkgName: packagePolicy.package.name, currentVersion: packagePolicy.package.version, }); + + if (packagePolicy.package.version !== currentVersion) { + sendTelemetryEvents( + appContextService.getLogger(), + appContextService.getTelemetryEventsSender(), + { + package_name: packagePolicy.package.name, + current_version: currentVersion || 'unknown', + new_version: packagePolicy.package.version, + status: 'success', + dryRun: false, + } + ); + } } return newPolicy; @@ -601,25 +616,19 @@ class PackagePolicyService { ); updatePackagePolicy.elasticsearch = registryPkgInfo.elasticsearch; - await this.update(soClient, esClient, id, updatePackagePolicy, options); + await this.update( + soClient, + esClient, + id, + updatePackagePolicy, + options, + packagePolicy.package.version + ); result.push({ id, name: packagePolicy.name, success: true, }); - - if (packagePolicy.package.version !== packageInfo.version) { - sendTelemetryEvents( - appContextService.getLogger(), - appContextService.getTelemetryEventsSender(), - { - package_name: packageInfo.name, - current_version: packagePolicy.package.version, - new_version: packageInfo.version, - status: 'success', - } - ); - } } catch (error) { // We only want to specifically handle validation errors for the new package policy. If a more severe or // general error is thrown elsewhere during the upgrade process, we want to surface that directly in @@ -676,19 +685,18 @@ class PackagePolicyService { const hasErrors = 'errors' in updatedPackagePolicy; if (packagePolicy.package.version !== packageInfo.version) { - if (hasErrors) { - sendTelemetryEvents( - appContextService.getLogger(), - appContextService.getTelemetryEventsSender(), - { - package_name: packageInfo.name, - current_version: packagePolicy.package.version, - new_version: packageInfo.version, - status: hasErrors ? 'failure' : 'success', - error: updatedPackagePolicy.errors, - } - ); - } + sendTelemetryEvents( + appContextService.getLogger(), + appContextService.getTelemetryEventsSender(), + { + package_name: packageInfo.name, + current_version: packagePolicy.package.version, + new_version: packageInfo.version, + status: hasErrors ? 'failure' : 'success', + error: hasErrors ? updatedPackagePolicy.errors : undefined, + dryRun: true, + } + ); } return { diff --git a/x-pack/plugins/fleet/server/services/upgrade_usage.test.ts b/x-pack/plugins/fleet/server/services/upgrade_usage.test.ts index c9c1a290d40902..e14aa37e4a5e7c 100644 --- a/x-pack/plugins/fleet/server/services/upgrade_usage.test.ts +++ b/x-pack/plugins/fleet/server/services/upgrade_usage.test.ts @@ -33,6 +33,7 @@ describe('sendTelemetryEvents', () => { { key: 'fieldX', message: ['Field X is required'] }, { key: 'fieldX', message: 'Invalid format' }, ], + dryRun: true, }; sendTelemetryEvents(loggerMock, eventsTelemetryMock, upgardeMessage); @@ -40,7 +41,7 @@ describe('sendTelemetryEvents', () => { expect(eventsTelemetryMock.queueTelemetryEvents).toHaveBeenCalledWith( [ { - id: 'aws_0.6.1_1.3.0_failure', + id: 'aws_0.6.1_1.3.0_failure_true', package_policy_upgrade: { current_version: '0.6.1', error: [ @@ -56,6 +57,7 @@ describe('sendTelemetryEvents', () => { new_version: '1.3.0', package_name: 'aws', status: 'failure', + dryRun: true, }, }, ], diff --git a/x-pack/plugins/fleet/server/services/upgrade_usage.ts b/x-pack/plugins/fleet/server/services/upgrade_usage.ts index c69ddf0261cd25..9be25ad3e8cbb4 100644 --- a/x-pack/plugins/fleet/server/services/upgrade_usage.ts +++ b/x-pack/plugins/fleet/server/services/upgrade_usage.ts @@ -15,6 +15,7 @@ export interface PackagePolicyUpgradeUsage { new_version: string; status: 'success' | 'failure'; error?: UpgradeError[]; + dryRun?: boolean; } export interface UpgradeError { @@ -44,7 +45,7 @@ export function sendTelemetryEvents( ? makeErrorGeneric(capErrorSize(upgradeUsage.error, MAX_ERROR_SIZE)) : undefined, }, - id: `${upgradeUsage.package_name}_${upgradeUsage.current_version}_${upgradeUsage.new_version}_${upgradeUsage.status}`, + id: `${upgradeUsage.package_name}_${upgradeUsage.current_version}_${upgradeUsage.new_version}_${upgradeUsage.status}_${upgradeUsage.dryRun}`, }, ], FLEET_UPGRADES_CHANNEL_NAME