mirror of
https://github.com/honeymoose/OpenSearch.git
synced 2025-02-23 05:15:04 +00:00
SQL: fix *SecurityIT tests by covering edge case scenarios when audit 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. (cherry picked from commit 6f642196cbab90079c610097befc794746170df1)
This commit is contained in:
parent
0227ac5690
commit
cfed5d65be
@ -188,6 +188,16 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
|
||||
} 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;
|
||||
});
|
||||
}
|
||||
@ -568,6 +578,9 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
|
||||
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) {
|
||||
@ -579,7 +592,7 @@ public abstract class SqlSecurityTestCase extends ESRestTestCase {
|
||||
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
|
||||
|
Loading…
x
Reference in New Issue
Block a user