Filter deprecated settings in CreateIndexFromSourceAction (elastic#120163)

When creating a destination index, filter any deprecated settings before copying the source settings to the dest index. Specifically, filter out settings with the property IndexSettingDeprecatedInV7AndRemovedInV8 or settings from a specific list  which do not have this property but have no effect in v8.

This also includes a change which reverts the changes that elastic#116996 made to MetadataCreateIndexService. That change factored the method copySettingsFromSource out of prepareResizeIndexSettings so it could be used to filter settings in CreateIndexFromSourceAction. The additional filtering of deprecated property caused the requirements in CreateIndexFromSourceAction and MetadataCreateIndexService to diverge. So this change reverts the original refactor to MetadataCreateIndexService and adds a method similar to copySettingsFromSource to CreateIndexFromSourceAction.

Author: parkertimmins

docs/changelog/120163.yaml

@@ -0,0 +1,5 @@
+pr: 120163
+summary: Filter deprecated settings when making dest index
+area: Data streams
+type: enhancement
+issues: []

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

@@ -1670,11 +1670,23 @@ static void prepareResizeIndexSettings(
             throw new IllegalStateException("unknown resize type is " + type);
         }
 
-        final Settings.Builder builder;
+        final Settings.Builder builder = Settings.builder();
         if (copySettings) {
-            builder = copySettingsFromSource(true, sourceMetadata.getSettings(), indexScopedSettings, indexSettingsBuilder);
+            // copy all settings and non-copyable settings and settings that have already been set (e.g., from the request)
+            for (final String key : sourceMetadata.getSettings().keySet()) {
+                final Setting<?> setting = indexScopedSettings.get(key);
+                if (setting == null) {
+                    assert indexScopedSettings.isPrivateSetting(key) : key;
+                } else if (setting.getProperties().contains(Setting.Property.NotCopyableOnResize)) {
+                    continue;
+                }
+                // do not override settings that have already been set (for example, from the request)
+                if (indexSettingsBuilder.keys().contains(key)) {
+                    continue;
+                }
+                builder.copy(key, sourceMetadata.getSettings());
+            }
         } else {
-            builder = Settings.builder();
             final Predicate<String> sourceSettingsPredicate = (s) -> (s.startsWith("index.similarity.")
                 || s.startsWith("index.analysis.")
                 || s.startsWith("index.sort.")
@@ -1692,36 +1704,6 @@ static void prepareResizeIndexSettings(
         }
     }
 
-    public static Settings.Builder copySettingsFromSource(
-        boolean copyPrivateSettings,
-        Settings sourceSettings,
-        IndexScopedSettings indexScopedSettings,
-        Settings.Builder indexSettingsBuilder
-    ) {
-        final Settings.Builder builder = Settings.builder();
-        for (final String key : sourceSettings.keySet()) {
-            final Setting<?> setting = indexScopedSettings.get(key);
-            if (setting == null) {
-                assert indexScopedSettings.isPrivateSetting(key) : key;
-                if (copyPrivateSettings == false) {
-                    continue;
-                }
-            } else if (setting.getProperties().contains(Setting.Property.NotCopyableOnResize)) {
-                continue;
-            } else if (setting.isPrivateIndex()) {
-                if (copyPrivateSettings == false) {
-                    continue;
-                }
-            }
-            // do not override settings that have already been set (for example, from the request)
-            if (indexSettingsBuilder.keys().contains(key)) {
-                continue;
-            }
-            builder.copy(key, sourceSettings);
-        }
-        return builder;
-    }
-
     /**
      * Returns a default number of routing shards based on the number of shards of the index. The default number of routing shards will
      * allow any index to be split at least once and at most 10 times by a factor of two. The closer the number or shards gets to 1024

x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/CreateIndexFromSourceActionIT.java

@@ -41,6 +41,41 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
         return List.of(MigratePlugin.class, ReindexPlugin.class, MockTransportService.TestPlugin.class, DataStreamsPlugin.class);
     }
 
+    public void testOldSettingsManuallyFiltered() throws Exception {
+        assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled());
+
+        var numShards = randomIntBetween(1, 10);
+        var staticSettings = Settings.builder()
+            // setting to filter
+            .put("index.soft_deletes.enabled", true)
+            // good setting to keep
+            .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numShards)
+            .build();
+
+        // start with a static setting
+        var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT);
+        indicesAdmin().create(new CreateIndexRequest(sourceIndex, staticSettings)).get();
+
+        // create from source
+        var destIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT);
+        assertAcked(
+            client().execute(CreateIndexFromSourceAction.INSTANCE, new CreateIndexFromSourceAction.Request(sourceIndex, destIndex))
+        );
+
+        // assert both static and dynamic settings set on dest index
+        var settingsResponse = indicesAdmin().getSettings(new GetSettingsRequest().indices(sourceIndex, destIndex)).actionGet();
+        var destSettings = settingsResponse.getIndexToSettings().get(destIndex);
+        var sourceSettings = settingsResponse.getIndexToSettings().get(sourceIndex);
+
+        // sanity check that source settings were added
+        assertEquals(true, sourceSettings.getAsBoolean("index.soft_deletes.enabled", false));
+        assertEquals(numShards, Integer.parseInt(destSettings.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS)));
+
+        // check that old setting was not added to index
+        assertNull(destSettings.get("index.soft_deletes.enabled"));
+        assertEquals(numShards, Integer.parseInt(destSettings.get(IndexMetadata.SETTING_NUMBER_OF_SHARDS)));
+    }
+
     public void testDestIndexCreated() throws Exception {
         assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled());
 

x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/CreateIndexFromSourceTransportAction.java

@@ -16,14 +16,15 @@
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MappingMetadata;
-import org.elasticsearch.cluster.metadata.MetadataCreateIndexService;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.IndexScopedSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.index.IndexNotFoundException;
+import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.injection.guice.Inject;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.tasks.TaskId;
@@ -35,6 +36,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 
 public class CreateIndexFromSourceTransportAction extends HandledTransportAction<
     CreateIndexFromSourceAction.Request,
@@ -122,7 +124,39 @@ private static Map<String, Object> mergeMappings(@Nullable MappingMetadata sourc
     // https://github.com/elastic/kibana/blob/8a8363f02cc990732eb9cbb60cd388643a336bed/x-pack
     // /plugins/upgrade_assistant/server/lib/reindexing/index_settings.ts#L155
     private Settings filterSettings(IndexMetadata sourceIndex) {
-        return MetadataCreateIndexService.copySettingsFromSource(false, sourceIndex.getSettings(), indexScopedSettings, Settings.builder())
-            .build();
+        Settings sourceSettings = sourceIndex.getSettings();
+        final Settings.Builder builder = Settings.builder();
+        for (final String key : sourceSettings.keySet()) {
+            final Setting<?> setting = indexScopedSettings.get(key);
+            if (setting == null) {
+                assert indexScopedSettings.isPrivateSetting(key) : key;
+                continue;
+            }
+            if (setting.isPrivateIndex()) {
+                continue;
+            }
+            if (setting.getProperties().contains(Setting.Property.NotCopyableOnResize)) {
+                continue;
+            }
+            if (setting.getProperties().contains(Setting.Property.IndexSettingDeprecatedInV7AndRemovedInV8)) {
+                continue;
+            }
+            if (SPECIFIC_SETTINGS_TO_REMOVE.contains(key)) {
+                continue;
+            }
+            builder.copy(key, sourceSettings);
+        }
+        return builder.build();
     }
+
+    private static final Set<String> SPECIFIC_SETTINGS_TO_REMOVE = Set.of(
+        /**
+         * These 3 settings were removed from indices created by UA in https://github.com/elastic/kibana/pull/93293
+         * That change only removed `index.translog.retention.size` and `index.translog.retention.age` if
+         * soft deletes were enabled. Since soft deletes are always enabled in v8, we can remove all three settings.
+         */
+        IndexSettings.INDEX_SOFT_DELETES_SETTING.getKey(),
+        IndexSettings.INDEX_TRANSLOG_RETENTION_SIZE_SETTING.getKey(),
+        IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING.getKey()
+    );
 }