honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix for ApiKeyIntegTests related to Expired API keys remover (#43477) (#47546) 

When API key is invalidated we do two things first it tries to trigger `ExpiredApiKeysRemover` task
and second, we do index the invalidation for the API key. The index invalidation may happen
before the `ExpiredApiKeysRemover` task is run and in that case, the API key
invalidated will also get deleted. If the `ExpiredApiKeysRemover` runs before the
API key invalidation is indexed then the API key is not deleted and will be
deleted in the future run.
This behavior was not captured in the tests related to `ExpiredApiKeysRemover`
causing intermittent failures.
This commit fixes those tests by checking if the API key invalidated is reported
back when we get API keys after invalidation and perform the checks based on that.

Closes #41747
This commit is contained in:
Yogesh Gaikwad 2019-10-04 13:17:52 +10:00 committed by GitHub
parent 5e4732f2bb
commit d371f9d44d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 40 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
View File

@ -7,6 +7,7 @@
package org.elasticsearch.xpack.security.authc;
import com.google.common.collect.Sets;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
@ -291,9 +292,26 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0));
assertThat(invalidateResponse.getErrors().size(), equalTo(0));
awaitApiKeysRemoverCompletion();
refreshSecurityIndex();
PlainActionFuture<GetApiKeyResponse> getApiKeyResponseListener = new PlainActionFuture<>();
securityClient.getApiKey(GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length, is(2));
Set<String> expectedKeyIds = Sets.newHashSet(createdApiKeys.get(0).getId(), createdApiKeys.get(1).getId());
boolean apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
assertThat(apiKey.getId(), isIn(expectedKeyIds));
if (apiKey.getId().equals(createdApiKeys.get(0).getId())) {
// has been invalidated but not yet deleted by ExpiredApiKeysRemover
assertThat(apiKey.isInvalidated(), is(true));
apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = true;
} else if (apiKey.getId().equals(createdApiKeys.get(1).getId())) {
// active api key
assertThat(apiKey.isInvalidated(), is(false));
}
}
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length,
is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 2 : 1));
client = waitForExpiredApiKeysRemoverTriggerReadyAndGetClient().filterWithHeader(
Collections.singletonMap("Authorization", UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
@ -306,16 +324,24 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
assertThat(listener.get().getInvalidatedApiKeys().size(), is(1));
awaitApiKeysRemoverCompletion();
refreshSecurityIndex();
// Verify that 1st invalidated API key is deleted whereas the next one is not
// Verify that 1st invalidated API key is deleted whereas the next one may be or may not be as it depends on whether update was
// indexed before ExpiredApiKeysRemover ran
getApiKeyResponseListener = new PlainActionFuture<>();
securityClient.getApiKey(GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length, is(1));
ApiKey apiKey = getApiKeyResponseListener.get().getApiKeyInfos()[0];
assertThat(apiKey.getId(), is(createdApiKeys.get(1).getId()));
assertThat(apiKey.isInvalidated(), is(true));
expectedKeyIds = Sets.newHashSet(createdApiKeys.get(1).getId());
apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
assertThat(apiKey.getId(), isIn(expectedKeyIds));
if (apiKey.getId().equals(createdApiKeys.get(1).getId())) {
// has been invalidated but not yet deleted by ExpiredApiKeysRemover
assertThat(apiKey.isInvalidated(), is(true));
apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = true;
}
}
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length,
is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 1 : 0));
}
private Client waitForExpiredApiKeysRemoverTriggerReadyAndGetClient() throws Exception {
@ -338,7 +364,6 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
return internalCluster().client(nodeWithMostRecentRun);
}
@AwaitsFix( bugUrl = "https://github.com/elastic/elasticsearch/issues/41747")
public void testExpiredApiKeysBehaviorWhenKeysExpired1WeekBeforeAnd1DayBefore() throws Exception {
Client client = waitForExpiredApiKeysRemoverTriggerReadyAndGetClient().filterWithHeader(
Collections.singletonMap("Authorization", UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_SUPERUSER,
@ -365,7 +390,7 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
.get();
assertThat(expirationDateUpdatedResponse.getResult(), is(DocWriteResponse.Result.UPDATED));
// Expire the 2nd key such that it cannot be deleted by the remover
// Expire the 2nd key such that it can be deleted by the remover
// hack doc to modify the expiration time to the week before
Instant weekBefore = created.minus(8L, ChronoUnit.DAYS);
assertTrue(Instant.now().isAfter(weekBefore));
@ -384,13 +409,13 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
refreshSecurityIndex();
// Verify get API keys does not return expired and deleted key
// Verify get API keys does not return api keys deleted by ExpiredApiKeysRemover
getApiKeyResponseListener = new PlainActionFuture<>();
securityClient.getApiKey(GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length, is(3));
Set<String> expectedKeyIds = Sets.newHashSet(createdApiKeys.get(0).getId(), createdApiKeys.get(2).getId(),
createdApiKeys.get(3).getId());
boolean apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
assertThat(apiKey.getId(), isIn(expectedKeyIds));
if (apiKey.getId().equals(createdApiKeys.get(0).getId())) {
@ -398,9 +423,10 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
assertTrue(apiKey.getExpiration().isBefore(Instant.now()));
assertThat(apiKey.isInvalidated(), is(false));
} else if (apiKey.getId().equals(createdApiKeys.get(2).getId())) {
// has not been expired as no expiration, but invalidated
// has not been expired as no expiration, is invalidated but not yet deleted by ExpiredApiKeysRemover
assertThat(apiKey.getExpiration(), is(nullValue()));
assertThat(apiKey.isInvalidated(), is(true));
apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = true;
} else if (apiKey.getId().equals(createdApiKeys.get(3).getId())) {
// has not been expired as no expiration, not invalidated
assertThat(apiKey.getExpiration(), is(nullValue()));
@ -409,6 +435,8 @@ public class ApiKeyIntegTests extends SecurityIntegTestCase {
fail("unexpected API key " + apiKey);
}
}
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length,
is((apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover) ? 3 : 2));
}
private void refreshSecurityIndex() throws Exception {