Snapshot/Restore: add validation of restored persistent settings

Closes #8830

src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java

@@ -186,7 +186,7 @@ public class TransportClusterUpdateSettingsAction extends TransportMasterNodeOpe
                 ImmutableSettings.Builder transientSettings = ImmutableSettings.settingsBuilder();
                 transientSettings.put(currentState.metaData().transientSettings());
                 for (Map.Entry<String, String> entry : request.transientSettings().getAsMap().entrySet()) {
-                    if (dynamicSettings.hasDynamicSetting(entry.getKey()) || entry.getKey().startsWith("logger.")) {
+                    if (dynamicSettings.isDynamicOrLoggingSetting(entry.getKey())) {
                         String error = dynamicSettings.validateDynamicSetting(entry.getKey(), entry.getValue());
                         if (error == null) {
                             transientSettings.put(entry.getKey(), entry.getValue());
@@ -203,7 +203,7 @@ public class TransportClusterUpdateSettingsAction extends TransportMasterNodeOpe
                 ImmutableSettings.Builder persistentSettings = ImmutableSettings.settingsBuilder();
                 persistentSettings.put(currentState.metaData().persistentSettings());
                 for (Map.Entry<String, String> entry : request.persistentSettings().getAsMap().entrySet()) {
-                    if (dynamicSettings.hasDynamicSetting(entry.getKey()) || entry.getKey().startsWith("logger.")) {
+                    if (dynamicSettings.isDynamicOrLoggingSetting(entry.getKey())) {
                         String error = dynamicSettings.validateDynamicSetting(entry.getKey(), entry.getValue());
                         if (error == null) {
                             persistentSettings.put(entry.getKey(), entry.getValue());

src/main/java/org/elasticsearch/cluster/settings/DynamicSettings.java

@@ -31,6 +31,11 @@ public class DynamicSettings {
 
     private ImmutableMap<String, Validator> dynamicSettings = ImmutableMap.of();
 
+
+    public boolean isDynamicOrLoggingSetting(String key) {
+        return hasDynamicSetting(key) || key.startsWith("logger.");
+    }
+
     public boolean hasDynamicSetting(String key) {
         for (String dynamicSetting : dynamicSettings.keySet()) {
             if (Regex.simpleMatch(dynamicSetting, key)) {

src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -34,11 +34,14 @@ import org.elasticsearch.cluster.metadata.RestoreMetaData.ShardRestoreStatus;
 import org.elasticsearch.cluster.routing.*;
 import org.elasticsearch.cluster.routing.allocation.AllocationService;
 import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
+import org.elasticsearch.cluster.settings.ClusterDynamicSettings;
+import org.elasticsearch.cluster.settings.DynamicSettings;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.index.shard.ShardId;
@@ -92,16 +95,20 @@ public class RestoreService extends AbstractComponent implements ClusterStateLis
 
     private final MetaDataCreateIndexService createIndexService;
 
+    private final DynamicSettings dynamicSettings;
+
     private final CopyOnWriteArrayList<ActionListener<RestoreCompletionResponse>> listeners = new CopyOnWriteArrayList<>();
 
     @Inject
-    public RestoreService(Settings settings, ClusterService clusterService, RepositoriesService repositoriesService, TransportService transportService, AllocationService allocationService, MetaDataCreateIndexService createIndexService) {
+    public RestoreService(Settings settings, ClusterService clusterService, RepositoriesService repositoriesService, TransportService transportService,
+                          AllocationService allocationService, MetaDataCreateIndexService createIndexService, @ClusterDynamicSettings DynamicSettings dynamicSettings) {
         super(settings);
         this.clusterService = clusterService;
         this.repositoriesService = repositoriesService;
         this.transportService = transportService;
         this.allocationService = allocationService;
         this.createIndexService = createIndexService;
+        this.dynamicSettings = dynamicSettings;
         transportService.registerHandler(UPDATE_RESTORE_ACTION_NAME, new UpdateRestoreStateRequestHandler());
         clusterService.add(this);
     }
@@ -283,7 +290,24 @@ public class RestoreService extends AbstractComponent implements ClusterStateLis
                 private void restoreGlobalStateIfRequested(MetaData.Builder mdBuilder) {
                     if (request.includeGlobalState()) {
                         if (metaData.persistentSettings() != null) {
-                            mdBuilder.persistentSettings(metaData.persistentSettings());
+                            boolean changed = false;
+                            ImmutableSettings.Builder persistentSettings = ImmutableSettings.settingsBuilder().put();
+                            for (Map.Entry<String, String> entry : metaData.persistentSettings().getAsMap().entrySet()) {
+                                if (dynamicSettings.isDynamicOrLoggingSetting(entry.getKey())) {
+                                    String error = dynamicSettings.validateDynamicSetting(entry.getKey(), entry.getValue());
+                                    if (error == null) {
+                                        persistentSettings.put(entry.getKey(), entry.getValue());
+                                        changed = true;
+                                    } else {
+                                        logger.warn("ignoring persistent setting [{}], [{}]", entry.getKey(), error);
+                                    }
+                                } else {
+                                    logger.warn("ignoring persistent setting [{}], not dynamically updateable", entry.getKey());
+                                }
+                            }
+                            if (changed) {
+                                mdBuilder.persistentSettings(persistentSettings.build());
+                            }
                         }
                         if (metaData.templates() != null) {
                             // TODO: Should all existing templates be deleted first?

src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreTests.java

@@ -51,11 +51,12 @@ import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.discovery.zen.elect.ElectMasterService;
 import org.elasticsearch.index.store.support.AbstractIndexStore;
+import org.elasticsearch.indices.ttl.IndicesTTLService;
 import org.elasticsearch.repositories.RepositoryMissingException;
 import org.elasticsearch.snapshots.mockstore.MockRepositoryModule;
 import org.elasticsearch.test.InternalTestCluster;
-import org.elasticsearch.threadpool.ThreadPool;
 import org.junit.Ignore;
 import org.junit.Test;
 
@@ -66,6 +67,7 @@ import java.util.EnumSet;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
@@ -82,16 +84,29 @@ public class DedicatedClusterSnapshotRestoreTests extends AbstractSnapshotTests
 
     @Test
     public void restorePersistentSettingsTest() throws Exception {
-        logger.info("--> start node");
+        logger.info("--> start 2 nodes");
-        internalCluster().startNode(settingsBuilder().put("gateway.type", "local"));
+        Settings nodeSettings = settingsBuilder()
+                .put("discovery.type", "zen")
+                .put("discovery.zen.ping_timeout", "200ms")
+                .put("discovery.initial_state_timeout", "500ms")
+                .build();
+        internalCluster().startNode(nodeSettings);
         Client client = client();
+        String secondNode = internalCluster().startNode(nodeSettings);
+
+        int random = randomIntBetween(10, 42);
 
-        // Add dummy persistent setting
         logger.info("--> set test persistent setting");
-        String settingValue = "test-" + randomInt();
+        client.admin().cluster().prepareUpdateSettings().setPersistentSettings(
-        client.admin().cluster().prepareUpdateSettings().setPersistentSettings(ImmutableSettings.settingsBuilder().put(ThreadPool.THREADPOOL_GROUP + "dummy.value", settingValue)).execute().actionGet();
+                ImmutableSettings.settingsBuilder()
+                        .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, 2)
+                        .put(IndicesTTLService.INDICES_TTL_INTERVAL, random, TimeUnit.MINUTES))
+                .execute().actionGet();
+
         assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState()
-                .getMetaData().persistentSettings().get(ThreadPool.THREADPOOL_GROUP + "dummy.value"), equalTo(settingValue));
+                .getMetaData().persistentSettings().getAsTime(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)).millis(), equalTo(TimeValue.timeValueMinutes(random).millis()));
+        assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState()
+                .getMetaData().persistentSettings().getAsInt(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, -1), equalTo(2));
 
         logger.info("--> create repository");
         PutRepositoryResponse putRepositoryResponse = client.admin().cluster().preparePutRepository("test-repo")
@@ -105,14 +120,25 @@ public class DedicatedClusterSnapshotRestoreTests extends AbstractSnapshotTests
         assertThat(client.admin().cluster().prepareGetSnapshots("test-repo").setSnapshots("test-snap").execute().actionGet().getSnapshots().get(0).state(), equalTo(SnapshotState.SUCCESS));
 
         logger.info("--> clean the test persistent setting");
-        client.admin().cluster().prepareUpdateSettings().setPersistentSettings(ImmutableSettings.settingsBuilder().put(ThreadPool.THREADPOOL_GROUP + "dummy.value", "")).execute().actionGet();
+        client.admin().cluster().prepareUpdateSettings().setPersistentSettings(
+                ImmutableSettings.settingsBuilder()
+                        .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, 1)
+                        .put(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)))
+                .execute().actionGet();
         assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState()
-                .getMetaData().persistentSettings().get(ThreadPool.THREADPOOL_GROUP + "dummy.value"), equalTo(""));
+                .getMetaData().persistentSettings().getAsTime(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)).millis(), equalTo(TimeValue.timeValueMinutes(1).millis()));
+
+        stopNode(secondNode);
+        assertThat(client.admin().cluster().prepareHealth().setWaitForNodes("1").get().isTimedOut(), equalTo(false));
 
         logger.info("--> restore snapshot");
         client.admin().cluster().prepareRestoreSnapshot("test-repo", "test-snap").setRestoreGlobalState(true).setWaitForCompletion(true).execute().actionGet();
         assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState()
-                .getMetaData().persistentSettings().get(ThreadPool.THREADPOOL_GROUP + "dummy.value"), equalTo(settingValue));
+                .getMetaData().persistentSettings().getAsTime(IndicesTTLService.INDICES_TTL_INTERVAL, TimeValue.timeValueMinutes(1)).millis(), equalTo(TimeValue.timeValueMinutes(random).millis()));
+
+        logger.info("--> ensure that zen discovery minimum master nodes wasn't restored");
+        assertThat(client.admin().cluster().prepareState().setRoutingTable(false).setNodes(false).execute().actionGet().getState()
+                .getMetaData().persistentSettings().getAsInt(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES, -1), not(equalTo(2)));
     }
 
     @Test