[Fleet] Force unenroll in Agent activity (#141208)

* fix for force unenroll * fixed tests * fixed checks, solve for one more edge case: only updating unenroll actions that do not have results yet * added more tests * added try catch * updated description on flyout Co-authored-by: Kibana Machine <[email protected]>
elastic · Sep 22, 2022 · 3433f5d · 3433f5d
1 parent 85f5544
commit 3433f5d
Show file tree

Hide file tree

Showing 9 changed files with 311 additions and 51 deletions.
diff --git a/x-pack/plugins/fleet/common/types/models/agent.ts b/x-pack/plugins/fleet/common/types/models/agent.ts
@@ -30,7 +30,13 @@ export type AgentStatus =
 
 export type SimplifiedAgentStatus = 'healthy' | 'unhealthy' | 'updating' | 'offline' | 'inactive';
 
-export type AgentActionType = 'UNENROLL' | 'UPGRADE' | 'SETTINGS' | 'POLICY_REASSIGN' | 'CANCEL';
+export type AgentActionType =
+  | 'UNENROLL'
+  | 'UPGRADE'
+  | 'SETTINGS'
+  | 'POLICY_REASSIGN'
+  | 'CANCEL'
+  | 'FORCE_UNENROLL';
 
 type FleetServerAgentComponentStatusTuple = typeof FleetServerAgentComponentStatuses;
 export type FleetServerAgentComponentStatus = FleetServerAgentComponentStatusTuple[number];

diff --git a/...c/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx b/...c/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx
@@ -143,7 +143,7 @@ export const AgentActivityFlyout: React.FunctionComponent<{
                   body={
                     <FormattedMessage
                       id="xpack.fleet.agentActivityFlyout.noActivityDescription"
-                      defaultMessage="Activity feed will appear here as agents get enrolled, upgraded, or configured."
+                      defaultMessage="Activity feed will appear here as agents are reassigned, upgraded, or unenrolled."
                     />
                   }
                 />
@@ -238,6 +238,11 @@ const actionNames: {
     completedText: 'unenrolled',
     cancelledText: 'unenrollment',
   },
+  FORCE_UNENROLL: {
+    inProgressText: 'Force unenrolling',
+    completedText: 'force unenrolled',
+    cancelledText: 'force unenrollment',
+  },
   CANCEL: { inProgressText: 'Cancelling', completedText: 'cancelled', cancelledText: '' },
   ACTION: { inProgressText: 'Actioning', completedText: 'actioned', cancelledText: 'action' },
 };

diff --git a/x-pack/plugins/fleet/server/services/agents/action.mock.ts b/x-pack/plugins/fleet/server/services/agents/action.mock.ts
@@ -124,6 +124,8 @@ export function createClientMock() {
     };
   });
 
+  esClientMock.search.mockResolvedValue({ hits: { hits: [] } } as any);
+
   return {
     soClient: soClientMock,
     esClient: esClientMock,

diff --git a/x-pack/plugins/fleet/server/services/agents/action_status.ts b/x-pack/plugins/fleet/server/services/agents/action_status.ts
@@ -36,7 +36,7 @@ export async function getActionStatuses(
       size: 0,
       aggs: {
         ack_counts: {
-          terms: { field: 'action_id' },
+          terms: { field: 'action_id', size: actions.length || 10 },
           aggs: {
             max_timestamp: { max: { field: '@timestamp' } },
           },
@@ -61,7 +61,7 @@ export async function getActionStatuses(
     const nbAgentsAck = matchingBucket?.doc_count ?? 0;
     const completionTime = (matchingBucket?.max_timestamp as any)?.value_as_string;
     const nbAgentsActioned = action.nbAgentsActioned || action.nbAgentsActionCreated;
-    const complete = nbAgentsAck === nbAgentsActioned;
+    const complete = nbAgentsAck >= nbAgentsActioned;
     const cancelledAction = cancelledActions.find((a) => a.actionId === action.actionId);
 
     let errorCount = 0;
@@ -161,13 +161,6 @@ async function _getActions(
             },
           },
         ],
-        must: [
-          {
-            exists: {
-              field: 'agents',
-            },
-          },
-        ],
       },
     },
     body: {

diff --git a/x-pack/plugins/fleet/server/services/agents/actions.ts b/x-pack/plugins/fleet/server/services/agents/actions.ts
@@ -106,7 +106,7 @@ export async function bulkCreateAgentActionResults(
   results: Array<{
     actionId: string;
     agentId: string;
-    error: string;
+    error?: string;
   }>
 ): Promise<void> {
   if (results.length === 0) {
@@ -164,6 +164,41 @@ export async function getAgentActions(esClient: ElasticsearchClient, actionId: s
   }));
 }
 
+export async function getUnenrollAgentActions(
+  esClient: ElasticsearchClient
+): Promise<FleetServerAgentAction[]> {
+  const res = await esClient.search<FleetServerAgentAction>({
+    index: AGENT_ACTIONS_INDEX,
+    query: {
+      bool: {
+        must: [
+          {
+            term: {
+              type: 'UNENROLL',
+            },
+          },
+          {
+            exists: {
+              field: 'agents',
+            },
+          },
+          {
+            range: {
+              expiration: { gte: new Date().toISOString() },
+            },
+          },
+        ],
+      },
+    },
+    size: SO_SEARCH_LIMIT,
+  });
+
+  return res.hits.hits.map((hit) => ({
+    ...hit._source,
+    id: hit._id,
+  }));
+}
+
 export async function cancelAgentAction(esClient: ElasticsearchClient, actionId: string) {
   const res = await esClient.search<FleetServerAgentAction>({
     index: AGENT_ACTIONS_INDEX,

diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll.test.ts b/x-pack/plugins/fleet/server/services/agents/unenroll.test.ts
@@ -6,15 +6,19 @@
  */
 import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
 
+import { AGENT_ACTIONS_INDEX } from '../../../common';
+
 import { HostedAgentPolicyRestrictionRelatedError } from '../../errors';
 import { invalidateAPIKeys } from '../api_keys';
 
 import { appContextService } from '../app_context';
 
 import { createAppContextStartContractMock } from '../../mocks';
 
+import type { Agent } from '../../types';
+
 import { unenrollAgent, unenrollAgents } from './unenroll';
-import { invalidateAPIKeysForAgents } from './unenroll_action_runner';
+import { invalidateAPIKeysForAgents, isAgentUnenrolled } from './unenroll_action_runner';
 import { createClientMock } from './action.mock';
 
 jest.mock('../api_keys');
@@ -137,6 +141,78 @@ describe('unenrollAgents (plural)', () => {
     expect(calledWithActionResults.body?.[1] as any).toEqual(expectedObject);
   });
 
+  it('force unenroll updates in progress unenroll actions', async () => {
+    const { soClient, esClient, agentInRegularDoc, agentInRegularDoc2 } = createClientMock();
+    esClient.search.mockReset();
+    esClient.search.mockImplementation((request) =>
+      Promise.resolve(
+        request?.index === AGENT_ACTIONS_INDEX
+          ? ({
+              hits: {
+                hits: [
+                  {
+                    _source: {
+                      agents: ['agent-in-regular-policy'],
+                      action_id: 'other-action',
+                    },
+                  },
+                ],
+              },
+            } as any)
+          : { hits: { hits: [] } }
+      )
+    );
+
+    const idsToUnenroll = [agentInRegularDoc._id, agentInRegularDoc2._id];
+    await unenrollAgents(soClient, esClient, {
+      agentIds: idsToUnenroll,
+      revoke: true,
+    });
+
+    expect(esClient.bulk.mock.calls.length).toEqual(3);
+    const bulkBody = (esClient.bulk.mock.calls[1][0] as estypes.BulkRequest)?.body?.[1] as any;
+    expect(bulkBody.agent_id).toEqual(agentInRegularDoc._id);
+    expect(bulkBody.action_id).toEqual('other-action');
+  });
+
+  it('force unenroll should not update completed unenroll actions', async () => {
+    const { soClient, esClient, agentInRegularDoc, agentInRegularDoc2 } = createClientMock();
+    esClient.search.mockReset();
+    esClient.search.mockImplementation((request) =>
+      Promise.resolve(
+        request?.index === AGENT_ACTIONS_INDEX
+          ? ({
+              hits: {
+                hits: [
+                  {
+                    _source: {
+                      agents: ['agent-in-regular-policy'],
+                      action_id: 'other-action1',
+                    },
+                  },
+                ],
+              },
+            } as any)
+          : {
+              hits: {
+                hits: [
+                  { _source: { action_id: 'other-action1', agent_id: 'agent-in-regular-policy' } },
+                ],
+              },
+            }
+      )
+    );
+
+    const idsToUnenroll = [agentInRegularDoc._id, agentInRegularDoc2._id];
+    await unenrollAgents(soClient, esClient, {
+      agentIds: idsToUnenroll,
+      revoke: true,
+    });
+
+    // agent and force unenroll results updated, no other action results
+    expect(esClient.bulk.mock.calls.length).toEqual(2);
+  });
+
   it('cannot unenroll from a hosted agent policy with revoke=true', async () => {
     const { soClient, esClient, agentInHostedDoc, agentInRegularDoc, agentInRegularDoc2 } =
       createClientMock();
@@ -165,7 +241,7 @@ describe('unenrollAgents (plural)', () => {
 
     // calls ES update with correct values
     const onlyRegular = [agentInRegularDoc._id, agentInRegularDoc2._id];
-    const calledWith = esClient.bulk.mock.calls[0][0];
+    const calledWith = esClient.bulk.mock.calls[2][0];
     const ids = (calledWith as estypes.BulkRequest)?.body
       ?.filter((i: any) => i.update !== undefined)
       .map((i: any) => i.update._id);
@@ -176,6 +252,21 @@ describe('unenrollAgents (plural)', () => {
     for (const doc of docs!) {
       expect(doc).toHaveProperty('unenrolled_at');
     }
+
+    const errorResults = esClient.bulk.mock.calls[1][0];
+    const errorIds = (errorResults as estypes.BulkRequest)?.body
+      ?.filter((i: any) => i.agent_id)
+      .map((i: any) => i.agent_id);
+    expect(errorIds).toEqual([agentInHostedDoc._id]);
+
+    const actionResults = esClient.bulk.mock.calls[0][0];
+    const resultIds = (actionResults as estypes.BulkRequest)?.body
+      ?.filter((i: any) => i.agent_id)
+      .map((i: any) => i.agent_id);
+    expect(resultIds).toEqual(onlyRegular);
+
+    const action = esClient.create.mock.calls[0][0] as any;
+    expect(action.body.type).toEqual('FORCE_UNENROLL');
   });
 
   it('can unenroll from hosted agent policy with force=true', async () => {
@@ -226,7 +317,7 @@ describe('unenrollAgents (plural)', () => {
     ]);
 
     // calls ES update with correct values
-    const calledWith = esClient.bulk.mock.calls[0][0];
+    const calledWith = esClient.bulk.mock.calls[1][0];
     const ids = (calledWith as estypes.BulkRequest)?.body
       ?.filter((i: any) => i.update !== undefined)
       .map((i: any) => i.update._id);
@@ -237,6 +328,15 @@ describe('unenrollAgents (plural)', () => {
     for (const doc of docs!) {
       expect(doc).toHaveProperty('unenrolled_at');
     }
+
+    const actionResults = esClient.bulk.mock.calls[0][0];
+    const resultIds = (actionResults as estypes.BulkRequest)?.body
+      ?.filter((i: any) => i.agent_id)
+      .map((i: any) => i.agent_id);
+    expect(resultIds).toEqual(idsToUnenroll);
+
+    const action = esClient.create.mock.calls[0][0] as any;
+    expect(action.body.type).toEqual('FORCE_UNENROLL');
   });
 });
 
@@ -270,3 +370,31 @@ describe('invalidateAPIKeysForAgents', () => {
     ]);
   });
 });
+
+describe('isAgentUnenrolled', () => {
+  it('should return true if revoke and unenrolled_at set', () => {
+    expect(isAgentUnenrolled({ unenrolled_at: '2022-09-21' } as Agent, true)).toBe(true);
+  });
+
+  it('should return false if revoke and unenrolled_at null', () => {
+    expect(
+      isAgentUnenrolled({ unenrolled_at: null, unenrollment_started_at: '2022-09-21' } as any, true)
+    ).toBe(false);
+  });
+
+  it('should return true if unenrolled_at set', () => {
+    expect(isAgentUnenrolled({ unenrolled_at: '2022-09-21' } as Agent)).toBe(true);
+  });
+
+  it('should return true if unenrollment_started_at set and unenrolled_at null', () => {
+    expect(
+      isAgentUnenrolled({ unenrolled_at: null, unenrollment_started_at: '2022-09-21' } as any)
+    ).toBe(true);
+  });
+
+  it('should return false if unenrollment_started_at null and unenrolled_at null', () => {
+    expect(isAgentUnenrolled({ unenrolled_at: null, unenrollment_started_at: null } as any)).toBe(
+      false
+    );
+  });
+});
diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll.ts b/x-pack/plugins/fleet/server/services/agents/unenroll.ts
@@ -7,6 +7,8 @@
 
 import type { ElasticsearchClient, SavedObjectsClientContract } from '@kbn/core/server';
 
+import uuid from 'uuid';
+
 import type { Agent, BulkActionResult } from '../../types';
 import { HostedAgentPolicyRestrictionRelatedError } from '../../errors';
 import { SO_SEARCH_LIMIT } from '../../constants';
@@ -20,6 +22,7 @@ import {
   invalidateAPIKeysForAgents,
   UnenrollActionRunner,
   unenrollBatch,
+  updateActionsForForceUnenroll,
 } from './unenroll_action_runner';
 
 async function unenrollAgentIsAllowed(
@@ -50,7 +53,7 @@ export async function unenrollAgent(
     await unenrollAgentIsAllowed(soClient, esClient, agentId);
   }
   if (options?.revoke) {
-    return forceUnenrollAgent(soClient, esClient, agentId);
+    return forceUnenrollAgent(esClient, agentId);
   }
   const now = new Date().toISOString();
   await createAgentAction(esClient, {
@@ -102,7 +105,6 @@ export async function unenrollAgents(
 }
 
 export async function forceUnenrollAgent(
-  soClient: SavedObjectsClientContract,
   esClient: ElasticsearchClient,
   agentIdOrAgent: string | Agent
 ) {
@@ -116,4 +118,5 @@ export async function forceUnenrollAgent(
     active: false,
     unenrolled_at: new Date().toISOString(),
   });
+  await updateActionsForForceUnenroll(esClient, [agent.id], uuid(), 1);
 }