diff --git a/internal/adc/translator/annotations_test.go b/internal/adc/translator/annotations_test.go index 7b4d1ec4..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,135 +455,77 @@ func TestAddServerPortVars(t *testing.T) { } func TestShouldInjectServerPortVars(t *testing.T) { - sectionName := gatewayv1.SectionName("http-main") - port := gatewayv1.PortNumber(9080) - + // 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{} - 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: {}, - }, + name: "auto mode: single port with explicit target", + mode: config.ListenerPortMatchModeAuto, + explicit: true, + ports: map[int32]struct{}{9080: {}}, expected: true, }, { - name: "multiple ports without sectionName", - mode: config.ListenerPortMatchModeAuto, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - 9081: {}, - }, + 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 multiple ports and no explicit target", - mode: config.ListenerPortMatchModeExplicit, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw"}, - }, - ports: map[int32]struct{}{ - 9080: {}, - 9081: {}, - }, + name: "explicit mode: multiple ports without explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: false, + 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: {}, - }, + name: "explicit mode: single port with explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: true, + ports: map[int32]struct{}{9080: {}}, 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: single port without explicit target", + mode: config.ListenerPortMatchModeExplicit, + explicit: false, + ports: map[int32]struct{}{9080: {}}, expected: false, }, { - name: "off mode ignores explicit target", - mode: config.ListenerPortMatchModeOff, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", SectionName: §ionName}, - }, - ports: map[int32]struct{}{ - 9080: {}, - 9081: {}, - }, + name: "off mode: ignores explicit target with multiple ports", + mode: config.ListenerPortMatchModeOff, + explicit: true, + ports: map[int32]struct{}{9080: {}, 9081: {}}, expected: false, }, { - name: "off mode ignores explicit parentRef.port target", - mode: config.ListenerPortMatchModeOff, - parentRefs: []gatewayv1.ParentReference{ - {Name: "gw", Port: &port}, - }, - 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: {}, - }, - 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, }, } @@ -593,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)) + 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 6b231e2d..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) { + 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 eb48f6fe..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) { + 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 new file mode 100644 index 00000000..778d2f87 --- /dev/null +++ b/internal/adc/translator/l4route_serverport_test.go @@ -0,0 +1,211 @@ +// 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" + 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: port, + } +} + +func udpListener(name string, port int32) gatewayv1.Listener { + return gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Protocol: gatewayv1.UDPProtocolType, + Port: port, + } +} + +func TestTranslateTCPRouteServerPort(t *testing.T) { + tests := []struct { + name string + // listeners the controller would have matched for this route's parentRefs + listeners []gatewayv1.Listener + // 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)}, + explicit: true, + 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)}, + explicit: true, + 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, + explicit: true, + 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.HasExplicitListenerMatch = tt.explicit + + 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 + explicit bool + wantPorts []int32 + wantNoMatch bool + }{ + { + 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", + 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, + explicit: true, + 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.HasExplicitListenerMatch = tt.explicit + + 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..c7a3192d 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[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.HasExplicitListenerMatch, 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/translator.go b/internal/adc/translator/translator.go index d4b06ade..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,31 +49,20 @@ func NewTranslator(log logr.Logger, mode config.ListenerPortMatchMode) *Translat } } -func hasExplicitListenerTarget(parentRefs []gatewayv1.ParentReference) 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 != "" { - return true - } - if parentRef.Port != nil { - return true - } - } - - return false -} - -func (t *Translator) shouldInjectServerPortVars(parentRefs []gatewayv1.ParentReference, ports map[int32]struct{}) 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) - switch t.ListenerPortMatchMode { case config.ListenerPortMatchModeExplicit: return explicit @@ -82,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/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/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 a2360157..b623d715 100644 --- a/internal/controller/tcproute_controller.go +++ b/internal/controller/tcproute_controller.go @@ -293,6 +293,14 @@ 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) + } + tctx.HasExplicitListenerMatch = tctx.HasExplicitListenerMatch || gateway.ExplicitListenerMatch } var backendRefErr error diff --git a/internal/controller/udproute_controller.go b/internal/controller/udproute_controller.go index 17fb5f6a..c2ba1ad1 100644 --- a/internal/controller/udproute_controller.go +++ b/internal/controller/udproute_controller.go @@ -293,6 +293,14 @@ 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) + } + 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 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