fix: filter hostnames in HTTPRoute

CHANGELOG.md

@@ -182,6 +182,13 @@ Adding a new version? You'll need three changes:
 - Admin and proxy listens in the deploy manifests now use the same parameters
   as the default upstream kong.conf.
   [#3165](https://github.com/Kong/kubernetes-ingress-controller/pull/3165)
+- Fix the behavior of filtering hostnames in `HTTPRoute` when listeners 
+  of parent gateways specified hostname.
+  If an `HTTPRoute` does not specify hostnames, and one of its parent listeners
+  has not specified hostname, the `HTTPRoute` matches any hostname. 
+  If an `HTTPRoute` specifies hostnames, and no intersecting hostnames 
+  could be found in its parent listners, it is not accepted.
+  [#3180](https://github.com/Kong/kubernetes-ingress-controller/pull/3180)
 
 ## [2.7.0]
 

internal/controllers/gateway/gateway_utils.go

@@ -153,6 +153,17 @@ func listSecretNamesReferredByGateway(gateway *gatewayv1beta1.Gateway) map[types
 	return nsNames
 }
 
+// extractListenerSpecFromGateway returns the spec of the listener with the given name.
+// returns nil if the listener with given name is not found.
+func extractListenerSpecFromGateway(gateway *gatewayv1beta1.Gateway, listenerName gatewayv1beta1.SectionName) *gatewayv1beta1.Listener {
+	for i, l := range gateway.Spec.Listeners {
+		if l.Name == listenerName {
+			return &gateway.Spec.Listeners[i]
+		}
+	}
+	return nil
+}
+
 // ListenerTracker holds Gateway Listeners and their statuses, and provides methods to update statuses upon
 // reconciliation.
 type ListenerTracker struct {

internal/controllers/gateway/httproute_controller.go

@@ -318,8 +318,23 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	// perform operations on the kong store only if the route is in accepted status
 	if isRouteAccepted(gateways) {
-		// remove all the hostnames that don't match with at least one Listener's Hostname
-		filteredHTTPRoute := filterHostnames(gateways, httproute.DeepCopy())
+		// if there is no matched hosts in listeners for the httproute, the httproute should not be accepted
+		// and have an "Accepted" condition with status false.
+		// https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.HTTPRoute
+		filteredHTTPRoute, err := filterHostnames(gateways, httproute.DeepCopy())
+		if err != nil {
+			debug(log, httproute, "not accepting a route: no matching hostnames found after filtering")
+			_, err := r.ensureParentsAcceptedCondition(
+				ctx,
+				httproute, gateways,
+				metav1.ConditionFalse,
+				RouteReasonNoMatchingListenerHostname,
+				err.Error(),
+			)
+			if err != nil {
+				return ctrl.Result{}, err
+			}
+		}
 
 		// if the gateways are ready, and the HTTPRoute is destined for them, ensure that
 		// the object is pushed to the dataplane.
@@ -585,3 +600,103 @@ func (r *HTTPRouteReconciler) getHTTPRouteRuleReason(ctx context.Context, httpRo
 	}
 	return gatewayv1beta1.RouteReasonResolvedRefs, nil
 }
+
+// ensureParentsAcceptedCondition sets the "Accepted" condition of HTTPRoute status.
+// returns a non-nil error if updating status failed,
+// and returns true in the first return value if status changed.
+func (r *HTTPRouteReconciler) ensureParentsAcceptedCondition(
+	ctx context.Context,
+	httproute *gatewayv1beta1.HTTPRoute,
+	gateways []supportedGatewayWithCondition,
+	conditionStatus metav1.ConditionStatus,
+	conditionReason gatewayv1beta1.RouteConditionReason,
+	conditionMessage string,
+) (bool, error) {
+	// map the existing parentStatues to avoid duplications
+	parentStatuses := make(map[string]*gatewayv1beta1.RouteParentStatus)
+	for _, existingParent := range httproute.Status.Parents {
+		namespace := httproute.Namespace
+		if existingParent.ParentRef.Namespace != nil {
+			namespace = string(*existingParent.ParentRef.Namespace)
+		}
+		existingParentCopy := existingParent
+		var sectionName string
+		if existingParent.ParentRef.SectionName != nil {
+			sectionName = string(*existingParent.ParentRef.SectionName)
+		}
+		parentStatuses[fmt.Sprintf("%s/%s/%s", namespace, existingParent.ParentRef.Name, sectionName)] = &existingParentCopy
+	}
+
+	statusChanged := false
+	for _, g := range gateways {
+		gateway := g.gateway
+		parentRefKey := fmt.Sprintf("%s/%s/%s", gateway.Namespace, gateway.Name, g.listenerName)
+		parentStatus, ok := parentStatuses[parentRefKey]
+		if ok {
+			// update existing parent in status.
+			changed := updateAcceptedConditionInRouteParentStatus(parentStatus, conditionStatus, conditionReason, conditionMessage, httproute.Generation)
+			statusChanged = statusChanged || changed
+		} else {
+			// add a new parent if the parent is not found in status.
+			newParentStatus := &gatewayv1beta1.RouteParentStatus{
+				ParentRef: gatewayv1beta1.ParentReference{
+					Namespace:   (*gatewayv1beta1.Namespace)(pointer.String(gateway.Namespace)),
+					Name:        gatewayv1beta1.ObjectName(gateway.Name),
+					SectionName: (*gatewayv1beta1.SectionName)(pointer.String(g.listenerName)),
+					// TODO: set port after gateway port matching implemented: https://github.com/Kong/kubernetes-ingress-controller/issues/3016
+				},
+				Conditions: []metav1.Condition{
+					{
+						Type:               string(gatewayv1beta1.RouteConditionAccepted),
+						Status:             conditionStatus,
+						ObservedGeneration: httproute.Generation,
+						LastTransitionTime: metav1.Now(),
+						Reason:             string(conditionReason),
+						Message:            conditionMessage,
+					},
+				},
+			}
+			httproute.Status.Parents = append(httproute.Status.Parents, *newParentStatus)
+			parentStatuses[parentRefKey] = newParentStatus
+			statusChanged = true
+		}
+	}
+
+	// update status if needed.
+	if statusChanged {
+		if err := r.Status().Update(ctx, httproute); err != nil {
+			return false, err
+		}
+		return true, nil
+	}
+	// no need to update if no status is changed.
+	return false, nil
+}
+
+// updateAcceptedConditionInRouteParentStatus updates conditions with type "Accepted" in parentStatus.
+// returns true if the parentStatus was modified.
+func updateAcceptedConditionInRouteParentStatus(
+	parentStatus *gatewayv1beta1.RouteParentStatus,
+	conditionStatus metav1.ConditionStatus,
+	conditionReason gatewayv1beta1.RouteConditionReason,
+	conditionMessage string,
+	generation int64,
+) bool {
+	changed := false
+	for i, condition := range parentStatus.Conditions {
+		if condition.Type == string(gatewayv1beta1.RouteConditionAccepted) {
+			if condition.Status != conditionStatus || condition.Reason != string(conditionReason) || condition.Message != conditionMessage {
+				parentStatus.Conditions[i] = metav1.Condition{
+					Type:               string(gatewayv1beta1.RouteConditionAccepted),
+					Status:             conditionStatus,
+					ObservedGeneration: generation,
+					LastTransitionTime: metav1.Now(),
+					Reason:             string(conditionReason),
+					Message:            conditionMessage,
+				}
+				changed = true
+			}
+		}
+	}
+	return changed
+}

internal/controllers/gateway/route_utils.go

@@ -27,6 +27,8 @@ const (
 	unsupportedGW = "no supported Gateway found for route"
 )
 
+var ErrNoMatchingListenerHostname = fmt.Errorf("no matching hostnames in listener")
+
 // supportedGatewayWithCondition is a struct that wraps a gateway and some further info
 // such as the condition Status condition Accepted of the gateway and the listenerName.
 type supportedGatewayWithCondition struct {
@@ -82,6 +84,9 @@ const (
 	// https://github.com/kubernetes-sigs/gateway-api/pull/1516
 	// TODO: swap this out with upstream const when released.
 	RouteReasonNoMatchingParent gatewayv1beta1.RouteConditionReason = "NoMatchingParent"
+	// This reason is used with the "Accepted" condition when no hostnames in listeners
+	// could match hostname in the spec of route.
+	RouteReasonNoMatchingListenerHostname gatewayv1beta1.RouteConditionReason = "NoMatchingListenerHostname"
 )
 
 // getSupportedGatewayForRoute will retrieve the Gateway and GatewayClass object for any
@@ -458,33 +463,74 @@ func listenerHostnameIntersectWithRouteHostnames[H types.HostnameT, L types.List
 	return false
 }
 
+// isListenterHostnameEffective returns true if the listener can specify an effective
+// hostname to match hostnames in requests.
+// It basically checks if the listener is using any these protocols: HTTP, HTTPS, or TLS.
+func isListenerHostnameEffective(listener gatewayv1beta1.Listener) bool {
+	return listener.Protocol == gatewayv1beta1.HTTPProtocolType ||
+		listener.Protocol == gatewayv1beta1.HTTPSProtocolType ||
+		listener.Protocol == gatewayv1beta1.TLSProtocolType
+}
+
 // filterHostnames accepts a HTTPRoute and returns a version of the same object with only a subset of the
 // hostnames, the ones matching with the listeners' hostname.
-func filterHostnames(gateways []supportedGatewayWithCondition, httpRoute *gatewayv1beta1.HTTPRoute) *gatewayv1beta1.HTTPRoute {
+// it returns an error if the intersection of hostname match in httproute and listeners is empty.
+func filterHostnames(gateways []supportedGatewayWithCondition, httpRoute *gatewayv1beta1.HTTPRoute) (*gatewayv1beta1.HTTPRoute, error) {
 	filteredHostnames := make([]gatewayv1beta1.Hostname, 0)
-
-	// if no hostnames are specified in the route spec, get all the hostnames from
-	// the gateway
+	// if no hostnames are specified in the route spec, we use the UNION of all hostnames in supported gateways.
+	// if any of supported listener has not specified hostname, the hostnames of HTTPRoute remains empty
+	// to match **ANY** hostname.
 	if len(httpRoute.Spec.Hostnames) == 0 {
-		for _, gateway := range gateways {
-			for _, listener := range gateway.gateway.Spec.Listeners {
-				if listenerName := gateway.listenerName; listenerName == "" || listenerName == string(listener.Name) {
-					if listener.Hostname != nil {
-						filteredHostnames = append(filteredHostnames, (*listener.Hostname))
-					}
-				}
-			}
+		var matchAnyHost bool
+		filteredHostnames, matchAnyHost = getUnionOfGatewayHostnames(gateways)
+		if matchAnyHost {
+			return httpRoute, nil
 		}
 	} else {
 		for _, hostname := range httpRoute.Spec.Hostnames {
 			if hostnameMatching := getMinimumHostnameIntersection(gateways, hostname); hostnameMatching != "" {
 				filteredHostnames = append(filteredHostnames, hostnameMatching)
 			}
 		}
+		if len(filteredHostnames) == 0 {
+			return httpRoute, ErrNoMatchingListenerHostname
+		}
 	}
 
 	httpRoute.Spec.Hostnames = filteredHostnames
-	return httpRoute
+	return httpRoute, nil
+}
+
+// getUnionOfGatewayHostnames returns UNION of hostnames specified in supported gateways.
+// the second return value is true if any hostname could be matched in at least one listener
+// in supported gateways and listeners, so the `HTTPRoute` could match any hostname.
+func getUnionOfGatewayHostnames(gateways []supportedGatewayWithCondition) ([]gatewayv1beta1.Hostname, bool) {
+	hostnames := make([]gatewayv1beta1.Hostname, 0)
+	for _, gateway := range gateways {
+		if gateway.listenerName != "" {
+			if listener := extractListenerSpecFromGateway(
+				gateway.gateway,
+				gatewayv1beta1.SectionName(gateway.listenerName),
+			); listener != nil {
+				// return true if the listener has not specified hostname to match any hostname.
+				if listener.Hostname == nil {
+					return nil, true
+				}
+				hostnames = append(hostnames, *listener.Hostname)
+			}
+		} else {
+			for _, listener := range gateway.gateway.Spec.Listeners {
+				// REVIEW: which listeners should we take into consideration here if no listener in supported gateways?
+				if isListenerHostnameEffective(listener) {
+					if listener.Hostname == nil {
+						return nil, true
+					}
+					hostnames = append(hostnames, *listener.Hostname)
+				}
+			}
+		}
+	}
+	return hostnames, false
 }
 
 // getMinimumHostnameIntersection returns the minimum intersecting hostname, in the sense that:

internal/controllers/gateway/route_utils_test.go

@@ -48,6 +48,9 @@ func TestFilterHostnames(t *testing.T) {
 					Name:     "listener-3",
 					Hostname: util.StringToGatewayAPIHostnamePtr("*.anotherwildcard.io"),
 				},
+				{
+					Name: "listener-4",
+				},
 			},
 		},
 	}
@@ -57,6 +60,8 @@ func TestFilterHostnames(t *testing.T) {
 		gateways          []supportedGatewayWithCondition
 		httpRoute         *gatewayv1beta1.HTTPRoute
 		expectedHTTPRoute *gatewayv1beta1.HTTPRoute
+		hasError          bool
+		errString         string
 	}{
 		{
 			name: "listener 1 - specific",
@@ -160,12 +165,31 @@ func TestFilterHostnames(t *testing.T) {
 			},
 		},
 		{
-			name: "no match",
+			name: "no listner specified - no hostname",
 			gateways: []supportedGatewayWithCondition{
 				{
 					gateway: commonGateway,
 				},
 			},
+			httpRoute: &gatewayv1beta1.HTTPRoute{
+				Spec: gatewayv1beta1.HTTPRouteSpec{
+					Hostnames: []gatewayv1beta1.Hostname{},
+				},
+			},
+			expectedHTTPRoute: &gatewayv1beta1.HTTPRoute{
+				Spec: gatewayv1beta1.HTTPRouteSpec{
+					Hostnames: []gatewayv1beta1.Hostname{},
+				},
+			},
+		},
+		{
+			name: "listener 1 - no match",
+			gateways: []supportedGatewayWithCondition{
+				{
+					gateway:      commonGateway,
+					listenerName: "listner-1",
+				},
+			},
 			httpRoute: &gatewayv1beta1.HTTPRoute{
 				Spec: gatewayv1beta1.HTTPRouteSpec{
 					Hostnames: []gatewayv1beta1.Hostname{
@@ -179,12 +203,20 @@ func TestFilterHostnames(t *testing.T) {
 					Hostnames: []gatewayv1beta1.Hostname{},
 				},
 			},
+			hasError:  true,
+			errString: "no matching hostnames in listener",
 		},
 	}
 
 	for _, tc := range testCases {
-		filteredHTTPRoute := filterHostnames(tc.gateways, tc.httpRoute)
-		assert.Equal(t, tc.expectedHTTPRoute.Spec, filteredHTTPRoute.Spec, tc.name)
+		filteredHTTPRoute, err := filterHostnames(tc.gateways, tc.httpRoute)
+		if tc.hasError {
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), tc.errString)
+		} else {
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expectedHTTPRoute.Spec, filteredHTTPRoute.Spec, tc.name)
+		}
 	}
 }