chore: unify the way to create RouteDestination (#1104)

* chore: unify the way to create RouteDestination * also fixes #786 Signed-off-by: muyuan0 <127020730+muyuan0@users.noreply.github.com>
envoyproxy · Mar 8, 2023 · 2d67b2d · 2d67b2d
1 parent aadebca
commit 2d67b2d
Show file tree

Hide file tree

Showing 16 changed files with 59 additions and 44 deletions.
diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -547,11 +547,11 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour
 					weight = uint32(*backendRef.Weight)
 				}
 
-				routeDestinations = append(routeDestinations, &ir.RouteDestination{
-					Host:   service.Spec.ClusterIP,
-					Port:   uint32(*backendRef.Port),
-					Weight: weight,
-				})
+				routeDestinations = append(routeDestinations, ir.NewRouteDestWithWeight(
+					service.Spec.ClusterIP,
+					uint32(*backendRef.Port),
+					weight,
+				))
 			}
 
 			// TODO handle:
@@ -684,10 +684,10 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour
 		}
 
 		// weight is not used in udp route destinations
-		routeDestinations = append(routeDestinations, &ir.RouteDestination{
-			Host: service.Spec.ClusterIP,
-			Port: uint32(*backendRef.Port),
-		})
+		routeDestinations = append(routeDestinations, ir.NewRouteDest(
+			service.Spec.ClusterIP,
+			uint32(*backendRef.Port),
+		))
 
 		accepted := false
 		for _, listener := range parentRef.listeners {
@@ -809,10 +809,10 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
 		}
 
 		// weight is not used in tcp route destinations
-		routeDestinations = append(routeDestinations, &ir.RouteDestination{
-			Host: service.Spec.ClusterIP,
-			Port: uint32(*backendRef.Port),
-		})
+		routeDestinations = append(routeDestinations, ir.NewRouteDest(
+			service.Spec.ClusterIP,
+			uint32(*backendRef.Port),
+		))
 
 		accepted := false
 		for _, listener := range parentRef.listeners {
@@ -886,11 +886,12 @@ func (t *Translator) processRouteDestination(backendRef v1beta1.BackendRef,
 		return nil, weight
 	}
 
-	return &ir.RouteDestination{
-		Host:   service.Spec.ClusterIP,
-		Port:   uint32(*backendRef.Port),
-		Weight: weight,
-	}, weight
+	// we need to validate backendRef first before using its data to create routeDestination
+	return ir.NewRouteDestWithWeight(
+		service.Spec.ClusterIP,
+		uint32(*backendRef.Port),
+		weight,
+	), weight
 
 }
 

diff --git a/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-tcproutes.out.yaml b/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-tcproutes.out.yaml
@@ -83,7 +83,6 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8163
-            weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-udproutes.out.yaml b/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-udproutes.out.yaml
@@ -83,7 +83,6 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8162
-            weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-tcp-port.out.yaml b/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-tcp-port.out.yaml
@@ -79,7 +79,6 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8163
-            weight: 0
 infraIR:
   envoy-gateway-gateway-1:
     proxy:

diff --git a/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-udp-port.out.yaml b/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-udp-port.out.yaml
@@ -79,7 +79,6 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8162
-            weight: 0
 infraIR:
   envoy-gateway-gateway-1:
     proxy:

diff --git a/.../gatewayapi/testdata/gateway-with-two-listeners-with-same-port-http-tcp-protocol.out.yaml b/.../gatewayapi/testdata/gateway-with-two-listeners-with-same-port-http-tcp-protocol.out.yaml
@@ -120,7 +120,6 @@ xdsIR:
       destinations:
       - host: "7.7.7.7"
         port: 8163
-        weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/.../gatewayapi/testdata/gateway-with-two-listeners-with-same-port-http-udp-protocol.out.yaml b/.../gatewayapi/testdata/gateway-with-two-listeners-with-same-port-http-udp-protocol.out.yaml
@@ -120,7 +120,6 @@ xdsIR:
       destinations:
       - host: "7.7.7.7"
         port: 8162
-        weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/...l/gatewayapi/testdata/gateway-with-two-listeners-with-tcproutes-with-sectionname.out.yaml b/...l/gatewayapi/testdata/gateway-with-two-listeners-with-tcproutes-with-sectionname.out.yaml
@@ -103,14 +103,12 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8163
-            weight: 0
       - name: "envoy-gateway-gateway-1-tcp2-tcproute-2"
         address: "0.0.0.0"
         port: 10163
         destinations:
           - host: "7.7.7.7"
             port: 8163
-            weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/...atewayapi/testdata/gateway-with-two-listeners-with-tcproutes-without-sectionname.out.yaml b/...atewayapi/testdata/gateway-with-two-listeners-with-tcproutes-without-sectionname.out.yaml
@@ -99,14 +99,12 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8163
-            weight: 0
       - name: "envoy-gateway-gateway-1-tcp2-tcproute-1"
         address: "0.0.0.0"
         port: 10162
         destinations:
           - host: "7.7.7.7"
             port: 8163
-            weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/...l/gatewayapi/testdata/gateway-with-two-listeners-with-udproutes-with-sectionname.out.yaml b/...l/gatewayapi/testdata/gateway-with-two-listeners-with-udproutes-with-sectionname.out.yaml
@@ -103,14 +103,12 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8162
-            weight: 0
       - name: "envoy-gateway-gateway-1-udp2-udproute-2"
         address: "0.0.0.0"
         port: 10163
         destinations:
           - host: "7.7.7.7"
             port: 8162
-            weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/...atewayapi/testdata/gateway-with-two-listeners-with-udproutes-without-sectionname.out.yaml b/...atewayapi/testdata/gateway-with-two-listeners-with-udproutes-without-sectionname.out.yaml
@@ -99,14 +99,12 @@ xdsIR:
         destinations:
           - host: "7.7.7.7"
             port: 8162
-            weight: 0
       - name: "envoy-gateway-gateway-1-udp2-udproute-1"
         address: "0.0.0.0"
         port: 10162
         destinations:
           - host: "7.7.7.7"
             port: 8162
-            weight: 0
 
 infraIR:
   envoy-gateway-gateway-1:

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -368,14 +368,15 @@ func (j *JwtRequestAuthentication) Validate() error {
 }
 
 // RouteDestination holds the destination details associated with the route
+// +kubebuilder:object:generate=true
 type RouteDestination struct {
 	// Host refers to the FQDN or IP address of the backend service.
 	Host string
 	// Port on the service to forward the request to.
 	Port uint32
 	// Weight associated with this destination.
-	// Note: Weight is not used in UDP route.
-	Weight uint32
+	// Note: Weight is not used in TCP/UDP route.
+	Weight *uint32
 }
 
 // Validate the fields within the RouteDestination structure
@@ -393,11 +394,18 @@ func (r RouteDestination) Validate() error {
 }
 
 // NewRouteDest creates a new RouteDestination.
-func NewRouteDest(host string, port uint32, weight uint32) *RouteDestination {
+func NewRouteDest(host string, port uint32) *RouteDestination {
+	return &RouteDestination{
+		Host: host,
+		Port: port,
+	}
+}
+
+func NewRouteDestWithWeight(host string, port uint32, weight uint32) *RouteDestination {
 	return &RouteDestination{
 		Host:   host,
 		Port:   port,
-		Weight: weight,
+		Weight: &weight,
 	}
 }
 

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/authentication.go b/internal/xds/translator/authentication.go
@@ -323,7 +323,7 @@ func createJwksClusters(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRoute
 					return err
 				}
 				if existingCluster := findXdsCluster(tCtx, jwks.name); existingCluster == nil {
-					routeDestinations := []*ir.RouteDestination{ir.NewRouteDest(jwks.hostname, jwks.port, 0)}
+					routeDestinations := []*ir.RouteDestination{ir.NewRouteDest(jwks.hostname, jwks.port)}
 					jwksServerCluster := buildXdsCluster(jwks.name, routeDestinations, false /*isHTTP2 */, jwks.isStatic)
 					tSocket, err := buildXdsUpstreamTLSSocket()
 					if err != nil {

diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go
@@ -78,8 +78,8 @@ func buildXdsEndpoints(destinations []*ir.RouteDestination) []*endpointv3.LbEndp
 				},
 			},
 		}
-		if destination.Weight != 0 {
-			lbEndpoint.LoadBalancingWeight = &wrapperspb.UInt32Value{Value: destination.Weight}
+		if destination.Weight != nil {
+			lbEndpoint.LoadBalancingWeight = &wrapperspb.UInt32Value{Value: *destination.Weight}
 		}
 		endpoints = append(endpoints, lbEndpoint)
 	}

diff --git a/internal/xds/translator/ratelimit.go b/internal/xds/translator/ratelimit.go
@@ -266,7 +266,7 @@ func (t *Translator) buildRateLimitServiceCluster(irListener *ir.HTTPListener) *
 
 	clusterName := getRateLimitServiceClusterName()
 	host, port := t.getRateLimitServiceGrpcHostPort()
-	routeDestinations := []*ir.RouteDestination{ir.NewRouteDest(host, uint32(port), 0)}
+	routeDestinations := []*ir.RouteDestination{ir.NewRouteDest(host, uint32(port))}
 	rateLimitServerCluster := buildXdsCluster(clusterName, routeDestinations, true /*isHTTP2 */, false /*isStatic */)
 
 	return rateLimitServerCluster