Skip to content

Commit

Permalink
NET-5951 - Unique route names for implicit routes
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmurret
jmurret committed Oct 12, 2023
1 parent 54a12ab commit 1671095
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 47 deletions.
53 changes: 28 additions & 25 deletions internal/mesh/internal/controllers/sidecarproxy/builder/destinations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,20 @@ func (b *Builder) buildDestination(

cpr := destination.ComputedPortRoutes

defaultDC := func(dc string) string {
if destination.Explicit != nil {
dc = orDefault(dc, destination.Explicit.Datacenter)
}
dc = orDefault(dc, b.localDatacenter)
if dc != b.localDatacenter {
panic("cross datacenter service discovery clusters are not supported in v2")
}
return dc
}

var lb *ListenerBuilder
if destination.Explicit != nil {
lb = b.addExplicitOutboundListener(destination.Explicit)
lb = b.addExplicitOutboundListener(destination.Explicit, defaultDC(""))
} else {
lb = tproxyOutboundListenerBuilder
}
Expand All @@ -72,23 +83,19 @@ func (b *Builder) buildDestination(
}
}

defaultDC := func(dc string) string {
if destination.Explicit != nil {
dc = orDefault(dc, destination.Explicit.Datacenter)
}
dc = orDefault(dc, b.localDatacenter)
if dc != b.localDatacenter {
panic("cross datacenter service discovery clusters are not supported in v2")
}
return dc
}

statPrefix := DestinationStatPrefix(
cpr.ParentRef.Ref,
cpr.ParentRef.Port,
defaultDC(""),
)

var routeName string
if destination.Explicit != nil {
routeName = lb.listener.Name
} else {
routeName = DestinationResourceID(cpr.ParentRef.Ref, defaultDC(""))
}

var (
useRDS bool
needsNullRouteCluster bool
Expand All @@ -100,8 +107,6 @@ func (b *Builder) buildDestination(

route := config.Http

listenerName := lb.listener.Name

// this corresponds to roughly "makeUpstreamRouteForDiscoveryChain"

var proxyRouteRules []*pbproxystate.RouteRule
Expand Down Expand Up @@ -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,
}},
})
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
}},
})
Expand Down Expand Up @@ -452,13 +455,13 @@ func (b *ListenerBuilder) addL7Router(statPrefix string, protocol pbcatalog.Prot
}

// addExplicitOutboundListener creates an outbound listener for an explicit destination.
func (b *Builder) addExplicitOutboundListener(explicit *pbmesh.Destination) *ListenerBuilder {
listener := makeExplicitListener(explicit, pbproxystate.Direction_DIRECTION_OUTBOUND)
func (b *Builder) addExplicitOutboundListener(explicit *pbmesh.Destination, datacenter string) *ListenerBuilder {
listener := makeExplicitListener(explicit, datacenter, pbproxystate.Direction_DIRECTION_OUTBOUND)

return b.NewListenerBuilder(listener)
}

func makeExplicitListener(explicit *pbmesh.Destination, direction pbproxystate.Direction) *pbproxystate.Listener {
func makeExplicitListener(explicit *pbmesh.Destination, datacenter string, direction pbproxystate.Direction) *pbproxystate.Listener {
if explicit == nil {
panic("explicit upstream required")
}
Expand All @@ -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, datacenter, explicit.DestinationPort, destinationAddr.IpPort.Ip, destinationAddr.IpPort.Port)
case *pbmesh.Destination_Unix:
destinationAddr := explicit.ListenAddr.(*pbmesh.Destination_Unix)
listener.BindAddress = &pbproxystate.Listener_UnixSocket{
Expand All @@ -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, datacenter, explicit.DestinationPort, destinationAddr.Unix.Path, 0)
}

return listener
Expand Down
15 changes: 12 additions & 3 deletions internal/mesh/internal/controllers/sidecarproxy/builder/naming.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, datacenter, portName string, address string, port uint32) string {
if port != 0 {
return fmt.Sprintf("%s:%s:%s:%d", name, portName, address, port)
return fmt.Sprintf("%s:%s:%s:%d", DestinationResourceID(destinationRef, datacenter), portName, address, port)
}

return fmt.Sprintf("%s:%s:%s", name, portName, address)
return fmt.Sprintf("%s:%s:%s", DestinationResourceID(destinationRef, datacenter), portName, address)
}

// XDSResourceID returns a string representation that uniquely identifies the
// upstream in a canonical but human readable way.
func DestinationResourceID(destinationRef *pbresource.Reference, datacenter string) string {
tenancyPrefix := fmt.Sprintf("%s/%s/%s/%s", destinationRef.Tenancy.Partition,
destinationRef.Tenancy.PeerName, destinationRef.Tenancy.Namespace,
datacenter)
return fmt.Sprintf("%s/%s", tenancyPrefix, destinationRef.Name)
}
2 changes: 1 addition & 1 deletion ...carproxy/builder/testdata/destination/l4-implicit-and-explicit-destinations-tproxy.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"host": "1.1.1.1",
"port": 1234
},
"name": "api-1:tcp:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:tcp:1.1.1.1:1234",
"routers": [
{
"l4": {
Expand Down
8 changes: 4 additions & 4 deletions ...nternal/controllers/sidecarproxy/builder/testdata/destination/l4-multi-destination.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@
"host": "1.1.1.1",
"port": 1234
},
"name": "api-1:tcp:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:tcp:1.1.1.1:1234",
"routers": [
{
"l4": {
Expand All @@ -167,7 +167,7 @@
},
{
"direction": "DIRECTION_OUTBOUND",
"name": "api-2:tcp:/path/to/socket",
"name": "default/local/default/dc1/api-2:tcp:/path/to/socket",
"routers": [
{
"l4": {
Expand All @@ -189,7 +189,7 @@
"host": "1.1.1.1",
"port": 2345
},
"name": "api-1:tcp2:1.1.1.1:2345",
"name": "default/local/default/dc1/api-1:tcp2:1.1.1.1:2345",
"routers": [
{
"l4": {
Expand All @@ -216,7 +216,7 @@
},
{
"direction": "DIRECTION_OUTBOUND",
"name": "api-2:tcp2:/path/to/socket",
"name": "default/local/default/dc1/api-2:tcp2:/path/to/socket",
"routers": [
{
"l4": {
Expand Down
2 changes: 1 addition & 1 deletion ...decarproxy/builder/testdata/destination/l4-single-destination-ip-port-bind-address.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"host": "1.1.1.1",
"port": 1234
},
"name": "api-1:tcp:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:tcp:1.1.1.1:1234",
"routers": [
{
"l4": {
Expand Down
2 changes: 1 addition & 1 deletion ...rproxy/builder/testdata/destination/l4-single-destination-unix-socket-bind-address.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"listeners": [
{
"direction": "DIRECTION_OUTBOUND",
"name": "api-2:tcp:/path/to/socket",
"name": "default/local/default/dc1/api-2:tcp:/path/to/socket",
"routers": [
{
"l4": {
Expand Down
12 changes: 6 additions & 6 deletions ...rnal/controllers/sidecarproxy/builder/testdata/destination/mixed-multi-destination.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@
"host": "1.1.1.1",
"port": 1234
},
"name": "api-1:tcp:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:tcp:1.1.1.1:1234",
"routers": [
{
"l4": {
Expand All @@ -199,7 +199,7 @@
},
{
"direction": "DIRECTION_OUTBOUND",
"name": "api-2:tcp:/path/to/socket",
"name": "default/local/default/dc1/api-2:tcp:/path/to/socket",
"routers": [
{
"l4": {
Expand All @@ -221,22 +221,22 @@
"host": "1.1.1.1",
"port": 1234
},
"name": "api-1:http:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:http:1.1.1.1:1234",
"routers": [
{
"l7": {
"name": "api-1:http:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:http:1.1.1.1:1234",
"statPrefix": "upstream."
}
}
]
}
],
"routes": {
"api-1:http:1.1.1.1:1234": {
"default/local/default/dc1/api-1:http:1.1.1.1:1234": {
"virtualHosts": [
{
"name": "api-1:http:1.1.1.1:1234",
"name": "default/local/default/dc1/api-1:http:1.1.1.1:1234",
"routeRules": [
{
"destination": {
Expand Down
25 changes: 23 additions & 2 deletions ...xy/builder/testdata/destination/multiport-l4-multiple-implicit-destinations-tproxy.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,31 @@
}
],
"routes": {
"outbound_listener": {
"default/local/default/dc1/api-app": {
"virtualHosts": [
{
"name": "outbound_listener",
"name": "default/local/default/dc1/api-app",
"routeRules": [
{
"destination": {
"cluster": {
"name": "http.api-app.default.dc1.internal.foo.consul"
}
},
"match": {
"pathMatch": {
"prefix": "/"
}
}
}
]
}
]
},
"default/local/default/dc1/api-app2": {
"virtualHosts": [
{
"name": "default/local/default/dc1/api-app2",
"routeRules": [
{
"destination": {
Expand Down
4 changes: 2 additions & 2 deletions ...proxy/builder/testdata/destination/multiport-l4-single-implicit-destination-tproxy.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@
}
],
"routes": {
"outbound_listener": {
"default/local/default/dc1/api-app": {
"virtualHosts": [
{
"name": "outbound_listener",
"name": "default/local/default/dc1/api-app",
"routeRules": [
{
"destination": {
Expand Down
4 changes: 2 additions & 2 deletions ...estination/multiport-l4-single-implicit-destination-with-multiple-workloads-tproxy.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@
}
],
"routes": {
"outbound_listener": {
"default/local/default/dc1/api-app": {
"virtualHosts": [
{
"name": "outbound_listener",
"name": "default/local/default/dc1/api-app",
"routeRules": [
{
"destination": {
Expand Down

0 comments on commit 1671095

Please sign in to comment.