[Fleet] Improve API logic for package policy upgrades (#108924)

* Improve API logic for package policy upgrades - Allow dry runs to be invoked for non-installed package versions - Re-use existing validation service for validating package policies following an attempted dry run or upgrade See https://github.com/elastic/kibana/issues/106048#issuecomment-899747732 for more details Ref #106048 * Fix input overriding in dry run/upgrade APIs * Fix i18n * Fix types * Fix var merge logic
2025-04-23 17:28:26 -04:00 · 2021-08-17 21:43:55 -04:00 · 2021-08-17 21:43:55 -04:00 · b1253db197
commit b1253db197
parent 4e0375f626
7 changed files with 127 additions and 117 deletions
--- a/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
+++ b/x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
@ -203,7 +203,11 @@ export const upgradePackagePolicyHandler: RequestHandler<
      const body: UpgradePackagePolicyDryRunResponse = [];

      for (const id of request.body.packagePolicyIds) {
-        const result = await packagePolicyService.getUpgradeDryRunDiff(soClient, id);
+        const result = await packagePolicyService.getUpgradeDryRunDiff(
+          soClient,
+          id,
+          request.body.packageVersion
+        );
        body.push(result);
      }
      return response.ok({
--- 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 { getFlattenedObject } from '@kbn/std';
 import type { KibanaRequest } from 'src/core/server';
 import type {
  ElasticsearchClient,
@ -21,6 +22,8 @@ import {
  packageToPackagePolicyInputs,
  isPackageLimited,
  doesAgentPolicyAlreadyIncludePackage,
+  validatePackagePolicy,
+  validationHasErrors,
 } from '../../common';
 import type {
  DeletePackagePoliciesResponse,
@ -442,7 +445,11 @@ class PackagePolicyService {
    return result;
  }

-  public async getUpgradePackagePolicyInfo(soClient: SavedObjectsClientContract, id: string) {
+  public async getUpgradePackagePolicyInfo(
+    soClient: SavedObjectsClientContract,
+    id: string,
+    packageVersion?: string
+  ) {
    const packagePolicy = await this.get(soClient, id);
    if (!packagePolicy) {
      throw new Error(
@ -462,28 +469,30 @@ class PackagePolicyService {
      );
    }

-    const installedPackage = await getInstallation({
-      savedObjectsClient: soClient,
-      pkgName: packagePolicy.package.name,
-    });
-    if (!installedPackage) {
-      throw new Error(
-        i18n.translate('xpack.fleet.packagePolicy.packageNotInstalledError', {
-          defaultMessage: 'Cannot upgrade package policy {id} because {pkgName} is not installed',
-          values: { id, pkgName: packagePolicy.package.name },
-        })
-      );
-    }
+    let packageInfo: PackageInfo;

-    const installedPkgInfo = await getPackageInfo({
-      savedObjectsClient: soClient,
-      pkgName: packagePolicy.package.name,
-      pkgVersion: installedPackage.version,
-    });
+    if (packageVersion) {
+      packageInfo = await getPackageInfo({
+        savedObjectsClient: soClient,
+        pkgName: packagePolicy.package.name,
+        pkgVersion: packageVersion,
+      });
+    } else {
+      const installedPackage = await getInstallation({
+        savedObjectsClient: soClient,
+        pkgName: packagePolicy.package.name,
+      });
+
+      packageInfo = await getPackageInfo({
+        savedObjectsClient: soClient,
+        pkgName: packagePolicy.package.name,
+        pkgVersion: installedPackage?.version ?? '',
+      });
+    }

    return {
      packagePolicy: packagePolicy as Required<PackagePolicy>,
-      installedPkgInfo,
+      packageInfo,
    };
  }

@ -497,25 +506,23 @@ class PackagePolicyService {

    for (const id of ids) {
      try {
-        const { packagePolicy, installedPkgInfo } = await this.getUpgradePackagePolicyInfo(
-          soClient,
-          id
-        );
+        const { packagePolicy, packageInfo } = await this.getUpgradePackagePolicyInfo(soClient, id);

        const updatePackagePolicy = overridePackageInputs(
          {
            ...omit(packagePolicy, 'id'),
-            inputs: packageToPackagePolicyInputs(installedPkgInfo),
+            inputs: packagePolicy.inputs,
            package: {
              ...packagePolicy.package,
-              version: installedPkgInfo.version,
+              version: packageInfo.version,
            },
          },
-          packagePolicy.inputs as InputsOverride[]
+          packageInfo,
+          packageToPackagePolicyInputs(packageInfo) as InputsOverride[]
        );

        updatePackagePolicy.inputs = await this.compilePackagePolicyInputs(
-          installedPkgInfo,
+          packageInfo,
          updatePackagePolicy.vars || {},
          updatePackagePolicy.inputs as PackagePolicyInput[]
        );
@ -546,29 +553,32 @@ class PackagePolicyService {

  public async getUpgradeDryRunDiff(
    soClient: SavedObjectsClientContract,
-    id: string
+    id: string,
+    packageVersion?: string
  ): Promise<UpgradePackagePolicyDryRunResponseItem> {
    try {
-      const { packagePolicy, installedPkgInfo } = await this.getUpgradePackagePolicyInfo(
+      const { packagePolicy, packageInfo } = await this.getUpgradePackagePolicyInfo(
        soClient,
-        id
+        id,
+        packageVersion
      );

      const updatedPackagePolicy = overridePackageInputs(
        {
          ...omit(packagePolicy, 'id'),
-          inputs: packageToPackagePolicyInputs(installedPkgInfo),
+          inputs: packagePolicy.inputs,
          package: {
            ...packagePolicy.package,
-            version: installedPkgInfo.version,
+            version: packageInfo.version,
          },
        },
-        packagePolicy.inputs as InputsOverride[],
+        packageInfo,
+        packageToPackagePolicyInputs(packageInfo) as InputsOverride[],
        true
      );

      updatedPackagePolicy.inputs = await this.compilePackagePolicyInputs(
-        installedPkgInfo,
+        packageInfo,
        updatedPackagePolicy.vars || {},
        updatedPackagePolicy.inputs as PackagePolicyInput[]
      );
@ -849,6 +859,7 @@ export type { PackagePolicyService };

 export function overridePackageInputs(
  basePackagePolicy: NewPackagePolicy,
+  packageInfo: PackageInfo,
  inputsOverride?: InputsOverride[],
  dryRun?: boolean
 ): DryRunPackagePolicy {
@ -856,11 +867,11 @@ export function overridePackageInputs(

  const inputs = [...basePackagePolicy.inputs];
  const packageName = basePackagePolicy.package!.name;
-  const errors = [];
-  let responseMissingVars: string[] = [];
+  let errors = [];

  for (const override of inputsOverride) {
    let originalInput = inputs.find((i) => i.type === override.type);
+
    if (!originalInput) {
      const e = {
        error: new Error(
@ -874,13 +885,16 @@ export function overridePackageInputs(
        ),
        package: { name: packageName, version: basePackagePolicy.package!.version },
      };
+
      if (dryRun) {
        errors.push({
          key: override.type,
          message: String(e.error),
        });
        continue;
-      } else throw e;
+      } else {
+        throw e;
+      }
    }

    if (typeof override.enabled !== 'undefined') originalInput.enabled = override.enabled;
@ -888,33 +902,7 @@ export function overridePackageInputs(
      originalInput.keep_enabled = override.keep_enabled;

    if (override.vars) {
-      try {
-        const { result, missingVars } = deepMergeVars(override, originalInput);
-        originalInput = result;
-        responseMissingVars = [...responseMissingVars, ...missingVars];
-      } catch (e) {
-        const varName = e.message;
-        const err = {
-          error: new Error(
-            i18n.translate('xpack.fleet.packagePolicyVarOverrideError', {
-              defaultMessage:
-                'Var {varName} does not exist on {inputType} of package {packageName}',
-              values: {
-                varName,
-                inputType: override.type,
-                packageName,
-              },
-            })
-          ),
-          package: { name: packageName, version: basePackagePolicy.package!.version },
-        };
-        if (dryRun) {
-          errors.push({
-            key: `${override.type}.vars.${varName}`,
-            message: String(err.error),
-          });
-        } else throw err;
-      }
+      originalInput = deepMergeVars(originalInput, override);
    }

    if (override.streams) {
@ -922,6 +910,7 @@ export function overridePackageInputs(
        let originalStream = originalInput?.streams.find(
          (s) => s.data_stream.dataset === stream.data_stream.dataset
        );
+
        if (!originalStream) {
          const streamSet = stream.data_stream.dataset;
          const e = {
@ -938,62 +927,61 @@ export function overridePackageInputs(
            ),
            package: { name: packageName, version: basePackagePolicy.package!.version },
          };
+
          if (dryRun) {
            errors.push({
              key: `${override.type}.streams.${streamSet}`,
              message: String(e.error),
            });
+
            continue;
-          } else throw e;
+          } else {
+            throw e;
+          }
        }

-        if (typeof stream.enabled !== 'undefined') originalStream.enabled = stream.enabled;
+        if (typeof stream.enabled !== 'undefined') {
+          originalStream.enabled = stream.enabled;
+        }

        if (stream.vars) {
-          try {
-            const { result, missingVars } = deepMergeVars(stream as InputsOverride, originalStream);
-            originalStream = result;
-            responseMissingVars = [...responseMissingVars, ...missingVars];
-          } catch (e) {
-            const varName = e.message;
-            const streamSet = stream.data_stream.dataset;
-            const err = {
-              error: new Error(
-                i18n.translate('xpack.fleet.packagePolicyStreamVarOverrideError', {
-                  defaultMessage:
-                    'Var {varName} does not exist on {streamSet} for {inputType} of package {packageName}',
-                  values: {
-                    varName,
-                    streamSet,
-                    inputType: override.type,
-                    packageName,
-                  },
-                })
-              ),
-              package: { name: packageName, version: basePackagePolicy.package!.version },
-            };
-            if (dryRun) {
-              errors.push({
-                key: `${override.type}.streams.${streamSet}.${varName}`,
-                message: String(err.error),
-              });
-            } else throw err;
-          }
+          originalStream = deepMergeVars(originalStream, stream as InputsOverride);
        }
      }
    }
  }

-  if (dryRun && errors.length) {
-    return { ...basePackagePolicy, inputs, errors, missingVars: responseMissingVars };
+  const resultingPackagePolicy: NewPackagePolicy = {
+    ...basePackagePolicy,
+    inputs,
+  };
+
+  const validationResults = validatePackagePolicy(resultingPackagePolicy, packageInfo);
+
+  if (validationHasErrors(validationResults)) {
+    const responseFormattedValidationErrors = Object.entries(getFlattenedObject(validationResults))
+      .map(([key, value]) => ({
+        key,
+        message: value,
+      }))
+      .filter(({ message }) => !!message);
+
+    errors = [...errors, ...responseFormattedValidationErrors];
  }

-  return { ...basePackagePolicy, inputs, missingVars: responseMissingVars };
+  if (dryRun && errors.length) {
+    return { ...resultingPackagePolicy, errors };
+  }
+
+  return resultingPackagePolicy;
 }

-function deepMergeVars(override: any, original: any): { result: any; missingVars: string[] } {
+function deepMergeVars(original: any, override: any): any {
  const result = { ...original };
-  const missingVars: string[] = [];
+
+  if (!result.vars || !override.vars) {
+    return;
+  }

  const overrideVars = Array.isArray(override.vars)
    ? override.vars
@ -1002,15 +990,15 @@ function deepMergeVars(override: any, original: any): { result: any; missingVars
        ...(rest as any),
      }));

-  for (const { name, ...val } of overrideVars) {
-    if (!original.vars || !(name in original.vars)) {
-      missingVars.push(name);
-      continue;
+  for (const { name, ...overrideVal } of overrideVars) {
+    const originalVar = original.vars[name];
+
+    if (!result.vars) {
+      result.vars = {};
    }

-    const originalVar = original.vars[name];
-    result[name] = { ...originalVar, ...val };
+    result.vars[name] = { ...overrideVal, ...originalVar };
  }

-  return { result, missingVars };
+  return result;
 }
--- a/x-pack/plugins/fleet/server/services/preconfiguration.ts
+++ b/x-pack/plugins/fleet/server/services/preconfiguration.ts
@ -28,7 +28,7 @@ import {
 import { escapeSearchQueryPhrase } from './saved_object';

 import { pkgToPkgKey } from './epm/registry';
-import { getInstallation } from './epm/packages';
+import { getInstallation, getPackageInfo } from './epm/packages';
 import { ensurePackagesCompletedInstall } from './epm/packages/install';
 import { bulkInstallPackages } from './epm/packages/bulk_install_packages';
 import { agentPolicyService, addPackageToAgentPolicy } from './agent_policy';
@ -277,6 +277,12 @@ async function addPreconfiguredPolicyPackages(
 ) {
  // Add packages synchronously to avoid overwriting
  for (const { installedPackage, name, description, inputs } of installedPackagePolicies) {
+    const packageInfo = await getPackageInfo({
+      savedObjectsClient: soClient,
+      pkgName: installedPackage.name,
+      pkgVersion: installedPackage.version,
+    });
+
    await addPackageToAgentPolicy(
      soClient,
      esClient,
@ -285,7 +291,7 @@ async function addPreconfiguredPolicyPackages(
      defaultOutput,
      name,
      description,
-      (policy) => overridePackageInputs(policy, inputs)
+      (policy) => overridePackageInputs(policy, packageInfo, inputs)
    );
  }
 }
--- a/x-pack/plugins/fleet/server/types/rest_spec/package_policy.ts
+++ b/x-pack/plugins/fleet/server/types/rest_spec/package_policy.ts
@ -41,5 +41,6 @@ export const UpgradePackagePoliciesRequestSchema = {
  body: schema.object({
    packagePolicyIds: schema.arrayOf(schema.string()),
    dryRun: schema.maybe(schema.boolean()),
+    packageVersion: schema.maybe(schema.string()),
  }),
 };
--- a/x-pack/plugins/translations/translations/ja-JP.json
+++ b/x-pack/plugins/translations/translations/ja-JP.json
@ -9814,13 +9814,11 @@
    "xpack.fleet.overviewPageTitle": "Fleet",
    "xpack.fleet.packagePolicyInputOverrideError": "パッケージ{packageName}には入力タイプ{inputType}が存在しません。",
    "xpack.fleet.packagePolicyStreamOverrideError": "パッケージ{packageName}の{inputType}にはデータストリーム{streamSet}が存在しません",
-    "xpack.fleet.packagePolicyStreamVarOverrideError": "パッケージ{packageName}の{inputType}の{streamSet}にはVar {varName}が存在しません",
    "xpack.fleet.packagePolicyValidation.invalidArrayErrorMessage": "無効なフォーマット",
    "xpack.fleet.packagePolicyValidation.invalidYamlFormatErrorMessage": "YAML形式が無効です",
    "xpack.fleet.packagePolicyValidation.nameRequiredErrorMessage": "名前が必要です",
    "xpack.fleet.packagePolicyValidation.quoteStringErrorMessage": "*や&amp;などの特殊YAML文字で始まる文字列は二重引用符で囲む必要があります。",
    "xpack.fleet.packagePolicyValidation.requiredErrorMessage": "{fieldName}が必要です",
-    "xpack.fleet.packagePolicyVarOverrideError": "パッケージ{packageName}の{inputType}にはVar {varName}が存在しません",
    "xpack.fleet.permissionDeniedErrorMessage": "Fleet へのアクセスが許可されていません。Fleet には{roleName}権限が必要です。",
    "xpack.fleet.permissionDeniedErrorTitle": "パーミッションが拒否されました",
    "xpack.fleet.permissionsRequestErrorMessageDescription": "Fleet アクセス権の確認中に問題が発生しました",
--- a/x-pack/plugins/translations/translations/zh-CN.json
+++ b/x-pack/plugins/translations/translations/zh-CN.json
@ -10075,13 +10075,11 @@
    "xpack.fleet.overviewPageTitle": "Fleet",
    "xpack.fleet.packagePolicyInputOverrideError": "输入类型 {inputType} 在软件包 {packageName} 上不存在",
    "xpack.fleet.packagePolicyStreamOverrideError": "数据流 {streamSet} 在软件包 {packageName} 的 {inputType} 上不存在",
-    "xpack.fleet.packagePolicyStreamVarOverrideError": "变量 {varName} 在软件包 {packageName} 的 {inputType} 的 {streamSet} 上不存在",
    "xpack.fleet.packagePolicyValidation.invalidArrayErrorMessage": "格式无效",
    "xpack.fleet.packagePolicyValidation.invalidYamlFormatErrorMessage": "YAML 格式无效",
    "xpack.fleet.packagePolicyValidation.nameRequiredErrorMessage": "“名称”必填",
    "xpack.fleet.packagePolicyValidation.quoteStringErrorMessage": "以特殊 YAML 字符（* 或 &amp;）开头的字符串需要使用双引号引起。",
    "xpack.fleet.packagePolicyValidation.requiredErrorMessage": "“{fieldName}”必填",
-    "xpack.fleet.packagePolicyVarOverrideError": "变量 {varName} 在软件包 {packageName} 的 {inputType} 上不存在",
    "xpack.fleet.permissionDeniedErrorMessage": "您无权访问 Fleet。Fleet 需要 {roleName} 权限。",
    "xpack.fleet.permissionDeniedErrorTitle": "权限被拒绝",
    "xpack.fleet.permissionsRequestErrorMessageDescription": "检查 Fleet 权限时遇到问题",
--- a/x-pack/test/fleet_api_integration/apis/package_policy/upgrade.ts
+++ b/x-pack/test/fleet_api_integration/apis/package_policy/upgrade.ts
@ -77,7 +77,23 @@ export default function (providerContext: FtrProviderContext) {
            policy_id: agentPolicyId,
            enabled: true,
            output_id: '',
-            inputs: [],
+            inputs: [
+              {
+                policy_template: 'package_policy_upgrade',
+                type: 'test_input',
+                enabled: true,
+                streams: [
+                  {
+                    id: 'test-package_policy_upgrade-xxxx',
+                    enabled: true,
+                    data_stream: {
+                      type: 'test_stream',
+                      dataset: 'package_policy_upgrade.test_stream',
+                    },
+                  },
+                ],
+              },
+            ],
            package: {
              name: 'package_policy_upgrade',
              title: 'This is a test package for upgrading package policies',
@ -226,7 +242,7 @@ export default function (providerContext: FtrProviderContext) {
      });

      describe('when "dryRun: true" is provided', function () {
-        it('should return a diff with missingVars', async function () {
+        it('should return a diff with no errors', async function () {
          const { body }: { body: UpgradePackagePolicyDryRunResponse } = await supertest
            .post(`/api/fleet/package_policies/upgrade`)
            .set('kbn-xsrf', 'xxxx')
@ -239,7 +255,6 @@ export default function (providerContext: FtrProviderContext) {
          expect(body.length).to.be(1);
          expect(body[0].diff?.length).to.be(2);
          expect(body[0].hasErrors).to.be(false);
-          expect(body[0].diff?.[1].missingVars).to.contain('test_var');
        });
      });