From 1cc4725901ed299dd3003aff680121a2efa99a09 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Tue, 16 Mar 2021 23:50:21 -0400 Subject: [PATCH] [Fleet] Add test/fix for invalid/missing ids in bulk agent reassign (#94632) (#94774) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem While working on changes for bulk reassign https://github.com/elastic/kibana/issues/90437, I found that the server has a runtime error and returns a 500 if given an invalid or missing id.
server error stack trace ``` │ proc [kibana] server log [12:21:48.953] [error][fleet][plugins] TypeError: Cannot read property 'policy_revision_idx' of undefined │ proc [kibana] at map (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/agents/helpers.ts:15:34) │ proc [kibana] at Array.map () │ proc [kibana] at getAgents (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/agents/crud.ts:191:32) │ proc [kibana] at runMicrotasks () │ proc [kibana] at processTicksAndRejections (internal/process/task_queues.js:93:5) │ proc [kibana] at Object.reassignAgents (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/services/agents/reassign.ts:91:9) │ proc [kibana] at postBulkAgentsReassignHandler (/Users/jfsiii/work/kibana/x-pack/plugins/fleet/server/routes/agent/handlers.ts:314:21) │ proc [kibana] at Router.handle (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:272:30) │ proc [kibana] at handler (/Users/jfsiii/work/kibana/src/core/server/http/router/router.ts:227:11) │ proc [kibana] at exports.Manager.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/toolkit.js:60:28) │ proc [kibana] at Object.internals.handler (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:46:20) │ proc [kibana] at exports.execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/handler.js:31:20) │ proc [kibana] at Request._lifecycle (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:370:32) │ proc [kibana] at Request._execute (/Users/jfsiii/work/kibana/node_modules/@hapi/hapi/lib/request.js:279:9) ```
see test added in this PR fail on master ``` 1) Fleet Endpoints reassign agent(s) bulk reassign agents should allow to reassign multiple agents by id -- some invalid: Error: expected 200 "OK", got 500 "Internal Server Error" ```
## Root cause Debugging runtime error in `searchHitToAgent` found some TS type mismatches for the ES values being returned. Perhaps from one or more of the recent changes to ES client & Fleet Server. Based on `test:jest` and `test:ftr`, it appears the possible types are `GetResponse` or `SearchResponse`, instead of only an `ESSearchHit`. https://github.com/elastic/kibana/pull/94632/files#diff-254d0f427979efc3b442f78762302eb28fb9c8857df68ea04f8d411e052f939cL11 While a `.search` result will include return matched values, a `.get` or `.mget` will return a row for each input and a `found: boolean`. e.g. `{ _id: "does-not-exist", found: false }`. The error occurs when [`searchHitToAgent`](https://github.com/jfsiii/kibana/blob/1702cf98f018c41ec0a080d829a12403168ac242/x-pack/plugins/fleet/server/services/agents/helpers.ts#L11) is run on a get miss instead of a search hit. ## PR Changes * Added a test to ensure it doesn't fail if invalid or missing IDs are given * Moved the `bulk_reassign` tests to their own test section * Filter out any missing results before calling `searchHitToAgent`, to match current behavior * Consolidate repeated arguments into and code for getting agents into single [function](https://github.com/elastic/kibana/pull/94632/files#diff-f7377ed9ad56eaa8ea188b64e957e771ccc7a7652fd1eaf44251c25b930f8448R70-R87): and [TS type](https://github.com/elastic/kibana/pull/94632/files#diff-f7377ed9ad56eaa8ea188b64e957e771ccc7a7652fd1eaf44251c25b930f8448R61-R68) * Rename some agent service functions to be more explicit (IMO) but behavior maintained. Same API names exported. This moves toward the "one result (success or error) per given id" approach for https://github.com/elastic/kibana/issues/90437 ### 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 # Conflicts: # x-pack/plugins/fleet/server/services/agents/crud.ts # x-pack/plugins/fleet/server/services/index.ts --- x-pack/plugins/fleet/server/plugin.ts | 8 +- .../fleet/server/routes/agent/handlers.ts | 9 +- .../fleet/server/routes/agent/index.ts | 2 +- .../server/routes/agent/upgrade_handler.ts | 4 +- .../server/routes/agent_policy/handlers.ts | 4 +- .../fleet/server/services/agent_policy.ts | 4 +- .../agents/checkin/state_new_actions.ts | 4 +- .../fleet/server/services/agents/crud.ts | 99 ++++++--- .../fleet/server/services/agents/helpers.ts | 9 +- .../server/services/agents/reassign.test.ts | 4 +- .../fleet/server/services/agents/reassign.ts | 52 +++-- .../fleet/server/services/agents/status.ts | 6 +- .../fleet/server/services/agents/unenroll.ts | 44 +--- .../fleet/server/services/agents/update.ts | 4 +- .../fleet/server/services/agents/upgrade.ts | 37 +--- x-pack/plugins/fleet/server/services/index.ts | 8 +- .../apis/agents/reassign.ts | 194 ++++++++++-------- 17 files changed, 252 insertions(+), 240 deletions(-) diff --git a/x-pack/plugins/fleet/server/plugin.ts b/x-pack/plugins/fleet/server/plugin.ts index cd7afc63ab2e..a14f71a813ff 100644 --- a/x-pack/plugins/fleet/server/plugin.ts +++ b/x-pack/plugins/fleet/server/plugin.ts @@ -78,8 +78,8 @@ import { import { getAgentStatusById, authenticateAgentWithAccessToken, - listAgents, - getAgent, + getAgentsByKuery, + getAgentById, } from './services/agents'; import { agentCheckinState } from './services/agents/checkin/state'; import { registerFleetUsageCollector } from './collectors/register'; @@ -320,8 +320,8 @@ export class FleetPlugin }, }, agentService: { - getAgent, - listAgents, + getAgent: getAgentById, + listAgents: getAgentsByKuery, getAgentStatusById, authenticateAgentWithAccessToken, }, diff --git a/x-pack/plugins/fleet/server/routes/agent/handlers.ts b/x-pack/plugins/fleet/server/routes/agent/handlers.ts index a949ea5c681b..424aaab94f2b 100644 --- a/x-pack/plugins/fleet/server/routes/agent/handlers.ts +++ b/x-pack/plugins/fleet/server/routes/agent/handlers.ts @@ -44,8 +44,7 @@ export const getAgentHandler: RequestHandler< const esClient = context.core.elasticsearch.client.asCurrentUser; try { - const agent = await AgentService.getAgent(esClient, request.params.agentId); - + const agent = await AgentService.getAgentById(esClient, request.params.agentId); const body: GetOneAgentResponse = { item: { ...agent, @@ -134,8 +133,7 @@ export const updateAgentHandler: RequestHandler< await AgentService.updateAgent(esClient, request.params.agentId, { user_provided_metadata: request.body.user_provided_metadata, }); - const agent = await AgentService.getAgent(esClient, request.params.agentId); - + const agent = await AgentService.getAgentById(esClient, request.params.agentId); const body = { item: { ...agent, @@ -245,7 +243,7 @@ export const getAgentsHandler: RequestHandler< const esClient = context.core.elasticsearch.client.asCurrentUser; try { - const { agents, total, page, perPage } = await AgentService.listAgents(esClient, { + const { agents, total, page, perPage } = await AgentService.getAgentsByKuery(esClient, { page: request.query.page, perPage: request.query.perPage, showInactive: request.query.showInactive, @@ -310,6 +308,7 @@ export const postBulkAgentsReassignHandler: RequestHandler< const soClient = context.core.savedObjects.client; const esClient = context.core.elasticsearch.client.asInternalUser; + try { const results = await AgentService.reassignAgents( soClient, diff --git a/x-pack/plugins/fleet/server/routes/agent/index.ts b/x-pack/plugins/fleet/server/routes/agent/index.ts index 58bbde95b918..e68bc3a88cc1 100644 --- a/x-pack/plugins/fleet/server/routes/agent/index.ts +++ b/x-pack/plugins/fleet/server/routes/agent/index.ts @@ -125,7 +125,7 @@ export const registerAPIRoutes = (router: IRouter, config: FleetConfigType) => { options: { tags: [`access:${PLUGIN_ID}-all`] }, }, postNewAgentActionHandlerBuilder({ - getAgent: AgentService.getAgent, + getAgent: AgentService.getAgentById, createAgentAction: AgentService.createAgentAction, }) ); diff --git a/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts b/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts index 5459ec32b94f..58149922f8e4 100644 --- a/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts +++ b/x-pack/plugins/fleet/server/routes/agent/upgrade_handler.ts @@ -15,7 +15,7 @@ import * as AgentService from '../../services/agents'; import { appContextService } from '../../services'; import { defaultIngestErrorHandler } from '../../errors'; import { isAgentUpgradeable } from '../../../common/services'; -import { getAgent } from '../../services/agents'; +import { getAgentById } from '../../services/agents'; export const postAgentUpgradeHandler: RequestHandler< TypeOf, @@ -36,7 +36,7 @@ export const postAgentUpgradeHandler: RequestHandler< }, }); } - const agent = await getAgent(esClient, request.params.agentId); + const agent = await getAgentById(esClient, request.params.agentId); if (agent.unenrollment_started_at || agent.unenrolled_at) { return response.customError({ statusCode: 400, diff --git a/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts index 0496c8c5b0b8..2a47d5121670 100644 --- a/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts +++ b/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts @@ -11,7 +11,7 @@ import bluebird from 'bluebird'; import { fullAgentPolicyToYaml } from '../../../common/services'; import { appContextService, agentPolicyService, packagePolicyService } from '../../services'; -import { listAgents } from '../../services/agents'; +import { getAgentsByKuery } from '../../services/agents'; import { AGENT_SAVED_OBJECT_TYPE } from '../../constants'; import { GetAgentPoliciesRequestSchema, @@ -58,7 +58,7 @@ export const getAgentPoliciesHandler: RequestHandler< await bluebird.map( items, (agentPolicy: GetAgentPoliciesResponseItem) => - listAgents(esClient, { + getAgentsByKuery(esClient, { showInactive: false, perPage: 0, page: 1, diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts index 31c184c598b1..2cafe2fe57c0 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.ts @@ -43,7 +43,7 @@ import { } from '../errors'; import { getFullAgentPolicyKibanaConfig } from '../../common/services/full_agent_policy_kibana_config'; -import { createAgentPolicyAction, listAgents } from './agents'; +import { createAgentPolicyAction, getAgentsByKuery } from './agents'; import { packagePolicyService } from './package_policy'; import { outputService } from './output'; import { agentPolicyUpdateEventHandler } from './agent_policy_update'; @@ -520,7 +520,7 @@ class AgentPolicyService { throw new Error('The default agent policy cannot be deleted'); } - const { total } = await listAgents(esClient, { + const { total } = await getAgentsByKuery(esClient, { showInactive: false, perPage: 0, page: 1, diff --git a/x-pack/plugins/fleet/server/services/agents/checkin/state_new_actions.ts b/x-pack/plugins/fleet/server/services/agents/checkin/state_new_actions.ts index 0fc83914bcdc..15d48b40f77c 100644 --- a/x-pack/plugins/fleet/server/services/agents/checkin/state_new_actions.ts +++ b/x-pack/plugins/fleet/server/services/agents/checkin/state_new_actions.ts @@ -38,7 +38,7 @@ import { getAgentPolicyActionByIds, } from '../actions'; import { appContextService } from '../../app_context'; -import { getAgent, updateAgent } from '../crud'; +import { getAgentById, updateAgent } from '../crud'; import { toPromiseAbortable, AbortError, createRateLimiter } from './rxjs_utils'; @@ -265,7 +265,7 @@ export function agentCheckinStateNewActionsFactory() { (action) => action.type === 'INTERNAL_POLICY_REASSIGN' ); if (hasConfigReassign) { - return from(getAgent(esClient, agent.id)).pipe( + return from(getAgentById(esClient, agent.id)).pipe( concatMap((refreshedAgent) => { if (!refreshedAgent.policy_id) { throw new Error('Agent does not have a policy assigned'); diff --git a/x-pack/plugins/fleet/server/services/agents/crud.ts b/x-pack/plugins/fleet/server/services/agents/crud.ts index 41cd11e73cd7..52a6b98bd0c4 100644 --- a/x-pack/plugins/fleet/server/services/agents/crud.ts +++ b/x-pack/plugins/fleet/server/services/agents/crud.ts @@ -6,17 +6,17 @@ */ import Boom from '@hapi/boom'; -import { SearchResponse } from 'elasticsearch'; +import type { SearchResponse, MGetResponse, GetResponse } from 'elasticsearch'; import type { SavedObjectsClientContract, ElasticsearchClient } from 'src/core/server'; import type { AgentSOAttributes, Agent, ListWithKuery } from '../../types'; import { appContextService, agentPolicyService } from '../../services'; -import { FleetServerAgent, isAgentUpgradeable, SO_SEARCH_LIMIT } from '../../../common'; +import type { FleetServerAgent } from '../../../common'; +import { isAgentUpgradeable, SO_SEARCH_LIMIT } from '../../../common'; import { AGENT_SAVED_OBJECT_TYPE, AGENTS_INDEX } from '../../constants'; - -import type { ESSearchHit } from '../../../../../../typings/elasticsearch'; import { escapeSearchQueryPhrase, normalizeKuery } from '../saved_object'; -import { esKuery, KueryNode } from '../../../../../../src/plugins/data/server'; +import type { KueryNode } from '../../../../../../src/plugins/data/server'; +import { esKuery } from '../../../../../../src/plugins/data/server'; import { IngestManagerError, isESClientError, AgentNotFoundError } from '../../errors'; import { searchHitToAgent, agentSOAttributesToFleetServerAgentDoc } from './helpers'; @@ -58,7 +58,35 @@ export function removeSOAttributes(kuery: string) { return kuery.replace(/attributes\./g, '').replace(/fleet-agents\./g, ''); } -export async function listAgents( +export type GetAgentsOptions = + | { + agentIds: string[]; + } + | { + kuery: string; + showInactive?: boolean; + }; + +export async function getAgents(esClient: ElasticsearchClient, options: GetAgentsOptions) { + let initialResults = []; + + if ('agentIds' in options) { + initialResults = await getAgentsById(esClient, options.agentIds); + } else if ('kuery' in options) { + initialResults = ( + await getAllAgentsByKuery(esClient, { + kuery: options.kuery, + showInactive: options.showInactive ?? false, + }) + ).agents; + } else { + throw new IngestManagerError('Cannot get agents'); + } + + return initialResults; +} + +export async function getAgentsByKuery( esClient: ElasticsearchClient, options: ListWithKuery & { showInactive: boolean; @@ -90,8 +118,7 @@ export async function listAgents( const kueryNode = _joinFilters(filters); const body = kueryNode ? { query: esKuery.toElasticsearchQuery(kueryNode) } : {}; - - const res = await esClient.search({ + const res = await esClient.search>({ index: AGENTS_INDEX, from: (page - 1) * perPage, size: perPage, @@ -100,27 +127,24 @@ export async function listAgents( body, }); - let agentResults: Agent[] = res.body.hits.hits.map(searchHitToAgent); - let total = res.body.hits.total.value; - + let agents = res.body.hits.hits.map(searchHitToAgent); // filtering for a range on the version string will not work, // nor does filtering on a flattened field (local_metadata), so filter here if (showUpgradeable) { - agentResults = agentResults.filter((agent) => + agents = agents.filter((agent) => isAgentUpgradeable(agent, appContextService.getKibanaVersion()) ); - total = agentResults.length; } return { - agents: res.body.hits.hits.map(searchHitToAgent), - total, + agents, + total: agents.length, page, perPage, }; } -export async function listAllAgents( +export async function getAllAgentsByKuery( esClient: ElasticsearchClient, options: Omit & { showInactive: boolean; @@ -129,7 +153,7 @@ export async function listAllAgents( agents: Agent[]; total: number; }> { - const res = await listAgents(esClient, { ...options, page: 1, perPage: SO_SEARCH_LIMIT }); + const res = await getAgentsByKuery(esClient, { ...options, page: 1, perPage: SO_SEARCH_LIMIT }); return { agents: res.agents, @@ -160,34 +184,51 @@ export async function countInactiveAgents( return res.body.hits.total.value; } -export async function getAgent(esClient: ElasticsearchClient, agentId: string) { +export async function getAgentById(esClient: ElasticsearchClient, agentId: string) { + const agentNotFoundError = new AgentNotFoundError(`Agent ${agentId} not found`); try { - const agentHit = await esClient.get>({ + const agentHit = await esClient.get>({ index: AGENTS_INDEX, id: agentId, }); + + if (agentHit.body.found === false) { + throw agentNotFoundError; + } const agent = searchHitToAgent(agentHit.body); return agent; } catch (err) { if (isESClientError(err) && err.meta.statusCode === 404) { - throw new AgentNotFoundError(`Agent ${agentId} not found`); + throw agentNotFoundError; } throw err; } } -export async function getAgents( +async function getAgentDocuments( esClient: ElasticsearchClient, agentIds: string[] -): Promise { - const body = { docs: agentIds.map((_id) => ({ _id })) }; - - const res = await esClient.mget({ - body, +): Promise>> { + const res = await esClient.mget>({ index: AGENTS_INDEX, + body: { docs: agentIds.map((_id) => ({ _id })) }, }); - const agents = res.body.docs.map(searchHitToAgent); + + return res.body.docs || []; +} + +export async function getAgentsById( + esClient: ElasticsearchClient, + agentIds: string[], + options: { includeMissing?: boolean } = { includeMissing: false } +): Promise { + 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)); + return agents; } @@ -200,7 +241,7 @@ export async function getAgentByAccessAPIKeyId( q: `access_api_key_id:${escapeSearchQueryPhrase(accessAPIKeyId)}`, }); - const [agent] = res.body.hits.hits.map(searchHitToAgent); + const agent = searchHitToAgent(res.body.hits.hits[0]); if (!agent) { throw new AgentNotFoundError('Agent not found'); @@ -287,7 +328,7 @@ export async function getAgentPolicyForAgent( esClient: ElasticsearchClient, agentId: string ) { - const agent = await getAgent(esClient, agentId); + const agent = await getAgentById(esClient, agentId); if (!agent.policy_id) { return; } diff --git a/x-pack/plugins/fleet/server/services/agents/helpers.ts b/x-pack/plugins/fleet/server/services/agents/helpers.ts index 3fdb347ed246..bcc065badcd5 100644 --- a/x-pack/plugins/fleet/server/services/agents/helpers.ts +++ b/x-pack/plugins/fleet/server/services/agents/helpers.ts @@ -5,10 +5,15 @@ * 2.0. */ -import type { ESSearchHit } from '../../../../../../typings/elasticsearch'; +import type { GetResponse, SearchResponse } from 'elasticsearch'; + import type { Agent, AgentSOAttributes, FleetServerAgent } from '../../types'; -export function searchHitToAgent(hit: ESSearchHit): Agent { +type FleetServerAgentESResponse = + | GetResponse + | SearchResponse['hits']['hits'][0]; + +export function searchHitToAgent(hit: FleetServerAgentESResponse): Agent { return { id: hit._id, ...hit._source, diff --git a/x-pack/plugins/fleet/server/services/agents/reassign.test.ts b/x-pack/plugins/fleet/server/services/agents/reassign.test.ts index 29e09312dcd1..987f46158723 100644 --- a/x-pack/plugins/fleet/server/services/agents/reassign.test.ts +++ b/x-pack/plugins/fleet/server/services/agents/reassign.test.ts @@ -121,7 +121,7 @@ function createClientsMock() { case unmanagedAgentPolicySO2.id: return unmanagedAgentPolicySO2; default: - throw new Error('Not found'); + throw new Error(`${id} not found`); } }); soClientMock.bulkGet.mockImplementation(async (options) => { @@ -147,7 +147,7 @@ function createClientsMock() { case agentInUnmanagedDoc._id: return { body: agentInUnmanagedDoc }; default: - throw new Error('Not found'); + throw new Error(`${id} not found`); } }); // @ts-expect-error diff --git a/x-pack/plugins/fleet/server/services/agents/reassign.ts b/x-pack/plugins/fleet/server/services/agents/reassign.ts index b221be55cd46..74e60c42b997 100644 --- a/x-pack/plugins/fleet/server/services/agents/reassign.ts +++ b/x-pack/plugins/fleet/server/services/agents/reassign.ts @@ -8,16 +8,12 @@ import type { SavedObjectsClientContract, ElasticsearchClient } from 'kibana/server'; import Boom from '@hapi/boom'; +import type { Agent } from '../../types'; import { agentPolicyService } from '../agent_policy'; import { AgentReassignmentError } from '../../errors'; -import { - getAgents, - getAgentPolicyForAgent, - listAllAgents, - updateAgent, - bulkUpdateAgents, -} from './crud'; +import { getAgents, getAgentPolicyForAgent, updateAgent, bulkUpdateAgents } from './crud'; +import type { GetAgentsOptions } from './index'; import { createAgentAction, bulkCreateAgentActions } from './actions'; export async function reassignAgent( @@ -71,13 +67,7 @@ export async function reassignAgentIsAllowed( export async function reassignAgents( soClient: SavedObjectsClientContract, esClient: ElasticsearchClient, - options: - | { - agentIds: string[]; - } - | { - kuery: string; - }, + options: { agents: Agent[] } | GetAgentsOptions, newAgentPolicyId: string ): Promise<{ items: Array<{ id: string; success: boolean; error?: Error }> }> { const agentPolicy = await agentPolicyService.get(soClient, newAgentPolicyId); @@ -85,25 +75,29 @@ export async function reassignAgents( throw Boom.notFound(`Agent policy not found: ${newAgentPolicyId}`); } - // Filter to agents that do not already use the new agent policy ID - const agents = - 'agentIds' in options - ? await getAgents(esClient, options.agentIds) - : ( - await listAllAgents(esClient, { - kuery: options.kuery, - showInactive: false, - }) - ).agents; - // And which are allowed to unenroll + const allResults = 'agents' in options ? options.agents : await getAgents(esClient, options); + // which are allowed to unenroll const settled = await Promise.allSettled( - agents.map((agent) => + allResults.map((agent) => reassignAgentIsAllowed(soClient, esClient, agent.id, newAgentPolicyId).then((_) => agent) ) ); - const agentsToUpdate = agents.filter( - (agent, index) => settled[index].status === 'fulfilled' && agent.policy_id !== 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 res = await bulkUpdateAgents( esClient, diff --git a/x-pack/plugins/fleet/server/services/agents/status.ts b/x-pack/plugins/fleet/server/services/agents/status.ts index 930f3ca22ccb..f3fb01655974 100644 --- a/x-pack/plugins/fleet/server/services/agents/status.ts +++ b/x-pack/plugins/fleet/server/services/agents/status.ts @@ -14,13 +14,13 @@ import { AgentStatusKueryHelper } from '../../../common/services'; import { esKuery } from '../../../../../../src/plugins/data/server'; import type { KueryNode } from '../../../../../../src/plugins/data/server'; -import { getAgent, listAgents, removeSOAttributes } from './crud'; +import { getAgentById, getAgentsByKuery, removeSOAttributes } from './crud'; export async function getAgentStatusById( esClient: ElasticsearchClient, agentId: string ): Promise { - const agent = await getAgent(esClient, agentId); + const agent = await getAgentById(esClient, agentId); return AgentStatusKueryHelper.getAgentStatus(agent); } @@ -64,7 +64,7 @@ export async function getAgentStatusForAgentPolicy( AgentStatusKueryHelper.buildKueryForUpdatingAgents(), ], (kuery) => - listAgents(esClient, { + getAgentsByKuery(esClient, { showInactive: false, perPage: 0, page: 1, diff --git a/x-pack/plugins/fleet/server/services/agents/unenroll.ts b/x-pack/plugins/fleet/server/services/agents/unenroll.ts index 96ac11c89f68..14f9aa46e9fa 100644 --- a/x-pack/plugins/fleet/server/services/agents/unenroll.ts +++ b/x-pack/plugins/fleet/server/services/agents/unenroll.ts @@ -11,12 +11,12 @@ import * as APIKeyService from '../api_keys'; import { AgentUnenrollmentError } from '../../errors'; import { createAgentAction, bulkCreateAgentActions } from './actions'; +import type { GetAgentsOptions } from './crud'; import { - getAgent, + getAgentById, + getAgents, updateAgent, getAgentPolicyForAgent, - getAgents, - listAllAgents, bulkUpdateAgents, } from './crud'; @@ -56,23 +56,9 @@ export async function unenrollAgent( export async function unenrollAgents( soClient: SavedObjectsClientContract, esClient: ElasticsearchClient, - options: - | { - agentIds: string[]; - } - | { - kuery: string; - } + options: GetAgentsOptions ) { - const agents = - 'agentIds' in options - ? await getAgents(esClient, options.agentIds) - : ( - await listAllAgents(esClient, { - kuery: options.kuery, - showInactive: false, - }) - ).agents; + const agents = await getAgents(esClient, options); // Filter to agents that are not already unenrolled, or unenrolling const agentsEnrolled = agents.filter( @@ -116,7 +102,7 @@ export async function forceUnenrollAgent( esClient: ElasticsearchClient, agentId: string ) { - const agent = await getAgent(esClient, agentId); + const agent = await getAgentById(esClient, agentId); await Promise.all([ agent.access_api_key_id @@ -136,24 +122,10 @@ export async function forceUnenrollAgent( export async function forceUnenrollAgents( soClient: SavedObjectsClientContract, esClient: ElasticsearchClient, - options: - | { - agentIds: string[]; - } - | { - kuery: string; - } + options: GetAgentsOptions ) { // Filter to agents that are not already unenrolled - const agents = - 'agentIds' in options - ? await getAgents(esClient, options.agentIds) - : ( - await listAllAgents(esClient, { - kuery: options.kuery, - showInactive: false, - }) - ).agents; + const agents = await getAgents(esClient, options); const agentsToUpdate = agents.filter((agent) => !agent.unenrolled_at); const now = new Date().toISOString(); const apiKeys: string[] = []; diff --git a/x-pack/plugins/fleet/server/services/agents/update.ts b/x-pack/plugins/fleet/server/services/agents/update.ts index 28f1788f3f9b..74386efe6561 100644 --- a/x-pack/plugins/fleet/server/services/agents/update.ts +++ b/x-pack/plugins/fleet/server/services/agents/update.ts @@ -9,7 +9,7 @@ import type { ElasticsearchClient, SavedObjectsClientContract } from 'src/core/s import { AGENT_SAVED_OBJECT_TYPE } from '../../constants'; -import { listAgents } from './crud'; +import { getAgentsByKuery } from './crud'; import { unenrollAgent } from './unenroll'; export async function unenrollForAgentPolicyId( @@ -20,7 +20,7 @@ export async function unenrollForAgentPolicyId( let hasMore = true; let page = 1; while (hasMore) { - const { agents } = await listAgents(esClient, { + const { agents } = await getAgentsByKuery(esClient, { kuery: `${AGENT_SAVED_OBJECT_TYPE}.policy_id:"${policyId}"`, page: page++, perPage: 1000, diff --git a/x-pack/plugins/fleet/server/services/agents/upgrade.ts b/x-pack/plugins/fleet/server/services/agents/upgrade.ts index c45b161a7936..12623be0ed04 100644 --- a/x-pack/plugins/fleet/server/services/agents/upgrade.ts +++ b/x-pack/plugins/fleet/server/services/agents/upgrade.ts @@ -15,13 +15,8 @@ import { isAgentUpgradeable } from '../../../common/services'; import { appContextService } from '../app_context'; import { bulkCreateAgentActions, createAgentAction } from './actions'; -import { - getAgents, - listAllAgents, - updateAgent, - bulkUpdateAgents, - getAgentPolicyForAgent, -} from './crud'; +import type { GetAgentsOptions } from './crud'; +import { getAgents, updateAgent, bulkUpdateAgents, getAgentPolicyForAgent } from './crud'; export async function sendUpgradeAgentAction({ soClient, @@ -82,31 +77,15 @@ export async function ackAgentUpgraded( export async function sendUpgradeAgentsActions( soClient: SavedObjectsClientContract, esClient: ElasticsearchClient, - options: - | { - agentIds: string[]; - sourceUri: string | undefined; - version: string; - force?: boolean; - } - | { - kuery: string; - sourceUri: string | undefined; - version: string; - force?: boolean; - } + options: GetAgentsOptions & { + sourceUri: string | undefined; + version: string; + force?: boolean; + } ) { const kibanaVersion = appContextService.getKibanaVersion(); // Filter out agents currently unenrolling, agents unenrolled, and agents not upgradeable - const agents = - 'agentIds' in options - ? await getAgents(esClient, options.agentIds) - : ( - await listAllAgents(esClient, { - kuery: options.kuery, - showInactive: false, - }) - ).agents; + const agents = await getAgents(esClient, options); // upgradeable if they pass the version check const upgradeableAgents = options.force diff --git a/x-pack/plugins/fleet/server/services/index.ts b/x-pack/plugins/fleet/server/services/index.ts index dec8ecb2b39c..5b4b8b65b203 100644 --- a/x-pack/plugins/fleet/server/services/index.ts +++ b/x-pack/plugins/fleet/server/services/index.ts @@ -10,8 +10,8 @@ import type { ElasticsearchClient, SavedObjectsClientContract } from 'kibana/ser import type { AgentStatus, Agent, EsAssetReference } from '../types'; -import { getAgent, listAgents } from './agents'; -import { agentPolicyService } from './agent_policy'; +import type { getAgentById, getAgentsByKuery } from './agents'; +import type { agentPolicyService } from './agent_policy'; import * as settingsService from './settings'; export { ESIndexPatternSavedObjectService } from './es_index_pattern'; @@ -46,7 +46,7 @@ export interface AgentService { /** * Get an Agent by id */ - getAgent: typeof getAgent; + getAgent: typeof getAgentById; /** * Authenticate an agent with access toekn */ @@ -61,7 +61,7 @@ export interface AgentService { /** * List agents */ - listAgents: typeof listAgents; + listAgents: typeof getAgentsByKuery; } export interface AgentPolicyServiceInterface { diff --git a/x-pack/test/fleet_api_integration/apis/agents/reassign.ts b/x-pack/test/fleet_api_integration/apis/agents/reassign.ts index 7b72e7af6bba..77da9ecce329 100644 --- a/x-pack/test/fleet_api_integration/apis/agents/reassign.ts +++ b/x-pack/test/fleet_api_integration/apis/agents/reassign.ts @@ -14,7 +14,7 @@ export default function (providerContext: FtrProviderContext) { const esArchiver = getService('esArchiver'); const supertest = getService('supertest'); - describe('fleet_reassign_agent', () => { + describe('reassign agent(s)', () => { before(async () => { await esArchiver.load('fleet/empty_fleet_server'); }); @@ -29,99 +29,121 @@ export default function (providerContext: FtrProviderContext) { await esArchiver.unload('fleet/empty_fleet_server'); }); - it('should allow to reassign single agent', async () => { - await supertest - .put(`/api/fleet/agents/agent1/reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - policy_id: 'policy2', - }) - .expect(200); - const { body } = await supertest.get(`/api/fleet/agents/agent1`); - expect(body.item.policy_id).to.eql('policy2'); - }); + describe('reassign single agent', () => { + it('should allow to reassign single agent', async () => { + await supertest + .put(`/api/fleet/agents/agent1/reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + policy_id: 'policy2', + }) + .expect(200); + const { body } = await supertest.get(`/api/fleet/agents/agent1`); + expect(body.item.policy_id).to.eql('policy2'); + }); - it('should throw an error for invalid policy id for single reassign', async () => { - await supertest - .put(`/api/fleet/agents/agent1/reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - policy_id: 'INVALID_ID', - }) - .expect(404); - }); + it('should throw an error for invalid policy id for single reassign', async () => { + await supertest + .put(`/api/fleet/agents/agent1/reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + policy_id: 'INVALID_ID', + }) + .expect(404); + }); - it('should allow to reassign multiple agents by id', async () => { - await supertest - .post(`/api/fleet/agents/bulk_reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - agents: ['agent2', 'agent3'], - policy_id: 'policy2', - }) - .expect(200); - const [agent2data, agent3data] = await Promise.all([ - supertest.get(`/api/fleet/agents/agent2`).set('kbn-xsrf', 'xxx'), - supertest.get(`/api/fleet/agents/agent3`).set('kbn-xsrf', 'xxx'), - ]); - expect(agent2data.body.item.policy_id).to.eql('policy2'); - expect(agent3data.body.item.policy_id).to.eql('policy2'); - }); + it('can reassign from unmanaged policy to unmanaged', async () => { + // policy2 is not managed + // reassign succeeds + await supertest + .put(`/api/fleet/agents/agent1/reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + policy_id: 'policy2', + }) + .expect(200); + }); - it('should allow to reassign multiple agents by kuery', async () => { - await supertest - .post(`/api/fleet/agents/bulk_reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - agents: 'fleet-agents.active: true', - policy_id: 'policy2', - }) - .expect(200); - const { body } = await supertest.get(`/api/fleet/agents`).set('kbn-xsrf', 'xxx'); - expect(body.total).to.eql(4); - body.list.forEach((agent: any) => { - expect(agent.policy_id).to.eql('policy2'); + it('cannot reassign from unmanaged policy to managed', 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); + + // reassign fails + await supertest + .put(`/api/fleet/agents/agent1/reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + policy_id: 'policy2', + }) + .expect(400); }); }); - it('should throw an error for invalid policy id for bulk reassign', async () => { - await supertest - .post(`/api/fleet/agents/bulk_reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - agents: ['agent2', 'agent3'], - policy_id: 'INVALID_ID', - }) - .expect(404); - }); + describe('bulk reassign agents', () => { + it('should allow to reassign multiple agents by id', async () => { + await supertest + .post(`/api/fleet/agents/bulk_reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + agents: ['agent2', 'agent3'], + policy_id: 'policy2', + }) + .expect(200); + const [agent2data, agent3data] = await Promise.all([ + supertest.get(`/api/fleet/agents/agent2`).set('kbn-xsrf', 'xxx'), + supertest.get(`/api/fleet/agents/agent3`).set('kbn-xsrf', 'xxx'), + ]); + expect(agent2data.body.item.policy_id).to.eql('policy2'); + expect(agent3data.body.item.policy_id).to.eql('policy2'); + }); - it('can reassign from unmanaged policy to unmanaged', async () => { - // policy2 is not managed - // reassign succeeds - await supertest - .put(`/api/fleet/agents/agent1/reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - policy_id: 'policy2', - }) - .expect(200); - }); - it('cannot reassign from unmanaged policy to managed', 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); + it('should allow to reassign multiple agents by id -- some invalid', async () => { + 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); + 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('policy2'); + expect(agent3data.body.item.policy_id).to.eql('policy2'); + }); - // reassign fails - await supertest - .put(`/api/fleet/agents/agent1/reassign`) - .set('kbn-xsrf', 'xxx') - .send({ - policy_id: 'policy2', - }) - .expect(400); + it('should allow to reassign multiple agents by kuery', async () => { + await supertest + .post(`/api/fleet/agents/bulk_reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + agents: 'fleet-agents.active: true', + policy_id: 'policy2', + }) + .expect(200); + const { body } = await supertest.get(`/api/fleet/agents`).set('kbn-xsrf', 'xxx'); + expect(body.total).to.eql(4); + body.list.forEach((agent: any) => { + expect(agent.policy_id).to.eql('policy2'); + }); + }); + + it('should throw an error for invalid policy id for bulk reassign', async () => { + await supertest + .post(`/api/fleet/agents/bulk_reassign`) + .set('kbn-xsrf', 'xxx') + .send({ + agents: ['agent2', 'agent3'], + policy_id: 'INVALID_ID', + }) + .expect(404); + }); }); }); }