Fix file settings service tests

muted-tests.yml

@@ -241,12 +241,6 @@ tests:
 - class: org.elasticsearch.xpack.inference.DefaultEndPointsIT
   method: testInferDeploysDefaultE5
   issue: https://github.com/elastic/elasticsearch/issues/115361
-- class: org.elasticsearch.reservedstate.service.FileSettingsServiceTests
-  method: testProcessFileChanges
-  issue: https://github.com/elastic/elasticsearch/issues/115280
-- class: org.elasticsearch.xpack.security.FileSettingsRoleMappingsRestartIT
-  method: testFileSettingsReprocessedOnRestartWithoutVersionChange
-  issue: https://github.com/elastic/elasticsearch/issues/115450
 - class: org.elasticsearch.xpack.restart.MLModelDeploymentFullClusterRestartIT
   method: testDeploymentSurvivesRestart {cluster=UPGRADED}
   issue: https://github.com/elastic/elasticsearch/issues/115528

server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java

@@ -24,6 +24,8 @@
 import org.elasticsearch.common.component.Lifecycle;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.core.IOUtils;
+import org.elasticsearch.core.Strings;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.env.BuildVersion;
 import org.elasticsearch.env.Environment;
@@ -39,9 +41,11 @@
 import org.mockito.stubbing.Answer;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.AccessDeniedException;
+import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -50,6 +54,8 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
 
+import static java.nio.file.StandardCopyOption.ATOMIC_MOVE;
+import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
 import static org.hamcrest.Matchers.anEmptyMap;
 import static org.hamcrest.Matchers.hasEntry;
@@ -192,8 +198,6 @@ public void testInitialFileWorks() throws Exception {
 
         CountDownLatch latch = new CountDownLatch(1);
 
-        fileSettingsService.addFileChangedListener(latch::countDown);
-
         Files.createDirectories(fileSettingsService.watchedFileDir());
         // contents of the JSON don't matter, we just need a file to exist
         writeTestFile(fileSettingsService.watchedFile(), "{}");
@@ -209,7 +213,7 @@ public void testInitialFileWorks() throws Exception {
         fileSettingsService.start();
         fileSettingsService.clusterChanged(new ClusterChangedEvent("test", clusterService.state(), ClusterState.EMPTY_STATE));
 
-        // wait for listener to be called
+        // wait file processing call
         assertTrue(latch.await(20, TimeUnit.SECONDS));
 
         verify(fileSettingsService, times(1)).processFileOnServiceStart();
@@ -223,40 +227,40 @@ public void testProcessFileChanges() throws Exception {
             return null;
         }).when(controller).process(any(), any(XContentParser.class), any(), any());
 
-        // we get three events: initial clusterChanged event, first write, second write
-        CountDownLatch latch = new CountDownLatch(3);
-
-        fileSettingsService.addFileChangedListener(latch::countDown);
-
-        Files.createDirectories(fileSettingsService.watchedFileDir());
-        // contents of the JSON don't matter, we just need a file to exist
-        writeTestFile(fileSettingsService.watchedFile(), "{}");
-
+        CountDownLatch changesOnStartLatch = new CountDownLatch(1);
         doAnswer((Answer<?>) invocation -> {
             try {
                 return invocation.callRealMethod();
             } finally {
-                latch.countDown();
+                changesOnStartLatch.countDown();
             }
         }).when(fileSettingsService).processFileOnServiceStart();
+
+        CountDownLatch changesLatch = new CountDownLatch(1);
         doAnswer((Answer<?>) invocation -> {
             try {
                 return invocation.callRealMethod();
             } finally {
-                latch.countDown();
+                changesLatch.countDown();
             }
         }).when(fileSettingsService).processFileChanges();
 
+        Files.createDirectories(fileSettingsService.watchedFileDir());
+        // contents of the JSON don't matter, we just need a file to exist
+        writeTestFile(fileSettingsService.watchedFile(), "{}");
+
         fileSettingsService.start();
         fileSettingsService.clusterChanged(new ClusterChangedEvent("test", clusterService.state(), ClusterState.EMPTY_STATE));
-        // second file change; contents still don't matter
-        overwriteTestFile(fileSettingsService.watchedFile(), "{}");
 
-        // wait for listener to be called (once for initial processing, once for subsequent update)
-        assertTrue(latch.await(20, TimeUnit.SECONDS));
+        assertTrue(changesOnStartLatch.await(20, TimeUnit.SECONDS));
 
         verify(fileSettingsService, times(1)).processFileOnServiceStart();
         verify(controller, times(1)).process(any(), any(XContentParser.class), eq(ReservedStateVersionCheck.HIGHER_OR_SAME_VERSION), any());
+
+        // second file change; contents still don't matter
+        writeTestFile(fileSettingsService.watchedFile(), "[]");
+        assertTrue(changesLatch.await(20, TimeUnit.SECONDS));
+
         verify(fileSettingsService, times(1)).processFileChanges();
         verify(controller, times(1)).process(any(), any(XContentParser.class), eq(ReservedStateVersionCheck.HIGHER_VERSION_ONLY), any());
     }
@@ -352,15 +356,22 @@ public void testHandleSnapshotRestoreResetsMetadata() throws Exception {
     }
 
     // helpers
-    private void writeTestFile(Path path, String contents) throws IOException {
-        Path tempFilePath = createTempFile();
-        Files.writeString(tempFilePath, contents);
-        Files.move(tempFilePath, path, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING);
-    }
+    private static void writeTestFile(Path path, String contents) {
+        Path tempFile = null;
+        try {
+            tempFile = Files.createTempFile(path.getParent(), path.getFileName().toString(), "tmp");
+            Files.writeString(tempFile, contents);
 
-    private void overwriteTestFile(Path path, String contents) throws IOException {
-        Path tempFilePath = createTempFile();
-        Files.writeString(tempFilePath, contents);
-        Files.move(tempFilePath, path, StandardCopyOption.REPLACE_EXISTING);
+            try {
+                Files.move(tempFile, path, REPLACE_EXISTING, ATOMIC_MOVE);
+            } catch (AtomicMoveNotSupportedException | AccessDeniedException e) {
+                Files.move(tempFile, path, REPLACE_EXISTING);
+            }
+        } catch (final IOException e) {
+            throw new UncheckedIOException(Strings.format("could not write file [%s]", path.toAbsolutePath()), e);
+        } finally {
+            // we are ignoring exceptions here, so we do not need handle whether or not tempFile was initialized nor if the file exists
+            IOUtils.deleteFilesIgnoringExceptions(tempFile);
+        }
     }
 }

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java

@@ -43,6 +43,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.AtomicMoveNotSupportedException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
@@ -178,7 +179,11 @@ private static void writeJSONFile(String node, String json, Logger logger, Atomi
         logger.info("--> before writing JSON config to node {} with path {}", node, tempFilePath);
         logger.info(Strings.format(json, version));
         Files.write(tempFilePath, Strings.format(json, version).getBytes(StandardCharsets.UTF_8));
-        Files.move(tempFilePath, fileSettingsService.watchedFile(), StandardCopyOption.ATOMIC_MOVE);
+        try {
+            Files.move(tempFilePath, fileSettingsService.watchedFile(), StandardCopyOption.ATOMIC_MOVE);
+        } catch (AtomicMoveNotSupportedException e) {
+            Files.move(tempFilePath, fileSettingsService.watchedFile());
+        }
         logger.info("--> after writing JSON config to node {} with path {}", node, tempFilePath);
     }
 

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

@@ -22,7 +22,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -228,41 +227,7 @@ 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
         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();