validate Gateway listener protocols/ports/hostnames

changelogs/unreleased/4462-skriss-small.md

@@ -0,0 +1 @@
+Gateway API: set appropriate conditions on Listeners if they don't specify the same port as other Listeners for their protocol group (i.e. HTTP, or HTTPS/TLS) or don't have a unique hostname within their group.

internal/dag/gatewayapi_processor.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/projectcontour/contour/internal/gatewayapi"
 	"github.com/projectcontour/contour/internal/k8s"
 	"github.com/projectcontour/contour/internal/status"
 
@@ -102,8 +103,21 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) {
 		}
 	}
 
+	// Validate listener protocols, ports and hostnames and add conditions
+	// for all invalid listeners.
+	validateListenersResult := gatewayapi.ValidateListeners(p.source.gateway.Spec.Listeners)
+	for name, cond := range validateListenersResult.InvalidListenerConditions {
+		gwAccessor.AddListenerCondition(
+			string(name),
+			gatewayapi_v1alpha2.ListenerConditionType(cond.Type),
+			cond.Status,
+			gatewayapi_v1alpha2.ListenerConditionReason(cond.Reason),
+			cond.Message,
+		)
+	}
+
 	for _, listener := range p.source.gateway.Spec.Listeners {
-		p.computeListener(listener, gwAccessor, gatewayNotReadyCondition == nil)
+		p.computeListener(listener, gwAccessor, gatewayNotReadyCondition == nil, validateListenersResult)
 	}
 
 	p.computeGatewayConditions(gwAccessor, gatewayNotReadyCondition)
@@ -143,7 +157,7 @@ func addressTypeDerefOr(addressType *gatewayapi_v1alpha2.AddressType, defaultAdd
 	return defaultAddressType
 }
 
-func (p *GatewayAPIProcessor) computeListener(listener gatewayapi_v1alpha2.Listener, gwAccessor *status.GatewayStatusUpdate, isGatewayValid bool) {
+func (p *GatewayAPIProcessor) computeListener(listener gatewayapi_v1alpha2.Listener, gwAccessor *status.GatewayStatusUpdate, isGatewayValid bool, validateListenersResult gatewayapi.ValidateListenersResult) {
 	// set the listener's "Ready" condition based on whether we've
 	// added any other conditions for the listener. The assumption
 	// here is that if another condition is set, the listener is
@@ -184,9 +198,15 @@ func (p *GatewayAPIProcessor) computeListener(listener gatewayapi_v1alpha2.Liste
 		}
 	}()
 
+	// 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
+	}
+
 	var listenerSecret *Secret
 
-	// Validate the listener protocol is a supported type.
+	// Validate TLS details for HTTPS/TLS protocol listeners.
 	switch listener.Protocol {
 	case gatewayapi_v1alpha2.HTTPSProtocolType:
 		// Validate that if protocol is type HTTPS, that TLS is defined.
@@ -242,17 +262,6 @@ func (p *GatewayAPIProcessor) computeListener(listener gatewayapi_v1alpha2.Liste
 				}
 			}
 		}
-	case gatewayapi_v1alpha2.HTTPProtocolType:
-		// no action required, this is a valid protocol type.
-	default:
-		gwAccessor.AddListenerCondition(
-			string(listener.Name),
-			gatewayapi_v1alpha2.ListenerConditionDetached,
-			metav1.ConditionTrue,
-			gatewayapi_v1alpha2.ListenerReasonUnsupportedProtocol,
-			fmt.Sprintf("Listener.Protocol %q is not supported.", listener.Protocol),
-		)
-		return
 	}
 
 	// Get a list of the route kinds that the listener accepts.

internal/dag/status_test.go

@@ -5030,7 +5030,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {
 							Type:    string(gatewayapi_v1alpha2.ListenerConditionDetached),
 							Status:  metav1.ConditionTrue,
 							Reason:  string(gatewayapi_v1alpha2.ListenerReasonUnsupportedProtocol),
-							Message: "Listener.Protocol \"invalid\" is not supported.",
+							Message: "Listener protocol \"invalid\" is unsupported, must be one of HTTP, HTTPS or TLS",
 						},
 					},
 				},

internal/gatewayapi/listeners.go

@@ -0,0 +1,136 @@
+// Copyright Project Contour Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package gatewayapi
+
+import (
+	"fmt"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	gatewayapi_v1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+)
+
+type ValidateListenersResult struct {
+	InsecurePort int
+	SecurePort   int
+
+	InvalidListenerConditions map[gatewayapi_v1alpha2.SectionName]metav1.Condition
+}
+
+// ValidateListeners validates protocols, ports and hostnames on a set of listeners.
+// It ensures that:
+//	- all protocols are supported
+//	- each listener group (grouped by protocol, with HTTPS & TLS going together) uses a single port
+//  - hostnames within each listener group are unique
+// It returns the insecure & secure ports to use, as well as conditions for all invalid listeners.
+// If a listener is not in the "InvalidListenerConditions" map, it is assumed to be valid according
+// to the above rules.
+func ValidateListeners(listeners []gatewayapi_v1alpha2.Listener) ValidateListenersResult {
+	result := ValidateListenersResult{
+		InvalidListenerConditions: map[gatewayapi_v1alpha2.SectionName]metav1.Condition{},
+	}
+
+	// All listeners with a protocol of "HTTP" must use the same port number
+	// Heuristic: the first port number encountered is allowed, any other listeners with a different port number are marked "Detached" with "PortUnavailable"
+	// All listeners with a protocol of "HTTP" using the one allowed port must have a unique hostname
+	// Any listener with a duplicate hostname is marked "Conflicted" with "HostnameConflict"
+
+	var (
+		insecureHostnames = map[string]int{}
+		secureHostnames   = map[string]int{}
+	)
+
+	for _, listener := range listeners {
+		switch listener.Protocol {
+		case gatewayapi_v1alpha2.HTTPProtocolType:
+			// Keep the first insecure listener port we see
+			if result.InsecurePort == 0 {
+				result.InsecurePort = int(listener.Port)
+			}
+
+			// Count hostnames among insecure listeners with the "valid" port.
+			// For other insecure listeners with an "invalid" port, the
+			// "PortUnavailable" reason will take precedence.
+			if int(listener.Port) == result.InsecurePort {
+				insecureHostnames[listenerHostname(listener)]++
+			}
+		case gatewayapi_v1alpha2.HTTPSProtocolType, gatewayapi_v1alpha2.TLSProtocolType:
+			// Keep the first secure listener port we see
+			if result.SecurePort == 0 {
+				result.SecurePort = int(listener.Port)
+			}
+
+			// Count hostnames among secure listeners with the "valid" port.
+			// For other secure listeners with an "invalid" port, the
+			// "PortUnavailable" reason will take precedence.
+			if int(listener.Port) == result.SecurePort {
+				secureHostnames[listenerHostname(listener)]++
+			}
+		}
+	}
+
+	for _, listener := range listeners {
+		switch listener.Protocol {
+		case gatewayapi_v1alpha2.HTTPProtocolType:
+			switch {
+			case int(listener.Port) != result.InsecurePort:
+				result.InvalidListenerConditions[listener.Name] = metav1.Condition{
+					Type:    string(gatewayapi_v1alpha2.ListenerConditionDetached),
+					Status:  metav1.ConditionTrue,
+					Reason:  string(gatewayapi_v1alpha2.ListenerReasonPortUnavailable),
+					Message: "Only one HTTP port is supported",
+				}
+			case insecureHostnames[listenerHostname(listener)] > 1:
+				result.InvalidListenerConditions[listener.Name] = metav1.Condition{
+					Type:    string(gatewayapi_v1alpha2.ListenerConditionConflicted),
+					Status:  metav1.ConditionTrue,
+					Reason:  string(gatewayapi_v1alpha2.ListenerReasonHostnameConflict),
+					Message: "Hostname must be unique among HTTP listeners",
+				}
+			}
+		case gatewayapi_v1alpha2.HTTPSProtocolType, gatewayapi_v1alpha2.TLSProtocolType:
+			switch {
+			case int(listener.Port) != result.SecurePort:
+				result.InvalidListenerConditions[listener.Name] = metav1.Condition{
+					Type:    string(gatewayapi_v1alpha2.ListenerConditionDetached),
+					Status:  metav1.ConditionTrue,
+					Reason:  string(gatewayapi_v1alpha2.ListenerReasonPortUnavailable),
+					Message: "Only one HTTPS/TLS port is supported",
+				}
+			case secureHostnames[listenerHostname(listener)] > 1:
+				result.InvalidListenerConditions[listener.Name] = metav1.Condition{
+					Type:    string(gatewayapi_v1alpha2.ListenerConditionConflicted),
+					Status:  metav1.ConditionTrue,
+					Reason:  string(gatewayapi_v1alpha2.ListenerReasonHostnameConflict),
+					Message: "Hostname must be unique among HTTPS/TLS listeners",
+				}
+			}
+		default:
+			result.InvalidListenerConditions[listener.Name] = metav1.Condition{
+				Type:    string(gatewayapi_v1alpha2.ListenerConditionDetached),
+				Status:  metav1.ConditionTrue,
+				Reason:  string(gatewayapi_v1alpha2.ListenerReasonUnsupportedProtocol),
+				Message: fmt.Sprintf("Listener protocol %q is unsupported, must be one of HTTP, HTTPS or TLS", listener.Protocol),
+			}
+		}
+	}
+
+	return result
+}
+
+func listenerHostname(listener gatewayapi_v1alpha2.Listener) string {
+	if listener.Hostname != nil {
+		return string(*listener.Hostname)
+	}
+	return ""
+}