Skip to content

Commit

Permalink
refactor(balancer): get headers as hash identifier by ngx.var (#13063)
Browse files Browse the repository at this point in the history
Before this PR we use `ngx.req.get_headers()` to get all possible header values as identifier,
but since nginx 1.23.0 or OpenResty  1.25.3.1 `ngx.var` can get a combined value for all header values 
with identical name (joined by comma), so I think that we could simplify these code. 

KAG-4572
  • Loading branch information
chronolaw authored May 30, 2024
1 parent 2022ac7 commit 3e05a9b
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
7 changes: 3 additions & 4 deletions kong/runloop/balancer/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,9 @@ local function get_value_to_hash(upstream, ctx)
identifier = var.remote_addr

elseif hash_on == "header" then
identifier = ngx.req.get_headers()[upstream[header_field_name]]
if type(identifier) == "table" then
identifier = table_concat(identifier)
end
-- since nginx 1.23.0/openresty 1.25.3.1
-- ngx.var will automatically combine all header values with identical name
identifier = var["http_" .. upstream[header_field_name]]

elseif hash_on == "cookie" then
identifier = var["cookie_" .. upstream.hash_on_cookie]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,45 @@ for _, strategy in helpers.each_strategy() do
["hashme"] = "just a value",
})
assert.are.equal(requests, oks)
assert.logfile().has.line("trying to get peer with value to hash: \\[just a value\\]")

-- collect server results; hitcount
-- one should get all the hits, the other 0
local count1 = server1:shutdown()
local count2 = server2:shutdown()

-- verify
assert(count1.total == 0 or count1.total == requests, "counts should either get 0 or ALL hits")
assert(count2.total == 0 or count2.total == requests, "counts should either get 0 or ALL hits")
assert(count1.total + count2.total == requests)
end)

it("hashing on multiple headers", function()
local requests = bu.SLOTS * 2 -- go round the balancer twice

bu.begin_testcase_setup(strategy, bp)
local upstream_name, upstream_id = bu.add_upstream(bp, {
hash_on = "header",
hash_on_header = "hashme",
})
local port1 = bu.add_target(bp, upstream_id, localhost)
local port2 = bu.add_target(bp, upstream_id, localhost)
local api_host = bu.add_api(bp, upstream_name)
bu.end_testcase_setup(strategy, bp)

-- setup target servers
local server1 = https_server.new(port1, localhost)
local server2 = https_server.new(port2, localhost)
server1:start()
server2:start()

-- Go hit them with our test requests
local oks = bu.client_requests(requests, {
["Host"] = api_host,
["hashme"] = { "1st value", "2nd value", },
})
assert.are.equal(requests, oks)
assert.logfile().has.line("trying to get peer with value to hash: \\[1st value, 2nd value\\]")

-- collect server results; hitcount
-- one should get all the hits, the other 0
Expand Down Expand Up @@ -95,6 +134,7 @@ for _, strategy in helpers.each_strategy() do
["nothashme"] = "just a value",
})
assert.are.equal(requests, oks)
assert.logfile().has.line("trying to get peer with value to hash: \\[\\]")

-- collect server results; hitcount
-- one should get all the hits, the other 0
Expand Down

1 comment on commit 3e05a9b

@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:3e05a9b7ef02dab9dbd6755e6808645e99852c80
Artifacts available https://github.com/Kong/kong/actions/runs/9295608786

Please sign in to comment.