Skip to content

Fixed 58441, RegularExpression routes incorrectly ranked below PathPr…#58641

Closed
vibhordubey333 wants to merge 3 commits intoistio:masterfrom
vibhordubey333:Refactor/58441
Closed

Fixed 58441, RegularExpression routes incorrectly ranked below PathPr…#58641
vibhordubey333 wants to merge 3 commits intoistio:masterfrom
vibhordubey333:Refactor/58441

Conversation

@vibhordubey333
Copy link

@vibhordubey333 vibhordubey333 commented Dec 25, 2025

Please provide a description of this PR:

Issue: #58441

Bug Fix: RegularExpression routes incorrectly ranked below PathPrefix routes

Problem

When using Kubernetes Gateway API HTTPRoute resources with both RegularExpression and PathPrefix path matches, requests are incorrectly routed to the less specific PathPrefix route instead of the more specific RegularExpression route.

Example:

  • Route A: RegularExpression: "/api/test/[0-9a-z-]+" → Service A
  • Route B: PathPrefix: "/api" → Service B

Request to /api/test/check should match Route A, but currently matches Route B.

Root Cause

The getURIRank() function incorrectly assigned ranks:

  • Before: Exact (3) > Prefix (2) > Regex (1)
  • After: Exact (3) > Regex (2) > Prefix (1)

This caused PathPrefix routes to be sorted before RegularExpression routes, so Envoy would always match the prefix route first, preventing the regex route from being evaluated.

Solution

Updated route ranking to prioritize RegularExpression over PathPrefix, ensuring more specific routes are evaluated first.

Changes

  • Fixed getURIRank() to assign correct ranks
  • Updated test case to reflect correct ordering
  • Added test case for the reported scenario

Testing

  • Updated TestSortHTTPRoutes unit test
  • Added new test case verifying RegularExpression routes are matched before PathPrefix routes
  • All tests pass

Impact

Breaking Change: Yes - This fixes incorrect behavior and aligns with expected Gateway API semantics.


Investigation Steps:

  1. Observed incorrect routing behavior
  2. Used istioctl proxy-config routes to inspect Envoy config
  3. Found PathPrefix routes appearing before RegularExpression routes
  4. Traced issue to getURIRank() function

To help us figure out who should review this PR, please put an X in all the areas that this PR affects.

  • Ambient
  • Configuration Infrastructure
  • Docs
  • Dual Stack
  • Installation
  • Networking
  • Performance and Scalability
  • Extensions and Telemetry
  • Security
  • Test and Release
  • [ X] User Experience
  • Developer Infrastructure
  • Upgrade
  • Multi Cluster
  • Virtual Machine
  • Control Plane Revisions

@istio-policy-bot
Copy link

😊 Welcome @vibhordubey333! This is either your first contribution to the Istio istio repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 25, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@istio-testing istio-testing added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test labels Dec 25, 2025
@istio-testing
Copy link
Collaborator

Hi @vibhordubey333. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@zirain
Copy link
Member

zirain commented Dec 25, 2025

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Dec 25, 2025
@istio-testing istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 25, 2025
@vibhordubey333
Copy link
Author

/retest

@howardjohn
Copy link
Member

Breaking Change: No - This fixes incorrect behavior and aligns with expected Gateway API semantics.

This is not really true. The Gateway API semantics are implementation specific, meaning we can order it however we want. And changing it is definitely a breaking change.

@Stevenjin8
Copy link
Contributor

The main argument for regex having higher priority is that it allows you to make 'prefix: /' be a catch all path. That said, we can achieve the same with 'regex: /.*'. So the most we gain with this change is matching envoy gateway's behavior. And maybe having 'prefix: /' be a catch all path is slightly more intuitive.

@istio-policy-bot istio-policy-bot added the lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while label Jan 29, 2026
@vibhordubey333
Copy link
Author

The main argument for regex having higher priority is that it allows you to make 'prefix: /' be a catch all path. That said, we can achieve the same with 'regex: /.*'. So the most we gain with this change is matching envoy gateway's behavior. And maybe having 'prefix: /' be a catch all path is slightly more intuitive.

Will incorporate the review comment.

@istio-policy-bot
Copy link

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2025-12-29. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.

@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Feb 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/user experience lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. lifecycle/stale Indicates a PR or issue hasn't been manipulated by an Istio team member for a while ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants