Skip to content
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.

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

fix: add unsupported status condition for filters within BackendRef #2620

Merged
merged 8 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions internal/gatewayapi/contexts.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,22 @@ func (r *RouteParentContext) HasCondition(route RouteContext, condType gwapiv1.R
}
return false
}

// BackendRefContext represents a generic BackendRef object (HTTPBackendRef, GRPCBackendRef or BackendRef itself)
type BackendRefContext any

func GetBackendRef(b BackendRefContext) *gwapiv1.BackendRef {
rv := reflect.ValueOf(b)
br := rv.FieldByName("BackendRef")
if br.IsValid() {
backendRef := br.Interface().(gwapiv1.BackendRef)
return &backendRef

}
backendRef := b.(gwapiv1.BackendRef)
return &backendRef
}

func GetFilters(b BackendRefContext) any {
return reflect.ValueOf(b).FieldByName("Filters").Interface()
}
10 changes: 6 additions & 4 deletions internal/gatewayapi/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,15 +704,17 @@ func (t *Translator) processRequestMirrorFilter(

// Wrap the filter's BackendObjectReference into a BackendRef so we can use existing tooling to check it
weight := int32(1)
mirrorBackendRef := gwapiv1.BackendRef{
BackendObjectReference: mirrorBackend,
Weight: &weight,
mirrorBackendRef := gwapiv1.HTTPBackendRef{
BackendRef: gwapiv1.BackendRef{
BackendObjectReference: mirrorBackend,
Weight: &weight,
},
}

// This sets the status on the HTTPRoute, should the usage be changed so that the status message reflects that the backendRef is from the filter?
filterNs := filterContext.Route.GetNamespace()
serviceNamespace := NamespaceDerefOr(mirrorBackend.Namespace, filterNs)
if !t.validateBackendRef(&mirrorBackendRef, filterContext.ParentRef, filterContext.Route,
if !t.validateBackendRef(mirrorBackendRef, filterContext.ParentRef, filterContext.Route,
resources, serviceNamespace, KindHTTPRoute) {
return
}
Expand Down
15 changes: 8 additions & 7 deletions internal/gatewayapi/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe
dstAddrTypeMap := make(map[ir.DestinationAddressType]int)

for _, backendRef := range rule.BackendRefs {
ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, httpRoute, resources)
backendRef := backendRef
ds, backendWeight := t.processDestination(backendRef, parentRef, httpRoute, resources)
if !t.EndpointRoutingDisabled && ds != nil && len(ds.Endpoints) > 0 && ds.AddressType != nil {
dstAddrTypeMap[*ds.AddressType]++
}
Expand Down Expand Up @@ -468,7 +469,8 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe
}

for _, backendRef := range rule.BackendRefs {
ds, backendWeight := t.processDestination(backendRef.BackendRef, parentRef, grpcRoute, resources)
backendRef := backendRef
ds, backendWeight := t.processDestination(backendRef, parentRef, grpcRoute, resources)
for _, route := range ruleRoutes {
// If the route already has a direct response or redirect configured, then it was from a filter so skip
// processing any destinations for this route.
Expand Down Expand Up @@ -1068,20 +1070,19 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
// processDestination takes a backendRef and translates it into destination setting or sets error statuses and
// returns the weight for the backend so that 500 error responses can be returned for invalid backends in
// the same proportion as the backend would have otherwise received
func (t *Translator) processDestination(backendRef gwapiv1.BackendRef,
func (t *Translator) processDestination(backendRefContext BackendRefContext,
parentRef *RouteParentContext,
route RouteContext,
resources *Resources) (ds *ir.DestinationSetting, backendWeight uint32) {

routeType := GetRouteType(route)
weight := uint32(1)
backendRef := GetBackendRef(backendRefContext)
if backendRef.Weight != nil {
weight = uint32(*backendRef.Weight)
}

backendNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace())

routeType := GetRouteType(route)
if !t.validateBackendRef(&backendRef, parentRef, route, resources, backendNamespace, routeType) {
if !t.validateBackendRef(backendRefContext, parentRef, route, resources, backendNamespace, routeType) {
return nil, weight
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
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
filters:
- type: ResponseHeaderModifier
responseHeaderModifier:
set:
- name: "set-header-1"
value: "some-value"
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
gateways:
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
creationTimestamp: null
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:
creationTimestamp: null
name: httproute-1
namespace: default
spec:
parentRefs:
- name: gateway-1
namespace: envoy-gateway
rules:
- backendRefs:
- filters:
- responseHeaderModifier:
set:
- name: set-header-1
value: some-value
type: ResponseHeaderModifier
name: service-1
port: 8080
matches:
- path:
type: Exact
value: /exact
status:
parents:
- conditions:
- lastTransitionTime: null
message: Route is accepted
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: The filters field within BackendRef is not supported
reason: UnsupportedRefValue
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
protocol: HTTP
servicePort: 80
metadata:
labels:
gateway.envoyproxy.io/owning-gateway-name: gateway-1
gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway
name: envoy-gateway/gateway-1
xdsIR:
envoy-gateway/gateway-1:
accessLog:
text:
- path: /dev/stdout
http:
- address: 0.0.0.0
hostnames:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 1
valid: 0
directResponse:
statusCode: 500
hostname: '*'
isHTTP2: false
name: httproute/default/httproute-1/rule/0/match/0/*
pathMatch:
distinct: false
exact: /exact
name: ""
33 changes: 32 additions & 1 deletion internal/gatewayapi/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@ import (
egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
)

func (t *Translator) validateBackendRef(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext,
func (t *Translator) validateBackendRef(backendRefContext BackendRefContext, parentRef *RouteParentContext, route RouteContext,
resources *Resources, backendNamespace string, routeKind gwapiv1.Kind) bool {
if !t.validateBackendRefFilters(backendRefContext, parentRef, route, routeKind) {
return false
}
backendRef := GetBackendRef(backendRefContext)

if !t.validateBackendRefGroup(backendRef, parentRef, route) {
return false
}
Expand Down Expand Up @@ -80,6 +85,32 @@ func (t *Translator) validateBackendRefKind(backendRef *gwapiv1a2.BackendRef, pa
return true
}

func (t *Translator) validateBackendRefFilters(backendRef BackendRefContext, parentRef *RouteParentContext, route RouteContext, routeKind gwapiv1.Kind) bool {
var filtersLen int
switch routeKind {
case KindHTTPRoute:
filters := GetFilters(backendRef).([]gwapiv1.HTTPRouteFilter)
filtersLen = len(filters)
case KindGRPCRoute:
filters := GetFilters(backendRef).([]gwapiv1a2.GRPCRouteFilter)
filtersLen = len(filters)
default:
return true
}

if filtersLen > 0 {
parentRef.SetCondition(route,
gwapiv1.RouteConditionResolvedRefs,
metav1.ConditionFalse,
"UnsupportedRefValue",
"The filters field within BackendRef is not supported",
)
return false
}

return true
}

func (t *Translator) validateBackendNamespace(backendRef *gwapiv1a2.BackendRef, parentRef *RouteParentContext, route RouteContext,
resources *Resources, routeKind gwapiv1.Kind) bool {
if backendRef.Namespace != nil && string(*backendRef.Namespace) != "" && string(*backendRef.Namespace) != route.GetNamespace() {
Expand Down