From d66346259529b87305dcca432782c6a83f153101 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 13 Jul 2021 13:00:16 -0500 Subject: [PATCH 1/3] Move ResolverContext class to permit reuse --- .../metadata/IndexNameExpressionResolver.java | 24 +++++++++++++++ .../core/ilm/GenerateSnapshotNameStep.java | 29 +------------------ .../ilm/GenerateSnapshotNameStepTests.java | 3 +- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 5cf44af983c7d..ac2dab9fa45a3 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -1340,4 +1340,28 @@ static String resolveExpression(String expression, final Context context) { return beforePlaceHolderSb.toString(); } } + + /** + * This is a context for the DateMathExpressionResolver which does not require {@code IndicesOptions} or {@code ClusterState} + * since it uses only the start time to resolve expressions. + */ + public static final class ResolverContext extends Context { + public ResolverContext() { + this(System.currentTimeMillis()); + } + + public ResolverContext(long startTime) { + super(null, null, startTime, false, false, false, false, SystemIndexAccessLevel.ALL, name -> false, name -> false); + } + + @Override + public ClusterState getState() { + throw new UnsupportedOperationException("should never be called"); + } + + @Override + public IndicesOptions getOptions() { + throw new UnsupportedOperationException("should never be called"); + } + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java index d05a77d980e91..63686014ba354 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java @@ -9,7 +9,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; @@ -18,7 +17,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; import org.elasticsearch.index.Index; -import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel; import java.util.Collections; import java.util.List; @@ -117,7 +115,7 @@ public boolean equals(Object obj) { * still result in unique snapshot names. */ public static String generateSnapshotName(String name) { - return generateSnapshotName(name, new ResolverContext()); + return generateSnapshotName(name, new IndexNameExpressionResolver.ResolverContext()); } public static String generateSnapshotName(String name, IndexNameExpressionResolver.Context context) { @@ -129,31 +127,6 @@ public static String generateSnapshotName(String name, IndexNameExpressionResolv return candidates.get(0) + "-" + UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT); } - /** - * This is a context for the DateMathExpressionResolver, which does not require - * {@code IndicesOptions} or {@code ClusterState} since it only uses the start - * time to resolve expressions - */ - public static final class ResolverContext extends IndexNameExpressionResolver.Context { - public ResolverContext() { - this(System.currentTimeMillis()); - } - - public ResolverContext(long startTime) { - super(null, null, startTime, false, false, false, false, SystemIndexAccessLevel.ALL, name -> false, name -> false); - } - - @Override - public ClusterState getState() { - throw new UnsupportedOperationException("should never be called"); - } - - @Override - public IndicesOptions getOptions() { - throw new UnsupportedOperationException("should never be called"); - } - } - @Nullable public static ActionRequestValidationException validateGeneratedSnapshotName(String snapshotPrefix, String snapshotName) { ActionRequestValidationException err = new ActionRequestValidationException(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStepTests.java index 2d1c2a4fe1dce..069d0b5b06015 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStepTests.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.Strings; @@ -86,7 +87,7 @@ public void testNameGeneration() { assertThat(generateSnapshotName("name"), startsWith("name-")); assertThat(generateSnapshotName("name").length(), greaterThan("name-".length())); - GenerateSnapshotNameStep.ResolverContext resolverContext = new GenerateSnapshotNameStep.ResolverContext(time); + IndexNameExpressionResolver.ResolverContext resolverContext = new IndexNameExpressionResolver.ResolverContext(time); assertThat(generateSnapshotName("", resolverContext), startsWith("name-2019.03.15-")); assertThat(generateSnapshotName("", resolverContext).length(), greaterThan("name-2019.03.15-".length())); From 52ada9e8ff9ba0b8fb9047f4c30a5723d97595e5 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 13 Jul 2021 13:39:22 -0500 Subject: [PATCH 2/3] resolve date math expressions before retrieving index metadata --- .../elasticsearch/ingest/IngestService.java | 15 +++++++++++++-- .../ingest/IngestServiceTests.java | 18 ++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 3033128e3a2ff..0688303e4e075 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -28,6 +28,7 @@ import org.elasticsearch.cluster.ClusterStateApplier; import org.elasticsearch.cluster.metadata.IndexAbstraction; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.IndexTemplateMetadata; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.metadata.MetadataIndexTemplateService; @@ -74,6 +75,8 @@ public class IngestService implements ClusterStateApplier, ReportingService originalRequest, 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(originalRequest.index()); + indexMetadata = metadata.indices().get(resolveIndexName(originalRequest.index())); } // check the alias for the index request (this is how normal index requests are modeled) if (indexMetadata == null && indexRequest.index() != null) { @@ -218,12 +221,20 @@ public static boolean resolvePipelines(final DocWriteRequest originalRequest, indexRequest.isPipelineResolved(true); } - // return whether this index request has a pipeline return NOOP_PIPELINE_NAME.equals(indexRequest.getPipeline()) == false || NOOP_PIPELINE_NAME.equals(indexRequest.getFinalPipeline()) == false; } + private static String resolveIndexName(String unresolvedIndexName) { + List resolvedNames = DATE_MATH_EXPRESSION_RESOLVER.resolve( + new IndexNameExpressionResolver.ResolverContext(), + List.of(unresolvedIndexName) + ); + assert resolvedNames.size() == 1; + return resolvedNames.get(0); + } + public ClusterService getClusterService() { return clusterService; } diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index 85005c1a98e0a..f8832584f1025 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -37,6 +37,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; @@ -1376,6 +1377,23 @@ public void testResolveFinalPipeline() { assertThat(indexRequest.getFinalPipeline(), equalTo("final-pipeline")); } + public void testResolveFinalPipelineWithDateMathExpression() { + final DateFormatter dateFormatter = DateFormatter.forPattern("uuuu.MM.dd"); + IndexMetadata.Builder builder = IndexMetadata.builder("idx-" + dateFormatter.formatMillis(System.currentTimeMillis())) + .settings(settings(Version.CURRENT).put(IndexSettings.FINAL_PIPELINE.getKey(), "final-pipeline")) + .numberOfShards(1) + .numberOfReplicas(0); + Metadata metadata = Metadata.builder().put(builder).build(); + + // index name matches with IDM: + IndexRequest indexRequest = new IndexRequest(""); + boolean result = IngestService.resolvePipelines(indexRequest, indexRequest, metadata); + assertThat(result, is(true)); + assertThat(indexRequest.isPipelineResolved(), is(true)); + assertThat(indexRequest.getPipeline(), equalTo("_none")); + assertThat(indexRequest.getFinalPipeline(), equalTo("final-pipeline")); + } + public void testResolveRequestOrDefaultPipelineAndFinalPipeline() { // no pipeline: { From b934eb4159d120f4a09ce96c8387aa9dcf204750 Mon Sep 17 00:00:00 2001 From: Dan Hermann Date: Tue, 13 Jul 2021 17:20:03 -0500 Subject: [PATCH 3/3] use single time for unit test --- .../org/elasticsearch/ingest/IngestService.java | 13 +++++++++---- .../elasticsearch/ingest/IngestServiceTests.java | 5 +++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 0688303e4e075..a6b07b9fde65a 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -125,7 +125,12 @@ private static Map processorFactories(List originalRequest, final IndexRequest indexRequest, - final Metadata metadata) { + final Metadata metadata) { + return resolvePipelines(originalRequest, indexRequest, metadata, System.currentTimeMillis()); + } + + public static boolean resolvePipelines(final DocWriteRequest originalRequest, final IndexRequest indexRequest, + final Metadata metadata, final long epochMillis) { if (indexRequest.isPipelineResolved() == false) { final String requestPipeline = indexRequest.getPipeline(); indexRequest.setPipeline(NOOP_PIPELINE_NAME); @@ -135,7 +140,7 @@ public static boolean resolvePipelines(final DocWriteRequest originalRequest, 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())); + indexMetadata = metadata.indices().get(resolveIndexName(originalRequest.index(), epochMillis)); } // check the alias for the index request (this is how normal index requests are modeled) if (indexMetadata == null && indexRequest.index() != null) { @@ -226,9 +231,9 @@ public static boolean resolvePipelines(final DocWriteRequest originalRequest, || NOOP_PIPELINE_NAME.equals(indexRequest.getFinalPipeline()) == false; } - private static String resolveIndexName(String unresolvedIndexName) { + private static String resolveIndexName(final String unresolvedIndexName, final long epochMillis) { List resolvedNames = DATE_MATH_EXPRESSION_RESOLVER.resolve( - new IndexNameExpressionResolver.ResolverContext(), + new IndexNameExpressionResolver.ResolverContext(epochMillis), List.of(unresolvedIndexName) ); assert resolvedNames.size() == 1; diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index f8832584f1025..1dfee9c6f8c68 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -1378,8 +1378,9 @@ public void testResolveFinalPipeline() { } public void testResolveFinalPipelineWithDateMathExpression() { + final long epochMillis = randomLongBetween(1, System.currentTimeMillis()); final DateFormatter dateFormatter = DateFormatter.forPattern("uuuu.MM.dd"); - IndexMetadata.Builder builder = IndexMetadata.builder("idx-" + dateFormatter.formatMillis(System.currentTimeMillis())) + IndexMetadata.Builder builder = IndexMetadata.builder("idx-" + dateFormatter.formatMillis(epochMillis)) .settings(settings(Version.CURRENT).put(IndexSettings.FINAL_PIPELINE.getKey(), "final-pipeline")) .numberOfShards(1) .numberOfReplicas(0); @@ -1387,7 +1388,7 @@ public void testResolveFinalPipelineWithDateMathExpression() { // index name matches with IDM: IndexRequest indexRequest = new IndexRequest(""); - boolean result = IngestService.resolvePipelines(indexRequest, indexRequest, metadata); + boolean result = IngestService.resolvePipelines(indexRequest, indexRequest, metadata, epochMillis); assertThat(result, is(true)); assertThat(indexRequest.isPipelineResolved(), is(true)); assertThat(indexRequest.getPipeline(), equalTo("_none"));