Skip to content

Commit

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

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Fleet] Fixes wrong artifact Id on bulkCreate response
(#160710)](#160710)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"David
Sánchez","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-06-28T13:49:31Z","message":"[Fleet]
Fixes wrong artifact Id on bulkCreate response (#160710)\n\n##
Summary\r\n\r\nThe artifact Id on the bulkCreateArtifact response was
wrong. There is\r\nno issue currently as this id is not used, but it
could end up in a bug\r\nif at some point someone somewhere uses this
wrong id.\r\n\r\nImproved unit test to handle this.\r\n\r\nRelated PR:
https://github.com/elastic/kibana/pull/159187","sha":"4cc04bd8e35320603a592de619791f26ce3b74a8","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","Team:Defend
Workflows","v8.9.0","v8.10.0"],"number":160710,"url":"https://github.com/elastic/kibana/pull/160710","mergeCommit":{"message":"[Fleet]
Fixes wrong artifact Id on bulkCreate response (#160710)\n\n##
Summary\r\n\r\nThe artifact Id on the bulkCreateArtifact response was
wrong. There is\r\nno issue currently as this id is not used, but it
could end up in a bug\r\nif at some point someone somewhere uses this
wrong id.\r\n\r\nImproved unit test to handle this.\r\n\r\nRelated PR:
https://github.com/elastic/kibana/pull/159187","sha":"4cc04bd8e35320603a592de619791f26ce3b74a8"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/160710","number":160710,"mergeCommit":{"message":"[Fleet]
Fixes wrong artifact Id on bulkCreate response (#160710)\n\n##
Summary\r\n\r\nThe artifact Id on the bulkCreateArtifact response was
wrong. There is\r\nno issue currently as this id is not used, but it
could end up in a bug\r\nif at some point someone somewhere uses this
wrong id.\r\n\r\nImproved unit test to handle this.\r\n\r\nRelated PR:
https://github.com/elastic/kibana/pull/159187","sha":"4cc04bd8e35320603a592de619791f26ce3b74a8"}}]}]
BACKPORT-->

Co-authored-by: David Sánchez <[email protected]>
  • Loading branch information
@kibanamachine @dasansol92
kibanamachine and dasansol92 authored Jun 28, 2023
1 parent b70cf70 commit c82af61
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
View file
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
View file
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 c82af61

Please sign in to comment.