Do not open indices with broken settings

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);

core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

@@ -264,17 +264,41 @@ public synchronized <T> void addSettingsUpdateConsumer(Setting<T> setting, Consu
     }
 
     /**
-     * Validates that all given settings are registered and valid
-     * @param settings the settings to validate
-     * @param validateDependencies if <code>true</code> 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<RuntimeException> 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<RuntimeException> 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);
             }
         }

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<Setting<?>> settingsSet) {
         super(settings, settingsSet, Property.IndexScope);

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<IndexEventListener> 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);

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]")));
+    }
+
 }

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(