[Fleet] Fix bug when upgrading Windows package policies (#110698) (#1…

…10724)

* Fix bug when upgrading Windows package policies

Ensure package policy merge logics accounts for cases in which an
input/stream which previously had no variables declared but has
variables in a later package version.

Fixes #110202

* Refactor original var set into deepMergeVars

Co-authored-by: Kyle Pollich <kyle.pollich@elastic.co>

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

@@ -923,7 +923,7 @@ export function overridePackageInputs(
     // If there's no corresponding input on the original package policy, just
     // take the override value from the new package as-is. This case typically
     // occurs when inputs or package policies are added/removed between versions.
-    if (!originalInput) {
+    if (originalInput === undefined) {
       inputs.push(override as NewPackagePolicyInput);
       continue;
     }
@@ -937,7 +937,7 @@ export function overridePackageInputs(
     }
 
     if (override.vars) {
-      originalInput = deepMergeVars(originalInput, override);
+      originalInput = deepMergeVars(originalInput, override) as NewPackagePolicyInput;
     }
 
     if (override.streams) {
@@ -946,6 +946,11 @@ export function overridePackageInputs(
           (s) => s.data_stream.dataset === stream.data_stream.dataset
         );
 
+        if (originalStream === undefined) {
+          originalInput.streams.push(stream);
+          continue;
+        }
+
         if (typeof stream.enabled !== 'undefined' && originalStream) {
           originalStream.enabled = stream.enabled;
         }
@@ -994,12 +999,12 @@ export function overridePackageInputs(
 }
 
 function deepMergeVars(original: any, override: any): any {
-  const result = { ...original };
-
-  if (!result.vars || !override.vars) {
-    return;
+  if (!original.vars) {
+    original.vars = { ...override.vars };
   }
 
+  const result = { ...original };
+
   const overrideVars = Array.isArray(override.vars)
     ? override.vars
     : Object.entries(override.vars!).map(([key, rest]) => ({
@@ -1009,11 +1014,6 @@ function deepMergeVars(original: any, override: any): any {
 
   for (const { name, ...overrideVal } of overrideVars) {
     const originalVar = original.vars[name];
-
-    if (!result.vars) {
-      result.vars = {};
-    }
-
     result.vars[name] = { ...overrideVal, ...originalVar };
   }
 

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.7.0-add-stream-with-no-vars/data_stream/test_stream/agent/stream/stream.yml.hbs

@@ -0,0 +1 @@
+config.version: "2"

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.7.0-add-stream-with-no-vars/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.

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.7.0-add-stream-with-no-vars/data_stream/test_stream/manifest.yml

@@ -0,0 +1,4 @@
+title: Test stream
+type: logs
+streams:
+  - input: test_input

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.7.0-add-stream-with-no-vars/docs/README.md

@@ -0,0 +1,3 @@
+# Test package
+
+This is a test package for testing automated upgrades for package policies

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.7.0-add-stream-with-no-vars/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.7.0-add-stream-with-no-vars
+categories: []
+release: beta
+type: integration
+license: basic
+requirement:
+  elasticsearch:
+    versions: '>7.7.0'
+  kibana:
+    versions: '>7.7.0'
+policy_templates:
+  - name: package_policy_upgrade_new
+    title: Package Policy Upgrade New
+    description: Test Package for Upgrading Package Policies
+    inputs:
+      - type: test_input
+        title: Test Input
+        description: Test Input
+        enabled: true

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.8.0-add-vars-to-stream-with-no-vars/data_stream/test_stream/agent/stream/stream.yml.hbs

@@ -0,0 +1 @@
+config.version: "2"

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.8.0-add-vars-to-stream-with-no-vars/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.

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.8.0-add-vars-to-stream-with-no-vars/data_stream/test_stream/manifest.yml

@@ -0,0 +1,17 @@
+title: Test stream
+type: logs
+streams:
+  - input: test_input
+    vars:
+    - name: test_var_new
+      type: text
+      title: Test Var New
+      default: Test Var New
+      required: true
+      show_user: true
+    - name: test_var_new_2
+      type: text
+      title: Test Var New 2
+      default: Test Var New 2
+      required: true
+      show_user: true

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.8.0-add-vars-to-stream-with-no-vars/docs/README.md

@@ -0,0 +1,3 @@
+# Test package
+
+This is a test package for testing automated upgrades for package policies

x-pack/test/fleet_api_integration/apis/fixtures/test_packages/package_policy_upgrade/0.8.0-add-vars-to-stream-with-no-vars/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.8.0-add-vars-to-stream-with-no-vars
+categories: []
+release: beta
+type: integration
+license: basic
+requirement:
+  elasticsearch:
+    versions: '>7.7.0'
+  kibana:
+    versions: '>7.7.0'
+policy_templates:
+  - name: package_policy_upgrade_new
+    title: Package Policy Upgrade New
+    description: Test Package for Upgrading Package Policies
+    inputs:
+      - type: test_input
+        title: Test Input
+        description: Test Input
+        enabled: true

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

@@ -803,6 +803,126 @@ export default function (providerContext: FtrProviderContext) {
       });
     });
 
+    describe('when upgrading to a version where an input with no variables has variables added', function () {
+      withTestPackageVersion('0.8.0-add-vars-to-stream-with-no-vars');
+
+      beforeEach(async function () {
+        const { body: agentPolicyResponse } = await supertest
+          .post(`/api/fleet/agent_policies`)
+          .set('kbn-xsrf', 'xxxx')
+          .send({
+            name: 'Test policy',
+            namespace: 'default',
+          })
+          .expect(200);
+
+        agentPolicyId = agentPolicyResponse.item.id;
+
+        const { body: packagePolicyResponse } = await supertest
+          .post(`/api/fleet/package_policies`)
+          .set('kbn-xsrf', 'xxxx')
+          .send({
+            name: 'package_policy_upgrade_1',
+            description: '',
+            namespace: 'default',
+            policy_id: agentPolicyId,
+            enabled: true,
+            output_id: '',
+            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',
+              version: '0.7.0-add-stream-with-no-vars',
+            },
+          });
+
+        packagePolicyId = packagePolicyResponse.item.id;
+      });
+
+      afterEach(async function () {
+        await supertest
+          .post(`/api/fleet/package_policies/delete`)
+          .set('kbn-xsrf', 'xxxx')
+          .send({ packagePolicyIds: [packagePolicyId] })
+          .expect(200);
+
+        await supertest
+          .post('/api/fleet/agent_policies/delete')
+          .set('kbn-xsrf', 'xxxx')
+          .send({ agentPolicyId })
+          .expect(200);
+      });
+
+      describe('dry run', function () {
+        it('returns a valid diff', async function () {
+          const { body }: { body: UpgradePackagePolicyDryRunResponse } = await supertest
+            .post(`/api/fleet/package_policies/upgrade`)
+            .set('kbn-xsrf', 'xxxx')
+            .send({
+              packagePolicyIds: [packagePolicyId],
+              dryRun: true,
+            })
+            .expect(200);
+
+          expect(body[0].hasErrors).to.be(false);
+
+          const oldInput = body[0].diff?.[0].inputs.find((input) => input.type === 'test_input');
+          const oldStream = oldInput?.streams.find(
+            (stream) => stream.data_stream.dataset === 'package_policy_upgrade.test_stream'
+          );
+
+          expect(oldStream?.vars).to.be(undefined);
+
+          const newInput = body[0].diff?.[1].inputs.find((input) => input.type === 'test_input');
+          const newStream = newInput?.streams.find(
+            (stream) => stream.data_stream.dataset === 'package_policy_upgrade.test_stream'
+          );
+
+          expect(newStream?.vars).to.eql({
+            test_var_new: {
+              value: 'Test Var New',
+              type: 'text',
+            },
+            test_var_new_2: {
+              value: 'Test Var New 2',
+              type: 'text',
+            },
+          });
+        });
+      });
+
+      describe('upgrade', function () {
+        it('successfully upgrades package policy', async function () {
+          const { body }: { body: UpgradePackagePolicyResponse } = await supertest
+            .post(`/api/fleet/package_policies/upgrade`)
+            .set('kbn-xsrf', 'xxxx')
+            .send({
+              packagePolicyIds: [packagePolicyId],
+              dryRun: false,
+            })
+            .expect(200);
+
+          expect(body[0].success).to.be(true);
+        });
+      });
+    });
+
     describe('when package policy is not found', function () {
       it('should return an 200 with errors when "dryRun:true" is provided', async function () {
         const { body }: { body: UpgradePackagePolicyDryRunResponse } = await supertest