[7.x] Allow the repository to search across all namespaces (#82863) (#…

…82975)
elastic · Nov 9, 2020 · 56c6b79 · 56c6b79
1 parent 299b66d
commit 56c6b79
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 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
@@ -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
@@ -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' } }],
       },
     };
@@ -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
@@ -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',
@@ -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();