[Fleet] Bulk reassign response should include all given ids (#95024)

## Summary
`/agents/bulk_reassign` should return a response with a result for each agent given; including invalid or missing ids. It currently filters out missing or invalid before updating. This PR leaves them in and includes their error results in the response. 

[Added/updated tests](https://github.com/elastic/kibana/pull/95024/files#diff-7ec94bee3e2bae79e5d98b8c17c17b26fad14736143ffa144f3e035773d4cad1R113-R128) to confirm

### Checklist
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/fleet/common/types/rest_spec/agent.ts

@@ -164,12 +164,13 @@ export interface PostBulkAgentReassignRequest {
   };
 }
 
-export interface PostBulkAgentReassignResponse {
-  [key: string]: {
+export type PostBulkAgentReassignResponse = Record<
+  Agent['id'],
+  {
     success: boolean;
-    error?: Error;
-  };
-}
+    error?: string;
+  }
+>;
 
 export interface GetOneAgentEventsRequest {
   params: {

x-pack/plugins/fleet/server/routes/agent/handlers.ts

@@ -308,26 +308,26 @@ export const postBulkAgentsReassignHandler: RequestHandler<
 
   const soClient = context.core.savedObjects.client;
   const esClient = context.core.elasticsearch.client.asInternalUser;
+  const agentOptions = Array.isArray(request.body.agents)
+    ? { agentIds: request.body.agents }
+    : { kuery: request.body.agents };
 
   try {
     const results = await AgentService.reassignAgents(
       soClient,
       esClient,
-      Array.isArray(request.body.agents)
-        ? { agentIds: request.body.agents }
-        : { kuery: request.body.agents },
+      agentOptions,
       request.body.policy_id
     );
 
-    const body: PostBulkAgentReassignResponse = results.items.reduce((acc, so) => {
-      return {
-        ...acc,
-        [so.id]: {
-          success: !so.error,
-          error: so.error || undefined,
-        },
+    const body = results.items.reduce<PostBulkAgentReassignResponse>((acc, so) => {
+      acc[so.id] = {
+        success: !so.error,
+        error: so.error?.message,
       };
+      return acc;
     }, {});
+
     return response.ok({ body });
   } catch (error) {
     return defaultIngestErrorHandler({ error, response });

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

@@ -9,8 +9,8 @@ import Boom from '@hapi/boom';
 import type { SearchResponse, MGetResponse, GetResponse } from 'elasticsearch';
 import type { SavedObjectsClientContract, ElasticsearchClient } from 'src/core/server';
 
+import type { AgentSOAttributes, Agent, BulkActionResult, ListWithKuery } from '../../types';
 import type { ESSearchResponse } from '../../../../../../typings/elasticsearch';
-import type { AgentSOAttributes, Agent, ListWithKuery } from '../../types';
 import { appContextService, agentPolicyService } from '../../services';
 import type { FleetServerAgent } from '../../../common';
 import { isAgentUpgradeable, SO_SEARCH_LIMIT } from '../../../common';
@@ -69,22 +69,23 @@ export type GetAgentsOptions =
     };
 
 export async function getAgents(esClient: ElasticsearchClient, options: GetAgentsOptions) {
-  let initialResults = [];
-
+  let agents: Agent[] = [];
   if ('agentIds' in options) {
-    initialResults = await getAgentsById(esClient, options.agentIds);
+    agents = await getAgentsById(esClient, options.agentIds);
   } else if ('kuery' in options) {
-    initialResults = (
+    agents = (
       await getAllAgentsByKuery(esClient, {
         kuery: options.kuery,
         showInactive: options.showInactive ?? false,
       })
     ).agents;
   } else {
-    throw new IngestManagerError('Cannot get agents');
+    throw new IngestManagerError(
+      'Either options.agentIds or options.kuery are required to get agents'
+    );
   }
 
-  return initialResults;
+  return agents;
 }
 
 export async function getAgentsByKuery(
@@ -188,7 +189,7 @@ export async function countInactiveAgents(
 export async function getAgentById(esClient: ElasticsearchClient, agentId: string) {
   const agentNotFoundError = new AgentNotFoundError(`Agent ${agentId} not found`);
   try {
-    const agentHit = await esClient.get<GetResponse<FleetServerAgent>>({
+    const agentHit = await esClient.get<ESAgentDocumentResult>({
       index: AGENTS_INDEX,
       id: agentId,
     });
@@ -207,10 +208,17 @@ export async function getAgentById(esClient: ElasticsearchClient, agentId: strin
   }
 }
 
-async function getAgentDocuments(
+export function isAgentDocument(
+  maybeDocument: any
+): maybeDocument is GetResponse<FleetServerAgent> {
+  return '_id' in maybeDocument && '_source' in maybeDocument;
+}
+
+export type ESAgentDocumentResult = GetResponse<FleetServerAgent>;
+export async function getAgentDocuments(
   esClient: ElasticsearchClient,
   agentIds: string[]
-): Promise<Array<GetResponse<FleetServerAgent>>> {
+): Promise<ESAgentDocumentResult[]> {
   const res = await esClient.mget<MGetResponse<FleetServerAgent>>({
     index: AGENTS_INDEX,
     body: { docs: agentIds.map((_id) => ({ _id })) },
@@ -221,14 +229,16 @@ async function getAgentDocuments(
 
 export async function getAgentsById(
   esClient: ElasticsearchClient,
-  agentIds: string[],
-  options: { includeMissing?: boolean } = { includeMissing: false }
+  agentIds: string[]
 ): Promise<Agent[]> {
   const allDocs = await getAgentDocuments(esClient, agentIds);
-  const agentDocs = options.includeMissing
-    ? allDocs
-    : allDocs.filter((res) => res._id && res._source);
-  const agents = agentDocs.map((doc) => searchHitToAgent(doc));
+  const agents = allDocs.reduce<Agent[]>((results, doc) => {
+    if (isAgentDocument(doc)) {
+      results.push(searchHitToAgent(doc));
+    }
+
+    return results;
+  }, []);
 
   return agents;
 }
@@ -276,7 +286,7 @@ export async function bulkUpdateAgents(
     agentId: string;
     data: Partial<AgentSOAttributes>;
   }>
-) {
+): Promise<{ items: BulkActionResult[] }> {
   if (updateData.length === 0) {
     return { items: [] };
   }

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

@@ -8,13 +8,20 @@
 import type { SavedObjectsClientContract, ElasticsearchClient } from 'kibana/server';
 import Boom from '@hapi/boom';
 
-import type { Agent } from '../../types';
+import type { Agent, BulkActionResult } from '../../types';
 import { agentPolicyService } from '../agent_policy';
 import { AgentReassignmentError } from '../../errors';
 
-import { getAgents, getAgentPolicyForAgent, updateAgent, bulkUpdateAgents } from './crud';
+import {
+  getAgentDocuments,
+  getAgents,
+  getAgentPolicyForAgent,
+  updateAgent,
+  bulkUpdateAgents,
+} from './crud';
 import type { GetAgentsOptions } from './index';
 import { createAgentAction, bulkCreateAgentActions } from './actions';
+import { searchHitToAgent } from './helpers';
 
 export async function reassignAgent(
   soClient: SavedObjectsClientContract,
@@ -67,39 +74,67 @@ export async function reassignAgentIsAllowed(
 export async function reassignAgents(
   soClient: SavedObjectsClientContract,
   esClient: ElasticsearchClient,
-  options: { agents: Agent[] } | GetAgentsOptions,
+  options: ({ agents: Agent[] } | GetAgentsOptions) & { force?: boolean },
   newAgentPolicyId: string
-): Promise<{ items: Array<{ id: string; success: boolean; error?: Error }> }> {
+): Promise<{ items: BulkActionResult[] }> {
   const agentPolicy = await agentPolicyService.get(soClient, newAgentPolicyId);
   if (!agentPolicy) {
     throw Boom.notFound(`Agent policy not found: ${newAgentPolicyId}`);
   }
 
-  const allResults = 'agents' in options ? options.agents : await getAgents(esClient, options);
+  const outgoingErrors: Record<Agent['id'], Error> = {};
+  let givenAgents: Agent[] = [];
+  if ('agents' in options) {
+    givenAgents = options.agents;
+  } else if ('agentIds' in options) {
+    const givenAgentsResults = await getAgentDocuments(esClient, options.agentIds);
+    for (const agentResult of givenAgentsResults) {
+      if (agentResult.found === false) {
+        outgoingErrors[agentResult._id] = new AgentReassignmentError(
+          `Cannot find agent ${agentResult._id}`
+        );
+      } else {
+        givenAgents.push(searchHitToAgent(agentResult));
+      }
+    }
+  } else if ('kuery' in options) {
+    givenAgents = await getAgents(esClient, options);
+  }
+  const givenOrder =
+    'agentIds' in options ? options.agentIds : givenAgents.map((agent) => agent.id);
+
   // which are allowed to unenroll
-  const settled = await Promise.allSettled(
-    allResults.map((agent) =>
-      reassignAgentIsAllowed(soClient, esClient, agent.id, newAgentPolicyId).then((_) => agent)
-    )
+  const agentResults = await Promise.allSettled(
+    givenAgents.map(async (agent, index) => {
+      if (agent.policy_id === newAgentPolicyId) {
+        throw new AgentReassignmentError(`${agent.id} is already assigned to ${newAgentPolicyId}`);
+      }
+
+      const isAllowed = await reassignAgentIsAllowed(
+        soClient,
+        esClient,
+        agent.id,
+        newAgentPolicyId
+      );
+      if (isAllowed) {
+        return agent;
+      }
+      throw new AgentReassignmentError(`${agent.id} may not be reassigned to ${newAgentPolicyId}`);
+    })
   );
 
   // Filter to agents that do not already use the new agent policy ID
-  const agentsToUpdate = allResults.filter((agent, index) => {
-    if (settled[index].status === 'fulfilled') {
-      if (agent.policy_id === newAgentPolicyId) {
-        settled[index] = {
-          status: 'rejected',
-          reason: new AgentReassignmentError(
-            `${agent.id} is already assigned to ${newAgentPolicyId}`
-          ),
-        };
-      } else {
-        return true;
-      }
+  const agentsToUpdate = agentResults.reduce<Agent[]>((agents, result, index) => {
+    if (result.status === 'fulfilled') {
+      agents.push(result.value);
+    } else {
+      const id = givenAgents[index].id;
+      outgoingErrors[id] = result.reason;
     }
-  });
+    return agents;
+  }, []);
 
-  const res = await bulkUpdateAgents(
+  await bulkUpdateAgents(
     esClient,
     agentsToUpdate.map((agent) => ({
       agentId: agent.id,
@@ -110,6 +145,18 @@ export async function reassignAgents(
     }))
   );
 
+  const orderedOut = givenOrder.map((agentId) => {
+    const hasError = agentId in outgoingErrors;
+    const result: BulkActionResult = {
+      id: agentId,
+      success: !hasError,
+    };
+    if (hasError) {
+      result.error = outgoingErrors[agentId];
+    }
+    return result;
+  });
+
   const now = new Date().toISOString();
   await bulkCreateAgentActions(
     soClient,
@@ -121,5 +168,5 @@ export async function reassignAgents(
     }))
   );
 
-  return res;
+  return { items: orderedOut };
 }

x-pack/plugins/fleet/server/types/index.tsx

@@ -90,5 +90,11 @@ export type AgentPolicyUpdateHandler = (
   agentPolicyId: string
 ) => Promise<void>;
 
+export interface BulkActionResult {
+  id: string;
+  success: boolean;
+  error?: Error;
+}
+
 export * from './models';
 export * from './rest_spec';

x-pack/test/fleet_api_integration/apis/agents/reassign.ts

@@ -101,15 +101,32 @@ export default function (providerContext: FtrProviderContext) {
         expect(agent3data.body.item.policy_id).to.eql('policy2');
       });
 
-      it('should allow to reassign multiple agents by id -- some invalid', async () => {
-        await supertest
+      it('should allow to reassign multiple agents by id -- mix valid & invalid', async () => {
+        const { body } = await supertest
           .post(`/api/fleet/agents/bulk_reassign`)
           .set('kbn-xsrf', 'xxx')
           .send({
             agents: ['agent2', 'INVALID_ID', 'agent3', 'MISSING_ID', 'etc'],
             policy_id: 'policy2',
-          })
-          .expect(200);
+          });
+
+        expect(body).to.eql({
+          agent2: { success: true },
+          INVALID_ID: {
+            success: false,
+            error: 'Cannot find agent INVALID_ID',
+          },
+          agent3: { success: true },
+          MISSING_ID: {
+            success: false,
+            error: 'Cannot find agent MISSING_ID',
+          },
+          etc: {
+            success: false,
+            error: 'Cannot find agent etc',
+          },
+        });
+
         const [agent2data, agent3data] = await Promise.all([
           supertest.get(`/api/fleet/agents/agent2`),
           supertest.get(`/api/fleet/agents/agent3`),
@@ -118,6 +135,45 @@ export default function (providerContext: FtrProviderContext) {
         expect(agent3data.body.item.policy_id).to.eql('policy2');
       });
 
+      it('should allow to reassign multiple agents by id -- mixed invalid, managed, etc', async () => {
+        // agent1 is enrolled in policy1. set policy1 to managed
+        await supertest
+          .put(`/api/fleet/agent_policies/policy1`)
+          .set('kbn-xsrf', 'xxx')
+          .send({ name: 'Test policy', namespace: 'default', is_managed: true })
+          .expect(200);
+
+        const { body } = await supertest
+          .post(`/api/fleet/agents/bulk_reassign`)
+          .set('kbn-xsrf', 'xxx')
+          .send({
+            agents: ['agent2', 'INVALID_ID', 'agent3'],
+            policy_id: 'policy2',
+          });
+
+        expect(body).to.eql({
+          agent2: {
+            success: false,
+            error: 'Cannot reassign an agent from managed agent policy policy1',
+          },
+          INVALID_ID: {
+            success: false,
+            error: 'Cannot find agent INVALID_ID',
+          },
+          agent3: {
+            success: false,
+            error: 'Cannot reassign an agent from managed agent policy policy1',
+          },
+        });
+
+        const [agent2data, agent3data] = await Promise.all([
+          supertest.get(`/api/fleet/agents/agent2`),
+          supertest.get(`/api/fleet/agents/agent3`),
+        ]);
+        expect(agent2data.body.item.policy_id).to.eql('policy1');
+        expect(agent3data.body.item.policy_id).to.eql('policy1');
+      });
+
       it('should allow to reassign multiple agents by kuery', async () => {
         await supertest
           .post(`/api/fleet/agents/bulk_reassign`)