diff --git a/docs/changelog/133134.yaml b/docs/changelog/133134.yaml new file mode 100644 index 0000000000000..715072de85896 --- /dev/null +++ b/docs/changelog/133134.yaml @@ -0,0 +1,5 @@ +pr: 133134 +summary: Fix sequences with conditions involving keys and non-keys +area: EQL +type: bug +issues: [] diff --git a/x-pack/plugin/eql/qa/rest/build.gradle b/x-pack/plugin/eql/qa/rest/build.gradle index 0ffecefb934f7..c77d0d2e3862c 100644 --- a/x-pack/plugin/eql/qa/rest/build.gradle +++ b/x-pack/plugin/eql/qa/rest/build.gradle @@ -12,7 +12,7 @@ apply plugin: 'elasticsearch.internal-test-artifact' restResources { restApi { - include '_common', 'bulk', 'indices', 'eql' + include '_common', 'bulk', 'indices', 'eql', 'capabilities' } } diff --git a/x-pack/plugin/eql/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/eql/70_functions_on_keys.yml b/x-pack/plugin/eql/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/eql/70_functions_on_keys.yml new file mode 100644 index 0000000000000..784de265983c2 --- /dev/null +++ b/x-pack/plugin/eql/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/eql/70_functions_on_keys.yml @@ -0,0 +1,119 @@ +--- +setup: + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: POST + path: /{index}/_eql/search + parameters: [ ] + capabilities: [ filters_on_keys_fix ] + reason: "Testing a recent fix" + - do: + indices.create: + index: eql_test + body: + { + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "event": { + "properties": { + "type": { + "type": "keyword", + "ignore_above": 1024 + } + } + }, + "user": { + "properties": { + "domain": { + "type": "keyword", + "ignore_above": 1024 + }, + "name": { + "type": "keyword", + "fields": { + "text": { + "type": "match_only_text" + } + } + } + } + }, + "winlog": { + "dynamic": "true", + "properties": { + "computer_name": { + "type": "keyword", + "ignore_above": 1024 + } + } + }, + "source": { + "properties": { + "ip": { + "type": "ip" + } + } + } + } + } + } + - do: + bulk: + refresh: true + body: + - index: + _index: eql_test + _id: "1" + - "winlog": + "computer_name": "foo.bar.baz" + "@timestamp": "2025-06-18T12:21:37.018Z" + "event": + "category": "authentication" + "code": "5145" + "user": + "domain": "bar.baz" + "name": "something" + "source": + "ip": "192.168.56.200" + - index: + _index: eql_test + _id: "2" + - "winlog": + "computer_name": "foo.bar.baz" + "@timestamp": "2025-06-18T12:21:37.093Z" + "event": + "category": "authentication" + "user": + "domain": "BAR.BAZ" + "name": "foo$" + "source": + "ip": "192.168.56.200" + +--- +"Test one join key": + - do: + eql.search: + index: eql_test + body: + query: 'sequence by source.ip with maxspan=5s [any where event.code : "5145" and winlog.computer_name == "foo.bar.baz" ] [any where winlog.computer_name == "foo.bar.baz" and startswith(winlog.computer_name, substring(user.name, 0, -1)) ]' + + - match: {timed_out: false} + - match: {hits.total.value: 1} + - match: {hits.total.relation: "eq"} + +--- +"Test two join keys ": + - do: + eql.search: + index: eql_test + body: + query: 'sequence by source.ip, winlog.computer_name with maxspan=5s [any where event.code : "5145" and winlog.computer_name == "foo.bar.baz" ] [any where winlog.computer_name == "foo.bar.baz" and startswith(winlog.computer_name, substring(user.name, 0, -1)) ]' + + - match: {timed_out: false} + - match: {hits.total.value: 1} + - match: {hits.total.relation: "eq"} + diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/optimizer/Optimizer.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/optimizer/Optimizer.java index 17f095db5033e..8f0757814e74a 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/optimizer/Optimizer.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/optimizer/Optimizer.java @@ -23,6 +23,7 @@ import org.elasticsearch.xpack.eql.session.Payload.Type; import org.elasticsearch.xpack.eql.util.MathUtils; import org.elasticsearch.xpack.eql.util.StringUtils; +import org.elasticsearch.xpack.ql.expression.Attribute; import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.FieldAttribute; import org.elasticsearch.xpack.ql.expression.Literal; @@ -416,23 +417,29 @@ private static List detectKeyConstraints(Expression condition, Keyed List and = Predicates.splitAnd(condition); for (Expression exp : and) { - // if there are no conjunction and at least one key matches, save the expression along with the key - // and its ordinal so it can be replaced - if (exp.anyMatch(Or.class::isInstance) == false) { - // comparisons against variables are not done - // hence why on the first key match, the expression is picked up - exp.anyMatch(e -> { - for (int i = 0; i < keys.size(); i++) { - Expression key = keys.get(i); - if (e.semanticEquals(key)) { - constraints.add(new Constraint(exp, filter, i)); - return true; - } - } - return false; - }); + // if the expression only involves filter keys, it's simple enough (eg. there are no conjunction), and at least one key + // matches, save the expression along with the key and its ordinal so it can be replaced + if (exp.anyMatch(Or.class::isInstance)) { + continue; + } + + // expressions that involve attributes other than the keys have to be discarded + if (exp.anyMatch(x -> x instanceof Attribute && keys.stream().noneMatch(k -> x.semanticEquals(k)))) { + continue; } + + exp.anyMatch(e -> { + for (int i = 0; i < keys.size(); i++) { + Expression key = keys.get(i); + if (e.semanticEquals(key)) { + constraints.add(new Constraint(exp, filter, i)); + return true; + } + } + return false; + }); } + return constraints; } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlCapabilities.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlCapabilities.java new file mode 100644 index 0000000000000..a3af8a932fa66 --- /dev/null +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/EqlCapabilities.java @@ -0,0 +1,26 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.eql.plugin; + +import java.util.HashSet; +import java.util.Set; + +public final class EqlCapabilities { + + private EqlCapabilities() {} + + /** Fix bug on filters that include join keys https://github.com/elastic/elasticsearch/issues/133065 */ + private static final String FILTERS_ON_KEYS_FIX = "filters_on_keys_fix"; + + public static final Set CAPABILITIES; + static { + HashSet capabilities = new HashSet<>(); + capabilities.add(FILTERS_ON_KEYS_FIX); + CAPABILITIES = Set.copyOf(capabilities); + } +} diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/RestEqlSearchAction.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/RestEqlSearchAction.java index 65def24563e5e..651868eb04183 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/RestEqlSearchAction.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/RestEqlSearchAction.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import static org.elasticsearch.rest.RestRequest.Method.GET; import static org.elasticsearch.rest.RestRequest.Method.POST; @@ -116,4 +117,9 @@ public void onFailure(Exception e) { public String getName() { return "eql_search"; } + + @Override + public Set supportedCapabilities() { + return EqlCapabilities.CAPABILITIES; + } } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/optimizer/OptimizerTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/optimizer/OptimizerTests.java index 199117c3df43a..934f913e469d4 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/optimizer/OptimizerTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/optimizer/OptimizerTests.java @@ -12,6 +12,7 @@ import org.elasticsearch.xpack.eql.analysis.Analyzer; import org.elasticsearch.xpack.eql.analysis.PostAnalyzer; import org.elasticsearch.xpack.eql.analysis.PreAnalyzer; +import org.elasticsearch.xpack.eql.expression.function.scalar.string.StartsWith; import org.elasticsearch.xpack.eql.expression.function.scalar.string.ToString; import org.elasticsearch.xpack.eql.parser.EqlParser; import org.elasticsearch.xpack.eql.plan.logical.AbstractJoin; @@ -576,6 +577,113 @@ public void testQueryLevelTwoKeyConstraints() { assertEquals(ruleBCondition, filterCondition(query2.child().children().get(0))); } + /** + * sequence + * 1. filter startsWith(a, "foo") by a + * 2. filter X by a + * == + * 1. filter startsWith(a, "foo") by a + * 2. filter startsWith(a, "foo") by a + * \filter X + */ + public void testKeyConstraintWithFunction() { + Attribute a = key("a"); + + Expression keyCondition = startsWithExp(a, new Literal(EMPTY, "foo", DataTypes.KEYWORD)); + Expression filter = equalsExpression(); + + KeyedFilter rule1 = keyedFilter(basicFilter(keyCondition), a); + KeyedFilter rule2 = keyedFilter(basicFilter(filter), a); + + AbstractJoin j = randomSequenceOrSample(rule1, rule2); + + List queries = j.queries(); + assertEquals(rule1, queries.get(0)); + assertEquals(keyCondition, filterCondition(queries.get(1).child())); + assertEquals(filterCondition(rule2.child()), filterCondition(queries.get(1).child().children().get(0))); + } + + /** + * sequence + * 1. filter startsWith(a, b) by a + * 2. filter X by a + * == + * same + */ + public void testKeyConstraintWithNonKey() { + Attribute a = key("a"); + + Expression keyCondition = startsWithExp(a, key("b")); + Expression filter = equalsExpression(); + + KeyedFilter rule1 = keyedFilter(basicFilter(keyCondition), a); + KeyedFilter rule2 = keyedFilter(basicFilter(filter), a); + + AbstractJoin j = randomSequenceOrSample(rule1, rule2); + + List queries = j.queries(); + assertEquals(rule1, queries.get(0)); + assertEquals(rule2, queries.get(1)); + } + + /** + * sequence + * 1. filter startsWith(a, b) and c > 10 by a, c + * 2. filter X by a, c + * == + * 1. filter startsWith(a, b) and c > 10 by a, c + * 2. filter c > 10 by a, c + * \filter X + */ + public void testKeyConstraintWithNonKeyPartialPropagation() { + Attribute a = key("a"); + Attribute b = key("b"); + Attribute c = key("c"); + + GreaterThan gtExp = gtExpression(c); + Expression keyCondition = new And(EMPTY, startsWithExp(a, b), gtExp); + Expression filter = equalsExpression(); + + KeyedFilter rule1 = keyedFilter(basicFilter(keyCondition), a, c); + KeyedFilter rule2 = keyedFilter(basicFilter(filter), a, c); + + AbstractJoin j = randomSequenceOrSample(rule1, rule2); + + List queries = j.queries(); + assertEquals(rule1, queries.get(0)); + assertEquals(gtExp, filterCondition(queries.get(1).child())); + assertEquals(filterCondition(rule2.child()), filterCondition(queries.get(1).child().children().get(0))); + } + + /** + * sequence + * 1. filter startsWith(a, b) by a, c + * 2. filter X and c > 10 by a, c + * == + * 1. filter c > 10 by a, c + * \filter startsWith(a, b) + * 2. filter X and c > 10 by a, c + */ + public void testKeyConstraintWithNonKeyPartialReversePropagation() { + Attribute a = key("a"); + Attribute b = key("b"); + Attribute c = key("c"); + + GreaterThan gtExp = gtExpression(c); + Expression keyCondition = startsWithExp(a, b); + Expression filter = new And(EMPTY, equalsExpression(), gtExp); + + KeyedFilter rule1 = keyedFilter(basicFilter(keyCondition), a, c); + KeyedFilter rule2 = keyedFilter(basicFilter(filter), a, c); + + AbstractJoin j = randomSequenceOrSample(rule1, rule2); + + List queries = j.queries(); + assertEquals(gtExp, filterCondition(queries.get(0).child())); + assertEquals(filterCondition(rule1.child()), filterCondition(queries.get(0).child().children().get(0))); + assertEquals(rule2, queries.get(1)); + } + /** * Key conditions inside a disjunction (OR) are ignored *

@@ -817,4 +925,8 @@ private static Equals equalsExpression() { private static GreaterThan gtExpression(Attribute b) { return new GreaterThan(EMPTY, b, new Literal(EMPTY, 1, INTEGER), UTC); } + + private static StartsWith startsWithExp(Expression a, Expression b) { + return new StartsWith(EMPTY, a, b, randomBoolean()); + } }