diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidator.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidator.java index ef28fed54cf48..1a292bbebeadd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidator.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidator.java @@ -46,7 +46,6 @@ public final class SourceDestValidator { // messages public static final String SOURCE_INDEX_MISSING = "Source index [{0}] does not exist"; - public static final String SOURCE_LOWERCASE = "Source index [{0}] must be lowercase"; public static final String DEST_IN_SOURCE = "Destination index [{0}] is included in source expression [{1}]"; public static final String DEST_LOWERCASE = "Destination index [{0}] must be lowercase"; public static final String NEEDS_REMOTE_CLUSTER_SEARCH = "Source index is configured with a remote index pattern(s) [{0}]" @@ -59,6 +58,7 @@ public final class SourceDestValidator { + "alias [{0}], at least a [{1}] license is required, found license [{2}]"; public static final String REMOTE_CLUSTER_LICENSE_INACTIVE = "License check failed for remote cluster " + "alias [{0}], license is not active"; + public static final String REMOTE_SOURCE_INDICES_NOT_SUPPORTED = "remote source indices are not supported"; private final IndexNameExpressionResolver indexNameExpressionResolver; private final RemoteClusterService remoteClusterService; @@ -216,7 +216,7 @@ private void resolveLocalAndRemoteSource() { } } - interface SourceDestValidation { + public interface SourceDestValidation { void validate(Context context, ActionListener listener); } @@ -228,18 +228,7 @@ interface SourceDestValidation { public static final SourceDestValidation REMOTE_SOURCE_VALIDATION = new RemoteSourceEnabledAndRemoteLicenseValidation(); public static final SourceDestValidation DESTINATION_IN_SOURCE_VALIDATION = new DestinationInSourceValidation(); public static final SourceDestValidation DESTINATION_SINGLE_INDEX_VALIDATION = new DestinationSingleIndexValidation(); - - // set of default validation collections, if you want to automatically benefit from new validators, use those - public static final List PREVIEW_VALIDATIONS = Arrays.asList(SOURCE_MISSING_VALIDATION, REMOTE_SOURCE_VALIDATION); - - public static final List ALL_VALIDATIONS = Arrays.asList( - SOURCE_MISSING_VALIDATION, - REMOTE_SOURCE_VALIDATION, - DESTINATION_IN_SOURCE_VALIDATION, - DESTINATION_SINGLE_INDEX_VALIDATION - ); - - public static final List NON_DEFERABLE_VALIDATIONS = Arrays.asList(DESTINATION_SINGLE_INDEX_VALIDATION); + public static final SourceDestValidation REMOTE_SOURCE_NOT_SUPPORTED_VALIDATION = new RemoteSourceNotSupportedValidation(); /** * Create a new Source Dest Validator @@ -299,10 +288,11 @@ public void validate( } }, listener::onFailure); + // We traverse the validations in reverse order as we chain the listeners from back to front for (int i = validations.size() - 1; i >= 0; i--) { - final SourceDestValidation validation = validations.get(i); + SourceDestValidation validation = validations.get(i); final ActionListener previousValidationListener = validationListener; - validationListener = ActionListener.wrap(c -> { validation.validate(c, previousValidationListener); }, listener::onFailure); + validationListener = ActionListener.wrap(c -> validation.validate(c, previousValidationListener), listener::onFailure); } validationListener.onResponse(context); @@ -427,13 +417,13 @@ public void validate(Context context, ActionListener listener) { return; } - if (context.resolvedSource.contains(destIndex)) { + if (context.resolveSource().contains(destIndex)) { context.addValidationError(DEST_IN_SOURCE, destIndex, Strings.arrayToCommaDelimitedString(context.getSource())); listener.onResponse(context); return; } - if (context.resolvedSource.contains(context.resolveDest())) { + if (context.resolveSource().contains(context.resolveDest())) { context.addValidationError( DEST_IN_SOURCE, context.resolveDest(), @@ -454,6 +444,17 @@ public void validate(Context context, ActionListener listener) { } } + static class RemoteSourceNotSupportedValidation implements SourceDestValidation { + + @Override + public void validate(Context context, ActionListener listener) { + if (context.resolveRemoteSource().isEmpty() == false) { + context.addValidationError(REMOTE_SOURCE_INDICES_NOT_SUPPORTED); + } + listener.onResponse(context); + } + } + private static String getMessage(String message, Object... args) { return new MessageFormat(message, Locale.ROOT).format(args); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutDataFrameAnalyticsAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutDataFrameAnalyticsAction.java index 4f4ddc388aed7..7ce5be3b980bd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutDataFrameAnalyticsAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutDataFrameAnalyticsAction.java @@ -18,9 +18,11 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.xpack.core.common.validation.SourceDestValidator; import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsConfig; import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsSource; import org.elasticsearch.xpack.core.ml.job.messages.Messages; +import org.elasticsearch.xpack.core.ml.utils.MlStrings; import java.io.IOException; import java.util.Objects; @@ -90,14 +92,29 @@ public DataFrameAnalyticsConfig getConfig() { @Override public ActionRequestValidationException validate() { ActionRequestValidationException error = null; + error = checkConfigIdIsValid(config, error); + error = SourceDestValidator.validateRequest(error, config.getDest().getIndex()); error = checkNoIncludedAnalyzedFieldsAreExcludedBySourceFiltering(config, error); return error; } + private ActionRequestValidationException checkConfigIdIsValid(DataFrameAnalyticsConfig config, + ActionRequestValidationException error) { + if (MlStrings.isValidId(config.getId()) == false) { + error = ValidateActions.addValidationError(Messages.getMessage(Messages.INVALID_ID, DataFrameAnalyticsConfig.ID, + config.getId()), error); + } + if (!MlStrings.hasValidLengthForId(config.getId())) { + error = ValidateActions.addValidationError(Messages.getMessage(Messages.ID_TOO_LONG, DataFrameAnalyticsConfig.ID, + config.getId(), MlStrings.ID_LENGTH_LIMIT), error); + } + return error; + } + private ActionRequestValidationException checkNoIncludedAnalyzedFieldsAreExcludedBySourceFiltering( DataFrameAnalyticsConfig config, ActionRequestValidationException error) { if (config.getAnalyzedFields() == null) { - return null; + return error; } for (String analyzedInclude : config.getAnalyzedFields().includes()) { if (config.getSource().isFieldExcluded(analyzedInclude)) { @@ -107,7 +124,7 @@ private ActionRequestValidationException checkNoIncludedAnalyzedFieldsAreExclude + DataFrameAnalyticsSource._SOURCE.getPreferredName() + "]", error); } } - return null; + return error; } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDest.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDest.java index 3bc435336f062..98f1bdeb19189 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDest.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDest.java @@ -13,15 +13,11 @@ import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.indices.InvalidIndexNameException; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import java.io.IOException; -import java.util.Locale; import java.util.Objects; -import static org.elasticsearch.cluster.metadata.MetaDataCreateIndexService.validateIndexOrAliasName; - public class DataFrameAnalyticsDest implements Writeable, ToXContentObject { public static final ParseField INDEX = new ParseField("index"); @@ -94,13 +90,4 @@ public String getIndex() { public String getResultsField() { return resultsField; } - - public void validate() { - if (index != null) { - validateIndexOrAliasName(index, InvalidIndexNameException::new); - if (index.toLowerCase(Locale.ROOT).equals(index) == false) { - throw new InvalidIndexNameException(index, "dest.index must be lowercase"); - } - } - } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidatorTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidatorTests.java index 566925258c879..4eab68271b7e1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidatorTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/validation/SourceDestValidatorTests.java @@ -41,7 +41,9 @@ import org.junit.After; import org.junit.Before; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -52,6 +54,10 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_VERSION_CREATED; import static org.elasticsearch.mock.orig.Mockito.when; +import static org.elasticsearch.xpack.core.common.validation.SourceDestValidator.DESTINATION_IN_SOURCE_VALIDATION; +import static org.elasticsearch.xpack.core.common.validation.SourceDestValidator.DESTINATION_SINGLE_INDEX_VALIDATION; +import static org.elasticsearch.xpack.core.common.validation.SourceDestValidator.REMOTE_SOURCE_VALIDATION; +import static org.elasticsearch.xpack.core.common.validation.SourceDestValidator.SOURCE_MISSING_VALIDATION; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.spy; @@ -67,6 +73,14 @@ public class SourceDestValidatorTests extends ESTestCase { private static final String REMOTE_PLATINUM = "remote-platinum"; private static final ClusterState CLUSTER_STATE; + + private static final List TEST_VALIDATIONS = Arrays.asList( + SOURCE_MISSING_VALIDATION, + DESTINATION_IN_SOURCE_VALIDATION, + DESTINATION_SINGLE_INDEX_VALIDATION, + REMOTE_SOURCE_VALIDATION + ); + private Client clientWithBasicLicense; private Client clientWithExpiredBasicLicense; private Client clientWithPlatinumLicense; @@ -184,13 +198,13 @@ public void closeComponents() throws Exception { ThreadPool.terminate(threadPool, 10, TimeUnit.SECONDS); } - public void testCheck_GivenSimpleSourceIndexAndValidDestIndex() throws InterruptedException { + public void testValidate_GivenSimpleSourceIndexAndValidDestIndex() throws InterruptedException { assertValidation( listener -> simpleNonRemoteValidator.validate( CLUSTER_STATE, new String[] { SOURCE_1 }, "dest", - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), true, @@ -204,7 +218,7 @@ public void testCheck_GivenNoSourceIndexAndValidDestIndex() throws InterruptedEx CLUSTER_STATE, new String[] {}, "dest", - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -221,7 +235,7 @@ public void testCheck_GivenMissingConcreteSourceIndex() throws InterruptedExcept CLUSTER_STATE, new String[] { "missing" }, "dest", - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -236,7 +250,7 @@ public void testCheck_GivenMissingConcreteSourceIndex() throws InterruptedExcept CLUSTER_STATE, new String[] { "missing" }, "dest", - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -250,7 +264,7 @@ public void testCheck_GivenMixedMissingAndExistingConcreteSourceIndex() throws I CLUSTER_STATE, new String[] { SOURCE_1, "missing" }, "dest", - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -265,7 +279,7 @@ public void testCheck_GivenMixedMissingAndExistingConcreteSourceIndex() throws I CLUSTER_STATE, new String[] { SOURCE_1, "missing" }, "dest", - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -279,7 +293,7 @@ public void testCheck_GivenMixedMissingWildcardExistingConcreteSourceIndex() thr CLUSTER_STATE, new String[] { SOURCE_1, "wildcard*", "missing" }, "dest", - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -294,7 +308,7 @@ public void testCheck_GivenMixedMissingWildcardExistingConcreteSourceIndex() thr CLUSTER_STATE, new String[] { SOURCE_1, "wildcard*", "missing" }, "dest", - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -308,7 +322,7 @@ public void testCheck_GivenWildcardSourceIndex() throws InterruptedException { CLUSTER_STATE, new String[] { "wildcard*" }, "dest", - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), true, @@ -322,7 +336,7 @@ public void testCheck_GivenDestIndexSameAsSourceIndex() throws InterruptedExcept CLUSTER_STATE, new String[] { SOURCE_1 }, SOURCE_1, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -340,7 +354,7 @@ public void testCheck_GivenDestIndexSameAsSourceIndex() throws InterruptedExcept CLUSTER_STATE, new String[] { SOURCE_1 }, SOURCE_1, - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -354,7 +368,7 @@ public void testCheck_GivenDestIndexMatchesSourceIndex() throws InterruptedExcep CLUSTER_STATE, new String[] { "source-*" }, SOURCE_2, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -372,7 +386,7 @@ public void testCheck_GivenDestIndexMatchesSourceIndex() throws InterruptedExcep CLUSTER_STATE, new String[] { "source-*" }, SOURCE_2, - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -386,7 +400,7 @@ public void testCheck_GivenDestIndexMatchesOneOfSourceIndices() throws Interrupt CLUSTER_STATE, new String[] { "source-1", "source-*" }, SOURCE_2, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -404,7 +418,7 @@ public void testCheck_GivenDestIndexMatchesOneOfSourceIndices() throws Interrupt CLUSTER_STATE, new String[] { "source-1", "source-*" }, SOURCE_2, - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -418,7 +432,7 @@ public void testCheck_GivenDestIndexMatchesMultipleSourceIndices() throws Interr CLUSTER_STATE, new String[] { "source-1", "source-*", "sou*" }, SOURCE_2, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -442,7 +456,7 @@ public void testCheck_GivenDestIndexIsAliasThatMatchesMultipleIndices() throws I CLUSTER_STATE, new String[] { SOURCE_1 }, DEST_ALIAS, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -464,21 +478,11 @@ public void testCheck_GivenDestIndexIsAliasThatMatchesMultipleIndices() throws I CLUSTER_STATE, new String[] { SOURCE_1 }, DEST_ALIAS, - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), - (Boolean) null, - e -> { - assertEquals(1, e.validationErrors().size()); - assertThat( - e.validationErrors().get(0), - equalTo( - "no write index is defined for alias [dest-alias]. " - + "The write index may be explicitly disabled using is_write_index=false or the alias points " - + "to multiple indices without one being designated as a write index" - ) - ); - } + true, + null ); } @@ -488,7 +492,7 @@ public void testCheck_GivenDestIndexIsAliasThatMatchesMultipleIndicesButHasSingl CLUSTER_STATE, new String[] { SOURCE_1 }, ALIAS_READ_WRITE_DEST, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -514,7 +518,7 @@ public void testCheck_GivenDestIndexIsAliasThatIsIncludedInSource() throws Inter CLUSTER_STATE, new String[] { SOURCE_1 }, SOURCE_1_ALIAS, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, @@ -532,7 +536,7 @@ public void testCheck_GivenDestIndexIsAliasThatIsIncludedInSource() throws Inter CLUSTER_STATE, new String[] { SOURCE_1 }, SOURCE_1_ALIAS, - SourceDestValidator.NON_DEFERABLE_VALIDATIONS, + Collections.emptyList(), listener ), true, @@ -546,7 +550,7 @@ public void testCheck_MultipleValidationErrors() throws InterruptedException { CLUSTER_STATE, new String[] { SOURCE_1, "missing" }, SOURCE_1_ALIAS, - SourceDestValidator.ALL_VALIDATIONS, + TEST_VALIDATIONS, listener ), (Boolean) null, diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDestTests.java index bf8ce4c8a99b0..7332687723805 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/dataframe/DataFrameAnalyticsDestTests.java @@ -7,14 +7,10 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.indices.InvalidIndexNameException; -import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.AbstractSerializingTestCase; import java.io.IOException; -import static org.hamcrest.Matchers.equalTo; - public class DataFrameAnalyticsDestTests extends AbstractSerializingTestCase { @Override @@ -37,19 +33,4 @@ public static DataFrameAnalyticsDest createRandom() { protected Writeable.Reader instanceReader() { return DataFrameAnalyticsDest::new; } - - public void testValidate_GivenIndexWithFunkyChars() { - expectThrows(InvalidIndexNameException.class, () -> new DataFrameAnalyticsDest("