Add proxy support to RemoteClusterConnection #33062
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds support for connecting to a remote cluster through a tcp proxy. A remote cluster can configured with an additional `search.remote.$clustername.proxy` setting. This proxy will be used to connect to remote nodes for every node connection established. We still try to sniff the remote clsuter and connect to nodes directly through the proxy which has to support some kind of routing to these nodes. Yet, this routing mechanism requires the handshake request to include some kind of information where to route to which is not yet implemented. The effort to use the hostname and an optional node attribute for routing is tracked in elastic#32517 Closes elastic#31840
s1monw
added
>enhancement
:Distributed/Network
|
Pinging @elastic/es-core-infra |
Tim-Brooks
reviewed
Aug 23, 2018
| @@ -77,25 +97,37 @@ protected RemoteClusterAware(Settings settings) { | |||
| this.clusterNameResolver = new ClusterNameExpressionResolver(settings); | |||
| } | |||
|
|
|||
| protected static Map<String, List<Supplier<DiscoveryNode>>> buildRemoteClustersSeeds(Settings settings) { | |||
| protected static Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> buildRemoteClustersSeeds(Settings settings) { | |||
There was a problem hiding this comment.
Maybe a doc comment here? With the new work, it is really tough to figure out what is going on in this method.
| throw new IllegalArgumentException("nodeMap must be non-empty"); | ||
| } | ||
|
|
||
| return new StubbableTransport(MockTransportService.newMockTransport(Settings.EMPTY, Version |
There was a problem hiding this comment.
Can we not extend and override StubbableTransport like this? Ideally maybe the class should be final. It provides methods to attach lambdas for "stubbing" the behavior (although I think the method will need to be made public). The method is either addConnectBehavior or you can add a setDefaultConnectbehavior. Similar to setDefaultSendBehavior.
There was a problem hiding this comment.
The theoretical idea here is to try to move away from overriding methods like crazy at the transport level. So if refactorings need to happen, we can (hopefully) just move the stubs to different locations, opposed to dealing with a billion different tightly couple to the Transport interface tests.
There was a problem hiding this comment.
++ good call. I made the class final.
|
@tbrooks8 I pushed changes for your feedback |
s1monw
added a commit
that referenced
this pull request
Aug 25, 2018
This adds support for connecting to a remote cluster through a tcp proxy. A remote cluster can configured with an additional `search.remote.$clustername.proxy` setting. This proxy will be used to connect to remote nodes for every node connection established. We still try to sniff the remote clsuter and connect to nodes directly through the proxy which has to support some kind of routing to these nodes. Yet, this routing mechanism requires the handshake request to include some kind of information where to route to which is not yet implemented. The effort to use the hostname and an optional node attribute for routing is tracked in #32517 Closes #31840
dnhatn
added a commit
that referenced
this pull request
Aug 26, 2018
* master: Add proxy support to RemoteClusterConnection (#33062) TEST: Skip assertSeqNos for closed shards (#33130) TEST: resync operation on replica should acquire shard permit (#33103) Switch remaining x-pack tests to new style Requests (#33108) Switch remaining tests to new style Requests (#33109) Switch remaining ml tests to new style Requests (#33107) Build: Line up IDE detection logic Security index expands to a single replica (#33131) HLRC: request/response homogeneity and JavaDoc improvements (#33133) Checkstyle! [Test] Fix sporadic failure in MembershipActionTests Revert "Do NOT allow termvectors on nested fields (#32728)" [Rollup] Move toAggCap() methods out of rollup config objects (#32583) Fix race condition in scheduler engine test
dnhatn
added a commit
that referenced
this pull request
Aug 26, 2018
* 6.x: Allow engine to recover from translog upto a seqno (#33032) TEST: Skip assertSeqNos for closed shards (#33130) TEST: resync operation on replica should acquire shard permit (#33103) Add proxy support to RemoteClusterConnection (#33062) Build: Line up IDE detection logic Security index expands to a single replica (#33131) Suppress more tests HLRC: request/response homogeneity and JavaDoc improvements (#33133) [Rollup] Move toAggCap() methods out of rollup config objects (#32583) Muted all these tests due to #33128 Fix race condition in scheduler engine test
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Aug 26, 2018
* master: Fix a mappings update test (elastic#33146) Reload Secure Settings REST specs & docs (elastic#32990) Refactor CachingUsernamePassword realm (elastic#32646) Add proxy support to RemoteClusterConnection (elastic#33062)
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Mar 20, 2019
In elastic#33062 we introduced the `cluster.remote.*.proxy` setting for proxied connections to remote clusters, but left it deliberately undocumented since it needed followup work so that it could work with SNI. However, since elastic#32517 is now closed we can add this documentation and remove the comment about its lack of documentation.
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Mar 28, 2019
In elastic#33062 we introduced the `cluster.remote.*.proxy` setting for proxied connections to remote clusters, but left it deliberately undocumented since it needed followup work so that it could work with SNI. However, since elastic#32517 is now closed we can add this documentation and remove the comment about its lack of documentation.
DaveCTurner
added a commit
that referenced
this pull request
Mar 28, 2019
In #33062 we introduced the `cluster.remote.*.proxy` setting for proxied connections to remote clusters, but left it deliberately undocumented since it needed followup work so that it could work with SNI. However, since #32517 is now closed we can add this documentation and remove the comment about its lack of documentation.
DaveCTurner
added a commit
that referenced
this pull request
Mar 28, 2019
In #33062 we introduced the `cluster.remote.*.proxy` setting for proxied connections to remote clusters, but left it deliberately undocumented since it needed followup work so that it could work with SNI. However, since #32517 is now closed we can add this documentation and remove the comment about its lack of documentation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for connecting to a remote cluster through
a tcp proxy. A remote cluster can configured with an additional
search.remote.$clustername.proxysetting. This proxy will be usedto connect to remote nodes for every node connection established.
We still try to sniff the remote clsuter and connect to nodes directly
through the proxy which has to support some kind of routing to these nodes.
Yet, this routing mechanism requires the handshake request to include some
kind of information where to route to which is not yet implemented. The effort
to use the hostname and an optional node attribute for routing is tracked
in #32517
Closes #31840