Test queryable built-in role synchronization (#118964)

Adds more tests for built-in roles synchronization, and fixes a bug where `synchronizationInProgress` hasn't been reset properly. Resolves #118806
elastic · Dec 19, 2024 · 845021d · 845021d
1 parent 74f37d1
commit 845021d
Show file tree

Hide file tree

Showing 5 changed files with 557 additions and 17 deletions.
diff --git a/muted-tests.yml b/muted-tests.yml
@@ -276,9 +276,6 @@ tests:
 - class: org.elasticsearch.xpack.ccr.rest.ShardChangesRestIT
   method: testShardChangesNoOperation
   issue: https://github.com/elastic/elasticsearch/issues/118800
-- class: org.elasticsearch.xpack.security.QueryableReservedRolesIT
-  method: testDeletingAndCreatingSecurityIndexTriggersSynchronization
-  issue: https://github.com/elastic/elasticsearch/issues/118806
 - class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT
   method: test {yaml=reference/indices/shard-stores/line_150}
   issue: https://github.com/elastic/elasticsearch/issues/118896

diff --git a/...main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java b/...main/java/org/elasticsearch/xpack/security/support/QueryableBuiltInRolesSynchronizer.java
@@ -199,20 +199,33 @@ public void clusterChanged(ClusterChangedEvent event) {
         }
     }
 
+    /**
+     * @return {@code true} if the synchronization of built-in roles is in progress, {@code false} otherwise
+     */
+    public boolean isSynchronizationInProgress() {
+        return synchronizationInProgress.get();
+    }
+
     private void syncBuiltInRoles(final QueryableBuiltInRoles roles) {
         if (synchronizationInProgress.compareAndSet(false, true)) {
-            final Map<String, String> indexedRolesDigests = readIndexedBuiltInRolesDigests(clusterService.state());
-            if (roles.rolesDigest().equals(indexedRolesDigests)) {
-                logger.debug("Security index already contains the latest built-in roles indexed, skipping synchronization");
-                return;
-            }
-            executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> {
-                logger.info("Successfully synced [" + roles.roleDescriptors().size() + "] built-in roles to .security index");
-                synchronizationInProgress.set(false);
-            }, e -> {
-                handleException(e);
+            try {
+                final Map<String, String> indexedRolesDigests = readIndexedBuiltInRolesDigests(clusterService.state());
+                if (roles.rolesDigest().equals(indexedRolesDigests)) {
+                    logger.debug("Security index already contains the latest built-in roles indexed, skipping roles synchronization");
+                    synchronizationInProgress.set(false);
+                } else {
+                    executor.execute(() -> doSyncBuiltinRoles(indexedRolesDigests, roles, ActionListener.wrap(v -> {
+                        logger.info("Successfully synced [" + roles.roleDescriptors().size() + "] built-in roles to .security index");
+                        synchronizationInProgress.set(false);
+                    }, e -> {
+                        handleException(e);
+                        synchronizationInProgress.set(false);
+                    })));
+                }
+            } catch (Exception e) {
+                logger.error("Failed to sync built-in roles", e);
                 synchronizationInProgress.set(false);
-            })));
+            }
         }
     }
 
@@ -452,6 +465,10 @@ static class MarkRolesAsSyncedTask implements ClusterStateTaskListener {
             this.newRoleDigests = newRoleDigests;
         }
 
+        public Map<String, String> getNewRoleDigests() {
+            return newRoleDigests;
+        }
+
         Tuple<ClusterState, Map<String, String>> execute(ClusterState state) {
             IndexMetadata indexMetadata = state.metadata().index(concreteSecurityIndexName);
             if (indexMetadata == null) {

diff --git a/...rc/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java b/...rc/main/java/org/elasticsearch/xpack/security/support/QueryableReservedRolesProvider.java
@@ -23,7 +23,7 @@
  * The reserved roles are static and do not change during runtime, hence this provider will never notify any listeners.
  * </p>
  */
-public final class QueryableReservedRolesProvider implements QueryableBuiltInRoles.Provider {
+public class QueryableReservedRolesProvider implements QueryableBuiltInRoles.Provider {
 
     private final Supplier<QueryableBuiltInRoles> reservedRolesSupplier;