EQL: Wrap a verification_exception in an index_not_found_exception in case there is no valid index available

x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/EqlRestValidationTestCase.java

@@ -25,70 +25,72 @@
 
 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*"};
+    private static final String indexName = "test_eql";
+    protected static final String[] existentIndexWithWildcard = new String[] {indexName + ",inexistent*", indexName + "*,inexistent*",
+        "inexistent*," + indexName};
+    private static final String[] existentIndexWithoutWildcard = new String[] {indexName + ",inexistent", "inexistent," + indexName};
+    protected static final String[] inexistentIndexNameWithWildcard = new String[] {"inexistent*", "inexistent1*,inexistent2*"};
+    protected static final String[] inexistentIndexNameWithoutWildcard = new String[] {"inexistent", "inexistent1,inexistent2"};
 
     @Before
     public void prepareIndices() throws IOException {
-        createIndex("test", Settings.EMPTY);
+        if (client().performRequest(new Request("HEAD", "/" + indexName)).getStatusLine().getStatusCode() == 404) {
+            createIndex(indexName, 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);
+        final Request request = new Request("POST", "/" + indexName + "/_doc/" + 0);
         request.setJsonEntity(Strings.toString(document));
         assertOK(client().performRequest(request));
 
-        assertOK(adminClient().performRequest(new Request("POST", "/test/_refresh")));
+        assertOK(adminClient().performRequest(new Request("POST", "/" + indexName + "/_refresh")));
     }
 
+    protected abstract String getInexistentIndexErrorMessage();
+
+    protected abstract void assertErrorMessageWhenAllowNoIndicesIsFalse(String reqParameter) throws IOException;
+
     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);
+        assertErrorMessages(inexistentIndexNameWithWildcard, EMPTY, getInexistentIndexErrorMessage());
+        assertErrorMessages(inexistentIndexNameWithoutWildcard, EMPTY, getInexistentIndexErrorMessage());
+        assertValidRequestOnIndices(existentIndexWithWildcard, EMPTY);
+        assertValidRequestOnIndices(existentIndexWithoutWildcard, 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);
+            assertErrorMessages(inexistentIndexNameWithWildcard, reqParameter, getInexistentIndexErrorMessage());
+            assertErrorMessages(inexistentIndexNameWithoutWildcard, reqParameter, getInexistentIndexErrorMessage());
+            assertValidRequestOnIndices(existentIndexWithWildcard, reqParameter);
+            assertValidRequestOnIndices(existentIndexWithoutWildcard, reqParameter);
+        } else {
+            assertValidRequestOnIndices(existentIndexWithoutWildcard, reqParameter);
+            assertErrorMessageWhenAllowNoIndicesIsFalse(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);
+    protected void assertErrorMessages(String[] indices, String reqParameter, String errorMessage) throws IOException {
+        for (String indexName : indices) {
+            assertErrorMessage(indexName, reqParameter, errorMessage + "[" + indexName + "]");
+        }
     }
 
-    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));
+    protected void assertErrorMessage(String indexName, String reqParameter, String errorMessage) throws IOException {
+        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));
-        }
+        assertThat(exc.getResponse().getStatusLine().getStatusCode(), equalTo(404));
+        assertThat(exc.getMessage(), containsString(errorMessage));
     }
 
     private Request createRequest(String indexName, String reqParameter) throws IOException {
@@ -102,8 +104,8 @@ private Request createRequest(String indexName, String reqParameter) throws IOEx
         return request;
     }
 
-    private void assertValidRequestOnExistentIndices(String reqParameter) throws IOException {
-        for (String indexName : existentIndexName) {
+    private void assertValidRequestOnIndices(String[] indices, String reqParameter) throws IOException {
+        for (String indexName : indices) {
             final Request request = createRequest(indexName, reqParameter);
             Response response = client().performRequest(request);
             assertOK(response);

x-pack/plugin/eql/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlRestValidationIT.java

@@ -2,6 +2,26 @@
 
 import org.elasticsearch.test.eql.EqlRestValidationTestCase;
 
+import java.io.IOException;
+
 public class EqlRestValidationIT extends EqlRestValidationTestCase {
 
+    @Override
+    protected String getInexistentIndexErrorMessage() {
+        return "\"root_cause\":[{\"type\":\"verification_exception\",\"reason\":\"Found 1 problem\\nline -1:-1: Unknown index ";
+    }
+
+    protected void assertErrorMessageWhenAllowNoIndicesIsFalse(String reqParameter) throws IOException {
+        assertErrorMessage("inexistent1*", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent1*]\"");
+        assertErrorMessage("inexistent1*,inexistent2*", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent1*]\"");
+        assertErrorMessage("test_eql,inexistent*", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent*]\"");
+        assertErrorMessage("inexistent", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent]\"");
+        //TODO: revisit after https://github.com/elastic/elasticsearch/issues/64197 is closed
+        assertErrorMessage("inexistent1,inexistent2", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [null]\"");
+    }
 }

x-pack/plugin/eql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/eql/EqlRestValidationIT.java

@@ -3,6 +3,8 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.eql.EqlRestValidationTestCase;
 
+import java.io.IOException;
+
 import static org.elasticsearch.xpack.eql.SecurityUtils.secureClientSettings;
 
 public class EqlRestValidationIT extends EqlRestValidationTestCase {
@@ -11,4 +13,26 @@ public class EqlRestValidationIT extends EqlRestValidationTestCase {
     protected Settings restClientSettings() {
         return secureClientSettings();
     }
+
+    @Override
+    protected String getInexistentIndexErrorMessage() {
+        return "\"root_cause\":[{\"type\":\"verification_exception\",\"reason\":\"Found 1 problem\\nline -1:-1: Unknown index [*,-*]\"}],"
+            + "\"type\":\"index_not_found_exception\",\"reason\":\"no such index ";
+    }
+
+    @Override
+    protected void assertErrorMessageWhenAllowNoIndicesIsFalse(String reqParameter) throws IOException {
+        assertErrorMessage("inexistent1*", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent1*]\"");
+        assertErrorMessage("inexistent1*,inexistent2*", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent1*]\"");
+        assertErrorMessage("test_eql,inexistent*", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [inexistent*]\"");
+        //TODO: revisit the next two tests when https://github.com/elastic/elasticsearch/issues/64190 is closed
+        assertErrorMessage("inexistent", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [[inexistent]]\"");
+        assertErrorMessage("inexistent1,inexistent2", reqParameter, "\"root_cause\":[{\"type\":\"index_not_found_exception\","
+            + "\"reason\":\"no such index [[inexistent1, inexistent2]]\"");
+    }
+
 }

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/PreAnalyzer.java

@@ -6,6 +6,7 @@
 
 package org.elasticsearch.xpack.eql.analysis;
 
+import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.xpack.ql.common.Failure;
 import org.elasticsearch.xpack.ql.index.IndexResolution;
 import org.elasticsearch.xpack.ql.plan.logical.EsRelation;
@@ -17,9 +18,11 @@
 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())));
+            VerificationException cause = new VerificationException(Collections.singletonList(Failure.fail(plan, indices.toString())));
+            // Wrapping the verification_exception in an infe to easily distinguish it on the rest layer in case it needs rewriting
+            // (see RestEqlSearchAction for its usage).
+            throw new IndexNotFoundException(indices.toString(), cause);
         }
         if (plan.analyzed() == false) {
             final EsRelation esRelation = new EsRelation(plan.source(), indices.get(), false);

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/plugin/RestEqlSearchAction.java

@@ -5,19 +5,21 @@
  */
 package org.elasticsearch.xpack.eql.plugin;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.BytesRestResponse;
 import org.elasticsearch.rest.RestRequest;
-import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.action.RestCancellableNodeClient;
-import org.elasticsearch.rest.action.RestResponseListener;
 import org.elasticsearch.xpack.eql.action.EqlSearchAction;
 import org.elasticsearch.xpack.eql.action.EqlSearchRequest;
 import org.elasticsearch.xpack.eql.action.EqlSearchResponse;
@@ -29,6 +31,7 @@
 import static org.elasticsearch.rest.RestRequest.Method.POST;
 
 public class RestEqlSearchAction extends BaseRestHandler {
+    private static Logger logger = LogManager.getLogger(RestEqlSearchAction.class);
     private static final String SEARCH_PATH = "/{index}/_eql/search";
 
     @Override
@@ -43,9 +46,11 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
         throws IOException {
 
         EqlSearchRequest eqlRequest;
+        String indices;
         try (XContentParser parser = request.contentOrSourceParamParser()) {
             eqlRequest = EqlSearchRequest.fromXContent(parser);
-            eqlRequest.indices(Strings.splitStringByCommaToArray(request.param("index")));
+            indices = request.param("index");
+            eqlRequest.indices(Strings.splitStringByCommaToArray(indices));
             eqlRequest.indicesOptions(IndicesOptions.fromRequest(request, eqlRequest.indicesOptions()));
             if (request.hasParam("wait_for_completion_timeout")) {
                 eqlRequest.waitForCompletionTimeout(
@@ -59,12 +64,39 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
 
         return channel -> {
             RestCancellableNodeClient cancellableClient = new RestCancellableNodeClient(client, request.getHttpChannel());
-            cancellableClient.execute(EqlSearchAction.INSTANCE, eqlRequest, new RestResponseListener<>(channel) {
+            cancellableClient.execute(EqlSearchAction.INSTANCE, eqlRequest, new ActionListener<>() {
                 @Override
-                public RestResponse buildResponse(EqlSearchResponse response) throws Exception {
-                    XContentBuilder builder = channel.newBuilder(request.getXContentType(), XContentType.JSON, true);
-                    response.toXContent(builder, request);
-                    return new BytesRestResponse(RestStatus.OK, builder);
+                public void onResponse(EqlSearchResponse response) {
+                    try {
+                        XContentBuilder builder = channel.newBuilder(request.getXContentType(), XContentType.JSON, true);
+                        response.toXContent(builder, request);
+                        channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder));
+                    } catch (Exception e) {
+                        onFailure(e);
+                    }
+                }
+
+                @Override
+                public void onFailure(Exception e) {
+                    Exception finalException = e;
+                    /* 
+                     * In a scenario when Security is enabled and a wildcarded pattern gets resolved to no index, the original error
+                     * message will not contain the initial pattern, but "*,-*". So, we'll throw a INFE from the PreAnalyzer that will
+                     * contain as cause the VerificationException with "*,-*" pattern but we'll rewrite the INFE here with the initial
+                     * pattern that failed resolving. More details here https://github.com/elastic/elasticsearch/issues/63529
+                     */
+                    if (e instanceof IndexNotFoundException) {
+                        IndexNotFoundException infe = (IndexNotFoundException) e;
+                        if (infe.getIndex() != null && infe.getIndex().getName().equals("Unknown index [*,-*]")) {
+                            finalException = new IndexNotFoundException(indices, infe.getCause());
+                        }
+                    }
+                    try {
+                        channel.sendResponse(new BytesRestResponse(channel, finalException));
+                    } catch (Exception inner) {
+                        inner.addSuppressed(finalException);
+                        logger.error("failed to send failure response", inner);
+                    }
                 }
             });
         };