Discontinue archiving broken cluster settings

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