From 1b0d24609a65d1aea07361ae8bef0c84a99cfb01 Mon Sep 17 00:00:00 2001
From: Yang Wang <yang.wang@elastic.co>
Date: Thu, 11 Jun 2020 10:03:46 +1000
Subject: [PATCH] Fix test failure of file role store auto-reload  (#56398)
 (#57961)

Ensure assertion is only performed when we can be sure that the desired changes are picked up by the file watcher.
---
 .../authz/store/FileRolesStoreTests.java      | 33 ++++++++++++-------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java
index f0cc9840e5e13..fe8ffa23474ff 100644
--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java
+++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java
@@ -394,48 +394,59 @@ public void testAutoReload() throws Exception {
             assertThat(role.cluster().check("cluster:admin/foo/bar", request, authentication), is(false));
 
             // truncate to remove some
-            final Set<String> truncatedFileRolesModified = new HashSet<>();
+            // Not asserting exact content of the role change set since file truncation and subsequent are not
+            // atomic and hence can result in different change set to be reported.
             final CountDownLatch truncateLatch = new CountDownLatch(1);
             store = new FileRolesStore(settings, env, watcherService, roleSet -> {
-                truncatedFileRolesModified.addAll(roleSet);
-                truncateLatch.countDown();
+                if (roleSet.contains("dummy1")) {
+                    truncateLatch.countDown();
+                }
             }, new XPackLicenseState(Settings.EMPTY), xContentRegistry());
 
             final Set<String> allRolesPreTruncate = store.getAllRoleNames();
+            assertTrue(allRolesPreTruncate.contains("role5"));
+            // Use a marker role so that when the countdown latch is triggered,
+            // we are sure it is triggered by the new file content instead of the initial truncation
             try (BufferedWriter writer = Files.newBufferedWriter(tmp, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING)) {
                 writer.append("role5:").append(System.lineSeparator());
                 writer.append("  cluster:").append(System.lineSeparator());
-                writer.append("    - 'MONITOR'");
+                writer.append("    - 'MONITOR'").append(System.lineSeparator());
+                writer.append("dummy1:").append(System.lineSeparator());
+                writer.append("  cluster:").append(System.lineSeparator());
+                writer.append("    - 'ALL'");
             }
 
-            truncateLatch.await();
-            assertEquals(allRolesPreTruncate.size() - 1, truncatedFileRolesModified.size());
-            assertTrue(allRolesPreTruncate.contains("role5"));
-            assertFalse(truncatedFileRolesModified.contains("role5"));
+            assertTrue(truncateLatch.await(5, TimeUnit.SECONDS));
             descriptors = store.roleDescriptors(Collections.singleton("role5"));
             assertThat(descriptors, notNullValue());
             assertEquals(1, descriptors.size());
+            assertArrayEquals(new String[]{"MONITOR"}, descriptors.iterator().next().getClusterPrivileges());
 
             // modify
             final Set<String> modifiedFileRolesModified = new HashSet<>();
             final CountDownLatch modifyLatch = new CountDownLatch(1);
             store = new FileRolesStore(settings, env, watcherService, roleSet -> {
                 modifiedFileRolesModified.addAll(roleSet);
-                modifyLatch.countDown();
+                if (roleSet.contains("dummy2")) {
+                    modifyLatch.countDown();
+                }
             }, new XPackLicenseState(Settings.EMPTY), xContentRegistry());
 
             try (BufferedWriter writer = Files.newBufferedWriter(tmp, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING)) {
                 writer.append("role5:").append(System.lineSeparator());
                 writer.append("  cluster:").append(System.lineSeparator());
+                writer.append("    - 'ALL'").append(System.lineSeparator());
+                writer.append("dummy2:").append(System.lineSeparator());
+                writer.append("  cluster:").append(System.lineSeparator());
                 writer.append("    - 'ALL'");
             }
 
-            modifyLatch.await();
-            assertEquals(1, modifiedFileRolesModified.size());
+            assertTrue(modifyLatch.await(5, TimeUnit.SECONDS));
             assertTrue(modifiedFileRolesModified.contains("role5"));
             descriptors = store.roleDescriptors(Collections.singleton("role5"));
             assertThat(descriptors, notNullValue());
             assertEquals(1, descriptors.size());
+            assertArrayEquals(new String[]{"ALL"}, descriptors.iterator().next().getClusterPrivileges());
         } finally {
             if (watcherService != null) {
                 watcherService.stop();