Skip to content

Commit

Permalink
Fix test failure of file role store auto-reload (#56398) (#56802)
Browse files Browse the repository at this point in the history
Ensure assertion is only performed when we can be sure that the desired changes are picked up by the file watcher.
  • Loading branch information
ywangd authored May 15, 2020
1 parent 9fb80d3 commit c66e7ec
Showing 1 changed file with 22 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.close();
Expand Down

0 comments on commit c66e7ec

Please sign in to comment.