From e462edc6d6231d4a1b9024ccf0059635d19adbcb Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 14 Sep 2016 11:14:27 -0600 Subject: [PATCH] Validate index settings differently when registering index template This was actually a byproduct of trying to remove a //norelease for index shard setting validation in MetaDataIndexService. This //norelease is now removed. Previously this check was *only* used by the template service, so we validated twice, once in the Settings infrastructure and once when actually creating the index. We now instead use the Settings infrastructure to validate the settings for shard count. --- .../metadata/MetaDataCreateIndexService.java | 9 --------- .../metadata/MetaDataIndexTemplateService.java | 17 ++++++++++++++++- .../put/MetaDataIndexTemplateServiceTests.java | 18 ++++++++++++++---- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 8b3cbec0ebd..f24c1bba3f8 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -500,15 +500,6 @@ public class MetaDataCreateIndexService extends AbstractComponent { validationErrors.add("custom path [" + customPath + "] is not a sub-path of path.shared_data [" + env.sharedDataFile() + "]"); } } - //norelease - this can be removed? - Integer number_of_primaries = settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_SHARDS, null); - Integer number_of_replicas = settings.getAsInt(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, null); - if (number_of_primaries != null && number_of_primaries <= 0) { - validationErrors.add("index must have 1 or more primary shards"); - } - if (number_of_replicas != null && number_of_replicas < 0) { - validationErrors.add("index must have 0 or more replica shards"); - } return validationErrors; } diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java index b88aa7b86ac..4ffcf33097f 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexTemplateService.java @@ -32,6 +32,7 @@ import org.elasticsearch.common.ValidationException; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.Index; @@ -63,15 +64,21 @@ public class MetaDataIndexTemplateService extends AbstractComponent { private final IndicesService indicesService; private final MetaDataCreateIndexService metaDataCreateIndexService; private final NodeServicesProvider nodeServicesProvider; + private final IndexScopedSettings indexScopedSettings; @Inject - public MetaDataIndexTemplateService(Settings settings, ClusterService clusterService, MetaDataCreateIndexService metaDataCreateIndexService, AliasValidator aliasValidator, IndicesService indicesService, NodeServicesProvider nodeServicesProvider) { + public MetaDataIndexTemplateService(Settings settings, ClusterService clusterService, + MetaDataCreateIndexService metaDataCreateIndexService, + AliasValidator aliasValidator, IndicesService indicesService, + NodeServicesProvider nodeServicesProvider, + IndexScopedSettings indexScopedSettings) { super(settings); this.clusterService = clusterService; this.aliasValidator = aliasValidator; this.indicesService = indicesService; this.metaDataCreateIndexService = metaDataCreateIndexService; this.nodeServicesProvider = nodeServicesProvider; + this.indexScopedSettings = indexScopedSettings; } public void removeTemplates(final RemoveRequest request, final RemoveListener listener) { @@ -260,6 +267,14 @@ public class MetaDataIndexTemplateService extends AbstractComponent { validationErrors.add("template must not contain the following characters " + Strings.INVALID_FILENAME_CHARS); } + try { + indexScopedSettings.validate(request.settings); + } catch (IllegalArgumentException iae) { + validationErrors.add(iae.getMessage()); + for (Throwable t : iae.getSuppressed()) { + validationErrors.add(t.getMessage()); + } + } List indexSettingsValidation = metaDataCreateIndexService.getIndexSettingsValidationErrors(request.settings); validationErrors.addAll(indexSettingsValidation); if (!validationErrors.isEmpty()) { diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java index d895ef9cbf5..3c89a6ab744 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/template/put/MetaDataIndexTemplateServiceTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.cluster.metadata.MetaDataCreateIndexService; import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService; import org.elasticsearch.cluster.metadata.MetaDataIndexTemplateService.PutRequest; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.NodeServicesProvider; @@ -54,12 +55,17 @@ public class MetaDataIndexTemplateServiceTests extends ESSingleNodeTestCase { Map map = new HashMap<>(); map.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, "0"); + map.put("index.shard.check_on_startup", "blargh"); request.settings(Settings.builder().put(map).build()); List throwables = putTemplate(request); assertEquals(throwables.size(), 1); assertThat(throwables.get(0), instanceOf(InvalidIndexTemplateException.class)); - assertThat(throwables.get(0).getMessage(), containsString("index must have 1 or more primary shards")); + assertThat(throwables.get(0).getMessage(), + containsString("Failed to parse value [0] for setting [index.number_of_shards] must be >= 1")); + assertThat(throwables.get(0).getMessage(), + containsString("unknown value for [index.shard.check_on_startup] " + + "must be one of [true, false, fix, checksum] but was: blargh")); } public void testIndexTemplateValidationAccumulatesValidationErrors() { @@ -75,7 +81,8 @@ public class MetaDataIndexTemplateServiceTests extends ESSingleNodeTestCase { assertThat(throwables.get(0), instanceOf(InvalidIndexTemplateException.class)); assertThat(throwables.get(0).getMessage(), containsString("name must not contain a space")); assertThat(throwables.get(0).getMessage(), containsString("template must not start with '_'")); - assertThat(throwables.get(0).getMessage(), containsString("index must have 1 or more primary shards")); + assertThat(throwables.get(0).getMessage(), + containsString("Failed to parse value [0] for setting [index.number_of_shards] must be >= 1")); } public void testIndexTemplateWithAliasNameEqualToTemplatePattern() { @@ -160,7 +167,9 @@ public class MetaDataIndexTemplateServiceTests extends ESSingleNodeTestCase { null, null, null, null, null); - MetaDataIndexTemplateService service = new MetaDataIndexTemplateService(Settings.EMPTY, null, createIndexService, new AliasValidator(Settings.EMPTY), null, null); + MetaDataIndexTemplateService service = new MetaDataIndexTemplateService(Settings.EMPTY, null, createIndexService, + new AliasValidator(Settings.EMPTY), null, null, + new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS)); final List throwables = new ArrayList<>(); service.putTemplate(request, new MetaDataIndexTemplateService.PutListener() { @@ -192,7 +201,8 @@ public class MetaDataIndexTemplateServiceTests extends ESSingleNodeTestCase { null, null); MetaDataIndexTemplateService service = new MetaDataIndexTemplateService( - Settings.EMPTY, clusterService, createIndexService, new AliasValidator(Settings.EMPTY), indicesService, nodeServicesProvider); + Settings.EMPTY, clusterService, createIndexService, new AliasValidator(Settings.EMPTY), indicesService, nodeServicesProvider, + new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS)); final List throwables = new ArrayList<>(); final CountDownLatch latch = new CountDownLatch(1);