From 56c6b79550e6723a7d0067b4fc475617d8511a2c Mon Sep 17 00:00:00 2001 From: Larry Gregory Date: Mon, 9 Nov 2020 14:13:31 -0500 Subject: [PATCH] [7.x] Allow the repository to search across all namespaces (#82863) (#82975) --- .../lib/search_dsl/query_params.test.ts | 21 +++++--- .../service/lib/search_dsl/query_params.ts | 51 +++++++++---------- .../apis/saved_objects/find.js | 25 ++++++++- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts index a8c5df8d64630..c35ec809fcf8d 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts @@ -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) => { @@ -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 { @@ -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: '*', @@ -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'])) ); }); diff --git a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts index f73777c4f454f..2ecba42e408e7 100644 --- a/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts +++ b/src/core/server/saved_objects/service/lib/search_dsl/query_params.ts @@ -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> = []; 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' } }], }, }; @@ -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 @@ -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, }, diff --git a/test/api_integration/apis/saved_objects/find.js b/test/api_integration/apis/saved_objects/find.js index e5da46644672b..8e8730b1e574a 100644 --- a/test/api_integration/apis/saved_objects/find.js +++ b/test/api_integration/apis/saved_objects/find.js @@ -160,7 +160,7 @@ 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) @@ -168,7 +168,7 @@ export default function ({ getService }) { expect(resp.body).to.eql({ page: 1, per_page: 20, - total: 1, + total: 2, saved_objects: [ { type: 'visualization', @@ -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();