From 3f520d1190a4d87de3bafb21262b78f0988dd6c1 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Wed, 29 Jul 2026 15:11:53 +0800 Subject: [PATCH 1/5] fix: honor TCPRoute/UDPRoute sectionName and listener port for StreamRoute matching TranslateTCPRoute/TranslateUDPRoute emitted a single StreamRoute with no match criteria, so every TCP/UDP connection on any stream listener matched it and all L4 routes collided onto one backend, ignoring parentRefs.sectionName and per-listener port. Make TCPRoute/UDPRoute participate in the existing listener_port_match_mode (added for HTTPRoute/GRPCRoute in #419): the controllers populate tctx.Listeners from the listeners ParseRouteParentRefs matched for each parentRef, and the translator sets StreamRoute.server_port from the matched listener port(s), one StreamRoute per port, gated by shouldInjectServerPortVars. Default mode is off, so L4 behavior is unchanged unless the user opts into auto/explicit. Sync from apache/apisix-ingress-controller#2818 (fixes #2802). --- .../adc/translator/l4route_serverport_test.go | 221 ++++++++++++++++++ internal/adc/translator/tcproute.go | 76 +++++- internal/adc/translator/udproute.go | 13 +- internal/controller/tcproute_controller.go | 7 + internal/controller/udproute_controller.go | 7 + 5 files changed, 302 insertions(+), 22 deletions(-) create mode 100644 internal/adc/translator/l4route_serverport_test.go diff --git a/internal/adc/translator/l4route_serverport_test.go b/internal/adc/translator/l4route_serverport_test.go new file mode 100644 index 00000000..c44afcb4 --- /dev/null +++ b/internal/adc/translator/l4route_serverport_test.go @@ -0,0 +1,221 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 translator + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/provider" +) + +func tcpListener(name string, port int32) gatewayv1.Listener { + return gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Protocol: gatewayv1.TCPProtocolType, + Port: gatewayv1.PortNumber(port), + } +} + +func udpListener(name string, port int32) gatewayv1.Listener { + return gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Protocol: gatewayv1.UDPProtocolType, + Port: gatewayv1.PortNumber(port), + } +} + +// sectionParentRefs builds the parentRefs a controller would set on tctx when a +// route explicitly targets a listener by sectionName. +func sectionParentRefs(section string) []gatewayv1.ParentReference { + return []gatewayv1.ParentReference{ + { + Name: "gw", + SectionName: ptr.To(gatewayv1.SectionName(section)), + }, + } +} + +func TestTranslateTCPRouteServerPort(t *testing.T) { + tests := []struct { + name string + // listeners the controller would have matched for this route's parentRefs + listeners []gatewayv1.Listener + // parentRefs the controller stored on tctx (drives server_port injection) + parentRefs []gatewayv1.ParentReference + wantPorts []int32 + wantNoMatch bool + }{ + { + name: "explicit sectionName injects the matching listener port", + listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100)}, + parentRefs: sectionParentRefs("tcp-a"), + wantPorts: []int32{9100}, + }, + { + name: "multiple listener ports fan out even without explicit targeting", + listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100), tcpListener("tcp-b", 9101)}, + wantPorts: []int32{9100, 9101}, + }, + { + name: "duplicate ports across gateways are de-duplicated", + listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100), tcpListener("tcp-a2", 9100)}, + parentRefs: sectionParentRefs("tcp-a"), + wantPorts: []int32{9100}, + }, + { + name: "single listener without explicit targeting keeps a portless StreamRoute", + listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100)}, + wantNoMatch: true, + }, + { + name: "no matched listener falls back to a single portless StreamRoute", + listeners: nil, + parentRefs: sectionParentRefs("tcp-a"), + wantNoMatch: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // listener_port_match_mode defaults to off; these cases assert the + // injection behavior, so exercise the translator in auto mode. + translator := NewTranslator(logr.Discard(), config.ListenerPortMatchModeAuto) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Listeners = tt.listeners + tctx.RouteParentRefs = tt.parentRefs + + route := &gatewayv1alpha2.TCPRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "my-tcp", Namespace: "default"}, + Spec: gatewayv1alpha2.TCPRouteSpec{ + Rules: []gatewayv1alpha2.TCPRouteRule{ + {BackendRefs: []gatewayv1alpha2.BackendRef{}}, + }, + }, + } + + result, err := translator.TranslateTCPRoute(tctx, route) + require.NoError(t, err) + require.Len(t, result.Services, 1) + streamRoutes := result.Services[0].StreamRoutes + + if tt.wantNoMatch { + require.Len(t, streamRoutes, 1) + assert.Zero(t, streamRoutes[0].ServerPort) + return + } + + require.Len(t, streamRoutes, len(tt.wantPorts)) + gotPorts := make([]int32, 0, len(streamRoutes)) + ids := make(map[string]struct{}) + names := make(map[string]struct{}) + for _, sr := range streamRoutes { + gotPorts = append(gotPorts, sr.ServerPort) + ids[sr.ID] = struct{}{} + names[sr.Name] = struct{}{} + } + assert.ElementsMatch(t, tt.wantPorts, gotPorts) + // Distinct name/ID per listener port so StreamRoutes do not collide. + assert.Len(t, ids, len(streamRoutes)) + assert.Len(t, names, len(streamRoutes)) + }) + } +} + +func TestTranslateUDPRouteServerPort(t *testing.T) { + tests := []struct { + name string + listeners []gatewayv1.Listener + parentRefs []gatewayv1.ParentReference + wantPorts []int32 + wantNoMatch bool + }{ + { + name: "explicit sectionName injects the matching listener port", + listeners: []gatewayv1.Listener{udpListener("udp-a", 9200)}, + parentRefs: sectionParentRefs("udp-a"), + wantPorts: []int32{9200}, + }, + { + name: "two listeners on different ports produce distinct StreamRoutes", + listeners: []gatewayv1.Listener{udpListener("udp-a", 9200), udpListener("udp-b", 9201)}, + wantPorts: []int32{9200, 9201}, + }, + { + name: "single listener without explicit targeting keeps a portless StreamRoute", + listeners: []gatewayv1.Listener{udpListener("udp-a", 9200)}, + wantNoMatch: true, + }, + { + name: "no matched listener falls back to a single portless StreamRoute", + listeners: nil, + parentRefs: sectionParentRefs("udp-a"), + wantNoMatch: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // listener_port_match_mode defaults to off; these cases assert the + // injection behavior, so exercise the translator in auto mode. + translator := NewTranslator(logr.Discard(), config.ListenerPortMatchModeAuto) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Listeners = tt.listeners + tctx.RouteParentRefs = tt.parentRefs + + route := &gatewayv1alpha2.UDPRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "my-udp", Namespace: "default"}, + Spec: gatewayv1alpha2.UDPRouteSpec{ + Rules: []gatewayv1alpha2.UDPRouteRule{ + {BackendRefs: []gatewayv1alpha2.BackendRef{}}, + }, + }, + } + + result, err := translator.TranslateUDPRoute(tctx, route) + require.NoError(t, err) + require.Len(t, result.Services, 1) + streamRoutes := result.Services[0].StreamRoutes + + if tt.wantNoMatch { + require.Len(t, streamRoutes, 1) + assert.Zero(t, streamRoutes[0].ServerPort) + return + } + + require.Len(t, streamRoutes, len(tt.wantPorts)) + gotPorts := make([]int32, 0, len(streamRoutes)) + ids := make(map[string]struct{}) + for _, sr := range streamRoutes { + gotPorts = append(gotPorts, sr.ServerPort) + ids[sr.ID] = struct{}{} + } + assert.ElementsMatch(t, tt.wantPorts, gotPorts) + assert.Len(t, ids, len(streamRoutes)) + }) + } +} diff --git a/internal/adc/translator/tcproute.go b/internal/adc/translator/tcproute.go index 81e12a34..0ea87826 100644 --- a/internal/adc/translator/tcproute.go +++ b/internal/adc/translator/tcproute.go @@ -19,6 +19,7 @@ package translator import ( "fmt" + "sort" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -42,6 +43,68 @@ func newDefaultUpstreamWithoutScheme() *adctypes.Upstream { } } +// listenerPortSet returns the de-duplicated set of ports of the listeners the +// route attaches to. tctx.Listeners is populated by the controller from the +// listeners that ParseRouteParentRefs already matched against the route's +// parentRefs (honoring sectionName, port, protocol and allowedRoutes). +func listenerPortSet(tctx *provider.TranslateContext) map[int32]struct{} { + portSet := make(map[int32]struct{}, len(tctx.Listeners)) + for _, listener := range tctx.Listeners { + portSet[int32(listener.Port)] = struct{}{} + } + return portSet +} + +// buildL4StreamRoutes builds the StreamRoutes for one L4 route rule. +// +// A StreamRoute without a server_port match matches every connection on any +// stream listener, so multiple L4 routes collide onto one backend (#2802). To +// isolate them we set server_port from the matched listener port(s), emitting one +// StreamRoute per port. +// +// Whether to inject server_port is gated by shouldInjectServerPortVars (the same +// listener_port_match_mode used by HTTPRoute/GRPCRoute): the listener port is a +// logical Gateway value that must equal APISIX's physical stream listen port for +// the match to work, so injection is opt-in (explicit sectionName/port targeting, +// or more than one listener port). When it is not injected we keep the previous +// single portless StreamRoute, preserving backward compatibility. +func (t *Translator) buildL4StreamRoutes(tctx *provider.TranslateContext, namespace, name string, ruleIndex int, typ, routeKind string, labels map[string]string) []*adctypes.StreamRoute { + var ports []int32 + if portSet := listenerPortSet(tctx); t.shouldInjectServerPortVars(tctx.RouteParentRefs, portSet) { + ports = make([]int32, 0, len(portSet)) + for port := range portSet { + ports = append(ports, port) + } + sort.Slice(ports, func(i, j int) bool { return ports[i] < ports[j] }) + } + if len(ports) == 0 { + // No server_port isolation: a single StreamRoute that matches all + // connections on the stream listener, as before. + ports = []int32{0} + } + streamRoutes := make([]*adctypes.StreamRoute, 0, len(ports)) + for _, port := range ports { + streamRoute := adctypes.NewDefaultStreamRoute() + ruleKey := fmt.Sprintf("%d", ruleIndex) + if port != 0 { + // Include the port in the name key so multiple listeners produce + // distinct StreamRoute names/IDs instead of colliding. + ruleKey = fmt.Sprintf("%d-%d", ruleIndex, port) + streamRoute.ServerPort = port + } + streamRouteName := adctypes.ComposeStreamRouteName(namespace, name, ruleKey, typ) + streamRoute.Name = streamRouteName + streamRoute.ID = id.GenID(streamRouteName) + streamRoute.Labels = labels + // Attach L4RoutePolicy plugins at the stream_route level: the APISIX stream proxy + // applies plugins from the stream_route, not from the service. + streamRoute.Plugins = make(adctypes.Plugins) + t.AttachL4RoutePolicyPlugins(tctx.L4RoutePolicies, namespace, name, routeKind, streamRoute.Plugins) + streamRoutes = append(streamRoutes, streamRoute) + } + return streamRoutes +} + func (t *Translator) TranslateTCPRoute(tctx *provider.TranslateContext, tcpRoute *gatewayv1alpha2.TCPRoute) (*TranslateResult, error) { result := &TranslateResult{} rules := tcpRoute.Spec.Rules @@ -150,17 +213,8 @@ func (t *Translator) TranslateTCPRoute(tctx *provider.TranslateContext, tcpRoute } } } - streamRoute := adctypes.NewDefaultStreamRoute() - streamRouteName := adctypes.ComposeStreamRouteName(tcpRoute.Namespace, tcpRoute.Name, fmt.Sprintf("%d", ruleIndex), "TCP") - streamRoute.Name = streamRouteName - streamRoute.ID = id.GenID(streamRouteName) - streamRoute.Labels = labels - // TODO: support remote_addr, server_addr, sni, server_port - // Attach L4RoutePolicy plugins at the stream_route level: the APISIX stream proxy - // applies plugins from the stream_route, not from the service. - streamRoute.Plugins = make(adctypes.Plugins) - t.AttachL4RoutePolicyPlugins(tctx.L4RoutePolicies, tcpRoute.Namespace, tcpRoute.Name, "TCPRoute", streamRoute.Plugins) - service.StreamRoutes = append(service.StreamRoutes, streamRoute) + // TODO: support remote_addr, server_addr, sni + service.StreamRoutes = t.buildL4StreamRoutes(tctx, tcpRoute.Namespace, tcpRoute.Name, ruleIndex, "TCP", "TCPRoute", labels) result.Services = append(result.Services, service) } diff --git a/internal/adc/translator/udproute.go b/internal/adc/translator/udproute.go index cc7b6361..3fdbd817 100644 --- a/internal/adc/translator/udproute.go +++ b/internal/adc/translator/udproute.go @@ -139,17 +139,8 @@ func (t *Translator) TranslateUDPRoute(tctx *provider.TranslateContext, udpRoute } } } - streamRoute := adctypes.NewDefaultStreamRoute() - streamRouteName := adctypes.ComposeStreamRouteName(udpRoute.Namespace, udpRoute.Name, fmt.Sprintf("%d", ruleIndex), "UDP") - streamRoute.Name = streamRouteName - streamRoute.ID = id.GenID(streamRouteName) - streamRoute.Labels = labels - // TODO: support remote_addr, server_addr, sni, server_port - // Attach L4RoutePolicy plugins at the stream_route level: the APISIX stream proxy - // applies plugins from the stream_route, not from the service. - streamRoute.Plugins = make(adctypes.Plugins) - t.AttachL4RoutePolicyPlugins(tctx.L4RoutePolicies, udpRoute.Namespace, udpRoute.Name, "UDPRoute", streamRoute.Plugins) - service.StreamRoutes = append(service.StreamRoutes, streamRoute) + // TODO: support remote_addr, server_addr, sni + service.StreamRoutes = t.buildL4StreamRoutes(tctx, udpRoute.Namespace, udpRoute.Name, ruleIndex, "UDP", "UDPRoute", labels) result.Services = append(result.Services, service) } diff --git a/internal/controller/tcproute_controller.go b/internal/controller/tcproute_controller.go index a2360157..d1380ac8 100644 --- a/internal/controller/tcproute_controller.go +++ b/internal/controller/tcproute_controller.go @@ -293,6 +293,13 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c acceptStatus.status = false acceptStatus.msg = err.Error() } + // Populate the matched listeners so the translator can derive the + // StreamRoute server_port from the listener the route attaches to. + if len(gateway.Listeners) > 0 { + tctx.Listeners = appendListeners(tctx.Listeners, gateway.Listeners...) + } else if gateway.Listener != nil { + tctx.Listeners = appendListeners(tctx.Listeners, *gateway.Listener) + } } var backendRefErr error diff --git a/internal/controller/udproute_controller.go b/internal/controller/udproute_controller.go index 17fb5f6a..f5eebbc5 100644 --- a/internal/controller/udproute_controller.go +++ b/internal/controller/udproute_controller.go @@ -293,6 +293,13 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c acceptStatus.status = false acceptStatus.msg = err.Error() } + // Populate the matched listeners so the translator can derive the + // StreamRoute server_port from the listener the route attaches to. + if len(gateway.Listeners) > 0 { + tctx.Listeners = appendListeners(tctx.Listeners, gateway.Listeners...) + } else if gateway.Listener != nil { + tctx.Listeners = appendListeners(tctx.Listeners, *gateway.Listener) + } } var backendRefErr error From 0271d26eddc5118a6a6c0bf5be75704301a2c8e7 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Wed, 29 Jul 2026 16:07:33 +0800 Subject: [PATCH 2/5] fix: drop redundant PortNumber/int32 conversions (unconvert) gateway-api v1.6.0 defines PortNumber as an int32 alias, so the explicit int32()/PortNumber() conversions are no-ops that fail the unconvert linter. --- internal/adc/translator/l4route_serverport_test.go | 4 ++-- internal/adc/translator/tcproute.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/adc/translator/l4route_serverport_test.go b/internal/adc/translator/l4route_serverport_test.go index c44afcb4..28415646 100644 --- a/internal/adc/translator/l4route_serverport_test.go +++ b/internal/adc/translator/l4route_serverport_test.go @@ -37,7 +37,7 @@ func tcpListener(name string, port int32) gatewayv1.Listener { return gatewayv1.Listener{ Name: gatewayv1.SectionName(name), Protocol: gatewayv1.TCPProtocolType, - Port: gatewayv1.PortNumber(port), + Port: port, } } @@ -45,7 +45,7 @@ func udpListener(name string, port int32) gatewayv1.Listener { return gatewayv1.Listener{ Name: gatewayv1.SectionName(name), Protocol: gatewayv1.UDPProtocolType, - Port: gatewayv1.PortNumber(port), + Port: port, } } diff --git a/internal/adc/translator/tcproute.go b/internal/adc/translator/tcproute.go index 0ea87826..32f0d95d 100644 --- a/internal/adc/translator/tcproute.go +++ b/internal/adc/translator/tcproute.go @@ -50,7 +50,7 @@ func newDefaultUpstreamWithoutScheme() *adctypes.Upstream { func listenerPortSet(tctx *provider.TranslateContext) map[int32]struct{} { portSet := make(map[int32]struct{}, len(tctx.Listeners)) for _, listener := range tctx.Listeners { - portSet[int32(listener.Port)] = struct{}{} + portSet[listener.Port] = struct{}{} } return portSet } From cca5d77d3ef1119fd178b6dc68d5671a6f67d35b Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 30 Jul 2026 10:41:42 +0800 Subject: [PATCH 3/5] test(e2e): set L4 gateway listener ports to APISIX physical stream ports Keep the L4 e2e Gateway listeners at APISIX's physical stream_proxy ports (TCP 9100, UDP 9200), mirroring apache/apisix-ingress-controller#2818, so the tests are correct if listener_port_match_mode is ever set to auto. Under EE's current default (off) the StreamRoute is portless and the port is irrelevant, so behavior is unchanged. --- test/e2e/gatewayapi/tcproute.go | 10 ++++++++-- test/e2e/gatewayapi/udproute.go | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/test/e2e/gatewayapi/tcproute.go b/test/e2e/gatewayapi/tcproute.go index 3af0d4ea..36147030 100644 --- a/test/e2e/gatewayapi/tcproute.go +++ b/test/e2e/gatewayapi/tcproute.go @@ -43,7 +43,10 @@ spec: listeners: - name: tcp protocol: TCP - port: 80 + # Must equal APISIX's physical stream_proxy TCP port so that when + # listener_port_match_mode=auto the injected server_port matches the port + # connections arrive on (see apache/apisix-ingress-controller#2818). + port: 9100 allowedRoutes: kinds: - kind: TCPRoute @@ -119,7 +122,10 @@ spec: listeners: - name: tcp protocol: TCP - port: 80 + # Must equal APISIX's physical stream_proxy TCP port so that when + # listener_port_match_mode=auto the injected server_port matches the port + # connections arrive on (see apache/apisix-ingress-controller#2818). + port: 9100 allowedRoutes: kinds: - kind: TCPRoute diff --git a/test/e2e/gatewayapi/udproute.go b/test/e2e/gatewayapi/udproute.go index f59737f2..85f2e544 100644 --- a/test/e2e/gatewayapi/udproute.go +++ b/test/e2e/gatewayapi/udproute.go @@ -40,7 +40,8 @@ spec: listeners: - name: udp protocol: UDP - port: 80 + # Must equal APISIX's physical stream_proxy UDP port (see tcproute.go). + port: 9200 allowedRoutes: kinds: - kind: UDPRoute From 512f826e927e431aaeb814c97b654865aaf64f8e Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 30 Jul 2026 13:31:23 +0800 Subject: [PATCH 4/5] fix: derive explicit listener target from matched listeners hasExplicitListenerTarget previously inspected the raw route parentRefs, so an explicit parentRef that did not resolve to any matched listener (e.g. a sectionName pointing at a non-existent listener) still forced server_port injection. Combined with a valid implicit parentRef on a single listener, that pinned the route to a logical port that need not equal APISIX's physical listen port, making it unreachable in auto mode. Derive the explicit-target signal from the listeners the route actually matched (tctx.Listeners), passed through shouldInjectServerPortVars. Shared by HTTPRoute/GRPCRoute/TCPRoute/UDPRoute. Sync from apache/apisix-ingress-controller#2818. --- internal/adc/translator/annotations_test.go | 29 ++++++++++++++++++--- internal/adc/translator/grpcroute.go | 2 +- internal/adc/translator/httproute.go | 2 +- internal/adc/translator/tcproute.go | 2 +- internal/adc/translator/translator.go | 25 ++++++++++++++---- 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/internal/adc/translator/annotations_test.go b/internal/adc/translator/annotations_test.go index 7b4d1ec4..08ab3e1f 100644 --- a/internal/adc/translator/annotations_test.go +++ b/internal/adc/translator/annotations_test.go @@ -458,13 +458,18 @@ func TestAddServerPortVars(t *testing.T) { func TestShouldInjectServerPortVars(t *testing.T) { sectionName := gatewayv1.SectionName("http-main") + missingSection := gatewayv1.SectionName("does-not-exist") port := gatewayv1.PortNumber(9080) + // matchedListener is the listener http-main/9080 the explicit parentRefs + // below actually resolve to; the explicit signal is derived from it. + matchedListener := gatewayv1.Listener{Name: sectionName, Port: port} tests := []struct { name string mode config.ListenerPortMatchMode parentRefs []gatewayv1.ParentReference ports map[int32]struct{} + listeners []gatewayv1.Listener expected bool }{ { @@ -493,7 +498,24 @@ func TestShouldInjectServerPortVars(t *testing.T) { ports: map[int32]struct{}{ 9080: {}, }, - expected: true, + listeners: []gatewayv1.Listener{matchedListener}, + expected: true, + }, + { + name: "auto mode: unmatched sectionName does not force injection on an implicit single-listener route", + mode: config.ListenerPortMatchModeAuto, + parentRefs: []gatewayv1.ParentReference{ + // Explicit but unresolved: no listener named "does-not-exist" + // was matched, so it must not count as an explicit target. + {Name: "gw", SectionName: &missingSection}, + // Valid implicit parentRef that matched the single listener. + {Name: "gw"}, + }, + ports: map[int32]struct{}{ + 9080: {}, + }, + listeners: []gatewayv1.Listener{matchedListener}, + expected: false, }, { name: "multiple ports without sectionName", @@ -528,7 +550,8 @@ func TestShouldInjectServerPortVars(t *testing.T) { ports: map[int32]struct{}{ 9080: {}, }, - expected: true, + listeners: []gatewayv1.Listener{matchedListener}, + expected: true, }, { name: "explicit mode with single port and no explicit target", @@ -593,7 +616,7 @@ func TestShouldInjectServerPortVars(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { translator := &Translator{ListenerPortMatchMode: tt.mode} - assert.Equal(t, tt.expected, translator.shouldInjectServerPortVars(tt.parentRefs, tt.ports)) + assert.Equal(t, tt.expected, translator.shouldInjectServerPortVars(tt.parentRefs, tt.ports, tt.listeners)) }) } } diff --git a/internal/adc/translator/grpcroute.go b/internal/adc/translator/grpcroute.go index 6b231e2d..008e4ffa 100644 --- a/internal/adc/translator/grpcroute.go +++ b/internal/adc/translator/grpcroute.go @@ -315,7 +315,7 @@ func (t *Translator) TranslateGRPCRoute(tctx *provider.TranslateContext, grpcRou listenerPorts[listener.Port] = struct{}{} } - if t.shouldInjectServerPortVars(tctx.RouteParentRefs, listenerPorts) { + if t.shouldInjectServerPortVars(tctx.RouteParentRefs, listenerPorts, tctx.Listeners) { for _, route := range routes { addServerPortVars(route, listenerPorts) } diff --git a/internal/adc/translator/httproute.go b/internal/adc/translator/httproute.go index eb48f6fe..33710042 100644 --- a/internal/adc/translator/httproute.go +++ b/internal/adc/translator/httproute.go @@ -761,7 +761,7 @@ func (t *Translator) TranslateHTTPRoute(tctx *provider.TranslateContext, httpRou // Add server_port matching only when a route explicitly targets a listener // or when multiple listener ports need to be disambiguated. - if t.shouldInjectServerPortVars(tctx.RouteParentRefs, listenerPorts) { + if t.shouldInjectServerPortVars(tctx.RouteParentRefs, listenerPorts, tctx.Listeners) { for _, route := range routes { addServerPortVars(route, listenerPorts) } diff --git a/internal/adc/translator/tcproute.go b/internal/adc/translator/tcproute.go index 32f0d95d..5a7af165 100644 --- a/internal/adc/translator/tcproute.go +++ b/internal/adc/translator/tcproute.go @@ -70,7 +70,7 @@ func listenerPortSet(tctx *provider.TranslateContext) map[int32]struct{} { // single portless StreamRoute, preserving backward compatibility. func (t *Translator) buildL4StreamRoutes(tctx *provider.TranslateContext, namespace, name string, ruleIndex int, typ, routeKind string, labels map[string]string) []*adctypes.StreamRoute { var ports []int32 - if portSet := listenerPortSet(tctx); t.shouldInjectServerPortVars(tctx.RouteParentRefs, portSet) { + if portSet := listenerPortSet(tctx); t.shouldInjectServerPortVars(tctx.RouteParentRefs, portSet, tctx.Listeners) { ports = make([]int32, 0, len(portSet)) for port := range portSet { ports = append(ports, port) diff --git a/internal/adc/translator/translator.go b/internal/adc/translator/translator.go index d4b06ade..17b58680 100644 --- a/internal/adc/translator/translator.go +++ b/internal/adc/translator/translator.go @@ -50,7 +50,14 @@ func NewTranslator(log logr.Logger, mode config.ListenerPortMatchMode) *Translat } } -func hasExplicitListenerTarget(parentRefs []gatewayv1.ParentReference) bool { +// hasExplicitListenerTarget reports whether the route explicitly targets one of +// the listeners it actually matched. The explicit signal is derived from the +// matched listeners, not the raw parentRefs: a parentRef whose sectionName/port +// does not resolve to any matched listener (e.g. a sectionName pointing at a +// non-existent listener) must not force server_port injection on an unrelated +// implicit parentRef, which would pin the route to a logical port that need not +// equal APISIX's physical listen port and make it unreachable. +func hasExplicitListenerTarget(parentRefs []gatewayv1.ParentReference, listeners []gatewayv1.Listener) bool { for _, parentRef := range parentRefs { // Skip non-Gateway parentRefs (e.g. GAMMA Service mesh refs) — they // are not relevant to listener port injection. @@ -58,22 +65,30 @@ func hasExplicitListenerTarget(parentRefs []gatewayv1.ParentReference) bool { continue } if parentRef.SectionName != nil && *parentRef.SectionName != "" { - return true + for _, listener := range listeners { + if listener.Name == *parentRef.SectionName { + return true + } + } } if parentRef.Port != nil { - return true + for _, listener := range listeners { + if listener.Port == *parentRef.Port { + return true + } + } } } return false } -func (t *Translator) shouldInjectServerPortVars(parentRefs []gatewayv1.ParentReference, ports map[int32]struct{}) bool { +func (t *Translator) shouldInjectServerPortVars(parentRefs []gatewayv1.ParentReference, ports map[int32]struct{}, listeners []gatewayv1.Listener) bool { if len(ports) == 0 { return false } - explicit := hasExplicitListenerTarget(parentRefs) + explicit := hasExplicitListenerTarget(parentRefs, listeners) switch t.ListenerPortMatchMode { case config.ListenerPortMatchModeExplicit: From f00815313473b4f388ce0e7d417a6cd8428f8f4e Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Thu, 30 Jul 2026 13:59:41 +0800 Subject: [PATCH 5/5] fix: derive listener explicit-match from the parentRef's own Gateway hasExplicitListenerTarget matched a parentRef's sectionName/port against the flattened union of matched listeners, losing Gateway provenance: an invalid explicit ref on one Gateway could be satisfied by an identically named/ported listener matched through a different parentRef's Gateway, wrongly forcing server_port injection in auto mode. Compute the explicit signal during ParseRouteParentRefs, where each parentRef's Gateway and matched listeners are known (RouteParentRefContext.ExplicitListenerMatch), aggregate it onto TranslateContext.HasExplicitListenerMatch, and have shouldInjectServerPortVars consume the boolean. This removes the fragile translator-side re-matching entirely. Add cross-Gateway name/port-collision coverage in ParseRouteParentRefs tests. --- internal/adc/translator/annotations_test.go | 171 +++++------------- internal/adc/translator/grpcroute.go | 2 +- internal/adc/translator/grpcroute_test.go | 105 +++-------- internal/adc/translator/httproute.go | 2 +- .../adc/translator/l4route_serverport_test.go | 52 +++--- internal/adc/translator/tcproute.go | 2 +- internal/adc/translator/translator.go | 49 ++--- internal/controller/context.go | 8 + internal/controller/grpcroute_controller.go | 1 + internal/controller/httproute_controller.go | 1 + internal/controller/tcproute_controller.go | 1 + internal/controller/udproute_controller.go | 1 + internal/controller/utils.go | 14 ++ internal/controller/utils_parentref_test.go | 92 ++++++++++ internal/provider/provider.go | 5 + 15 files changed, 226 insertions(+), 280 deletions(-) diff --git a/internal/adc/translator/annotations_test.go b/internal/adc/translator/annotations_test.go index 08ab3e1f..b61249a6 100644 --- a/internal/adc/translator/annotations_test.go +++ b/internal/adc/translator/annotations_test.go @@ -21,8 +21,6 @@ import ( "github.com/incubator4/go-resty-expr/expr" "github.com/stretchr/testify/assert" - "k8s.io/utils/ptr" - gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" adctypes "github.com/apache/apisix-ingress-controller/api/adc" "github.com/apache/apisix-ingress-controller/internal/adc/translator/annotations" @@ -457,158 +455,77 @@ func TestAddServerPortVars(t *testing.T) { } func TestShouldInjectServerPortVars(t *testing.T) { - sectionName := gatewayv1.SectionName("http-main") - missingSection := gatewayv1.SectionName("does-not-exist") - port := gatewayv1.PortNumber(9080) - // matchedListener is the listener http-main/9080 the explicit parentRefs - // below actually resolve to; the explicit signal is derived from it. - matchedListener := gatewayv1.Listener{Name: sectionName, Port: port} - + // explicit is the provenance-aware signal the controller derives from the + // matched RouteParentRefContext (TranslateContext.HasExplicitListenerMatch); + // here we exercise how each mode consumes it together with the port count. tests := []struct { - name string - mode config.ListenerPortMatchMode - parentRefs []gatewayv1.ParentReference - ports map[int32]struct{} - listeners []gatewayv1.Listener - expected bool + name string + mode config.ListenerPortMatchMode + explicit bool + ports map[int32]struct{} + expected bool }{ { name: "empty listener ports", mode: config.ListenerPortMatchModeAuto, + explicit: true, ports: map[int32]struct{}{}, expected: false, }, { - name: "single port without sectionName", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, + name: "auto mode: single port without explicit target", + mode: config.ListenerPortMatchModeAuto, + explicit: false, + ports: map[int32]struct{}{9080: {}}, expected: false, }, { - name: "single port with sectionName", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, - listeners: []gatewayv1.Listener{matchedListener}, - expected: true, - }, - { - name: "auto mode: unmatched sectionName does not force injection on an implicit single-listener route", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - // Explicit but unresolved: no listener named "does-not-exist" - // was matched, so it must not count as an explicit target. - {Name: "gw", SectionName: &missingSection}, - // Valid implicit parentRef that matched the single listener. - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, - listeners: []gatewayv1.Listener{matchedListener}, - expected: false, - }, - { - name: "multiple ports without sectionName", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - 9081: {}, - }, + name: "auto mode: single port with explicit target", + mode: config.ListenerPortMatchModeAuto, + explicit: true, + ports: map[int32]struct{}{9080: {}}, expected: true, }, { - name: "explicit mode with multiple ports and no explicit target", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - 9081: {}, - }, - expected: false, - }, - { - name: "explicit mode with parentRef.port", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", Port: &port}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, - listeners: []gatewayv1.Listener{matchedListener}, - expected: true, + name: "auto mode: multiple ports without explicit target", + mode: config.ListenerPortMatchModeAuto, + explicit: false, + ports: map[int32]struct{}{9080: {}, 9081: {}}, + expected: true, }, { - name: "explicit mode with single port and no explicit target", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, + name: "explicit mode: multiple ports without explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: false, + ports: map[int32]struct{}{9080: {}, 9081: {}}, expected: false, }, { - name: "off mode ignores explicit target", - mode: config.ListenerPortMatchModeOff, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, - ports: map[int32]struct{}{ - 9080: {}, - 9081: {}, - }, - expected: false, + name: "explicit mode: single port with explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: true, + ports: map[int32]struct{}{9080: {}}, + expected: true, }, { - name: "off mode ignores explicit parentRef.port target", - mode: config.ListenerPortMatchModeOff, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", Port: &port}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, + name: "explicit mode: single port without explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: false, + ports: map[int32]struct{}{9080: {}}, expected: false, }, { - name: "explicit mode: non-Gateway parentRef with port is not treated as explicit target", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - {Name: "svc", Kind: ptr.To(gatewayv1.Kind("Service")), Port: &port}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, + name: "off mode: ignores explicit target with multiple ports", + mode: config.ListenerPortMatchModeOff, + explicit: true, + ports: map[int32]struct{}{9080: {}, 9081: {}}, expected: false, }, { - name: "auto mode: non-Gateway parentRef with port does not trigger single-port injection", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - {Name: "svc", Kind: ptr.To(gatewayv1.Kind("Service")), Port: &port}, - }, - ports: map[int32]struct{}{ - 9080: {}, - }, + name: "off mode: ignores explicit target with single port", + mode: config.ListenerPortMatchModeOff, + explicit: true, + ports: map[int32]struct{}{9080: {}}, expected: false, }, } @@ -616,7 +533,7 @@ func TestShouldInjectServerPortVars(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { translator := &Translator{ListenerPortMatchMode: tt.mode} - assert.Equal(t, tt.expected, translator.shouldInjectServerPortVars(tt.parentRefs, tt.ports, tt.listeners)) + assert.Equal(t, tt.expected, translator.shouldInjectServerPortVars(tt.explicit, tt.ports)) }) } } diff --git a/internal/adc/translator/grpcroute.go b/internal/adc/translator/grpcroute.go index 008e4ffa..bf378759 100644 --- a/internal/adc/translator/grpcroute.go +++ b/internal/adc/translator/grpcroute.go @@ -315,7 +315,7 @@ func (t *Translator) TranslateGRPCRoute(tctx *provider.TranslateContext, grpcRou listenerPorts[listener.Port] = struct{}{} } - if t.shouldInjectServerPortVars(tctx.RouteParentRefs, listenerPorts, tctx.Listeners) { + if t.shouldInjectServerPortVars(tctx.HasExplicitListenerMatch, listenerPorts) { for _, route := range routes { addServerPortVars(route, listenerPorts) } diff --git a/internal/adc/translator/grpcroute_test.go b/internal/adc/translator/grpcroute_test.go index 79838b21..201747cc 100644 --- a/internal/adc/translator/grpcroute_test.go +++ b/internal/adc/translator/grpcroute_test.go @@ -30,9 +30,6 @@ import ( ) func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { - sectionName := gatewayv1.SectionName("grpc-main") - parentPort := gatewayv1.PortNumber(9080) - singlePortVars := adctypes.Vars{ { {StrVal: "server_port"}, @@ -52,40 +49,27 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { } tests := []struct { - name string - mode config.ListenerPortMatchMode - parentRefs []gatewayv1.ParentReference - listeners []gatewayv1.Listener - expected adctypes.Vars + name string + mode config.ListenerPortMatchMode + // explicit is the provenance-aware signal the controller stores on tctx + // (HasExplicitListenerMatch); the cross-Gateway sectionName/port + // resolution that computes it is covered by the controller tests. + explicit bool + listeners []gatewayv1.Listener + expected adctypes.Vars }{ { name: "auto mode: no injection for single listener without explicit target", mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, }, expected: nil, }, { - name: "auto mode: inject for sectionName target", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, - listeners: []gatewayv1.Listener{ - {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, - }, - expected: singlePortVars, - }, - { - name: "auto mode: inject for port target", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", Port: &parentPort}, - }, + name: "auto mode: inject for explicit sectionName target", + mode: config.ListenerPortMatchModeAuto, + explicit: true, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, }, @@ -94,9 +78,6 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { { name: "auto mode: inject for multiple listener ports", mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9081)}, {Name: "grpc-alt", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, @@ -104,35 +85,9 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { expected: multiPortVars, }, { - name: "auto mode: inject for multiple listener ports when listener names collide across gateways", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw-a"}, - {Name: "gw-b"}, - }, - listeners: []gatewayv1.Listener{ - {Name: "http", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9081)}, - {Name: "http", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, - }, - expected: multiPortVars, - }, - { - name: "explicit mode: inject for sectionName target", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, - listeners: []gatewayv1.Listener{ - {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, - }, - expected: singlePortVars, - }, - { - name: "explicit mode: inject for port target", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", Port: &parentPort}, - }, + name: "explicit mode: inject for explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: true, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, }, @@ -141,9 +96,6 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { { name: "explicit mode: no injection for multiple listener ports without explicit target", mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9081)}, {Name: "grpc-alt", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, @@ -151,11 +103,9 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { expected: nil, }, { - name: "off mode: no injection even with sectionName target", - mode: config.ListenerPortMatchModeOff, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, + name: "off mode: no injection even with explicit target", + mode: config.ListenerPortMatchModeOff, + explicit: true, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, }, @@ -164,9 +114,6 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { { name: "off mode: no injection for multiple listener ports", mode: config.ListenerPortMatchModeOff, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9081)}, {Name: "grpc-alt", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, @@ -174,22 +121,18 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { expected: nil, }, { - name: "empty mode normalizes to off", - mode: "", - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", Port: &parentPort}, - }, + name: "empty mode normalizes to off", + mode: "", + explicit: true, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, }, expected: nil, }, { - name: "unknown mode normalizes to off", - mode: "bogus", - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, + name: "unknown mode normalizes to off", + mode: "bogus", + explicit: true, listeners: []gatewayv1.Listener{ {Name: "grpc-main", Protocol: gatewayv1.HTTPProtocolType, Port: gatewayv1.PortNumber(9080)}, }, @@ -200,7 +143,7 @@ func TestTranslateGRPCRouteServerPortVarsByMode(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tctx := provider.NewDefaultTranslateContext(context.Background()) - tctx.RouteParentRefs = tt.parentRefs + tctx.HasExplicitListenerMatch = tt.explicit tctx.Listeners = tt.listeners grpcRoute := &gatewayv1.GRPCRoute{ diff --git a/internal/adc/translator/httproute.go b/internal/adc/translator/httproute.go index 33710042..b382adcc 100644 --- a/internal/adc/translator/httproute.go +++ b/internal/adc/translator/httproute.go @@ -761,7 +761,7 @@ func (t *Translator) TranslateHTTPRoute(tctx *provider.TranslateContext, httpRou // Add server_port matching only when a route explicitly targets a listener // or when multiple listener ports need to be disambiguated. - if t.shouldInjectServerPortVars(tctx.RouteParentRefs, listenerPorts, tctx.Listeners) { + if t.shouldInjectServerPortVars(tctx.HasExplicitListenerMatch, listenerPorts) { for _, route := range routes { addServerPortVars(route, listenerPorts) } diff --git a/internal/adc/translator/l4route_serverport_test.go b/internal/adc/translator/l4route_serverport_test.go index 28415646..778d2f87 100644 --- a/internal/adc/translator/l4route_serverport_test.go +++ b/internal/adc/translator/l4route_serverport_test.go @@ -25,7 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -49,32 +48,23 @@ func udpListener(name string, port int32) gatewayv1.Listener { } } -// sectionParentRefs builds the parentRefs a controller would set on tctx when a -// route explicitly targets a listener by sectionName. -func sectionParentRefs(section string) []gatewayv1.ParentReference { - return []gatewayv1.ParentReference{ - { - Name: "gw", - SectionName: ptr.To(gatewayv1.SectionName(section)), - }, - } -} - func TestTranslateTCPRouteServerPort(t *testing.T) { tests := []struct { name string // listeners the controller would have matched for this route's parentRefs listeners []gatewayv1.Listener - // parentRefs the controller stored on tctx (drives server_port injection) - parentRefs []gatewayv1.ParentReference + // explicit is the provenance-aware signal the controller stores on tctx + // (HasExplicitListenerMatch) when the route targeted a listener by an + // explicit sectionName or port. + explicit bool wantPorts []int32 wantNoMatch bool }{ { - name: "explicit sectionName injects the matching listener port", - listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100)}, - parentRefs: sectionParentRefs("tcp-a"), - wantPorts: []int32{9100}, + name: "explicit sectionName injects the matching listener port", + listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100)}, + explicit: true, + wantPorts: []int32{9100}, }, { name: "multiple listener ports fan out even without explicit targeting", @@ -82,10 +72,10 @@ func TestTranslateTCPRouteServerPort(t *testing.T) { wantPorts: []int32{9100, 9101}, }, { - name: "duplicate ports across gateways are de-duplicated", - listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100), tcpListener("tcp-a2", 9100)}, - parentRefs: sectionParentRefs("tcp-a"), - wantPorts: []int32{9100}, + name: "duplicate ports across gateways are de-duplicated", + listeners: []gatewayv1.Listener{tcpListener("tcp-a", 9100), tcpListener("tcp-a2", 9100)}, + explicit: true, + wantPorts: []int32{9100}, }, { name: "single listener without explicit targeting keeps a portless StreamRoute", @@ -95,7 +85,7 @@ func TestTranslateTCPRouteServerPort(t *testing.T) { { name: "no matched listener falls back to a single portless StreamRoute", listeners: nil, - parentRefs: sectionParentRefs("tcp-a"), + explicit: true, wantNoMatch: true, }, } @@ -107,7 +97,7 @@ func TestTranslateTCPRouteServerPort(t *testing.T) { translator := NewTranslator(logr.Discard(), config.ListenerPortMatchModeAuto) tctx := provider.NewDefaultTranslateContext(context.Background()) tctx.Listeners = tt.listeners - tctx.RouteParentRefs = tt.parentRefs + tctx.HasExplicitListenerMatch = tt.explicit route := &gatewayv1alpha2.TCPRoute{ ObjectMeta: metav1.ObjectMeta{Name: "my-tcp", Namespace: "default"}, @@ -150,15 +140,15 @@ func TestTranslateUDPRouteServerPort(t *testing.T) { tests := []struct { name string listeners []gatewayv1.Listener - parentRefs []gatewayv1.ParentReference + explicit bool wantPorts []int32 wantNoMatch bool }{ { - name: "explicit sectionName injects the matching listener port", - listeners: []gatewayv1.Listener{udpListener("udp-a", 9200)}, - parentRefs: sectionParentRefs("udp-a"), - wantPorts: []int32{9200}, + name: "explicit sectionName injects the matching listener port", + listeners: []gatewayv1.Listener{udpListener("udp-a", 9200)}, + explicit: true, + wantPorts: []int32{9200}, }, { name: "two listeners on different ports produce distinct StreamRoutes", @@ -173,7 +163,7 @@ func TestTranslateUDPRouteServerPort(t *testing.T) { { name: "no matched listener falls back to a single portless StreamRoute", listeners: nil, - parentRefs: sectionParentRefs("udp-a"), + explicit: true, wantNoMatch: true, }, } @@ -185,7 +175,7 @@ func TestTranslateUDPRouteServerPort(t *testing.T) { translator := NewTranslator(logr.Discard(), config.ListenerPortMatchModeAuto) tctx := provider.NewDefaultTranslateContext(context.Background()) tctx.Listeners = tt.listeners - tctx.RouteParentRefs = tt.parentRefs + tctx.HasExplicitListenerMatch = tt.explicit route := &gatewayv1alpha2.UDPRoute{ ObjectMeta: metav1.ObjectMeta{Name: "my-udp", Namespace: "default"}, diff --git a/internal/adc/translator/tcproute.go b/internal/adc/translator/tcproute.go index 5a7af165..c7a3192d 100644 --- a/internal/adc/translator/tcproute.go +++ b/internal/adc/translator/tcproute.go @@ -70,7 +70,7 @@ func listenerPortSet(tctx *provider.TranslateContext) map[int32]struct{} { // single portless StreamRoute, preserving backward compatibility. func (t *Translator) buildL4StreamRoutes(tctx *provider.TranslateContext, namespace, name string, ruleIndex int, typ, routeKind string, labels map[string]string) []*adctypes.StreamRoute { var ports []int32 - if portSet := listenerPortSet(tctx); t.shouldInjectServerPortVars(tctx.RouteParentRefs, portSet, tctx.Listeners) { + if portSet := listenerPortSet(tctx); t.shouldInjectServerPortVars(tctx.HasExplicitListenerMatch, portSet) { ports = make([]int32, 0, len(portSet)) for port := range portSet { ports = append(ports, port) diff --git a/internal/adc/translator/translator.go b/internal/adc/translator/translator.go index 17b58680..50ae5122 100644 --- a/internal/adc/translator/translator.go +++ b/internal/adc/translator/translator.go @@ -19,7 +19,6 @@ package translator import ( "github.com/go-logr/logr" - gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" adctypes "github.com/apache/apisix-ingress-controller/api/adc" "github.com/apache/apisix-ingress-controller/internal/controller/config" @@ -50,46 +49,20 @@ func NewTranslator(log logr.Logger, mode config.ListenerPortMatchMode) *Translat } } -// hasExplicitListenerTarget reports whether the route explicitly targets one of -// the listeners it actually matched. The explicit signal is derived from the -// matched listeners, not the raw parentRefs: a parentRef whose sectionName/port -// does not resolve to any matched listener (e.g. a sectionName pointing at a -// non-existent listener) must not force server_port injection on an unrelated -// implicit parentRef, which would pin the route to a logical port that need not -// equal APISIX's physical listen port and make it unreachable. -func hasExplicitListenerTarget(parentRefs []gatewayv1.ParentReference, listeners []gatewayv1.Listener) bool { - for _, parentRef := range parentRefs { - // Skip non-Gateway parentRefs (e.g. GAMMA Service mesh refs) — they - // are not relevant to listener port injection. - if parentRef.Kind != nil && *parentRef.Kind != "Gateway" { - continue - } - if parentRef.SectionName != nil && *parentRef.SectionName != "" { - for _, listener := range listeners { - if listener.Name == *parentRef.SectionName { - return true - } - } - } - if parentRef.Port != nil { - for _, listener := range listeners { - if listener.Port == *parentRef.Port { - return true - } - } - } - } - - return false -} - -func (t *Translator) shouldInjectServerPortVars(parentRefs []gatewayv1.ParentReference, ports map[int32]struct{}, listeners []gatewayv1.Listener) bool { +// shouldInjectServerPortVars decides whether to pin StreamRoutes/routes to the +// matched listener port(s) via server_port. +// +// explicit reports whether the route attached to its Gateway through an explicit +// sectionName or port. It is computed by the controller from the matched +// RouteParentRefContext (provider.TranslateContext.HasExplicitListenerMatch), +// where each parentRef's Gateway and matched listeners are known, so an invalid +// explicit ref on one Gateway can never be satisfied by a same-named/ported +// listener matched through a different parentRef's Gateway. +func (t *Translator) shouldInjectServerPortVars(explicit bool, ports map[int32]struct{}) bool { if len(ports) == 0 { return false } - explicit := hasExplicitListenerTarget(parentRefs, listeners) - switch t.ListenerPortMatchMode { case config.ListenerPortMatchModeExplicit: return explicit @@ -97,7 +70,7 @@ func (t *Translator) shouldInjectServerPortVars(parentRefs []gatewayv1.ParentRef return explicit || len(ports) > 1 default: // off, including anything normalizeMode resolved to it if explicit { - t.Log.V(1).Info("listener_port_match_mode is 'off'; ignoring explicit listener targeting", "parent_refs", len(parentRefs)) + t.Log.V(1).Info("listener_port_match_mode is 'off'; ignoring explicit listener targeting") } return false } diff --git a/internal/controller/context.go b/internal/controller/context.go index 686dd046..25c318bf 100644 --- a/internal/controller/context.go +++ b/internal/controller/context.go @@ -29,5 +29,13 @@ type RouteParentRefContext struct { Listener *gatewayv1.Listener Listeners []gatewayv1.Listener + // ExplicitListenerMatch reports whether this parentRef attached to its + // Gateway through an explicit sectionName or port that resolved to one of + // the listeners above. It is computed here, where each parentRef's Gateway + // and matched listeners are known, so the server_port injection decision + // never matches a sectionName/port against a listener from a different + // parentRef's Gateway. + ExplicitListenerMatch bool + Conditions []metav1.Condition } diff --git a/internal/controller/grpcroute_controller.go b/internal/controller/grpcroute_controller.go index 63b97105..6336d0b7 100644 --- a/internal/controller/grpcroute_controller.go +++ b/internal/controller/grpcroute_controller.go @@ -208,6 +208,7 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // Fallback for backward compatibility tctx.Listeners = appendListeners(tctx.Listeners, *gateway.Listener) } + tctx.HasExplicitListenerMatch = tctx.HasExplicitListenerMatch || gateway.ExplicitListenerMatch } var backendRefErr error diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index fd750062..63747796 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -214,6 +214,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // Fallback for backward compatibility tctx.Listeners = appendListeners(tctx.Listeners, *gateway.Listener) } + tctx.HasExplicitListenerMatch = tctx.HasExplicitListenerMatch || gateway.ExplicitListenerMatch } var backendRefErr error diff --git a/internal/controller/tcproute_controller.go b/internal/controller/tcproute_controller.go index d1380ac8..b623d715 100644 --- a/internal/controller/tcproute_controller.go +++ b/internal/controller/tcproute_controller.go @@ -300,6 +300,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } else if gateway.Listener != nil { tctx.Listeners = appendListeners(tctx.Listeners, *gateway.Listener) } + tctx.HasExplicitListenerMatch = tctx.HasExplicitListenerMatch || gateway.ExplicitListenerMatch } var backendRefErr error diff --git a/internal/controller/udproute_controller.go b/internal/controller/udproute_controller.go index f5eebbc5..c2ba1ad1 100644 --- a/internal/controller/udproute_controller.go +++ b/internal/controller/udproute_controller.go @@ -300,6 +300,7 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } else if gateway.Listener != nil { tctx.Listeners = appendListeners(tctx.Listeners, *gateway.Listener) } + tctx.HasExplicitListenerMatch = tctx.HasExplicitListenerMatch || gateway.ExplicitListenerMatch } var backendRefErr error diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 57d04398..5e3231d8 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -326,6 +326,16 @@ func SetRouteParentRef(routeParentStatus *gatewayv1.RouteParentStatus, gatewayNa routeParentStatus.ControllerName = gatewayv1.GatewayController(config.ControllerConfig.ControllerName) } +// parentRefTargetsListenerExplicitly reports whether a parentRef names a +// specific listener, via a non-empty sectionName or an explicit port. It is only +// meaningful for a parentRef that already matched a listener on its Gateway. +func parentRefTargetsListenerExplicitly(parentRef gatewayv1.ParentReference) bool { + if parentRef.SectionName != nil && *parentRef.SectionName != "" { + return true + } + return parentRef.Port != nil +} + func ParseRouteParentRefs( ctx context.Context, mgrc client.Client, @@ -474,6 +484,10 @@ func ParseRouteParentRefs( ListenerName: listenerName, Listener: &matchedListener, Listeners: matchedListeners, + // The parentRef explicitly targeted a listener only when an + // explicit sectionName or port was given and it resolved to a + // matched listener on this Gateway (we are in the matched branch). + ExplicitListenerMatch: parentRefTargetsListenerExplicitly(parentRef), Conditions: []metav1.Condition{{ Type: string(gatewayv1.RouteConditionAccepted), Status: metav1.ConditionTrue, diff --git a/internal/controller/utils_parentref_test.go b/internal/controller/utils_parentref_test.go index 3c915c58..9a66b49f 100644 --- a/internal/controller/utils_parentref_test.go +++ b/internal/controller/utils_parentref_test.go @@ -107,6 +107,98 @@ func TestParseRouteParentRefs_ReasonOrderIndependent(t *testing.T) { } } +// TestParseRouteParentRefs_ExplicitListenerMatch verifies ExplicitListenerMatch +// is derived per parentRef against its own Gateway. In particular a sectionName +// or port that does not resolve on the referenced Gateway must not be treated as +// an explicit match just because another Gateway, matched through a different +// parentRef, happens to have an identically named or ported listener. +func TestParseRouteParentRefs_ExplicitListenerMatch(t *testing.T) { + scheme := parentRefTestScheme(t) + + // gwA only has a listener named "other"; a sectionName "web" reference to it + // resolves to nothing. + gwA := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw-a"}, + Spec: gatewayv1.GatewaySpec{ + GatewayClassName: "apisix", + Listeners: []gatewayv1.Listener{ + {Name: "other", Port: 80, Protocol: gatewayv1.HTTPProtocolType}, + }, + }, + } + // gwB has a listener named "web" on port 8080 that an implicit reference matches. + gwB := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw-b"}, + Spec: gatewayv1.GatewaySpec{ + GatewayClassName: "apisix", + Listeners: []gatewayv1.Listener{ + {Name: "web", Port: 8080, Protocol: gatewayv1.HTTPProtocolType}, + }, + }, + } + route := &gatewayv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "r"}, + } + cli := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(newParentRefGatewayClass(), gwA, gwB).Build() + + webSection := gatewayv1.SectionName("web") + port8080 := gatewayv1.PortNumber(8080) + + t.Run("invalid explicit ref is not satisfied by another gateway's listener", func(t *testing.T) { + got, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + []gatewayv1.ParentReference{ + // Explicit sectionName "web" on gw-a, which has no such listener. + {Name: "gw-a", SectionName: &webSection}, + // Implicit ref to gw-b, which does have a "web" listener. + {Name: "gw-b"}, + }) + require.NoError(t, err) + require.Len(t, got, 2) + + byGateway := map[string]RouteParentRefContext{} + for _, ctx := range got { + byGateway[ctx.Gateway.Name] = ctx + } + // gw-a's explicit ref matched no listener, so it is neither Accepted nor + // an explicit match. + assert.Nil(t, byGateway["gw-a"].Listener) + assert.False(t, byGateway["gw-a"].ExplicitListenerMatch) + // gw-b matched implicitly; the stray "web" sectionName on gw-a must not + // promote it to an explicit match. + assert.NotNil(t, byGateway["gw-b"].Listener) + assert.False(t, byGateway["gw-b"].ExplicitListenerMatch, + "an implicit match must not be marked explicit by another gateway's ref") + }) + + t.Run("explicit sectionName on the owning gateway is an explicit match", func(t *testing.T) { + got, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + []gatewayv1.ParentReference{{Name: "gw-b", SectionName: &webSection}}) + require.NoError(t, err) + require.Len(t, got, 1) + assert.NotNil(t, got[0].Listener) + assert.True(t, got[0].ExplicitListenerMatch) + }) + + t.Run("explicit port on the owning gateway is an explicit match", func(t *testing.T) { + got, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + []gatewayv1.ParentReference{{Name: "gw-b", Port: &port8080}}) + require.NoError(t, err) + require.Len(t, got, 1) + assert.NotNil(t, got[0].Listener) + assert.True(t, got[0].ExplicitListenerMatch) + }) + + t.Run("implicit ref is not an explicit match", func(t *testing.T) { + got, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + []gatewayv1.ParentReference{{Name: "gw-b"}}) + require.NoError(t, err) + require.Len(t, got, 1) + assert.NotNil(t, got[0].Listener) + assert.False(t, got[0].ExplicitListenerMatch) + }) +} + // TestParseRouteParentRefs_ConflictingTLSModePort verifies a route does not // attach to a listener whose port carries a conflicting tls.mode: such a listener // is not programmable, so the route must not be Accepted. diff --git a/internal/provider/provider.go b/internal/provider/provider.go index f1807654..858c95bc 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -47,6 +47,11 @@ type TranslateContext struct { GatewayTLSConfig []gatewayv1.ListenerTLSConfig Credentials []v1alpha1.Credential Listeners []gatewayv1.Listener + // HasExplicitListenerMatch is true when any accepted parentRef attached to + // its Gateway through an explicit sectionName or port. It gates server_port + // injection (see Translator.shouldInjectServerPortVars) and is computed from + // the matched RouteParentRefContext, preserving each parentRef's Gateway. + HasExplicitListenerMatch bool EndpointSlices map[k8stypes.NamespacedName][]discoveryv1.EndpointSlice Secrets map[k8stypes.NamespacedName]*corev1.Secret