Allow legacy index settings on legacy indices

docs/changelog/90264.yaml

@@ -0,0 +1,6 @@
+pr: 90264
+summary: Allow legacy index settings on legacy indices
+area: Infra/Core
+type: enhancement
+issues:
+ - 84992

qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/UpgradeWithOldIndexSettingsIT.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.upgrades;
+
+import org.apache.http.util.EntityUtils;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+
+import static org.elasticsearch.rest.action.search.RestSearchAction.TOTAL_HITS_AS_INT_PARAM;
+import static org.hamcrest.Matchers.is;
+
+public class UpgradeWithOldIndexSettingsIT extends AbstractRollingTestCase {
+
+    private static final String INDEX_NAME = "test_index_old_settings";
+    private static final String EXPECTED_WARNING = "[index.indexing.slowlog.level] setting was deprecated in Elasticsearch and will "
+        + "be removed in a future release! See the breaking changes documentation for the next major version.";
+
+    @SuppressWarnings("unchecked")
+    public void testOldIndexSettings() throws Exception {
+        switch (CLUSTER_TYPE) {
+            case OLD -> {
+                // create index with settings no longer valid
+                Request createTestIndex = new Request("PUT", "/" + INDEX_NAME);
+                createTestIndex.setJsonEntity("{\"settings\": {\"index.indexing.slowlog.level\": \"INFO\"}}");
+                createTestIndex.setOptions(expectWarnings(EXPECTED_WARNING));
+                client().performRequest(createTestIndex);
+
+                // add some data
+                Request bulk = new Request("POST", "/_bulk");
+                bulk.addParameter("refresh", "true");
+                bulk.setOptions(expectWarnings(EXPECTED_WARNING));
+                bulk.setJsonEntity(String.format(Locale.ROOT, """
+                    {"index": {"_index": "%s"}}
+                    {"f1": "v1", "f2": "v2"}
+                    """, INDEX_NAME));
+                client().performRequest(bulk);
+            }
+            case MIXED -> {
+                // add some more data
+                Request bulk = new Request("POST", "/_bulk");
+                bulk.addParameter("refresh", "true");
+                bulk.setOptions(expectWarnings(EXPECTED_WARNING));
+                bulk.setJsonEntity(String.format(Locale.ROOT, """
+                    {"index": {"_index": "%s"}}
+                    {"f1": "v3", "f2": "v4"}
+                    """, INDEX_NAME));
+                client().performRequest(bulk);
+            }
+            case UPGRADED -> {
+                Request indexSettingsRequest = new Request("GET", "/" + INDEX_NAME + "/_settings");
+                Map<String, Object> response = entityAsMap(client().performRequest(indexSettingsRequest));
+
+                var slowLog = (Map<?, ?>) ((List<?>) (XContentMapValues.extractValue("settings.index.indexing.slowlog", response))).get(0);
+
+                // Make sure our non-system index is still non-system
+                assertThat(slowLog.get("level"), is("INFO"));
+                assertCount(INDEX_NAME, 2);
+            }
+        }
+    }
+
+    private void assertCount(String index, int count) throws IOException {
+        Request searchTestIndexRequest = new Request("POST", "/" + index + "/_search");
+        searchTestIndexRequest.addParameter(TOTAL_HITS_AS_INT_PARAM, "true");
+        searchTestIndexRequest.addParameter("filter_path", "hits.total");
+        Response searchTestIndexResponse = client().performRequest(searchTestIndexRequest);
+        assertEquals(
+            "{\"hits\":{\"total\":" + count + "}}",
+            EntityUtils.toString(searchTestIndexResponse.getEntity(), StandardCharsets.UTF_8)
+        );
+    }
+}

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

@@ -491,10 +491,41 @@ public final void validate(
         final boolean ignorePrivateSettings,
         final boolean ignoreArchivedSettings,
         final boolean validateInternalOrPrivateIndex
+    ) {
+        validate(settings, validateValues, ignorePrivateSettings, ignoreArchivedSettings, validateInternalOrPrivateIndex, false);
+    }
+
+    /**
+     * Validates that all registered settings are valid, but not necessarily registered unless asked for
+     *
+     * <p>
+     * We ignore unregistered settings in validation of index settings on legacy indices, e.g. with major one
+     * behind our current major. When settings are missing registration we simply ignore them,
+     * otherwise we won't be able to perform rolling upgrades.
+     * Protected on purpose, because it's used only by IndexScopedSettings.
+     *
+     * @param settings                       the settings
+     * @param validateValues                 true if values should be validated, otherwise only keys are validated
+     * @param ignorePrivateSettings          true if private settings should be ignored during validation
+     * @param ignoreArchivedSettings         true if archived settings should be ignored during validation
+     * @param validateInternalOrPrivateIndex true if index internal settings should be validated
+     * @param ignoreUnregistered             true if we should ignore the unregistered settings
+     * @see Setting#getSettingsDependencies(String)
+     */
+    protected final void validate(
+        final Settings settings,
+        final boolean validateValues,
+        final boolean ignorePrivateSettings,
+        final boolean ignoreArchivedSettings,
+        final boolean validateInternalOrPrivateIndex,
+        final boolean ignoreUnregistered
     ) {
         final List<RuntimeException> exceptions = new ArrayList<>();
         for (final String key : settings.keySet()) { // settings iterate in deterministic fashion
             final Setting<?> setting = getRaw(key);
+            if (setting == null && ignoreUnregistered) {
+                continue;
+            }
             if (((isPrivateSetting(key) || (setting != null && setting.isPrivateIndex())) && ignorePrivateSettings)) {
                 continue;
             }

server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java

@@ -7,6 +7,7 @@
  */
 package org.elasticsearch.common.settings;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MetadataIndexStateService;
 import org.elasticsearch.cluster.routing.UnassignedInfo;
@@ -225,4 +226,22 @@ public boolean isPrivateSetting(String key) {
                 return IndexMetadata.INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING.getRawKey().match(key);
         }
     }
+
+    /**
+     * Validates that all registered settings are valid, but not necessarily registered
+     *
+     * <p>
+     * This is used for validation of index settings on legacy indices, e.g. with major one
+     * behind our current major. When settings are missing registration we simply ignore them,
+     * otherwise we won't be able to perform rolling upgrades. This method is not to be used on
+     * the current major.
+     *
+     * @param indexMetadata the index metadata
+     * @see Setting#getSettingsDependencies(String)
+     */
+    public void validateLegacySettings(final IndexMetadata indexMetadata) {
+        assert indexMetadata.getCreationVersion().major < Version.CURRENT.major;
+
+        validate(indexMetadata.getSettings(), true, true, true, false, true);
+    }
 }

server/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -699,8 +699,13 @@ private synchronized IndexService createIndexService(
         IndexingOperationListener... indexingOperationListeners
     ) throws IOException {
         final IndexSettings idxSettings = new IndexSettings(indexMetadata, settings, indexScopedSettings);
-        // we ignore private settings since they are not registered settings
-        indexScopedSettings.validate(indexMetadata.getSettings(), true, true, true);
+        if (indexMetadata.getCreationVersion().major < Version.CURRENT.major) {
+            // we ignore unregistered settings altogether because we might be dealing with legacy settings
+            indexScopedSettings.validateLegacySettings(indexMetadata);
+        } else {
+            // 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(),