Skip to content

fix(stream/traffic-split): set route_id in stream preread phase#13284

Merged
nic-6443 merged 2 commits intoapache:masterfrom
nic-6443:fix/stream-traffic-split-route-id
Apr 25, 2026
Merged

fix(stream/traffic-split): set route_id in stream preread phase#13284
nic-6443 merged 2 commits intoapache:masterfrom
nic-6443:fix/stream-traffic-split-route-id

Conversation

@nic-6443
Copy link
Copy Markdown
Member

In stream_preread_phase, api_ctx.route_id was never assigned after route matching, so ctx.var.route_id always returned nil in stream context. The HTTP access_phase sets it, but stream was missing it.

This caused match conditions using route_id in plugins like traffic-split to always evaluate as nil and never match.

The fix adds api_ctx.route_id (and route_name) assignment right before plugins run — matching the pattern already established in the HTTP path.

Also adds a test case that creates a stream route with a traffic-split rule matching on route_id, and verifies traffic is correctly directed to the plugin-configured upstream.

Copilot AI review requested due to automatic review settings April 23, 2026 09:22
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Apr 23, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes stream (TCP/UDP) request context so route_id/route_name are populated after stream route matching, enabling plugins (e.g., traffic-split) to evaluate match conditions against route_id in the stream preread phase—consistent with the HTTP request path.

Changes:

  • Assign api_ctx.route_id and api_ctx.route_name in stream_preread_phase() after route match and before running preread plugins.
  • Add a stream plugin test that configures traffic-split with a route_id-based match condition and validates upstream selection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
apisix/init.lua Sets api_ctx.route_id/route_name in stream preread before plugins run so ctx.var.route_id works in stream context.
t/stream-plugin/traffic-split.t Adds regression coverage for traffic-split matching on route_id in stream routes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread t/stream-plugin/traffic-split.t Outdated
Comment thread t/stream-plugin/traffic-split.t Outdated
@nic-6443
Copy link
Copy Markdown
Member Author

Fixed CI: TEST 5 was causing nginx to restart from stream mode to HTTP mode after TEST 4's 10 concurrent stream connections, which triggered nginx: [emerg] unexpected ':' in nginx.conf. Added --- stream_enable to TEST 5 so nginx stays in stream mode during the setup step — the HTTP admin API is still accessible in stream mode.

@nic-6443
Copy link
Copy Markdown
Member Author

Fixed the nginx.conf parse error. The issue was that TEST 5 had both --- config (custom HTTP location for admin setup) and --- stream_enable. The test framework's preprocessors run in LIFO order, so Stream.pm runs last — it sees stream_config is defined (set by APISIX.pm) and appends a location = /t { sock:connect(...) } block after APISIX.pm's already large server config, producing a malformed nginx.conf. Since TEST 5 is just making HTTP admin API calls (create/delete routes), it doesn't actually need the stream server at all — removing --- stream_enable from that test fixes the issue.

@nic-6443
Copy link
Copy Markdown
Member Author

Found the real root cause of the nginx.conf parse error. The --- stream_enable was a red herring.

The actual issue: "vars": [["route_id", "==", "3"]] inside the outer [[{...}]] Lua long bracket string contains ]] (closing the inner JSON array-of-arrays), which prematurely closes the outer level-0 long bracket string. Nginx's content_by_lua_block scanner then exits the Lua block early and starts parsing the remaining JSON as nginx config directives — hitting "upstream": and seeing an unexpected :.

Fixed by using a level-1 long bracket string [=[...]=] for the outer JSON, since ]=] won't appear in the content.

Baoyuantop
Baoyuantop previously approved these changes Apr 24, 2026
In stream_preread_phase, api_ctx.route_id was never assigned after route
matching, so ctx.var.route_id always returned nil in stream context. The
HTTP access_phase sets it, but stream was missing it.

This caused match conditions using route_id in plugins like traffic-split
to always evaluate as nil and never match.

The fix adds api_ctx.route_id (and route_name) assignment right before
plugins run — matching the pattern already established in the HTTP path.

Also adds a test case that creates a stream route with a traffic-split
rule matching on route_id, and verifies traffic is correctly directed to
the plugin-configured upstream.
@nic-6443 nic-6443 force-pushed the fix/stream-traffic-split-route-id branch from 8da1857 to a22fe3d Compare April 24, 2026 06:41
@nic-6443 nic-6443 merged commit 08bf624 into apache:master Apr 25, 2026
19 checks passed
@nic-6443 nic-6443 deleted the fix/stream-traffic-split-route-id branch April 25, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants