Skip to content

fix: do not return 500 for all requests when part of BackendRefs are invalid #7488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhaohuabing merged 10 commits into from
Nov 13, 2025
Merged

fix: do not return 500 for all requests when part of BackendRefs are invalid #7488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bbd6e9c
do not return 500 for all requests when part of BackendRefs are invalid
zhaohuabing Nov 11, 2025
f2a16f3
fix test
zhaohuabing Nov 11, 2025
6561c0e
move invalid empty DS to processDestination
zhaohuabing Nov 11, 2025
854fe95
address comments
zhaohuabing Nov 12, 2025
5333c76
Update release-notes/current.yaml
zhaohuabing Nov 12, 2025
9d4a0cc
address comments
zhaohuabing Nov 12, 2025
3410624
add e2e test
zhaohuabing Nov 12, 2025
dd271ed
Merge remote-tracking branch 'upstream/main' into fix-7483
zhaohuabing Nov 12, 2025
833f4c0
fix e2e
zhaohuabing Nov 12, 2025
2df9fdb
modify release note
zhaohuabing Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 42 additions & 40 deletions internal/gatewayapi/route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Copy link
Copy Markdown
Member Author

@zhaohuabing zhaohuabing Nov 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look great - the ir ds.weight should be a value, not a pointer. I'm leaving it as is to avoid touching too many files in this PR and keep focus on the bug fix.

We can clean this up in a follow-up PR.

continue
}
allDs = append(allDs, ds)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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...)
}

Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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,
Expand All @@ -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
Expand Down
45 changes: 45 additions & 0 deletions internal/gatewayapi/testdata/grpcroute-with-mixed-invalid-and-valid-backend-refs.in.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Loading