Skip to content

Commit

Permalink
bulkGet saved objects across spaces (elastic#109967)
Browse files Browse the repository at this point in the history
  • Loading branch information
jportner authored and kibanamachine committed Aug 26, 2021
1 parent 047fe77 commit 1f1d829
Show file tree
Hide file tree
Showing 19 changed files with 606 additions and 138 deletions.
10 changes: 10 additions & 0 deletions docs/api/saved-objects/bulk_get.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ experimental[] Retrieve multiple {kib} saved objects by ID.
`fields`::
(Optional, array) The fields to return in the `attributes` key of the object response.

`namespaces`::
(Optional, string array) Identifiers for the <<xpack-spaces,spaces>> in which to search for this object. If this is provided, the object
is searched for only in the explicitly defined spaces. If this is not provided, the object is searched for in the current space (default
behavior).
* 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.

[[saved-objects-api-bulk-get-response-body]]
==== Response body

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ export interface SavedObjectsBulkGetObject
| --- | --- | --- |
| [fields](./kibana-plugin-core-server.savedobjectsbulkgetobject.fields.md) | <code>string[]</code> | SavedObject fields to include in the response |
| [id](./kibana-plugin-core-server.savedobjectsbulkgetobject.id.md) | <code>string</code> | |
| [namespaces](./kibana-plugin-core-server.savedobjectsbulkgetobject.namespaces.md) | <code>string[]</code> | 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 <code>namespaceType: 'multiple'</code>): this option can be used to specify one or more spaces, including the "All spaces" identifier (<code>'*'</code>). \* For isolated object types (registered with <code>namespaceType: 'single'</code> or <code>namespaceType: 'multiple-isolated'</code>): this option can only be used to specify a single space, and the "All spaces" identifier (<code>'*'</code>) is not allowed. \* For global object types (registered with <code>namespaceType: 'agnostic'</code>): this option cannot be used. |
| [type](./kibana-plugin-core-server.savedobjectsbulkgetobject.type.md) | <code>string</code> | |

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) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectsBulkGetObject](./kibana-plugin-core-server.savedobjectsbulkgetobject.md) &gt; [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[];
```
1 change: 1 addition & 0 deletions src/core/server/saved_objects/routes/bulk_get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const registerBulkGetRoute = (router: IRouter, { coreUsageData }: RouteDe
type: schema.string(),
id: schema.string(),
fields: schema.maybe(schema.arrayOf(schema.string())),
namespaces: schema.maybe(schema.arrayOf(schema.string())),
})
),
},
Expand Down
85 changes: 85 additions & 0 deletions src/core/server/saved_objects/service/lib/internal_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
getBulkOperationError,
getSavedObjectFromSource,
rawDocExistsInNamespace,
rawDocExistsInNamespaces,
} from './internal_utils';
import { ALL_NAMESPACES_STRING } from './utils';

Expand Down Expand Up @@ -241,3 +242,87 @@ describe('#rawDocExistsInNamespace', () => {
});
});
});

describe('#rawDocExistsInNamespaces', () => {
const SINGLE_NAMESPACE_TYPE = 'single-type';
const MULTI_NAMESPACE_TYPE = 'multi-type';
const NAMESPACE_AGNOSTIC_TYPE = 'agnostic-type';

const registry = typeRegistryMock.create();
registry.isSingleNamespace.mockImplementation((type) => type === SINGLE_NAMESPACE_TYPE);
registry.isMultiNamespace.mockImplementation((type) => type === MULTI_NAMESPACE_TYPE);
registry.isNamespaceAgnostic.mockImplementation((type) => type === NAMESPACE_AGNOSTIC_TYPE);

function createRawDoc(
type: string,
namespaceAttrs: { namespace?: string; namespaces?: string[] }
) {
return {
// other fields exist on the raw document, but they are not relevant to these test cases
_source: {
type,
...namespaceAttrs,
},
} as SavedObjectsRawDoc;
}

describe('single-namespace type', () => {
it('returns true regardless of namespace or namespaces fields', () => {
// Technically, a single-namespace type does not exist in a space unless it has a namespace prefix in its raw ID and a matching
// 'namespace' field. However, historically we have not enforced the latter, we have just relied on searching for and deserializing
// documents with the correct namespace prefix. We may revisit this in the future.
const doc1 = createRawDoc(SINGLE_NAMESPACE_TYPE, { namespace: 'some-space' }); // the namespace field is ignored
const doc2 = createRawDoc(SINGLE_NAMESPACE_TYPE, { namespaces: ['some-space'] }); // the namespaces field is ignored
expect(rawDocExistsInNamespaces(registry, doc1, [])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc1, ['some-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc1, ['other-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc2, [])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc2, ['some-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc2, ['other-space'])).toBe(true);
});
});

describe('multi-namespace type', () => {
const docInDefaultSpace = createRawDoc(MULTI_NAMESPACE_TYPE, { namespaces: ['default'] });
const docInSomeSpace = createRawDoc(MULTI_NAMESPACE_TYPE, { namespaces: ['some-space'] });
const docInAllSpaces = createRawDoc(MULTI_NAMESPACE_TYPE, {
namespaces: [ALL_NAMESPACES_STRING],
});
const docInNoSpace = createRawDoc(MULTI_NAMESPACE_TYPE, { namespaces: [] });

it('returns true when the document namespaces matches', () => {
expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, ['default'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, docInAllSpaces, ['default'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, docInSomeSpace, ['some-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, docInAllSpaces, ['some-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, ['*'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, docInSomeSpace, ['*'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, docInAllSpaces, ['*'])).toBe(true);
});

it('returns false when the document namespace does not match', () => {
expect(rawDocExistsInNamespaces(registry, docInSomeSpace, ['default'])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInNoSpace, ['default'])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, ['some-space'])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInNoSpace, ['some-space'])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInNoSpace, ['*'])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInDefaultSpace, [])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInSomeSpace, [])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInAllSpaces, [])).toBe(false);
expect(rawDocExistsInNamespaces(registry, docInNoSpace, [])).toBe(false);
});
});

describe('namespace-agnostic type', () => {
it('returns true regardless of namespace or namespaces fields', () => {
const doc1 = createRawDoc(NAMESPACE_AGNOSTIC_TYPE, { namespace: 'some-space' }); // the namespace field is ignored
const doc2 = createRawDoc(NAMESPACE_AGNOSTIC_TYPE, { namespaces: ['some-space'] }); // the namespaces field is ignored
expect(rawDocExistsInNamespaces(registry, doc1, [])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc1, ['some-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc1, ['other-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc2, [])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc2, ['some-space'])).toBe(true);
expect(rawDocExistsInNamespaces(registry, doc2, ['other-space'])).toBe(true);
});
});
});
38 changes: 38 additions & 0 deletions src/core/server/saved_objects/service/lib/internal_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,41 @@ 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);
const existingNamespaces = raw._source.namespaces ?? [];

if (namespacesToCheck.size === 0 || existingNamespaces.length === 0) {
return false;
}
if (namespacesToCheck.has(ALL_NAMESPACES_STRING)) {
return true;
}

return existingNamespaces.some((x) => x === ALL_NAMESPACES_STRING || namespacesToCheck.has(x));
}
93 changes: 67 additions & 26 deletions src/core/server/saved_objects/service/lib/repository.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ describe('SavedObjectsRepository', () => {

const bulkGet = async (objects, options) =>
savedObjectsRepository.bulkGet(
objects.map(({ type, id }) => ({ type, id })), // bulkGet only uses type and id
objects.map(({ type, id, namespaces }) => ({ type, id, namespaces })), // bulkGet only uses type, id, and optionally namespaces
options
);
const bulkGetSuccess = async (objects, options) => {
Expand Down Expand Up @@ -992,6 +992,13 @@ describe('SavedObjectsRepository', () => {
_expectClientCallArgs([obj1, obj2], { getId });
});

it(`prepends namespace to the id when providing namespaces for single-namespace type`, async () => {
const getId = (type, id) => `${namespace}:${type}:${id}`; // test that the raw document ID equals this (e.g., has a namespace prefix)
const objects = [obj1, obj2].map((obj) => ({ ...obj, namespaces: [namespace] }));
await bulkGetSuccess(objects, { namespace: 'some-other-ns' });
_expectClientCallArgs([obj1, obj2], { getId });
});

it(`doesn't prepend namespace to the id when providing no namespace for single-namespace type`, async () => {
const getId = (type, id) => `${type}:${id}`; // test that the raw document ID equals this (e.g., does not have a namespace prefix)
await bulkGetSuccess([obj1, obj2]);
Expand All @@ -1011,33 +1018,35 @@ describe('SavedObjectsRepository', () => {
_expectClientCallArgs(objects, { getId });

client.mget.mockClear();
objects = [obj1, obj2].map((obj) => ({ ...obj, type: MULTI_NAMESPACE_ISOLATED_TYPE }));
objects = [obj1, { ...obj2, namespaces: ['some-other-ns'] }].map((obj) => ({
...obj,
type: MULTI_NAMESPACE_ISOLATED_TYPE,
}));
await bulkGetSuccess(objects, { namespace });
_expectClientCallArgs(objects, { getId });
});
});

describe('errors', () => {
const bulkGetErrorInvalidType = async ([obj1, obj, obj2]) => {
const response = getMockMgetResponse([obj1, obj2]);
const bulkGetError = async (obj, isBulkError, expectedErrorResult) => {
let response;
if (isBulkError) {
// mock the bulk error for only the second object
mockGetBulkOperationError.mockReturnValueOnce(undefined);
mockGetBulkOperationError.mockReturnValueOnce(expectedErrorResult.error);
response = getMockMgetResponse([obj1, obj, obj2]);
} else {
response = getMockMgetResponse([obj1, obj2]);
}
client.mget.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(response)
);
const result = await bulkGet([obj1, obj, obj2]);
expect(client.mget).toHaveBeenCalled();
expect(result).toEqual({
saved_objects: [expectSuccess(obj1), expectErrorInvalidType(obj), expectSuccess(obj2)],
});
};

const bulkGetErrorNotFound = async ([obj1, obj, obj2], options, response) => {
client.mget.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(response)
);
const result = await bulkGet([obj1, obj, obj2], options);
const objects = [obj1, obj, obj2];
const result = await bulkGet(objects);
expect(client.mget).toHaveBeenCalled();
expect(result).toEqual({
saved_objects: [expectSuccess(obj1), expectErrorNotFound(obj), expectSuccess(obj2)],
saved_objects: [expectSuccess(obj1), expectedErrorResult, expectSuccess(obj2)],
});
};

Expand All @@ -1063,33 +1072,65 @@ describe('SavedObjectsRepository', () => {
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`returns error when namespaces is used with a space-agnostic object`, async () => {
const obj = { type: NAMESPACE_AGNOSTIC_TYPE, id: 'three', namespaces: [] };
await bulkGetError(
obj,
undefined,
expectErrorResult(
obj,
createBadRequestError('"namespaces" cannot be used on space-agnostic types')
)
);
});

it(`returns error when namespaces is used with a space-isolated object and does not specify a single space`, async () => {
const doTest = async (objType, namespaces) => {
const obj = { type: objType, id: 'three', namespaces };
await bulkGetError(
obj,
undefined,
expectErrorResult(
obj,
createBadRequestError(
'"namespaces" can only specify a single space when used with space-isolated types'
)
)
);
};
await doTest('dashboard', ['spacex', 'spacey']);
await doTest('dashboard', ['*']);
await doTest(MULTI_NAMESPACE_ISOLATED_TYPE, ['spacex', 'spacey']);
await doTest(MULTI_NAMESPACE_ISOLATED_TYPE, ['*']);
});

it(`returns error when type is invalid`, async () => {
const obj = { type: 'unknownType', id: 'three' };
await bulkGetErrorInvalidType([obj1, obj, obj2]);
await bulkGetError(obj, undefined, expectErrorInvalidType(obj));
});

it(`returns error when type is hidden`, async () => {
const obj = { type: HIDDEN_TYPE, id: 'three' };
await bulkGetErrorInvalidType([obj1, obj, obj2]);
await bulkGetError(obj, undefined, expectErrorInvalidType(obj));
});

it(`returns error when document is not found`, async () => {
const obj = { type: 'dashboard', id: 'three', found: false };
const response = getMockMgetResponse([obj1, obj, obj2]);
await bulkGetErrorNotFound([obj1, obj, obj2], undefined, response);
await bulkGetError(obj, true, expectErrorNotFound(obj));
});

it(`handles missing ids gracefully`, async () => {
const obj = { type: 'dashboard', id: undefined, found: false };
const response = getMockMgetResponse([obj1, obj, obj2]);
await bulkGetErrorNotFound([obj1, obj, obj2], undefined, response);
await bulkGetError(obj, true, expectErrorNotFound(obj));
});

it(`returns error when type is multi-namespace and the document exists, but not in this namespace`, async () => {
const obj = { type: MULTI_NAMESPACE_ISOLATED_TYPE, id: 'three' };
const response = getMockMgetResponse([obj1, obj, obj2]);
response.docs[1].namespaces = ['bar-namespace'];
await bulkGetErrorNotFound([obj1, obj, obj2], { namespace }, response);
const obj = {
type: MULTI_NAMESPACE_ISOLATED_TYPE,
id: 'three',
namespace: 'bar-namespace',
};
await bulkGetError(obj, true, expectErrorNotFound(obj));
});

it(`throws when ES mget action responds with a 404 and a missing Elasticsearch product header`, async () => {
Expand Down
Loading

0 comments on commit 1f1d829

Please sign in to comment.