Do not refresh realm cache unless required (#42212)

If there are no realms that depend on the native role mapping store,
then changes should it should not perform any cache refresh.
A refresh with an empty realm array will refresh all realms.

This also fixes a spurious log warning that could occur if the
role mapping store was notified that the security index was recovered
before any realm were attached.

Backport of: #42169
This commit is contained in:
Tim Vernum 2019-05-21 18:14:22 +10:00 committed by GitHub
parent 7abeaba8bb
commit 7b3a9c7033
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 45 additions and 20 deletions

View File

@ -15,6 +15,7 @@ import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.support.ContextPreservingActionListener;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.CheckedBiConsumer;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ThreadContext;
@ -329,7 +330,12 @@ public class NativeRoleMappingStore implements UserRoleMapper {
}
private <Result> void refreshRealms(ActionListener<Result> listener, Result result) {
String[] realmNames = this.realmsToRefresh.toArray(new String[realmsToRefresh.size()]);
if (realmsToRefresh.isEmpty()) {
listener.onResponse(result);
return;
}
final String[] realmNames = this.realmsToRefresh.toArray(Strings.EMPTY_ARRAY);
final SecurityClient securityClient = new SecurityClient(client);
executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN,
securityClient.prepareClearRealmCache().realms(realmNames).request(),
@ -340,7 +346,7 @@ public class NativeRoleMappingStore implements UserRoleMapper {
listener.onResponse(result);
},
ex -> {
logger.warn("Failed to clear cache for realms [{}]", Arrays.toString(realmNames));
logger.warn(new ParameterizedMessage("Failed to clear cache for realms [{}]", Arrays.toString(realmNames)), ex);
listener.onFailure(ex);
}),
securityClient::clearRealmCache);

View File

@ -143,7 +143,7 @@ public class NativeRoleMappingStoreTests extends ESTestCase {
public void testCacheClearOnIndexHealthChange() {
final AtomicInteger numInvalidation = new AtomicInteger(0);
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation);
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation, true);
int expectedInvalidation = 0;
// existing to no longer present
@ -180,7 +180,7 @@ public class NativeRoleMappingStoreTests extends ESTestCase {
public void testCacheClearOnIndexOutOfDateChange() {
final AtomicInteger numInvalidation = new AtomicInteger(0);
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation);
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation, true);
store.onSecurityIndexStateChange(
new SecurityIndexManager.State(Instant.now(), false, true, true, null, concreteSecurityIndexName, null),
@ -193,26 +193,46 @@ public class NativeRoleMappingStoreTests extends ESTestCase {
assertEquals(2, numInvalidation.get());
}
private NativeRoleMappingStore buildRoleMappingStoreForInvalidationTesting(AtomicInteger invalidationCounter) {
public void testCacheIsNotClearedIfNoRealmsAreAttached() {
final AtomicInteger numInvalidation = new AtomicInteger(0);
final NativeRoleMappingStore store = buildRoleMappingStoreForInvalidationTesting(numInvalidation, false);
final SecurityIndexManager.State noIndexState = dummyState(null);
final SecurityIndexManager.State greenIndexState = dummyState(ClusterHealthStatus.GREEN);
store.onSecurityIndexStateChange(noIndexState, greenIndexState);
assertEquals(0, numInvalidation.get());
}
private NativeRoleMappingStore buildRoleMappingStoreForInvalidationTesting(AtomicInteger invalidationCounter, boolean attachRealm) {
final Settings settings = Settings.builder().put("path.home", createTempDir()).build();
final ThreadPool threadPool = mock(ThreadPool.class);
final ThreadContext threadContext = new ThreadContext(settings);
when(threadPool.getThreadContext()).thenReturn(threadContext);
final String realmName = randomAlphaOfLengthBetween(4, 8);
final Client client = mock(Client.class);
when(client.threadPool()).thenReturn(threadPool);
when(client.settings()).thenReturn(settings);
doAnswer(invocationOnMock -> {
assertThat(invocationOnMock.getArguments(), Matchers.arrayWithSize(3));
final ClearRealmCacheRequest request = (ClearRealmCacheRequest) invocationOnMock.getArguments()[1];
assertThat(request.realms(), Matchers.arrayContaining(realmName));
ActionListener<ClearRealmCacheResponse> listener = (ActionListener<ClearRealmCacheResponse>) invocationOnMock.getArguments()[2];
invalidationCounter.incrementAndGet();
listener.onResponse(new ClearRealmCacheResponse(new ClusterName("cluster"), Collections.emptyList(), Collections.emptyList()));
return null;
}).when(client).execute(eq(ClearRealmCacheAction.INSTANCE), any(ClearRealmCacheRequest.class), any(ActionListener.class));
final NativeRoleMappingStore store = new NativeRoleMappingStore(Settings.EMPTY, client, mock(SecurityIndexManager.class),
mock(ScriptService.class));
if (attachRealm) {
final Environment env = TestEnvironment.newEnvironment(settings);
final RealmConfig realmConfig = new RealmConfig(new RealmConfig.RealmIdentifier("ldap", getTestName()),
settings, env, threadContext);
final RealmConfig.RealmIdentifier identifier = new RealmConfig.RealmIdentifier("ldap", realmName);
final RealmConfig realmConfig = new RealmConfig(identifier, settings, env, threadContext);
final CachingUsernamePasswordRealm mockRealm = new CachingUsernamePasswordRealm(realmConfig, threadPool) {
@Override
protected void doAuthenticate(UsernamePasswordToken token, ActionListener<AuthenticationResult> listener) {
@ -224,9 +244,8 @@ public class NativeRoleMappingStoreTests extends ESTestCase {
listener.onResponse(null);
}
};
final NativeRoleMappingStore store = new NativeRoleMappingStore(Settings.EMPTY, client, mock(SecurityIndexManager.class),
mock(ScriptService.class));
store.refreshRealmOnChange(mockRealm);
}
return store;
}
}