Skip to content

Commit

Permalink
SQL: fix *SecurityIT tests by covering edge case scenarios when audit…
Browse files Browse the repository at this point in the history
… file rolls over at midnight (#41328)

* Handle the scenario where assertLogs() is not called from a test method
but the audit rolling file rolls over.
* Use a local boolean variable instead of the static one to account for
assertBusy() code block possibly being called multiple times and having
different execution paths.
  • Loading branch information
astefan committed Apr 18, 2019
1 parent e58cb90 commit 6f64219
Showing 1 changed file with 14 additions and 1 deletion.
Expand Up @@ -188,6 +188,16 @@ public void setInitialAuditLogOffset() {
} catch (IOException e) {
throw new RuntimeException(e);
}

// The log file can roll over without being caught by assertLogs() method: in those tests where exceptions are being handled
// and no audit logs being read (and, thus, assertLogs() is not called) - for example testNoMonitorMain() method: there are no
// calls to auditLogs(), and the method could run while the audit file is rolled over.
// If this happens, next call to auditLogs() will make the tests read from the rolled over file using the main audit file
// offset, which will most likely not going to work since the offset will happen somewhere in the middle of a json line.
if (auditFileRolledOver == false && Files.exists(ROLLED_OVER_AUDIT_LOG_FILE)) {
// once the audit file rolled over, it will stay like this
auditFileRolledOver = true;
}
return null;
});
}
Expand Down Expand Up @@ -568,6 +578,9 @@ public void assertLogs() throws Exception {
assertFalse("Previous test had an audit-related failure. All subsequent audit related assertions are bogus because we can't "
+ "guarantee that we fully cleaned up after the last test.", auditFailure);
try {
// use a second variable since the `assertBusy()` block can be executed multiple times and the
// static auditFileRolledOver value can change and mess up subsequent calls of this code block
boolean localAuditFileRolledOver = auditFileRolledOver;
assertBusy(() -> {
SecurityManager sm = System.getSecurityManager();
if (sm != null) {
Expand All @@ -579,7 +592,7 @@ public void assertLogs() throws Exception {
try {
// the audit log file rolled over during the test
// and we need to consume the rest of the rolled over file plus the new audit log file
if (auditFileRolledOver == false && Files.exists(ROLLED_OVER_AUDIT_LOG_FILE)) {
if (localAuditFileRolledOver == false && Files.exists(ROLLED_OVER_AUDIT_LOG_FILE)) {
// once the audit file rolled over, it will stay like this
auditFileRolledOver = true;
// the order in the array matters, as the readers will be used in that order
Expand Down

0 comments on commit 6f64219

Please sign in to comment.