fix: honor TCPRoute/UDPRoute sectionName and listener port for StreamRoute matching - #445
Open
AlinsRan wants to merge 5 commits into
Open
fix: honor TCPRoute/UDPRoute sectionName and listener port for StreamRoute matching#445AlinsRan wants to merge 5 commits into
AlinsRan wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughTCP and UDP reconciliation now passes matched Gateway listeners into translation. Shared L4 route construction injects selected listener ports into ChangesL4 listener port isolation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TCPRouteReconciler
participant UDPRouteReconciler
participant Translator
participant buildL4StreamRoutes
participant StreamRoutes
TCPRouteReconciler->>Translator: pass matched Gateway listeners
UDPRouteReconciler->>Translator: pass matched Gateway listeners
Translator->>buildL4StreamRoutes: resolve parent references and listener ports
buildL4StreamRoutes->>StreamRoutes: create port-specific or portless routes
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-30T07:54:57Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
contact:
- https://github.com/apache/apisix-ingress-controller/issues
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
- HTTPRouteInvalidBackendRefUnknownKind
- HTTPRouteInvalidCrossNamespaceBackendRef
- HTTPRouteInvalidNonExistentBackendRef
- HTTPRouteListenerHostnameMatching
- HTTPRouteMultipleGateways
- HTTPRouteNoBackendRefs
statistics:
Failed: 0
Passed: 30
Skipped: 7
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 12
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- BackendTLSPolicy
- BackendTLSPolicySANValidation
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRoute303RedirectStatusCode
- HTTPRoute307RedirectStatusCode
- HTTPRoute308RedirectStatusCode
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteCORS
- HTTPRouteNamedRouteRule
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
- HTTPRouteRetry
- HTTPRouteRetryBackendTimeout
- HTTPRouteRetryConnectionError
- ListenerSet
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 7 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- GRPCRouteListenerHostnameMatching
statistics:
Failed: 0
Passed: 14
Skipped: 1
extended:
result: success
statistics:
Failed: 0
Passed: 1
Skipped: 0
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
unsupportedFeatures:
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- ListenerSet
name: GATEWAY-GRPC
summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
result: partial
skippedTests:
- TLSRouteHostnameIntersection
- TLSRouteInvalidBackendRefNonexistent
- TLSRouteInvalidBackendRefUnknownKind
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 16
Skipped: 4
extended:
result: partial
skippedTests:
- TLSRouteTerminateSimpleSameNamespace
statistics:
Failed: 0
Passed: 3
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- TLSRouteModeTerminate
unsupportedFeatures:
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- ListenerSet
- TLSRouteModeMixed
name: GATEWAY-TLS
summary: Core tests partially succeeded with 4 test skips. Extended tests partially
succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue |
Contributor
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-30T07:55:11Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
contact:
- https://github.com/apache/apisix-ingress-controller/issues
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
- HTTPRouteInvalidBackendRefUnknownKind
- HTTPRouteInvalidCrossNamespaceBackendRef
- HTTPRouteInvalidNonExistentBackendRef
- HTTPRouteListenerHostnameMatching
- HTTPRouteMultipleGateways
- HTTPRouteNoBackendRefs
statistics:
Failed: 0
Passed: 30
Skipped: 7
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 12
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- BackendTLSPolicy
- BackendTLSPolicySANValidation
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRoute303RedirectStatusCode
- HTTPRoute307RedirectStatusCode
- HTTPRoute308RedirectStatusCode
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteCORS
- HTTPRouteNamedRouteRule
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
- HTTPRouteRetry
- HTTPRouteRetryBackendTimeout
- HTTPRouteRetryConnectionError
- ListenerSet
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 7 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- GRPCRouteListenerHostnameMatching
statistics:
Failed: 0
Passed: 14
Skipped: 1
extended:
result: success
statistics:
Failed: 0
Passed: 1
Skipped: 0
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
unsupportedFeatures:
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- ListenerSet
name: GATEWAY-GRPC
summary: Core tests partially succeeded with 1 test skips. Extended tests succeeded.
- core:
result: partial
skippedTests:
- TLSRouteHostnameIntersection
- TLSRouteInvalidBackendRefNonexistent
- TLSRouteInvalidBackendRefUnknownKind
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 16
Skipped: 4
extended:
result: partial
skippedTests:
- TLSRouteTerminateSimpleSameNamespace
statistics:
Failed: 0
Passed: 3
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- TLSRouteModeTerminate
unsupportedFeatures:
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- ListenerSet
- TLSRouteModeMixed
name: GATEWAY-TLS
summary: Core tests partially succeeded with 4 test skips. Extended tests partially
succeeded with 1 test skips.
succeededProvisionalTests:
- GatewayOptionalAddressValue |
Contributor
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-07-30T08:13:06Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.6.0
implementation:
contact:
- https://github.com/apache/apisix-ingress-controller/issues
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GatewayModifyListeners
- HTTPRouteMultipleGateways
- HTTPRouteNoBackendRefs
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 3
Passed: 33
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 12
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- BackendTLSPolicy
- BackendTLSPolicySANValidation
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRoute303RedirectStatusCode
- HTTPRoute307RedirectStatusCode
- HTTPRoute308RedirectStatusCode
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteCORS
- HTTPRouteNamedRouteRule
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
- HTTPRouteRetry
- HTTPRouteRetryBackendTimeout
- HTTPRouteRetryConnectionError
- ListenerSet
name: GATEWAY-HTTP
summary: Core tests failed with 3 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
failedTests:
- GatewayModifyListeners
result: failure
statistics:
Failed: 1
Passed: 14
Skipped: 0
extended:
result: success
statistics:
Failed: 0
Passed: 1
Skipped: 0
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
unsupportedFeatures:
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- ListenerSet
name: GATEWAY-GRPC
summary: Core tests failed with 1 test failures. Extended tests succeeded.
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteHostnameIntersection
- TLSRouteInvalidBackendRefNonexistent
- TLSRouteInvalidBackendRefUnknownKind
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 5
Passed: 15
Skipped: 0
extended:
failedTests:
- TLSRouteTerminateSimpleSameNamespace
result: failure
statistics:
Failed: 1
Passed: 3
Skipped: 0
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- TLSRouteModeTerminate
unsupportedFeatures:
- GatewayBackendClientCertificate
- GatewayFrontendClientCertificateValidation
- GatewayFrontendClientCertificateValidationInsecureFallback
- GatewayHTTPListenerIsolation
- GatewayHTTPSListenerDetectMisdirectedRequests
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- ListenerSet
- TLSRouteModeMixed
name: GATEWAY-TLS
summary: Core tests failed with 5 test failures. Extended tests failed with 1 test
failures.
succeededProvisionalTests:
- GatewayOptionalAddressValue |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/controller/tcproute_controller.go (1)
291-303: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winZero-value listener leak into
tctx.Listenersfor rejected parentRefs, in both TCP and UDP reconcilers. Root cause:RouteParentRefContext.Listener(set ininternal/controller/utils.go'sParseRouteParentRefs) is nevernil— it points to a zero-valuegatewayv1.Listener{}even when the parentRef didn't match any listener — so the newelse if gateway.Listener != nilfallback added in both reconcilers appends a bogusListener{Port: 0}whenever a route has a mix of accepted and rejected parentRefs, poisoning the port set the translator uses forserver_portisolation.
internal/controller/tcproute_controller.go#L291-L303: guard the fallback, e.g.else if gateway.Listener != nil && gateway.Listener.Port != 0.internal/controller/udproute_controller.go#L291-L303: apply the same guard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/controller/tcproute_controller.go` around lines 291 - 303, Prevent zero-value listeners from being appended in the gateway listener population logic: in internal/controller/tcproute_controller.go lines 291-303 and internal/controller/udproute_controller.go lines 291-303, update the fallback within the ProcessGatewayProxy loop to require gateway.Listener.Port != 0 in addition to gateway.Listener != nil. Keep the existing gateway.Listeners handling unchanged.internal/controller/utils.go (1)
373-442: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick wininternal/controller/utils.go: keep collecting all matching listeners
ParseRouteParentRefsis meant to fillRouteParentRefContext.Listenerswith every compatible listener for a parentRef, but the unconditionalbreakstops at the first match. For TCP/UDP routes that leavestctx.Listenerswith one port, so multi-port fan-out never kicks in; keepListener/ListenerNameon the first hit, but continue scanning and appending toListeners.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/controller/utils.go` around lines 373 - 442, Remove the unconditional break after a successful match in ParseRouteParentRefs so the loop continues scanning all listeners compatible with the parentRef. Preserve ListenerName and Listener from the first match, while appending every subsequent compatible listener to matchedListeners for RouteParentRefContext.Listeners and multi-port fan-out.Source: Coding guidelines
🧹 Nitpick comments (2)
internal/adc/translator/tcproute.go (1)
46-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded
"Gateway"kind literal.Duplicates the
Gatewaykind check already expressed viaKindGatewayininternal/controller/utils.go. Cross-package, so may not be trivially shareable, but worth a local constant to avoid silent divergence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/adc/translator/tcproute.go` around lines 46 - 52, Replace the hardcoded "Gateway" kind comparison in hasExplicitListenerTarget with a local named constant representing the Gateway kind, keeping the existing non-Gateway filtering behavior unchanged and aligned with the KindGateway value used elsewhere.internal/adc/translator/l4route_serverport_test.go (1)
62-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood translator-level coverage, but doesn't exercise the real
ParseRouteParentRefs→tctx.Listenerswiring.These tests inject
tctx.Listenersdirectly, so the "multiple listener ports fan out even without explicit targeting" case never runs throughParseRouteParentRefs, where the loop'sbreak(flagged separately ininternal/controller/utils.go) actually prevents that scenario from ever populating more than one listener in production. Consider adding aParseRouteParentRefs-level test for an untargeted parentRef matching multiple compatible listeners on one Gateway to catch this class of regression.Also applies to: 146-216
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/adc/translator/l4route_serverport_test.go` around lines 62 - 144, Add coverage for the ParseRouteParentRefs path rather than only assigning tctx.Listeners directly in TestTranslateTCPRouteServerPort. Create an untargeted parentRef matching multiple compatible listeners on the same Gateway, invoke ParseRouteParentRefs, and assert that all matching listeners populate the translation context so TCP route translation fans out across their ports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/controller/tcproute_controller.go`:
- Around line 291-303: Prevent zero-value listeners from being appended in the
gateway listener population logic: in internal/controller/tcproute_controller.go
lines 291-303 and internal/controller/udproute_controller.go lines 291-303,
update the fallback within the ProcessGatewayProxy loop to require
gateway.Listener.Port != 0 in addition to gateway.Listener != nil. Keep the
existing gateway.Listeners handling unchanged.
In `@internal/controller/utils.go`:
- Around line 373-442: Remove the unconditional break after a successful match
in ParseRouteParentRefs so the loop continues scanning all listeners compatible
with the parentRef. Preserve ListenerName and Listener from the first match,
while appending every subsequent compatible listener to matchedListeners for
RouteParentRefContext.Listeners and multi-port fan-out.
---
Nitpick comments:
In `@internal/adc/translator/l4route_serverport_test.go`:
- Around line 62-144: Add coverage for the ParseRouteParentRefs path rather than
only assigning tctx.Listeners directly in TestTranslateTCPRouteServerPort.
Create an untargeted parentRef matching multiple compatible listeners on the
same Gateway, invoke ParseRouteParentRefs, and assert that all matching
listeners populate the translation context so TCP route translation fans out
across their ports.
In `@internal/adc/translator/tcproute.go`:
- Around line 46-52: Replace the hardcoded "Gateway" kind comparison in
hasExplicitListenerTarget with a local named constant representing the Gateway
kind, keeping the existing non-Gateway filtering behavior unchanged and aligned
with the KindGateway value used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81ff22b5-f4d5-4964-b6ac-317ae84db223
📒 Files selected for processing (9)
internal/adc/translator/l4route_serverport_test.gointernal/adc/translator/tcproute.gointernal/adc/translator/udproute.gointernal/controller/context.gointernal/controller/tcproute_controller.gointernal/controller/udproute_controller.gointernal/controller/utils.gotest/e2e/gatewayapi/tcproute.gotest/e2e/gatewayapi/udproute.go
…Route 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).
AlinsRan
force-pushed
the
ee-2802-l4-server-port
branch
from
July 29, 2026 07:12
666ee3c to
3f520d1
Compare
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.
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.
shreemaan-abhishek
approved these changes
Jul 30, 2026
jarvis9443
reviewed
Jul 30, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/adc/translator/l4route_serverport_test.go (1)
149-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror duplicate-port coverage for UDP.
The TCP cases cover duplicate listener ports and uniqueness of generated IDs and names, but the UDP cases do not. Add an equivalent UDP duplicate-port scenario so the UDP translation path cannot regress independently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/adc/translator/l4route_serverport_test.go` around lines 149 - 221, Add a UDP test case to TestTranslateUDPRouteServerPort with multiple listeners sharing the same port, mirroring the TCP duplicate-port coverage. Assert that translation produces the expected StreamRoutes and that both generated route IDs and names remain unique.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/adc/translator/l4route_serverport_test.go`:
- Around line 149-221: Add a UDP test case to TestTranslateUDPRouteServerPort
with multiple listeners sharing the same port, mirroring the TCP duplicate-port
coverage. Assert that translation produces the expected StreamRoutes and that
both generated route IDs and names remain unique.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88a23e41-05ee-45f6-9bf9-1e514911eaa2
📒 Files selected for processing (4)
internal/adc/translator/l4route_serverport_test.gointernal/adc/translator/tcproute.gotest/e2e/gatewayapi/tcproute.gotest/e2e/gatewayapi/udproute.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/adc/translator/tcproute.go
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.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Relevant declarations and call sites =='
ast-grep outline internal/controller/utils.go --items all --match 'ParseRouteParentRefs' --view expanded
ast-grep outline internal/controller/tcproute_controller.go --items all --match 'Reconcile' --view expanded
ast-grep outline internal/controller/udproute_controller.go --items all --match 'Reconcile' --view expanded
ast-grep outline internal/adc/translator/tcproute.go --items all --match 'buildL4StreamRoutes|shouldInject|explicit|parentRef' --view expanded || true
echo '== Parser and reconciler context =='
rg -n -C 12 'func ParseRouteParentRefs|ParseRouteParentRefs\(|gateway\.Listeners|gateway\.Listener' \
internal/controller/utils.go internal/controller/tcproute_controller.go internal/controller/udproute_controller.go
echo '== L4 explicit-target / port-injection context =='
rg -n -i -C 10 'explicit|sectionname|routeparentrefs|serverport|server_port|matched.*listener' \
internal/adc/translator/tcproute.go internal/adc/translator/udproute.go
echo '== Referenced commit =='
git show --stat --oneline 512f826e
git show --format=fuller --find-renames --find-copies 512f826e -- \
internal/adc/translator/tcproute.go \
internal/adc/translator/udproute.go \
internal/adc/translator/l4route_serverport_test.goLength of output: 24164 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== ParseRouteParentRefs implementation =='
sed -n '329,510p' internal/controller/utils.go
echo '== Explicit listener-target predicate =='
ast-grep outline internal/adc/translator/translator.go --items all --match 'shouldInjectServerPortVars|hasExplicitListenerTarget' --view expanded
rg -n -C 14 'func \(t \*Translator\) shouldInjectServerPortVars|func hasExplicitListenerTarget|shouldInjectServerPortVars\(' \
internal/adc/translator/translator.go
echo '== Tests covering parser selection / rejection and port injection =='
rg -n -i -C 5 'ParseRouteParentRefs|sectionname|sectionName|Listener: nil|listener.*fan|serverport|server_port|explicit.*target' \
internal/controller --glob '*_test.go' \
internal/adc/translator --glob '*_test.go'Length of output: 60159
These findings are no longer applicable. ✏️ Learnings added
You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/adc/translator/translator.go`:
- Around line 67-80: The explicit-target matching in the translator must
preserve parent Gateway provenance instead of matching sectionName and port
across the union of listeners. Update the parsing and resolution flow around the
parentRef/listeners logic to retain each parentRef’s namespace, name, group, and
associated Gateway/listener, then require all supplied target fields to match
the same Gateway and listener; add coverage for name/port collisions across
Gateways.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 784f5c99-7567-4970-bf89-06499f390cb2
📒 Files selected for processing (5)
internal/adc/translator/annotations_test.gointernal/adc/translator/grpcroute.gointernal/adc/translator/httproute.gointernal/adc/translator/tcproute.gointernal/adc/translator/translator.go
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.
AlinsRan
force-pushed
the
ee-2802-l4-server-port
branch
from
July 30, 2026 07:39
0cc520c to
f008153
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change:
What this PR does / why we need it:
Sync from apache/apisix-ingress-controller#2818 (fixes apache/apisix-ingress-controller#2802).
TranslateTCPRoute/TranslateUDPRouteemitted a singleStreamRoutewith no match criteria (the old// TODO: ... server_port). In APISIX stream mode a StreamRoute withoutserver_portmatches every connection on any stream listener, so all L4 routes across all Gateways collided onto one backend —parentRefs.sectionName/ per-listener port routing were ignored.This makes TCPRoute/UDPRoute participate in the existing
listener_port_match_modemechanism (added for HTTPRoute/GRPCRoute in #419):tcproute_controller.go,udproute_controller.go): populatetctx.Listenersfrom the listenersParseRouteParentRefsmatched for each parentRef (honoringsectionName, port, protocol,allowedRoutes), mirroring the HTTPRoute/GRPCRoute path.tcproute.go,udproute.go):buildL4StreamRoutessetsStreamRoute.server_portfrom the matched listener port(s), one StreamRoute per port with a port-qualified name/ID, gated by the sameshouldInjectServerPortVars.Rebased on latest master
An earlier revision of this PR carried a self-contained gating helper because EE master did not yet have
listener_port_match_mode. Master has since merged port-based routing (#419), so this PR is rebuilt on master and now reuseslistener_port_match_modedirectly — no duplicated gating, no new config. Diff is line-for-line equivalent to the upstream OSS PR #2818.Backward compatibility
listener_port_match_modedefaults tooff, so L4 behavior is unchanged by default (single portless StreamRoute) — no upgrade risk. Isolation is opt-in: setlistener_port_match_mode: auto/explicitand make the Gateway listener port equal APISIX's physicalstream_proxyport (9100/9200), same contract as HTTPRoute.Limitation:
server_portisolates across distinct listener ports only; two L4 routes on the same physical port can't be disambiguated (no SNI/host at bare TCP/UDP).Pre-submission checklist:
l4route_serverport_test.go, exercised in auto mode)off→ unchanged L4 behavior)Summary by CodeRabbit
server_portbased on the matched Gateway listener (including auto port matching and explicit listener targeting).server_portinjection, fan-out, deduplication, and fallback.