Skip to content

Commit

Permalink
[7.x] Allow the repository to search across all namespaces (#82863) (#…
Browse files Browse the repository at this point in the history 
…82975)
  • Loading branch information
@legrego
legrego authored Nov 9, 2020
1 parent 299b66d commit 56c6b79
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 38 deletions.
21 changes: 13 additions & 8 deletions 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,8 +21,8 @@
import { esKuery } from '../../../es_query';
type KueryNode = any;

import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';
import { SavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
import { ALL_NAMESPACES_STRING } from '../utils';
import { getQueryParams, getClauseForReference } from './query_params';

const registerTypes = (registry: SavedObjectTypeRegistry) => {
Expand Down Expand Up @@ -101,20 +101,25 @@ describe('#getQueryParams', () => {

const createTypeClause = (type: string, namespaces?: string[]) => {
if (registry.isMultiNamespace(type)) {
const array = [...(namespaces ?? ['default']), ALL_NAMESPACES_STRING];
const array = [...(namespaces ?? [DEFAULT_NAMESPACE_STRING]), ALL_NAMESPACES_STRING];

const namespacesClause = { terms: { namespaces: array } };
return {
bool: {
must: expect.arrayContaining([{ terms: { namespaces: array } }]),
must: namespaces?.includes(ALL_NAMESPACES_STRING)
? expect.not.arrayContaining([namespacesClause])
: expect.arrayContaining([namespacesClause]),
must_not: [{ exists: { field: 'namespace' } }],
},
};
} else if (registry.isSingleNamespace(type)) {
const nonDefaultNamespaces = namespaces?.filter((n) => n !== 'default') ?? [];
const nonDefaultNamespaces = namespaces?.filter((n) => n !== DEFAULT_NAMESPACE_STRING) ?? [];
const searchingAcrossAllNamespaces = namespaces?.includes(ALL_NAMESPACES_STRING) ?? false;
const should: any = [];
if (nonDefaultNamespaces.length > 0) {
if (nonDefaultNamespaces.length > 0 && !searchingAcrossAllNamespaces) {
should.push({ terms: { namespace: nonDefaultNamespaces } });
}
if (namespaces?.includes('default')) {
if (namespaces?.includes(DEFAULT_NAMESPACE_STRING)) {
should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } });
}
return {
Expand Down Expand Up @@ -352,7 +357,7 @@ describe('#getQueryParams', () => {
expectResult(result, ...ALL_TYPES.map((x) => createTypeClause(x, namespaces)));
};

it('normalizes and deduplicates provided namespaces', () => {
it('deduplicates provided namespaces', () => {
const result = getQueryParams({
registry,
search: '*',
Expand All @@ -361,7 +366,7 @@ describe('#getQueryParams', () => {

expectResult(
result,
...ALL_TYPES.map((x) => createTypeClause(x, ['foo', 'default', 'bar']))
...ALL_TYPES.map((x) => createTypeClause(x, ['foo', '*', 'bar', 'default']))
);
});

Expand Down
51 changes: 23 additions & 28 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 @@ -75,34 +75,41 @@ function getClauseForType(
if (namespaces.length === 0) {
throw new Error('cannot specify empty namespaces array');
}
const searchAcrossAllNamespaces = namespaces.includes(ALL_NAMESPACES_STRING);

if (registry.isMultiNamespace(type)) {
const namespacesFilterClause = searchAcrossAllNamespaces
? {}
: { terms: { namespaces: [...namespaces, ALL_NAMESPACES_STRING] } };

return {
bool: {
must: [
{ term: { type } },
{ terms: { namespaces: [...namespaces, ALL_NAMESPACES_STRING] } },
],
must: [{ term: { type } }, namespacesFilterClause],
must_not: [{ exists: { field: 'namespace' } }],
},
};
} else if (registry.isSingleNamespace(type)) {
const should: Array<Record<string, any>> = [];
const eligibleNamespaces = namespaces.filter((x) => x !== DEFAULT_NAMESPACE_STRING);
if (eligibleNamespaces.length > 0) {
if (eligibleNamespaces.length > 0 && !searchAcrossAllNamespaces) {
should.push({ terms: { namespace: eligibleNamespaces } });
}
if (namespaces.includes(DEFAULT_NAMESPACE_STRING)) {
should.push({ bool: { must_not: [{ exists: { field: 'namespace' } }] } });
}
if (should.length === 0) {
// This is indicitive of a bug, and not user error.
throw new Error('unhandled search condition: expected at least 1 `should` clause.');
}

const shouldClauseProps =
should.length > 0
? {
should,
minimum_should_match: 1,
}
: {};

return {
bool: {
must: [{ term: { type } }],
should,
minimum_should_match: 1,
...shouldClauseProps,
must_not: [{ exists: { field: 'namespaces' } }],
},
};
Expand Down Expand Up @@ -181,21 +188,9 @@ export function getClauseForReference(reference: HasReferenceQueryParams) {
};
}

// A de-duplicated set of namespaces makes for a more efficient query.
//
// Additionally, we treat the `*` namespace as the `default` namespace.
// In the Default Distribution, the `*` is automatically expanded to include all available namespaces.
// However, the OSS distribution (and certain configurations of the Default Distribution) can allow the `*`
// to pass through to the SO Repository, and eventually to this module. When this happens, we translate to `default`,
// since that is consistent with how a single-namespace search behaves in the OSS distribution. Leaving the wildcard in place
// would result in no results being returned, as the wildcard is treated as a literal, and not _actually_ as a wildcard.
// We had a good discussion around the tradeoffs here: https://github.com/elastic/kibana/pull/67644#discussion_r441055716
const normalizeNamespaces = (namespacesToNormalize?: string[]) =>
namespacesToNormalize
? Array.from(
new Set(namespacesToNormalize.map((x) => (x === '*' ? DEFAULT_NAMESPACE_STRING : x)))
)
: undefined;
// A de-duplicated set of namespaces makes for a more effecient query.
const uniqNamespaces = (namespacesToNormalize?: string[]) =>
namespacesToNormalize ? Array.from(new Set(namespacesToNormalize)) : undefined;

/**
* Get the "query" related keys for the search body
Expand Down Expand Up @@ -229,10 +224,10 @@ export function getQueryParams({
{
bool: {
should: types.map((shouldType) => {
const normalizedNamespaces = normalizeNamespaces(
const deduplicatedNamespaces = uniqNamespaces(
typeToNamespacesMap ? typeToNamespacesMap.get(shouldType) : namespaces
);
return getClauseForType(registry, normalizedNamespaces, shouldType);
return getClauseForType(registry, deduplicatedNamespaces, shouldType);
}),
minimum_should_match: 1,
},
Expand Down
25 changes: 23 additions & 2 deletions test/api_integration/apis/saved_objects/find.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,15 @@ export default function ({ getService }) {
});

describe('wildcard namespace', () => {
it('should return 200 with individual responses from the default namespace', async () =>
it('should return 200 with individual responses from the all namespaces', async () =>
await supertest
.get('/api/saved_objects/_find?type=visualization&fields=title&namespaces=*')
.expect(200)
.then((resp) => {
expect(resp.body).to.eql({
page: 1,
per_page: 20,
total: 1,
total: 2,
saved_objects: [
{
type: 'visualization',
Expand All @@ -189,6 +189,27 @@ export default function ({ getService }) {
],
updated_at: '2017-09-21T18:51:23.794Z',
},
{
attributes: {
title: 'Count of requests',
},
id: 'dd7caf20-9efd-11e7-acb3-3dab96693fab',
migrationVersion: {
visualization: '7.10.0',
},
namespaces: ['foo-ns'],
references: [
{
id: '91200a00-9efd-11e7-acb3-3dab96693fab',
name: 'kibanaSavedObjectMeta.searchSourceJSON.index',
type: 'index-pattern',
},
],
score: 0,
type: 'visualization',
updated_at: '2017-09-21T18:51:23.794Z',
version: 'WzYsMV0=',
},
],
});
expect(resp.body.saved_objects[0].migrationVersion).to.be.ok();
Expand Down

0 comments on commit 56c6b79

Please sign in to comment.