From 573e02d5819cdf3a42dedee4e4bc5d981bc6bd82 Mon Sep 17 00:00:00 2001
From: Zacqary Adam Xeper <Zacqary@users.noreply.github.com>
Date: Wed, 7 Jul 2021 16:23:56 -0500
Subject: [PATCH] [Fleet] Fix policy revision number getting bumped for no
 reason (#104696)

* [Fleet] Fix policy revision number getting bumped for no reason

* Add test for comparing preconfig policy to current
---
 .../server/services/preconfiguration.test.ts  | 77 ++++++++++++++++++-
 .../fleet/server/services/preconfiguration.ts | 25 ++++--
 2 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/x-pack/plugins/fleet/server/services/preconfiguration.test.ts b/x-pack/plugins/fleet/server/services/preconfiguration.test.ts
index 9b3e9b7a57369..cf06136c487e4 100644
--- a/x-pack/plugins/fleet/server/services/preconfiguration.test.ts
+++ b/x-pack/plugins/fleet/server/services/preconfiguration.test.ts
@@ -14,7 +14,10 @@ import type { AgentPolicy, NewPackagePolicy, Output } from '../types';
 
 import { AGENT_POLICY_SAVED_OBJECT_TYPE } from '../constants';
 
-import { ensurePreconfiguredPackagesAndPolicies } from './preconfiguration';
+import {
+  ensurePreconfiguredPackagesAndPolicies,
+  comparePreconfiguredPolicyToCurrent,
+} from './preconfiguration';
 
 jest.mock('./agent_policy_update');
 
@@ -279,3 +282,75 @@ describe('policy preconfiguration', () => {
     expect(policiesB[0].updated_at).toEqual(policiesA[0].updated_at);
   });
 });
+
+describe('comparePreconfiguredPolicyToCurrent', () => {
+  const baseConfig = {
+    name: 'Test policy',
+    namespace: 'default',
+    description: 'This is a test policy',
+    id: 'test-id',
+    unenroll_timeout: 60,
+    package_policies: [
+      {
+        package: { name: 'test_package' },
+        name: 'Test package',
+      },
+    ],
+  };
+
+  const basePackagePolicy: AgentPolicy = {
+    id: 'test-id',
+    namespace: 'default',
+    monitoring_enabled: ['logs', 'metrics'],
+    name: 'Test policy',
+    description: 'This is a test policy',
+    unenroll_timeout: 60,
+    is_preconfigured: true,
+    status: 'active',
+    is_managed: true,
+    revision: 1,
+    updated_at: '2021-07-07T16:29:55.144Z',
+    updated_by: 'system',
+    package_policies: [
+      {
+        package: { name: 'test_package', title: 'Test package', version: '1.0.0' },
+        name: 'Test package',
+        namespace: 'default',
+        enabled: true,
+        id: 'test-package-id',
+        revision: 1,
+        updated_at: '2021-07-07T16:29:55.144Z',
+        updated_by: 'system',
+        created_at: '2021-07-07T16:29:55.144Z',
+        created_by: 'system',
+        inputs: [],
+        policy_id: 'abc123',
+        output_id: 'default',
+      },
+    ],
+  };
+
+  it('should return hasChanged when a top-level policy field changes', () => {
+    const { hasChanged } = comparePreconfiguredPolicyToCurrent(
+      { ...baseConfig, unenroll_timeout: 120 },
+      basePackagePolicy
+    );
+    expect(hasChanged).toBe(true);
+  });
+
+  it('should not return hasChanged when no top-level fields change', () => {
+    const { hasChanged } = comparePreconfiguredPolicyToCurrent(
+      {
+        ...baseConfig,
+        package_policies: [
+          {
+            package: { name: 'different_package' },
+            name: 'Different package',
+          },
+        ],
+      },
+      basePackagePolicy
+    );
+    expect(hasChanged).toBe(false);
+  });
+});
diff --git a/x-pack/plugins/fleet/server/services/preconfiguration.ts b/x-pack/plugins/fleet/server/services/preconfiguration.ts
index f153ed3e68d30..0e24871628dcd 100644
--- a/x-pack/plugins/fleet/server/services/preconfiguration.ts
+++ b/x-pack/plugins/fleet/server/services/preconfiguration.ts
@@ -7,7 +7,7 @@
 
 import type { ElasticsearchClient, SavedObjectsClientContract } from 'src/core/server';
 import { i18n } from '@kbn/i18n';
-import { groupBy, omit, isEqual } from 'lodash';
+import { groupBy, omit, pick, isEqual } from 'lodash';
 
 import type {
   NewPackagePolicy,
@@ -142,14 +142,16 @@ export async function ensurePreconfiguredPackagesAndPolicies(
 
       if (!created) {
         if (!policy?.is_managed) return { created, policy };
-        const configTopLevelFields = omit(preconfiguredAgentPolicy, 'package_policies', 'id');
-        const currentTopLevelFields = omit(policy, 'package_policies');
-        if (!isEqual(configTopLevelFields, currentTopLevelFields)) {
+        const { hasChanged, fields } = comparePreconfiguredPolicyToCurrent(
+          preconfiguredAgentPolicy,
+          policy
+        );
+        if (hasChanged) {
           const updatedPolicy = await agentPolicyService.update(
             soClient,
             esClient,
             String(preconfiguredAgentPolicy.id),
-            configTopLevelFields
+            fields
           );
           return { created, policy: updatedPolicy };
         }
@@ -243,6 +245,19 @@ export async function ensurePreconfiguredPackagesAndPolicies(
   };
 }
 
+export function comparePreconfiguredPolicyToCurrent(
+  policyFromConfig: PreconfiguredAgentPolicy,
+  currentPolicy: AgentPolicy
+) {
+  const configTopLevelFields = omit(policyFromConfig, 'package_policies', 'id');
+  const currentTopLevelFields = pick(currentPolicy, ...Object.keys(configTopLevelFields));
+
+  return {
+    hasChanged: !isEqual(configTopLevelFields, currentTopLevelFields),
+    fields: configTopLevelFields,
+  };
+}
+
 async function addPreconfiguredPolicyPackages(
   soClient: SavedObjectsClientContract,
   esClient: ElasticsearchClient,