From 8e6ef4aacf60a6b80ff87e90f9f51f42a0b1a72d Mon Sep 17 00:00:00 2001 From: John Murret Date: Fri, 13 Oct 2023 14:25:00 -0600 Subject: [PATCH] PR feedback - LocalConnection and InboundConnection do not affect exposed routes. configure L7 route destinations. fix connection proto sequence numbers. --- .../sidecarproxy/builder/expose_paths.go | 2 +- .../sidecarproxy/builder/local_app.go | 25 ++- .../sidecarproxy/builder/local_app_test.go | 21 ++- .../local-and-inbound-connections.golden | 154 +++++++++++++++++- proto-public/pbmesh/v2beta1/connection.pb.go | 8 +- proto-public/pbmesh/v2beta1/connection.proto | 4 +- 6 files changed, 193 insertions(+), 21 deletions(-) diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/expose_paths.go b/internal/mesh/internal/controllers/sidecarproxy/builder/expose_paths.go index 61aa666e73bc6..6215ab540ca54 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/expose_paths.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/expose_paths.go @@ -22,7 +22,7 @@ func (b *Builder) buildExposePaths(workload *pbcatalog.Workload) { buildListener() b.addExposePathsRoute(exposePath, clusterName). - addLocalAppCluster(clusterName, fmt.Sprintf("%d", exposePath.LocalPathPort)). + addLocalAppCluster(clusterName, nil). addLocalAppStaticEndpoints(clusterName, exposePath.LocalPathPort) } } diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go index 1dcb25e23c9d3..6bde969346d12 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app.go @@ -40,7 +40,7 @@ func (b *Builder) BuildLocalApp(workload *pbcatalog.Workload, ctp *pbauth.Comput if isL7(port.Protocol) { b.addLocalAppRoute(routeName, clusterName) } - b.addLocalAppCluster(clusterName, portName). + b.addLocalAppCluster(clusterName, &portName). addLocalAppStaticEndpoints(clusterName, port.GetPort()) } } @@ -267,7 +267,7 @@ func (b *Builder) addInboundListener(name string, workload *pbcatalog.Workload) // Add TLS inspection capability to be able to parse ALPN and/or SNI information from inbound connections. listener.Capabilities = append(listener.Capabilities, pbproxystate.Capability_CAPABILITY_L4_TLS_INSPECTION) - if b.proxyCfg != nil && b.proxyCfg.DynamicConfig != nil && b.proxyCfg.DynamicConfig.InboundConnections != nil { + if b.proxyCfg.GetDynamicConfig() != nil && b.proxyCfg.GetDynamicConfig().InboundConnections != nil { listener.BalanceConnections = pbproxystate.BalanceConnections(b.proxyCfg.DynamicConfig.InboundConnections.BalanceInboundConnections) } return b.NewListenerBuilder(listener) @@ -301,9 +301,9 @@ func (l *ListenerBuilder) addInboundRouter(clusterName string, routeName string, if ic != nil { // MaxInboundConnections is uint32 that is used on: - // - router destinations MaxInboundConnection (uint64) - // - cluster circuit breakers UpstreamLimits.MaxConnections (uint32) - // It is cast to a uint64 here similarly as it is to the proxystateconverter code + // - router destinations MaxInboundConnection (uint64). + // - cluster circuit breakers UpstreamLimits.MaxConnections (uint32). + // It is cast to a uint64 here similarly as it is to the proxystateconverter code. r.GetL4().MaxInboundConnections = uint64(ic.MaxInboundConnections) } @@ -326,6 +326,13 @@ func (l *ListenerBuilder) addInboundRouter(clusterName string, routeName string, AlpnProtocols: []string{getAlpnProtocolFromPortName(portName)}, }, } + + if ic != nil { + // MaxInboundConnections is cast to a uint64 here similarly as it is to the + // as the L4 case statement above and in proxystateconverter code. + r.GetL7().MaxInboundConnections = uint64(ic.MaxInboundConnections) + } + l.listener.Routers = append(l.listener.Routers, r) } return l @@ -391,7 +398,7 @@ func isL7(protocol pbcatalog.Protocol) bool { return false } -func (b *Builder) addLocalAppCluster(clusterName, portName string) *Builder { +func (b *Builder) addLocalAppCluster(clusterName string, portName *string) *Builder { // Make cluster for this router destination. cluster := &pbproxystate.Cluster{ Group: &pbproxystate.Cluster_EndpointGroup{ @@ -404,8 +411,8 @@ func (b *Builder) addLocalAppCluster(clusterName, portName string) *Builder { } // configure inbound connections or connection timeout if either is defined - if b.proxyCfg.GetDynamicConfig() != nil { - lc, lcOK := b.proxyCfg.DynamicConfig.LocalConnection[portName] + if b.proxyCfg.GetDynamicConfig() != nil && portName != nil { + lc, lcOK := b.proxyCfg.DynamicConfig.LocalConnection[*portName] if lcOK || b.proxyCfg.DynamicConfig.InboundConnections != nil { cluster.GetEndpointGroup().GetStatic().Config = &pbproxystate.StaticEndpointGroupConfig{} @@ -429,7 +436,7 @@ func (b *Builder) addLocalAppCluster(clusterName, portName string) *Builder { } func (b *Builder) addBlackHoleCluster() *Builder { - return b.addLocalAppCluster(xdscommon.BlackHoleClusterName, "") + return b.addLocalAppCluster(xdscommon.BlackHoleClusterName, nil) } func (b *Builder) addLocalAppStaticEndpoints(clusterName string, port uint32) { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go index 6a1fb93835dc0..33dcab7155228 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/local_app_test.go @@ -142,6 +142,9 @@ func TestBuildLocalApp_WithProxyConfiguration(t *testing.T) { }, }, }, + // source/local-and-inbound-connections shows that configuring LocalCOnnection + // and InboundConnections in DynamicConfig will set fields on standard clusters and routes, + // but will not set fields on exposed path clusters and routes. "source/local-and-inbound-connections": { workload: &pbcatalog.Workload{ Addresses: []*pbcatalog.WorkloadAddress{ @@ -152,7 +155,7 @@ func TestBuildLocalApp_WithProxyConfiguration(t *testing.T) { Ports: map[string]*pbcatalog.WorkloadPort{ "port1": {Port: 8080, Protocol: pbcatalog.Protocol_PROTOCOL_TCP}, "port2": {Port: 20000, Protocol: pbcatalog.Protocol_PROTOCOL_MESH}, - "port3": {Port: 8081, Protocol: pbcatalog.Protocol_PROTOCOL_TCP}, + "port3": {Port: 8081, Protocol: pbcatalog.Protocol_PROTOCOL_HTTP}, }, }, proxyCfg: &pbmesh.ComputedProxyConfiguration{ @@ -169,6 +172,22 @@ func TestBuildLocalApp_WithProxyConfiguration(t *testing.T) { MaxInboundConnections: 123, BalanceInboundConnections: pbmesh.BalanceConnections(pbproxystate.BalanceConnections_BALANCE_CONNECTIONS_EXACT), }, + ExposeConfig: &pbmesh.ExposeConfig{ + ExposePaths: []*pbmesh.ExposePath{ + { + ListenerPort: 1234, + Path: "/health", + LocalPathPort: 9090, + Protocol: pbmesh.ExposePathProtocol_EXPOSE_PATH_PROTOCOL_HTTP, + }, + { + ListenerPort: 1235, + Path: "GetHealth", + LocalPathPort: 9091, + Protocol: pbmesh.ExposePathProtocol_EXPOSE_PATH_PROTOCOL_HTTP2, + }, + }, + }, }, }, }, diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/local-and-inbound-connections.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/local-and-inbound-connections.golden index 757a2caa8f40c..bd9820f7c6b20 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/local-and-inbound-connections.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/local-and-inbound-connections.golden @@ -1,6 +1,18 @@ { "proxyState": { "clusters": { + "exposed_cluster_9090": { + "endpointGroup": { + "static": {} + }, + "name": "exposed_cluster_9090" + }, + "exposed_cluster_9091": { + "endpointGroup": { + "static": {} + }, + "name": "exposed_cluster_9091" + }, "local_app:port1": { "endpointGroup": { "static": { @@ -33,6 +45,26 @@ } }, "endpoints": { + "exposed_cluster_9090": { + "endpoints": [ + { + "hostPort": { + "host": "127.0.0.1", + "port": 9090 + } + } + ] + }, + "exposed_cluster_9091": { + "endpoints": [ + { + "hostPort": { + "host": "127.0.0.1", + "port": 9091 + } + } + ] + }, "local_app:port1": { "endpoints": [ { @@ -116,12 +148,13 @@ } } }, - "l4": { - "cluster": { - "name": "local_app:port3" + "l7": { + "route": { + "name": "public_listener:port3" }, "maxInboundConnections": 123, "statPrefix": "public_listener", + "staticRoute": true, "trafficPermissions": {} }, "match": { @@ -131,8 +164,121 @@ } } ] + }, + { + "direction": "DIRECTION_INBOUND", + "hostPort": { + "host": "10.0.0.1", + "port": 9090 + }, + "name": "exposed_path_health", + "routers": [ + { + "l7": { + "route": { + "name": "exposed_path_filter_health_1234" + }, + "statPrefix": "exposed_path_filter_health_1234", + "staticRoute": true + } + } + ] + }, + { + "direction": "DIRECTION_INBOUND", + "hostPort": { + "host": "10.0.0.1", + "port": 9091 + }, + "name": "exposed_path_GetHealth", + "routers": [ + { + "l7": { + "protocol": "L7_PROTOCOL_HTTP2", + "route": { + "name": "exposed_path_filter_GetHealth_1235" + }, + "statPrefix": "exposed_path_filter_GetHealth_1235", + "staticRoute": true + } + } + ] } - ] + ], + "routes": { + "exposed_path_filter_GetHealth_1235": { + "virtualHosts": [ + { + "domains": [ + "*" + ], + "name": "exposed_path_filter_GetHealth_1235", + "routeRules": [ + { + "destination": { + "cluster": { + "name": "exposed_cluster_9091" + } + }, + "match": { + "pathMatch": { + "exact": "GetHealth" + } + } + } + ] + } + ] + }, + "exposed_path_filter_health_1234": { + "virtualHosts": [ + { + "domains": [ + "*" + ], + "name": "exposed_path_filter_health_1234", + "routeRules": [ + { + "destination": { + "cluster": { + "name": "exposed_cluster_9090" + } + }, + "match": { + "pathMatch": { + "exact": "/health" + } + } + } + ] + } + ] + }, + "public_listener:port3": { + "virtualHosts": [ + { + "domains": [ + "*" + ], + "name": "public_listener:port3", + "routeRules": [ + { + "destination": { + "cluster": { + "name": "local_app:port3" + } + }, + "match": { + "pathMatch": { + "prefix": "/" + } + } + } + ] + } + ] + } + } }, "requiredLeafCertificates": { "test-identity": { diff --git a/proto-public/pbmesh/v2beta1/connection.pb.go b/proto-public/pbmesh/v2beta1/connection.pb.go index 909071da81c7e..591df222fc93e 100644 --- a/proto-public/pbmesh/v2beta1/connection.pb.go +++ b/proto-public/pbmesh/v2beta1/connection.pb.go @@ -137,8 +137,8 @@ type InboundConnectionsConfig struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - MaxInboundConnections uint32 `protobuf:"varint,12,opt,name=max_inbound_connections,json=maxInboundConnections,proto3" json:"max_inbound_connections,omitempty"` - BalanceInboundConnections BalanceConnections `protobuf:"varint,13,opt,name=balance_inbound_connections,json=balanceInboundConnections,proto3,enum=hashicorp.consul.mesh.v2beta1.BalanceConnections" json:"balance_inbound_connections,omitempty"` + MaxInboundConnections uint32 `protobuf:"varint,1,opt,name=max_inbound_connections,json=maxInboundConnections,proto3" json:"max_inbound_connections,omitempty"` + BalanceInboundConnections BalanceConnections `protobuf:"varint,2,opt,name=balance_inbound_connections,json=balanceInboundConnections,proto3,enum=hashicorp.consul.mesh.v2beta1.BalanceConnections" json:"balance_inbound_connections,omitempty"` } func (x *InboundConnectionsConfig) Reset() { @@ -209,11 +209,11 @@ var file_pbmesh_v2beta1_connection_proto_rawDesc = []byte{ 0x0a, 0x18, 0x49, 0x6e, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x43, 0x6f, 0x6e, 0x66, 0x69, 0x67, 0x12, 0x36, 0x0a, 0x17, 0x6d, 0x61, 0x78, 0x5f, 0x69, 0x6e, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x5f, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, - 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x18, 0x0c, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x15, 0x6d, 0x61, 0x78, + 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x15, 0x6d, 0x61, 0x78, 0x49, 0x6e, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x12, 0x71, 0x0a, 0x1b, 0x62, 0x61, 0x6c, 0x61, 0x6e, 0x63, 0x65, 0x5f, 0x69, 0x6e, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x5f, 0x63, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, - 0x73, 0x18, 0x0d, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x31, 0x2e, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, + 0x73, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x31, 0x2e, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2e, 0x63, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x2e, 0x6d, 0x65, 0x73, 0x68, 0x2e, 0x76, 0x32, 0x62, 0x65, 0x74, 0x61, 0x31, 0x2e, 0x42, 0x61, 0x6c, 0x61, 0x6e, 0x63, 0x65, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x52, 0x19, 0x62, 0x61, 0x6c, 0x61, diff --git a/proto-public/pbmesh/v2beta1/connection.proto b/proto-public/pbmesh/v2beta1/connection.proto index 3106dbe733ae9..65cb21e586dd2 100644 --- a/proto-public/pbmesh/v2beta1/connection.proto +++ b/proto-public/pbmesh/v2beta1/connection.proto @@ -17,8 +17,8 @@ message ConnectionConfig { // Referenced by ProxyConfiguration message InboundConnectionsConfig { - uint32 max_inbound_connections = 12; - BalanceConnections balance_inbound_connections = 13; + uint32 max_inbound_connections = 1; + BalanceConnections balance_inbound_connections = 2; } // +kubebuilder:validation:Enum=BALANCE_CONNECTIONS_DEFAULT;BALANCE_CONNECTIONS_EXACT