fix(router): fix incorrectly calculated priorities in traditional_compat #10615
Conversation
This file contains 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
hanshuebner
added a commit
to Kong/docs.konghq.com
that referenced
this pull request
Apr 5, 2023
This PR updates the documentation to reflect the change in Kong/kong#10615, which makes traditional_compat mode behave like the traditional router with respect to priorities of routes having multiple paths.
2 tasks
samugi
reviewed
Apr 5, 2023
hanshuebner
force-pushed
the
fix/kag-727-traditional-compat
branch
from
57eb136 to
28e2640
Compare
hbagdi
reviewed
Apr 5, 2023
hanshuebner
force-pushed
the
fix/kag-727-traditional-compat
branch
from
dd6b078 to
a7b6d07
Compare
bungle
reviewed
Apr 11, 2023
Guaris
pushed a commit
to Kong/docs.konghq.com
that referenced
this pull request
Apr 11, 2023
This PR updates the documentation to reflect the change in Kong/kong#10615, which makes traditional_compat mode behave like the traditional router with respect to priorities of routes having multiple paths.
hanshuebner
added a commit
to Kong/docs.konghq.com
that referenced
this pull request
Apr 17, 2023
)" This reverts commit 7149a59. This documentation update will have to wait until Kong/kong#10615 is merged.
acgoldsmith
pushed a commit
to Kong/docs.konghq.com
that referenced
this pull request
Apr 17, 2023
)" (#5442) This reverts commit 7149a59. This documentation update will have to wait until Kong/kong#10615 is merged.
chronolaw
reviewed
Apr 19, 2023
chronolaw
reviewed
Apr 19, 2023
flrgh
reviewed
Apr 27, 2023
flrgh
reviewed
Apr 27, 2023
flrgh
reviewed
Apr 27, 2023
flrgh
reviewed
Apr 27, 2023
flrgh
reviewed
Apr 27, 2023
flrgh
reviewed
Apr 27, 2023
There was a problem hiding this comment.
Nicely done! I left a few small suggestions, and I'd like to know your thoughts about the table copy comment that I raised.
hanshuebner
force-pushed
the
fix/kag-727-traditional-compat
branch
from
1b0140a to
10717c6
Compare
When prefix and regex routes were mixed or prefixes of different lengths were specified in one route, traditional_compat assigned the highest priority of all paths to the route for all paths, causing unexpected routing matches that were incompatible with the traditional router. With this fix, each multi-path route is split into multiple routes with separate priorities. KAG-727
hanshuebner
force-pushed
the
fix/kag-727-traditional-compat
branch
from
faf9e8a to
56cabca
Compare
|
@bungle can I get a final review and approval, please? |
team-gateway-bot
pushed a commit
that referenced
this pull request
May 5, 2023
…pat (#10615) * fix(router): fix incorrectly calculated priorities in traditional_compat When prefix and regex routes were mixed or prefixes of different lengths were specified in one route, traditional_compat assigned the highest priority of all paths to the route for all paths, causing unexpected routing matches that were incompatible with the traditional router. With this fix, each multi-path route is split into multiple routes with separate priorities. KAG-727 * address review comments * use uuidv5 to create stable split route ids * improve naming * refactor for call-site clarity * some loop refactoring (cherry picked from commit 9beff64)
2 tasks
chronolaw
added a commit
that referenced
this pull request
May 6, 2023
This was referenced May 6, 2023
hanshuebner
pushed a commit
that referenced
this pull request
May 6, 2023
* fix a mistake introduced by #10615 * adjust order of PRs * remove a comma
3 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
When prefix and regex routes were mixed or prefixes of different lengths were specified in one route, traditional_compat assigned the highest priority of all paths to the route for all paths, causing unexpected routing matches that were incompatible with the traditional router.
With this fix, each multi-path route is split into multiple routes with separate priorities.
Checklist
Issue reference
KAG-727
KAG-1446