From f92e7a020ad82f7e51cbcf9527a79029ab2770ae Mon Sep 17 00:00:00 2001
From: Nicolas Chaulet <nicolas.chaulet@elastic.co>
Date: Mon, 21 Sep 2020 17:06:42 -0400
Subject: [PATCH] [Ingest Manager] Fix agent action acknowledgement

---
 .../server/services/agents/acks.test.ts       | 110 +++++++++++++++++-
 .../server/services/agents/acks.ts            |   8 +-
 2 files changed, 111 insertions(+), 7 deletions(-)

diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts
index 866aa587b8a56a..c7b40988038271 100644
--- a/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts
+++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.test.ts
@@ -57,7 +57,7 @@ describe('test agent acks services', () => {
     );
   });
 
-  it('should update config field on the agent if a policy change is acknowledged', async () => {
+  it('should update config field on the agent if a policy change is acknowledged with an agent without policy', async () => {
     const mockSavedObjectsClient = savedObjectsClientMock.create();
 
     const actionAttributes = {
@@ -116,6 +116,114 @@ describe('test agent acks services', () => {
     `);
   });
 
+  it('should update config field on the agent if a policy change is acknowledged with a higher revision than the agent one', async () => {
+    const mockSavedObjectsClient = savedObjectsClientMock.create();
+
+    const actionAttributes = {
+      type: 'CONFIG_CHANGE',
+      policy_id: 'policy1',
+      policy_revision: 4,
+      sent_at: '2020-03-14T19:45:02.620Z',
+      timestamp: '2019-01-04T14:32:03.36764-05:00',
+      created_at: '2020-03-14T19:45:02.620Z',
+      ack_data: JSON.stringify({ packages: ['system'] }),
+    };
+
+    mockSavedObjectsClient.bulkGet.mockReturnValue(
+      Promise.resolve({
+        saved_objects: [
+          {
+            id: 'action2',
+            references: [],
+            type: AGENT_ACTION_SAVED_OBJECT_TYPE,
+            attributes: actionAttributes,
+          },
+        ],
+      } as SavedObjectsBulkResponse<BaseAgentActionSOAttributes>)
+    );
+
+    await acknowledgeAgentActions(
+      mockSavedObjectsClient,
+      ({
+        id: 'id',
+        type: AGENT_TYPE_PERMANENT,
+        policy_id: 'policy1',
+        policy_revision: 3,
+      } as unknown) as Agent,
+      [
+        {
+          type: 'ACTION_RESULT',
+          subtype: 'CONFIG',
+          timestamp: '2019-01-04T14:32:03.36764-05:00',
+          action_id: 'action2',
+          agent_id: 'id',
+        } as AgentEvent,
+      ]
+    );
+    expect(mockSavedObjectsClient.bulkUpdate).toBeCalled();
+    expect(mockSavedObjectsClient.bulkUpdate.mock.calls[0][0]).toHaveLength(1);
+    expect(mockSavedObjectsClient.bulkUpdate.mock.calls[0][0][0]).toMatchInlineSnapshot(`
+      Object {
+        "attributes": Object {
+          "packages": Array [
+            "system",
+          ],
+          "policy_revision": 4,
+        },
+        "id": "id",
+        "type": "fleet-agents",
+      }
+    `);
+  });
+
+  it('should not update config field on the agent if a policy change is acknowledged with a lower revision than the agent one', async () => {
+    const mockSavedObjectsClient = savedObjectsClientMock.create();
+
+    const actionAttributes = {
+      type: 'CONFIG_CHANGE',
+      policy_id: 'policy1',
+      policy_revision: 4,
+      sent_at: '2020-03-14T19:45:02.620Z',
+      timestamp: '2019-01-04T14:32:03.36764-05:00',
+      created_at: '2020-03-14T19:45:02.620Z',
+      ack_data: JSON.stringify({ packages: ['system'] }),
+    };
+
+    mockSavedObjectsClient.bulkGet.mockReturnValue(
+      Promise.resolve({
+        saved_objects: [
+          {
+            id: 'action2',
+            references: [],
+            type: AGENT_ACTION_SAVED_OBJECT_TYPE,
+            attributes: actionAttributes,
+          },
+        ],
+      } as SavedObjectsBulkResponse<BaseAgentActionSOAttributes>)
+    );
+
+    await acknowledgeAgentActions(
+      mockSavedObjectsClient,
+      ({
+        id: 'id',
+        type: AGENT_TYPE_PERMANENT,
+        policy_id: 'policy1',
+        policy_revision: 5,
+      } as unknown) as Agent,
+      [
+        {
+          type: 'ACTION_RESULT',
+          subtype: 'CONFIG',
+          timestamp: '2019-01-04T14:32:03.36764-05:00',
+          action_id: 'action2',
+          agent_id: 'id',
+        } as AgentEvent,
+      ]
+    );
+    expect(mockSavedObjectsClient.bulkUpdate).toBeCalled();
+    expect(mockSavedObjectsClient.bulkUpdate.mock.calls[0][0]).toHaveLength(0);
+  });
+
   it('should not update config field on the agent if a policy change for an old revision is acknowledged', async () => {
     const mockSavedObjectsClient = savedObjectsClientMock.create();
 
diff --git a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts
index d29dfcec7ef307..1392710eb0eff4 100644
--- a/x-pack/plugins/ingest_manager/server/services/agents/acks.ts
+++ b/x-pack/plugins/ingest_manager/server/services/agents/acks.ts
@@ -139,16 +139,12 @@ function getLatestConfigChangePolicyActionIfUpdated(
       !isAgentPolicyAction(action) ||
       action.type !== 'CONFIG_CHANGE' ||
       action.policy_id !== agent.policy_id ||
-      (acc?.policy_revision ?? 0) < (agent.policy_revision || 0)
+      (action?.policy_revision ?? 0) < (agent.policy_revision || 0)
     ) {
       return acc;
     }
 
-    if (action.policy_revision > (acc?.policy_revision ?? 0)) {
-      return action;
-    }
-
-    return acc;
+    return action;
   }, null);
 }