-
Notifications
You must be signed in to change notification settings - Fork 798
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
[Feature] Add Initial Metadata for Sds Grpc Client; Support Sds Grpc Client Multiplexing #1960
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1960 +/- ##
==========================================
+ Coverage 61.51% 61.58% +0.07%
==========================================
Files 398 398
Lines 34343 34391 +48
==========================================
+ Hits 21125 21179 +54
+ Misses 11133 11123 -10
- Partials 2085 2089 +4
Continue to review full report at Codecov.
|
@@ -132,6 +144,11 @@ func (s *fakeSdsServer) StreamSecrets(stream envoy_sds.SecretDiscoveryService_St | |||
log.DefaultLogger.Infof("server receive a ack request: %v", req) | |||
continue | |||
} | |||
if len(s.requiredMetadata) > 0 { |
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里 针对不同的metadata 会返回不同的证书?只是因为目前这个server 没有实现这些逻辑,才这样写的?
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.
作为 sds client 的测试只需要确保 metadata 能够被 server 收到就行了,至于 server 返回不同证书这点,并不影响结果,所以也不需要在 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.
其实这里还是有一些影响。设计的时候应该没有考虑过,就是比如都使用default 作为sds证书的名字,但是一个配置有metadata,一个配置没有metadata,这里就会有问题。
如果说不同的证书一定是不同的名字,那么metadata可能就是用于server去生成证书的参数需要?这里还是要明确一下行为的
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.
其实这里还是有一些影响。设计的时候应该没有考虑过,就是比如都使用default 作为sds证书的名字,但是一个配置有metadata,一个配置没有metadata,这里就会有问题。 如果说不同的证书一定是不同的名字,那么metadata可能就是用于server去生成证书的参数需要?这里还是要明确一下行为的
有这样一个场景,返回的都是应用的证书(也就是证书的名字是同一个),但是在某些场景下需要返回该应用的国密证书,某些场景下需要返回国际证书,所以这个地方就需要设置一些 metadata 信息告知 nodeagent。
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.
fix, 按照设计,现在允许不同 tls 配置共用 default 作为 sds 证书的名字,底层 sds client 实现会根据 tls 配置的哈希值来创建不同的 grpc stream:拉取国际证书 (tls 配置不含 metadata) 和拉取国密证书 (tls 配置包含 metadata) 两个场景将使用不同的 stream,但底层共用同一条 uds 连接。
Related Issue: #1959