From b48ed85c8ad65f0c6034003a26c66b450edf27a1 Mon Sep 17 00:00:00 2001 From: Joshua Schmid Date: Wed, 17 Jul 2024 14:17:11 +0200 Subject: [PATCH] fix(proxy-cache): correctly add age header on cache hit --- .../kong/proxy-cache-fix-age-header.yml | 4 + kong/plugins/proxy-cache/handler.lua | 2 +- kong/plugins/proxy-cache/schema.lua | 2 +- .../31-proxy-cache/02-access_spec.lua | 84 ++++++++++++++++++- 4 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 changelog/unreleased/kong/proxy-cache-fix-age-header.yml diff --git a/changelog/unreleased/kong/proxy-cache-fix-age-header.yml b/changelog/unreleased/kong/proxy-cache-fix-age-header.yml new file mode 100644 index 000000000000..abf18c8f24e3 --- /dev/null +++ b/changelog/unreleased/kong/proxy-cache-fix-age-header.yml @@ -0,0 +1,4 @@ +message: | + **proxy-cache**: Fixed a bug where the Age header was not being updated correctly when serving +scope: Plugin +type: bugfix diff --git a/kong/plugins/proxy-cache/handler.lua b/kong/plugins/proxy-cache/handler.lua index 69e11ae081b3..4b2c04421951 100644 --- a/kong/plugins/proxy-cache/handler.lua +++ b/kong/plugins/proxy-cache/handler.lua @@ -401,7 +401,7 @@ function ProxyCacheHandler:access(conf) reset_res_header(res) - set_res_header(res, "Age", floor(time() - res.timestamp), conf) + set_res_header(res, "age", floor(time() - res.timestamp), conf) set_res_header(res, "X-Cache-Status", "Hit", conf) set_res_header(res, "X-Cache-Key", cache_key, conf) diff --git a/kong/plugins/proxy-cache/schema.lua b/kong/plugins/proxy-cache/schema.lua index 34efa9648b56..768e6f06975f 100644 --- a/kong/plugins/proxy-cache/schema.lua +++ b/kong/plugins/proxy-cache/schema.lua @@ -78,7 +78,7 @@ return { description = "Caching related diagnostic headers that should be included in cached responses", type = "record", fields = { - { ["Age"] = {type = "boolean", default = true} }, + { age = {type = "boolean", default = true} }, { ["X-Cache-Status"] = {type = "boolean", default = true} }, { ["X-Cache-Key"] = {type = "boolean", default = true} }, }, diff --git a/spec/03-plugins/31-proxy-cache/02-access_spec.lua b/spec/03-plugins/31-proxy-cache/02-access_spec.lua index 665e23fade03..250b07a78302 100644 --- a/spec/03-plugins/31-proxy-cache/02-access_spec.lua +++ b/spec/03-plugins/31-proxy-cache/02-access_spec.lua @@ -102,6 +102,12 @@ do local route22 = assert(bp.routes:insert({ hosts = { "route-22.test" }, })) + local route23 = assert(bp.routes:insert({ + hosts = { "route-23.test" }, + })) + local route24 = assert(bp.routes:insert({ + hosts = { "route-24.test" }, + })) local consumer1 = assert(bp.consumers:insert { username = "bob", @@ -329,13 +335,39 @@ do content_type = { "text/plain", "application/json" }, [policy] = policy_config, response_headers = { - ["Age"] = false, + age = false, ["X-Cache-Status"] = false, ["X-Cache-Key"] = false }, }, }) + assert(bp.plugins:insert { + name = "proxy-cache", + route = { id = route23.id }, + config = { + strategy = policy, + content_type = { "text/plain", "application/json" }, + [policy] = policy_config, + response_headers = { + age = true, + ["X-Cache-Status"] = true, + ["X-Cache-Key"] = true + }, + }, + }) + + assert(bp.plugins:insert { + name = "proxy-cache", + route = { id = route24.id }, + config = { + strategy = policy, + content_type = { "text/plain", "application/json" }, + [policy] = policy_config, + -- leave reponse_header to default values + }, + }) + assert(helpers.start_kong({ plugins = "bundled", nginx_conf = "spec/fixtures/custom_nginx.template", @@ -416,6 +448,56 @@ do assert.is_not_nil(res.headers["X-Cache-Key"]) end) + it("response_headers headers on the response when configured", function() + -- Initial query to set cache + local res = assert(client:get("/get", { + headers = { + Host = "route-23.test", + }, + })) + -- Cache should be Miss + assert.res_status(200, res) + assert.is_same("Miss", res.headers["X-Cache-Status"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + assert.is_nil(res.headers["Age"]) + -- Cache should be HIT + res = assert(client:get("/get", { + headers = { + Host = "route-23.test", + }, + })) + assert.res_status(200, res) + assert.same("Hit", res.headers["X-Cache-Status"]) + -- response_headers are configured + assert.is_not_nil(res.headers["Age"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + end) + + it("response_headers headers on the response when set to default", function() + -- Initial query to set cache + local res = assert(client:get("/get", { + headers = { + Host = "route-24.test", + }, + })) + -- Cache should be Miss + assert.res_status(200, res) + assert.is_same("Miss", res.headers["X-Cache-Status"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + assert.is_nil(res.headers["Age"]) + res = assert(client:get("/get", { + headers = { + Host = "route-24.test", + }, + })) + -- Cache should be Hit + assert.res_status(200, res) + assert.same("Hit", res.headers["X-Cache-Status"]) + -- response_headers are on by default + assert.is_not_nil(res.headers["Age"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + end) + it("respects cache ttl", function() local res = assert(get(client, "route-6.test"))