Skip to content

Commit

Permalink
[8.x] Fix issue with duplicate references in error object when copyin…
Browse files Browse the repository at this point in the history
…g saved objects to space (#200053) (#200601)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Fix issue with duplicate references in error object when copying
saved objects to space
(#200053)](#200053)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT
[{"author":{"name":"Sid","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-18T15:46:07Z","message":"Fix
issue with duplicate references in error object when copying saved
objects to space (#200053)\n\nCloses
https://github.com/elastic/kibana/issues/158027\r\n\r\n##
Summary\r\n\r\nSimply dedupes references to objects if they are part of
the\r\nmissing_references in the copy saved objects to SO
endpoint\r\n\r\n### Notes\r\n- Update forEach over SOs to a regular for
loop since we had a couple of\r\nearly exit scenarios\r\n- Checks
against the set for references already added to the missing\r\nlist and
adds only if not present\r\n\r\n------\r\n\r\n**Old response: Note the
duplicate references**\r\n\r\n<img width=\"400\" alt=\"Screenshot
2024-11-14 at 01 52
54\"\r\nsrc=\"https://github.com/user-attachments/assets/67078080-e39d-43b2-bf7c-7abb76866fa4\">\r\n\r\n\r\n**New
response**\r\n\r\n<img width=\"800\" alt=\"Screenshot 2024-11-14 at 01
50
41\"\r\nsrc=\"https://github.com/user-attachments/assets/776db189-af8c-4522-bb03-f8efbb7cdcd9\">\r\n\r\n\r\n###
Release note\r\nDedupe results from copy saved objects to spaces API
when object\r\ncontains references to other
objects.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"262b48f1cf4d4f624be99c2f42d169e4ab1f1f44","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Security","Feature:Saved
Objects","v9.0.0","backport:prev-minor","backport:prev-major","v8.17.0"],"title":"Fix
issue with duplicate references in error object when copying saved
objects to
space","number":200053,"url":"https://github.com/elastic/kibana/pull/200053","mergeCommit":{"message":"Fix
issue with duplicate references in error object when copying saved
objects to space (#200053)\n\nCloses
https://github.com/elastic/kibana/issues/158027\r\n\r\n##
Summary\r\n\r\nSimply dedupes references to objects if they are part of
the\r\nmissing_references in the copy saved objects to SO
endpoint\r\n\r\n### Notes\r\n- Update forEach over SOs to a regular for
loop since we had a couple of\r\nearly exit scenarios\r\n- Checks
against the set for references already added to the missing\r\nlist and
adds only if not present\r\n\r\n------\r\n\r\n**Old response: Note the
duplicate references**\r\n\r\n<img width=\"400\" alt=\"Screenshot
2024-11-14 at 01 52
54\"\r\nsrc=\"https://github.com/user-attachments/assets/67078080-e39d-43b2-bf7c-7abb76866fa4\">\r\n\r\n\r\n**New
response**\r\n\r\n<img width=\"800\" alt=\"Screenshot 2024-11-14 at 01
50
41\"\r\nsrc=\"https://github.com/user-attachments/assets/776db189-af8c-4522-bb03-f8efbb7cdcd9\">\r\n\r\n\r\n###
Release note\r\nDedupe results from copy saved objects to spaces API
when object\r\ncontains references to other
objects.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"262b48f1cf4d4f624be99c2f42d169e4ab1f1f44"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200053","number":200053,"mergeCommit":{"message":"Fix
issue with duplicate references in error object when copying saved
objects to space (#200053)\n\nCloses
https://github.com/elastic/kibana/issues/158027\r\n\r\n##
Summary\r\n\r\nSimply dedupes references to objects if they are part of
the\r\nmissing_references in the copy saved objects to SO
endpoint\r\n\r\n### Notes\r\n- Update forEach over SOs to a regular for
loop since we had a couple of\r\nearly exit scenarios\r\n- Checks
against the set for references already added to the missing\r\nlist and
adds only if not present\r\n\r\n------\r\n\r\n**Old response: Note the
duplicate references**\r\n\r\n<img width=\"400\" alt=\"Screenshot
2024-11-14 at 01 52
54\"\r\nsrc=\"https://github.com/user-attachments/assets/67078080-e39d-43b2-bf7c-7abb76866fa4\">\r\n\r\n\r\n**New
response**\r\n\r\n<img width=\"800\" alt=\"Screenshot 2024-11-14 at 01
50
41\"\r\nsrc=\"https://github.com/user-attachments/assets/776db189-af8c-4522-bb03-f8efbb7cdcd9\">\r\n\r\n\r\n###
Release note\r\nDedupe results from copy saved objects to spaces API
when object\r\ncontains references to other
objects.\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"262b48f1cf4d4f624be99c2f42d169e4ab1f1f44"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Sid <[email protected]>
  • Loading branch information
kibanamachine and SiddharthMantri authored Nov 18, 2024
1 parent dd67c8c commit 85c830d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -267,4 +267,36 @@ describe('validateReferences()', () => {
'Error fetching references for imported objects'
);
});

// test that when references are missing returns only deduplicated errors
test('returns only deduplicated errors when references are missing', async () => {
const params = setup({
objects: [
{
id: '2',
type: 'visualization',
attributes: { title: 'My Visualization 2' },
references: [
{ name: 'ref_0', type: 'index-pattern', id: '3' },
{ name: 'ref_0', type: 'index-pattern', id: '3' },
],
},
],
});
params.savedObjectsClient.bulkGet.mockResolvedValue({
saved_objects: [createNotFoundError({ type: 'index-pattern', id: '3' })],
});

const result = await validateReferences(params);
expect(result).toEqual([
expect.objectContaining({
type: 'visualization',
id: '2',
error: {
type: 'missing_references',
references: [{ type: 'index-pattern', id: '3' }],
},
}),
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -102,30 +102,35 @@ export async function validateReferences(params: ValidateReferencesParams) {
const nonExistingReferenceKeys = await getNonExistingReferenceAsKeys(params);

// Filter out objects with missing references, add to error object
objects.forEach(({ type, id, references, attributes }) => {
if (objectsToSkip.has(`${type}:${id}`)) {
for (const obj of objects) {
const { type, id, references, attributes } = obj;
const objectKey = `${type}:${id}`;
if (objectsToSkip.has(objectKey)) {
// skip objects with retries that have specified `ignoreMissingReferences`
return;
continue;
}

const missingReferences = [];
const enforcedTypeReferences = (references || []).filter(filterReferencesToValidate);
const missingReferences: Array<{ type: string; id: string }> = [];
const enforcedTypeReferences = references?.filter(filterReferencesToValidate) || [];

const seenReferences = new Set();
for (const { type: refType, id: refId } of enforcedTypeReferences) {
if (nonExistingReferenceKeys.includes(`${refType}:${refId}`)) {
const refKey = `${refType}:${refId}`;

if (nonExistingReferenceKeys.includes(refKey) && !seenReferences.has(refKey)) {
missingReferences.push({ type: refType, id: refId });
seenReferences.add(refKey);
}
}
if (missingReferences.length === 0) {
return;
if (missingReferences.length > 0) {
errorMap[objectKey] = {
id,
type,
meta: { title: attributes.title },
error: { type: 'missing_references', references: missingReferences },
};
}
const { title } = attributes;
errorMap[`${type}:${id}`] = {
id,
type,
meta: { title },
error: { type: 'missing_references', references: missingReferences },
};
});
}

return Object.values(errorMap);
}

0 comments on commit 85c830d

Please sign in to comment.