Fix index expression options for requests with a single expression (#…

…91231)

This PR affects requests that contain a single index name
or a single pattern (wildcard/datemath).
It aims to systematize the handling of the `allow_no_indices`
and `ignore_unavailable`indices options:
 * the allow_no_indices option is to be concerned with
wildcards that expand to nothing (or the entire request
expands to nothing)
 * the ignore_unavailable option is to be concerned with
explicit names only (not wildcards)

In addition, the behavior of the above options will now be
independent of the number of expressions in a request.

docs/changelog/91231.yaml

@@ -0,0 +1,5 @@
+pr: 91231
+summary: Fix index expression options for requests with a single name or pattern
+area: Infra/Core
+type: bug
+issues: []

modules/rank-eval/src/internalClusterTest/java/org/elasticsearch/index/rankeval/RankEvalRequestIT.java

@@ -291,7 +291,7 @@ public void testIndicesOptions() {
 
         // test expand_wildcards
         request = new RankEvalRequest(task, new String[] { "tes*" });
-        request.indicesOptions(IndicesOptions.fromParameters("none", null, null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
+        request.indicesOptions(IndicesOptions.fromParameters("none", "true", null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
         response = client().execute(RankEvalAction.INSTANCE, request).actionGet();
         details = (PrecisionAtK.Detail) response.getPartialResults().get("amsterdam_query").getMetricDetails();
         assertEquals(0, details.getRetrieved());

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.get_mapping/50_wildcard_expansion.yml

@@ -111,6 +111,7 @@ setup:
     indices.get_mapping:
         index: test-x*
         expand_wildcards: none
+        ignore_unavailable: true
 
  - match: { '':  {} }
 ---
@@ -121,6 +122,19 @@ setup:
         index: test-x*
         expand_wildcards: none
         allow_no_indices: false
+        ignore_unavailable: true
+---
+"Get test-* with wildcard_expansion=none ignore_unavailable=false":
+  - skip:
+      version: " - 8.5.99"
+      reason: "bug fixed in 8.6"
+  - do:
+      catch: missing
+      indices.get_mapping:
+        index: test-x*
+        expand_wildcards: none
+        allow_no_indices: true
+        ignore_unavailable: false
 ---
 "Get test-* with wildcard_expansion=open,closed":
 

server/src/internalClusterTest/java/org/elasticsearch/validate/SimpleValidateQueryIT.java

@@ -260,7 +260,7 @@ public void testValidateEmptyCluster() {
             client().admin().indices().prepareValidateQuery().get();
             fail("Expected IndexNotFoundException");
         } catch (IndexNotFoundException e) {
-            assertThat(e.getMessage(), is("no such index [null] and no indices exist"));
+            assertThat(e.getMessage(), is("no such index [_all] and no indices exist"));
         }
     }
 

server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

@@ -335,28 +335,12 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
                 }
             }
         }
-        // If only one index is specified then whether we fail a request if an index is missing depends on the allow_no_indices
-        // option. At some point we should change this, because there shouldn't be a reason why whether a single index
-        // or multiple indices are specified yield different behaviour.
-        final boolean failNoIndices = indexExpressions.length == 1
-            ? options.allowNoIndices() == false
-            : options.ignoreUnavailable() == false;
+
         final Collection<String> expressions = resolveExpressions(Arrays.asList(indexExpressions), context);
 
-        if (expressions.isEmpty()) {
+        if (expressions.isEmpty() || (expressions.size() == 1 && expressions.iterator().next().equals(Metadata.ALL))) {
             if (options.allowNoIndices() == false) {
-                IndexNotFoundException infe;
-                if (indexExpressions.length == 1) {
-                    if (indexExpressions[0].equals(Metadata.ALL)) {
-                        infe = new IndexNotFoundException("no indices exist", (String) null);
-                    } else {
-                        infe = new IndexNotFoundException((String) null);
-                    }
-                } else {
-                    infe = new IndexNotFoundException((String) null);
-                }
-                infe.setResources("index_expression", indexExpressions);
-                throw infe;
+                throw notFoundException(indexExpressions);
             } else {
                 return Index.EMPTY_ARRAY;
             }
@@ -368,20 +352,15 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
         for (String expression : expressions) {
             IndexAbstraction indexAbstraction = indicesLookup.get(expression);
             if (indexAbstraction == null) {
-                if (failNoIndices) {
-                    IndexNotFoundException infe;
-                    if (expression.equals(Metadata.ALL)) {
-                        infe = new IndexNotFoundException("no indices exist", expression);
-                    } else {
-                        infe = new IndexNotFoundException(expression);
-                    }
-                    infe.setResources("index_expression", expression);
-                    throw infe;
+                if (options.ignoreUnavailable() == false) {
+                    assert options.expandWildcardExpressions() == false;
+                    throw notFoundException(expression);
                 } else {
                     continue;
                 }
             } else if (indexAbstraction.getType() == Type.ALIAS && context.getOptions().ignoreAliases()) {
-                if (failNoIndices) {
+                if (options.ignoreUnavailable() == false) {
+                    assert options.expandWildcardExpressions() == false;
                     throw aliasesNotSupportedException(expression);
                 } else {
                     continue;
@@ -436,8 +415,7 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
         }
 
         if (options.allowNoIndices() == false && concreteIndices.isEmpty()) {
-            IndexNotFoundException infe = new IndexNotFoundException((String) null);
-            infe.setResources("index_expression", indexExpressions);
+            IndexNotFoundException infe = notFoundException(indexExpressions);
             if (excludedDataStreams) {
                 // Allows callers to handle IndexNotFoundException differently based on whether data streams were excluded.
                 infe.addMetadata(EXCLUDED_DATA_STREAMS_KEY, "true");
@@ -448,6 +426,22 @@ Index[] concreteIndices(Context context, String... indexExpressions) {
         return concreteIndices.toArray(Index.EMPTY_ARRAY);
     }
 
+    private IndexNotFoundException notFoundException(String... indexExpressions) {
+        IndexNotFoundException infe;
+        if (indexExpressions.length == 1) {
+            if (indexExpressions[0].equals(Metadata.ALL)) {
+                infe = new IndexNotFoundException("no indices exist", indexExpressions[0]);
+            } else {
+                infe = new IndexNotFoundException(indexExpressions[0]);
+            }
+            infe.setResources("index_expression", indexExpressions[0]);
+        } else {
+            infe = new IndexNotFoundException((String) null);
+            infe.setResources("index_expression", indexExpressions);
+        }
+        return infe;
+    }
+
     private void checkSystemIndexAccess(Context context, Set<Index> concreteIndices) {
         final Metadata metadata = context.getState().metadata();
         final Predicate<String> systemIndexAccessPredicate = context.getSystemIndexAccessPredicate().negate();

server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java

@@ -478,8 +478,9 @@ public void testConcreteIndexNamesExpandWildcards() {
         results = indexNameExpressionResolver.concreteIndexNames(context, Strings.EMPTY_ARRAY);
         assertThat(results, emptyArray());
 
-        results = indexNameExpressionResolver.concreteIndexNames(context, "h*");
-        assertThat(results, emptyArray());
+        IndexNameExpressionResolver.Context context3 = context;
+        infe = expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(context3, "h*"));
+        assertThat(infe.getResourceId().toString(), equalTo("[h*]"));
 
         results = indexNameExpressionResolver.concreteIndexNames(context, "hidden");
         assertThat(results, arrayContainingInAnyOrder("hidden"));
@@ -562,8 +563,11 @@ public void testConcreteIndexNamesNoExpandWildcards() {
                 SystemIndexAccessLevel.NONE
             );
             {
-                String[] results = indexNameExpressionResolver.concreteIndexNames(context, "baz*");
-                assertThat(results, emptyArray());
+                IndexNotFoundException infe = expectThrows(
+                    IndexNotFoundException.class,
+                    () -> indexNameExpressionResolver.concreteIndexNames(context, "baz*")
+                );
+                assertThat(infe.getIndex().getName(), equalTo("baz*"));
             }
             {
                 IndexNotFoundException infe = expectThrows(
@@ -829,7 +833,7 @@ public void testConcreteIndicesNoIndicesErrorMessage() {
             IndexNotFoundException.class,
             () -> indexNameExpressionResolver.concreteIndices(context, new String[] {})
         );
-        assertThat(infe.getMessage(), is("no such index [null] and no indices exist"));
+        assertThat(infe.getMessage(), is("no such index [_all] and no indices exist"));
     }
 
     public void testConcreteIndicesNoIndicesErrorMessageNoExpand() {
@@ -1292,13 +1296,14 @@ public void testConcreteIndicesWildcardNoMatch() {
                 SystemIndexAccessLevel.NONE
             );
 
-            // asking for non existing wildcard pattern should return empty list or exception
-            if (indicesOptions.allowNoIndices()) {
+            if (indicesOptions.allowNoIndices() == false
+                || indicesOptions.expandWildcardExpressions() == false && indicesOptions.ignoreUnavailable() == false) {
+                expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(context, "Foo*"));
+            } else {
+                // asking for non existing wildcard pattern should return empty list or exception
                 String[] concreteIndices = indexNameExpressionResolver.concreteIndexNames(context, "Foo*");
                 assertThat(concreteIndices, notNullValue());
                 assertThat(concreteIndices.length, equalTo(0));
-            } else {
-                expectThrows(IndexNotFoundException.class, () -> indexNameExpressionResolver.concreteIndexNames(context, "Foo*"));
             }
         }
     }
@@ -2471,7 +2476,7 @@ public void testDataStreams() {
                 IndexNotFoundException.class,
                 () -> indexNameExpressionResolver.concreteWriteIndex(state, indicesOptions, "my-data-stream", false, false)
             );
-            assertThat(e.getMessage(), equalTo("no such index [null]"));
+            assertThat(e.getMessage(), equalTo("no such index [my-data-stream]"));
         }
     }