From 35b954e03d09835ac86424c6eb2d718ece4aa999 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 29 Jun 2023 14:14:45 +0200 Subject: [PATCH 1/6] Uses fleet artifacts list method instead of getting all one by one --- .../manifest_manager/manifest_manager.test.ts | 19 +++++-- .../manifest_manager/manifest_manager.ts | 55 +++++++++++-------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.test.ts b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.test.ts index f6859015f42fc..6a498d142a383 100644 --- a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.test.ts +++ b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.test.ts @@ -42,6 +42,7 @@ import { ManifestManager } from './manifest_manager'; import type { EndpointArtifactClientInterface } from '../artifact_client'; import { InvalidInternalManifestError } from '../errors'; import { EndpointError } from '../../../../../common/endpoint/errors'; +import type { Artifact } from '@kbn/fleet-plugin/server'; const getArtifactObject = (artifact: InternalArtifactSchema) => JSON.parse(Buffer.from(artifact.body!, 'base64').toString()); @@ -195,8 +196,8 @@ describe('ManifestManager', () => { ( manifestManagerContext.artifactClient as jest.Mocked - ).getArtifact.mockImplementation(async (id) => { - return ARTIFACTS_BY_ID[id]; + ).listArtifacts.mockImplementation(async () => { + return { items: ARTIFACTS as Artifact[], total: 100, page: 1, perPage: 100 }; }); const manifest = await manifestManager.getLastComputedManifest(); @@ -254,9 +255,19 @@ describe('ManifestManager', () => { ( manifestManagerContext.artifactClient as jest.Mocked - ).getArtifact.mockImplementation(async (id) => { + ).listArtifacts.mockImplementation(async (id) => { // report the MACOS Exceptions artifact as not found - return id === ARTIFACT_ID_EXCEPTIONS_MACOS ? undefined : ARTIFACTS_BY_ID[id]; + return { + items: [ + ARTIFACT_TRUSTED_APPS_MACOS, + ARTIFACT_EXCEPTIONS_WINDOWS, + ARTIFACT_TRUSTED_APPS_WINDOWS, + ARTIFACTS_BY_ID[ARTIFACT_ID_EXCEPTIONS_LINUX], + ] as Artifact[], + total: 100, + page: 1, + perPage: 100, + }; }); const manifest = await manifestManager.getLastComputedManifest(); diff --git a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts index b7c98ef1b8773..bf35c655b649a 100644 --- a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts +++ b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts @@ -7,7 +7,7 @@ import pMap from 'p-map'; import semver from 'semver'; -import { isEqual, isEmpty, chunk } from 'lodash'; +import { isEqual, isEmpty, chunk, keyBy } from 'lodash'; import type { ElasticsearchClient } from '@kbn/core/server'; import { type Logger, type SavedObjectsClientContract } from '@kbn/core/server'; import { @@ -18,7 +18,7 @@ import { ENDPOINT_LIST_ID, } from '@kbn/securitysolution-list-constants'; import type { ListResult, PackagePolicy } from '@kbn/fleet-plugin/common'; -import type { PackagePolicyClient } from '@kbn/fleet-plugin/server'; +import type { Artifact, PackagePolicyClient } from '@kbn/fleet-plugin/server'; import type { ExceptionListClient } from '@kbn/lists-plugin/server'; import type { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; import type { ManifestSchemaVersion } from '../../../../../common/endpoint/schema/common'; @@ -477,8 +477,11 @@ export class ManifestManager { soVersion: manifestSo.version, }); + const fleetArtifacts = await this.listAllArtifacts(); + const fleetArtifactsById = keyBy(fleetArtifacts, (artifact) => getArtifactId(artifact)); + for (const entry of manifestSo.attributes.artifacts) { - const artifact = await this.artifactClient.getArtifact(entry.artifactId); + const artifact = fleetArtifactsById[entry.artifactId]; if (!artifact) { this.logger.error( @@ -694,32 +697,40 @@ export class ManifestManager { } /** - * Cleanup .fleet-artifacts index if there are some orphan artifacts + * Retrieves all .fleet-artifacts for endpoint package + * @returns Artifact[] */ - public async cleanup(manifest: Manifest) { - try { - const fleetArtifacts = []; - const perPage = 100; - let page = 1; + private async listAllArtifacts(): Promise { + const fleetArtifacts = []; + const perPage = 100; + let page = 1; - let fleetArtifactsResponse = await this.artifactClient.listArtifacts({ + let fleetArtifactsResponse = await this.artifactClient.listArtifacts({ + perPage, + page, + }); + fleetArtifacts.push(...fleetArtifactsResponse.items); + + while ( + fleetArtifactsResponse.total > fleetArtifacts.length && + !isEmpty(fleetArtifactsResponse.items) + ) { + page += 1; + fleetArtifactsResponse = await this.artifactClient.listArtifacts({ perPage, page, }); fleetArtifacts.push(...fleetArtifactsResponse.items); + } + return fleetArtifacts; + } - while ( - fleetArtifactsResponse.total > fleetArtifacts.length && - !isEmpty(fleetArtifactsResponse.items) - ) { - page += 1; - fleetArtifactsResponse = await this.artifactClient.listArtifacts({ - perPage, - page, - }); - fleetArtifacts.push(...fleetArtifactsResponse.items); - } - + /** + * Cleanup .fleet-artifacts index if there are some orphan artifacts + */ + public async cleanup(manifest: Manifest) { + try { + const fleetArtifacts = await this.listAllArtifacts(); if (isEmpty(fleetArtifacts)) { return; } From c3a5a6247a4ffb87226f1c2ac251b3c9f4bcb0f4 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 29 Jun 2023 14:15:57 +0200 Subject: [PATCH 2/6] Adds log for how much time task is running --- .../security_solution/server/endpoint/lib/artifacts/task.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts b/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts index d6f8e92c02c8a..426f48e22c1e2 100644 --- a/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts +++ b/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts @@ -53,7 +53,12 @@ export class ManifestTask { return { run: async () => { const taskInterval = (await this.endpointAppContext.config()).packagerTaskInterval; + const initTime = new Date().getTime(); await this.runTask(taskInstance.id); + const endTime = new Date().getTime(); + this.logger.debug( + `${ManifestTaskConstants.TYPE} task run took ${endTime - initTime}ms` + ); const nextRun = new Date(); if (taskInterval.endsWith('s')) { const seconds = parseInt(taskInterval.slice(0, -1), 10); From b0b087093724eee86056f16c2bf0116e06c1bb06 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 29 Jun 2023 15:29:53 +0200 Subject: [PATCH 3/6] Adds log for when a new artifact is added to the manifest --- .../services/artifacts/manifest_manager/manifest_manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts index bf35c655b649a..453bc6383cb1e 100644 --- a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts +++ b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts @@ -421,8 +421,8 @@ export class ManifestManager { const fleetArtifact = fleetArtfactsByIdentifier[artifactId]; if (!fleetArtifact) return; - newManifest.replaceArtifact(fleetArtifact); + this.logger.debug(`New created artifact ${artifactId} added to the manifest`); }); } From 6f6019b7a9dc202877884711db62ca33440f7e20 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 29 Jun 2023 15:39:18 +0200 Subject: [PATCH 4/6] Uses const for perPage value and reuse it --- .../security_solution/server/endpoint/lib/artifacts/lists.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/lists.ts b/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/lists.ts index f8fe9d6e71453..d1c07d203d580 100644 --- a/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/lists.ts +++ b/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/lists.ts @@ -99,6 +99,7 @@ export async function getFilteredEndpointExceptionListRaw({ filter: string; listId: ArtifactListId; }): Promise { + const perPage = 1000; let exceptions: ExceptionListItemSchema[] = []; let page = 1; let paging = true; @@ -108,7 +109,7 @@ export async function getFilteredEndpointExceptionListRaw({ listId, namespaceType: 'agnostic', filter, - perPage: 1000, + perPage, page, sortField: 'created_at', sortOrder: 'desc', @@ -117,7 +118,7 @@ export async function getFilteredEndpointExceptionListRaw({ if (response?.data !== undefined) { exceptions = exceptions.concat(response.data); - paging = (page - 1) * 1000 + response.data.length < response.total; + paging = (page - 1) * perPage + response.data.length < response.total; page++; } else { break; From 6c6e88ac424f5855aee65882ae4b5858e2ea4ab7 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 29 Jun 2023 15:39:55 +0200 Subject: [PATCH 5/6] Adds missing return types to functions --- .../manifest_manager/manifest_manager.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts index 453bc6383cb1e..e6b15c07262f7 100644 --- a/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts +++ b/x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts @@ -34,7 +34,10 @@ import { Manifest, convertExceptionsToEndpointFormat, } from '../../../lib/artifacts'; -import type { InternalArtifactCompleteSchema } from '../../../schemas/artifacts'; +import type { + InternalArtifactCompleteSchema, + WrappedTranslatedExceptionList, +} from '../../../schemas/artifacts'; import { internalArtifactCompleteSchema } from '../../../schemas/artifacts'; import type { EndpointArtifactClientInterface } from '../artifact_client'; import { ManifestClient } from '../manifest_client'; @@ -155,7 +158,7 @@ export class ManifestManager { os: string; policyId?: string; schemaVersion: string; - }) { + }): Promise { if (!this.cachedExceptionsListsByOs.has(`${listId}-${os}`)) { const itemsByListId = await getAllItemsFromEndpointExceptionList({ elClient, @@ -218,7 +221,7 @@ export class ManifestManager { allPolicyIds: string[], supportedOSs: string[], osOptions: BuildArtifactsForOsOptions - ) { + ): Promise> { const policySpecificArtifacts: Record = {}; await pMap( allPolicyIds, @@ -618,7 +621,7 @@ export class ManifestManager { currentBatch ); - // Parse errors + // Update errors if (!isEmpty(response.failedPolicies)) { errors.push( ...response.failedPolicies.map((failedPolicy) => { @@ -667,7 +670,10 @@ export class ManifestManager { this.logger.info(`Committed manifest ${manifest.getSemanticVersion()}`); } - private async listEndpointPolicies(page: number, perPage: number) { + private async listEndpointPolicies( + page: number, + perPage: number + ): Promise> { return this.packagePolicyService.list(this.savedObjectsClient, { page, perPage, @@ -675,7 +681,7 @@ export class ManifestManager { }); } - private async listEndpointPolicyIds() { + private async listEndpointPolicyIds(): Promise { const allPolicyIds: string[] = []; await iterateAllListItems( (page, perPage) => { From 339d971742e5e86ef02fa643c8557423635194fd Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 29 Jun 2023 16:03:45 +0200 Subject: [PATCH 6/6] Renames start time const --- .../security_solution/server/endpoint/lib/artifacts/task.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts b/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts index 426f48e22c1e2..dafa13141a0c6 100644 --- a/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts +++ b/x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts @@ -53,11 +53,11 @@ export class ManifestTask { return { run: async () => { const taskInterval = (await this.endpointAppContext.config()).packagerTaskInterval; - const initTime = new Date().getTime(); + const startTime = new Date().getTime(); await this.runTask(taskInstance.id); const endTime = new Date().getTime(); this.logger.debug( - `${ManifestTaskConstants.TYPE} task run took ${endTime - initTime}ms` + `${ManifestTaskConstants.TYPE} task run took ${endTime - startTime}ms` ); const nextRun = new Date(); if (taskInterval.endsWith('s')) {