-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bulkGet saved objects across spaces #109967
Changes from 10 commits
0b30d72
e6588b7
9f6efb6
beedf6f
e90b900
5e5e00c
f224966
1215e2c
8d0bfa4
c738184
dcec828
fab5c21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [SavedObjectsBulkGetObject](./kibana-plugin-core-server.savedobjectsbulkgetobject.md) > [namespaces](./kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md) | ||
|
||
## SavedObjectsBulkGetObject.namespaces property | ||
|
||
Optional namespace(s) for the object to be retrieved in. If this is defined, it will supersede the namespace ID that is in the top-level options. | ||
|
||
\* For shareable object types (registered with `namespaceType: 'multiple'`<!-- -->): this option can be used to specify one or more spaces, including the "All spaces" identifier (`'*'`<!-- -->). \* For isolated object types (registered with `namespaceType: 'single'` or `namespaceType: 'multiple-isolated'`<!-- -->): this option can only be used to specify a single space, and the "All spaces" identifier (`'*'`<!-- -->) is not allowed. \* For global object types (registered with `namespaceType: 'agnostic'`<!-- -->): this option cannot be used. | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
namespaces?: string[]; | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,3 +141,40 @@ export function rawDocExistsInNamespace( | |
namespaces?.includes(ALL_NAMESPACES_STRING); | ||
return existsInNamespace ?? false; | ||
} | ||
|
||
/** | ||
* Check to ensure that a raw document exists in at least one of the given namespaces. If the document is not a multi-namespace type, then | ||
* this returns `true` as we rely on the guarantees of the document ID format. If the document is a multi-namespace type, this checks to | ||
* ensure that the document's `namespaces` value includes the string representation of at least one of the given namespaces. | ||
* | ||
* WARNING: This should only be used for documents that were retrieved from Elasticsearch. Otherwise, the guarantees of the document ID | ||
* format mentioned above do not apply. | ||
* | ||
* @param registry | ||
* @param raw | ||
* @param namespaces | ||
*/ | ||
export function rawDocExistsInNamespaces( | ||
registry: ISavedObjectTypeRegistry, | ||
raw: SavedObjectsRawDoc, | ||
namespaces: string[] | ||
) { | ||
const rawDocType = raw._source.type; | ||
|
||
// if the type is namespace isolated, or namespace agnostic, we can continue to rely on the guarantees | ||
// of the document ID format and don't need to check this | ||
if (!registry.isMultiNamespace(rawDocType)) { | ||
return true; | ||
} | ||
|
||
const namespacesToCheck = new Set(namespaces); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: I don't think there's any need to convert to a set here? Can't we just work with the initial array? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, had the same thought. The only corner case I can imagine where it'd be beneficial to have a Having a set here will not completely protect us from a malicious actor anyway, they can just do something like this...., for every ID: const namespaces = [...Array.from({ length: 1000000 }).map(() => Math.random().toString()), 'valid-ns'] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The purpose of the Set here is to reduce the time complexity, this takes us down from |
||
const existingNamespaces = raw._source.namespaces ?? []; | ||
|
||
if (namespacesToCheck.size === 0 || existingNamespaces.length === 0) { | ||
return false; | ||
} else if (namespacesToCheck.has(ALL_NAMESPACES_STRING)) { | ||
return true; | ||
} | ||
|
||
return existingNamespaces.some((x) => x === ALL_NAMESPACES_STRING || namespacesToCheck.has(x)); | ||
jportner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a remark: We already talked about it in a similar function from a previous PR, but I still don't like the way single-NS docs are short-circuiting these functions, as it could lead to errors if a developer decides to use it elsewhere than in a part of the code where we're already assured that the fetched single-NS objects are of the correct requested space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll one-up you, I don't like single-NS docs at all 😄