Skip to content

Commit f0fc793

Browse files
sarthypartysarthyparty
committed
Add support for Port in ParentReference
1 parent fc6aa9a commit f0fc793

File tree

3 files changed

+211
-49
lines changed

3 files changed

+211
-49
lines changed

‎internal/controller/state/graph/route_common.go

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -307,24 +307,35 @@ func buildSectionNameRefs(
307307
gwNsName: gwNsName,
308308
}
309309

310-
// If there is no section name, we create ParentRefs for each listener in the gateway
310+
// If there is no section name, handle based on whether port is specified
311311
if p.SectionName == nil {
312-
for _, l := range gw.Listeners {
313-
k.sectionName = string(l.Source.Name)
314-
315-
if err := checkUniqueSections(k); err != nil {
316-
return nil, err
317-
}
318-
312+
// If port is specified, preserve the port-only nature for proper validation
313+
if p.Port != nil {
319314
sectionNameRefs = append(sectionNameRefs, ParentRef{
320-
// if the ParentRefs we create are for each listener in the same gateway, we keep the
321-
// parentRefIndex the same so when we look at a route's parentRef's we can see
322-
// if the parentRef is a unique parentRef or one we created internally
323315
Idx: i,
324316
Gateway: CreateParentRefGateway(gw),
325-
SectionName: &l.Source.Name,
317+
SectionName: nil, // Keep as nil to preserve port-only semantics
326318
Port: p.Port,
327319
})
320+
} else {
321+
// If no port and no sectionName, expand to all listeners (existing behavior)
322+
for _, l := range gw.Listeners {
323+
k.sectionName = string(l.Source.Name)
324+
325+
if err := checkUniqueSections(k); err != nil {
326+
return nil, err
327+
}
328+
329+
sectionNameRefs = append(sectionNameRefs, ParentRef{
330+
// if the ParentRefs we create are for each listener in the same gateway, we keep the
331+
// parentRefIndex the same so when we look at a route's parentRef's we can see
332+
// if the parentRef is a unique parentRef or one we created internally
333+
Idx: i,
334+
Gateway: CreateParentRefGateway(gw),
335+
SectionName: &l.Source.Name,
336+
Port: p.Port,
337+
})
338+
}
328339
}
329340

330341
continue
@@ -532,10 +543,8 @@ func validateParentRef(
532543

533544
ref.Attachment = attachment
534545

535-
path := field.NewPath("spec").Child("parentRefs").Index(ref.Idx)
536-
537546
attachableListeners, listenerExists := findAttachableListeners(
538-
getSectionName(ref.SectionName),
547+
ref,
539548
gw.Listeners,
540549
)
541550

@@ -546,17 +555,7 @@ func validateParentRef(
546555
return attachment, nil
547556
}
548557

549-
// Case 2: Attachment is not possible due to unsupported configuration.
550-
551-
if ref.Port != nil {
552-
valErr := field.Forbidden(path.Child("port"), "cannot be set")
553-
attachment.FailedConditions = append(
554-
attachment.FailedConditions, conditions.NewRouteUnsupportedValue(valErr.Error()),
555-
)
556-
return attachment, attachableListeners
557-
}
558-
559-
// Case 3: Attachment is not possible because Gateway is invalid
558+
// Case 2: Attachment is not possible because Gateway is invalid
560559

561560
if !gw.Valid {
562561
attachment.FailedConditions = append(attachment.FailedConditions, conditions.NewRouteInvalidGateway())
@@ -873,29 +872,48 @@ func tryToAttachL7RouteToListeners(
873872

874873
// findAttachableListeners returns a list of attachable listeners and whether the listener exists for a non-empty
875874
// sectionName.
876-
func findAttachableListeners(sectionName string, listeners []*Listener) ([]*Listener, bool) {
875+
func findAttachableListeners(ref *ParentRef, listeners []*Listener) ([]*Listener, bool) {
876+
sectionName := getSectionName(ref.SectionName)
877+
878+
// Case 1: sectionName is specified - look for that specific listener
877879
if sectionName != "" {
878880
for _, l := range listeners {
879881
if l.Name == sectionName {
880-
if l.Attachable {
882+
if l.Attachable && (ref.Port == nil || l.Source.Port == *ref.Port) {
881883
return []*Listener{l}, true
882884
}
885+
if ref.Port != nil && l.Source.Port != *ref.Port {
886+
return nil, false
887+
}
883888
return nil, true
884889
}
885890
}
886891
return nil, false
887892
}
888893

889-
attachableListeners := make([]*Listener, 0, len(listeners))
890-
for _, l := range listeners {
891-
if !l.Attachable {
892-
continue
894+
// Case 2: Only port is specified - find all attachable listeners matching that port
895+
if ref.Port != nil {
896+
var attachableListeners []*Listener
897+
var foundListener bool
898+
for _, l := range listeners {
899+
if l.Source.Port == *ref.Port {
900+
foundListener = true
901+
if l.Attachable {
902+
attachableListeners = append(attachableListeners, l)
903+
}
904+
}
893905
}
894-
895-
attachableListeners = append(attachableListeners, l)
906+
return attachableListeners, foundListener
896907
}
897908

898-
return attachableListeners, true
909+
// Case 3: Neither sectionName nor port specified - return all attachable listeners
910+
var attachableListeners []*Listener
911+
for _, l := range listeners {
912+
if l.Attachable {
913+
attachableListeners = append(attachableListeners, l)
914+
}
915+
}
916+
return attachableListeners, len(listeners) > 0
899917
}
900918

901919
func findAcceptedHostnames(listenerHostname *v1.Hostname, routeHostnames []v1.Hostname) []string {

‎internal/controller/state/graph/route_common_test.go

Lines changed: 157 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,9 @@ func TestBindRouteToListeners(t *testing.T) {
720720
Source: gw,
721721
Valid: true,
722722
Listeners: []*Listener{
723-
createListener("listener-80-1"),
723+
createModifiedListener("listener-80-1", func(l *Listener) {
724+
l.Source.Port = 80
725+
}),
724726
},
725727
},
726728
expectedSectionNameRefs: []ParentRef{
@@ -729,19 +731,24 @@ func TestBindRouteToListeners(t *testing.T) {
729731
Gateway: &ParentRefGateway{NamespacedName: client.ObjectKeyFromObject(gw)},
730732
SectionName: hrWithPort.Spec.ParentRefs[0].SectionName,
731733
Attachment: &ParentRefAttachmentStatus{
732-
Attached: false,
733-
FailedConditions: []conditions.Condition{
734-
conditions.NewRouteUnsupportedValue(
735-
`spec.parentRefs[0].port: Forbidden: cannot be set`,
736-
),
734+
Attached: true,
735+
FailedConditions: nil,
736+
AcceptedHostnames: map[string][]string{
737+
"test/gateway/listener-80-1": {"foo.example.com"},
737738
},
738-
AcceptedHostnames: map[string][]string{},
739+
ListenerPort: 80,
739740
},
740741
Port: hrWithPort.Spec.ParentRefs[0].Port,
741742
},
742743
},
743744
expectedGatewayListeners: []*Listener{
744-
createListener("listener-80-1"),
745+
func() *Listener {
746+
l := createModifiedListener("listener-80-1", func(l *Listener) {
747+
l.Source.Port = 80
748+
})
749+
l.Routes[CreateRouteKey(hrWithPort)] = routeWithPort
750+
return l
751+
}(),
745752
},
746753
name: "port is configured",
747754
},
@@ -1904,17 +1911,17 @@ func TestBindL4RouteToListeners(t *testing.T) {
19041911
Name: "gateway",
19051912
},
19061913
Listeners: []*Listener{
1907-
createListener("listener-443"),
1914+
createModifiedListener("listener-443", func(l *Listener) {
1915+
l.Source.Port = 443
1916+
}),
19081917
},
19091918
},
19101919
expectedSectionNameRefs: []ParentRef{
19111920
{
19121921
Attachment: &ParentRefAttachmentStatus{
19131922
AcceptedHostnames: map[string][]string{},
19141923
FailedConditions: []conditions.Condition{
1915-
conditions.NewRouteUnsupportedValue(
1916-
`spec.parentRefs[0].port: Forbidden: cannot be set`,
1917-
),
1924+
conditions.NewRouteNoMatchingParent(),
19181925
},
19191926
Attached: false,
19201927
},
@@ -1925,7 +1932,9 @@ func TestBindL4RouteToListeners(t *testing.T) {
19251932
},
19261933
},
19271934
expectedGatewayListeners: []*Listener{
1928-
createListener("listener-443"),
1935+
createModifiedListener("listener-443", func(l *Listener) {
1936+
l.Source.Port = 443
1937+
}),
19291938
},
19301939
name: "port is not nil",
19311940
},
@@ -3720,3 +3729,138 @@ func TestBindRoutesToListeners(t *testing.T) {
37203729
bindRoutesToListeners(nil, nil, nil, nil)
37213730
}).ToNot(Panic())
37223731
}
3732+
3733+
func TestFindAttachableListenersWithPort(t *testing.T) {
3734+
t.Parallel()
3735+
3736+
port80 := gatewayv1.PortNumber(80)
3737+
port443 := gatewayv1.PortNumber(443)
3738+
port8080 := gatewayv1.PortNumber(8080)
3739+
3740+
listeners := []*Listener{
3741+
{
3742+
Name: "http-80",
3743+
Attachable: true,
3744+
Source: gatewayv1.Listener{
3745+
Name: "http-80",
3746+
Port: port80,
3747+
},
3748+
},
3749+
{
3750+
Name: "https-443",
3751+
Attachable: true,
3752+
Source: gatewayv1.Listener{
3753+
Name: "https-443",
3754+
Port: port443,
3755+
},
3756+
},
3757+
{
3758+
Name: "http-8080",
3759+
Attachable: false, // not attachable
3760+
Source: gatewayv1.Listener{
3761+
Name: "http-8080",
3762+
Port: port8080,
3763+
},
3764+
},
3765+
}
3766+
3767+
tests := []struct {
3768+
name string
3769+
parentRef *ParentRef
3770+
expectedListeners []*Listener
3771+
expectedListenerExists bool
3772+
}{
3773+
{
3774+
name: "port 80 filter returns only port 80 listener",
3775+
parentRef: &ParentRef{
3776+
Port: &port80,
3777+
},
3778+
expectedListeners: []*Listener{listeners[0]}, // only http-80
3779+
expectedListenerExists: true,
3780+
},
3781+
{
3782+
name: "port 443 filter returns only port 443 listener",
3783+
parentRef: &ParentRef{
3784+
Port: &port443,
3785+
},
3786+
expectedListeners: []*Listener{listeners[1]}, // only https-443
3787+
expectedListenerExists: true,
3788+
},
3789+
{
3790+
name: "port 8080 filter returns empty because listener is not attachable",
3791+
parentRef: &ParentRef{
3792+
Port: &port8080,
3793+
},
3794+
expectedListeners: []*Listener{},
3795+
expectedListenerExists: true,
3796+
},
3797+
{
3798+
name: "port 9999 filter returns empty because no listener has that port",
3799+
parentRef: &ParentRef{
3800+
Port: helpers.GetPointer(gatewayv1.PortNumber(9999)),
3801+
},
3802+
expectedListeners: []*Listener{},
3803+
expectedListenerExists: false,
3804+
},
3805+
{
3806+
name: "no port specified returns all attachable listeners",
3807+
parentRef: &ParentRef{
3808+
Port: nil,
3809+
},
3810+
expectedListeners: []*Listener{listeners[0], listeners[1]}, // both attachable listeners
3811+
expectedListenerExists: true,
3812+
},
3813+
{
3814+
name: "sectionName with matching port returns that specific listener",
3815+
parentRef: &ParentRef{
3816+
SectionName: helpers.GetPointer(gatewayv1.SectionName("http-80")),
3817+
Port: &port80,
3818+
},
3819+
expectedListeners: []*Listener{listeners[0]},
3820+
expectedListenerExists: true,
3821+
},
3822+
{
3823+
name: "sectionName with non-matching port returns empty",
3824+
parentRef: &ParentRef{
3825+
SectionName: helpers.GetPointer(gatewayv1.SectionName("http-80")),
3826+
Port: &port443, // wrong port for http-80 listener
3827+
},
3828+
expectedListeners: []*Listener{},
3829+
expectedListenerExists: false,
3830+
},
3831+
{
3832+
name: "sectionName that doesn't exist returns empty with false",
3833+
parentRef: &ParentRef{
3834+
SectionName: helpers.GetPointer(gatewayv1.SectionName("nonexistent")),
3835+
Port: &port80,
3836+
},
3837+
expectedListeners: []*Listener{},
3838+
expectedListenerExists: false,
3839+
},
3840+
}
3841+
3842+
for _, tt := range tests {
3843+
t.Run(tt.name, func(t *testing.T) {
3844+
t.Parallel()
3845+
g := NewWithT(t)
3846+
3847+
attachableListeners, listenerExists := findAttachableListeners(tt.parentRef, listeners)
3848+
3849+
g.Expect(listenerExists).To(Equal(tt.expectedListenerExists))
3850+
g.Expect(attachableListeners).To(HaveLen(len(tt.expectedListeners)))
3851+
3852+
// Compare listeners by name since they're the same instances
3853+
expectedNames := make([]string, len(tt.expectedListeners))
3854+
for i, l := range tt.expectedListeners {
3855+
expectedNames[i] = l.Name
3856+
}
3857+
3858+
actualNames := make([]string, len(attachableListeners))
3859+
for i, l := range attachableListeners {
3860+
actualNames[i] = l.Name
3861+
}
3862+
3863+
g.Expect(actualNames).To(ConsistOf(expectedNames))
3864+
})
3865+
}
3866+
}

‎tests/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ GW_SERVICE_TYPE = NodePort## Service type to use for the gateway
1212
NGF_VERSION ?= edge## NGF version to be tested
1313
PULL_POLICY = Never## Pull policy for the images
1414
NGINX_CONF_DIR = internal/controller/nginx/conf
15-
SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteRequestPercentageMirror,HTTPRouteBackendProtocolWebSocket
15+
SUPPORTED_EXTENDED_FEATURES = HTTPRouteQueryParamMatching,HTTPRouteMethodMatching,HTTPRoutePortRedirect,HTTPRouteSchemeRedirect,HTTPRouteHostRewrite,HTTPRoutePathRewrite,GatewayPort8080,GatewayAddressEmpty,HTTPRouteResponseHeaderModification,HTTPRoutePathRedirect,GatewayHTTPListenerIsolation,GatewayInfrastructurePropagation,HTTPRouteRequestMirror,HTTPRouteRequestMultipleMirrors,HTTPRouteRequestPercentageMirror,HTTPRouteBackendProtocolWebSocket,HTTPRouteParentRefPort,HTTPRouteDestinationPortMatching
1616
STANDARD_CONFORMANCE_PROFILES = GATEWAY-HTTP,GATEWAY-GRPC
1717
EXPERIMENTAL_CONFORMANCE_PROFILES = GATEWAY-TLS
1818
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.

0 commit comments

Comments
 (0)