[data] change KQL node builder to not generate recursive and/or clauses (#89345) (#89890)

resolves https://github.com/elastic/kibana/issues/88367

Prior to this PR, the KQL node_builder code was using recursion to generate
"and" & "or" expressions.  Eg, `and(foo1=bar1, foo2=bar2, foo3=bar3)` would
be generated as if was specified as `and(foo1=bar1, and(foo2=bar2, foo3=bar3))`.

Calls to the builder with long lists of expressions would generate nested JSON
as deep as the lists are long.  This is problematic, as Elasticsearch is
changing the default limit on nested bools to 20 levels, and alerting already
generates nested bools greater than that limit.
See: https://github.com/elastic/elasticsearch/issues/55303

This PR changes the generated shape of above, so that all the nodes are at the
same level, instead of the previous "recursive" treatment.
This commit is contained in:
Patrick Mueller 2021-02-01 14:31:50 -05:00 committed by GitHub
parent f845e19146
commit 7c9028ac08
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 1086 additions and 35 deletions

View file

@ -246,7 +246,7 @@ exports.Cluster = class Cluster {
this._log.info(chalk.bold('Starting'));
this._log.indent(4);
const esArgs = ['indices.query.bool.max_nested_depth=100'].concat(options.esArgs || []);
const esArgs = options.esArgs || [];
// Add to esArgs if ssl is enabled
if (this._ssl) {

View file

@ -264,9 +264,7 @@ describe('#start(installPath)', () => {
expect(extractConfigFiles.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Array [
"indices.query.bool.max_nested_depth=100",
],
Array [],
undefined,
Object {
"log": <ToolingLog>,
@ -342,9 +340,7 @@ describe('#run()', () => {
expect(extractConfigFiles.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Array [
"indices.query.bool.max_nested_depth=100",
],
Array [],
undefined,
Object {
"log": <ToolingLog>,

View file

@ -0,0 +1,280 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* and the Server Side Public License, v 1; you may not use this file except in
* compliance with, at your election, the Elastic License or the Server Side
* Public License, v 1.
*/
import { nodeBuilder } from './node_builder';
import { toElasticsearchQuery } from '../index';
describe('nodeBuilder', () => {
describe('is method', () => {
test('string value', () => {
const nodes = nodeBuilder.is('foo', 'bar');
const query = toElasticsearchQuery(nodes);
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo": "bar",
},
},
],
},
}
`);
});
test('KueryNode value', () => {
const literalValue = {
type: 'literal' as 'literal',
value: 'bar',
};
const nodes = nodeBuilder.is('foo', literalValue);
const query = toElasticsearchQuery(nodes);
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo": "bar",
},
},
],
},
}
`);
});
});
describe('and method', () => {
test('single clause', () => {
const nodes = [nodeBuilder.is('foo', 'bar')];
const query = toElasticsearchQuery(nodeBuilder.and(nodes));
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo": "bar",
},
},
],
},
}
`);
});
test('two clauses', () => {
const nodes = [nodeBuilder.is('foo1', 'bar1'), nodeBuilder.is('foo2', 'bar2')];
const query = toElasticsearchQuery(nodeBuilder.and(nodes));
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"filter": Array [
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo1": "bar1",
},
},
],
},
},
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo2": "bar2",
},
},
],
},
},
],
},
}
`);
});
test('three clauses', () => {
const nodes = [
nodeBuilder.is('foo1', 'bar1'),
nodeBuilder.is('foo2', 'bar2'),
nodeBuilder.is('foo3', 'bar3'),
];
const query = toElasticsearchQuery(nodeBuilder.and(nodes));
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"filter": Array [
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo1": "bar1",
},
},
],
},
},
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo2": "bar2",
},
},
],
},
},
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo3": "bar3",
},
},
],
},
},
],
},
}
`);
});
});
describe('or method', () => {
test('single clause', () => {
const nodes = [nodeBuilder.is('foo', 'bar')];
const query = toElasticsearchQuery(nodeBuilder.or(nodes));
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo": "bar",
},
},
],
},
}
`);
});
test('two clauses', () => {
const nodes = [nodeBuilder.is('foo1', 'bar1'), nodeBuilder.is('foo2', 'bar2')];
const query = toElasticsearchQuery(nodeBuilder.or(nodes));
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo1": "bar1",
},
},
],
},
},
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo2": "bar2",
},
},
],
},
},
],
},
}
`);
});
test('three clauses', () => {
const nodes = [
nodeBuilder.is('foo1', 'bar1'),
nodeBuilder.is('foo2', 'bar2'),
nodeBuilder.is('foo3', 'bar3'),
];
const query = toElasticsearchQuery(nodeBuilder.or(nodes));
expect(query).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo1": "bar1",
},
},
],
},
},
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo2": "bar2",
},
},
],
},
},
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"foo3": "bar3",
},
},
],
},
},
],
},
}
`);
});
});
});

View file

@ -16,12 +16,10 @@ export const nodeBuilder = {
nodeTypes.literal.buildNode(false),
]);
},
or: ([first, ...args]: KueryNode[]): KueryNode => {
return args.length ? nodeTypes.function.buildNode('or', [first, nodeBuilder.or(args)]) : first;
or: (nodes: KueryNode[]): KueryNode => {
return nodes.length > 1 ? nodeTypes.function.buildNode('or', nodes) : nodes[0];
},
and: ([first, ...args]: KueryNode[]): KueryNode => {
return args.length
? nodeTypes.function.buildNode('and', [first, nodeBuilder.and(args)])
: first;
and: (nodes: KueryNode[]): KueryNode => {
return nodes.length > 1 ? nodeTypes.function.buildNode('and', nodes) : nodes[0];
},
};

View file

@ -0,0 +1,316 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`AlertsAuthorization getFindAuthorizationFilter creates a filter based on the privileged types 1`] = `
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "myAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myAppWithSubFeature",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "myOtherAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myAppWithSubFeature",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "mySecondAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myAppWithSubFeature",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
},
],
"function": "or",
"type": "function",
}
`;

View file

@ -0,0 +1,448 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`asFiltersByAlertTypeAndConsumer constructs filter for multiple alert types across authorized consumer 1`] = `
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "myAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myAppWithSubFeature",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "myOtherAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myAppWithSubFeature",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "mySecondAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myAppWithSubFeature",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
},
],
"function": "or",
"type": "function",
}
`;
exports[`asFiltersByAlertTypeAndConsumer constructs filter for single alert type with multiple authorized consumer 1`] = `
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "myAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "alerts",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myOtherApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
},
],
"function": "and",
"type": "function",
}
`;
exports[`asFiltersByAlertTypeAndConsumer constructs filter for single alert type with single authorized consumer 1`] = `
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.alertTypeId",
},
Object {
"type": "literal",
"value": "myAppAlertType",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"type": "literal",
"value": "alert.attributes.consumer",
},
Object {
"type": "literal",
"value": "myApp",
},
Object {
"type": "literal",
"value": false,
},
],
"function": "is",
"type": "function",
},
],
"function": "and",
"type": "function",
}
`;

View file

@ -6,7 +6,6 @@
import { KibanaRequest } from 'kibana/server';
import { alertTypeRegistryMock } from '../alert_type_registry.mock';
import { securityMock } from '../../../../plugins/security/server/mocks';
import { esKuery } from '../../../../../src/plugins/data/server';
import {
PluginStartContract as FeaturesStartContract,
KibanaFeature,
@ -627,11 +626,17 @@ describe('AlertsAuthorization', () => {
});
alertTypeRegistry.list.mockReturnValue(setOfAlertTypes);
expect((await alertAuthorization.getFindAuthorizationFilter()).filter).toEqual(
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
)
);
// TODO: once issue https://github.com/elastic/kibana/issues/89473 is
// resolved, we can start using this code again, instead of toMatchSnapshot():
//
// expect((await alertAuthorization.getFindAuthorizationFilter()).filter).toEqual(
// esKuery.fromKueryExpression(
// `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
// )
// );
// This code is the replacement code for above
expect((await alertAuthorization.getFindAuthorizationFilter()).filter).toMatchSnapshot();
expect(auditLogger.alertsAuthorizationSuccess).not.toHaveBeenCalled();
});

View file

@ -3,7 +3,6 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { esKuery } from '../../../../../src/plugins/data/server';
import { RecoveredActionGroup } from '../../common';
import {
asFiltersByAlertTypeAndConsumer,
@ -30,11 +29,14 @@ describe('asFiltersByAlertTypeAndConsumer', () => {
},
])
)
).toEqual(
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp)))`
)
);
).toMatchSnapshot();
// TODO: once issue https://github.com/elastic/kibana/issues/89473 is
// resolved, we can start using this code again instead of toMatchSnapshot()
// ).toEqual(
// esKuery.fromKueryExpression(
// `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp)))`
// )
// );
});
test('constructs filter for single alert type with multiple authorized consumer', async () => {
@ -58,11 +60,14 @@ describe('asFiltersByAlertTypeAndConsumer', () => {
},
])
)
).toEqual(
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)))`
)
);
).toMatchSnapshot();
// TODO: once issue https://github.com/elastic/kibana/issues/89473 is
// resolved, we can start using this code again, instead of toMatchSnapshot():
// ).toEqual(
// esKuery.fromKueryExpression(
// `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)))`
// )
// );
});
test('constructs filter for multiple alert types across authorized consumer', async () => {
@ -119,11 +124,14 @@ describe('asFiltersByAlertTypeAndConsumer', () => {
},
])
)
).toEqual(
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
)
);
).toMatchSnapshot();
// TODO: once issue https://github.com/elastic/kibana/issues/89473 is
// resolved, we can start using this code again, instead of toMatchSnapshot():
// ).toEqual(
// esKuery.fromKueryExpression(
// `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
// )
// );
});
});