Skip to content

Commit 1b0d246

Browse files
authored
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.
1 parent c5f12c1 commit 1b0d246

File tree

1 file changed

+22
-11
lines changed

1 file changed

+22
-11
lines changed

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java

+22-11
Original file line numberDiff line numberDiff line change
@@ -394,48 +394,59 @@ public void testAutoReload() throws Exception {
394394
assertThat(role.cluster().check("cluster:admin/foo/bar", request, authentication), is(false));
395395

396396
// truncate to remove some
397-
final Set<String> truncatedFileRolesModified = new HashSet<>();
397+
// Not asserting exact content of the role change set since file truncation and subsequent are not
398+
// atomic and hence can result in different change set to be reported.
398399
final CountDownLatch truncateLatch = new CountDownLatch(1);
399400
store = new FileRolesStore(settings, env, watcherService, roleSet -> {
400-
truncatedFileRolesModified.addAll(roleSet);
401-
truncateLatch.countDown();
401+
if (roleSet.contains("dummy1")) {
402+
truncateLatch.countDown();
403+
}
402404
}, new XPackLicenseState(Settings.EMPTY), xContentRegistry());
403405

404406
final Set<String> allRolesPreTruncate = store.getAllRoleNames();
407+
assertTrue(allRolesPreTruncate.contains("role5"));
408+
// Use a marker role so that when the countdown latch is triggered,
409+
// we are sure it is triggered by the new file content instead of the initial truncation
405410
try (BufferedWriter writer = Files.newBufferedWriter(tmp, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING)) {
406411
writer.append("role5:").append(System.lineSeparator());
407412
writer.append(" cluster:").append(System.lineSeparator());
408-
writer.append(" - 'MONITOR'");
413+
writer.append(" - 'MONITOR'").append(System.lineSeparator());
414+
writer.append("dummy1:").append(System.lineSeparator());
415+
writer.append(" cluster:").append(System.lineSeparator());
416+
writer.append(" - 'ALL'");
409417
}
410418

411-
truncateLatch.await();
412-
assertEquals(allRolesPreTruncate.size() - 1, truncatedFileRolesModified.size());
413-
assertTrue(allRolesPreTruncate.contains("role5"));
414-
assertFalse(truncatedFileRolesModified.contains("role5"));
419+
assertTrue(truncateLatch.await(5, TimeUnit.SECONDS));
415420
descriptors = store.roleDescriptors(Collections.singleton("role5"));
416421
assertThat(descriptors, notNullValue());
417422
assertEquals(1, descriptors.size());
423+
assertArrayEquals(new String[]{"MONITOR"}, descriptors.iterator().next().getClusterPrivileges());
418424

419425
// modify
420426
final Set<String> modifiedFileRolesModified = new HashSet<>();
421427
final CountDownLatch modifyLatch = new CountDownLatch(1);
422428
store = new FileRolesStore(settings, env, watcherService, roleSet -> {
423429
modifiedFileRolesModified.addAll(roleSet);
424-
modifyLatch.countDown();
430+
if (roleSet.contains("dummy2")) {
431+
modifyLatch.countDown();
432+
}
425433
}, new XPackLicenseState(Settings.EMPTY), xContentRegistry());
426434

427435
try (BufferedWriter writer = Files.newBufferedWriter(tmp, StandardCharsets.UTF_8, StandardOpenOption.TRUNCATE_EXISTING)) {
428436
writer.append("role5:").append(System.lineSeparator());
429437
writer.append(" cluster:").append(System.lineSeparator());
438+
writer.append(" - 'ALL'").append(System.lineSeparator());
439+
writer.append("dummy2:").append(System.lineSeparator());
440+
writer.append(" cluster:").append(System.lineSeparator());
430441
writer.append(" - 'ALL'");
431442
}
432443

433-
modifyLatch.await();
434-
assertEquals(1, modifiedFileRolesModified.size());
444+
assertTrue(modifyLatch.await(5, TimeUnit.SECONDS));
435445
assertTrue(modifiedFileRolesModified.contains("role5"));
436446
descriptors = store.roleDescriptors(Collections.singleton("role5"));
437447
assertThat(descriptors, notNullValue());
438448
assertEquals(1, descriptors.size());
449+
assertArrayEquals(new String[]{"ALL"}, descriptors.iterator().next().getClusterPrivileges());
439450
} finally {
440451
if (watcherService != null) {
441452
watcherService.stop();

0 commit comments

Comments
 (0)