diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index e851818454..a8a20d4ed7 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -233,6 +233,7 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe BackendRef: &rule.BackendRefs[i].BackendRef, Filters: rule.BackendRefs[i].Filters, } + // ds will never be nil here because processDestination returns an empty DestinationSetting for invalid backendRefs. ds, unstructuredRef, err := t.processDestination(settingName, backendRefCtx, parentRef, httpRoute, resources) if err != nil { // Gateway API conformance: When backendRef Service exists but has no endpoints, @@ -250,13 +251,12 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe )) processDestinationError = err } - continue } if unstructuredRef != nil { backendCustomRefs = append(backendCustomRefs, unstructuredRef) } - // ds can be nil if the backendRef weight is 0 - if ds == nil { + // skip backendRefs with weight 0 as they do not affect the traffic distribution + if ds.Weight != nil && *ds.Weight == 0 { continue } allDs = append(allDs, ds) @@ -275,9 +275,9 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe Metadata: routeRuleMetadata, } switch { - // return 500 if any destination setting is invalid + // return 500 if no valid destination settings exist // the error is already added to the error list when processing the destination - case processDestinationError != nil: + case processDestinationError != nil && destination.ToBackendWeights().Valid == 0: routesWithDirectResponse := sets.New[string]() for _, irRoute := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip @@ -297,9 +297,9 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe "error", processDestinationError, ) } - // return 503 if endpoints does not exist + // return 503 if no ready endpoints exist // the error is already added to the error list when processing the destination - case failedNoReadyEndpoints && len(allDs) == 0: + case failedNoReadyEndpoints && destination.ToBackendWeights().Valid == 0: routesWithDirectResponse := sets.New[string]() for _, irRoute := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip @@ -334,8 +334,8 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe t.Logger.Info("setting 500 direct response in routes due to all valid destinations having 0 weight", "routes", sets.List(routesWithDirectResponse)) } - // A route can only have one destination if this destination is a dynamic resolver, because the behavior of - // multiple destinations with one being a dynamic resolver just doesn't make sense. + // A route can only have one destination if this destination is a dynamic resolver, because the behavior of + // multiple destinations with one being a dynamic resolver just doesn't make sense. case hasDynamicResolver && len(rule.BackendRefs) > 1: routesWithDirectResponse := sets.New[string]() for _, irRoute := range ruleRoutes { @@ -388,10 +388,6 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe } } - // TODO handle: - // - sum of weights for valid backend refs is 0 - // - etc. - irRoutes = append(irRoutes, ruleRoutes...) } if errorCollector.Empty() { @@ -826,6 +822,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe BackendRef: &rule.BackendRefs[i].BackendRef, Filters: rule.BackendRefs[i].Filters, } + // ds will never be nil here because processDestination returns an empty DestinationSetting for invalid backendRefs. ds, _, err := t.processDestination(settingName, backendRefCtx, parentRef, grpcRoute, resources) if err != nil { // Gateway API conformance: When backendRef Service exists but has no endpoints, @@ -843,10 +840,10 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe )) processDestinationError = err } - continue } - if ds == nil { + // skip backendRefs with weight 0 as they do not affect the traffic distribution + if ds.Weight != nil && *ds.Weight == 0 { continue } allDs = append(allDs, ds) @@ -862,7 +859,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe switch { // return 500 if any destination setting is invalid // the error is already added to the error list when processing the destination - case processDestinationError != nil: + case processDestinationError != nil && destination.ToBackendWeights().Valid == 0: routesWithDirectResponse := sets.New[string]() for _, irRoute := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip @@ -883,7 +880,7 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } // return 503 if endpoints does not exist // the error is already added to the error list when processing the destination - case failedNoReadyEndpoints && len(allDs) == 0: + case failedNoReadyEndpoints && destination.ToBackendWeights().Valid == 0: routesWithDirectResponse := sets.New[string]() for _, irRoute := range ruleRoutes { // If the route already has a direct response or redirect configured, then it was from a filter so skip @@ -945,10 +942,6 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } } - // TODO handle: - // - sum of weights for valid backend refs is 0 - // - etc. - irRoutes = append(irRoutes, ruleRoutes...) } @@ -1195,12 +1188,14 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour for i := range rule.BackendRefs { settingName := irDestinationSettingName(destName, i) backendRefCtx := DirectBackendRef{BackendRef: &rule.BackendRefs[i]} + // ds will never be nil here because processDestination returns an empty DestinationSetting for invalid backendRefs. ds, _, err := t.processDestination(settingName, backendRefCtx, parentRef, tlsRoute, resources) if err != nil { resolveErrs.Add(err) continue } - if ds != nil { + // skip backendRefs with weight 0 as they do not affect the traffic distribution + if ds.Weight != nil && *ds.Weight > 0 { destSettings = append(destSettings, ds) } } @@ -1351,14 +1346,15 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour for i := range udpRoute.Spec.Rules[0].BackendRefs { settingName := irDestinationSettingName(destName, i) backendRefCtx := DirectBackendRef{BackendRef: &udpRoute.Spec.Rules[0].BackendRefs[i]} + // ds will never be nil here because processDestination returns an empty DestinationSetting for invalid backendRefs. ds, _, err := t.processDestination(settingName, backendRefCtx, parentRef, udpRoute, resources) if err != nil { resolveErrs.Add(err) continue } - // Skip nil destination settings - if ds != nil { + // skip backendRefs with weight 0 as they do not affect the traffic distribution + if ds.Weight != nil && *ds.Weight > 0 { destSettings = append(destSettings, ds) } } @@ -1507,8 +1503,8 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour resolveErrs.Add(err) continue } - // Skip nil destination settings - if ds != nil { + // skip backendRefs with weight 0 as they do not affect the traffic distribution + if ds.Weight != nil && *ds.Weight > 0 { destSettings = append(destSettings, ds) } } @@ -1613,27 +1609,33 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour func (t *Translator) processDestination(name string, backendRefContext BackendRefContext, parentRef *RouteParentContext, route RouteContext, resources *resource.Resources, ) (ds *ir.DestinationSetting, unstructuredRef *ir.UnstructuredRef, err status.Error) { - routeType := route.GetRouteType() - weight := uint32(1) - backendRef := backendRefContext.GetBackendRef() - if backendRef.Weight != nil { - weight = uint32(*backendRef.Weight) + var ( + routeType = route.GetRouteType() + weight = (uint32(ptr.Deref(backendRefContext.GetBackendRef().Weight, int32(1)))) + backendRef = backendRefContext.GetBackendRef() + ) + + // Create an empty DS without endpoints + // This represents an invalid DS. + emptyDS := &ir.DestinationSetting{ + Name: name, + Weight: &weight, } backendNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace()) if !t.isCustomBackendResource(backendRef.Group, KindDerefOr(backendRef.Kind, resource.KindService)) { err = t.validateBackendRef(backendRefContext, route, resources, backendNamespace, routeType) { - // return with empty endpoint means the backend is invalid and an error to fail the associated route. + // Empty DS means the backend is invalid and an error to fail the associated route. if err != nil { - return nil, nil, err + return emptyDS, nil, err } } } // Skip processing backends with 0 weight if weight == 0 { - return nil, nil, nil + return emptyDS, nil, nil } var envoyProxy *egv1a1.EnvoyProxy @@ -1660,19 +1662,19 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe envoyProxy, ) if tlsErr != nil { - return nil, nil, status.NewRouteStatusError(tlsErr, status.RouteReasonInvalidBackendTLS) + return emptyDS, nil, status.NewRouteStatusError(tlsErr, status.RouteReasonInvalidBackendTLS) } switch KindDerefOr(backendRef.Kind, resource.KindService) { case resource.KindServiceImport: ds, err = t.processServiceImportDestinationSetting(name, backendRef.BackendObjectReference, backendNamespace, protocol, resources, envoyProxy) if err != nil { - return nil, nil, err + return emptyDS, nil, err } case resource.KindService: ds, err = t.processServiceDestinationSetting(name, backendRef.BackendObjectReference, backendNamespace, protocol, resources, envoyProxy) if err != nil { - return nil, nil, err + return emptyDS, nil, err } svc := resources.GetService(backendNamespace, string(backendRef.Name)) ds.IPFamily = getServiceIPFamily(svc) @@ -1687,7 +1689,7 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe // Check if the custom backend resource was found if unstructuredRef == nil { - return nil, nil, status.NewRouteStatusError( + return emptyDS, nil, status.NewRouteStatusError( fmt.Errorf("custom backend %s %s/%s not found", KindDerefOr(backendRef.Kind, resource.KindService), backendNamespace, @@ -1709,11 +1711,11 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe var filtersErr error ds.Filters, filtersErr = t.processDestinationFilters(routeType, backendRefContext, parentRef, route, resources) if filtersErr != nil { - return nil, nil, status.NewRouteStatusError(filtersErr, status.RouteReasonInvalidBackendFilters) + return emptyDS, nil, status.NewRouteStatusError(filtersErr, status.RouteReasonInvalidBackendFilters) } if err := validateDestinationSettings(ds, t.IsEnvoyServiceRouting(envoyProxy), backendRef.Kind); err != nil { - return nil, nil, err + return emptyDS, nil, err } ds.Weight = &weight diff --git a/internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.in.yaml b/internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.in.yaml new file mode 100644 index 0000000000..c7268e9b91 --- /dev/null +++ b/internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.in.yaml @@ -0,0 +1,45 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + namespace: default + name: grpcroute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + sectionName: http + rules: + - matches: + - method: + method: ExampleExact + type: Exact + backendRefs: + - name: service-1 + port: 8080 + - name: service-not-exist + port: 8080 +services: +- apiVersion: v1 + kind: Service + metadata: + name: service-1 + spec: + clusterIP: 7.7.7.7 + ports: + - port: 8080 diff --git a/internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.out.yaml b/internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.out.yaml new file mode 100644 index 0000000000..374daf4fda --- /dev/null +++ b/internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.out.yaml @@ -0,0 +1,177 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +grpcRoutes: +- apiVersion: gateway.networking.k8s.io/v1alpha2 + kind: GRPCRoute + metadata: + name: grpcroute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + - name: service-not-exist + port: 8080 + matches: + - method: + method: ExampleExact + type: Exact + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'Failed to process route rule 0 backendRef 1: service default/service-not-exist + not found.' + reason: BackendNotFound + status: "False" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway + sectionName: http +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + isHTTP2: true + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: GRPCRoute + name: grpcroute-1 + namespace: default + name: grpcroute/default/grpcroute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + name: service-1 + namespace: default + sectionName: "8080" + name: grpcroute/default/grpcroute-1/rule/0/backend/0 + protocol: GRPC + weight: 1 + - name: grpcroute/default/grpcroute-1/rule/0/backend/1 + weight: 1 + headerMatches: + - distinct: false + name: :path + suffix: /ExampleExact + hostname: '*' + isHTTP2: true + metadata: + kind: GRPCRoute + name: grpcroute-1 + namespace: default + name: grpcroute/default/grpcroute-1/rule/0/match/0/* + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-multiple-backend-backendrefs-diff-address-type.out.yaml b/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-multiple-backend-backendrefs-diff-address-type.out.yaml index 7a73b7b2be..e92e385665 100644 --- a/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-multiple-backend-backendrefs-diff-address-type.out.yaml +++ b/internal/gatewayapi/testdata/httproute-attaching-to-listener-with-multiple-backend-backendrefs-diff-address-type.out.yaml @@ -313,8 +313,37 @@ xdsIR: mergeSlashes: true port: 10080 routes: - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - name: httproute/default/httproute-1/rule/0/backend/0 + weight: 1 + - addressType: IP + endpoints: + - host: 1.1.1.1 + port: 3001 + metadata: + kind: Backend + name: backend-ip + namespace: default + name: httproute/default/httproute-1/rule/0/backend/1 + protocol: HTTP + weight: 1 + - addressType: FQDN + endpoints: + - host: primary.foo.com + port: 3000 + metadata: + kind: Backend + name: backend-fqdn + namespace: default + name: httproute/default/httproute-1/rule/0/backend/2 + protocol: HTTP + weight: 1 hostname: '*' isHTTP2: false metadata: @@ -326,8 +355,26 @@ xdsIR: distinct: false name: "" prefix: /1 - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-2 + namespace: default + name: httproute/default/httproute-2/rule/0 + settings: + - name: httproute/default/httproute-2/rule/0/backend/0 + weight: 1 + - addressType: IP + endpoints: + - host: 1.1.1.1 + port: 3001 + metadata: + kind: Backend + name: backend-ip + namespace: default + name: httproute/default/httproute-2/rule/0/backend/1 + protocol: HTTP + weight: 1 hostname: '*' isHTTP2: false metadata: @@ -379,8 +426,26 @@ xdsIR: distinct: false name: "" prefix: /3 - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-3 + namespace: default + name: httproute/default/httproute-3/rule/0 + settings: + - name: httproute/default/httproute-3/rule/0/backend/0 + weight: 1 + - addressType: FQDN + endpoints: + - host: primary.foo.com + port: 3000 + metadata: + kind: Backend + name: backend-fqdn + namespace: default + name: httproute/default/httproute-3/rule/0/backend/1 + protocol: HTTP + weight: 1 hostname: '*' isHTTP2: false metadata: diff --git a/internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.in.yaml b/internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.in.yaml new file mode 100644 index 0000000000..809ce1c5d8 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.in.yaml @@ -0,0 +1,46 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-1 + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + rules: + - matches: + - path: + type: Exact + value: "/exact" + backendRefs: + - name: service-1 + port: 8080 + weight: 80 + - name: service-not-exist + port: 8080 + weight: 20 +services: +- apiVersion: v1 + kind: Service + metadata: + name: service-1 + spec: + clusterIP: 7.7.7.7 + ports: + - port: 8080 diff --git a/internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.out.yaml b/internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.out.yaml new file mode 100644 index 0000000000..9b76cbe1c2 --- /dev/null +++ b/internal/gatewayapi/testdata/httproute-with-mixed-invalid-and-valid-backend-refs.out.yaml @@ -0,0 +1,177 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: httproute-1 + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + rules: + - backendRefs: + - name: service-1 + port: 8080 + weight: 80 + - name: service-not-exist + port: 8080 + weight: 20 + matches: + - path: + type: Exact + value: /exact + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: 'Failed to process route rule 0 backendRef 1: service default/service-not-exist + not found.' + reason: BackendNotFound + status: "False" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway +infraIR: + envoy-gateway/gateway-1: + proxy: + listeners: + - address: null + name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + isHTTP2: false + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/0 + protocol: HTTP + weight: 80 + - name: httproute/default/httproute-1/rule/0/backend/1 + weight: 20 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0/match/0/* + pathMatch: + distinct: false + exact: /exact + name: "" + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/gatewayapi/testdata/httproute-with-some-invalid-backend-refs-no-service.out.yaml b/internal/gatewayapi/testdata/httproute-with-some-invalid-backend-refs-no-service.out.yaml index 73fb824935..1196a28cb8 100644 --- a/internal/gatewayapi/testdata/httproute-with-some-invalid-backend-refs-no-service.out.yaml +++ b/internal/gatewayapi/testdata/httproute-with-some-invalid-backend-refs-no-service.out.yaml @@ -140,8 +140,28 @@ xdsIR: mergeSlashes: true port: 10080 routes: - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - name: httproute/default/httproute-1/rule/0/backend/0 + weight: 1 + - name: httproute/default/httproute-1/rule/0/backend/1 + weight: 1 + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + name: service-1 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/2 + protocol: HTTP + weight: 1 hostname: '*' isHTTP2: false metadata: diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 2e9db1e3cb..2e801c4dec 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -838,6 +838,11 @@ func (h *HTTPRoute) NeedsClusterPerSetting() bool { h.Traffic.LoadBalancer.PreferLocal != nil { return true } + // When the destination has both valid and invalid backend weights, we use weighted clusters to distribute between + // valid backends and the `invalid-backend-cluster` for 500 responses according to their configured weights. + if h.Destination.ToBackendWeights().Invalid > 0 { + return true + } return h.Destination.NeedsClusterPerSetting() } @@ -1655,9 +1660,9 @@ func (r *RouteDestination) ToBackendWeights() *BackendWeights { w.Valid += *s.Weight case s.IsCustomBackend: // Custom backends has no endpoints w.Valid += *s.Weight - case len(s.Endpoints) > 0: + case len(s.Endpoints) > 0: // All other cases should have endpoints w.Valid += *s.Weight - default: + default: // DestinationSetting with no endpoints is considered invalid w.Invalid += *s.Weight } } diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.clusters.yaml index 054f90bb71..92313ce2eb 100644 --- a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.clusters.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.clusters.yaml @@ -8,7 +8,7 @@ edsConfig: ads: {} resourceApiVersion: V3 - serviceName: first-route-dest + serviceName: first-route-dest/backend/0 ignoreHealthOnHostRemoval: true lbPolicy: LEAST_REQUEST loadBalancingPolicy: @@ -19,6 +19,30 @@ '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest localityLbConfig: localityWeightedLbConfig: {} - name: first-route-dest + name: first-route-dest/backend/0 + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_PREFERRED + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: first-route-dest/backend/1 + ignoreHealthOnHostRemoval: true + lbPolicy: LEAST_REQUEST + loadBalancingPolicy: + policies: + - typedExtensionConfig: + name: envoy.load_balancing_policies.least_request + typedConfig: + '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest + localityLbConfig: + localityWeightedLbConfig: {} + name: first-route-dest/backend/1 perConnectionBufferLimitBytes: 32768 type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.endpoints.yaml index 7f8a028132..407ba8414d 100644 --- a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.endpoints.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-invalid-backend.endpoints.yaml @@ -1,4 +1,4 @@ -- clusterName: first-route-dest +- clusterName: first-route-dest/backend/0 endpoints: - lbEndpoints: - endpoint: @@ -10,6 +10,8 @@ loadBalancingWeight: 1 locality: region: first-route-dest/backend/0 +- clusterName: first-route-dest/backend/1 + endpoints: - loadBalancingWeight: 1 locality: region: first-route-dest/backend/1 diff --git a/release-notes/current.yaml b/release-notes/current.yaml index 3271a4f907..a9bcc60d38 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -10,7 +10,7 @@ security updates: | new features: | bug fixes: | - + Fix 500 errors caused by partially invalid BackendRefs; traffic is now correctly routed between valid backends and 500 responses according to their configured weights. # Enhancements that improve performance. performance improvements: | diff --git a/test/e2e/testdata/weighted-backend-mixed-valid-and-invalid.yaml b/test/e2e/testdata/weighted-backend-mixed-valid-and-invalid.yaml new file mode 100644 index 0000000000..d77e8487c4 --- /dev/null +++ b/test/e2e/testdata/weighted-backend-mixed-valid-and-invalid.yaml @@ -0,0 +1,20 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: weight-mixed-valid-and-invalid-http-route + namespace: gateway-conformance-infra +spec: + parentRefs: + - name: same-namespace + rules: + - matches: + - path: + type: PathPrefix + value: /mixed-valid-and-invalid + backendRefs: + - name: infra-backend-v1 + port: 8080 + weight: 80 + - name: infra-backend-not-existing + port: 8080 + weight: 20 diff --git a/test/e2e/tests/weighted_backend.go b/test/e2e/tests/weighted_backend.go index fbce5ddc5a..a2e73cf03b 100644 --- a/test/e2e/tests/weighted_backend.go +++ b/test/e2e/tests/weighted_backend.go @@ -31,6 +31,7 @@ var WeightedBackendTest = suite.ConformanceTest{ "testdata/weighted-backend-all-equal.yaml", "testdata/weighted-backend-bluegreen.yaml", "testdata/weighted-backend-completing-rollout.yaml", + "testdata/weighted-backend-mixed-valid-and-invalid.yaml", }, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { t.Run("SameWeight", func(t *testing.T) { @@ -57,6 +58,11 @@ var WeightedBackendTest = suite.ConformanceTest{ } runWeightedBackendTest(t, suite, nil, "weight-complete-rollout-http-route", "/complete-rollout", "infra-backend", expected) }) + + t.Run("MixedValidAndInvalid", func(t *testing.T) { + // Requests should be distributed to valid and invalid backends according to their weights + testMixedValidAndInvalid(t, suite) + }) }, } @@ -129,3 +135,40 @@ func extractPodNamePrefix(podName, prefix string) string { return podName } + +func testMixedValidAndInvalid(t *testing.T, suite *suite.ConformanceTestSuite) { + weightEqualRoute := types.NamespacedName{Name: "weight-mixed-valid-and-invalid-http-route", Namespace: ConformanceInfraNamespace} + gatewayRef := kubernetes.NewGatewayRef(SameNamespaceGateway) + gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, gatewayRef, weightEqualRoute) + + // Make sure all test resources are ready + kubernetes.NamespacesMustBeReady(t, suite.Client, suite.TimeoutConfig, []string{ConformanceInfraNamespace}) + + expectedResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/mixed-valid-and-invalid", + }, + Namespace: ConformanceInfraNamespace, + } + req := http.MakeRequest(t, &expectedResponse, gwAddr, "HTTP", "http") + + var ( + successCount = 0 + failCount = 0 + ) + for range sendRequests { + _, response, err := suite.RoundTripper.CaptureRoundTrip(req) + if err != nil { + t.Errorf("failed to get expected response: %v", err) + } + if response.StatusCode == 200 { + successCount++ + } else { + failCount++ + } + } + + if !AlmostEquals(successCount, 40, 3) { // The weight of valid backend is 80%, so the expected success count is 50*80%=40 + t.Errorf("The actual success count is not within the expected range, success %d", successCount) + } +}