Skip to content

Fix test failure of file role store auto-reload #56398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 13, 2020
Merged

Fix test failure of file role store auto-reload #56398

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f3c8abb
Fix file role store reload test failure
ywangd May 8, 2020
86e6262
Merge remote-tracking branch 'origin/master' into es-25955-file-role-…
ywangd May 8, 2020
b34e45a
checkstyle
ywangd May 8, 2020
7445bec
Remove forbidden API usage
ywangd May 8, 2020
86ff9f5
address feedback
ywangd May 12, 2020
b757813
Merge remote-tracking branch 'origin/master' into es-25955-file-role-…
ywangd May 12, 2020
ff8aa94
Use marker role for better semantics
ywangd May 12, 2020
4f20dc0
Address feedback
ywangd May 13, 2020
ab37392
Merge branch 'master' into es-25955-file-role-store-test-failure
elasticmachine May 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions ...urity/src/test/java/org/elasticsearch/xpack/security/authz/store/FileRolesStoreTests.java
View file
Open in desktop
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal but you can maintain the modifiedFileRolesModified.addAll(roleSet); from before and assert it contains role5.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added back.

}
}, 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