Skip to content

Commit

Permalink
[Fleet] Fixes wrong artifact Id on bulkCreate response (#160710)
Browse files Browse the repository at this point in the history
## Summary

The artifact Id on the bulkCreateArtifact response was wrong. There is
no issue currently as this id is not used, but it could end up in a bug
if at some point someone somewhere uses this wrong id.

Improved unit test to handle this.

Related PR: #159187
  • Loading branch information
dasansol92 authored Jun 28, 2023
1 parent 67312ab commit 4cc04bd
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 32 deletions.
34 changes: 21 additions & 13 deletions x-pack/plugins/fleet/server/services/artifacts/artifacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { ArtifactsElasticsearchError } from '../../errors';
import { appContextService } from '../app_context';
import { createAppContextStartContractMock } from '../../mocks';

import { newArtifactToElasticsearchProperties } from './mappings';
import { newArtifactToElasticsearchProperties, uniqueIdFromArtifact } from './mappings';

import {
generateArtifactEsGetSingleHitMock,
Expand Down Expand Up @@ -186,14 +186,22 @@ describe('When using the artifacts services', () => {
});

it('should create and return a multiple big artifacts', async () => {
const { ...generatedArtifact1 } = generateArtifactMock({ encodedSize: 5_000_500 });
const newBigArtifact1 = generatedArtifact1;
const { ...generatedArtifact2 } = generateArtifactMock({ encodedSize: 500 });
const newBigArtifact2 = generatedArtifact2;
const { ...generatedArtifact3 } = generateArtifactMock({ encodedSize: 233 });
const newBigArtifact3 = generatedArtifact3;
const { ...generatedArtifact4 } = generateArtifactMock({ encodedSize: 7_000_000 });
const newBigArtifact4 = generatedArtifact4;
const newBigArtifact1 = generateArtifactMock({
encodedSize: 5_000_500,
decodedSha256: '1234',
});
const newBigArtifact2 = generateArtifactMock({
encodedSize: 500,
decodedSha256: '2345',
});
const newBigArtifact3 = generateArtifactMock({
encodedSize: 233,
decodedSha256: '3456',
});
const newBigArtifact4 = generateArtifactMock({
encodedSize: 7_000_000,
decodedSha256: '4567',
});

const { artifacts } = await bulkCreateArtifacts(esClientMock, [
newBigArtifact1,
Expand Down Expand Up @@ -267,22 +275,22 @@ describe('When using the artifacts services', () => {

expect(artifact1).toEqual({
...newBigArtifact1,
id: expect.any(String),
id: uniqueIdFromArtifact(newBigArtifact1),
created: expect.any(String),
});
expect(artifact2).toEqual({
...newBigArtifact2,
id: expect.any(String),
id: uniqueIdFromArtifact(newBigArtifact2),
created: expect.any(String),
});
expect(artifact3).toEqual({
...newBigArtifact3,
id: expect.any(String),
id: uniqueIdFromArtifact(newBigArtifact3),
created: expect.any(String),
});
expect(artifact4).toEqual({
...newBigArtifact4,
id: expect.any(String),
id: uniqueIdFromArtifact(newBigArtifact4),
created: expect.any(String),
});
});
Expand Down
33 changes: 14 additions & 19 deletions x-pack/plugins/fleet/server/services/artifacts/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,20 +95,13 @@ export const BULK_CREATE_MAX_ARTIFACTS_BYTES = 4_000_000;
const generateArtifactBatches = (
artifacts: NewArtifact[],
maxArtifactsBatchSizeInBytes: number = BULK_CREATE_MAX_ARTIFACTS_BYTES
): {
batches: Array<Array<ArtifactElasticsearchProperties | { create: { _id: string } }>>;
artifactsEsResponse: Artifact[];
} => {
): Array<Array<ArtifactElasticsearchProperties | { create: { _id: string } }>> => {
const batches: Array<Array<ArtifactElasticsearchProperties | { create: { _id: string } }>> = [];
const artifactsEsResponse: Artifact[] = [];

let artifactsBatchLengthInBytes = 0;
const sortedArtifacts = sortBy(artifacts, 'encodedSize');

sortedArtifacts.forEach((artifact, index) => {
const esArtifactResponse = esSearchHitToArtifact({
_id: uniqueIdFromArtifact(artifact),
_source: newArtifactToElasticsearchProperties(artifacts[index]),
});
sortedArtifacts.forEach((artifact) => {
const esArtifact = newArtifactToElasticsearchProperties(artifact);
const bulkOperation = {
create: {
Expand All @@ -120,9 +113,6 @@ const generateArtifactBatches = (
// If there is no artifact yet added to the current batch, we add it anyway ignoring the batch limit as the batch size has to be > 0.
if (artifact.encodedSize + artifactsBatchLengthInBytes >= maxArtifactsBatchSizeInBytes) {
artifactsBatchLengthInBytes = artifact.encodedSize;
// Use non sorted artifacts array to preserve the artifacts order in the response
artifactsEsResponse.push(esArtifactResponse);

batches.push([bulkOperation, esArtifact]);
} else {
// Case it's the first one
Expand All @@ -131,22 +121,19 @@ const generateArtifactBatches = (
}
// Adds the next artifact to the current batch and increases the batch size count with the artifact size.
artifactsBatchLengthInBytes += artifact.encodedSize;
// Use non sorted artifacts array to preserve the artifacts order in the response
artifactsEsResponse.push(esArtifactResponse);

batches[batches.length - 1].push(bulkOperation, esArtifact);
}
});

return { batches, artifactsEsResponse };
return batches;
};

export const bulkCreateArtifacts = async (
esClient: ElasticsearchClient,
artifacts: NewArtifact[],
refresh = false
): Promise<{ artifacts?: Artifact[]; errors?: Error[] }> => {
const { batches, artifactsEsResponse } = generateArtifactBatches(
const batches = generateArtifactBatches(
artifacts,
appContextService.getConfig()?.createArtifactsBulkBatchSize
);
Expand Down Expand Up @@ -185,8 +172,16 @@ export const bulkCreateArtifacts = async (
return { errors: nonConflictErrors };
}

// Use non sorted artifacts array to preserve the artifacts order in the response
const nonSortedEsArtifactsResponse: Artifact[] = artifacts.map((artifact) => {
return esSearchHitToArtifact({
_id: uniqueIdFromArtifact(artifact),
_source: newArtifactToElasticsearchProperties(artifact),
});
});

return {
artifacts: artifactsEsResponse,
artifacts: nonSortedEsArtifactsResponse,
};
};

Expand Down

0 comments on commit 4cc04bd

Please sign in to comment.