Repository: nginx/nginx-gateway-fabric

Issue:
Support for Port ParentReference
As a user of NGF
I want to reference a port as my parent reference on my route
So that I have a simple way to direct all traffic for a particular port to my application.

### Acceptance
- The Port parentRef functionality is implemented as specified in the [Gateway API spec](https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io%2fv1.ParentReference):

  Port is the network port this Route targets. It can be interpreted differently based on the type of parent resource.

  When the parent resource is a Gateway, this targets all listeners listening on the specified port that also support this kind of Route(and select this Route). It’s not recommended to set Port unless the networking behaviors specified in a Route must apply to a specific port as opposed to a listener(s) whose port(s) may be changed. When both Port and SectionName are specified, the name and port of the selected listener must match both specified values.

  Implementations MAY choose to support other parent resources. Implementations supporting other types of parent resources MUST clearly document how/if Port is interpreted.

  For the purpose of status, an attachment is considered successful as long as the parent resource accepts it partially. For example, Gateway listeners can restrict which Routes can attach to them by Route kind, namespace, or hostname. If 1 of 2 Gateway listeners accept attachment from the referencing Route, the Route MUST be considered successfully attached. If no Gateway listeners accept attachment from this Route, the Route MUST be considered detached from the Gateway.
- The HTTPRouteDesinationPortMatching and HTTPRouteParentRefPort conformance tests are enabled and pass.

Discussion:

Solution patch:
diff --git a/internal/controller/state/graph/policies.go b/internal/controller/state/graph/policies.go
index c4b13567..f790f478 100644
--- a/internal/controller/state/graph/policies.go
+++ b/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 {
diff --git a/internal/controller/state/graph/route_common.go b/internal/controller/state/graph/route_common.go
index d097e72c..c156ca73 100644
--- a/internal/controller/state/graph/route_common.go
+++ b/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 {



Test patch:
diff --git a/internal/controller/state/graph/route_common_test.go b/internal/controller/state/graph/route_common_test.go
index 0e5d28f0..6ded299d 100644
--- a/internal/controller/state/graph/route_common_test.go
+++ b/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,7 +1911,9 @@ func TestBindL4RouteToListeners(t *testing.T) {
 					Name:      "gateway",
 				},
 				Listeners: []*Listener{
-					createListener("listener-443"),
+					createModifiedListener("listener-443", func(l *Listener) {
+						l.Source.Port = 443
+					}),
 				},
 			},
 			expectedSectionNameRefs: []ParentRef{
@@ -1912,9 +1921,7 @@ func TestBindL4RouteToListeners(t *testing.T) {
 					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))
+		})
+	}
+}
diff --git a/tests/Makefile b/tests/Makefile
index 379e3b88..f1ccfdd3 100644
--- a/tests/Makefile
+++ b/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.