Security: use default scroll keepalive (#33639)

Security previously hardcoded a default scroll keepalive of 10 seconds, but in some cases this is not enough time as there can be network issues or overloading of host machines. After this change, security will now use the default keepalive timeout, which is controllable using a setting and the default value is 5 minutes.
elastic · Sep 26, 2018 · a48b86e · a48b86e
1 parent fcb60ac
commit a48b86e
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 11 deletions.
diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/ScrollHelper.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/ScrollHelper.java
@@ -5,14 +5,16 @@
  */
 package org.elasticsearch.xpack.core.security;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.search.ClearScrollRequest;
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.search.SearchScrollRequest;
 import org.elasticsearch.action.support.ContextPreservingActionListener;
 import org.elasticsearch.client.Client;
-import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.search.SearchHit;
 
@@ -25,6 +27,7 @@
 
 public final class ScrollHelper {
 
+    private static final Logger LOGGER = LogManager.getLogger(ScrollHelper.class);
     private ScrollHelper() {}
 
     /**
@@ -35,13 +38,15 @@ public static <T> void fetchAllByEntity(Client client, SearchRequest request, fi
                                             Function<SearchHit, T> hitParser) {
         final List<T> results = new ArrayList<>();
         if (request.scroll() == null) { // we do scroll by default lets see if we can get rid of this at some point.
-            request.scroll(TimeValue.timeValueSeconds(10L));
+            throw new IllegalArgumentException("request must have scroll set");
         }
         final Consumer<SearchResponse> clearScroll = (response) -> {
             if (response != null && response.getScrollId() != null) {
                 ClearScrollRequest clearScrollRequest = new ClearScrollRequest();
                 clearScrollRequest.addScrollId(response.getScrollId());
-                client.clearScroll(clearScrollRequest, ActionListener.wrap((r) -> {}, (e) -> {}));
+                client.clearScroll(clearScrollRequest, ActionListener.wrap((r) -> {}, e ->
+                    LOGGER.warn(new ParameterizedMessage("clear scroll failed for scroll id [{}]", response.getScrollId()), e)
+                ));
             }
         };
         // This function is MADNESS! But it works, don't think about it too hard...

diff --git a/...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java
@@ -118,6 +118,7 @@
 
 import static org.elasticsearch.action.support.TransportActions.isShardNotAvailableException;
 import static org.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
+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;
 
@@ -846,7 +847,7 @@ public void findActiveTokensForRealm(String realmName, ActionListener<Collection
                 );
 
         final SearchRequest request = client.prepareSearch(SecurityIndexManager.SECURITY_INDEX_NAME)
-                .setScroll(TimeValue.timeValueSeconds(10L))
+                .setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
                 .setQuery(boolQuery)
                 .setVersion(false)
                 .setSize(1000)

diff --git a/...urity/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java b/...urity/src/main/java/org/elasticsearch/xpack/security/authc/esnative/NativeUsersStore.java
@@ -28,7 +28,6 @@
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexNotFoundException;
@@ -62,6 +61,7 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
+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;
 import static org.elasticsearch.xpack.core.ClientHelper.stashWithOrigin;
@@ -139,7 +139,7 @@ public void getUsers(String[] userNames, final ActionListener<Collection<User>>
                 final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
                 try (ThreadContext.StoredContext ignore = stashWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN)) {
                     SearchRequest request = client.prepareSearch(SECURITY_INDEX_NAME)
-                            .setScroll(TimeValue.timeValueSeconds(10L))
+                            .setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
                             .setQuery(query)
                             .setSize(1000)
                             .setFetchSource(true)

diff --git a/...in/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java b/...in/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStore.java
@@ -16,7 +16,6 @@
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@@ -56,6 +55,7 @@
 import static org.elasticsearch.action.DocWriteResponse.Result.CREATED;
 import static org.elasticsearch.action.DocWriteResponse.Result.DELETED;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+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;
 import static org.elasticsearch.xpack.core.ClientHelper.stashWithOrigin;
@@ -129,7 +129,7 @@ void loadMappings(ActionListener<List<ExpressionRoleMapping>> listener) {
         final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
         try (ThreadContext.StoredContext ignore = stashWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN)) {
             SearchRequest request = client.prepareSearch(SECURITY_INDEX_NAME)
-                    .setScroll(TimeValue.timeValueSeconds(10L))
+                    .setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
                     .setTypes(SECURITY_GENERIC_TYPE)
                     .setQuery(query)
                     .setSize(1000)

diff --git a/...rity/src/main/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStore.java b/...rity/src/main/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStore.java
@@ -24,7 +24,6 @@
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.CollectionUtils;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.iterable.Iterables;
@@ -56,6 +55,7 @@
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+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;
 import static org.elasticsearch.xpack.core.ClientHelper.stashWithOrigin;
@@ -115,7 +115,7 @@ public void getPrivileges(Collection<String> applications, Collection<String> na
                 final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
                 try (ThreadContext.StoredContext ignore = stashWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN)) {
                     SearchRequest request = client.prepareSearch(SECURITY_INDEX_NAME)
-                        .setScroll(TimeValue.timeValueSeconds(10L))
+                        .setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
                         .setQuery(query)
                         .setSize(1000)
                         .setFetchSource(true)

diff --git a/...security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java b/...security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java
@@ -59,6 +59,7 @@
 
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.elasticsearch.index.query.QueryBuilders.existsQuery;
+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;
 import static org.elasticsearch.xpack.core.ClientHelper.stashWithOrigin;
@@ -120,7 +121,7 @@ public void getRoleDescriptors(String[] names, final ActionListener<Collection<R
                 final Supplier<ThreadContext.StoredContext> supplier = client.threadPool().getThreadContext().newRestorableContext(false);
                 try (ThreadContext.StoredContext ignore = stashWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN)) {
                     SearchRequest request = client.prepareSearch(SecurityIndexManager.SECURITY_INDEX_NAME)
-                            .setScroll(TimeValue.timeValueSeconds(10L))
+                            .setScroll(DEFAULT_KEEPALIVE_SETTING.get(settings))
                             .setQuery(query)
                             .setSize(1000)
                             .setFetchSource(true)

diff --git a/...lugin/security/src/test/java/org/elasticsearch/xpack/security/ScrollHelperIntegTests.java b/...lugin/security/src/test/java/org/elasticsearch/xpack/security/ScrollHelperIntegTests.java
@@ -79,6 +79,7 @@ public void testFetchAllByEntityWithBrokenScroll() {
         when(client.threadPool()).thenReturn(threadPool);
         when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY));
         SearchRequest request = new SearchRequest();
+        request.scroll(TimeValue.timeValueHours(10L));
 
         String scrollId = randomAlphaOfLength(5);
         SearchHit[] hits = new SearchHit[] {new SearchHit(1), new SearchHit(2)};

diff --git a/.../security/src/test/java/org/elasticsearch/xpack/security/audit/index/AuditTrailTests.java b/.../security/src/test/java/org/elasticsearch/xpack/security/audit/index/AuditTrailTests.java
@@ -15,6 +15,7 @@
 import org.elasticsearch.client.Requests;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.test.SecurityIntegTestCase;
@@ -161,6 +162,7 @@ private Collection<Map<String, Object>> getAuditEvents() throws Exception {
         client.admin().indices().refresh(Requests.refreshRequest(indexName)).get();
 
         final SearchRequest request = client.prepareSearch(indexName)
+                .setScroll(TimeValue.timeValueMinutes(10L))
                 .setTypes(IndexAuditTrail.DOC_TYPE)
                 .setQuery(QueryBuilders.matchAllQuery())
                 .setSize(1000)