Block restore on mis-matching replication type setting

Signed-off-by: Suraj Singh <[email protected]>

server/src/internalClusterTest/java/org/opensearch/indices/replication/SegmentReplicationClusterSettingIT.java

@@ -31,7 +31,7 @@
 import java.util.function.BiConsumer;
 
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
-import static org.opensearch.indices.IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING;
+import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING;
 import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
 import static org.hamcrest.Matchers.hasSize;
@@ -346,7 +346,7 @@ private void executeTest(
 
         Settings settings = Settings.builder()
             .put(CLUSTER_SETTING_REPLICATION_TYPE, clusterLevelReplication)
-            .put(CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey(), restrictIndexLevelReplicationTypeSetting)
+            .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), restrictIndexLevelReplicationTypeSetting)
             .build();
         internalCluster().startClusterManagerOnlyNode(settings);
         final String dataNodeOne = internalCluster().startDataOnlyNode(settings);

server/src/internalClusterTest/java/org/opensearch/snapshots/SegmentReplicationSnapshotIT.java

@@ -31,7 +31,7 @@
 import java.util.concurrent.TimeUnit;
 
 import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_REPLICATION_TYPE;
-import static org.opensearch.indices.IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING;
+import static org.opensearch.indices.IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING;
 import static org.opensearch.indices.IndicesService.CLUSTER_SETTING_REPLICATION_TYPE;
 import static org.opensearch.indices.replication.SegmentReplicationBaseIT.waitForSearchableDocs;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
@@ -48,6 +48,9 @@ public class SegmentReplicationSnapshotIT extends AbstractSnapshotIntegTestCase
     private static final String REPOSITORY_NAME = "test-segrep-repo";
     private static final String SNAPSHOT_NAME = "test-segrep-snapshot";
 
+    protected static final String REPLICATION_MISMATCH_VALIDATION_ERROR =
+        "Validation Failed: 1: index setting [index.replication.type] is not allowed to be set as [cluster.force.index.replication_type=true];";
+
     public Settings segRepEnableIndexSettings() {
         return getShardSettings().put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT).build();
     }
@@ -327,6 +330,7 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception {
                 // we want to override cluster replication setting by passing a index replication setting
                 .put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.DOCUMENT)
                 .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, REPLICA_COUNT)
+                .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, SHARD_COUNT)
         ).get();
         ensureYellowAndNoInitializingShards(INDEX_NAME);
         final String replicaNode = internalCluster().startDataOnlyNode();
@@ -345,8 +349,10 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception {
         // Start new set of nodes with cluster level replication type setting and restrict replication type setting.
         Settings settings = Settings.builder()
             .put(CLUSTER_SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT)
-            .put(CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey(), true)
+            .put(CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey(), true)
             .build();
+
+        // Start new cluster manager node
         String newClusterManagerNode = internalCluster().startClusterManagerOnlyNode(settings);
 
         // Remove older nodes
@@ -357,21 +363,10 @@ public void testSnapshotRestoreOnRestrictReplicationSetting() throws Exception {
         String newPrimaryNode = internalCluster().startDataOnlyNode(settings);
         String newReplicaNode = internalCluster().startDataOnlyNode(settings);
 
-        RestoreSnapshotResponse restoreSnapshotResponse = restoreSnapshotWithSettings(restoreIndexSegRepSettings());
-
-        // Assertions
-        assertEquals(restoreSnapshotResponse.status(), RestStatus.ACCEPTED);
-        ensureGreen(RESTORED_INDEX_NAME);
-        logger.info("--> ClusterState {}", client().admin().cluster().prepareState().execute().actionGet().getState());
-        GetSettingsResponse settingsResponse = client().admin()
-            .indices()
-            .getSettings(new GetSettingsRequest().indices(RESTORED_INDEX_NAME).includeDefaults(true))
-            .get();
-        assertEquals(settingsResponse.getSetting(RESTORED_INDEX_NAME, SETTING_REPLICATION_TYPE), ReplicationType.DOCUMENT.toString());
+        // Perform snapshot restore
+        logger.info("--> Performing snapshot restore to target index");
 
-        // Verify index setting isSegRepEnabled.
-        Index index = resolveIndex(RESTORED_INDEX_NAME);
-        IndicesService indicesService = internalCluster().getInstance(IndicesService.class);
-        assertEquals(indicesService.indexService(index).getIndexSettings().isSegRepEnabled(), false);
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> restoreSnapshotWithSettings(null));
+        assertEquals(REPLICATION_MISMATCH_VALIDATION_ERROR, exception.getMessage());
     }
 }

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java

@@ -907,8 +907,6 @@ static Settings aggregateIndexSettings(
             );
         }
 
-        // Validate index settings here to validate replication type where replication type is updated via templates or
-        // resize index requests
         List<String> validationErrors = new ArrayList<>();
         validateIndexReplicationTypeSettings(indexSettingsBuilder.build(), clusterSettings).ifPresent(validationErrors::add);
         validateErrors(request.index(), validationErrors);
@@ -1246,19 +1244,13 @@ private static void validateActiveShardCount(ActiveShardCount waitForActiveShard
 
     private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) {
         validateIndexName(request.index(), state);
-        validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings, false);
+        validateIndexSettings(request.index(), request.settings(), forbidPrivateIndexSettings);
     }
 
-    public void validateIndexSettings(
-        String indexName,
-        final Settings settings,
-        final boolean forbidPrivateIndexSettings,
-        final boolean ignoreRestrictReplicationTypeSetting
-    ) throws IndexCreationException {
-        List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, Optional.of(indexName));
-        if (ignoreRestrictReplicationTypeSetting == false) {
-            validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add);
-        }
+    public void validateIndexSettings(String indexName, final Settings settings, final boolean forbidPrivateIndexSettings)
+        throws IndexCreationException {
+        List<String> validationErrors = getIndexSettingsValidationErrors(settings, forbidPrivateIndexSettings, indexName);
+        validateIndexReplicationTypeSettings(settings, clusterService.getClusterSettings()).ifPresent(validationErrors::add);
         validateErrors(indexName, validationErrors);
     }
 
@@ -1346,13 +1338,13 @@ private static List<String> validateIndexCustomPath(Settings settings, @Nullable
      * @param clusterSettings cluster setting
      */
     private static Optional<String> validateIndexReplicationTypeSettings(Settings requestSettings, ClusterSettings clusterSettings) {
-        if (clusterSettings.get(IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING)
+        if (clusterSettings.get(IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING)
             && requestSettings.hasValue(SETTING_REPLICATION_TYPE)
             && requestSettings.get(INDEX_REPLICATION_TYPE_SETTING.getKey())
                 .equals(clusterSettings.get(CLUSTER_REPLICATION_TYPE_SETTING).name()) == false) {
             return Optional.of(
                 "index setting [index.replication.type] is not allowed to be set as ["
-                    + IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING.getKey()
+                    + IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING.getKey()
                     + "=true]"
             );
         }

server/src/main/java/org/opensearch/common/settings/ClusterSettings.java

@@ -702,7 +702,7 @@ public void apply(Settings value, Settings current, Settings previous) {
                 CpuBasedAdmissionControllerSettings.CPU_BASED_ADMISSION_CONTROLLER_TRANSPORT_LAYER_MODE,
                 CpuBasedAdmissionControllerSettings.INDEXING_CPU_USAGE_LIMIT,
                 CpuBasedAdmissionControllerSettings.SEARCH_CPU_USAGE_LIMIT,
-                IndicesService.CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING
+                IndicesService.CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING
             )
         )
     );

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

@@ -306,7 +306,7 @@ public class IndicesService extends AbstractLifecycleComponent
      * with the replication type index setting, set at cluster level i.e. 'cluster.indices.replication.strategy'.
      * If disabled, the replication type can be specified.
      */
-    public static final Setting<Boolean> CLUSTER_RESTRICT_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting(
+    public static final Setting<Boolean> CLUSTER_FORCE_INDEX_REPLICATION_TYPE_SETTING = Setting.boolSetting(
         "cluster.force.index.replication_type",
         false,
         Property.NodeScope,

server/src/main/java/org/opensearch/snapshots/RestoreService.java

@@ -428,12 +428,7 @@ public ClusterState execute(ClusterState currentState) {
                                     boolean isHidden = IndexMetadata.INDEX_HIDDEN_SETTING.get(snapshotIndexMetadata.getSettings());
                                     createIndexService.validateIndexName(renamedIndexName, currentState);
                                     createIndexService.validateDotIndex(renamedIndexName, isHidden);
-                                    createIndexService.validateIndexSettings(
-                                        renamedIndexName,
-                                        snapshotIndexMetadata.getSettings(),
-                                        false,
-                                        true
-                                    );
+                                    createIndexService.validateIndexSettings(renamedIndexName, snapshotIndexMetadata.getSettings(), false);
                                     MetadataCreateIndexService.validateRefreshIntervalSettings(
                                         snapshotIndexMetadata.getSettings(),
                                         clusterSettings