[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 <42973632+kibanamachine@users.noreply.github.com>

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];

x-pack/plugins/fleet/public/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' },
 };

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,

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: {

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,

x-pack/plugins/fleet/server/services/agents/unenroll.test.ts

@@ -6,6 +6,8 @@
  */
 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';
 
@@ -13,8 +15,10 @@ 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
+    );
+  });
+});

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);
 }

x-pack/plugins/fleet/server/services/agents/unenroll_action_runner.ts

@@ -7,9 +7,13 @@
 import uuid from 'uuid';
 import type { SavedObjectsClientContract, ElasticsearchClient } from '@kbn/core/server';
 
+import { intersection } from 'lodash';
+
+import { AGENT_ACTIONS_RESULTS_INDEX } from '../../../common';
+
 import type { Agent, BulkActionResult } from '../../types';
 
-import { HostedAgentPolicyRestrictionRelatedError } from '../../errors';
+import { FleetError, HostedAgentPolicyRestrictionRelatedError } from '../../errors';
 
 import { invalidateAPIKeys } from '../api_keys';
 
@@ -18,7 +22,11 @@ import { appContextService } from '../app_context';
 import { ActionRunner } from './action_runner';
 
 import { errorsToResults, bulkUpdateAgents } from './crud';
-import { bulkCreateAgentActionResults, createAgentAction } from './actions';
+import {
+  bulkCreateAgentActionResults,
+  createAgentAction,
+  getUnenrollAgentActions,
+} from './actions';
 import { getHostedPolicies, isHostedAgent } from './hosted_agent';
 import { BulkActionTaskType } from './bulk_actions_resolver';
 
@@ -36,6 +44,13 @@ export class UnenrollActionRunner extends ActionRunner {
   }
 }
 
+export function isAgentUnenrolled(agent: Agent, revoke?: boolean): boolean {
+  return Boolean(
+    (revoke && agent.unenrolled_at) ||
+      (!revoke && (agent.unenrollment_started_at || agent.unenrolled_at))
+  );
+}
+
 export async function unenrollBatch(
   soClient: SavedObjectsClientContract,
   esClient: ElasticsearchClient,
@@ -48,23 +63,16 @@ export async function unenrollBatch(
   },
   skipSuccess?: boolean
 ): Promise<{ items: BulkActionResult[] }> {
-  // Filter to those not already unenrolled, or unenrolling
-  const agentsEnrolled = givenAgents.filter((agent) => {
-    if (options.revoke) {
-      return !agent.unenrolled_at;
-    }
-    return !agent.unenrollment_started_at && !agent.unenrolled_at;
-  });
-
-  const hostedPolicies = await getHostedPolicies(soClient, agentsEnrolled);
-
+  const hostedPolicies = await getHostedPolicies(soClient, givenAgents);
   const outgoingErrors: Record<Agent['id'], Error> = {};
 
   // And which are allowed to unenroll
   const agentsToUpdate = options.force
-    ? agentsEnrolled
-    : agentsEnrolled.reduce<Agent[]>((agents, agent) => {
-        if (isHostedAgent(hostedPolicies, agent)) {
+    ? givenAgents
+    : givenAgents.reduce<Agent[]>((agents, agent) => {
+        if (isAgentUnenrolled(agent, options.revoke)) {
+          outgoingErrors[agent.id] = new FleetError(`Agent ${agent.id} already unenrolled`);
+        } else if (isHostedAgent(hostedPolicies, agent)) {
           outgoingErrors[agent.id] = new HostedAgentPolicyRestrictionRelatedError(
             `Cannot unenroll ${agent.id} from a hosted agent policy ${agent.policy_id}`
           );
@@ -76,39 +84,43 @@ export async function unenrollBatch(
 
   const actionId = options.actionId ?? uuid();
   const errorCount = Object.keys(outgoingErrors).length;
-  const total = options.total ?? agentsToUpdate.length + errorCount;
+  const total = options.total ?? givenAgents.length;
+
+  const agentIds = agentsToUpdate.map((agent) => agent.id);
 
   const now = new Date().toISOString();
   if (options.revoke) {
     // Get all API keys that need to be invalidated
     await invalidateAPIKeysForAgents(agentsToUpdate);
+
+    await updateActionsForForceUnenroll(esClient, agentIds, actionId, total);
   } else {
     // Create unenroll action for each agent
     await createAgentAction(esClient, {
       id: actionId,
-      agents: agentsToUpdate.map((agent) => agent.id),
+      agents: agentIds,
       created_at: now,
       type: 'UNENROLL',
       total,
     });
+  }
 
-    if (errorCount > 0) {
-      appContextService
-        .getLogger()
-        .info(
-          `Skipping ${errorCount} agents, as failed validation (cannot unenroll from a hosted policy)`
-        );
-
-      // writing out error result for those agents that failed validation, so the action is not going to stay in progress forever
-      await bulkCreateAgentActionResults(
-        esClient,
-        Object.keys(outgoingErrors).map((agentId) => ({
-          agentId,
-          actionId,
-          error: outgoingErrors[agentId].message,
-        }))
+  if (errorCount > 0) {
+    appContextService
+      .getLogger()
+      .info(
+        `Skipping ${errorCount} agents, as failed validation (cannot unenroll from a hosted policy or already unenrolled)`
       );
-    }
+
+    // writing out error result for those agents that failed validation, so the action is not going to stay in progress forever
+    await bulkCreateAgentActionResults(
+      esClient,
+      Object.keys(outgoingErrors).map((agentId) => ({
+        agentId,
+        actionId,
+        error: outgoingErrors[agentId].message,
+      }))
+    );
   }
 
   // Update the necessary agents
@@ -126,6 +138,82 @@ export async function unenrollBatch(
   };
 }
 
+export async function updateActionsForForceUnenroll(
+  esClient: ElasticsearchClient,
+  agentIds: string[],
+  actionId: string,
+  total: number
+) {
+  // creating an unenroll so that force unenroll shows up in activity
+  await createAgentAction(esClient, {
+    id: actionId,
+    agents: [],
+    created_at: new Date().toISOString(),
+    type: 'FORCE_UNENROLL',
+    total,
+  });
+  await bulkCreateAgentActionResults(
+    esClient,
+    agentIds.map((agentId) => ({
+      agentId,
+      actionId,
+    }))
+  );
+
+  // updating action results for those agents that are there in a pending unenroll action
+  const unenrollActions = await getUnenrollAgentActions(esClient);
+  for (const action of unenrollActions) {
+    const commonAgents = intersection(action.agents, agentIds);
+    if (commonAgents.length > 0) {
+      // filtering out agents with action results
+      const agentsToUpdate = await getAgentsWithoutActionResults(
+        esClient,
+        action.action_id!,
+        commonAgents
+      );
+      if (agentsToUpdate.length > 0) {
+        await bulkCreateAgentActionResults(
+          esClient,
+          agentsToUpdate.map((agentId) => ({
+            agentId,
+            actionId: action.action_id!,
+          }))
+        );
+      }
+    }
+  }
+}
+
+async function getAgentsWithoutActionResults(
+  esClient: ElasticsearchClient,
+  actionId: string,
+  commonAgents: string[]
+): Promise<string[]> {
+  try {
+    const res = await esClient.search({
+      index: AGENT_ACTIONS_RESULTS_INDEX,
+      query: {
+        bool: {
+          must: [{ term: { action_id: actionId } }, { terms: { agent_id: commonAgents } }],
+        },
+      },
+      size: commonAgents.length,
+    });
+    const agentsToUpdate = commonAgents.filter(
+      (agentId) => !res.hits.hits.find((hit) => (hit._source as any)?.agent_id === agentId)
+    );
+    return agentsToUpdate;
+  } catch (err) {
+    if (err.statusCode === 404) {
+      // .fleet-actions-results does not yet exist
+      appContextService.getLogger().debug(err);
+    } else {
+      throw err;
+    }
+  }
+  return commonAgents;
+}
+
 export async function invalidateAPIKeysForAgents(agents: Agent[]) {
   const apiKeys = agents.reduce<string[]>((keys, agent) => {
     if (agent.access_api_key_id) {

x-pack/plugins/fleet/server/services/preconfiguration/reset_agent_policies.ts

@@ -163,7 +163,7 @@ async function _deleteExistingData(
   // Delete
   if (agents.length > 0) {
     logger.info(`Force unenrolling ${agents.length} agents`);
-    await pMap(agents, (agent) => forceUnenrollAgent(soClient, esClient, agent.id), {
+    await pMap(agents, (agent) => forceUnenrollAgent(esClient, agent.id), {
       concurrency: 20,
     });
   }