Browse Source

Improve date math exclusions in expressions (#90298)

The goal here is to reach a consistent way for datemath exclusion
evaluation. The following describes the status quo, before the changes
in this PR:

#### When Security is enabled: * `*,-<index-{now/d}>` correctly
functions as an exclusions, i.e. evaluates to all indices but
`index-2022.09.23` * `*,<-index-{now/d}>` does NOT function as an
exclusion, instead returns a 404 error "no such index
[-index-2022.09.23]"

Also, an expression item containing "-" but which is not preceded by a
wildcard item, errors out in both cases (though with slightly different
messages): * `-<index-{now/d}>,*` errors with "no such index
[-<index-{now/d}>]" * `<-index-{now/d}>,*` errors with "no such index
[-index-2022.09.23]"

#### When Security is disabled:

* `*,-<index-{now/d}>` does NOT function as an exclusion, instead errors with "no such index [<index-{now/d}>]"
* `*,<-index-{now/d}>` correctly functions as an exclusions, i.e. evaluates to all indices but `index-2022.09.23`

Similarly, an expression item containing "-" but which is not preceded
by a wildcard item, erros out in both cases: * `-<index-{now/d}>,*`
errors with "no such index [-<index-{now/d}>]" * `<-index-{now/d}>,*`
errors with "no such index [-index-2022.09.23]"

This PR makes it so that both formats, ie `-<index-{now/d}>` and
`<-index-{now/d}>`, work as exclusions when **Security is disabled**.
This PR doesn't change the behavior when Security is enabled, but this
will be addressed in a follow-up when the Security code will be
completely replaced by the code in core.
Albert Zaharovits 3 years ago
parent
commit
ba46e4e099

+ 5 - 0
docs/changelog/90298.yaml

@@ -0,0 +1,5 @@
+pr: 90298
+summary: Improve date math exclusions in expressions
+area: Infra/Core
+type: enhancement
+issues: []

+ 10 - 1
server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

@@ -1474,8 +1474,17 @@ public class IndexNameExpressionResolver {
 
         public static List<String> resolve(final Context context, List<String> expressions) {
             List<String> result = new ArrayList<>(expressions.size());
+            boolean wildcardSeen = false;
             for (String expression : expressions) {
-                result.add(resolveExpression(expression, context::getStartTime));
+                // accepts date-math exclusions that are of the form "-<...{}>", i.e. the "-" is outside the "<>" date-math template
+                if (Strings.hasLength(expression) && expression.charAt(0) == '-' && wildcardSeen) {
+                    result.add("-" + resolveExpression(expression.substring(1), context::getStartTime));
+                } else {
+                    result.add(resolveExpression(expression, context::getStartTime));
+                }
+                if (Regex.isSimpleMatchPattern(expression)) {
+                    wildcardSeen = true;
+                }
             }
             return result;
         }

+ 2 - 10
server/src/main/java/org/elasticsearch/ingest/IngestService.java

@@ -214,7 +214,8 @@ public class IngestService implements ClusterStateApplier, ReportingService<Inge
             IndexMetadata indexMetadata = null;
             // start to look for default or final pipelines via settings found in the index meta data
             if (originalRequest != null) {
-                indexMetadata = metadata.indices().get(resolveIndexName(originalRequest.index(), epochMillis));
+                indexMetadata = metadata.indices()
+                    .get(IndexNameExpressionResolver.resolveDateMathExpression(originalRequest.index(), epochMillis));
             }
             // check the alias for the index request (this is how normal index requests are modeled)
             if (indexMetadata == null && indexRequest.index() != null) {
@@ -308,15 +309,6 @@ public class IngestService implements ClusterStateApplier, ReportingService<Inge
             || NOOP_PIPELINE_NAME.equals(indexRequest.getFinalPipeline()) == false;
     }
 
-    private static String resolveIndexName(final String unresolvedIndexName, final long epochMillis) {
-        List<String> resolvedNames = IndexNameExpressionResolver.DateMathExpressionResolver.resolve(
-            new IndexNameExpressionResolver.ResolverContext(epochMillis),
-            List.of(unresolvedIndexName)
-        );
-        assert resolvedNames.size() == 1;
-        return resolvedNames.get(0);
-    }
-
     public ClusterService getClusterService() {
         return clusterService;
     }

+ 22 - 0
server/src/test/java/org/elasticsearch/cluster/metadata/DateMathExpressionResolverTests.java

@@ -16,6 +16,7 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.DateMathExpressionResolver;
 import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel;
 import org.elasticsearch.test.ESTestCase;
+import org.hamcrest.Matchers;
 
 import java.time.Instant;
 import java.time.ZoneId;
@@ -70,6 +71,27 @@ public class DateMathExpressionResolverTests extends ESTestCase {
         assertThat(result.get(2), equalTo("logstash-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))));
     }
 
+    public void testExpressionWithWildcardAndExclusions() {
+        List<String> indexExpressions = Arrays.asList(
+            "<-before-inner-{now}>",
+            "-<before-outer-{now}>",
+            "<wild*card-{now}*>",
+            "<-after-inner-{now}>",
+            "-<after-outer-{now}>"
+        );
+        List<String> result = DateMathExpressionResolver.resolve(context, indexExpressions);
+        assertThat(
+            result,
+            Matchers.contains(
+                equalTo("-before-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
+                equalTo("-<before-outer-{now}>"), // doesn't evaluate because it doesn't start with "<" and it is not an exclusion
+                equalTo("wild*card-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())) + "*"),
+                equalTo("-after-inner-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime()))),
+                equalTo("-after-outer-" + formatDate("uuuu.MM.dd", dateFromMillis(context.getStartTime())))
+            )
+        );
+    }
+
     public void testEmpty() throws Exception {
         List<String> result = DateMathExpressionResolver.resolve(context, Collections.<String>emptyList());
         assertThat(result.size(), equalTo(0));

+ 2 - 7
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java

@@ -19,8 +19,6 @@ import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.Index;
 
-import java.util.Collections;
-import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
 
@@ -135,12 +133,9 @@ public class GenerateSnapshotNameStep extends ClusterStateActionStep {
     }
 
     public static String generateSnapshotName(String name, IndexNameExpressionResolver.Context context) {
-        List<String> candidates = IndexNameExpressionResolver.DateMathExpressionResolver.resolve(context, Collections.singletonList(name));
-        if (candidates.size() != 1) {
-            throw new IllegalStateException("resolving snapshot name " + name + " generated more than one candidate: " + candidates);
-        }
+        String candidate = IndexNameExpressionResolver.resolveDateMathExpression(name, context.getStartTime());
         // TODO: we are breaking the rules of UUIDs by lowercasing this here, find an alternative (snapshot names must be lowercase)
-        return candidates.get(0) + "-" + UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT);
+        return candidate + "-" + UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT);
     }
 
     @Nullable