-
Notifications
You must be signed in to change notification settings - Fork 25k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrap rest httpclient with doPrivileged blocks #22603
Conversation
the change looks good but where do we grant the permission to this client? is this coming in a different PR? |
This client is a dependency of the reindex module. So when I modifying the policy files to officially move connect out of core, reindex will get connect. And rest needs to have the doPrivileged block for that. |
fair enough, so nothing else does a connect in the client? are all connections established once the client is created? |
So I have not found any other place where connections occur in the rest client. Even when you call the "synchronous" request API it is still using the async API (reactor thread) and you are just blocking on the future. But my change here was mostly influenced by how the reindex module was using the client. Other modules (test framework, x-pack) may have different needs. So I'll take a closer look at the client today and see if I am missing anything. |
@tbrooks8 that sounds about right I don't think you are missing anything. |
@tbrooks8 wanna pulls this in? |
Yep. I merged it. |
* master: (131 commits) Replace EngineClosedException with AlreadyClosedExcpetion (elastic#22631) Remove HttpServer and HttpServerAdapter in favor of a simple dispatch method (elastic#22636) move ignore parameter support from yaml test client to low level rest client (elastic#22637) Fix thread safety of Stempel's token filter factory (elastic#22610) Update profile.asciidoc Wrap rest httpclient with doPrivileged blocks (elastic#22603) Revert "Add a deprecation notice to shadow replicas (elastic#22025)" Revert "Don'y use `INDEX_SHARED_FS_ALLOW_RECOVERY_ON_ANY_NODE_SETTING` directly as it triggers (many) deprecation logging" Don'y use `INDEX_SHARED_FS_ALLOW_RECOVERY_ON_ANY_NODE_SETTING` directly as it triggers (many) deprecation logging Add a deprecation notice to shadow replicas (elastic#22025) [Docs] Fix section title in profile.asciidoc ProfileResult and CollectorResult should print machine readable timing information (elastic#22561) Add replica ops with version conflict to translog Replace custom Functional interface in ElasticsearchException with CheckedFunction Make RestChannelConsumer extend CheckedConsumer<RestChannel, Exception> replace ShardSearchRequest.FilterParser functional interface with CheckedFunction replace custom functional interface with CheckedFunction in percolate module [TEST] replace SizeFunction with Function<Integer, Integer> Expose CheckedFunction Expose logs base path ...
This is related to #22116. A number of modules (reindex, etc) use the
rest client. The rest client opens connections using the apache http
client. To avoid throwing
SecurityException
when using theSecurityManager
these operations must be privileged. This is trickybecause connections are opened within the httpclient code on its
reactor thread. The way I confronted this was to wrap the creation
of the client (and creation of reactor thread) in a
doPrivileged
block. The new thread inherits the existing security context.