Skip to content

Commit

Permalink
[Security Solution][Endpoint] Reduce fleet artifact calls (#160890)
Browse files Browse the repository at this point in the history
## Summary

- Reduces fleet artifact get calls by getting them all at one time
instead of requesting it one by one using id's
- Updates unit test.
- Adds duration time log to endpoint packager task.
- Adds missing return types to functions.

Follow up of: #160387
  • Loading branch information
dasansol92 authored Jun 30, 2023
1 parent 3a6d710 commit 5666b14
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export async function getFilteredEndpointExceptionListRaw({
filter: string;
listId: ArtifactListId;
}): Promise<ExceptionListItemSchema[]> {
const perPage = 1000;
let exceptions: ExceptionListItemSchema[] = [];
let page = 1;
let paging = true;
Expand All @@ -108,7 +109,7 @@ export async function getFilteredEndpointExceptionListRaw({
listId,
namespaceType: 'agnostic',
filter,
perPage: 1000,
perPage,
page,
sortField: 'created_at',
sortOrder: 'desc',
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,12 @@ export class ManifestTask {
return {
run: async () => {
const taskInterval = (await this.endpointAppContext.config()).packagerTaskInterval;
const startTime = new Date().getTime();
await this.runTask(taskInstance.id);
const endTime = new Date().getTime();
this.logger.debug(
`${ManifestTaskConstants.TYPE} task run took ${endTime - startTime}ms`
);
const nextRun = new Date();
if (taskInterval.endsWith('s')) {
const seconds = parseInt(taskInterval.slice(0, -1), 10);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -195,8 +196,8 @@ describe('ManifestManager', () => {

(
manifestManagerContext.artifactClient as jest.Mocked<EndpointArtifactClientInterface>
).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();
Expand Down Expand Up @@ -254,9 +255,19 @@ describe('ManifestManager', () => {

(
manifestManagerContext.artifactClient as jest.Mocked<EndpointArtifactClientInterface>
).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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -155,7 +158,7 @@ export class ManifestManager {
os: string;
policyId?: string;
schemaVersion: string;
}) {
}): Promise<WrappedTranslatedExceptionList> {
if (!this.cachedExceptionsListsByOs.has(`${listId}-${os}`)) {
const itemsByListId = await getAllItemsFromEndpointExceptionList({
elClient,
Expand Down Expand Up @@ -218,7 +221,7 @@ export class ManifestManager {
allPolicyIds: string[],
supportedOSs: string[],
osOptions: BuildArtifactsForOsOptions
) {
): Promise<Record<string, InternalArtifactCompleteSchema[]>> {
const policySpecificArtifacts: Record<string, InternalArtifactCompleteSchema[]> = {};
await pMap(
allPolicyIds,
Expand Down Expand Up @@ -421,8 +424,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`);
});
}

Expand Down Expand Up @@ -477,8 +480,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(
Expand Down Expand Up @@ -615,7 +621,7 @@ export class ManifestManager {
currentBatch
);

// Parse errors
// Update errors
if (!isEmpty(response.failedPolicies)) {
errors.push(
...response.failedPolicies.map((failedPolicy) => {
Expand Down Expand Up @@ -664,15 +670,18 @@ 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<ListResult<PackagePolicy>> {
return this.packagePolicyService.list(this.savedObjectsClient, {
page,
perPage,
kuery: 'ingest-package-policies.package.name:endpoint',
});
}

private async listEndpointPolicyIds() {
private async listEndpointPolicyIds(): Promise<string[]> {
const allPolicyIds: string[] = [];
await iterateAllListItems(
(page, perPage) => {
Expand All @@ -694,32 +703,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<Artifact[]> {
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;
}
Expand Down

0 comments on commit 5666b14

Please sign in to comment.