Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the repository to search across all namespaces #82863

Merged
merged 2 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { esKuery } from '../../../es_query';
type KueryNode = any;

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

const registry = typeRegistryMock.create();
Expand Down Expand Up @@ -53,20 +53,26 @@ const ALL_TYPE_SUBSETS = ALL_TYPES.reduce(

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 @@ -318,7 +324,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({
mappings,
registry,
Expand All @@ -328,7 +334,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
Original file line number Diff line number Diff line change
Expand Up @@ -81,34 +81,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 @@ -215,20 +222,8 @@ export function getQueryParams({
}

// A de-duplicated set of namespaces makes for a more effecient query.
//
// Additonally, 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;
const uniqNamespaces = (namespacesToNormalize?: string[]) =>
namespacesToNormalize ? Array.from(new Set(namespacesToNormalize)) : undefined;

const bool: any = {
filter: [
Expand All @@ -239,10 +234,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
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