diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java index 2ff5fd5c2b217..d58ed04a930f9 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java @@ -82,7 +82,6 @@ public MetaDataIndexUpgradeService(Settings settings, NamedXContentRegistry xCon public IndexMetaData upgradeIndexMetaData(IndexMetaData indexMetaData, Version minimumIndexCompatibilityVersion) { // Throws an exception if there are too-old segments: if (isUpgraded(indexMetaData)) { - assert indexMetaData == archiveBrokenIndexSettings(indexMetaData) : "all settings must have been upgraded before"; return indexMetaData; } checkSupportedVersion(indexMetaData, minimumIndexCompatibilityVersion); diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index f952eb36a0de8..e2f4d7697b62d 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -264,17 +264,41 @@ public synchronized void addSettingsUpdateConsumer(Setting setting, Consu } /** - * Validates that all given settings are registered and valid - * @param settings the settings to validate - * @param validateDependencies if true settings dependencies are validated as well. + * Validates that all settings are registered and valid. + * + * @param settings the settings to validate + * @param validateDependencies true if dependent settings should be validated * @see Setting#getSettingsDependencies(String) */ - public final void validate(Settings settings, boolean validateDependencies) { - List exceptions = new ArrayList<>(); - for (String key : settings.keySet()) { // settings iterate in deterministic fashion + public final void validate(final Settings settings, final boolean validateDependencies) { + validate(settings, validateDependencies, false, false); + } + + /** + * Validates that all settings are registered and valid. + * + * @param settings the settings + * @param validateDependencies true if dependent settings should be validated + * @param ignorePrivateSettings true if private settings should be ignored during validation + * @param ignoreArchivedSettings true if archived settings should be ignored during validation + * @see Setting#getSettingsDependencies(String) + */ + public final void validate( + final Settings settings, + final boolean validateDependencies, + final boolean ignorePrivateSettings, + final boolean ignoreArchivedSettings) { + final List exceptions = new ArrayList<>(); + for (final String key : settings.keySet()) { // settings iterate in deterministic fashion + if (isPrivateSetting(key) && ignorePrivateSettings) { + continue; + } + if (key.startsWith(ARCHIVED_SETTINGS_PREFIX) && ignoreArchivedSettings) { + continue; + } try { validate(key, settings, validateDependencies); - } catch (RuntimeException ex) { + } catch (final RuntimeException ex) { exceptions.add(ex); } } diff --git a/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java index ddbb8b83d133f..83de9ae377964 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java @@ -169,8 +169,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings { ))); - public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, - BUILT_IN_INDEX_SETTINGS); + public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS); public IndexScopedSettings(Settings settings, Set> settingsSet) { super(settings, settingsSet, Property.IndexScope); diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesService.java b/core/src/main/java/org/elasticsearch/indices/IndicesService.java index 314b46b4d08e0..7e0bff5384183 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -152,7 +152,7 @@ public class IndicesService extends AbstractLifecycleComponent private final TimeValue shardsClosedTimeout; private final AnalysisRegistry analysisRegistry; private final IndexNameExpressionResolver indexNameExpressionResolver; - private final IndexScopedSettings indexScopeSetting; + private final IndexScopedSettings indexScopedSettings; private final IndicesFieldDataCache indicesFieldDataCache; private final CacheCleaner cacheCleaner; private final ThreadPool threadPool; @@ -198,7 +198,7 @@ public IndicesService(Settings settings, PluginsService pluginsService, NodeEnvi indexingMemoryController = new IndexingMemoryController(settings, threadPool, // ensure we pull an iter with new shards - flatten makes a copy () -> Iterables.flatten(this).iterator()); - this.indexScopeSetting = indexScopedSettings; + this.indexScopedSettings = indexScopedSettings; this.circuitBreakerService = circuitBreakerService; this.bigArrays = bigArrays; this.scriptService = scriptService; @@ -432,7 +432,9 @@ private synchronized IndexService createIndexService(final String reason, IndicesFieldDataCache indicesFieldDataCache, List builtInListeners, IndexingOperationListener... indexingOperationListeners) throws IOException { - final IndexSettings idxSettings = new IndexSettings(indexMetaData, this.settings, indexScopeSetting); + final IndexSettings idxSettings = new IndexSettings(indexMetaData, this.settings, indexScopedSettings); + // we ignore private settings since they are not registered settings + indexScopedSettings.validate(indexMetaData.getSettings(), true, true, true); logger.debug("creating Index [{}], shards [{}]/[{}] - reason [{}]", indexMetaData.getIndex(), idxSettings.getNumberOfShards(), @@ -470,7 +472,7 @@ private synchronized IndexService createIndexService(final String reason, * Note: the returned {@link MapperService} should be closed when unneeded. */ public synchronized MapperService createIndexMapperService(IndexMetaData indexMetaData) throws IOException { - final IndexSettings idxSettings = new IndexSettings(indexMetaData, this.settings, indexScopeSetting); + final IndexSettings idxSettings = new IndexSettings(indexMetaData, this.settings, indexScopedSettings); final IndexModule indexModule = new IndexModule(idxSettings, analysisRegistry); pluginsService.onIndexModule(indexModule); return indexModule.newIndexMapperService(xContentRegistry, mapperRegistry, scriptService); diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java b/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java index 5385902bc3690..7477f11601f54 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexIT.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.create; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.UnavailableShardsException; import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse; @@ -32,6 +33,7 @@ import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.query.RangeQueryBuilder; import org.elasticsearch.test.ESIntegTestCase; @@ -51,6 +53,8 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasToString; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.core.IsNull.notNullValue; @@ -344,4 +348,37 @@ public void testIndexNameInResponse() { assertEquals("Should have index name in response", "foo", response.index()); } + + public void testIndexWithUnknownSetting() throws Exception { + final int replicas = internalCluster().numDataNodes() - 1; + final Settings settings = Settings.builder().put("index.number_of_shards", 1).put("index.number_of_replicas", replicas).build(); + client().admin().indices().prepareCreate("test").setSettings(settings).get(); + ensureGreen("test"); + final ClusterState state = client().admin().cluster().prepareState().get().getState(); + final IndexMetaData metaData = state.getMetaData().index("test"); + for (final NodeEnvironment services : internalCluster().getInstances(NodeEnvironment.class)) { + final IndexMetaData brokenMetaData = + IndexMetaData + .builder(metaData) + .settings(Settings.builder().put(metaData.getSettings()).put("index.foo", "true")) + .build(); + // so evil + IndexMetaData.FORMAT.write(brokenMetaData, services.indexPaths(brokenMetaData.getIndex())); + } + internalCluster().fullRestart(); + ensureGreen(metaData.getIndex().getName()); // we have to wait for the index to show up in the metadata or we will fail in a race + final ClusterState stateAfterRestart = client().admin().cluster().prepareState().get().getState(); + + // the index should not be open after we restart and recover the broken index metadata + assertThat(stateAfterRestart.getMetaData().index(metaData.getIndex()).getState(), equalTo(IndexMetaData.State.CLOSE)); + + // try to open the index + final ElasticsearchException e = + expectThrows(ElasticsearchException.class, () -> client().admin().indices().prepareOpen("test").get()); + assertThat(e, hasToString(containsString("Failed to verify index " + metaData.getIndex()))); + assertNotNull(e.getCause()); + assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); + assertThat(e, hasToString(containsString("unknown setting [index.foo]"))); + } + } diff --git a/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java b/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java index 79c306f43f151..1149111f6134d 100644 --- a/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/index/IndexSettingsTests.java @@ -21,6 +21,7 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.settings.AbstractScopedSettings; import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -444,6 +445,55 @@ public void testTranslogGenerationSizeThreshold() { assertEquals(actual, settings.getGenerationThresholdSize()); } + public void testPrivateSettingsValidation() { + final Settings settings = Settings.builder().put(IndexMetaData.SETTING_CREATION_DATE, System.currentTimeMillis()).build(); + final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); + + { + // validation should fail since we are not ignoring private settings + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> indexScopedSettings.validate(settings, randomBoolean())); + assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); + } + + { + // validation should fail since we are not ignoring private settings + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> indexScopedSettings.validate(settings, randomBoolean(), false, randomBoolean())); + assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); + } + + // nothing should happen since we are ignoring private settings + indexScopedSettings.validate(settings, randomBoolean(), true, randomBoolean()); + } + + public void testArchivedSettingsValidation() { + final Settings settings = + Settings.builder().put(AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX + "foo", System.currentTimeMillis()).build(); + final IndexScopedSettings indexScopedSettings = new IndexScopedSettings(settings, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); + + { + // validation should fail since we are not ignoring archived settings + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> indexScopedSettings.validate(settings, randomBoolean())); + assertThat(e, hasToString(containsString("unknown setting [archived.foo]"))); + } + + { + // validation should fail since we are not ignoring archived settings + final IllegalArgumentException e = expectThrows( + IllegalArgumentException.class, + () -> indexScopedSettings.validate(settings, randomBoolean(), randomBoolean(), false)); + assertThat(e, hasToString(containsString("unknown setting [archived.foo]"))); + } + + // nothing should happen since we are ignoring archived settings + indexScopedSettings.validate(settings, randomBoolean(), randomBoolean(), true); + } + public void testArchiveBrokenIndexSettings() { Settings settings = IndexScopedSettings.DEFAULT_SCOPED_SETTINGS.archiveUnknownOrInvalidSettings(