From ab7b02dbb9a6f3139297f8c4467b562f10aa1382 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 22 Sep 2022 01:57:56 -0600 Subject: [PATCH] [Fleet] Force unenroll in Agent activity (#141208) (#141346) * 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> (cherry picked from commit 3433f5d112d1e7737edd3dcef7ce57224c3ac535) Co-authored-by: Julia Bardi <90178898+juliaElastic@users.noreply.github.com> --- .../fleet/common/types/models/agent.ts | 8 +- .../components/agent_activity_flyout.tsx | 7 +- .../server/services/agents/action.mock.ts | 2 + .../server/services/agents/action_status.ts | 11 +- .../fleet/server/services/agents/actions.ts | 37 ++++- .../server/services/agents/unenroll.test.ts | 134 ++++++++++++++- .../fleet/server/services/agents/unenroll.ts | 7 +- .../services/agents/unenroll_action_runner.ts | 154 ++++++++++++++---- .../preconfiguration/reset_agent_policies.ts | 2 +- 9 files changed, 311 insertions(+), 51 deletions(-) diff --git a/x-pack/plugins/fleet/common/types/models/agent.ts b/x-pack/plugins/fleet/common/types/models/agent.ts index 7415ad01ee0c9..809e32c78c1b8 100644 --- 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/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx index e0cc1ae984671..a2fdf6d145d55 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_flyout.tsx +++ b/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={ } /> @@ -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 index 015914a1016cc..8d0bcb8b4ee4e 100644 --- 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 index 2f5e6ff80cd9e..0db6d6db9d86c 100644 --- 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 index f215e6dffee7e..20cb2fb94e51d 100644 --- 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 { if (results.length === 0) { @@ -164,6 +164,41 @@ export async function getAgentActions(esClient: ElasticsearchClient, actionId: s })); } +export async function getUnenrollAgentActions( + esClient: ElasticsearchClient +): Promise { + const res = await esClient.search({ + 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({ 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 index f3bbb2036954f..d96e8b89d2fc1 100644 --- a/x-pack/plugins/fleet/server/services/agents/unenroll.test.ts +++ b/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 + ); + }); +}); diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll.ts b/x-pack/plugins/fleet/server/services/agents/unenroll.ts index 941e1260894b4..f7c49f3efcf1c 100644 --- 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); } diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll_action_runner.ts b/x-pack/plugins/fleet/server/services/agents/unenroll_action_runner.ts index 70cfcece227ba..dfa7fac5e0e4c 100644 --- a/x-pack/plugins/fleet/server/services/agents/unenroll_action_runner.ts +++ b/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 = {}; // And which are allowed to unenroll const agentsToUpdate = options.force - ? agentsEnrolled - : agentsEnrolled.reduce((agents, agent) => { - if (isHostedAgent(hostedPolicies, agent)) { + ? givenAgents + : givenAgents.reduce((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 { + 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((keys, agent) => { if (agent.access_api_key_id) { diff --git a/x-pack/plugins/fleet/server/services/preconfiguration/reset_agent_policies.ts b/x-pack/plugins/fleet/server/services/preconfiguration/reset_agent_policies.ts index 6d456a01d551f..9375c3d82b7cb 100644 --- a/x-pack/plugins/fleet/server/services/preconfiguration/reset_agent_policies.ts +++ b/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, }); }