Fixes session idle timeout API test #153303
Conversation
…ts to increase determinism
jeramysoucy
added
v8.8.0
Team:Security
|
Pinging @elastic/kibana-security (Team:Security) |
jeramysoucy
added
test-failure-flaky
release_note:skip
|
ACK: will review later today or tomorrow! |
| @@ -32,6 +33,7 @@ export default function ({ getService }: FtrProviderContext) { | |||
| provider: AuthenticationProvider | |||
| ) { | |||
| log.debug(`Verifying session cookie for ${username}.`); | |||
| await es.indices.refresh({ index: '.kibana_security_session*' }); | |||
There was a problem hiding this comment.
question: If I understand correctly the refresh doesn't affect the /internal/security/me call that goes after it, since it retrieves session by ID. Shouldn't we instead call the refresh inside runCleanupTaskSoon, where we need to make sure that the search operation that cleanup does relies on the latest data?
There was a problem hiding this comment.
That makes sense. I don't think we even need it in runCleanupTaskSoon though either because we have such long wait times anyhow. We're on the second iteration of the cleanup task by the time the session expires.
| @@ -93,6 +99,7 @@ export default function ({ getService }: FtrProviderContext) { | |||
| it('should extend the session', async () => { | |||
| // browsers will follow the redirect and return the new session info, but this testing framework does not | |||
| // we simulate that behavior in this test by sending another GET request | |||
| await setTimeoutAsync(200); // wait a bit | |||
There was a problem hiding this comment.
nit: maybe it'd make more sense to have this delay right before we try to extend session and add a comment in the code to explain why we have it (what you mentioned in the issue description does a great job explaining why we need this delay).
There was a problem hiding this comment.
Sure! We will have to move the first getSessionInfo call with it, otherwise we'll end up with the same issue as before. But I agree, bringing this all to one block with a better comment is a great idea.
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
|
@azasypkin Thanks for the suggestions! I've made some updates and re-ran the flaky test runner (see link in desc). All looks good if you are ready to approve. |
Closes elastic#152260 Closes elastic#121482 Closes elastic#136688 ## Description - Adds security index refresh to `getSessionInfo` to ensure each query is running on updated data - Triggers the cleanup routine just before checking idle session timeouts to increase determinism (same methodology used in concurrent sessions tests) - Adds a short static delay when testing session extend to ensure the original session time has somewhat elapsed (when this was failing it was only by a few milliseconds) ### Tests x-pack/test/security_api_integration/session_idle.config.ts... - 'should properly clean up session expired because of idle timeout' - 'should properly clean up session expired because of idle timeout when providers override global session config' - 'should extend the session' ### Flaky Test Runner https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2031
Closes #152260
Closes #121482
Closes #136688
Description
getSessionInfoto ensure each query is running on updated dataTests
x-pack/test/security_api_integration/session_idle.config.ts...
Flaky Test Runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2031