From 35519111e8b0ad9dc0752ec57eca37f7639687d3 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Mon, 12 Apr 2021 11:46:03 -0400 Subject: [PATCH] internal/dag: Add weight processing to HTTPRoute.ForwardTos Adds weight processing to the HTTPRoute.ForwardTos. If not specified, defaults to the API default of "1". If set to zero, and all other services are also set to zero or are invalid, an HTTP 503 response is returned. Fixes #3560 Signed-off-by: Steve Sloka --- internal/dag/builder_test.go | 259 +++++++++++++++++++++++++++ internal/dag/gatewayapi_processor.go | 10 +- 2 files changed, 268 insertions(+), 1 deletion(-) diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index 05328f56de5..01252e59cbb 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -55,6 +55,36 @@ func TestDAGInsertGatewayAPI(t *testing.T) { }, } + kuardService2 := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kuard2", + Namespace: "projectcontour", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 8080, + TargetPort: intstr.FromInt(8080), + }}, + }, + } + + kuardService3 := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kuard3", + Namespace: "projectcontour", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{ + Name: "http", + Protocol: "TCP", + Port: 8080, + TargetPort: intstr.FromInt(8080), + }}, + }, + } + kuardServiceCustomNs := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "kuard", @@ -286,6 +316,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -328,6 +359,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -366,6 +398,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -433,6 +466,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -479,6 +513,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -525,6 +560,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -560,6 +596,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -609,6 +646,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -659,6 +697,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -694,6 +733,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }, { Matches: []gatewayapi_v1alpha1.HTTPRouteMatch{{ @@ -705,6 +745,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("blogsvc"), Port: gatewayPort(80), + Weight: 1, }}, }}, }, @@ -750,6 +791,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -791,6 +833,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -832,6 +875,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -874,6 +918,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }, }, @@ -910,6 +955,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }, }, @@ -946,6 +992,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }, }, @@ -979,6 +1026,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -1017,6 +1065,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: nil, + Weight: 1, }}, }}, }, @@ -1051,6 +1100,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -1104,6 +1154,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -1152,6 +1203,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -1211,6 +1263,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -1265,6 +1318,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ ServiceName: pointer.StringPtr("kuard"), Port: gatewayPort(8080), + Weight: 1, }}, }}, }, @@ -1285,6 +1339,193 @@ func TestDAGInsertGatewayAPI(t *testing.T) { }, ), }, + "different weights for multiple forwardTos": { + gateway: gatewayWithSelector, + objs: []interface{}{ + kuardService, + kuardService2, + kuardService3, + &gatewayapi_v1alpha1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + Labels: map[string]string{ + "app": "contour", + "type": "controller", + }, + }, + Spec: gatewayapi_v1alpha1.HTTPRouteSpec{ + Rules: []gatewayapi_v1alpha1.HTTPRouteRule{{ + Matches: httpRouteMatch(gatewayapi_v1alpha1.PathMatchPrefix, "/"), + ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{ + { + ServiceName: pointer.StringPtr("kuard"), + Port: gatewayPort(8080), + Weight: 5, + }, { + ServiceName: pointer.StringPtr("kuard2"), + Port: gatewayPort(8080), + Weight: 10, + }, + { + ServiceName: pointer.StringPtr("kuard3"), + Port: gatewayPort(8080), + Weight: 15, + }, + }, + }}, + }, + }, + }, + want: listeners( + &Listener{ + Port: 80, + VirtualHosts: virtualhosts( + virtualhost("*", prefixroute("/", + &Service{ + Weighted: WeightedService{ + Weight: 5, + ServiceName: kuardService.Name, + ServiceNamespace: kuardService.Namespace, + ServicePort: kuardService.Spec.Ports[0], + }, + }, + &Service{ + Weighted: WeightedService{ + Weight: 10, + ServiceName: kuardService2.Name, + ServiceNamespace: kuardService2.Namespace, + ServicePort: kuardService2.Spec.Ports[0], + }, + }, + &Service{ + Weighted: WeightedService{ + Weight: 15, + ServiceName: kuardService3.Name, + ServiceNamespace: kuardService3.Namespace, + ServicePort: kuardService3.Spec.Ports[0], + }, + }, + )), + ), + }, + ), + }, + "one service weight zero w/weights for other forwardTos": { + gateway: gatewayWithSelector, + objs: []interface{}{ + kuardService, + kuardService2, + kuardService3, + &gatewayapi_v1alpha1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + Labels: map[string]string{ + "app": "contour", + "type": "controller", + }, + }, + Spec: gatewayapi_v1alpha1.HTTPRouteSpec{ + Rules: []gatewayapi_v1alpha1.HTTPRouteRule{{ + Matches: httpRouteMatch(gatewayapi_v1alpha1.PathMatchPrefix, "/"), + ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{ + { + ServiceName: pointer.StringPtr("kuard"), + Port: gatewayPort(8080), + Weight: 5, + }, { + ServiceName: pointer.StringPtr("kuard2"), + Port: gatewayPort(8080), + Weight: 0, + }, + { + ServiceName: pointer.StringPtr("kuard3"), + Port: gatewayPort(8080), + Weight: 15, + }, + }, + }}, + }, + }, + }, + want: listeners( + &Listener{ + Port: 80, + VirtualHosts: virtualhosts( + virtualhost("*", prefixroute("/", + &Service{ + Weighted: WeightedService{ + Weight: 5, + ServiceName: kuardService.Name, + ServiceNamespace: kuardService.Namespace, + ServicePort: kuardService.Spec.Ports[0], + }, + }, + &Service{ + Weighted: WeightedService{ + Weight: 0, + ServiceName: kuardService2.Name, + ServiceNamespace: kuardService2.Namespace, + ServicePort: kuardService2.Spec.Ports[0], + }, + }, + &Service{ + Weighted: WeightedService{ + Weight: 15, + ServiceName: kuardService3.Name, + ServiceNamespace: kuardService3.Namespace, + ServicePort: kuardService3.Spec.Ports[0], + }, + }, + )), + ), + }, + ), + }, + "weight of zero for a single forwardTo results in 503": { + gateway: gatewayWithSelector, + objs: []interface{}{ + kuardService, + kuardService2, + kuardService3, + &gatewayapi_v1alpha1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + Labels: map[string]string{ + "app": "contour", + "type": "controller", + }, + }, + Spec: gatewayapi_v1alpha1.HTTPRouteSpec{ + Rules: []gatewayapi_v1alpha1.HTTPRouteRule{{ + Matches: httpRouteMatch(gatewayapi_v1alpha1.PathMatchPrefix, "/"), + ForwardTo: []gatewayapi_v1alpha1.HTTPRouteForwardTo{{ + ServiceName: pointer.StringPtr("kuard"), + Port: gatewayPort(8080), + Weight: 0, + }}, + }}, + }, + }, + }, + want: listeners( + &Listener{ + Port: 80, + VirtualHosts: virtualhosts( + virtualhost("*", directResponseRouteService("/", http.StatusServiceUnavailable, &Service{ + Weighted: WeightedService{ + Weight: 0, + ServiceName: kuardService.Name, + ServiceNamespace: kuardService.Namespace, + ServicePort: kuardService.Spec.Ports[0], + }, + })), + ), + }, + ), + }, } for name, tc := range tests { @@ -9838,6 +10079,24 @@ func directResponseRoute(prefix string, statusCode uint32) *Route { } } +func directResponseRouteService(prefix string, statusCode uint32, first *Service, rest ...*Service) *Route { + services := append([]*Service{first}, rest...) + return &Route{ + PathMatchCondition: prefixString(prefix), + DirectResponse: &DirectResponse{StatusCode: statusCode}, + Clusters: clusters(services...), + } +} + +func httpRouteMatch(pathType gatewayapi_v1alpha1.PathMatchType, value string) []gatewayapi_v1alpha1.HTTPRouteMatch { + return []gatewayapi_v1alpha1.HTTPRouteMatch{{ + Path: gatewayapi_v1alpha1.HTTPPathMatch{ + Type: pathType, + Value: value, + }, + }} +} + func prefixroute(prefix string, first *Service, rest ...*Service) *Route { services := append([]*Service{first}, rest...) return &Route{ diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 89e73349477..3438ea624dc 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -250,6 +250,7 @@ func (p *GatewayAPIProcessor) computeHTTPRoute(route *gatewayapi_v1alpha1.HTTPRo } // Validate the ForwardTos. + totalWeight := int32(0) for _, forward := range rule.ForwardTo { // Verify the service is valid @@ -272,6 +273,11 @@ func (p *GatewayAPIProcessor) computeHTTPRoute(route *gatewayapi_v1alpha1.HTTPRo routeAccessor.AddCondition(status.ConditionResolvedRefs, metav1.ConditionFalse, status.ReasonDegraded, fmt.Sprintf("Service %q does not exist in namespace %q", meta.Name, meta.Namespace)) continue } + + // Apply the weight of the forwardTo. + service.Weighted.Weight = uint32(forward.Weight) + totalWeight += forward.Weight + services = append(services, service) } @@ -279,7 +285,9 @@ func (p *GatewayAPIProcessor) computeHTTPRoute(route *gatewayapi_v1alpha1.HTTPRo for _, vhost := range hosts { vhost := p.dag.EnsureVirtualHost(ListenerName{Name: vhost, ListenerName: "ingress_http"}) for _, route := range routes { - if len(services) == 0 { + // If there aren't any valid services, or the total weight of all of + // them equal zero, then return 503 responses to the caller. + if len(services) == 0 || totalWeight == 0 { // Configure a direct response HTTP status code of 503 so the // route still matches the configured conditions since the // service is missing or invalid.