xds: replace deprecated fields for Upstream and Downstream TlsContext #7010
Conversation
9563cf8 to
009ae80
Compare
| updateBuilder.setUpstreamTlsContext(cluster.getTlsContext()); | ||
| try { | ||
| UpstreamTlsContext upstreamTlsContext = getTlsContextFromCluster(cluster); | ||
| if (upstreamTlsContext != null && upstreamTlsContext.isInitialized()) { |
There was a problem hiding this comment.
isInitialized is always true for proto3 since it checks mandatory fields are set or not. this need to be fixed.
also, can upstreamTlsContext be null?
There was a problem hiding this comment.
isInitialized only checks for mandatory fields? It's quite useless then. I'll then use hasCommonTlsContext() unless that too always returns true :-)
upstreamTlsContext can be null when we switch to v3 protos and cluster.getTlsContext() won't be there anymore so getTlsContextFromCluster will return null in that case.
There was a problem hiding this comment.
it was useful for proto2 because proto2 has required fields. in proto3, all fields are optional.
| Listener xdsListener = Listener.fromEnvoyProtoListener(listener); | ||
| List<EnvoyServerProtoData.FilterChain> filterChains = xdsListener.getFilterChains(); | ||
| EnvoyServerProtoData.FilterChain inFilter = filterChains.get(0); | ||
| assertThat(inFilter).isNotNull(); |
There was a problem hiding this comment.
this always pass or it won't execute this line. i think you should check size of filterChains
| private static DownstreamTlsContext getTlsContextFromFilterChain( | ||
| io.envoyproxy.envoy.api.v2.listener.FilterChain filterChain) | ||
| throws InvalidProtocolBufferException { | ||
| if (filterChain.hasTransportSocket() && "tls" |
There was a problem hiding this comment.
nit: && should be in front
if (condition
&& another_condition) {
...
}
|
@voidzcy do you have any comments? |
No description provided.