EQL: make allow_no_indices true by default (#63573) (#63645)

* Allow all indices options variants Irrespective of allow_no_indices value, throw VerificationException when there is no index validated Co-authored-by: Andrei Stefan <[email protected]>
elastic · Oct 14, 2020 · 7e8769a · 7e8769a
1 parent 9015b50
commit 7e8769a
Show file tree

Hide file tree

Showing 12 changed files with 179 additions and 22 deletions.
diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/eql/EqlSearchRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/eql/EqlSearchRequest.java
@@ -34,7 +34,7 @@
 public class EqlSearchRequest implements Validatable, ToXContentObject {
 
     private String[] indices;
-    private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, false, true, false);
+    private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, true, true, false);
 
     private QueryBuilder filter = null;
     private String timestampField = "@timestamp";

diff --git a/...gin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlRestValidationTestCase.java b/...gin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlRestValidationTestCase.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+package org.elasticsearch.test.eql;
+
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.client.ResponseException;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.test.rest.ESRestTestCase;
+import org.junit.Before;
+
+import java.io.IOException;
+
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.xpack.ql.util.StringUtils.EMPTY;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+
+public abstract class EqlRestValidationTestCase extends ESRestTestCase {
+
+    // TODO: handle for existent indices the patterns "test,inexistent", "inexistent,test" that seem to work atm as is
+    private static final String[] existentIndexName = new String[] {"test,inexistent*", "test*,inexistent*", "inexistent*,test"};
+    private static final String[] inexistentIndexName = new String[] {"inexistent", "inexistent*", "inexistent1*,inexistent2*"};
+
+    @Before
+    public void prepareIndices() throws IOException {
+        createIndex("test", Settings.EMPTY);
+
+        Object[] fieldsAndValues = new Object[] {"event_type", "my_event", "@timestamp", "2020-10-08T12:35:48Z", "val", 0};
+        XContentBuilder document = jsonBuilder().startObject();
+        for (int i = 0; i < fieldsAndValues.length; i += 2) {
+            document.field((String) fieldsAndValues[i], fieldsAndValues[i + 1]);
+        }
+        document.endObject();
+        final Request request = new Request("POST", "/test/_doc/" + 0);
+        request.setJsonEntity(Strings.toString(document));
+        assertOK(client().performRequest(request));
+
+        assertOK(adminClient().performRequest(new Request("POST", "/test/_refresh")));
+    }
+
+    public void testDefaultIndicesOptions() throws IOException {
+        String message = "\"root_cause\":[{\"type\":\"verification_exception\",\"reason\":\"Found 1 problem\\nline -1:-1: Unknown index";
+        assertErrorMessageOnInexistentIndices(EMPTY, true, message, EMPTY);
+        assertErrorMessageOnExistentIndices("?allow_no_indices=false", false, message, EMPTY);
+        assertValidRequestOnExistentIndices(EMPTY);
+    }
+
+    public void testAllowNoIndicesOption() throws IOException {
+        boolean allowNoIndices = randomBoolean();
+        boolean setAllowNoIndices = randomBoolean();
+        boolean isAllowNoIndices = allowNoIndices || setAllowNoIndices == false;
+
+        String allowNoIndicesTrueMessage = "\"root_cause\":[{\"type\":\"verification_exception\",\"reason\":"
+            + "\"Found 1 problem\\nline -1:-1: Unknown index";
+        String allowNoIndicesFalseMessage = "\"root_cause\":[{\"type\":\"index_not_found_exception\",\"reason\":\"no such index";
+        String reqParameter = setAllowNoIndices ? "?allow_no_indices=" + allowNoIndices : EMPTY;
+
+        assertErrorMessageOnInexistentIndices(reqParameter, isAllowNoIndices, allowNoIndicesTrueMessage, allowNoIndicesFalseMessage);
+        if (isAllowNoIndices) {
+            assertValidRequestOnExistentIndices(reqParameter);
+        }
+    }
+
+    private void assertErrorMessageOnExistentIndices(String reqParameter, boolean isAllowNoIndices, String allowNoIndicesTrueMessage,
+        String allowNoIndicesFalseMessage) throws IOException {
+        assertErrorMessages(existentIndexName, reqParameter, isAllowNoIndices, allowNoIndicesTrueMessage, allowNoIndicesFalseMessage);
+    }
+
+    private void assertErrorMessageOnInexistentIndices(String reqParameter, boolean isAllowNoIndices, String allowNoIndicesTrueMessage,
+        String allowNoIndicesFalseMessage) throws IOException {
+        assertErrorMessages(inexistentIndexName, reqParameter, isAllowNoIndices, allowNoIndicesTrueMessage, allowNoIndicesFalseMessage);
+    }
+
+    private void assertErrorMessages(String[] indices, String reqParameter, boolean isAllowNoIndices, String allowNoIndicesTrueMessage,
+        String allowNoIndicesFalseMessage) throws IOException {
+        for (String indexName : indices) {
+            final Request request = createRequest(indexName, reqParameter);
+            ResponseException exc = expectThrows(ResponseException.class, () ->  client().performRequest(request));
+
+            assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(isAllowNoIndices ? 400 : 404));
+            // TODO add the index name to the message to be checked. Waiting on https://github.com/elastic/elasticsearch/issues/63529
+            assertThat(exc.getMessage(), containsString(isAllowNoIndices ? allowNoIndicesTrueMessage : allowNoIndicesFalseMessage));
+        }
+    }
+
+    private Request createRequest(String indexName, String reqParameter) throws IOException {
+        final Request request = new Request("POST", "/" + indexName + "/_eql/search" + reqParameter);
+        request.setJsonEntity(Strings.toString(JsonXContent.contentBuilder()
+            .startObject()
+            .field("event_category_field", "event_type")
+            .field("query", "my_event where true")
+            .endObject()));
+
+        return request;
+    }
+
+    private void assertValidRequestOnExistentIndices(String reqParameter) throws IOException {
+        for (String indexName : existentIndexName) {
+            final Request request = createRequest(indexName, reqParameter);
+            Response response = client().performRequest(request);
+            assertOK(response);
+        }
+    }
+}
diff --git a/...in/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlRestValidationIT.java b/...in/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlRestValidationIT.java
@@ -0,0 +1,7 @@
+package org.elasticsearch.xpack.eql;
+
+import org.elasticsearch.test.eql.EqlRestValidationTestCase;
+
+public class EqlRestValidationIT extends EqlRestValidationTestCase {
+
+}
diff --git a/...eql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/eql/AsyncEqlSecurityIT.java b/...eql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/eql/AsyncEqlSecurityIT.java
@@ -89,7 +89,7 @@ private void testCase(String user, String other) throws Exception {
         }
         ResponseException exc = expectThrows(ResponseException.class,
             () -> submitAsyncEqlSearch("index-" + other, "*", TimeValue.timeValueSeconds(10), user));
-        assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(404));
+        assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(400));
     }
 
     static String extractResponseId(Response response) throws IOException {

diff --git a/...ql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlRestValidationIT.java b/...ql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlRestValidationIT.java
@@ -0,0 +1,14 @@
+package org.elasticsearch.xpack.eql;
+
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.test.eql.EqlRestValidationTestCase;
+
+import static org.elasticsearch.xpack.eql.SecurityUtils.secureClientSettings;
+
+public class EqlRestValidationIT extends EqlRestValidationTestCase {
+
+    @Override
+    protected Settings restClientSettings() {
+        return secureClientSettings();
+    }
+}
diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/action/EqlSearchRequest.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/action/EqlSearchRequest.java
@@ -40,8 +40,7 @@ public class EqlSearchRequest extends ActionRequest implements IndicesRequest.Re
     public static TimeValue DEFAULT_KEEP_ALIVE = TimeValue.timeValueDays(5);
 
     private String[] indices;
-    private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true,
-        false, true, false);
+    private IndicesOptions indicesOptions = IndicesOptions.fromOptions(true, true, true, false);
 
     private QueryBuilder filter = null;
     private String timestampField = FIELD_TIMESTAMP;
@@ -122,13 +121,8 @@ public ActionRequestValidationException validate() {
 
         if (indicesOptions == null) {
             validationException = addValidationError("indicesOptions is null", validationException);
-        } else {
-            if (indicesOptions.allowNoIndices()) {
-                validationException = addValidationError("allowNoIndices must be false", validationException);
-            }
         }
 
-
         if (query == null || query.isEmpty()) {
             validationException = addValidationError("query is null or empty", validationException);
         }

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/PreAnalyzer.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/PreAnalyzer.java
@@ -6,14 +6,21 @@
 
 package org.elasticsearch.xpack.eql.analysis;
 
+import org.elasticsearch.xpack.ql.common.Failure;
 import org.elasticsearch.xpack.ql.index.IndexResolution;
 import org.elasticsearch.xpack.ql.plan.logical.EsRelation;
 import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.ql.plan.logical.UnresolvedRelation;
 
+import java.util.Collections;
+
 public class PreAnalyzer {
 
     public LogicalPlan preAnalyze(LogicalPlan plan, IndexResolution indices) {
+        // wrap a potential index_not_found_exception with a VerificationException (expected by client)
+        if (indices.isValid() == false) {
+            throw new VerificationException(Collections.singletonList(Failure.fail(plan, indices.toString())));
+        }
         if (plan.analyzed() == false) {
             final EsRelation esRelation = new EsRelation(plan.source(), indices.get(), false);
             // FIXME: includeFrozen needs to be set already

diff --git a/...plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java b/...plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/TransportEqlSearchAction.java
@@ -108,7 +108,6 @@ public static void operation(PlanExecutor planExecutor, EqlSearchTask task, EqlS
         ZoneId zoneId = DateUtils.of("Z");
         QueryBuilder filter = request.filter();
         TimeValue timeout = TimeValue.timeValueSeconds(30);
-        boolean includeFrozen = request.indicesOptions().ignoreThrottled() == false;
         String clientId = null;
 
         ParserParams params = new ParserParams(zoneId)
@@ -118,8 +117,8 @@ public static void operation(PlanExecutor planExecutor, EqlSearchTask task, EqlS
             .size(request.size())
             .fetchSize(request.fetchSize());
 
-        EqlConfiguration cfg = new EqlConfiguration(request.indices(), zoneId, username, clusterName, filter, timeout, includeFrozen,
-                request.fetchSize(), clientId, new TaskId(nodeId, task.getId()), task);
+        EqlConfiguration cfg = new EqlConfiguration(request.indices(), zoneId, username, clusterName, filter, timeout,
+                request.indicesOptions(), request.fetchSize(), clientId, new TaskId(nodeId, task.getId()), task);
         planExecutor.eql(cfg, request.query(), params, wrap(r -> listener.onResponse(createResponse(r, task.getExecutionId())),
             listener::onFailure));
     }

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlConfiguration.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlConfiguration.java
@@ -6,6 +6,7 @@
 
 package org.elasticsearch.xpack.eql.session;
 
+import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.unit.TimeValue;
@@ -20,7 +21,7 @@ public class EqlConfiguration extends org.elasticsearch.xpack.ql.session.Configu
     private final String[] indices;
     private final TimeValue requestTimeout;
     private final String clientId;
-    private final boolean includeFrozenIndices;
+    private final IndicesOptions indicesOptions;
     private final TaskId taskId;
     private final EqlSearchTask task;
     private final int fetchSize;
@@ -29,15 +30,15 @@ public class EqlConfiguration extends org.elasticsearch.xpack.ql.session.Configu
     private final QueryBuilder filter;
 
     public EqlConfiguration(String[] indices, ZoneId zi, String username, String clusterName, QueryBuilder filter, TimeValue requestTimeout,
-                            boolean includeFrozen, int fetchSize, String clientId, TaskId taskId,
+                            IndicesOptions indicesOptions, int fetchSize, String clientId, TaskId taskId,
                             EqlSearchTask task) {
         super(zi, username, clusterName);
 
         this.indices = indices;
         this.filter = filter;
         this.requestTimeout = requestTimeout;
         this.clientId = clientId;
-        this.includeFrozenIndices = includeFrozen;
+        this.indicesOptions = indicesOptions;
         this.taskId = taskId;
         this.task = task;
         this.fetchSize = fetchSize;
@@ -67,8 +68,8 @@ public String clientId() {
         return clientId;
     }
 
-    public boolean includeFrozen() {
-        return includeFrozenIndices;
+    public IndicesOptions indicesOptions() {
+        return indicesOptions;
     }
 
     public boolean isCancelled() {

diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/session/EqlSession.java
@@ -100,7 +100,7 @@ private <T> void preAnalyze(LogicalPlan parsed, ActionListener<LogicalPlan> list
             listener.onFailure(new TaskCancelledException("cancelled"));
             return;
         }
-        indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.includeFrozen(), configuration.filter(),
+        indexResolver.resolveAsMergedMapping(indexWildcard, null, configuration.indicesOptions(), configuration.filter(),
             map(listener, r -> preAnalyzer.preAnalyze(parsed, r))
         );
     }

diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/EqlTestUtils.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/EqlTestUtils.java
@@ -6,6 +6,7 @@
 
 package org.elasticsearch.xpack.eql;
 
+import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.tasks.TaskId;
 import org.elasticsearch.xpack.core.async.AsyncExecutionId;
@@ -32,7 +33,7 @@ private EqlTestUtils() {
     }
 
     public static final EqlConfiguration TEST_CFG = new EqlConfiguration(new String[] {"none"},
-            org.elasticsearch.xpack.ql.util.DateUtils.UTC, "nobody", "cluster", null, TimeValue.timeValueSeconds(30), false, 
+            org.elasticsearch.xpack.ql.util.DateUtils.UTC, "nobody", "cluster", null, TimeValue.timeValueSeconds(30), null, 
             123, "", new TaskId("test", 123), null);
 
     public static EqlConfiguration randomConfiguration() {
@@ -42,7 +43,7 @@ public static EqlConfiguration randomConfiguration() {
             randomAlphaOfLength(16),
             null,
             new TimeValue(randomNonNegativeLong()),
-            randomBoolean(),
+            randomIndicesOptions(),
             randomIntBetween(1, 1000),
             randomAlphaOfLength(16),
             new TaskId(randomAlphaOfLength(10), randomNonNegativeLong()),
@@ -62,4 +63,9 @@ public static InsensitiveNotEquals sneq(Expression left, Expression right) {
         return new InsensitiveNotEquals(EMPTY, left, right, randomZone());
     }
 
+    public static IndicesOptions randomIndicesOptions() {
+        return IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(),
+            randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean());
+    }
+
 }
diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java
@@ -277,6 +277,18 @@ private void filterResults(String javaRegex, GetAliasesResponse aliases, GetInde
         listener.onResponse(result);
     }
 
+    /**
+     * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping.
+     */
+    public void resolveAsMergedMapping(String indexWildcard, String javaRegex, IndicesOptions indicesOptions, QueryBuilder filter,
+            ActionListener<IndexResolution> listener) {
+        FieldCapabilitiesRequest fieldRequest = createFieldCapsRequest(indexWildcard, indicesOptions, filter);
+        client.fieldCaps(fieldRequest,
+                ActionListener.wrap(
+                        response -> listener.onResponse(mergedMappings(typeRegistry, indexWildcard, response.getIndices(), response.get())),
+                        listener::onFailure));
+    }
+
     /**
      * Resolves a pattern to one (potentially compound meaning that spawns multiple indices) mapping.
      */
@@ -458,15 +470,20 @@ private static EsField createField(DataTypeRegistry typeRegistry, String fieldNa
         return new EsField(fieldName, esType, props, isAggregateable, isAlias);
     }
 
-    private static FieldCapabilitiesRequest createFieldCapsRequest(String index, boolean includeFrozen, QueryBuilder filter) {
+    private static FieldCapabilitiesRequest createFieldCapsRequest(String index, IndicesOptions indicesOptions, QueryBuilder filter) {
         return new FieldCapabilitiesRequest()
                 .indices(Strings.commaDelimitedListToStringArray(index))
                 .fields("*")
                 .includeUnmapped(true)
                 .indexFilter(filter)
                 //lenient because we throw our own errors looking at the response e.g. if something was not resolved
                 //also because this way security doesn't throw authorization exceptions but rather honors ignore_unavailable
-                .indicesOptions(includeFrozen ? FIELD_CAPS_FROZEN_INDICES_OPTIONS : FIELD_CAPS_INDICES_OPTIONS);
+                .indicesOptions(indicesOptions);
+    }
+
+    private static FieldCapabilitiesRequest createFieldCapsRequest(String index, boolean includeFrozen, QueryBuilder filter) {
+        IndicesOptions indicesOptions = includeFrozen ? FIELD_CAPS_FROZEN_INDICES_OPTIONS : FIELD_CAPS_INDICES_OPTIONS;
+        return createFieldCapsRequest(index, indicesOptions, filter);
     }
 
     /**