Discontinue archiving broken cluster settings

Currently unknown or invalid cluster settings get archived.

For a better user experience, we stop archving broken cluster settings.
Instead, we will fail to recover the cluster state.
The solution for users in an upgrade case would be to rollback
to the previous version, address the settings that would be unknown
or  invalid the next major version, and then proceed with the upgrade.

Closes #28026

Author: mayya-sharipova

docs/reference/migration/migrate_7_0/cluster.asciidoc

@@ -14,3 +14,9 @@ primary shards of the opened index to be allocated.
 
 ==== Shard preferences `_primary`, `_primary_first`, `_replica`, and `_replica_first` are removed
 These shard preferences are removed in favour of the `_prefer_nodes` and `_only_nodes` preferences.
+
+==== Discontinue archiving broken cluster settings
+We will no longer archive unknown or invalid cluster settings (prepending "archived." to a broken setting's name).
+Instead, we will fail to recover a cluster state with broken cluster settings.
+The solution for users in an upgrade case would be to rollback to the previous version,
+address the settings that would be unknown or invalid in the next major version, and then proceed with the upgrade.

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

@@ -681,6 +681,44 @@ public Settings archiveUnknownOrInvalidSettings(
         }
     }
 
+
+    /**
+     * Checks invalid or unknown settings. Any setting that is not recognized or fails validation
+     * will be processed by consumers.
+     * An exception will be thrown if any invalid or unknown setting is found.
+     *
+     * @param settings        the {@link Settings} instance to scan for unknown or invalid settings
+     * @param unknownConsumer callback on unknown settings (consumer receives unknown key and its
+     *                        associated value)
+     * @param invalidConsumer callback on invalid settings (consumer receives invalid key, its
+     *                        associated value and an exception)
+     */
+    public void checkUnknownOrInvalidSettings(
+            final Settings settings,
+            final Consumer<Map.Entry<String, String>> unknownConsumer,
+            final BiConsumer<Map.Entry<String, String>, IllegalArgumentException> invalidConsumer) {
+        List<String> failedKeys = new ArrayList<>();
+        for (String key : settings.keySet()) {
+            try {
+                Setting<?> setting = get(key);
+                if (setting != null) {
+                    setting.get(settings);
+                } else {
+                    if (!isPrivateSetting(key)) {
+                        failedKeys.add(key);
+                        unknownConsumer.accept(new Entry(key, settings));
+                    }
+                }
+            } catch (IllegalArgumentException ex) {
+                failedKeys.add(key);
+                invalidConsumer.accept(new Entry(key, settings), ex);
+            }
+        }
+        if (failedKeys.size() > 0) {
+            throw new IllegalStateException("Invalid or unknown settings: " + String.join(", ", failedKeys));
+        }
+    }
+
     private static final class Entry implements Map.Entry<String, String> {
 
         private final String key;

server/src/main/java/org/elasticsearch/gateway/Gateway.java

@@ -137,27 +137,25 @@ public void performStateRecovery(final GatewayStateRecoveredListener listener) t
             }
         }
         final ClusterSettings clusterSettings = clusterService.getClusterSettings();
-        metaDataBuilder.persistentSettings(
-            clusterSettings.archiveUnknownOrInvalidSettings(
-                metaDataBuilder.persistentSettings(),
-                e -> logUnknownSetting("persistent", e),
-                (e, ex) -> logInvalidSetting("persistent", e, ex)));
-        metaDataBuilder.transientSettings(
-            clusterSettings.archiveUnknownOrInvalidSettings(
-                metaDataBuilder.transientSettings(),
-                e -> logUnknownSetting("transient", e),
-                (e, ex) -> logInvalidSetting("transient", e, ex)));
+        clusterSettings.checkUnknownOrInvalidSettings(
+            metaDataBuilder.persistentSettings(),
+            e -> logUnknownSetting("persistent", e),
+            (e, ex) -> logInvalidSetting("persistent", e, ex));
+        clusterSettings.checkUnknownOrInvalidSettings(
+            metaDataBuilder.transientSettings(),
+            e -> logUnknownSetting("transient", e),
+            (e, ex) -> logInvalidSetting("transient", e, ex));
         ClusterState.Builder builder = clusterService.newClusterStateBuilder();
         builder.metaData(metaDataBuilder);
         listener.onSuccess(builder.build());
     }
 
     private void logUnknownSetting(String settingType, Map.Entry<String, String> e) {
-        logger.warn("ignoring unknown {} setting: [{}] with value [{}]; archiving", settingType, e.getKey(), e.getValue());
+        logger.warn("unknown {} setting: [{}] with value [{}]", settingType, e.getKey(), e.getValue());
     }
 
     private void logInvalidSetting(String settingType, Map.Entry<String, String> e, IllegalArgumentException ex) {
-        logger.warn(() -> new ParameterizedMessage("ignoring invalid {} setting: [{}] with value [{}]; archiving",
+        logger.warn(() -> new ParameterizedMessage("invalid {} setting: [{}] with value [{}]",
                     settingType, e.getKey(), e.getValue()), ex);
     }
 

server/src/test/java/org/elasticsearch/gateway/GatewayIndexStateIT.java

@@ -49,6 +49,8 @@
 import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
 import org.elasticsearch.test.ESIntegTestCase.Scope;
 import org.elasticsearch.test.InternalTestCluster.RestartCallback;
+import org.junit.Rule;
+import org.junit.rules.ExpectedException;
 
 import java.io.IOException;
 import java.util.List;
@@ -64,6 +66,8 @@
 @ClusterScope(scope = Scope.TEST, numDataNodes = 0)
 public class GatewayIndexStateIT extends ESIntegTestCase {
 
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
     private final Logger logger = Loggers.getLogger(GatewayIndexStateIT.class);
 
     public void testMappingMetaDataParsed() throws Exception {
@@ -479,7 +483,7 @@ public void testRecoverMissingAnalyzer() throws Exception {
         assertThat(ex.getCause().getMessage(), containsString("analyzer [test] not found for field [field1]"));
     }
 
-    public void testArchiveBrokenClusterSettings() throws Exception {
+    public void testFailBrokenClusterSettings() throws Exception {
         logger.info("--> starting one node");
         internalCluster().startNode();
         client().prepareIndex("test", "type1", "1").setSource("field1", "value1").setRefreshPolicy(IMMEDIATE).get();
@@ -502,20 +506,9 @@ public void testArchiveBrokenClusterSettings() throws Exception {
                 .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), "broken").build()).build();
             MetaData.FORMAT.write(brokenMeta, nodeEnv.nodeDataPaths());
         }
-        internalCluster().fullRestart();
-        ensureYellow("test"); // wait for state recovery
-        state = client().admin().cluster().prepareState().get().getState();
-        assertEquals("true", state.metaData().persistentSettings().get("archived.this.is.unknown"));
-        assertEquals("broken", state.metaData().persistentSettings().get("archived."
-            + ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey()));
 
-        // delete these settings
-        client().admin().cluster().prepareUpdateSettings().setPersistentSettings(Settings.builder().putNull("archived.*")).get();
-
-        state = client().admin().cluster().prepareState().get().getState();
-        assertNull(state.metaData().persistentSettings().get("archived.this.is.unknown"));
-        assertNull(state.metaData().persistentSettings().get("archived."
-            + ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey()));
-        assertHitCount(client().prepareSearch().setQuery(matchAllQuery()).get(), 1L);
+        expectedException.expect(ElasticsearchException.class);
+        internalCluster().fullRestart();
     }
+
 }