Skip to content

Commit

Permalink
fix(router/atc): reset rebuilding flag if route has no id (#12654)
Browse files Browse the repository at this point in the history
The variable rebuilding is a flag to prevent duplicate router building action, if any error occurs it should be set to false, or else the router will never be rebuilt again.

KAG-3857
  • Loading branch information
chronolaw authored Mar 14, 2024
1 parent 15a3797 commit 35c292c
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/kong/fix-router-rebuing-flag.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
message: |
Fixed an issue where router may not work correctly
when the routes configuration changed.
type: bugfix
scope: Core
1 change: 1 addition & 0 deletions kong/router/atc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ local function new_from_previous(routes, get_exp_and_priority, old_router)
local route_id = route.id

if not route_id then
old_router.rebuilding = false
return nil, "could not categorize route"
end

Expand Down
40 changes: 40 additions & 0 deletions spec/01-unit/08-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2131,6 +2131,46 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
assert.spy(add_matcher).was_called(2)
assert.spy(remove_matcher).was_called(2)
end)

it("works well if route has no proper id", function()
local wrong_use_case = {
{
service = service,
route = {
id = nil, -- no id here
paths = { "/foo", },
updated_at = 100,
},
},
}

local nrouter = new_router(wrong_use_case, router)
assert.is_nil(nrouter)
assert.falsy(router.rebuilding)

local use_case = {
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
paths = { "/foo1", },
updated_at = 100,
},
},
}

local nrouter = assert(new_router(use_case, router))

assert.equal(nrouter, router)
assert.falsy(router.rebuilding)

local match_t = nrouter:select("GET", "/foo1")
assert.truthy(match_t)
assert.same(use_case[1].route, match_t.route)

match_t = nrouter:select("GET", "/bar")
assert.falsy(match_t)
end)
end)

describe("check empty route fields", function()
Expand Down

1 comment on commit 35c292c

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Bazel Build

Docker image available kong/kong:35c292c676bb2ce20c00e7582e065de8d01e07c3
Artifacts available https://github.com/Kong/kong/actions/runs/8274562980

Please sign in to comment.