Fix session cleanup test (#126966)

This commit is contained in:
Joe Portner 2022-03-07 18:00:49 -05:00 committed by GitHub
parent 0821c31fa5
commit 2f83c6516d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 13 deletions

View file

@ -215,6 +215,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).not.toHaveBeenCalled();
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).not.toHaveBeenCalled(); // since the search failed, we don't refresh the index
});
it('throws if bulk delete call to Elasticsearch fails', async () => {
@ -227,7 +228,20 @@ describe('Session index', () => {
expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); // since we attempted to delete sessions, we still refresh the index
});
it('does not throw if index refresh call to Elasticsearch fails', async () => {
const failureReason = new errors.ResponseError(
securityMock.createApiResponse(securityMock.createApiResponse({ body: { type: 'Uh oh.' } }))
);
mockElasticsearchClient.indices.refresh.mockRejectedValue(failureReason);
await sessionIndex.cleanUp();
expect(mockElasticsearchClient.openPointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1); // since we attempted to delete sessions, we still refresh the index
});
it('when neither `lifespan` nor `idleTimeout` is configured', async () => {
@ -388,6 +402,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('when only `idleTimeout` is configured', async () => {
@ -474,6 +489,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('when both `lifespan` and `idleTimeout` are configured', async () => {
@ -570,6 +586,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('when both `lifespan` and `idleTimeout` are configured and multiple providers are enabled', async () => {
@ -714,6 +731,7 @@ describe('Session index', () => {
}
);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('should clean up sessions in batches of 10,000', async () => {
@ -729,6 +747,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(2);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('should limit number of batches to 10', async () => {
@ -742,6 +761,7 @@ describe('Session index', () => {
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(10);
expect(mockElasticsearchClient.bulk).toHaveBeenCalledTimes(10);
expect(mockElasticsearchClient.closePointInTime).toHaveBeenCalledTimes(1);
expect(mockElasticsearchClient.indices.refresh).toHaveBeenCalledTimes(1);
});
it('should log audit event', async () => {

View file

@ -444,20 +444,21 @@ export class SessionIndex {
* Trigger a removal of any outdated session values.
*/
async cleanUp() {
this.options.logger.debug(`Running cleanup routine.`);
const { auditLogger, elasticsearchClient, logger } = this.options;
logger.debug(`Running cleanup routine.`);
let error: Error | undefined;
let indexNeedsRefresh = false;
try {
for await (const sessionValues of this.getSessionValuesInBatches()) {
const operations: Array<Required<Pick<BulkOperationContainer, 'delete'>>> = [];
sessionValues.forEach(({ _id, _source }) => {
const { usernameHash, provider } = _source!;
this.options.auditLogger.log(
sessionCleanupEvent({ sessionId: _id, usernameHash, provider })
);
auditLogger.log(sessionCleanupEvent({ sessionId: _id, usernameHash, provider }));
operations.push({ delete: { _id } });
});
if (operations.length > 0) {
const bulkResponse = await this.options.elasticsearchClient.bulk(
const bulkResponse = await elasticsearchClient.bulk(
{
index: this.indexName,
operations,
@ -471,24 +472,40 @@ export class SessionIndex {
0
);
if (errorCount < bulkResponse.items.length) {
this.options.logger.warn(
logger.warn(
`Failed to clean up ${errorCount} of ${bulkResponse.items.length} invalid or expired sessions. The remaining sessions were cleaned up successfully.`
);
indexNeedsRefresh = true;
} else {
this.options.logger.error(
logger.error(
`Failed to clean up ${bulkResponse.items.length} invalid or expired sessions.`
);
}
} else {
this.options.logger.debug(
`Cleaned up ${bulkResponse.items.length} invalid or expired sessions.`
);
logger.debug(`Cleaned up ${bulkResponse.items.length} invalid or expired sessions.`);
indexNeedsRefresh = true;
}
}
}
} catch (err) {
this.options.logger.error(`Failed to clean up sessions: ${err.message}`);
throw err;
logger.error(`Failed to clean up sessions: ${err.message}`);
error = err;
}
if (indexNeedsRefresh) {
// Only refresh the index if we have actually deleted one or more sessions. The index will auto-refresh eventually anyway, this just
// ensures that searches after the cleanup process are accurate, and this only impacts integration tests.
try {
await elasticsearchClient.indices.refresh({ index: this.indexName });
logger.debug(`Refreshed session index.`);
} catch (err) {
logger.error(`Failed to refresh session index: ${err.message}`);
}
}
if (error) {
// If we couldn't fetch or delete sessions, throw an error so the task will be retried.
throw error;
}
}