Add test for system index migration using reindexing script (elastic#120644)

Author: alexey-ivanov-es

modules/reindex/build.gradle

@@ -45,6 +45,9 @@ dependencies {
   clusterModules project(':modules:lang-painless')
   clusterModules project(':modules:parent-join')
   clusterModules project(":modules:rest-root")
+
+  internalClusterTestImplementation project(':modules:lang-painless')
+  internalClusterTestImplementation project(':modules:lang-painless:spi')
 }
 
 restResources {

modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/AbstractFeatureMigrationIntegTest.java

@@ -68,6 +68,7 @@ public abstract class AbstractFeatureMigrationIntegTest extends ESIntegTestCase
     static final String INTERNAL_MANAGED_INDEX_NAME = ".int-man-old";
     static final int INDEX_DOC_COUNT = 100; // arbitrarily chosen
     static final int INTERNAL_MANAGED_FLAG_VALUE = 1;
+    static final String FIELD_NAME = "some_field";
     public static final Version NEEDS_UPGRADE_VERSION = TransportGetFeatureUpgradeStatusAction.NO_UPGRADE_REQUIRED_VERSION.previousMajor();
     public static final IndexVersion NEEDS_UPGRADE_INDEX_VERSION = IndexVersionUtils.getPreviousMajorVersion(
         TransportGetFeatureUpgradeStatusAction.NO_UPGRADE_REQUIRED_INDEX_VERSION
@@ -157,7 +158,7 @@ public <T extends Plugin> T getPlugin(Class<T> type) {
         return pluginsService.filterPlugins(type).findFirst().get();
     }
 
-    public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) throws InterruptedException {
+    protected void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) {
         assertThat(
             "the strategy used below to create index names for descriptors without a primary index name only works for simple patterns",
             descriptor.getIndexPattern(),
@@ -191,9 +192,13 @@ public void createSystemIndexForDescriptor(SystemIndexDescriptor descriptor) thr
         CreateIndexResponse response = createRequest.get();
         Assert.assertTrue(response.isShardsAcknowledged());
 
+        indexDocs(indexName);
+    }
+
+    protected void indexDocs(String indexName) {
         List<IndexRequestBuilder> docs = new ArrayList<>(INDEX_DOC_COUNT);
         for (int i = 0; i < INDEX_DOC_COUNT; i++) {
-            docs.add(ESIntegTestCase.prepareIndex(indexName).setId(Integer.toString(i)).setSource("some_field", "words words"));
+            docs.add(ESIntegTestCase.prepareIndex(indexName).setId(Integer.toString(i)).setSource(FIELD_NAME, "words words"));
         }
         indexRandom(true, docs);
         IndicesStatsResponse indexStats = ESIntegTestCase.indicesAdmin().prepareStats(indexName).setDocs(true).get();
@@ -218,7 +223,7 @@ static String createMapping(boolean descriptorManaged, boolean descriptorInterna
                 builder.field("dynamic", "strict");
                 builder.startObject("properties");
                 {
-                    builder.startObject("some_field");
+                    builder.startObject(FIELD_NAME);
                     builder.field("type", "keyword");
                     builder.endObject();
                 }
@@ -232,7 +237,7 @@ static String createMapping(boolean descriptorManaged, boolean descriptorInterna
         }
     }
 
-    public void assertIndexHasCorrectProperties(
+    protected void assertIndexHasCorrectProperties(
         Metadata metadata,
         String indexName,
         int settingsFlagValue,

modules/reindex/src/internalClusterTest/java/org/elasticsearch/migration/FeatureMigrationIT.java

@@ -21,6 +21,7 @@
 import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
 import org.elasticsearch.action.admin.indices.template.put.PutComponentTemplateAction;
 import org.elasticsearch.action.admin.indices.template.put.TransportPutComposableIndexTemplateAction;
+import org.elasticsearch.action.search.SearchRequestBuilder;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ClusterStateUpdateTask;
@@ -33,8 +34,11 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.indices.SystemIndexDescriptor;
+import org.elasticsearch.painless.PainlessPlugin;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.plugins.SystemIndexPlugin;
 import org.elasticsearch.reindex.ReindexPlugin;
 import org.elasticsearch.upgrades.FeatureMigrationResults;
 import org.elasticsearch.upgrades.SingleFeatureMigrationResult;
@@ -51,6 +55,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCountAndNoFailures;
 import static org.hamcrest.Matchers.aMapWithSize;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.equalTo;
@@ -62,6 +67,27 @@
 import static org.hamcrest.Matchers.nullValue;
 
 public class FeatureMigrationIT extends AbstractFeatureMigrationIntegTest {
+    private static final String INTERNAL_MANAGED_WITH_SCRIPT_INDEX_NAME = ".int-mans-old";
+    private static final String SCRIPTED_INDEX_FEATURE_NAME = "B-test-feature";
+    private static final SystemIndexDescriptor INTERNAL_MANAGED_WITH_SCRIPT = SystemIndexDescriptor.builder()
+        .setIndexPattern(".int-mans-*")
+        .setAliasName(".internal-managed-with-script-alias")
+        .setPrimaryIndex(INTERNAL_MANAGED_WITH_SCRIPT_INDEX_NAME)
+        .setType(SystemIndexDescriptor.Type.INTERNAL_MANAGED)
+        .setSettings(createSettings(NEEDS_UPGRADE_INDEX_VERSION, INTERNAL_MANAGED_FLAG_VALUE))
+        .setMappings(createMapping(true, true))
+        .setOrigin(ORIGIN)
+        .setVersionMetaKey(VERSION_META_KEY)
+        .setAllowedElasticProductOrigins(Collections.emptyList())
+        .setMinimumNodeVersion(NEEDS_UPGRADE_VERSION)
+        .setPriorSystemIndexDescriptors(Collections.emptyList())
+        .setMigrationScript("""
+            if (ctx._source.some_field != null) {
+              ctx._source.some_field = 'migrated';
+            }
+            """)
+        .build();
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
         return Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings)).build();
@@ -77,7 +103,9 @@ protected boolean forbidPrivateIndexSettings() {
     protected Collection<Class<? extends Plugin>> nodePlugins() {
         List<Class<? extends Plugin>> plugins = new ArrayList<>(super.nodePlugins());
         plugins.add(TestPlugin.class);
+        plugins.add(SecondTestPlugin.class);
         plugins.add(ReindexPlugin.class);
+        plugins.add(PainlessPlugin.class);
         return plugins;
     }
 
@@ -115,7 +143,7 @@ public void testStartMigrationAndImmediatelyCheckStatus() throws Exception {
         });
     }
 
-    public void testMigrateInternalManagedSystemIndex() throws Exception {
+    public void testMigrateSystemIndex() throws Exception {
         createSystemIndexForDescriptor(INTERNAL_MANAGED);
         createSystemIndexForDescriptor(INTERNAL_UNMANAGED);
         createSystemIndexForDescriptor(EXTERNAL_MANAGED);
@@ -171,40 +199,15 @@ public void testMigrateInternalManagedSystemIndex() throws Exception {
             postUpgradeHookCalled.set(true);
         });
 
-        PostFeatureUpgradeRequest migrationRequest = new PostFeatureUpgradeRequest(TEST_REQUEST_TIMEOUT);
-        PostFeatureUpgradeResponse migrationResponse = client().execute(PostFeatureUpgradeAction.INSTANCE, migrationRequest).get();
-        assertThat(migrationResponse.getReason(), nullValue());
-        assertThat(migrationResponse.getElasticsearchException(), nullValue());
-        final Set<String> migratingFeatures = migrationResponse.getFeatures()
-            .stream()
-            .map(PostFeatureUpgradeResponse.Feature::getFeatureName)
-            .collect(Collectors.toSet());
-        assertThat(migratingFeatures, hasItem(FEATURE_NAME));
-
-        GetFeatureUpgradeStatusRequest getStatusRequest = new GetFeatureUpgradeStatusRequest(TEST_REQUEST_TIMEOUT);
-        // The feature upgrade may take longer than ten seconds when tests are running
-        // in parallel, so we give assertBusy a sixty-second timeout.
-        assertBusy(() -> {
-            GetFeatureUpgradeStatusResponse statusResponse = client().execute(GetFeatureUpgradeStatusAction.INSTANCE, getStatusRequest)
-                .get();
-            logger.info(Strings.toString(statusResponse));
-            assertThat(statusResponse.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
-        }, 60, TimeUnit.SECONDS);
+        executeMigration(FEATURE_NAME);
 
         // Waiting for shards to stabilize if indices were moved around
         ensureGreen();
 
         assertTrue("the pre-migration hook wasn't actually called", preUpgradeHookCalled.get());
         assertTrue("the post-migration hook wasn't actually called", postUpgradeHookCalled.get());
 
-        Metadata finalMetadata = clusterAdmin().prepareState(TEST_REQUEST_TIMEOUT).get().getState().metadata();
-        // Check that the results metadata is what we expect.
-        FeatureMigrationResults currentResults = finalMetadata.custom(FeatureMigrationResults.TYPE);
-        assertThat(currentResults, notNullValue());
-        assertThat(currentResults.getFeatureStatuses(), allOf(aMapWithSize(1), hasKey(FEATURE_NAME)));
-        assertThat(currentResults.getFeatureStatuses().get(FEATURE_NAME).succeeded(), is(true));
-        assertThat(currentResults.getFeatureStatuses().get(FEATURE_NAME).getFailedIndexName(), nullValue());
-        assertThat(currentResults.getFeatureStatuses().get(FEATURE_NAME).getException(), nullValue());
+        Metadata finalMetadata = assertMetadataAfterMigration(FEATURE_NAME);
 
         assertIndexHasCorrectProperties(
             finalMetadata,
@@ -240,6 +243,18 @@ public void testMigrateInternalManagedSystemIndex() throws Exception {
         );
     }
 
+    private static Metadata assertMetadataAfterMigration(String featureName) {
+        Metadata finalMetadata = clusterAdmin().prepareState(TEST_REQUEST_TIMEOUT).get().getState().metadata();
+        // Check that the results metadata is what we expect.
+        FeatureMigrationResults currentResults = finalMetadata.custom(FeatureMigrationResults.TYPE);
+        assertThat(currentResults, notNullValue());
+        assertThat(currentResults.getFeatureStatuses(), allOf(aMapWithSize(1), hasKey(featureName)));
+        assertThat(currentResults.getFeatureStatuses().get(featureName).succeeded(), is(true));
+        assertThat(currentResults.getFeatureStatuses().get(featureName).getFailedIndexName(), nullValue());
+        assertThat(currentResults.getFeatureStatuses().get(featureName).getException(), nullValue());
+        return finalMetadata;
+    }
+
     public void testMigrateIndexWithWriteBlock() throws Exception {
         createSystemIndexForDescriptor(INTERNAL_UNMANAGED);
 
@@ -317,6 +332,50 @@ public void onFailure(Exception e) {
         });
     }
 
+    private void executeMigration(String featureName) throws Exception {
+        PostFeatureUpgradeRequest migrationRequest = new PostFeatureUpgradeRequest(TEST_REQUEST_TIMEOUT);
+        PostFeatureUpgradeResponse migrationResponse = client().execute(PostFeatureUpgradeAction.INSTANCE, migrationRequest).get();
+        assertThat(migrationResponse.getReason(), nullValue());
+        assertThat(migrationResponse.getElasticsearchException(), nullValue());
+        final Set<String> migratingFeatures = migrationResponse.getFeatures()
+            .stream()
+            .map(PostFeatureUpgradeResponse.Feature::getFeatureName)
+            .collect(Collectors.toSet());
+        assertThat(migratingFeatures, hasItem(featureName));
+
+        GetFeatureUpgradeStatusRequest getStatusRequest = new GetFeatureUpgradeStatusRequest(TEST_REQUEST_TIMEOUT);
+        // The feature upgrade may take longer than ten seconds when tests are running
+        // in parallel, so we give assertBusy a sixty-second timeout.
+        assertBusy(() -> {
+            GetFeatureUpgradeStatusResponse statusResponse = client().execute(GetFeatureUpgradeStatusAction.INSTANCE, getStatusRequest)
+                .get();
+            logger.info(Strings.toString(statusResponse));
+            assertThat(statusResponse.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
+        }, 60, TimeUnit.SECONDS);
+    }
+
+    public void testMigrateUsingScript() throws Exception {
+        createSystemIndexForDescriptor(INTERNAL_MANAGED_WITH_SCRIPT);
+
+        executeMigration(SCRIPTED_INDEX_FEATURE_NAME);
+        ensureGreen();
+
+        Metadata metadata = assertMetadataAfterMigration(SCRIPTED_INDEX_FEATURE_NAME);
+        String newIndexName = ".int-mans-old-reindexed-for-9";
+        assertIndexHasCorrectProperties(
+            metadata,
+            newIndexName,
+            INTERNAL_MANAGED_FLAG_VALUE,
+            true,
+            true,
+            Arrays.asList(".int-mans-old", ".internal-managed-with-script-alias")
+        );
+
+        SearchRequestBuilder searchRequestBuilder = prepareSearch(newIndexName).setQuery(QueryBuilders.termsQuery(FIELD_NAME, "migrated"))
+            .setSize(0);
+        assertHitCountAndNoFailures(searchRequestBuilder, INDEX_DOC_COUNT);
+    }
+
     private String featureUpgradeErrorResponse(GetFeatureUpgradeStatusResponse statusResp) {
         return statusResp.getFeatureUpgradeStatuses()
             .stream()
@@ -463,4 +522,21 @@ public void testMigrateWithTemplatesV2() throws Exception {
             assertThat(statusResp.getUpgradeStatus(), equalTo(GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_MIGRATION_NEEDED));
         });
     }
+
+    public static class SecondTestPlugin extends Plugin implements SystemIndexPlugin {
+        @Override
+        public String getFeatureName() {
+            return SCRIPTED_INDEX_FEATURE_NAME;
+        }
+
+        @Override
+        public String getFeatureDescription() {
+            return "a plugin for testing system index migration";
+        }
+
+        @Override
+        public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings settings) {
+            return Collections.singletonList(INTERNAL_MANAGED_WITH_SCRIPT);
+        }
+    }
 }

modules/reindex/src/main/plugin-metadata/entitlement-policy.yaml

@@ -1,2 +1,4 @@
 ALL-UNNAMED:
   - outbound_network
+org.elasticsearch.painless:
+  - create_class_loader

modules/reindex/src/main/plugin-metadata/plugin-security.policy

@@ -10,6 +10,9 @@
 grant {
    // reindex opens socket connections using the rest client
    permission java.net.SocketPermission "*", "connect";
+
+  // needed for Painless to generate runtime classes
+  permission java.lang.RuntimePermission "createClassLoader";
 };
 
 grant codeBase "${codebase.elasticsearch-rest-client}" {

server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java

@@ -24,7 +24,6 @@
 import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.injection.guice.Inject;
 import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
-import org.elasticsearch.persistent.PersistentTasksService;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
@@ -56,7 +55,6 @@ public class TransportGetFeatureUpgradeStatusAction extends TransportMasterNodeA
     public static final IndexVersion NO_UPGRADE_REQUIRED_INDEX_VERSION = IndexVersions.V_8_0_0;
 
     private final SystemIndices systemIndices;
-    PersistentTasksService persistentTasksService;
 
     @Inject
     public TransportGetFeatureUpgradeStatusAction(
@@ -65,7 +63,6 @@ public TransportGetFeatureUpgradeStatusAction(
         ActionFilters actionFilters,
         ClusterService clusterService,
         IndexNameExpressionResolver indexNameExpressionResolver,
-        PersistentTasksService persistentTasksService,
         SystemIndices systemIndices
     ) {
         super(
@@ -80,7 +77,6 @@ public TransportGetFeatureUpgradeStatusAction(
         );
 
         this.systemIndices = systemIndices;
-        this.persistentTasksService = persistentTasksService;
     }
 
     @Override