Skip to content

Commit

Permalink
Update saved objects repository to support '*' namespace string
Browse files Browse the repository at this point in the history 
This is treated as "all namespaces".
  • Loading branch information
@jportner
jportner committed Sep 11, 2020
1 parent 13c2c76 commit ebaa032
Show file tree
Hide file tree
Showing 51 changed files with 326 additions and 81 deletions.
88 changes: 86 additions & 2 deletions src/core/server/saved_objects/service/lib/repository.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { SavedObjectsRepository } from './repository';
import * as getSearchDslNS from './search_dsl/search_dsl';
import { SavedObjectsErrorHelpers } from './errors';
import { ALL_NAMESPACES_STRING } from './utils';
import { SavedObjectsSerializer } from '../../serialization';
import { encodeHitVersion } from '../../version';
import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry';
Expand Down Expand Up @@ -725,6 +726,12 @@ describe('SavedObjectsRepository', () => {
});
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.bulkCreate([obj3], { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`returns error when type is invalid`, async () => {
const obj = { ...obj3, type: 'unknownType' };
await bulkCreateError(obj, undefined, expectErrorInvalidType(obj));
Expand Down Expand Up @@ -1042,6 +1049,13 @@ describe('SavedObjectsRepository', () => {
});
};

it(`throws when options.namespace is '*'`, async () => {
const obj = { type: 'dashboard', id: 'three' };
await expect(
savedObjectsRepository.bulkGet([obj], { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`returns error when type is invalid`, async () => {
const obj = { type: 'unknownType', id: 'three' };
await bulkGetErrorInvalidType([obj1, obj, obj2]);
Expand Down Expand Up @@ -1467,6 +1481,12 @@ describe('SavedObjectsRepository', () => {
});
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.bulkUpdate([obj], { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`returns error when type is invalid`, async () => {
const _obj = { ...obj, type: 'unknownType' };
await bulkUpdateError(_obj, undefined, expectErrorNotFound(_obj));
Expand All @@ -1477,6 +1497,15 @@ describe('SavedObjectsRepository', () => {
await bulkUpdateError(_obj, undefined, expectErrorNotFound(_obj));
});

it(`returns error when object namespace is '*'`, async () => {
const _obj = { ...obj, namespace: '*' };
await bulkUpdateError(
_obj,
undefined,
expectErrorResult(obj, createBadRequestError('"namespace" cannot be "*"'))
);
});

it(`returns error when ES is unable to find the document (mget)`, async () => {
const _obj = { ...obj, type: MULTI_NAMESPACE_TYPE, found: false };
const mgetResponse = getMockMgetResponse([_obj]);
Expand Down Expand Up @@ -1630,7 +1659,7 @@ describe('SavedObjectsRepository', () => {
);
};

describe('cluster calls', () => {
describe('client calls', () => {
it(`doesn't make a cluster call if the objects array is empty`, async () => {
await checkConflicts([]);
expect(client.mget).not.toHaveBeenCalled();
Expand Down Expand Up @@ -1662,6 +1691,14 @@ describe('SavedObjectsRepository', () => {
});
});

describe('errors', () => {
it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.checkConflicts([obj1], { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});
});

describe('returns', () => {
it(`expected results`, async () => {
const unknownTypeObj = { type: 'unknownType', id: 'three' };
Expand Down Expand Up @@ -1909,6 +1946,12 @@ describe('SavedObjectsRepository', () => {
});

describe('errors', () => {
it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.create(type, attributes, { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`throws when type is invalid`, async () => {
await expect(savedObjectsRepository.create('unknownType', attributes)).rejects.toThrowError(
createUnsupportedTypeError('unknownType')
Expand Down Expand Up @@ -2120,6 +2163,12 @@ describe('SavedObjectsRepository', () => {
);
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.delete(type, id, { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`throws when type is invalid`, async () => {
await expectNotFoundError('unknownType', id);
expect(client.delete).not.toHaveBeenCalled();
Expand Down Expand Up @@ -2169,6 +2218,20 @@ describe('SavedObjectsRepository', () => {
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when the type is multi-namespace and the document has all namespaces and the force option is not enabled`, async () => {
const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace });
response._source.namespaces = ['*'];
client.get.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(response)
);
await expect(
savedObjectsRepository.delete(MULTI_NAMESPACE_TYPE, id, { namespace })
).rejects.toThrowError(
'Unable to delete saved object that exists in multiple namespaces, use the `force` option to delete it anyway'
);
expect(client.get).toHaveBeenCalledTimes(1);
});

it(`throws when ES is unable to find the document during delete`, async () => {
client.delete.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' })
Expand Down Expand Up @@ -2252,7 +2315,7 @@ describe('SavedObjectsRepository', () => {
});

describe('errors', () => {
it(`throws when namespace is not a string`, async () => {
it(`throws when namespace is not a string or is '*'`, async () => {
const test = async (namespace) => {
await expect(savedObjectsRepository.deleteByNamespace(namespace)).rejects.toThrowError(
`namespace is required, and must be a string`
Expand All @@ -2263,6 +2326,7 @@ describe('SavedObjectsRepository', () => {
await test(['namespace']);
await test(123);
await test(true);
await test(ALL_NAMESPACES_STRING);
});
});

Expand Down Expand Up @@ -2802,6 +2866,12 @@ describe('SavedObjectsRepository', () => {
);
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.get(type, id, { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`throws when type is invalid`, async () => {
await expectNotFoundError('unknownType', id);
expect(client.get).not.toHaveBeenCalled();
Expand Down Expand Up @@ -2993,6 +3063,14 @@ describe('SavedObjectsRepository', () => {
);
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.incrementCounter(type, id, field, {
namespace: ALL_NAMESPACES_STRING,
})
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`throws when type is not a string`, async () => {
const test = async (type) => {
await expect(
Expand Down Expand Up @@ -3649,6 +3727,12 @@ describe('SavedObjectsRepository', () => {
);
};

it(`throws when options.namespace is '*'`, async () => {
await expect(
savedObjectsRepository.update(type, id, attributes, { namespace: ALL_NAMESPACES_STRING })
).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"'));
});

it(`throws when type is invalid`, async () => {
await expectNotFoundError('unknownType', id);
expect(client.update).not.toHaveBeenCalled();
Expand Down
40 changes: 33 additions & 7 deletions src/core/server/saved_objects/service/lib/repository.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ import {
} from '../../types';
import { SavedObjectTypeRegistry } from '../../saved_objects_type_registry';
import { validateConvertFilterToKueryNode } from './filter_utils';
import { SavedObjectsUtils } from './utils';
import { ALL_NAMESPACES_STRING, SavedObjectsUtils } from './utils';

// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
Expand Down Expand Up @@ -562,7 +562,10 @@ export class SavedObjectsRepository {
if (this._registry.isMultiNamespace(type)) {
preflightResult = await this.preflightCheckIncludesNamespace(type, id, namespace);
const existingNamespaces = getSavedObjectNamespaces(undefined, preflightResult) ?? [];
if (!force && existingNamespaces.length > 1) {
if (
!force &&
(existingNamespaces.length > 1 || existingNamespaces.includes(ALL_NAMESPACES_STRING))
) {
throw SavedObjectsErrorHelpers.createBadRequestError(
'Unable to delete saved object that exists in multiple namespaces, use the `force` option to delete it anyway'
);
Expand Down Expand Up @@ -610,8 +613,8 @@ export class SavedObjectsRepository {
namespace: string,
options: SavedObjectsDeleteByNamespaceOptions = {}
): Promise<any> {
if (!namespace || typeof namespace !== 'string') {
throw new TypeError(`namespace is required, and must be a string`);
if (!namespace || typeof namespace !== 'string' || namespace === '*') {
throw new TypeError(`namespace is required, and must be a string that is not equal to '*'`);
}

const allTypes = Object.keys(getRootPropertiesObjects(this._mappings));
Expand Down Expand Up @@ -1211,6 +1214,19 @@ export class SavedObjectsRepository {
}

const { attributes, references, version, namespace: objectNamespace } = object;

if (objectNamespace === ALL_NAMESPACES_STRING) {
return {
tag: 'Left' as 'Left',
error: {
id,
type,
error: errorContent(
SavedObjectsErrorHelpers.createBadRequestError('"namespace" cannot be "*"')
),
},
};
}
// `objectNamespace` is a namespace string, while `namespace` is a namespace ID.
// The object namespace string, if defined, will supersede the operation's namespace ID.

Expand Down Expand Up @@ -1526,7 +1542,10 @@ export class SavedObjectsRepository {
}

const namespaces = raw._source.namespaces;
return namespaces?.includes(SavedObjectsUtils.namespaceIdToString(namespace)) ?? false;
const existsInNamespace =
namespaces?.includes(SavedObjectsUtils.namespaceIdToString(namespace)) ||
namespaces?.includes('*');
return existsInNamespace ?? false;
}

/**
Expand Down Expand Up @@ -1653,8 +1672,15 @@ function getSavedObjectNamespaces(
* Ensure that a namespace is always in its namespace ID representation.
* This allows `'default'` to be used interchangeably with `undefined`.
*/
const normalizeNamespace = (namespace?: string) =>
namespace === undefined ? namespace : SavedObjectsUtils.namespaceStringToId(namespace);
const normalizeNamespace = (namespace?: string) => {
if (namespace === ALL_NAMESPACES_STRING) {
throw SavedObjectsErrorHelpers.createBadRequestError('"options.namespace" cannot be "*"');
} else if (namespace === undefined) {
return namespace;
} else {
return SavedObjectsUtils.namespaceStringToId(namespace);
}
};

/**
* Extracts the contents of a decorated error to return the attributes for bulk operations.
Expand Down
4 changes: 3 additions & 1 deletion src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import { esKuery, KueryNode } from '../../../../../../plugins/data/server';

import { typeRegistryMock } from '../../../saved_objects_type_registry.mock';
import { ALL_NAMESPACES_STRING } from '../utils';
import { getQueryParams } from './query_params';

const registry = typeRegistryMock.create();
Expand Down Expand Up @@ -199,9 +200,10 @@ describe('#getQueryParams', () => {
describe('`namespaces` parameter', () => {
const createTypeClause = (type: string, namespaces?: string[]) => {
if (registry.isMultiNamespace(type)) {
const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING];
return {
bool: {
must: expect.arrayContaining([{ terms: { namespaces: namespaces ?? ['default'] } }]),
must: expect.arrayContaining([{ terms: { namespaces: array } }]),
must_not: [{ exists: { field: 'namespace' } }],
},
};
Expand Down
7 changes: 5 additions & 2 deletions src/core/server/saved_objects/service/lib/search_dsl/query_params.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { esKuery, KueryNode } from '../../../../../../plugins/data/server';

import { getRootPropertiesObjects, IndexMapping } from '../../../mappings';
import { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
import { DEFAULT_NAMESPACE_STRING } from '../utils';
import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';

/**
* Gets the types based on the type. Uses mappings to support
Expand Down Expand Up @@ -83,7 +83,10 @@ function getClauseForType(
if (registry.isMultiNamespace(type)) {
return {
bool: {
must: [{ term: { type } }, { terms: { namespaces } }],
must: [
{ term: { type } },
{ terms: { namespaces: [...namespaces, ALL_NAMESPACES_STRING] } },
],
must_not: [{ exists: { field: 'namespace' } }],
},
};
Expand Down
1 change: 1 addition & 0 deletions src/core/server/saved_objects/service/lib/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

export const DEFAULT_NAMESPACE_STRING = 'default';
export const ALL_NAMESPACES_STRING = '*';

/**
* @public
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/spaces/server/lib/utils/namespace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import { SavedObjectsUtils } from '../../../../../../src/core/server';

export const ALL_SPACES_STRING = '*';

/**
* Converts a Space ID string to its namespace ID representation. Note that a Space ID string is equivalent to a namespace string.
*
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/spaces/server/routes/api/external/share_add_spaces.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { wrapError } from '../../../lib/errors';
import { ExternalRouteDeps } from '.';
import { SPACE_ID_REGEX } from '../../../lib/space_schema';
import { createLicensedRouteHandler } from '../../lib';
import { ALL_SPACES_STRING } from '../../../lib/utils/namespace';

const uniq = <T>(arr: T[]): T[] => Array.from(new Set<T>(arr));
export function initShareAddSpacesApi(deps: ExternalRouteDeps) {
Expand All @@ -22,8 +23,8 @@ export function initShareAddSpacesApi(deps: ExternalRouteDeps) {
spaces: schema.arrayOf(
schema.string({
validate: (value) => {
if (!SPACE_ID_REGEX.test(value)) {
return `lower case, a-z, 0-9, "_", and "-" are allowed`;
if (value !== ALL_SPACES_STRING && !SPACE_ID_REGEX.test(value)) {
return `lower case, a-z, 0-9, "_", and "-" are allowed, OR "*"`;
}
},
}),
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/spaces/server/routes/api/external/share_remove_spaces.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { wrapError } from '../../../lib/errors';
import { ExternalRouteDeps } from '.';
import { SPACE_ID_REGEX } from '../../../lib/space_schema';
import { createLicensedRouteHandler } from '../../lib';
import { ALL_SPACES_STRING } from '../../../lib/utils/namespace';

const uniq = <T>(arr: T[]): T[] => Array.from(new Set<T>(arr));
export function initShareRemoveSpacesApi(deps: ExternalRouteDeps) {
Expand All @@ -22,8 +23,8 @@ export function initShareRemoveSpacesApi(deps: ExternalRouteDeps) {
spaces: schema.arrayOf(
schema.string({
validate: (value) => {
if (!SPACE_ID_REGEX.test(value)) {
return `lower case, a-z, 0-9, "_", and "-" are allowed`;
if (value !== ALL_SPACES_STRING && !SPACE_ID_REGEX.test(value)) {
return `lower case, a-z, 0-9, "_", and "-" are allowed, OR "*"`;
}
},
}),
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
ISavedObjectTypeRegistry,
} from 'src/core/server';
import { SpacesServiceSetup } from '../spaces_service/spaces_service';
import { spaceIdToNamespace } from '../lib/utils/namespace';
import { ALL_SPACES_STRING, spaceIdToNamespace } from '../lib/utils/namespace';
import { SpacesClient } from '../lib/spaces_client';

interface SpacesSavedObjectsClientOptions {
Expand Down Expand Up @@ -165,7 +165,7 @@ export class SpacesSavedObjectsClient implements SavedObjectsClientContract {
if (namespaces) {
const spacesClient = await this.getSpacesClient;
const availableSpaces = await spacesClient.getAll('findSavedObjects');
if (namespaces.includes('*')) {
if (namespaces.includes(ALL_SPACES_STRING)) {
namespaces = availableSpaces.map((space) => space.id);
} else {
namespaces = namespaces.filter((namespace) =>
Expand Down
Loading

0 comments on commit ebaa032

Please sign in to comment.