Review comments.

grpc · Oct 6, 2020 · 608af3c · 608af3c
1 parent 8bcbe48
commit 608af3c
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 15 deletions.
diff --git a/xds/internal/client/client.go b/xds/internal/client/client.go
@@ -222,7 +222,7 @@ type ClusterUpdate struct {
 	// EnableLRS indicates whether or not load should be reported through LRS.
 	EnableLRS bool
 	// SecurityCfg contains security configuration sent by the xDS server.
-	SecurityCfg SecurityConfig
+	SecurityCfg *SecurityConfig
 }
 
 // OverloadDropConfig contains the config to drop overloads.

diff --git a/xds/internal/client/client_cds_test.go b/xds/internal/client/client_cds_test.go
@@ -322,7 +322,7 @@ func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
 			wantUpdate: ClusterUpdate{
 				ServiceName: serviceName,
 				EnableLRS:   false,
-				SecurityCfg: SecurityConfig{
+				SecurityCfg: &SecurityConfig{
 					RootInstanceName: rootPluginInstance,
 					RootCertName:     rootCertName,
 				},
@@ -370,7 +370,7 @@ func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
 			wantUpdate: ClusterUpdate{
 				ServiceName: serviceName,
 				EnableLRS:   false,
-				SecurityCfg: SecurityConfig{
+				SecurityCfg: &SecurityConfig{
 					RootInstanceName:     rootPluginInstance,
 					RootCertName:         rootCertName,
 					IdentityInstanceName: identityPluginInstance,
@@ -428,7 +428,7 @@ func (s) TestValidateClusterWithSecurityConfig(t *testing.T) {
 			wantUpdate: ClusterUpdate{
 				ServiceName: serviceName,
 				EnableLRS:   false,
-				SecurityCfg: SecurityConfig{
+				SecurityCfg: &SecurityConfig{
 					RootInstanceName:     rootPluginInstance,
 					RootCertName:         rootCertName,
 					IdentityInstanceName: identityPluginInstance,

diff --git a/xds/internal/client/client_xds.go b/xds/internal/client/client_xds.go
@@ -407,20 +407,15 @@ func validateCluster(cluster *v3clusterpb.Cluster) (ClusterUpdate, error) {
 		return emptyUpdate, fmt.Errorf("xds: unexpected lbPolicy %v in response: %+v", cluster.GetLbPolicy(), cluster)
 	}
 
-	cu := ClusterUpdate{
-		ServiceName: cluster.GetEdsClusterConfig().GetServiceName(),
-		EnableLRS:   cluster.GetLrsServer().GetSelf() != nil,
-	}
 	sc, err := securityConfigFromCluster(cluster)
 	if err != nil {
 		return emptyUpdate, err
 	}
-	if sc != nil {
-		// A valid Cluster resource could contain no security configuration
-		// (transport_socket field is nil). Hence we need a nil check here.
-		cu.SecurityCfg = *sc
-	}
-	return cu, nil
+	return ClusterUpdate{
+		ServiceName: cluster.GetEdsClusterConfig().GetServiceName(),
+		EnableLRS:   cluster.GetLrsServer().GetSelf() != nil,
+		SecurityCfg: sc,
+	}, nil
 }
 
 // securityConfigFromCluster extracts the relevant security configuration from

diff --git a/xds/internal/version/version.go b/xds/internal/version/version.go
@@ -45,6 +45,6 @@ const (
 	V3RouteConfigURL        = "type.googleapis.com/envoy.config.route.v3.RouteConfiguration"
 	V3ClusterURL            = "type.googleapis.com/envoy.config.cluster.v3.Cluster"
 	V3EndpointsURL          = "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"
-	V3HTTPConnManagerURL    = "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"
+	V3HTTPConnManagerURL    = "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v3.HttpConnectionManager"
 	V3UpstreamTLSContextURL = "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext"
 )