Add security migration for cleaning up ECK role mappings

server/src/main/java/org/elasticsearch/index/IndexVersions.java

@@ -117,6 +117,7 @@ private static IndexVersion def(int id, Version luceneVersion) {
     public static final IndexVersion ENABLE_IGNORE_MALFORMED_LOGSDB = def(8_514_00_0, Version.LUCENE_9_11_1);
     public static final IndexVersion MERGE_ON_RECOVERY_VERSION = def(8_515_00_0, Version.LUCENE_9_11_1);
     public static final IndexVersion UPGRADE_TO_LUCENE_9_12 = def(8_516_00_0, Version.LUCENE_9_12_0);
+    public static final IndexVersion ADD_ROLE_MAPPING_MIGRATION = def(8_517_00_0, Version.LUCENE_9_12_0);
 
     /*
      * STOP! READ THIS FIRST! No, really,

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/SecurityFeatures.java

@@ -17,13 +17,14 @@
 import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MIGRATION_FRAMEWORK;
 import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_PROFILE_ORIGIN_FEATURE;
 import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLES_METADATA_FLATTENED;
+import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_ROLE_MAPPING_CLEANUP;
 import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.VERSION_SECURITY_PROFILE_ORIGIN;
 
 public class SecurityFeatures implements FeatureSpecification {
 
     @Override
     public Set<NodeFeature> getFeatures() {
-        return Set.of(SECURITY_ROLES_METADATA_FLATTENED, SECURITY_MIGRATION_FRAMEWORK);
+        return Set.of(SECURITY_ROLE_MAPPING_CLEANUP, SECURITY_ROLES_METADATA_FLATTENED, SECURITY_MIGRATION_FRAMEWORK);
     }
 
     @Override

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

@@ -31,6 +31,7 @@
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.cluster.metadata.ReservedStateMetadata;
 import org.elasticsearch.cluster.routing.IndexRoutingTable;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.core.TimeValue;
@@ -46,9 +47,12 @@
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.threadpool.Scheduler;
 import org.elasticsearch.xcontent.XContentType;
+import org.elasticsearch.xpack.core.security.authz.RoleMappingMetadata;
 import org.elasticsearch.xpack.security.SecurityFeatures;
+import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction;
 
 import java.time.Instant;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -74,7 +78,8 @@
 public class SecurityIndexManager implements ClusterStateListener {
 
     public static final String SECURITY_VERSION_STRING = "security-version";
-
+    private static final String FILE_SETTINGS_METADATA_NAMESPACE = "file_settings";
+    private static final String HANDLER_ROLE_MAPPINGS_NAME = "role_mappings";
     private static final Logger logger = LogManager.getLogger(SecurityIndexManager.class);
 
     /**
@@ -267,6 +272,11 @@ private static boolean isCreatedOnLatestVersion(IndexMetadata indexMetadata) {
         return indexVersionCreated != null && indexVersionCreated.onOrAfter(IndexVersion.current());
     }
 
+    private static Set<String> getFileSettingsMetadataHandlerRoleMappingKeys(ClusterState clusterState) {
+        ReservedStateMetadata fileSettingsMetadata = clusterState.metadata().reservedStateMetadata().get(FILE_SETTINGS_METADATA_NAMESPACE);
+        return fileSettingsMetadata.handlers().get(HANDLER_ROLE_MAPPINGS_NAME).keys();
+    }
+
     @Override
     public void clusterChanged(ClusterChangedEvent event) {
         if (event.state().blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) {
@@ -284,6 +294,10 @@ public void clusterChanged(ClusterChangedEvent event) {
         Tuple<Boolean, Boolean> available = checkIndexAvailable(event.state());
         final boolean indexAvailableForWrite = available.v1();
         final boolean indexAvailableForSearch = available.v2();
+        final Set<String> reservedStateRoleMappingNames = getFileSettingsMetadataHandlerRoleMappingKeys(event.state());
+        final boolean reservedRoleMappingsSynced = reservedStateRoleMappingNames.size() == RoleMappingMetadata.getFromClusterState(
+            event.state()
+        ).getRoleMappings().size();
         final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state());
         final int migrationsVersion = getMigrationVersionFromIndexMetadata(indexMetadata);
         final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion = getMinSecurityIndexMappingVersion(event.state());
@@ -314,6 +328,7 @@ public void clusterChanged(ClusterChangedEvent event) {
             indexAvailableForWrite,
             mappingIsUpToDate,
             createdOnLatestVersion,
+            reservedRoleMappingsSynced,
             migrationsVersion,
             minClusterMappingVersion,
             indexMappingVersion,
@@ -323,7 +338,8 @@ public void clusterChanged(ClusterChangedEvent event) {
             indexUUID,
             allSecurityFeatures.stream()
                 .filter(feature -> featureService.clusterHasFeature(event.state(), feature))
-                .collect(Collectors.toSet())
+                .collect(Collectors.toSet()),
+            reservedStateRoleMappingNames
         );
         this.state = newState;
 
@@ -334,6 +350,10 @@ public void clusterChanged(ClusterChangedEvent event) {
         }
     }
 
+    public Set<String> getReservedStateRoleMappingNames() {
+        return state.reservedStateRoleMappingNames;
+    }
+
     public static int getMigrationVersionFromIndexMetadata(IndexMetadata indexMetadata) {
         Map<String, String> customMetadata = indexMetadata == null ? null : indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY);
         if (customMetadata == null) {
@@ -438,7 +458,8 @@ private Tuple<Boolean, Boolean> checkIndexAvailable(ClusterState state) {
 
     public boolean isEligibleSecurityMigration(SecurityMigrations.SecurityMigration securityMigration) {
         return state.securityFeatures.containsAll(securityMigration.nodeFeaturesRequired())
-            && state.indexMappingVersion >= securityMigration.minMappingVersion();
+            && state.indexMappingVersion >= securityMigration.minMappingVersion()
+            && securityMigration.checkPreConditions(state);
     }
 
     public boolean isReadyForSecurityMigration(SecurityMigrations.SecurityMigration securityMigration) {
@@ -671,13 +692,15 @@ public static class State {
             false,
             false,
             false,
+            false,
             null,
             null,
             null,
             null,
             null,
             null,
             null,
+            Set.of(),
             Set.of()
         );
         public final Instant creationTime;
@@ -686,6 +709,7 @@ public static class State {
         public final boolean indexAvailableForWrite;
         public final boolean mappingUpToDate;
         public final boolean createdOnLatestVersion;
+        public final boolean reservedRoleMappingsSynced;
         public final Integer migrationsVersion;
         // Min mapping version supported by the descriptors in the cluster
         public final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion;
@@ -696,6 +720,7 @@ public static class State {
         public final IndexMetadata.State indexState;
         public final String indexUUID;
         public final Set<NodeFeature> securityFeatures;
+        public final Set<String> reservedStateRoleMappingNames;
 
         public State(
             Instant creationTime,
@@ -704,14 +729,16 @@ public State(
             boolean indexAvailableForWrite,
             boolean mappingUpToDate,
             boolean createdOnLatestVersion,
+            boolean reservedRoleMappingsSynced,
             Integer migrationsVersion,
             SystemIndexDescriptor.MappingsVersion minClusterMappingVersion,
             Integer indexMappingVersion,
             String concreteIndexName,
             ClusterHealthStatus indexHealth,
             IndexMetadata.State indexState,
             String indexUUID,
-            Set<NodeFeature> securityFeatures
+            Set<NodeFeature> securityFeatures,
+            Set<String> reservedStateRoleMappingNames
         ) {
             this.creationTime = creationTime;
             this.isIndexUpToDate = isIndexUpToDate;
@@ -720,13 +747,15 @@ public State(
             this.mappingUpToDate = mappingUpToDate;
             this.migrationsVersion = migrationsVersion;
             this.createdOnLatestVersion = createdOnLatestVersion;
+            this.reservedRoleMappingsSynced = reservedRoleMappingsSynced;
             this.minClusterMappingVersion = minClusterMappingVersion;
             this.indexMappingVersion = indexMappingVersion;
             this.concreteIndexName = concreteIndexName;
             this.indexHealth = indexHealth;
             this.indexState = indexState;
             this.indexUUID = indexUUID;
             this.securityFeatures = securityFeatures;
+            this.reservedStateRoleMappingNames = reservedStateRoleMappingNames;
         }
 
         @Override
@@ -740,13 +769,15 @@ public boolean equals(Object o) {
                 && indexAvailableForWrite == state.indexAvailableForWrite
                 && mappingUpToDate == state.mappingUpToDate
                 && createdOnLatestVersion == state.createdOnLatestVersion
+                && reservedRoleMappingsSynced == state.reservedRoleMappingsSynced
                 && Objects.equals(indexMappingVersion, state.indexMappingVersion)
                 && Objects.equals(migrationsVersion, state.migrationsVersion)
                 && Objects.equals(minClusterMappingVersion, state.minClusterMappingVersion)
                 && Objects.equals(concreteIndexName, state.concreteIndexName)
                 && indexHealth == state.indexHealth
                 && indexState == state.indexState
-                && Objects.equals(securityFeatures, state.securityFeatures);
+                && Objects.equals(securityFeatures, state.securityFeatures)
+                && Objects.equals(reservedStateRoleMappingNames, state.reservedStateRoleMappingNames);
         }
 
         public boolean indexExists() {
@@ -762,12 +793,14 @@ public int hashCode() {
                 indexAvailableForWrite,
                 mappingUpToDate,
                 createdOnLatestVersion,
+                reservedRoleMappingsSynced,
                 migrationsVersion,
                 minClusterMappingVersion,
                 indexMappingVersion,
                 concreteIndexName,
                 indexHealth,
-                securityFeatures
+                securityFeatures,
+                reservedStateRoleMappingNames
             );
         }
 
@@ -786,6 +819,8 @@ public String toString() {
                 + mappingUpToDate
                 + ", createdOnLatestVersion="
                 + createdOnLatestVersion
+                + ", reservedRoleMappingsSynced="
+                + reservedRoleMappingsSynced
                 + ", migrationsVersion="
                 + migrationsVersion
                 + ", minClusterMappingVersion="
@@ -804,6 +839,9 @@ public String toString() {
                 + '\''
                 + ", securityFeatures="
                 + securityFeatures
+                + ", reservedStateRoleMappingNames=["
+                + Arrays.toString(reservedStateRoleMappingNames.toArray(String[]::new))
+                + "]"
                 + '}';
         }
     }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrationExecutor.java

@@ -10,6 +10,8 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.admin.indices.refresh.RefreshAction;
+import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
 import org.elasticsearch.action.support.ThreadedActionListener;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.core.TimeValue;
@@ -24,6 +26,9 @@
 import java.util.TreeMap;
 import java.util.concurrent.Executor;
 
+import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN;
+import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
+
 public class SecurityMigrationExecutor extends PersistentTasksExecutor<SecurityMigrationTaskParams> {
 
     private static final Logger logger = LogManager.getLogger(SecurityMigrationExecutor.class);
@@ -83,13 +88,17 @@ private void applyOutstandingMigrations(AllocatedPersistentTask task, int curren
                         response -> updateMigrationVersion(
                             migrationEntry.getKey(),
                             securityIndexManager.getConcreteIndexName(),
-                            new ThreadedActionListener<>(
-                                this.getExecutor(),
-                                ActionListener.wrap(
-                                    updateResponse -> applyOutstandingMigrations(task, migrationEntry.getKey(), listener),
-                                    listener::onFailure
-                                )
-                            )
+                            new ThreadedActionListener<>(this.getExecutor(), ActionListener.wrap(updateResponse -> {
+                                refreshSecurityIndex(
+                                    new ThreadedActionListener<>(
+                                        this.getExecutor(),
+                                        ActionListener.wrap(
+                                            refreshResponse -> applyOutstandingMigrations(task, migrationEntry.getKey(), listener),
+                                            listener::onFailure
+                                        )
+                                    )
+                                );
+                            }, listener::onFailure))
                         ),
                         listener::onFailure
                     )
@@ -100,6 +109,21 @@ private void applyOutstandingMigrations(AllocatedPersistentTask task, int curren
         }
     }
 
+    /**
+     * Refresh security index to make sure that docs that were migrated are visible to the next migration and to prevent version conflicts
+     * or unexpected behaviour by APIs relying on migrated docs.
+     */
+    private void refreshSecurityIndex(ActionListener<Void> listener) {
+        RefreshRequest refreshRequest = new RefreshRequest(securityIndexManager.getConcreteIndexName());
+        executeAsyncWithOrigin(
+            client,
+            SECURITY_ORIGIN,
+            RefreshAction.INSTANCE,
+            refreshRequest,
+            ActionListener.wrap(response -> listener.onResponse(null), listener::onFailure)
+        );
+    }
+
     private void updateMigrationVersion(int migrationVersion, String indexName, ActionListener<Void> listener) {
         client.execute(
             UpdateIndexMigrationVersionAction.INSTANCE,