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
Support TLS Grpc communication between clusters. #11549
Conversation
@KomachiSion Please take a preliminary look to see if the implemented approach is correct. If it is, I will proceed to refine the code and enhance unit tests. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #11549 +/- ##
=============================================
- Coverage 67.85% 67.83% -0.02%
- Complexity 8915 8918 +3
=============================================
Files 1237 1241 +4
Lines 40466 40518 +52
Branches 4291 4285 -6
=============================================
+ Hits 27459 27487 +28
- Misses 11038 11062 +24
Partials 1969 1969
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
May I ask if it can also support TLS RPC communication between raft cluster? |
This PR only provides support for GRPC TLS communication between cluster nodes and does not yet support TLS communication between Raft clusters. |
it should be submit to jraft community |
@@ -34,11 +34,11 @@ | |||
* @version $Id: RpcClientFactory.java, v 0.1 2020年07月14日 3:41 PM liuzunfei Exp $ | |||
*/ | |||
public class RpcClientFactory { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent problem.
Please use nacos code style to reformat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
…lop-#issue-11456
@KomachiSion @shiyiyue1102 Please review. |
…ract ProtocolNegotiatorBuilderSingleton.
# Conflicts: # common/src/test/java/com/alibaba/nacos/common/utils/ExceptionUtilTest.java
* | ||
* @author githubcheng2978. | ||
* @author stone-98 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不要修改作者,可以添加一行author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
改错了🤣,我改回来。
@@ -14,53 +14,60 @@ | |||
* limitations under the License. | |||
*/ | |||
|
|||
package com.alibaba.nacos.core.remote.tls; | |||
package com.alibaba.nacos.common.remote.tls; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server的其实可以不动, 继续放在core里。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
放common里,但是client用不到这个类, 是在增加client的大小
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
但是client中依赖RpcTlsFactory创建TlsClientConfig,所以RpcTlsFactory必须放在common,而RpcTlsFactory又直接依赖它,如果放在core将引用不到。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那应该把RpcTlsFactory进一步拆分, 而且我没找到RpcTlsFactory这个类在哪。。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
是RpcTlsConfigFactory,你的意思是要将他拆分成client的factory和cluster的factory是吗?然后分别位于common和core里面
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done。
/** | ||
* reload ssl context. | ||
*/ | ||
public void reloadProtocolNegotiator() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
为什么要删除这个?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我理解因为之前仅仅支持sdk server tls,所以将它放在GrpcSdkServer,但是新增cluster server tls后,我将它下沉到BaseRpcServer中去了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我理解因为之前仅仅支持sdk server tls,所以将它放在GrpcSdkServer,但是新增cluster server tls后,我将它下沉到BaseRpcServer中去了。
没看到BaseRpcServer中有这个method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -14,53 +14,60 @@ | |||
* limitations under the License. | |||
*/ | |||
|
|||
package com.alibaba.nacos.core.remote.tls; | |||
package com.alibaba.nacos.common.remote.tls; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那应该把RpcTlsFactory进一步拆分, 而且我没找到RpcTlsFactory这个类在哪。。
/** | ||
* reload ssl context. | ||
*/ | ||
public void reloadProtocolNegotiator() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我理解因为之前仅仅支持sdk server tls,所以将它放在GrpcSdkServer,但是新增cluster server tls后,我将它下沉到BaseRpcServer中去了。
没看到BaseRpcServer中有这个method
我把RpcTlsConfigFactory拆分为client和server的,然后调整cluster的参数指定为"nacos.remote.peer.rpc.tls" |
…mote.peer.rpc.tls".
# Conflicts: # client/src/main/java/com/alibaba/nacos/client/config/impl/ClientWorker.java
…op-#issue-11456
What is the purpose of the change
For #11456
Currently, Nacos only supports TLS communication between SDK and Server, and there is a need to support communication between Server instances.
Brief changelog
Overview of Previous Logic
In the original system, only gRPC TLS communication from the SDK to the server was supported, with TLS configurations divided into client (RpcClientTlsConfig) and server (RpcServerTlsConfig) parts. The client had three dependencies on TLS configurations, while the server relied on a protocol negotiator to build TLS configurations.
Dependency Diagram:
Adjustments Made
RpcTlsConfigFactory
interface and its implementation classes to create TLS configurations according to the specific requirements of clients and servers.AbstractProtocolNegotiatorBuilderSingleton
, and implementedSdkProtocolNegotiatorBuilderSingleton
andClusterProtocolNegotiatorBuilderSingleton
based on it, for SDK and cluster protocol negotiation, respectively. Support for TLS gRPC interaction in clusters was added.RpcServerSslContextRefresherHolder
was adjusted to include support for cluster SSL context refresh, enhancing security.Dependency Diagram:
Verifying this change
Demonstrating a three-node cluster
Configuring cluster.conf
The content of cluster.conf is as follows:
Start Nacos on ports 8848, 8850, and 8852 respectively.
Generating certificates & signing
The generated files are as follows:
Starting three Nacos instances
Start each Nacos instance with the following specified parameters:
Result
The cluster is running successfully:
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true
to make sure basic checks pass. Runmvn clean install -DskipITs
to make sure unit-test pass. Runmvn clean test-compile failsafe:integration-test
to make sure integration-test pass.