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

Gateway API: fix for failing GatewayWithAttachedRoutes conformance test #5961

Merged
merged 14 commits into from
Dec 12, 2023
1 change: 1 addition & 0 deletions changelogs/unreleased/5961-izturn-small.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
For Gateway API v1.0, the successful attachment of a Route to a Listener is based solely on the combination of the AllowedRoutes field on the corresponding Listener and the Route's ParentRefs field.
158 changes: 84 additions & 74 deletions internal/dag/gatewayapi_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,9 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) {
}

// Compute listeners and save a list of the valid/ready ones.
var readyListeners []*listenerInfo

var listenerInfos []*listenerInfo
for _, listener := range p.source.gateway.Spec.Listeners {
if ready, listenerInfo := p.computeListener(listener, gwAccessor, validateListenersResult); ready {
readyListeners = append(readyListeners, listenerInfo)
}
listenerInfos = append(listenerInfos, p.computeListener(listener, gwAccessor, validateListenersResult))
}

// Keep track of the number of routes attached
Expand All @@ -150,22 +147,22 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) {

// Process HTTPRoutes.
for _, httpRoute := range p.source.httproutes {
p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1beta1.HTTPRoute{})
p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1beta1.HTTPRoute{})
}

// Process TLSRoutes.
for _, tlsRoute := range p.source.tlsroutes {
p.processRoute(KindTLSRoute, tlsRoute, tlsRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1alpha2.TLSRoute{})
p.processRoute(KindTLSRoute, tlsRoute, tlsRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1alpha2.TLSRoute{})
}

// Process GRPCRoutes.
for _, grpcRoute := range p.source.grpcroutes {
p.processRoute(KindGRPCRoute, grpcRoute, grpcRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1alpha2.GRPCRoute{})
p.processRoute(KindGRPCRoute, grpcRoute, grpcRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1alpha2.GRPCRoute{})
}

// Process TCPRoutes.
for _, tcpRoute := range p.source.tcproutes {
p.processRoute(KindTCPRoute, tcpRoute, tcpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1alpha2.TCPRoute{})
p.processRoute(KindTCPRoute, tcpRoute, tcpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1alpha2.TCPRoute{})
}

for listenerName, attachedRoutes := range listenerAttachedRoutes {
Expand All @@ -180,7 +177,7 @@ func (p *GatewayAPIProcessor) processRoute(
route client.Object,
parentRefs []gatewayapi_v1beta1.ParentReference,
gatewayNotProgrammedCondition *metav1.Condition,
readyListeners []*listenerInfo,
listeners []*listenerInfo,
listenerAttachedRoutes map[string]int,
emptyResource client.Object,
) {
Expand All @@ -199,15 +196,18 @@ func (p *GatewayAPIProcessor) processRoute(

routeParentStatus := routeStatus.StatusUpdateFor(routeParentRef)

// Get the list of listeners that are
// (a) included by this parent ref, and
// (b) allow the route (based on kind, namespace)
// (c) pass the all check for it
izturn marked this conversation as resolved.
Show resolved Hide resolved
allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, listeners, listenerAttachedRoutes, routeParentStatus)
izturn marked this conversation as resolved.
Show resolved Hide resolved

// If the Gateway is invalid, set status on the route and we're done.
if gatewayNotProgrammedCondition != nil {
routeParentStatus.AddCondition(gatewayapi_v1beta1.RouteConditionAccepted, metav1.ConditionFalse, status.ReasonInvalidGateway, "Invalid Gateway")
continue
}

// Get the list of listeners that are (a) included by this parent ref, and
// (b) allow the route (based on kind, namespace).
allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, readyListeners, routeParentStatus)
if len(allowedListeners) == 0 {
p.resolveRouteRefs(route, routeParentStatus)
}
Expand Down Expand Up @@ -251,21 +251,15 @@ func (p *GatewayAPIProcessor) processRoute(
}
}

var attached bool

switch route := route.(type) {
case *gatewayapi_v1beta1.HTTPRoute:
attached = p.computeHTTPRouteForListener(route, routeParentStatus, listener, hosts)
p.computeHTTPRouteForListener(route, routeParentStatus, listener, hosts)
case *gatewayapi_v1alpha2.TLSRoute:
attached = p.computeTLSRouteForListener(route, routeParentStatus, listener, hosts)
p.computeTLSRouteForListener(route, routeParentStatus, listener, hosts)
case *gatewayapi_v1alpha2.GRPCRoute:
attached = p.computeGRPCRouteForListener(route, routeParentStatus, listener, hosts)
p.computeGRPCRouteForListener(route, routeParentStatus, listener, hosts)
case *gatewayapi_v1alpha2.TCPRoute:
attached = p.computeTCPRouteForListener(route, routeParentStatus, listener)
}

if attached {
listenerAttachedRoutes[string(listener.listener.Name)]++
p.computeTCPRouteForListener(route, routeParentStatus, listener)
}

hostCount += hosts.Len()
Expand Down Expand Up @@ -307,7 +301,8 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
routeParentRef gatewayapi_v1beta1.ParentReference,
routeNamespace string,
routeKind gatewayapi_v1beta1.Kind,
validListeners []*listenerInfo,
listeners []*listenerInfo,
attachedRoutes map[string]int,
routeParentStatusAccessor *status.RouteParentStatusUpdate,
) []*listenerInfo {

Expand All @@ -316,30 +311,30 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
// gateway, or one of them, if the ref is to a specific listener,
// or none of them, if the listener(s) the ref targets are invalid).
var selectedListeners []*listenerInfo
for _, validListener := range validListeners {
for _, listener := range listeners {
// We've already verified the parent ref is for this Gateway,
// now check if it has a listener name and port specified.
// Both need to match the listener if specified.
if (routeParentRef.SectionName == nil || *routeParentRef.SectionName == validListener.listener.Name) &&
(routeParentRef.Port == nil || *routeParentRef.Port == validListener.listener.Port) {
selectedListeners = append(selectedListeners, validListener)
if (routeParentRef.SectionName == nil || *routeParentRef.SectionName == listener.listener.Name) &&
(routeParentRef.Port == nil || *routeParentRef.Port == listener.listener.Port) {
selectedListeners = append(selectedListeners, listener)
}
}

if len(selectedListeners) == 0 {
routeParentStatusAccessor.AddCondition(
gatewayapi_v1beta1.RouteConditionAccepted,
metav1.ConditionFalse,
gatewayapi_v1.RouteReasonNoMatchingParent,
"No listeners match this parent ref",
)
return nil
}

// Now find the subset of those listeners that allow this route
// to select them, based on route kind and namespace.
var allowedListeners []*listenerInfo

readyListenerCount := 0

for _, selectedListener := range selectedListeners {

// for compute the AttachedRoutes, the listener that not passed its check(s), had been selected too
// so ignore it.
if selectedListener.ready {
readyListenerCount++
}

// Check if the listener allows routes of this kind
if !selectedListener.AllowsKind(routeKind) {
continue
Expand All @@ -350,7 +345,21 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
continue
}

allowedListeners = append(allowedListeners, selectedListener)
attachedRoutes[string(selectedListener.listener.Name)]++

if selectedListener.ready {
allowedListeners = append(allowedListeners, selectedListener)
}

}
if readyListenerCount == 0 {
routeParentStatusAccessor.AddCondition(
gatewayapi_v1beta1.RouteConditionAccepted,
metav1.ConditionFalse,
gatewayapi_v1.RouteReasonNoMatchingParent,
"No listeners match this parent ref",
)
return nil
}

if len(allowedListeners) == 0 {
Expand All @@ -372,6 +381,7 @@ type listenerInfo struct {
allowedKinds []gatewayapi_v1beta1.Kind
namespaceSelector labels.Selector
tlsSecret *Secret
ready bool
}

func (l *listenerInfo) AllowsKind(kind gatewayapi_v1beta1.Kind) bool {
Expand Down Expand Up @@ -419,7 +429,12 @@ func (p *GatewayAPIProcessor) computeListener(
listener gatewayapi_v1beta1.Listener,
gwAccessor *status.GatewayStatusUpdate,
validateListenersResult gatewayapi.ValidateListenersResult,
) (bool, *listenerInfo) {
) (info *listenerInfo) {
izturn marked this conversation as resolved.
Show resolved Hide resolved

info = &listenerInfo{
listener: listener,
dagListenerName: validateListenersResult.ListenerNames[string(listener.Name)],
}

addInvalidListenerCondition := func(msg string) {
gwAccessor.AddListenerCondition(
Expand Down Expand Up @@ -507,39 +522,37 @@ func (p *GatewayAPIProcessor) computeListener(
}
}()

// If the listener had an invalid protocol/port/hostname, we don't need to go
// any further.
if _, ok := validateListenersResult.InvalidListenerConditions[listener.Name]; ok {
return false, nil
}

// Get a list of the route kinds that the listener accepts.
listenerRouteKinds := p.getListenerRouteKinds(listener, gwAccessor)
gwAccessor.SetListenerSupportedKinds(string(listener.Name), listenerRouteKinds)

var selector labels.Selector
info.allowedKinds = p.getListenerRouteKinds(listener, gwAccessor)
gwAccessor.SetListenerSupportedKinds(string(listener.Name), info.allowedKinds)

if listener.AllowedRoutes != nil && listener.AllowedRoutes.Namespaces != nil &&
listener.AllowedRoutes.Namespaces.From != nil && *listener.AllowedRoutes.Namespaces.From == gatewayapi_v1.NamespacesFromSelector {

if listener.AllowedRoutes.Namespaces.Selector == nil {
addInvalidListenerCondition("Listener.AllowedRoutes.Namespaces.Selector is required when Listener.AllowedRoutes.Namespaces.From is set to \"Selector\".")
return false, nil
return
}

if len(listener.AllowedRoutes.Namespaces.Selector.MatchExpressions)+len(listener.AllowedRoutes.Namespaces.Selector.MatchLabels) == 0 {
addInvalidListenerCondition("Listener.AllowedRoutes.Namespaces.Selector must specify at least one MatchLabel or MatchExpression.")
return false, nil
return
}

var err error
selector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector)
info.namespaceSelector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector)
if err != nil {
addInvalidListenerCondition(fmt.Sprintf("Error parsing Listener.AllowedRoutes.Namespaces.Selector: %v.", err))
return false, nil
return
}
}

// If the listener had an invalid protocol/port/hostname, we reach here just for pick the information to compute the AttachedRoutes later,
// we don't need to go any further.
if _, invalid := validateListenersResult.InvalidListenerConditions[listener.Name]; invalid {
return
}

var listenerSecret *Secret

// Validate TLS details for HTTPS/TLS protocol listeners.
Expand All @@ -551,27 +564,27 @@ func (p *GatewayAPIProcessor) computeListener(

if listener.TLS == nil {
addInvalidListenerCondition(fmt.Sprintf("Listener.TLS is required when protocol is %q.", listener.Protocol))
return false, nil
return
}

if listener.TLS.Mode != nil && *listener.TLS.Mode != gatewayapi_v1.TLSModeTerminate {
addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.Mode must be %q when protocol is %q.", gatewayapi_v1.TLSModeTerminate, listener.Protocol))
return false, nil
return
}

// Resolve the TLS secret.
if listenerSecret = p.resolveListenerSecret(listener.TLS.CertificateRefs, string(listener.Name), gwAccessor); listenerSecret == nil {
// If TLS was configured on the Listener, but the secret ref is invalid, don't allow any
// routes to be bound to this listener since it can't serve TLS traffic.
return false, nil
return
}
case gatewayapi_v1.TLSProtocolType:
// The TLS protocol is used for TCP traffic encrypted with TLS.
// Gateway API allows TLS to be either terminated at the proxy
// or passed through to the backend.
if listener.TLS == nil {
addInvalidListenerCondition(fmt.Sprintf("Listener.TLS is required when protocol is %q.", listener.Protocol))
return false, nil
return
}

switch {
Expand All @@ -580,26 +593,23 @@ func (p *GatewayAPIProcessor) computeListener(
if listenerSecret = p.resolveListenerSecret(listener.TLS.CertificateRefs, string(listener.Name), gwAccessor); listenerSecret == nil {
// If TLS was configured on the Listener, but the secret ref is invalid, don't allow any
// routes to be bound to this listener since it can't serve TLS traffic.
return false, nil
return
}
case *listener.TLS.Mode == gatewayapi_v1.TLSModePassthrough:
if len(listener.TLS.CertificateRefs) != 0 {
addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.CertificateRefs cannot be defined when Listener.TLS.Mode is %q.", gatewayapi_v1.TLSModePassthrough))
return false, nil
return
}
default:
addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.Mode must be %q or %q.", gatewayapi_v1.TLSModeTerminate, gatewayapi_v1.TLSModePassthrough))
return false, nil
return
}
}

return true, &listenerInfo{
listener: listener,
dagListenerName: validateListenersResult.ListenerNames[string(listener.Name)],
allowedKinds: listenerRouteKinds,
tlsSecret: listenerSecret,
namespaceSelector: selector,
}
info.tlsSecret = listenerSecret
info.ready = true
return

}

// getListenerRouteKinds gets a list of the valid route kinds that
Expand Down Expand Up @@ -1103,8 +1113,12 @@ func (p *GatewayAPIProcessor) resolveRouteRefs(route any, routeAccessor *status.
}
}

func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1beta1.HTTPRoute, routeAccessor *status.RouteParentStatusUpdate, listener *listenerInfo, hosts sets.Set[string]) bool {
var programmed bool
func (p *GatewayAPIProcessor) computeHTTPRouteForListener(
route *gatewayapi_v1beta1.HTTPRoute,
routeAccessor *status.RouteParentStatusUpdate,
listener *listenerInfo,
hosts sets.Set[string]) {

izturn marked this conversation as resolved.
Show resolved Hide resolved
for ruleIndex, rule := range route.Spec.Rules {
// Get match conditions for the rule.
var matchconditions []*matchConditions
Expand Down Expand Up @@ -1385,13 +1399,9 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be
vhost := p.dag.EnsureVirtualHost(listener.dagListenerName, host)
vhost.AddRoute(route)
}

programmed = true
}
}
}

return programmed
}

func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1alpha2.GRPCRoute, routeAccessor *status.RouteParentStatusUpdate, listener *listenerInfo, hosts sets.Set[string]) bool {
Expand Down
Loading