Skip to content

Commit

Permalink
fix(proxy-cache-advanced): the field age in response_headers has be…
Browse files Browse the repository at this point in the history
…en changed from `age` to `Age` (#9360)

This PR fixed an issue where the field `age`  in response_headers has been changed from `age` to `Age`.
**Note:**
The value configured for age before the upgrade was not effective, and after the upgrade, the Age configuration will take effect. To maintain consistency in behavior before and after the upgrade, the "Age" needs to be set to false.

https://konghq.atlassian.net/browse/KM-198
  • Loading branch information
raoxiaoyan authored Jul 8, 2024
1 parent 71dcff4 commit d3fc46c
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 3 deletions.
4 changes: 4 additions & 0 deletions changelog/unreleased/kong-ee/proxy_cache_advanced_Age.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
"**proxy-cache-advanced**: The configuration `Age` in response_headers has been changed from lowercase to uppercase. Please reconfigure the `Age` if you need it."
type: bugfix
scope: Plugin
24 changes: 24 additions & 0 deletions kong/clustering/compat/checkers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,30 @@ end


local compatible_checkers = {
{ 3008000000, --[[ 3.8.0.0 ]]
function(config_table, dp_version, log_suffix)
local has_update
for _, plugin in ipairs(config_table.plugins or {}) do
if plugin.name == 'proxy-cache-advanced' then
local config = plugin.config
if config.response_headers["Age"] ~= nil then
config.response_headers.age = config.response_headers["Age"]
config.response_headers["Age"] = nil
has_update = true
end
end

if has_update then
log_warn_message(
'configures ' .. plugin.name .. ' plugin with:' ..
"the Age in response_headers",
"will be changed to age",
dp_version, log_suffix)
end
end
return has_update
end
},
{
3007001002, --[[3.7.1.2]]
function(config_table, dp_version, log_suffix)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ return {
description = "Caching related diagnostic headers that should be included in cached responses",
type = "record",
fields = {
{ age = {type = "boolean", default = true} },
{ ["X-Cache-Status"] = {type = "boolean", default = true} },
{ ["X-Cache-Key"] = {type = "boolean", default = true} },
{ ["Age"] = { type = "boolean", default = true } },
{ ["X-Cache-Status"] = { type = "boolean", default = true } },
{ ["X-Cache-Key"] = { type = "boolean", default = true } },
},
}},
{ redis = redis.config_schema }, -- redis schema is provided by
Expand All @@ -100,6 +100,30 @@ return {
default = false,
}},
},
shorthand_fields = {
{
response_headers = {
type = "json",
elements = { type = "boolean" },
deprecation = { message = "proxy-cache-advanced: config.response_headers.age has been changed from lowercase to uppercase, please use config.response_headers.Age instead", removal_in_version = "3.8", },
func = function(value)
local response_headers = {
["Age"] = value["Age"],
["X-Cache-Status"] = value["X-Cache-Status"],
["X-Cache-Key"] = value["X-Cache-Key"],
}

if value["age"] ~= nil then
response_headers["Age"] = false
end

return {
response_headers = response_headers,
}
end
}
},
}
}
},
},
Expand Down
62 changes: 62 additions & 0 deletions plugins-ee/proxy-cache-advanced/spec/02-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ for _, policy in ipairs({"memory", "redis"}) do
local route22 = assert(bp.routes:insert {
hosts = { "route-22.test" },
})
local route23 = assert(bp.routes:insert {
hosts = { "route-23.test" },
})

local consumer1 = assert(bp.consumers:insert {
username = "bob",
Expand Down Expand Up @@ -392,6 +395,19 @@ for _, policy in ipairs({"memory", "redis"}) do
},
},
})

assert(bp.plugins:insert {
name = "proxy-cache-advanced",
route = { id = route23.id },
config = {
strategy = policy,
content_type = { "text/plain", "application/json" },
[policy] = policy_config,
response_headers = {
age = true,
},
},
})

assert(helpers.start_kong({
plugins = "bundled,proxy-cache-advanced",
Expand Down Expand Up @@ -574,6 +590,7 @@ for _, policy in ipairs({"memory", "redis"}) do

local body1 = assert.res_status(200, res)
assert.same("Miss", res.headers["X-Cache-Status"])
assert.is_nil(res.headers["Age"])

-- cache key is a sha256sum of the prefix uuid, method, and $request
local cache_key1 = res.headers["X-Cache-Key"]
Expand All @@ -592,6 +609,7 @@ for _, policy in ipairs({"memory", "redis"}) do

local body2 = assert.res_status(200, res)
assert.same("Hit", res.headers["X-Cache-Status"])
assert.is_not_nil(res.headers["Age"])
local cache_key2 = res.headers["X-Cache-Key"]
assert.same(cache_key1, cache_key2)

Expand Down Expand Up @@ -629,6 +647,50 @@ for _, policy in ipairs({"memory", "redis"}) do
assert.is_not_nil(res.headers["Age"])
assert.is_not_nil(res.headers["X-Cache-Key"])
end)

it("No Age headers on the response when enabling the response_headers.age", function()
local request = {
method = "GET",
path = "/get",
headers = {
host = "route-23.test",
},
}

local res = assert(client:send(request))
assert.res_status(200, res)
local res = assert(client:send(request))
assert.res_status(200, res)
assert.is_nil(res.headers["Age"])
end)

it("error log should have deprecated warning information for the response_headers.age", function()
local route24 = assert(bp.routes:insert({
hosts = { "route-24.test" },
}))
local request = {
method = "POST",
path = "/routes/" .. route24.id .. "/plugins",
headers = { ["Content-Type"] = "application/json", },
body = {
config = {
strategy = policy,
content_type = { "text/plain", "application/json" },
[policy] = policy_config,
response_headers = {
age = true,
}
},
name = "proxy-cache-advanced",
}
}

local res = assert(admin_client:send(request))
assert.res_status(201, res)
assert.logfile().has.line(
"proxy-cache-advanced: config.response_headers.age has been changed from lowercase to uppercase, please use config.response_headers.Age instead",
true, 1)
end)

it("respects cache ttl", function()
local res = assert(client:send {
Expand Down
15 changes: 15 additions & 0 deletions spec-ee/02-integration/14-hybrid_mode/04-config-compat_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,21 @@ describe("CP/DP config compat #" .. strategy, function()
return config.redis.cluster_max_redirections == nil
end
},
{
plugin = "proxy-cache-advanced",
label = "w/ the field Age in response_headers has been changed from Age to age",
pending = false,
config = {
response_headers = {
Age = false,
},
strategy = "memory"
},
status = STATUS.NORMAL,
validator = function(config)
return config.response_headers.Age == nil and config.response_headers.age ~= nil
end
},

}

Expand Down

0 comments on commit d3fc46c

Please sign in to comment.