Skip to content

Commit

Permalink
fix(response-ratelimiting): fix missing usage headers for upstream (#…
Browse files Browse the repository at this point in the history 
…13696)

`response-ratelimiting` plugin should send usage headers to upstream server.
But in Kong 3.8, there are no usage headers such as `X-RateLimit-Remaining-Videos: 10` for upstream requests.

In this change, it coming back usage headers for upstream.

KAG-5447

---------

Co-authored-by: Guilherme Salazar <[email protected]>
Co-authored-by: BrianChen <[email protected]>
  • Loading branch information
@t-yuki @gszr @ProBrian
3 people authored Feb 6, 2025
1 parent 55e4ff5 commit 2582e76
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 45 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/fix-response-ratelimiting-upstream-headers.yml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**response-ratelimiting**: fixed an issue in response rate limiting plugin where usage headers ought to be sent to upstream are lost."
type: bugfix
scope: Plugin
10 changes: 1 addition & 9 deletions kong/plugins/response-ratelimiting/access.lua
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
local policies = require "kong.plugins.response-ratelimiting.policies"
local timestamp = require "kong.tools.timestamp"
local pdk_private_rl = require "kong.pdk.private.rate_limiting"


local kong = kong
Expand All @@ -10,10 +9,6 @@ local error = error
local tostring = tostring


local pdk_rl_store_response_header = pdk_private_rl.store_response_header
local pdk_rl_apply_response_headers = pdk_private_rl.apply_response_headers


local EMPTY = require("kong.tools.table").EMPTY
local HTTP_TOO_MANY_REQUESTS = 429
local RATELIMIT_REMAINING = "X-RateLimit-Remaining"
Expand Down Expand Up @@ -89,7 +84,6 @@ function _M.execute(conf)
end

-- Append usage headers to the upstream request. Also checks "block_on_first_violation".
local ngx_ctx = ngx.ctx
for k in pairs(conf.limits) do
local remaining
for _, lv in pairs(usage[k]) do
Expand All @@ -103,11 +97,9 @@ function _M.execute(conf)
end
end

pdk_rl_store_response_header(ngx_ctx, RATELIMIT_REMAINING .. "-" .. k, remaining)
kong.service.request.set_header(RATELIMIT_REMAINING .. "-" .. k, remaining)
end

pdk_rl_apply_response_headers(ngx_ctx)

kong.ctx.plugin.usage = usage -- For later use
end

Expand Down
81 changes: 45 additions & 36 deletions spec/03-plugins/24-response-rate-limiting/04-access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ for _, strategy in helpers.each_strategy() do
goto continue
end

describe(fmt("#flaky Plugin: response-ratelimiting (access) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()
describe(fmt("Plugin: response-ratelimiting (access) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()

lazy_setup(function()
local bp = init_db(strategy, policy)
Expand Down Expand Up @@ -384,15 +384,15 @@ for _, strategy in helpers.each_strategy() do
wait()
local n = math.floor(ITERATIONS / 2)
for _ = 1, n do
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "test1.test" },
})
assert.res_status(200, res)
end

ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit

local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "test1.test" },
})
assert.res_status(200, res)
Expand All @@ -409,7 +409,7 @@ for _, strategy in helpers.each_strategy() do
["-v"] = true,
},
}
assert.truthy(ok)
assert.truthy(ok, res)
assert.matches("x%-ratelimit%-limit%-video%-second: %d+", res)
assert.matches("x%-ratelimit%-remaining%-video%-second: %d+", res)

Expand All @@ -420,21 +420,21 @@ for _, strategy in helpers.each_strategy() do
end)

it("blocks if exceeding limit", function()
test_limit("/response-headers?x-kong-limit=video=1", "test1.test")
test_limit("/response-headers?x-kong-limit=video%3D1", "test1.test")
end)

it("counts against the same service register from different routes", function()
wait()
local n = math.floor(ITERATIONS / 2)
for i = 1, n do
local res = proxy_client():get("/response-headers?x-kong-limit=video=1, test=" .. ITERATIONS, {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1%2C%20test%3D" .. ITERATIONS, {
headers = { Host = "test-service1.test" },
})
assert.res_status(200, res)
end

for i = n+1, ITERATIONS do
local res = proxy_client():get("/response-headers?x-kong-limit=video=1, test=" .. ITERATIONS, {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1%2C%20test%3D" .. ITERATIONS, {
headers = { Host = "test-service2.test" },
})
assert.res_status(200, res)
Expand All @@ -443,7 +443,7 @@ for _, strategy in helpers.each_strategy() do
ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the list

-- Additional request, while limit is ITERATIONS/second
local res = proxy_client():get("/response-headers?x-kong-limit=video=1, test=" .. ITERATIONS, {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1%2C%20test%3D" .. ITERATIONS, {
headers = { Host = "test-service1.test" },
})
assert.res_status(429, res)
Expand All @@ -457,7 +457,7 @@ for _, strategy in helpers.each_strategy() do
if i == n then
ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit
end
res = proxy_client():get("/response-headers?x-kong-limit=video=2, image=1", {
res = proxy_client():get("/response-headers?x-kong-limit=video%3D2%2C%20image%3D1", {
headers = { Host = "test2.test" },
})
assert.res_status(200, res)
Expand All @@ -471,33 +471,39 @@ for _, strategy in helpers.each_strategy() do
assert.equal(ITERATIONS - n, tonumber(res.headers["x-ratelimit-remaining-image-second"]))

for i = n+1, ITERATIONS do
res = proxy_client():get("/response-headers?x-kong-limit=video=1, image=1", {
if i == ITERATIONS then
ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit
end
res = proxy_client():get("/response-headers?x-kong-limit=video%3D1%2C%20image%3D1", {
headers = { Host = "test2.test" },
})
assert.res_status(200, res)
end
assert.equal(0, tonumber(res.headers["x-ratelimit-remaining-image-second"]))
assert.equal(ITERATIONS * 4 - (n * 2) - (ITERATIONS - n), tonumber(res.headers["x-ratelimit-remaining-video-minute"]))
assert.equal(ITERATIONS * 2 - (n * 2) - (ITERATIONS - n), tonumber(res.headers["x-ratelimit-remaining-video-second"]))

ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit

local res = proxy_client():get("/response-headers?x-kong-limit=video=1, image=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1%2C%20image%3D1", {
headers = { Host = "test2.test" },
})

assert.equal(0, tonumber(res.headers["x-ratelimit-remaining-image-second"]))
assert.equal(ITERATIONS * 4 - (n * 2) - (ITERATIONS - n), tonumber(res.headers["x-ratelimit-remaining-video-minute"]))
assert.equal(ITERATIONS * 2 - (n * 2) - (ITERATIONS - n), tonumber(res.headers["x-ratelimit-remaining-video-second"]))
assert.equal(ITERATIONS * 4 - (n * 2) - (ITERATIONS - n) - 1, tonumber(res.headers["x-ratelimit-remaining-video-minute"]))
assert.equal(ITERATIONS * 2 - (n * 2) - (ITERATIONS - n) - 1, tonumber(res.headers["x-ratelimit-remaining-video-second"]))
assert.res_status(429, res)
end)
end)

describe("With authentication", function()
describe("API-specific plugin", function()
it("blocks if exceeding limit and a per consumer & route setting", function()
test_limit("/response-headers?apikey=apikey123&x-kong-limit=video=1", "test3.test", ITERATIONS - 2)
test_limit("/response-headers?apikey=apikey123&x-kong-limit=video%3D1", "test3.test", ITERATIONS - 2)
end)

it("blocks if exceeding limit and a per route setting", function()
test_limit("/response-headers?apikey=apikey124&x-kong-limit=video=1", "test3.test", ITERATIONS - 3)
test_limit("/response-headers?apikey=apikey124&x-kong-limit=video%3D1", "test3.test", ITERATIONS - 3)
end)
end)
end)
Expand All @@ -513,10 +519,12 @@ for _, strategy in helpers.each_strategy() do
assert.equal(ITERATIONS, tonumber(json.headers["x-ratelimit-remaining-video"]))

-- Actually consume the limits
local res = proxy_client():get("/response-headers?x-kong-limit=video=2, image=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D2%2C%20image%3D1", {
headers = { Host = "test8.test" },
})
assert.res_status(200, res)
local json2 = cjson.decode(assert.res_status(200, res))
assert.equal(ITERATIONS-1, tonumber(json2.headers["x-ratelimit-remaining-image"]))
assert.equal(ITERATIONS, tonumber(json2.headers["x-ratelimit-remaining-video"]))

ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit

Expand All @@ -530,6 +538,7 @@ for _, strategy in helpers.each_strategy() do

it("combines multiple x-kong-limit headers from upstream", function()
wait()
-- NOTE: this test is not working as intended because multiple response headers are merged into one comma-joined header by send_text_response function
for _ = 1, ITERATIONS do
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D2&x-kong-limit=image%3D1", {
headers = { Host = "test4.test" },
Expand All @@ -549,25 +558,25 @@ for _, strategy in helpers.each_strategy() do

assert.res_status(429, res)
assert.equal(0, tonumber(res.headers["x-ratelimit-remaining-image-second"]))
assert.equal(1, tonumber(res.headers["x-ratelimit-remaining-video-second"]))
assert.equal(0, tonumber(res.headers["x-ratelimit-remaining-video-second"]))
end)
end)

it("should block on first violation", function()
wait()
local res = proxy_client():get("/response-headers?x-kong-limit=video=2, image=4", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D2%2C%20image%3D4", {
headers = { Host = "test7.test" },
})
assert.res_status(200, res)

ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit

local res = proxy_client():get("/response-headers?x-kong-limit=video=2", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D2", {
headers = { Host = "test7.test" },
})
local body = assert.res_status(429, res)
local json = cjson.decode(body)
assert.same({ message = "API rate limit exceeded for 'image'" }, json)
assert.matches("API rate limit exceeded for 'image'", json.message)
end)

describe("Config with hide_client_headers", function()
Expand All @@ -584,7 +593,7 @@ for _, strategy in helpers.each_strategy() do
end)
end)

describe(fmt("#flaky Plugin: response-ratelimiting (expirations) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()
describe(fmt("Plugin: response-ratelimiting (expirations) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()

lazy_setup(function()
local bp = init_db(strategy, policy)
Expand Down Expand Up @@ -624,7 +633,7 @@ for _, strategy in helpers.each_strategy() do

it("expires a counter", function()
wait()
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "expire1.test" },
})

Expand All @@ -637,7 +646,7 @@ for _, strategy in helpers.each_strategy() do
ngx.sleep(0.01)
wait() -- Wait for counter to expire

local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "expire1.test" },
})

Expand All @@ -649,7 +658,7 @@ for _, strategy in helpers.each_strategy() do
end)
end)

describe(fmt("#flaky Plugin: response-ratelimiting (access - global for single consumer) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()
describe(fmt("Plugin: response-ratelimiting (access - global for single consumer) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()

lazy_setup(function()
local bp = init_db(strategy, policy)
Expand Down Expand Up @@ -700,11 +709,11 @@ for _, strategy in helpers.each_strategy() do
end)

it("blocks when the consumer exceeds their quota, no matter what service/route used", function()
test_limit("/response-headers?apikey=apikey126&x-kong-limit=video=1", "test%d.test")
test_limit("/response-headers?apikey=apikey126&x-kong-limit=video%3D1", "test%d.test")
end)
end)

describe(fmt("#flaky Plugin: response-ratelimiting (access - global) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()
describe(fmt("Plugin: response-ratelimiting (access - global) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()

lazy_setup(function()
local bp = init_db(strategy, policy)
Expand Down Expand Up @@ -749,7 +758,7 @@ for _, strategy in helpers.each_strategy() do
it("blocks if exceeding limit", function()
wait()
for i = 1, ITERATIONS do
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = fmt("test%d.test", i) },
})
assert.res_status(200, res)
Expand All @@ -758,7 +767,7 @@ for _, strategy in helpers.each_strategy() do
ngx.sleep(SLEEP_TIME) -- Wait for async timer to increment the limit

-- last query, while limit is ITERATIONS/second
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "test1.test" },
})
assert.res_status(429, res)
Expand All @@ -767,7 +776,7 @@ for _, strategy in helpers.each_strategy() do
end)
end)

describe(fmt("#flaky Plugin: response-ratelimiting (fault tolerance) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()
describe(fmt("Plugin: response-ratelimiting (fault tolerance) with policy: #%s #%s [#%s]", redis_conf_name, policy, strategy), function()
if policy == "cluster" then
local bp, db

Expand Down Expand Up @@ -832,7 +841,7 @@ for _, strategy in helpers.each_strategy() do
end)

it("does not work if an error occurs", function()
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "failtest1.test" },
})
assert.res_status(200, res)
Expand All @@ -846,16 +855,16 @@ for _, strategy in helpers.each_strategy() do
-- affecting subsequent tests.

-- Make another request
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "failtest1.test" },
})
local body = assert.res_status(500, res)
local json = cjson.decode(body)
assert.same({ message = "An unexpected error occurred" }, json)
assert.matches("An unexpected error occurred", json.message)
end)

it("keeps working if an error occurs", function()
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "failtest2.test" },
})
assert.res_status(200, res)
Expand All @@ -869,7 +878,7 @@ for _, strategy in helpers.each_strategy() do
-- affecting subsequent tests.

-- Make another request
local res = proxy_client():get("/response-headers?x-kong-limit=video=1", {
local res = proxy_client():get("/response-headers?x-kong-limit=video%3D1", {
headers = { Host = "failtest2.test" },
})
assert.res_status(200, res)
Expand Down Expand Up @@ -940,7 +949,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = assert.res_status(500, res)
local json = cjson.decode(body)
assert.same({ message = "An unexpected error occurred" }, json)
assert.matches("An unexpected error occurred", json.message)
end)
it("keeps working if an error occurs", function()
-- Make another request
Expand Down

1 comment on commit 2582e76

@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-dev:2582e760461907305c3e93fba6fe7b3db40ebef9
Artifacts available https://github.com/Kong/kong/actions/runs/13174861414

Please sign in to comment.