Skip to content

Commit

Permalink
Fix intermittent failure in ApiKeyIntegTests
Browse files Browse the repository at this point in the history
Few tests failed intermittently and most of the
times due to invalidate or expired keys that were
deleted were still reported in search results.

When ExpiredApiKeysRemover is triggered, the tests
did not await its termination thereby sometimes
the results would be wrong for a search operation.

DELETE_INTERVAL setting has been further reduced to
100ms so we can trigger ExpiredApiKeysRemover faster.

Closes#38408
  • Loading branch information
Yogesh Gaikwad committed Feb 8, 2019
1 parent c921a87 commit 80c69f7
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.support.SecurityIndexManager;

import javax.crypto.SecretKeyFactory;
import java.io.Closeable;
import java.io.IOException;
import java.io.UncheckedIOException;
Expand All @@ -92,6 +91,8 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import javax.crypto.SecretKeyFactory;

import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING;
import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN;
import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;
Expand Down Expand Up @@ -692,7 +693,6 @@ private void findApiKeys(final BoolQueryBuilder boolQuery, boolean filterOutInva
expiredQuery.should(QueryBuilders.boolQuery().mustNot(QueryBuilders.existsQuery("expiration_time")));
boolQuery.filter(expiredQuery);
}

final SearchRequest request = client.prepareSearch(SecurityIndexManager.SECURITY_INDEX_NAME)
.setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
.setQuery(boolQuery)
Expand Down Expand Up @@ -852,10 +852,16 @@ private <E extends Throwable> E traceLog(String action, E exception) {
return exception;
}

// pkg scoped for testing
boolean isExpirationInProgress() {
return expiredApiKeysRemover.isExpirationInProgress();
}

// pkg scoped for testing
long lastTimeWhenApiKeysRemoverWasTriggered() {
return lastExpirationRunMs;
}

private void maybeStartApiKeyRemover() {
if (securityIndex.isAvailable()) {
if (client.threadPool().relativeTimeInMillis() - lastExpirationRunMs > deleteInterval.getMillis()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.bulk.BulkItemResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
Expand Down Expand Up @@ -64,7 +63,6 @@ public void doRun() {
.minimumShouldMatch(1)
);

logger.trace(() -> new ParameterizedMessage("Removing old api keys: [{}]", Strings.toString(expiredDbq)));
executeAsyncWithOrigin(client, SECURITY_ORIGIN, DeleteByQueryAction.INSTANCE, expiredDbq,
ActionListener.wrap(r -> {
debugDbqResponse(r);
Expand Down
Loading

0 comments on commit 80c69f7

Please sign in to comment.