Do not open indices with broken settings

Today we are lenient and we open an index if it has broken
settings. This can happen if a user installs a plugin that registers an
index setting, creates an index with that setting, stop their node,
removes the plugin, and then restarts the node. In this case, the index
will have a setting that we do not recognize yet we open the index
anyway. This leniency is dangerous so this commit removes it. Note that
we still are lenient on upgrades and we should really reconsider this in
a follow-up.

Relates #26995

core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexUpgradeService.java

@@ -82,7 +82,6 @@ public class MetaDataIndexUpgradeService extends AbstractComponent {
     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 abstract class AbstractScopedSettings extends AbstractComponent {
     }
 
     /**
-     * Validates that all given settings are registered and valid
+     * Validates that all settings are registered and valid.
-     * @param settings the settings to validate
+     *
-     * @param validateDependencies if <code>true</code> settings dependencies are validated as well.
+     * @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) {
+    public final void validate(final Settings settings, final boolean validateDependencies) {
-        List<RuntimeException> exceptions = new ArrayList<>();
+        validate(settings, validateDependencies, false, false);
-        for (String key : settings.keySet()) { // settings iterate in deterministic fashion
+    }
+
+    /**
+     * 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,
+    public static final IndexScopedSettings DEFAULT_SCOPED_SETTINGS = new IndexScopedSettings(Settings.EMPTY, BUILT_IN_INDEX_SETTINGS);
-            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 class IndicesService extends AbstractLifecycleComponent
         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 @@ public class IndicesService extends AbstractLifecycleComponent
                                                          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 @@ public class IndicesService extends AbstractLifecycleComponent
      * 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.cluster.metadata.MetaData;
 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.allOf;
 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 class CreateIndexIT extends ESIntegTestCase {
 
         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 @@ package org.elasticsearch.index;
 
 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 class IndexSettingsTests extends ESTestCase {
         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(