[Fleet] fix bug when dryrun gave error on latest package policy version (#121094) (#121173)

* fix bug when dryrun gave error on latest package policy version

* fix bug when package is older than policy

* added unit tests

* fix test

* fix tests

Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com>

x-pack/plugins/fleet/server/routes/package_policy/handlers.ts

@@ -263,10 +263,10 @@ export const dryRunUpgradePackagePolicyHandler: RequestHandler<
   const soClient = context.core.savedObjects.client;
   try {
     const body: UpgradePackagePolicyDryRunResponse = [];
-    const { packagePolicyIds, packageVersion } = request.body;
+    const { packagePolicyIds } = request.body;
 
     for (const id of packagePolicyIds) {
-      const result = await packagePolicyService.getUpgradeDryRunDiff(soClient, id, packageVersion);
+      const result = await packagePolicyService.getUpgradeDryRunDiff(soClient, id);
       body.push(result);
     }
 

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

@@ -11,7 +11,11 @@ import {
   httpServerMock,
 } from 'src/core/server/mocks';
 
-import type { SavedObjectsClient, SavedObjectsUpdateResponse } from 'src/core/server';
+import type {
+  SavedObjectsClient,
+  SavedObjectsClientContract,
+  SavedObjectsUpdateResponse,
+} from 'src/core/server';
 import type { KibanaRequest } from 'kibana/server';
 
 import type {
@@ -38,7 +42,7 @@ import type {
 } from '../../common';
 import { packageToPackagePolicy } from '../../common';
 
-import { IngestManagerError } from '../errors';
+import { IngestManagerError, PackagePolicyIneligibleForUpgradeError } from '../errors';
 
 import {
   preconfigurePackageInputs,
@@ -93,6 +97,21 @@ hosts:
   ];
 }
 
+async function mockedGetInstallation(params: any) {
+  let pkg;
+  if (params.pkgName === 'apache') pkg = { version: '1.3.2' };
+  if (params.pkgName === 'aws') pkg = { version: '0.3.3' };
+  return Promise.resolve(pkg);
+}
+
+async function mockedGetPackageInfo(params: any) {
+  let pkg;
+  if (params.pkgName === 'apache') pkg = { version: '1.3.2' };
+  if (params.pkgName === 'aws') pkg = { version: '0.3.3' };
+  if (params.pkgName === 'endpoint') pkg = {};
+  return Promise.resolve(pkg);
+}
+
 function mockedRegistryInfo(): RegistryPackage {
   return {} as RegistryPackage;
 }
@@ -105,7 +124,8 @@ jest.mock('./epm/packages/assets', () => {
 
 jest.mock('./epm/packages', () => {
   return {
-    getPackageInfo: () => ({}),
+    getPackageInfo: mockedGetPackageInfo,
+    getInstallation: mockedGetInstallation,
   };
 });
 
@@ -3033,6 +3053,63 @@ describe('Package policy service', () => {
       });
     });
   });
+
+  describe('upgrade package policy info', () => {
+    let savedObjectsClient: jest.Mocked<SavedObjectsClientContract>;
+    beforeEach(() => {
+      savedObjectsClient = savedObjectsClientMock.create();
+    });
+    function mockPackage(pkgName: string) {
+      const mockPackagePolicy = createPackagePolicyMock();
+
+      const attributes = {
+        ...mockPackagePolicy,
+        inputs: [],
+        package: {
+          ...mockPackagePolicy.package,
+          name: pkgName,
+        },
+      };
+
+      savedObjectsClient.get.mockResolvedValueOnce({
+        id: 'package-policy-id',
+        type: 'abcd',
+        references: [],
+        version: '1.3.2',
+        attributes,
+      });
+    }
+    it('should return success if package and policy versions match', async () => {
+      mockPackage('apache');
+
+      const response = await packagePolicyService.getUpgradePackagePolicyInfo(
+        savedObjectsClient,
+        'package-policy-id'
+      );
+
+      expect(response).toBeDefined();
+    });
+
+    it('should return error if package policy newer than package version', async () => {
+      mockPackage('aws');
+
+      expect(
+        packagePolicyService.getUpgradePackagePolicyInfo(savedObjectsClient, 'package-policy-id')
+      ).rejects.toEqual(
+        new PackagePolicyIneligibleForUpgradeError(
+          "Package policy c6d16e42-c32d-4dce-8a88-113cfe276ad1's package version 0.9.0 of package aws is newer than the installed package version. Please install the latest version of aws."
+        )
+      );
+    });
+
+    it('should return error if package not installed', async () => {
+      mockPackage('notinstalled');
+
+      expect(
+        packagePolicyService.getUpgradePackagePolicyInfo(savedObjectsClient, 'package-policy-id')
+      ).rejects.toEqual(new IngestManagerError('Package notinstalled is not installed'));
+    });
+  });
 });
 
 describe('_applyIndexPrivileges()', () => {

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

@@ -7,7 +7,7 @@
 
 import { omit, partition } from 'lodash';
 import { i18n } from '@kbn/i18n';
-import semverLte from 'semver/functions/lte';
+import semverLt from 'semver/functions/lt';
 import { getFlattenedObject } from '@kbn/std';
 import type { KibanaRequest } from 'src/core/server';
 import type {
@@ -498,11 +498,7 @@ class PackagePolicyService {
     return result;
   }
 
-  public async getUpgradePackagePolicyInfo(
-    soClient: SavedObjectsClientContract,
-    id: string,
-    packageVersion?: string
-  ) {
+  public async getUpgradePackagePolicyInfo(soClient: SavedObjectsClientContract, id: string) {
     const packagePolicy = await this.get(soClient, id);
     if (!packagePolicy) {
       throw new IngestManagerError(
@@ -522,48 +518,38 @@ class PackagePolicyService {
       );
     }
 
-    let packageInfo: PackageInfo;
+    const installedPackage = await getInstallation({
+      savedObjectsClient: soClient,
+      pkgName: packagePolicy.package.name,
+    });
 
-    if (packageVersion) {
-      packageInfo = await getPackageInfo({
-        savedObjectsClient: soClient,
-        pkgName: packagePolicy.package.name,
-        pkgVersion: packageVersion,
-      });
-    } else {
-      const installedPackage = await getInstallation({
-        savedObjectsClient: soClient,
-        pkgName: packagePolicy.package.name,
-      });
-
-      if (!installedPackage) {
-        throw new IngestManagerError(
-          i18n.translate('xpack.fleet.packagePolicy.packageNotInstalledError', {
-            defaultMessage: 'Package {name} is not installed',
-            values: {
-              name: packagePolicy.package.name,
-            },
-          })
-        );
-      }
-
-      packageInfo = await getPackageInfo({
-        savedObjectsClient: soClient,
-        pkgName: packagePolicy.package.name,
-        pkgVersion: installedPackage?.version ?? '',
-      });
+    if (!installedPackage) {
+      throw new IngestManagerError(
+        i18n.translate('xpack.fleet.packagePolicy.packageNotInstalledError', {
+          defaultMessage: 'Package {name} is not installed',
+          values: {
+            name: packagePolicy.package.name,
+          },
+        })
+      );
     }
 
-    const isInstalledVersionLessThanOrEqualToPolicyVersion = semverLte(
+    const packageInfo = await getPackageInfo({
+      savedObjectsClient: soClient,
+      pkgName: packagePolicy.package.name,
+      pkgVersion: installedPackage?.version ?? '',
+    });
+
+    const isInstalledVersionLessThanPolicyVersion = semverLt(
       packageInfo?.version ?? '',
       packagePolicy.package.version
     );
 
-    if (isInstalledVersionLessThanOrEqualToPolicyVersion) {
+    if (isInstalledVersionLessThanPolicyVersion) {
       throw new PackagePolicyIneligibleForUpgradeError(
         i18n.translate('xpack.fleet.packagePolicy.ineligibleForUpgradeError', {
           defaultMessage:
-            "Package policy {id}'s package version {version} of package {name} is up to date with the installed package. Please install the latest version of {name}.",
+            "Package policy {id}'s package version {version} of package {name} is newer than the installed package version. Please install the latest version of {name}.",
           values: {
             id: packagePolicy.id,
             name: packagePolicy.package.name,
@@ -639,15 +625,10 @@ class PackagePolicyService {
 
   public async getUpgradeDryRunDiff(
     soClient: SavedObjectsClientContract,
-    id: string,
-    packageVersion?: string
+    id: string
   ): Promise<UpgradePackagePolicyDryRunResponseItem> {
     try {
-      const { packagePolicy, packageInfo } = await this.getUpgradePackagePolicyInfo(
-        soClient,
-        id,
-        packageVersion
-      );
+      const { packagePolicy, packageInfo } = await this.getUpgradePackagePolicyInfo(soClient, id);
 
       const updatedPackagePolicy = updatePackageInputs(
         {

x-pack/test/fleet_api_integration/apis/package_policy/upgrade.ts

@@ -122,6 +122,7 @@ export default function (providerContext: FtrProviderContext) {
       });
 
       describe('dry run', function () {
+        withTestPackageVersion('0.2.0-add-non-required-test-var');
         it('returns a valid diff', async function () {
           const { body }: { body: UpgradePackagePolicyDryRunResponse } = await supertest
             .post(`/api/fleet/package_policies/upgrade/dryrun`)
@@ -144,7 +145,25 @@ export default function (providerContext: FtrProviderContext) {
       });
 
       describe('upgrade', function () {
+        withTestPackageVersion('0.2.0-add-non-required-test-var');
         it('should respond with an error', async function () {
+          // upgrade policy to 0.2.0
+          await supertest
+            .post(`/api/fleet/package_policies/upgrade`)
+            .set('kbn-xsrf', 'xxxx')
+            .send({
+              packagePolicyIds: [packagePolicyId],
+            })
+            .expect(200);
+
+          // downgrade package
+          await supertest
+            .post(`/api/fleet/epm/packages/package_policy_upgrade/0.1.0`)
+            .set('kbn-xsrf', 'xxxx')
+            .send({ force: true })
+            .expect(200);
+
+          // try upgrade policy to 0.1.0: error
           await supertest
             .post(`/api/fleet/package_policies/upgrade`)
             .set('kbn-xsrf', 'xxxx')
@@ -1103,7 +1122,7 @@ export default function (providerContext: FtrProviderContext) {
       });
 
       describe('dry run', function () {
-        it('should respond with a bad request', async function () {
+        it('should respond with a 200 ok', async function () {
           await supertest
             .post(`/api/fleet/package_policies/upgrade/dryrun`)
             .set('kbn-xsrf', 'xxxx')
@@ -1111,19 +1130,19 @@ export default function (providerContext: FtrProviderContext) {
               packagePolicyIds: [packagePolicyId],
               packageVersion: '0.1.0',
             })
-            .expect(400);
+            .expect(200);
         });
       });
 
       describe('upgrade', function () {
-        it('should respond with a bad request', async function () {
+        it('should respond with a 200 ok', async function () {
           await supertest
             .post(`/api/fleet/package_policies/upgrade`)
             .set('kbn-xsrf', 'xxxx')
             .send({
               packagePolicyIds: [packagePolicyId],
             })
-            .expect(400);
+            .expect(200);
         });
       });
     });