From dbca544d2590d86fca7ec3d5334a85799f6d3eb4 Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 12 Oct 2023 14:46:31 -0600 Subject: [PATCH] NET-5951 - Unique route names for implicit routes (#19174) * NET-5951 - Unique route names for implicit routes * remove use of datacenter * PR feedback --- .../sidecarproxy/builder/destinations.go | 23 +++++++++-------- .../sidecarproxy/builder/naming.go | 15 ++++++++--- ...it-and-explicit-destinations-tproxy.golden | 2 +- .../destination/l4-multi-destination.golden | 8 +++--- ...le-destination-ip-port-bind-address.golden | 2 +- ...estination-unix-socket-bind-address.golden | 2 +- .../mixed-multi-destination.golden | 12 ++++----- ...ltiple-implicit-destinations-tproxy.golden | 25 +++++++++++++++++-- ...-single-implicit-destination-tproxy.golden | 4 +-- ...tion-with-multiple-workloads-tproxy.golden | 4 +-- 10 files changed, 65 insertions(+), 32 deletions(-) diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/destinations.go b/internal/mesh/internal/controllers/sidecarproxy/builder/destinations.go index 9bb2b1f61299..79dc0d94531c 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/destinations.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/destinations.go @@ -89,6 +89,13 @@ func (b *Builder) buildDestination( defaultDC(""), ) + var routeName string + if destination.Explicit != nil { + routeName = lb.listener.Name + } else { + routeName = DestinationResourceID(cpr.ParentRef.Ref) + } + var ( useRDS bool needsNullRouteCluster bool @@ -100,8 +107,6 @@ func (b *Builder) buildDestination( route := config.Http - listenerName := lb.listener.Name - // this corresponds to roughly "makeUpstreamRouteForDiscoveryChain" var proxyRouteRules []*pbproxystate.RouteRule @@ -134,9 +139,9 @@ func (b *Builder) buildDestination( } } - b.addRoute(listenerName, &pbproxystate.Route{ + b.addRoute(routeName, &pbproxystate.Route{ VirtualHosts: []*pbproxystate.VirtualHost{{ - Name: listenerName, + Name: routeName, RouteRules: proxyRouteRules, }}, }) @@ -145,8 +150,6 @@ func (b *Builder) buildDestination( useRDS = true route := config.Grpc - listenerName := lb.listener.Name - var proxyRouteRules []*pbproxystate.RouteRule for _, routeRule := range route.Rules { for _, backendRef := range routeRule.BackendRefs { @@ -178,9 +181,9 @@ func (b *Builder) buildDestination( } } - b.addRoute(listenerName, &pbproxystate.Route{ + b.addRoute(routeName, &pbproxystate.Route{ VirtualHosts: []*pbproxystate.VirtualHost{{ - Name: listenerName, + Name: routeName, RouteRules: proxyRouteRules, }}, }) @@ -479,7 +482,7 @@ func makeExplicitListener(explicit *pbmesh.Destination, direction pbproxystate.D Port: destinationAddr.IpPort.Port, }, } - listener.Name = DestinationListenerName(explicit.DestinationRef.Name, explicit.DestinationPort, destinationAddr.IpPort.Ip, destinationAddr.IpPort.Port) + listener.Name = DestinationListenerName(explicit.DestinationRef, explicit.DestinationPort, destinationAddr.IpPort.Ip, destinationAddr.IpPort.Port) case *pbmesh.Destination_Unix: destinationAddr := explicit.ListenAddr.(*pbmesh.Destination_Unix) listener.BindAddress = &pbproxystate.Listener_UnixSocket{ @@ -488,7 +491,7 @@ func makeExplicitListener(explicit *pbmesh.Destination, direction pbproxystate.D Mode: destinationAddr.Unix.Mode, }, } - listener.Name = DestinationListenerName(explicit.DestinationRef.Name, explicit.DestinationPort, destinationAddr.Unix.Path, 0) + listener.Name = DestinationListenerName(explicit.DestinationRef, explicit.DestinationPort, destinationAddr.Unix.Path, 0) } return listener diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/naming.go b/internal/mesh/internal/controllers/sidecarproxy/builder/naming.go index 6826191bdf1e..31eba2e7ce88 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/naming.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/naming.go @@ -28,10 +28,19 @@ func DestinationStatPrefix(serviceRef *pbresource.Reference, portName, datacente datacenter) } -func DestinationListenerName(name, portName string, address string, port uint32) string { +func DestinationListenerName(destinationRef *pbresource.Reference, portName string, address string, port uint32) string { + name := fmt.Sprintf("%s:%s:%s", DestinationResourceID(destinationRef), portName, address) if port != 0 { - return fmt.Sprintf("%s:%s:%s:%d", name, portName, address, port) + return fmt.Sprintf("%s:%d", name, port) } - return fmt.Sprintf("%s:%s:%s", name, portName, address) + return name +} + +// DestinationResourceID returns a string representation that uniquely identifies the +// upstream in a canonical but human readable way. +func DestinationResourceID(destinationRef *pbresource.Reference) string { + tenancyPrefix := fmt.Sprintf("%s/%s/%s", destinationRef.Tenancy.Partition, + destinationRef.Tenancy.PeerName, destinationRef.Tenancy.Namespace) + return fmt.Sprintf("%s/%s", tenancyPrefix, destinationRef.Name) } diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden index 9bea581717d3..322ea928f42b 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden @@ -86,7 +86,7 @@ "host": "1.1.1.1", "port": 1234 }, - "name": "api-1:tcp:1.1.1.1:1234", + "name": "default/local/default/api-1:tcp:1.1.1.1:1234", "routers": [ { "l4": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden index 81bf10656ac4..eb7c3e60fcd4 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden @@ -140,7 +140,7 @@ "host": "1.1.1.1", "port": 1234 }, - "name": "api-1:tcp:1.1.1.1:1234", + "name": "default/local/default/api-1:tcp:1.1.1.1:1234", "routers": [ { "l4": { @@ -167,7 +167,7 @@ }, { "direction": "DIRECTION_OUTBOUND", - "name": "api-2:tcp:/path/to/socket", + "name": "default/local/default/api-2:tcp:/path/to/socket", "routers": [ { "l4": { @@ -189,7 +189,7 @@ "host": "1.1.1.1", "port": 2345 }, - "name": "api-1:tcp2:1.1.1.1:2345", + "name": "default/local/default/api-1:tcp2:1.1.1.1:2345", "routers": [ { "l4": { @@ -216,7 +216,7 @@ }, { "direction": "DIRECTION_OUTBOUND", - "name": "api-2:tcp2:/path/to/socket", + "name": "default/local/default/api-2:tcp2:/path/to/socket", "routers": [ { "l4": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden index 5655bb82062c..7ba711123559 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden @@ -86,7 +86,7 @@ "host": "1.1.1.1", "port": 1234 }, - "name": "api-1:tcp:1.1.1.1:1234", + "name": "default/local/default/api-1:tcp:1.1.1.1:1234", "routers": [ { "l4": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden index cd19e4146902..ca72e84ef73c 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden @@ -45,7 +45,7 @@ "listeners": [ { "direction": "DIRECTION_OUTBOUND", - "name": "api-2:tcp:/path/to/socket", + "name": "default/local/default/api-2:tcp:/path/to/socket", "routers": [ { "l4": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden index 37113cbb9874..b1b5b19bff49 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden @@ -172,7 +172,7 @@ "host": "1.1.1.1", "port": 1234 }, - "name": "api-1:tcp:1.1.1.1:1234", + "name": "default/local/default/api-1:tcp:1.1.1.1:1234", "routers": [ { "l4": { @@ -199,7 +199,7 @@ }, { "direction": "DIRECTION_OUTBOUND", - "name": "api-2:tcp:/path/to/socket", + "name": "default/local/default/api-2:tcp:/path/to/socket", "routers": [ { "l4": { @@ -221,11 +221,11 @@ "host": "1.1.1.1", "port": 1234 }, - "name": "api-1:http:1.1.1.1:1234", + "name": "default/local/default/api-1:http:1.1.1.1:1234", "routers": [ { "l7": { - "name": "api-1:http:1.1.1.1:1234", + "name": "default/local/default/api-1:http:1.1.1.1:1234", "statPrefix": "upstream." } } @@ -233,10 +233,10 @@ } ], "routes": { - "api-1:http:1.1.1.1:1234": { + "default/local/default/api-1:http:1.1.1.1:1234": { "virtualHosts": [ { - "name": "api-1:http:1.1.1.1:1234", + "name": "default/local/default/api-1:http:1.1.1.1:1234", "routeRules": [ { "destination": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-multiple-implicit-destinations-tproxy.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-multiple-implicit-destinations-tproxy.golden index 63bf1af9756e..e42968487730 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-multiple-implicit-destinations-tproxy.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-multiple-implicit-destinations-tproxy.golden @@ -321,10 +321,31 @@ } ], "routes": { - "outbound_listener": { + "default/local/default/api-app": { "virtualHosts": [ { - "name": "outbound_listener", + "name": "default/local/default/api-app", + "routeRules": [ + { + "destination": { + "cluster": { + "name": "http.api-app.default.dc1.internal.foo.consul" + } + }, + "match": { + "pathMatch": { + "prefix": "/" + } + } + } + ] + } + ] + }, + "default/local/default/api-app2": { + "virtualHosts": [ + { + "name": "default/local/default/api-app2", "routeRules": [ { "destination": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-tproxy.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-tproxy.golden index 1bcb38a8bff2..1ff271dabb20 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-tproxy.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-tproxy.golden @@ -179,10 +179,10 @@ } ], "routes": { - "outbound_listener": { + "default/local/default/api-app": { "virtualHosts": [ { - "name": "outbound_listener", + "name": "default/local/default/api-app", "routeRules": [ { "destination": { diff --git a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-with-multiple-workloads-tproxy.golden b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-with-multiple-workloads-tproxy.golden index 1bcb38a8bff2..1ff271dabb20 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-with-multiple-workloads-tproxy.golden +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/testdata/destination/multiport-l4-single-implicit-destination-with-multiple-workloads-tproxy.golden @@ -179,10 +179,10 @@ } ], "routes": { - "outbound_listener": { + "default/local/default/api-app": { "virtualHosts": [ { - "name": "outbound_listener", + "name": "default/local/default/api-app", "routeRules": [ { "destination": {