From 8e736c4918afdf3f038c03ae3ea75e37259ad431 Mon Sep 17 00:00:00 2001 From: Shahzad Date: Wed, 14 Aug 2024 17:27:41 +0200 Subject: [PATCH] [Fleet] Avoid extra fetch package policy calls (#190398) ## Summary i noticed few extra calls which can be prevented during package policy updates/creation while investigating synthetics project monitor API call !! --------- Co-authored-by: Nicolas Chaulet --- .../fleet/common/types/rest_spec/common.ts | 1 + .../agent_policies/full_agent_policy.ts | 11 ++++- .../fleet/server/services/agent_policy.ts | 49 ++++++++++++++----- .../server/services/agent_policy_update.ts | 8 +-- .../services/fleet_server/index.test.ts | 1 + .../server/services/fleet_server/index.ts | 6 ++- .../fleet/server/services/package_policy.ts | 10 +++- 7 files changed, 66 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/fleet/common/types/rest_spec/common.ts b/x-pack/plugins/fleet/common/types/rest_spec/common.ts index 07bd3c7019c168..2eda7dd7f99830 100644 --- a/x-pack/plugins/fleet/common/types/rest_spec/common.ts +++ b/x-pack/plugins/fleet/common/types/rest_spec/common.ts @@ -13,6 +13,7 @@ export interface ListWithKuery extends HttpFetchQuery { sortField?: string; sortOrder?: 'desc' | 'asc'; kuery?: string; + fields?: string[]; } export interface ListResult { diff --git a/x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts index 0cfce9eb264f45..19d01af65a5c6d 100644 --- a/x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policies/full_agent_policy.ts @@ -25,6 +25,7 @@ import type { FullAgentPolicyOutput, FleetProxy, FleetServerHost, + AgentPolicy, } from '../../types'; import type { FullAgentPolicyMonitoring, @@ -67,11 +68,17 @@ async function fetchAgentPolicy(soClient: SavedObjectsClientContract, id: string export async function getFullAgentPolicy( soClient: SavedObjectsClientContract, id: string, - options?: { standalone: boolean } + options?: { standalone?: boolean; agentPolicy?: AgentPolicy } ): Promise { const standalone = options?.standalone ?? false; - const agentPolicy = await fetchAgentPolicy(soClient, id); + let agentPolicy: AgentPolicy | null; + if (options?.agentPolicy?.package_policies) { + agentPolicy = options.agentPolicy; + } else { + agentPolicy = await fetchAgentPolicy(soClient, id); + } + if (!agentPolicy) { return null; } diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts index 42bede95986b6c..a7176083a718f7 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.ts @@ -121,7 +121,7 @@ class AgentPolicyService { esClient: ElasticsearchClient, action: 'created' | 'updated' | 'deleted', agentPolicyId: string, - options?: { skipDeploy?: boolean; spaceId?: string } + options?: { skipDeploy?: boolean; spaceId?: string; agentPolicy?: AgentPolicy | null } ) => { return agentPolicyUpdateEventHandler(esClient, action, agentPolicyId, options); }; @@ -132,10 +132,16 @@ class AgentPolicyService { id: string, agentPolicy: Partial, user?: AuthenticatedUser, - options: { bumpRevision: boolean; removeProtection: boolean; skipValidation: boolean } = { + options: { + bumpRevision: boolean; + removeProtection: boolean; + skipValidation: boolean; + returnUpdatedPolicy?: boolean; + } = { bumpRevision: true, removeProtection: false, skipValidation: false, + returnUpdatedPolicy: true, } ): Promise { auditLoggingService.writeCustomSoAuditLog({ @@ -173,6 +179,7 @@ class AgentPolicyService { getAllowedOutputTypeForPolicy(existingAgentPolicy) ); } + await soClient.update(SAVED_OBJECT_TYPE, id, { ...agentPolicy, ...(options.bumpRevision ? { revision: existingAgentPolicy.revision + 1 } : {}), @@ -183,9 +190,14 @@ class AgentPolicyService { updated_by: user ? user.username : 'system', }); + const newAgentPolicy = await this.get(soClient, id, false); + + newAgentPolicy!.package_policies = existingAgentPolicy.package_policies; + if (options.bumpRevision || options.removeProtection) { await this.triggerAgentPolicyUpdatedEvent(esClient, 'updated', id, { spaceId: soClient.getCurrentNamespace(), + agentPolicy: newAgentPolicy, }); } logger.debug( @@ -193,7 +205,10 @@ class AgentPolicyService { options.bumpRevision ? existingAgentPolicy.revision + 1 : existingAgentPolicy.revision }` ); - return (await this.get(soClient, id)) as AgentPolicy; + if (options.returnUpdatedPolicy !== false) { + return (await this.get(soClient, id)) as AgentPolicy; + } + return newAgentPolicy as AgentPolicy; } public async ensurePreconfiguredAgentPolicy( @@ -772,14 +787,13 @@ class AgentPolicyService { esClient: ElasticsearchClient, id: string, options?: { user?: AuthenticatedUser; removeProtection?: boolean } - ): Promise { - const res = await this._update(soClient, esClient, id, {}, options?.user, { + ): Promise { + await this._update(soClient, esClient, id, {}, options?.user, { bumpRevision: true, removeProtection: options?.removeProtection ?? false, skipValidation: false, + returnUpdatedPolicy: false, }); - - return res; } /** @@ -1132,11 +1146,19 @@ class AgentPolicyService { }; } - public async deployPolicy(soClient: SavedObjectsClientContract, agentPolicyId: string) { - await this.deployPolicies(soClient, [agentPolicyId]); + public async deployPolicy( + soClient: SavedObjectsClientContract, + agentPolicyId: string, + agentPolicy?: AgentPolicy | null + ) { + await this.deployPolicies(soClient, [agentPolicyId], agentPolicy ? [agentPolicy] : undefined); } - public async deployPolicies(soClient: SavedObjectsClientContract, agentPolicyIds: string[]) { + public async deployPolicies( + soClient: SavedObjectsClientContract, + agentPolicyIds: string[], + agentPolicies?: AgentPolicy[] + ) { // Use internal ES client so we have permissions to write to .fleet* indices const esClient = appContextService.getInternalUserESClient(); const defaultOutputId = await outputService.getDefaultDataOutputId(soClient); @@ -1158,7 +1180,10 @@ class AgentPolicyService { // There are some potential performance concerns around using `getFullAgentPolicy` in this context, e.g. // re-fetching outputs, settings, and upgrade download source URI data for each policy. This could potentially // be a bottleneck in environments with several thousand agent policies being deployed here. - (agentPolicyId) => agentPolicyService.getFullAgentPolicy(soClient, agentPolicyId), + (agentPolicyId) => + agentPolicyService.getFullAgentPolicy(soClient, agentPolicyId, { + agentPolicy: agentPolicies?.find((policy) => policy.id === agentPolicyId), + }), { concurrency: 50, } @@ -1359,7 +1384,7 @@ class AgentPolicyService { public async getFullAgentPolicy( soClient: SavedObjectsClientContract, id: string, - options?: { standalone: boolean } + options?: { standalone?: boolean; agentPolicy?: AgentPolicy } ): Promise { return getFullAgentPolicy(soClient, id, options); } diff --git a/x-pack/plugins/fleet/server/services/agent_policy_update.ts b/x-pack/plugins/fleet/server/services/agent_policy_update.ts index 54203900a7dc11..b295c2ca6d7448 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy_update.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy_update.ts @@ -8,6 +8,8 @@ import type { KibanaRequest } from '@kbn/core/server'; import type { ElasticsearchClient } from '@kbn/core/server'; +import type { AgentPolicy } from '../../common'; + import { generateEnrollmentAPIKey, deleteEnrollmentApiKeyForAgentPolicyId } from './api_keys'; import { unenrollForAgentPolicyId } from './agents'; import { agentPolicyService } from './agent_policy'; @@ -32,7 +34,7 @@ export async function agentPolicyUpdateEventHandler( esClient: ElasticsearchClient, action: string, agentPolicyId: string, - options?: { skipDeploy?: boolean; spaceId?: string } + options?: { skipDeploy?: boolean; spaceId?: string; agentPolicy?: AgentPolicy | null } ) { // `soClient` from ingest `appContextService` is used to create policy change actions // to ensure encrypted SOs are handled correctly @@ -47,12 +49,12 @@ export async function agentPolicyUpdateEventHandler( forceRecreate: true, }); if (!options?.skipDeploy) { - await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId); + await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId, options?.agentPolicy); } } if (action === 'updated') { - await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId); + await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId, options?.agentPolicy); } if (action === 'deleted') { diff --git a/x-pack/plugins/fleet/server/services/fleet_server/index.test.ts b/x-pack/plugins/fleet/server/services/fleet_server/index.test.ts index 7faea8c5268192..ac97411a5b92fc 100644 --- a/x-pack/plugins/fleet/server/services/fleet_server/index.test.ts +++ b/x-pack/plugins/fleet/server/services/fleet_server/index.test.ts @@ -45,6 +45,7 @@ describe('checkFleetServerVersionsForSecretsStorage', () => { afterEach(() => { appContextService.stop(); jest.clearAllMocks(); + jest.restoreAllMocks(); }); const esClientMock = elasticsearchServiceMock.createElasticsearchClient(); diff --git a/x-pack/plugins/fleet/server/services/fleet_server/index.ts b/x-pack/plugins/fleet/server/services/fleet_server/index.ts index a0d508f0929e93..7a5c4a48695d6b 100644 --- a/x-pack/plugins/fleet/server/services/fleet_server/index.ts +++ b/x-pack/plugins/fleet/server/services/fleet_server/index.ts @@ -113,18 +113,20 @@ export async function checkFleetServerVersionsForSecretsStorage( let hasMore = true; const policyIds = new Set(); let page = 1; + const perPage = 200; while (hasMore) { const res = await packagePolicyService.list(soClient, { page: page++, - perPage: 20, + perPage, kuery: 'ingest-package-policies.package.name:fleet_server', + fields: ['policy_ids'], }); for (const item of res.items) { item.policy_ids.forEach((id) => policyIds.add(id)); } - if (res.items.length === 0) { + if (res.items.length < perPage) { hasMore = false; } } diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 65495d51f0f60c..cafb7e85d9d32a 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -739,7 +739,14 @@ class PackagePolicyClientImpl implements PackagePolicyClient { soClient: SavedObjectsClientContract, options: ListWithKuery & { spaceId?: string } ): Promise> { - const { page = 1, perPage = 20, sortField = 'updated_at', sortOrder = 'desc', kuery } = options; + const { + page = 1, + perPage = 20, + sortField = 'updated_at', + sortOrder = 'desc', + kuery, + fields, + } = options; const packagePolicies = await soClient.find({ type: SAVED_OBJECT_TYPE, @@ -747,6 +754,7 @@ class PackagePolicyClientImpl implements PackagePolicyClient { sortOrder, page, perPage, + fields, filter: kuery ? normalizeKuery(SAVED_OBJECT_TYPE, kuery) : undefined, namespaces: options.spaceId ? [options.spaceId] : undefined, });