Add support for Port in ParentReference

internal/controller/state/graph/policies.go

@@ -457,6 +457,9 @@ func checkForRouteOverlap(route *L7Route, hostPortPaths map[string]string) *cond
 	for _, parentRef := range route.ParentRefs {
 		if parentRef.Attachment != nil {
 			port := parentRef.Attachment.ListenerPort
+			// FIXME(sarthyparty): https://github.com/nginx/nginx-gateway-fabric/issues/3811
+			// Need to merge listener hostnames with route hostnames so wildcards are handled correctly
+			// Also the AcceptedHostnames is a map of slices of strings, so we need to flatten it
 			for _, hostname := range parentRef.Attachment.AcceptedHostnames {
 				for _, rule := range route.Spec.Rules {
 					for _, match := range rule.Matches {

internal/controller/state/graph/route_common.go

@@ -46,6 +46,8 @@ type ParentRefAttachmentStatus struct {
 	// still attach. The backendRef condition would be displayed here.
 	FailedConditions []conditions.Condition
 	// ListenerPort is the port on the Listener that the Route is attached to.
+	// FIXME(sarthyparty): https://github.com/nginx/nginx-gateway-fabric/issues/3811
+	// Needs to be a map of <gatewayNamespacedName/listenerName> to port number
 	ListenerPort v1.PortNumber
 	// Attached indicates if the ParentRef is attached to the Gateway.
 	Attached bool
@@ -307,24 +309,37 @@ func buildSectionNameRefs(
 			gwNsName: gwNsName,
 		}
 
-		// If there is no section name, we create ParentRefs for each listener in the gateway
+		// If there is no section name, handle based on whether port is specified
+		// FIXME(sarthyparty): https://github.com/nginx/nginx-gateway-fabric/issues/3811
+		// this logic seems to be duplicated in findAttachableListeners so we should refactor this,
+		// either here or in findAttachableListeners
 		if p.SectionName == nil {
-			for _, l := range gw.Listeners {
-				k.sectionName = string(l.Source.Name)
-
-				if err := checkUniqueSections(k); err != nil {
-					return nil, err
-				}
-
+			// If port is specified, preserve the port-only nature for proper validation
+			if p.Port != nil {
 				sectionNameRefs = append(sectionNameRefs, ParentRef{
-					// if the ParentRefs we create are for each listener in the same gateway, we keep the
-					// parentRefIndex the same so when we look at a route's parentRef's we can see
-					// if the parentRef is a unique parentRef or one we created internally
 					Idx:         i,
 					Gateway:     CreateParentRefGateway(gw),
-					SectionName: &l.Source.Name,
+					SectionName: nil, // Keep as nil to preserve port-only semantics
 					Port:        p.Port,
 				})
+			} else {
+				// If there is no port and section name, we create ParentRefs for each listener in the gateway
+				for _, l := range gw.Listeners {
+					k.sectionName = string(l.Source.Name)
+
+					if err := checkUniqueSections(k); err != nil {
+						return nil, err
+					}
+
+					sectionNameRefs = append(sectionNameRefs, ParentRef{
+						// if the ParentRefs we create are for each listener in the same gateway, we keep the
+						// parentRefIndex the same so when we look at a route's parentRef's we can see
+						// if the parentRef is a unique parentRef or one we created internally
+						Idx:         i,
+						Gateway:     CreateParentRefGateway(gw),
+						SectionName: &l.Source.Name,
+					})
+				}
 			}
 
 			continue
@@ -532,10 +547,8 @@ func validateParentRef(
 
 	ref.Attachment = attachment
 
-	path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx)
-
 	attachableListeners, listenerExists := findAttachableListeners(
-		getSectionName(ref.SectionName),
+		ref,
 		gw.Listeners,
 	)
 
@@ -546,17 +559,7 @@ func validateParentRef(
 		return attachment, nil
 	}
 
-	// Case 2: Attachment is not possible due to unsupported configuration.
-
-	if ref.Port != nil {
-		valErr := field.Forbidden(path.Child("port"), "cannot be set")
-		attachment.FailedConditions = append(
-			attachment.FailedConditions, conditions.NewRouteUnsupportedValue(valErr.Error()),
-		)
-		return attachment, attachableListeners
-	}
-
-	// Case 3: Attachment is not possible because Gateway is invalid
+	// Case 2: Attachment is not possible because Gateway is invalid
 
 	if !gw.Valid {
 		attachment.FailedConditions = append(attachment.FailedConditions, conditions.NewRouteInvalidGateway())
@@ -873,29 +876,50 @@ func tryToAttachL7RouteToListeners(
 
 // findAttachableListeners returns a list of attachable listeners and whether the listener exists for a non-empty
 // sectionName.
-func findAttachableListeners(sectionName string, listeners []*Listener) ([]*Listener, bool) {
+func findAttachableListeners(ref *ParentRef, listeners []*Listener) ([]*Listener, bool) {
+	sectionName := getSectionName(ref.SectionName)
+
+	// Case 1: sectionName is specified - look for that specific listener
 	if sectionName != "" {
 		for _, l := range listeners {
 			if l.Name == sectionName {
-				if l.Attachable {
+				if l.Attachable && (ref.Port == nil || l.Source.Port == *ref.Port) {
 					return []*Listener{l}, true
 				}
+				// We return false because we didn't find a listener that matches the port
+				if ref.Port != nil && l.Source.Port != *ref.Port {
+					return nil, false
+				}
+				// Return true because we found a listener that matches the sectionName and port but is not attachable
 				return nil, true
 			}
 		}
 		return nil, false
 	}
 
-	attachableListeners := make([]*Listener, 0, len(listeners))
-	for _, l := range listeners {
-		if !l.Attachable {
-			continue
+	// Case 2: Only port is specified - find all attachable listeners matching that port
+	if ref.Port != nil {
+		var attachableListeners []*Listener
+		var foundListener bool
+		for _, l := range listeners {
+			if l.Source.Port == *ref.Port {
+				foundListener = true
+				if l.Attachable {
+					attachableListeners = append(attachableListeners, l)
+				}
+			}
 		}
-
-		attachableListeners = append(attachableListeners, l)
+		return attachableListeners, foundListener
 	}
 
-	return attachableListeners, true
+	// Case 3: Neither sectionName nor port specified - return all attachable listeners
+	var attachableListeners []*Listener
+	for _, l := range listeners {
+		if l.Attachable {
+			attachableListeners = append(attachableListeners, l)
+		}
+	}
+	return attachableListeners, len(listeners) > 0
 }
 
 func findAcceptedHostnames(listenerHostname *v1.Hostname, routeHostnames []v1.Hostname) []string {

internal/controller/state/graph/route_common_test.go

@@ -720,7 +720,9 @@ func TestBindRouteToListeners(t *testing.T) {
 				Source: gw,
 				Valid:  true,
 				Listeners: []*Listener{
-					createListener("listener-80-1"),
+					createModifiedListener("listener-80-1", func(l *Listener) {
+						l.Source.Port = 80
+					}),
 				},
 			},
 			expectedSectionNameRefs: []ParentRef{
@@ -729,19 +731,24 @@ func TestBindRouteToListeners(t *testing.T) {
 					Gateway:     &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
 					SectionName: hrWithPort.Spec.ParentRefs[0].SectionName,
 					Attachment: &ParentRefAttachmentStatus{
-						Attached: false,
-						FailedConditions: []conditions.Condition{
-							conditions.NewRouteUnsupportedValue(
-								`spec.parentRefs[0].port: Forbidden: cannot be set`,
-							),
+						Attached:         true,
+						FailedConditions: nil,
+						AcceptedHostnames: map[string][]string{
+							"test/gateway/listener-80-1": {"foo.example.com"},
 						},
-						AcceptedHostnames: map[string][]string{},
+						ListenerPort: 80,
 					},
 					Port: hrWithPort.Spec.ParentRefs[0].Port,
 				},
 			},
 			expectedGatewayListeners: []*Listener{
-				createListener("listener-80-1"),
+				func() *Listener {
+					l := createModifiedListener("listener-80-1", func(l *Listener) {
+						l.Source.Port = 80
+					})
+					l.Routes[CreateRouteKey(hrWithPort)] = routeWithPort
+					return l
+				}(),
 			},
 			name: "port is configured",
 		},
@@ -1904,17 +1911,17 @@ func TestBindL4RouteToListeners(t *testing.T) {
 					Name:      "gateway",
 				},
 				Listeners: []*Listener{
-					createListener("listener-443"),
+					createModifiedListener("listener-443", func(l *Listener) {
+						l.Source.Port = 443
+					}),
 				},
 			},
 			expectedSectionNameRefs: []ParentRef{
 				{
 					Attachment: &ParentRefAttachmentStatus{
 						AcceptedHostnames: map[string][]string{},
 						FailedConditions: []conditions.Condition{
-							conditions.NewRouteUnsupportedValue(
-								`spec.parentRefs[0].port: Forbidden: cannot be set`,
-							),
+							conditions.NewRouteNoMatchingParent(),
 						},
 						Attached: false,
 					},
@@ -1925,7 +1932,9 @@ func TestBindL4RouteToListeners(t *testing.T) {
 				},
 			},
 			expectedGatewayListeners: []*Listener{
-				createListener("listener-443"),
+				createModifiedListener("listener-443", func(l *Listener) {
+					l.Source.Port = 443
+				}),
 			},
 			name: "port is not nil",
 		},
@@ -3720,3 +3729,141 @@ func TestBindRoutesToListeners(t *testing.T) {
 		bindRoutesToListeners(nil, nil, nil, nil)
 	}).ToNot(Panic())
 }
+
+func TestFindAttachableListenersWithPort(t *testing.T) {
+	t.Parallel()
+
+	port80 := gatewayv1.PortNumber(80)
+	port443 := gatewayv1.PortNumber(443)
+	port8080 := gatewayv1.PortNumber(8080)
+
+	httpListener := &Listener{
+		Name:       "http-80",
+		Attachable: true,
+		Source: gatewayv1.Listener{
+			Name: "http-80",
+			Port: port80,
+		},
+	}
+
+	httpsListener := &Listener{
+		Name:       "https-443",
+		Attachable: true,
+		Source: gatewayv1.Listener{
+			Name: "https-443",
+			Port: port443,
+		},
+	}
+
+	nonAttachableListener := &Listener{
+		Name:       "http-8080",
+		Attachable: false, // not attachable
+		Source: gatewayv1.Listener{
+			Name: "http-8080",
+			Port: port8080,
+		},
+	}
+
+	tests := []struct {
+		name                   string
+		parentRef              *ParentRef
+		expectedListeners      []*Listener
+		expectedListenerExists bool
+	}{
+		{
+			name: "port 80 filter returns only port 80 listener",
+			parentRef: &ParentRef{
+				Port: &port80,
+			},
+			expectedListeners:      []*Listener{httpListener},
+			expectedListenerExists: true,
+		},
+		{
+			name: "port 443 filter returns only port 443 listener",
+			parentRef: &ParentRef{
+				Port: &port443,
+			},
+			expectedListeners:      []*Listener{httpsListener},
+			expectedListenerExists: true,
+		},
+		{
+			name: "port 8080 filter returns empty because listener is not attachable",
+			parentRef: &ParentRef{
+				Port: &port8080,
+			},
+			expectedListeners:      []*Listener{},
+			expectedListenerExists: true,
+		},
+		{
+			name: "port 9999 filter returns empty because no listener has that port",
+			parentRef: &ParentRef{
+				Port: helpers.GetPointer(gatewayv1.PortNumber(9999)),
+			},
+			expectedListeners:      []*Listener{},
+			expectedListenerExists: false,
+		},
+		{
+			name: "no port specified returns all attachable listeners",
+			parentRef: &ParentRef{
+				Port: nil,
+			},
+			expectedListeners:      []*Listener{httpListener, httpsListener},
+			expectedListenerExists: true,
+		},
+		{
+			name: "sectionName with matching port returns that specific listener",
+			parentRef: &ParentRef{
+				SectionName: helpers.GetPointer(gatewayv1.SectionName("http-80")),
+				Port:        &port80,
+			},
+			expectedListeners:      []*Listener{httpListener},
+			expectedListenerExists: true,
+		},
+		{
+			name: "sectionName with non-matching port returns empty",
+			parentRef: &ParentRef{
+				SectionName: helpers.GetPointer(gatewayv1.SectionName("http-80")),
+				Port:        &port443, // wrong port for http-80 listener
+			},
+			expectedListeners:      []*Listener{},
+			expectedListenerExists: false,
+		},
+		{
+			name: "sectionName that doesn't exist returns empty with false",
+			parentRef: &ParentRef{
+				SectionName: helpers.GetPointer(gatewayv1.SectionName("nonexistent")),
+				Port:        &port80,
+			},
+			expectedListeners:      []*Listener{},
+			expectedListenerExists: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			g := NewWithT(t)
+
+			attachableListeners, listenerExists := findAttachableListeners(
+				tt.parentRef,
+				[]*Listener{httpListener, httpsListener, nonAttachableListener},
+			)
+
+			g.Expect(listenerExists).To(Equal(tt.expectedListenerExists))
+			g.Expect(attachableListeners).To(HaveLen(len(tt.expectedListeners)))
+
+			// Compare listeners by name since they're the same instances
+			expectedNames := make([]string, len(tt.expectedListeners))
+			for i, l := range tt.expectedListeners {
+				expectedNames[i] = l.Name
+			}
+
+			actualNames := make([]string, len(attachableListeners))
+			for i, l := range attachableListeners {
+				actualNames[i] = l.Name
+			}
+
+			g.Expect(actualNames).To(ConsistOf(expectedNames))
+		})
+	}
+}

tests/Makefile

@@ -12,7 +12,7 @@ GW_SERVICE_TYPE = NodePort## Service type to use for the gateway
 NGF_VERSION ?= edge## NGF version to be tested
 PULL_POLICY = Never## Pull policy for the images
 NGINX_CONF_DIR = internal/controller/nginx/conf
-SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteRequestPercentageMirror,HTTPRouteBackendProtocolWebSocket
+SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteRequestPercentageMirror,HTTPRouteBackendProtocolWebSocket,HTTPRouteParentRefPort,HTTPRouteDestinationPortMatching
 STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC
 EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS
 CONFORMANCE_PROFILES = $(STANDARD_CONFORMANCE_PROFILES) # by default we use the standard conformance profiles. If experimental is enabled we override this and add the experimental profiles.