[Fleet] Improve Integration Tests for Package Policy Upgrade API (#10…

…9448) (#109699) * Improve integration tests + fix various error cases for package policy upgrade API * Fix test name * Throw string error when validation fails instead of array * Update validatePackagePolicy tests to include bool check * Address PR feedback + use IngestError class Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kyle Pollich <[email protected]>
elastic · Aug 23, 2021 · f047b20 · f047b20
1 parent e87ef74
commit f047b20
Show file tree

Hide file tree

Showing 19 changed files with 489 additions and 70 deletions.
diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts
@@ -69,6 +69,16 @@ describe('Fleet - validatePackagePolicy()', () => {
             },
           ],
         },
+        {
+          dataset: 'bar3',
+          streams: [
+            {
+              input: 'bar',
+              title: 'Bar 3',
+              vars: [{ default: true, name: 'var-name', type: 'bool' }],
+            },
+          ],
+        },
         {
           dataset: 'disabled',
           streams: [
@@ -185,6 +195,11 @@ describe('Fleet - validatePackagePolicy()', () => {
               enabled: true,
               vars: { 'var-name': { value: undefined, type: 'text' } },
             },
+            {
+              data_stream: { dataset: 'bar3', type: 'logs' },
+              enabled: true,
+              vars: { 'var-name': { value: true, type: 'text' } },
+            },
           ],
         },
         {
@@ -266,6 +281,11 @@ describe('Fleet - validatePackagePolicy()', () => {
               enabled: true,
               vars: { 'var-name': { value: undefined, type: 'text' } },
             },
+            {
+              data_stream: { dataset: 'bar3', type: 'logs' },
+              enabled: true,
+              vars: { 'var-name': { value: 'not a bool', type: 'bool' } },
+            },
           ],
         },
         {
@@ -330,6 +350,7 @@ describe('Fleet - validatePackagePolicy()', () => {
           streams: {
             bar: { vars: { 'var-name': null } },
             bar2: { vars: { 'var-name': null } },
+            bar3: { vars: { 'var-name': null } },
           },
         },
         'with-disabled-streams': {
@@ -377,6 +398,7 @@ describe('Fleet - validatePackagePolicy()', () => {
             streams: {
               bar: { vars: { 'var-name': ['var-name is required'] } },
               bar2: { vars: { 'var-name': null } },
+              bar3: { vars: { 'var-name': ['Boolean values must be either true or false'] } },
             },
           },
           'with-disabled-streams': {
@@ -440,6 +462,7 @@ describe('Fleet - validatePackagePolicy()', () => {
             streams: {
               bar: { vars: { 'var-name': null } },
               bar2: { vars: { 'var-name': null } },
+              bar3: { vars: { 'var-name': null } },
             },
           },
           'with-disabled-streams': {

diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.ts
@@ -266,6 +266,18 @@ export const validatePackagePolicyConfig = (
     }
   }
 
+  if (
+    varDef.type === 'bool' &&
+    parsedValue &&
+    !['true', 'false'].includes(parsedValue.toString())
+  ) {
+    errors.push(
+      i18n.translate('xpack.fleet.packagePolicyValidation.boolValueError', {
+        defaultMessage: 'Boolean values must be either true or false',
+      })
+    );
+  }
+
   return errors.length ? errors : null;
 };
 

diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts
@@ -7,6 +7,7 @@
 
 import { omit } from 'lodash';
 import { i18n } from '@kbn/i18n';
+import semverLte from 'semver/functions/lte';
 import { getFlattenedObject } from '@kbn/std';
 import type { KibanaRequest } from 'src/core/server';
 import type {
@@ -452,7 +453,7 @@ class PackagePolicyService {
   ) {
     const packagePolicy = await this.get(soClient, id);
     if (!packagePolicy) {
-      throw new Error(
+      throw new IngestManagerError(
         i18n.translate('xpack.fleet.packagePolicy.policyNotFoundError', {
           defaultMessage: 'Package policy with id {id} not found',
           values: { id },
@@ -461,7 +462,7 @@ class PackagePolicyService {
     }
 
     if (!packagePolicy.package?.name) {
-      throw new Error(
+      throw new IngestManagerError(
         i18n.translate('xpack.fleet.packagePolicy.packageNotFoundError', {
           defaultMessage: 'Package policy with id {id} has no named package',
           values: { id },
@@ -483,11 +484,41 @@ class PackagePolicyService {
         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 ?? '',
       });
+
+      const isInstalledVersionLessThanOrEqualToPolicyVersion = semverLte(
+        installedPackage?.version ?? '',
+        packagePolicy.package.version
+      );
+
+      if (isInstalledVersionLessThanOrEqualToPolicyVersion) {
+        throw new IngestManagerError(
+          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}.",
+            values: {
+              id: packagePolicy.id,
+              name: packagePolicy.package.name,
+              version: packagePolicy.package.version,
+            },
+          })
+        );
+      }
     }
 
     return {
@@ -527,13 +558,7 @@ class PackagePolicyService {
           updatePackagePolicy.inputs as PackagePolicyInput[]
         );
 
-        await this.update(
-          soClient,
-          esClient,
-          id,
-          omit(updatePackagePolicy, 'missingVars'),
-          options
-        );
+        await this.update(soClient, esClient, id, updatePackagePolicy, options);
         result.push({
           id,
           name: packagePolicy.name,
@@ -895,7 +920,7 @@ export function overridePackageInputs(
 
     if (!originalInput) {
       const e = {
-        error: new Error(
+        error: new IngestManagerError(
           i18n.translate('xpack.fleet.packagePolicyInputOverrideError', {
             defaultMessage: 'Input type {inputType} does not exist on package {packageName}',
             values: {
@@ -935,7 +960,7 @@ export function overridePackageInputs(
         if (!originalStream) {
           const streamSet = stream.data_stream.dataset;
           const e = {
-            error: new Error(
+            error: new IngestManagerError(
               i18n.translate('xpack.fleet.packagePolicyStreamOverrideError', {
                 defaultMessage:
                   'Data stream {streamSet} does not exist on {inputType} of package {packageName}',
@@ -990,8 +1015,17 @@ export function overridePackageInputs(
     errors = [...errors, ...responseFormattedValidationErrors];
   }
 
-  if (dryRun && errors.length) {
-    return { ...resultingPackagePolicy, errors };
+  if (errors.length) {
+    if (dryRun) {
+      return { ...resultingPackagePolicy, errors };
+    }
+
+    throw new IngestManagerError(
+      i18n.translate('xpack.fleet.packagePolicyInvalidError', {
+        defaultMessage: 'Package policy is invalid: {errors}',
+        values: { errors: errors.map(({ key, message }) => `${key}: ${message}`).join('\n') },
+      })
+    );
   }
 
   return resultingPackagePolicy;

diff --git a/...t_stream/agent/stream/test_stream.yml.hbs → ...t_stream/agent/stream/test_stream.yml.hbs b/...t_stream/agent/stream/test_stream.yml.hbs → ...t_stream/agent/stream/test_stream.yml.hbs
diff --git a/...data_stream/test_stream/fields/fields.yml → ...data_stream/test_stream/fields/fields.yml b/...data_stream/test_stream/fields/fields.yml → ...data_stream/test_stream/fields/fields.yml
diff --git a/....2.0/data_stream/test_stream/manifest.yml → ...-var/data_stream/test_stream/manifest.yml b/....2.0/data_stream/test_stream/manifest.yml → ...-var/data_stream/test_stream/manifest.yml
@@ -6,6 +6,5 @@ streams:
     - name: test_var
       type: text
       title: Test Var
-      required: true
       show_user: true
       default: Test Value
diff --git a/...ckage_policy_upgrade/0.2.0/docs/README.md → ...-add-non-required-test-var/docs/README.md b/...ckage_policy_upgrade/0.2.0/docs/README.md → ...-add-non-required-test-var/docs/README.md
diff --git a/...ixtures/test_packages/package_policy_upgrade/0.2.0-add-non-required-test-var/manifest.yml b/...ixtures/test_packages/package_policy_upgrade/0.2.0-add-non-required-test-var/manifest.yml
@@ -0,0 +1,23 @@
+format_version: 1.0.0
+name: package_policy_upgrade
+title: Tests package policy upgrades
+description: This is a test package for upgrading package policies
+version: 0.2.0-add-non-required-test-var
+categories: []
+release: beta
+type: integration
+license: basic
+requirement:
+  elasticsearch:
+    versions: '>7.7.0'
+  kibana:
+    versions: '>7.7.0'
+policy_templates:
+  - name: package_policy_upgrade
+    title: Package Policy Upgrade
+    description: Test Package for Upgrading Package Policies
+    inputs:
+      - type: test_input
+        title: Test Input
+        description: Test Input
+        enabled: true
diff --git a/...t_stream/agent/stream/test_stream.yml.hbs → ...t_stream/agent/stream/test_stream.yml.hbs b/...t_stream/agent/stream/test_stream.yml.hbs → ...t_stream/agent/stream/test_stream.yml.hbs
diff --git a/...data_stream/test_stream/fields/fields.yml → ...data_stream/test_stream/fields/fields.yml b/...data_stream/test_stream/fields/fields.yml → ...data_stream/test_stream/fields/fields.yml
diff --git a/....3.0/data_stream/test_stream/manifest.yml → ...-var/data_stream/test_stream/manifest.yml b/....3.0/data_stream/test_stream/manifest.yml → ...-var/data_stream/test_stream/manifest.yml
diff --git a/...ckage_policy_upgrade/0.3.0/docs/README.md → ...rade/0.3.0-remove-test-var/docs/README.md b/...ckage_policy_upgrade/0.3.0/docs/README.md → ...rade/0.3.0-remove-test-var/docs/README.md
diff --git a/...package_policy_upgrade/0.2.0/manifest.yml → ...pgrade/0.3.0-remove-test-var/manifest.yml b/...package_policy_upgrade/0.2.0/manifest.yml → ...pgrade/0.3.0-remove-test-var/manifest.yml
@@ -2,7 +2,7 @@ format_version: 1.0.0
 name: package_policy_upgrade
 title: Tests package policy upgrades
 description: This is a test package for upgrading package policies
-version: 0.3.0
+version: 0.3.0-remove-test-var
 categories: []
 release: beta
 type: integration

diff --git a/...grade/0.4.0-add-test-var-as-bool/data_stream/test_stream/agent/stream/test_stream.yml.hbs b/...grade/0.4.0-add-test-var-as-bool/data_stream/test_stream/agent/stream/test_stream.yml.hbs
@@ -0,0 +1 @@
+config.version: "2"
diff --git a/...ckage_policy_upgrade/0.4.0-add-test-var-as-bool/data_stream/test_stream/fields/fields.yml b/...ckage_policy_upgrade/0.4.0-add-test-var-as-bool/data_stream/test_stream/fields/fields.yml
@@ -0,0 +1,16 @@
+- name: data_stream.type
+  type: constant_keyword
+  description: >
+    Data stream type.
+- name: data_stream.dataset
+  type: constant_keyword
+  description: >
+    Data stream dataset.
+- name: data_stream.namespace
+  type: constant_keyword
+  description: >
+    Data stream namespace.
+- name: '@timestamp'
+  type: date
+  description: >
+    Event timestamp.
diff --git a/...es/package_policy_upgrade/0.4.0-add-test-var-as-bool/data_stream/test_stream/manifest.yml b/...es/package_policy_upgrade/0.4.0-add-test-var-as-bool/data_stream/test_stream/manifest.yml
@@ -0,0 +1,10 @@
+title: Test stream
+type: logs
+streams:
+  - input: test_input
+    vars:
+    - name: test_var
+      type: bool
+      title: Test Var
+      required: true
+      show_user: true
diff --git a/.../test_packages/package_policy_upgrade/0.4.0-add-test-var-as-bool/docs/README.md b/.../test_packages/package_policy_upgrade/0.4.0-add-test-var-as-bool/docs/README.md
@@ -0,0 +1,3 @@
+# Test package
+
+This is a test package for testing automated upgrades for package policies
diff --git a/...package_policy_upgrade/0.3.0/manifest.yml → ...e/0.4.0-add-test-var-as-bool/manifest.yml b/...package_policy_upgrade/0.3.0/manifest.yml → ...e/0.4.0-add-test-var-as-bool/manifest.yml
@@ -2,7 +2,7 @@ format_version: 1.0.0
 name: package_policy_upgrade
 title: Tests package policy upgrades
 description: This is a test package for upgrading package policies
-version: 0.3.0
+version: 0.4.0-add-test-var-as-bool
 categories: []
 release: beta
 type: integration
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		# Test package

		This is a test package for testing automated upgrades for package policies