From 145d99504510c4ff0d28ce6265a98256032b5be2 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 --- .../builder/destination_builder.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/destination_builder.go b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder.go index 9bb2b1f61299..79dc0d94531c 100644 --- a/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder.go +++ b/internal/mesh/internal/controllers/sidecarproxy/builder/destination_builder.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 a6650f6710e3..d7bddb49eeed 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 @@ -81,7 +81,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 6d85b6dc6e32..9031e4fcb127 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 @@ -135,7 +135,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": { @@ -162,7 +162,7 @@ }, { "direction": "DIRECTION_OUTBOUND", - "name": "api-2:tcp:/path/to/socket", + "name": "default/local/default/api-2:tcp:/path/to/socket", "routers": [ { "l4": { @@ -184,7 +184,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": { @@ -211,7 +211,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 19536f05ccb8..a4dcf6393a1a 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 @@ -81,7 +81,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 37e499a6ae3e..b343e1ee43b0 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 @@ -40,7 +40,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 5ccf817640da..f4ab07f4d43a 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 @@ -167,7 +167,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": { @@ -194,7 +194,7 @@ }, { "direction": "DIRECTION_OUTBOUND", - "name": "api-2:tcp:/path/to/socket", + "name": "default/local/default/api-2:tcp:/path/to/socket", "routers": [ { "l4": { @@ -216,11 +216,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." } } @@ -228,10 +228,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 53aacf89445c..0a51b7990cdf 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 @@ -316,10 +316,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 11d37d6b19ab..b2206ff82673 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 @@ -174,10 +174,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 11d37d6b19ab..b2206ff82673 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 @@ -174,10 +174,10 @@ } ], "routes": { - "outbound_listener": { + "default/local/default/api-app": { "virtualHosts": [ { - "name": "outbound_listener", + "name": "default/local/default/api-app", "routeRules": [ { "destination": {