[8.16] Fix file settings service restart IT (#115917) (#116163)

* Resolve

* Fix

Author: n1v0lg

muted-tests.yml

@@ -291,9 +291,6 @@ tests:
   issue: https://github.com/elastic/elasticsearch/issues/115361
 - class: org.elasticsearch.upgrades.UpgradeClusterClientYamlTestSuiteIT
   issue: https://github.com/elastic/elasticsearch/issues/115170
-- class: org.elasticsearch.xpack.security.FileSettingsRoleMappingsRestartIT
-  method: testFileSettingsReprocessedOnRestartWithoutVersionChange
-  issue: https://github.com/elastic/elasticsearch/issues/115450
 - class: org.elasticsearch.xpack.inference.rest.ServerSentEventsRestActionListenerTests
   method: testNoStream
   issue: https://github.com/elastic/elasticsearch/issues/114788

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsRestartIT.java

@@ -9,6 +9,7 @@
 
 import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest;
 import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.core.Tuple;
 import org.elasticsearch.reservedstate.service.FileSettingsService;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.SecurityIntegTestCase;
@@ -36,6 +37,7 @@
 @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false)
 public class FileSettingsRoleMappingsRestartIT extends SecurityIntegTestCase {
 
+    private static final int MAX_WAIT_TIME_SECONDS = 20;
     private final AtomicLong versionCounter = new AtomicLong(1);
 
     @Before
@@ -115,10 +117,9 @@ public void testReservedStatePersistsOnRestart() throws Exception {
         awaitFileSettingsWatcher();
         logger.info("--> write some role mappings, no other file settings");
         writeJSONFile(masterNode, testJSONOnlyRoleMappings, logger, versionCounter);
-        boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS);
-        assertTrue(awaitSuccessful);
 
-        assertRoleMappingsInClusterState(
+        assertRoleMappingsInClusterStateWithAwait(
+            savedClusterState,
             new ExpressionRoleMapping(
                 "everyone_kibana_alone",
                 new FieldExpression("username", List.of(new FieldExpression.FieldValue("*"))),
@@ -175,11 +176,8 @@ public void testReservedStatePersistsOnRestart() throws Exception {
             )
         );
 
-        // now remove the role mappings via the same settings file
-        cleanupClusterState(masterNode);
-
-        // no role mappings
-        assertRoleMappingsInClusterState();
+        // now remove the role mappings via an empty settings file
+        cleanupClusterStateAndAssertNoMappings(masterNode);
 
         // and restart the master to confirm the role mappings are all gone
         logger.info("--> restart master again");
@@ -195,14 +193,13 @@ public void testFileSettingsReprocessedOnRestartWithoutVersionChange() throws Ex
 
         final String masterNode = internalCluster().getMasterName();
 
-        var savedClusterState = setupClusterStateListener(masterNode, "everyone_kibana_alone");
+        Tuple<CountDownLatch, AtomicLong> savedClusterState = setupClusterStateListener(masterNode, "everyone_kibana_alone");
         awaitFileSettingsWatcher();
         logger.info("--> write some role mappings, no other file settings");
         writeJSONFile(masterNode, testJSONOnlyRoleMappings, logger, versionCounter);
-        boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS);
-        assertTrue(awaitSuccessful);
 
-        assertRoleMappingsInClusterState(
+        assertRoleMappingsInClusterStateWithAwait(
+            savedClusterState,
             new ExpressionRoleMapping(
                 "everyone_kibana_alone",
                 new FieldExpression("username", List.of(new FieldExpression.FieldValue("*"))),
@@ -228,41 +225,8 @@ public void testFileSettingsReprocessedOnRestartWithoutVersionChange() throws Ex
             )
         );
 
-        final CountDownLatch latch = new CountDownLatch(1);
-        final FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode);
-        fileSettingsService.addFileChangedListener(latch::countDown);
-        // Don't increment version but write new file contents to test re-processing on restart
+        // write without version increment and assert that change gets applied on restart
         writeJSONFileWithoutVersionIncrement(masterNode, testJSONOnlyUpdatedRoleMappings, logger, versionCounter);
-        // Make sure we saw a file settings update so that we know it got processed, but it did not affect cluster state
-        assertTrue(latch.await(20, TimeUnit.SECONDS));
-
-        // Nothing changed yet because version is the same and there was no restart
-        assertRoleMappingsInClusterState(
-            new ExpressionRoleMapping(
-                "everyone_kibana_alone",
-                new FieldExpression("username", List.of(new FieldExpression.FieldValue("*"))),
-                List.of("kibana_user"),
-                List.of(),
-                Map.of("uuid", "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", "_foo", "something", METADATA_NAME_FIELD, "everyone_kibana_alone"),
-                true
-            ),
-            new ExpressionRoleMapping(
-                "everyone_fleet_alone",
-                new FieldExpression("username", List.of(new FieldExpression.FieldValue("*"))),
-                List.of("fleet_user"),
-                List.of(),
-                Map.of(
-                    "uuid",
-                    "b9a59ba9-6b92-4be3-bb8d-02bb270cb3a7",
-                    "_foo",
-                    "something_else",
-                    METADATA_NAME_FIELD,
-                    "everyone_fleet_alone"
-                ),
-                false
-            )
-        );
-
         logger.info("--> restart master");
         internalCluster().restartNode(masterNode);
         ensureGreen();
@@ -286,28 +250,52 @@ public void testFileSettingsReprocessedOnRestartWithoutVersionChange() throws Ex
                     ),
                     true
                 )
-            )
+            ),
+            MAX_WAIT_TIME_SECONDS,
+            TimeUnit.SECONDS
         );
 
-        cleanupClusterState(masterNode);
+        cleanupClusterStateAndAssertNoMappings(masterNode);
     }
 
-    private void assertRoleMappingsInClusterState(ExpressionRoleMapping... expectedRoleMappings) {
-        var clusterState = clusterAdmin().state(new ClusterStateRequest(TEST_REQUEST_TIMEOUT)).actionGet().getState();
+    private void assertRoleMappingsInClusterStateWithAwait(
+        Tuple<CountDownLatch, AtomicLong> latchWithClusterStateVersion,
+        ExpressionRoleMapping... expectedRoleMappings
+    ) throws InterruptedException {
+        boolean awaitSuccessful = latchWithClusterStateVersion.v1().await(MAX_WAIT_TIME_SECONDS, TimeUnit.SECONDS);
+        assertTrue(awaitSuccessful);
+        var clusterState = clusterAdmin().state(
+            new ClusterStateRequest(TEST_REQUEST_TIMEOUT).waitForMetadataVersion(latchWithClusterStateVersion.v2().get())
+        ).actionGet().getState();
+        assertRoleMappingsInClusterState(clusterState, expectedRoleMappings);
+    }
+
+    private void assertRoleMappingsInClusterState(ClusterState clusterState, ExpressionRoleMapping... expectedRoleMappings) {
         String[] expectedRoleMappingNames = Arrays.stream(expectedRoleMappings).map(ExpressionRoleMapping::getName).toArray(String[]::new);
         assertRoleMappingReservedMetadata(clusterState, expectedRoleMappingNames);
         var actualRoleMappings = new ArrayList<>(RoleMappingMetadata.getFromClusterState(clusterState).getRoleMappings());
         assertThat(actualRoleMappings, containsInAnyOrder(expectedRoleMappings));
     }
 
-    private void cleanupClusterState(String masterNode) throws Exception {
-        // now remove the role mappings via the same settings file
+    private void assertRoleMappingsInClusterState(ExpressionRoleMapping... expectedRoleMappings) {
+        assertRoleMappingsInClusterState(
+            clusterAdmin().state(new ClusterStateRequest(TEST_REQUEST_TIMEOUT)).actionGet().getState(),
+            expectedRoleMappings
+        );
+    }
+
+    private void cleanupClusterStateAndAssertNoMappings(String masterNode) throws Exception {
         var savedClusterState = setupClusterStateListenerForCleanup(masterNode);
         awaitFileSettingsWatcher();
         logger.info("--> remove the role mappings with an empty settings file");
         writeJSONFile(masterNode, emptyJSON, logger, versionCounter);
-        boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS);
+        boolean awaitSuccessful = savedClusterState.v1().await(MAX_WAIT_TIME_SECONDS, TimeUnit.SECONDS);
         assertTrue(awaitSuccessful);
+        // ensure cluster-state update got propagated to expected version
+        var clusterState = clusterAdmin().state(
+            new ClusterStateRequest(TEST_REQUEST_TIMEOUT).waitForMetadataVersion(savedClusterState.v2().get())
+        ).actionGet();
+        assertRoleMappingsInClusterState(clusterState.getState());
     }
 
     private void assertRoleMappingReservedMetadata(ClusterState clusterState, String... names) {