Browse Source

Index expression exclusions never trigger "not found" (#90902)

When Security is disabled, exclusions of names of resources that
do not exist will no longer trigger the "not found" exception.
For example, the expression *,-foo currently throws the "not found"
exception if there is no foo resource (and the ignore_unavailable
request option is false, which is the default for many APIs).
This will no longer be the case. An exclusion will ensure the name is not
included and will never raise any errors.
Albert Zaharovits 3 năm trước cách đây
mục cha
commit
882a477243

+ 5 - 0
docs/changelog/90902.yaml

@@ -0,0 +1,5 @@
+pr: 90902
+summary: Index expression exclusions never trigger "not found"
+area: Infra/Core
+type: bug
+issues: []

+ 56 - 57
server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

@@ -51,6 +51,7 @@ import java.util.SortedMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.LongSupplier;
 import java.util.function.Predicate;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -1196,52 +1197,60 @@ public class IndexNameExpressionResolver {
             Collection<String> result = null;
             boolean wildcardSeen = false;
             for (int i = 0; i < expressions.size(); i++) {
-                String expression = expressions.get(i);
-                validateAliasOrIndex(expression);
-                if (aliasOrIndexExists(context, expression, false)) {
+                String expression = validateAliasOrIndex(expressions.get(i));
+                final Supplier<RuntimeException> missingExpressionException = aliasOrIndexExists(context, expression);
+                if (missingExpressionException == null) {
+                    // expression exists but skip adding it to result for optimisation purposes; they will be added later
                     if (result != null) {
                         result.add(expression);
                     }
-                    continue;
-                }
-                if (result == null) {
-                    // add all the previous ones...
-                    result = new HashSet<>(expressions.subList(0, i));
-                }
-                final boolean add;
-                if (expression.charAt(0) == '-' && wildcardSeen) {
-                    add = false;
-                    expression = expression.substring(1);
                 } else {
-                    add = true;
-                }
-                if (Regex.isSimpleMatchPattern(expression) == false) {
-                    // TODO why does wildcard resolver throw exceptions regarding non wildcarded expressions? This should not be done here.
-                    if (context.getOptions().ignoreUnavailable() == false) {
-                        aliasOrIndexExists(context, expression, true);
+                    if (result == null) {
+                        // add all the previous expressions because they exist but were not added, as an optimisation
+                        result = new HashSet<>(expressions.subList(0, i));
                     }
-                    if (add) {
-                        result.add(expression);
+                    /* An expression does not exist when:
+                       * it should be treated as exclusion because it starts with "-"
+                       * it should be treated as a wildcard (inclusion or exclusion) because it contains a "*"
+                       * it genuinely does not exist or is a datastream or an alias and the request type and options disallow it
+                     */
+                    final boolean add;
+                    if (expression.charAt(0) == '-' && wildcardSeen) {
+                        add = false;
+                        expression = expression.substring(1);
                     } else {
-                        result.remove(expression);
+                        add = true;
+                    }
+                    if (Regex.isSimpleMatchPattern(expression) == false) {
+                        if (add) {
+                            // missing expression that is neither an exclusion nor a wildcard
+                            // this must be a name for a resource that genuinely does not exist
+                            // TODO investigate if this check can be moved outside the wildcard resolver
+                            if (context.getOptions().ignoreUnavailable() == false) {
+                                throw missingExpressionException.get();
+                            }
+                            result.add(expression);
+                        } else {
+                            result.remove(expression);
+                        }
+                    } else {
+                        wildcardSeen = true;
+                        Stream<IndexAbstraction> matchingResources = matchResourcesToWildcard(context, expression);
+                        Stream<String> matchingOpenClosedNames = expandToOpenClosed(context, matchingResources);
+                        AtomicBoolean emptyWildcardExpansion = new AtomicBoolean(false);
+                        if (context.getOptions().allowNoIndices() == false) {
+                            emptyWildcardExpansion.set(true);
+                            matchingOpenClosedNames = matchingOpenClosedNames.peek(x -> emptyWildcardExpansion.set(false));
+                        }
+                        if (add) {
+                            matchingOpenClosedNames.forEachOrdered(result::add);
+                        } else {
+                            matchingOpenClosedNames.forEachOrdered(result::remove);
+                        }
+                        if (emptyWildcardExpansion.get()) {
+                            throw indexNotFoundException(expression);
+                        }
                     }
-                    continue;
-                }
-                wildcardSeen = true;
-                Stream<IndexAbstraction> matchingResources = matchResourcesToWildcard(context, expression);
-                Stream<String> matchingOpenClosedNames = expandToOpenClosed(context, matchingResources);
-                AtomicBoolean emptyWildcardExpansion = new AtomicBoolean(false);
-                if (context.getOptions().allowNoIndices() == false) {
-                    emptyWildcardExpansion.set(true);
-                    matchingOpenClosedNames = matchingOpenClosedNames.peek(x -> emptyWildcardExpansion.set(false));
-                }
-                if (add) {
-                    matchingOpenClosedNames.forEachOrdered(result::add);
-                } else {
-                    matchingOpenClosedNames.forEachOrdered(result::remove);
-                }
-                if (emptyWildcardExpansion.get()) {
-                    throw indexNotFoundException(expression);
                 }
             }
             if (result == null) {
@@ -1252,7 +1261,7 @@ public class IndexNameExpressionResolver {
             }
         }
 
-        private static void validateAliasOrIndex(String expression) {
+        private static String validateAliasOrIndex(String expression) {
             if (Strings.isEmpty(expression)) {
                 throw indexNotFoundException(expression);
             }
@@ -1263,34 +1272,24 @@ public class IndexNameExpressionResolver {
             if (expression.charAt(0) == '_') {
                 throw new InvalidIndexNameException(expression, "must not start with '_'.");
             }
+            return expression;
         }
 
-        private static boolean aliasOrIndexExists(Context context, String expression, boolean throwExceptionIfAbsent) {
+        @Nullable
+        private static Supplier<RuntimeException> aliasOrIndexExists(Context context, String expression) {
             final IndicesOptions options = context.getOptions();
             IndexAbstraction indexAbstraction = context.getState().getMetadata().getIndicesLookup().get(expression);
             if (indexAbstraction == null) {
-                if (throwExceptionIfAbsent) {
-                    throw indexNotFoundException(expression);
-                }
-                return false;
+                return () -> indexNotFoundException(expression);
             }
-
             // treat aliases as unavailable indices when ignoreAliases is set to true (e.g. delete index and update aliases api)
             if (indexAbstraction.getType() == Type.ALIAS && options.ignoreAliases()) {
-                if (throwExceptionIfAbsent) {
-                    throw aliasesNotSupportedException(expression);
-                }
-                return false;
+                return () -> aliasesNotSupportedException(expression);
             }
-
             if (indexAbstraction.isDataStreamRelated() && context.includeDataStreams() == false) {
-                if (throwExceptionIfAbsent) {
-                    throw indexNotFoundException(expression);
-                }
-                return false;
+                return () -> indexNotFoundException(expression);
             }
-
-            return true;
+            return null;
         }
 
         private static IndexNotFoundException indexNotFoundException(String expression) {

+ 21 - 3
server/src/test/java/org/elasticsearch/cluster/metadata/WildcardExpressionResolverTests.java

@@ -41,9 +41,10 @@ public class WildcardExpressionResolverTests extends ESTestCase {
             .put(indexBuilder("kuku"));
         ClusterState state = ClusterState.builder(new ClusterName("_name")).metadata(mdBuilder).build();
 
+        IndicesOptions indicesOptions = randomFrom(IndicesOptions.strictExpandOpen(), IndicesOptions.lenientExpandOpen());
         IndexNameExpressionResolver.Context context = new IndexNameExpressionResolver.Context(
             state,
-            IndicesOptions.lenientExpandOpen(),
+            indicesOptions,
             SystemIndexAccessLevel.NONE
         );
         assertThat(
@@ -79,9 +80,26 @@ public class WildcardExpressionResolverTests extends ESTestCase {
             equalTo(newHashSet("testXXX", "testXYY", "testYYY"))
         );
         assertThat(
-            newHashSet(IndexNameExpressionResolver.WildcardExpressionResolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))),
-            equalTo(newHashSet("testXXX", "-testXXX"))
+            newHashSet(
+                IndexNameExpressionResolver.WildcardExpressionResolver.resolve(
+                    context,
+                    Arrays.asList("testX*", "-doe", "-testXXX", "-testYYY")
+                )
+            ),
+            equalTo(newHashSet("testXYY"))
         );
+        if (indicesOptions == IndicesOptions.lenientExpandOpen()) {
+            assertThat(
+                newHashSet(IndexNameExpressionResolver.WildcardExpressionResolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))),
+                equalTo(newHashSet("testXXX", "-testXXX"))
+            );
+        } else if (indicesOptions == IndicesOptions.strictExpandOpen()) {
+            IndexNotFoundException infe = expectThrows(
+                IndexNotFoundException.class,
+                () -> IndexNameExpressionResolver.WildcardExpressionResolver.resolve(context, Arrays.asList("testXXX", "-testXXX"))
+            );
+            assertEquals("-testXXX", infe.getIndex().getName());
+        }
         assertThat(
             newHashSet(IndexNameExpressionResolver.WildcardExpressionResolver.resolve(context, Arrays.asList("testXXX", "-testX*"))),
             equalTo(newHashSet("testXXX"))