From e40b16ea79342a8e29e485012fdd6885cbb379d9 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Fri, 1 Sep 2023 10:59:04 +0530 Subject: [PATCH 1/9] fix: provide error instead of nil panic when cache_zone is missing --- apisix/plugins/proxy-cache/memory.lua | 10 +++++ t/plugin/proxy-cache/memory.t | 55 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/apisix/plugins/proxy-cache/memory.lua b/apisix/plugins/proxy-cache/memory.lua index 0112db63b568..6cdcb55cbce9 100644 --- a/apisix/plugins/proxy-cache/memory.lua +++ b/apisix/plugins/proxy-cache/memory.lua @@ -32,6 +32,10 @@ end function _M:set(key, obj, ttl) + if self.dict == nil then + return nil, "no cache_zone provided" + end + local obj_json = core.json.encode(obj) if not obj_json then return nil, "could not encode object" @@ -43,6 +47,9 @@ end function _M:get(key) + if self.dict == nil then + return nil, "no cache_zone provided" + end -- If the key does not exist or has expired, then res_json will be nil. local res_json, err = self.dict:get(key) if not res_json then @@ -63,6 +70,9 @@ end function _M:purge(key) + if self.dict == nil then + return nil, "no cache_zone provided" + end self.dict:delete(key) end diff --git a/t/plugin/proxy-cache/memory.t b/t/plugin/proxy-cache/memory.t index 5984dbd47776..7c398586e426 100644 --- a/t/plugin/proxy-cache/memory.t +++ b/t/plugin/proxy-cache/memory.t @@ -661,3 +661,58 @@ GET /hello --- more_headers Cache-Control: only-if-cached --- error_code: 504 + + + +=== TEST 36: configure plugin without memory_cache zone +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "proxy-cache": { + "cache_strategy": "memory", + "cache_key":["$host","$uri"], + "cache_bypass": ["$arg_bypass"], + "cache_control": true, + "cache_method": ["GET"], + "cache_ttl": 10, + "cache_http_status": [200] + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1986": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- error_code: 200 +--- response_body +passed + + + +=== TEST 37: hit route +--- LAST +--- request +GET /hello +--- grep_error_log eval +qr/no cache_zone provided/ +--- grep_error_log_out +no cache_zone provided +no cache_zone provided \ No newline at end of file From 42478d70dbc534dffd1be38f7dc033cd628197bb Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Fri, 1 Sep 2023 11:01:35 +0530 Subject: [PATCH 2/9] fix lint Signed-off-by: Ashish Tiwari --- t/plugin/proxy-cache/memory.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/plugin/proxy-cache/memory.t b/t/plugin/proxy-cache/memory.t index 7c398586e426..382eb3766049 100644 --- a/t/plugin/proxy-cache/memory.t +++ b/t/plugin/proxy-cache/memory.t @@ -715,4 +715,4 @@ GET /hello qr/no cache_zone provided/ --- grep_error_log_out no cache_zone provided -no cache_zone provided \ No newline at end of file +no cache_zone provided From 18f7097797fcfe22e5bb504eef292e5afd0149e1 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Fri, 1 Sep 2023 11:05:30 +0530 Subject: [PATCH 3/9] fix lint Signed-off-by: Ashish Tiwari --- t/plugin/proxy-cache/memory.t | 1 - 1 file changed, 1 deletion(-) diff --git a/t/plugin/proxy-cache/memory.t b/t/plugin/proxy-cache/memory.t index 382eb3766049..66dece71e138 100644 --- a/t/plugin/proxy-cache/memory.t +++ b/t/plugin/proxy-cache/memory.t @@ -708,7 +708,6 @@ passed === TEST 37: hit route ---- LAST --- request GET /hello --- grep_error_log eval From 086ee245c208c67eb0f04412b5f059a63043a52f Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Tue, 5 Sep 2023 13:58:33 +0530 Subject: [PATCH 4/9] throw error on invalid proxy-cache config --- apisix/plugins/proxy-cache/init.lua | 10 +++++++++- apisix/plugins/proxy-cache/memory.lua | 7 ++++--- t/plugin/proxy-cache/memory.t | 16 +++------------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/apisix/plugins/proxy-cache/init.lua b/apisix/plugins/proxy-cache/init.lua index 333c20e20e1b..03a835a00c32 100644 --- a/apisix/plugins/proxy-cache/init.lua +++ b/apisix/plugins/proxy-cache/init.lua @@ -20,11 +20,14 @@ local disk_handler = require("apisix.plugins.proxy-cache.disk_handler") local util = require("apisix.plugins.proxy-cache.util") local core = require("apisix.core") local ipairs = ipairs +local ngx = ngx +local ngx_shared = ngx.shared local plugin_name = "proxy-cache" local STRATEGY_DISK = "disk" local STRATEGY_MEMORY = "memory" +local DEFAULT_CACHE_ZONE = "disk_cache_one" local schema = { type = "object", @@ -33,7 +36,7 @@ local schema = { type = "string", minLength = 1, maxLength = 100, - default = "disk_cache_one", + default = DEFAULT_CACHE_ZONE, }, cache_strategy = { type = "string", @@ -140,6 +143,11 @@ function _M.check_schema(conf) end end + -- For memory based cache, the default cache_zone cannot be used. cache_zone will also be set as default value in case when passed empty. + if conf.cache_strategy == STRATEGY_MEMORY and conf.cache_zone == DEFAULT_CACHE_ZONE then + return false, "invalid or empty cache_zone for cache_strategy: "..conf.cache_strategy + end + return true end diff --git a/apisix/plugins/proxy-cache/memory.lua b/apisix/plugins/proxy-cache/memory.lua index 6cdcb55cbce9..9d5c665a8d92 100644 --- a/apisix/plugins/proxy-cache/memory.lua +++ b/apisix/plugins/proxy-cache/memory.lua @@ -33,7 +33,7 @@ end function _M:set(key, obj, ttl) if self.dict == nil then - return nil, "no cache_zone provided" + return nil, "invalid cache_zone provided" end local obj_json = core.json.encode(obj) @@ -48,8 +48,9 @@ end function _M:get(key) if self.dict == nil then - return nil, "no cache_zone provided" + return nil, "invalid cache_zone provided" end + -- If the key does not exist or has expired, then res_json will be nil. local res_json, err = self.dict:get(key) if not res_json then @@ -71,7 +72,7 @@ end function _M:purge(key) if self.dict == nil then - return nil, "no cache_zone provided" + return nil, "invalid cache_zone provided" end self.dict:delete(key) end diff --git a/t/plugin/proxy-cache/memory.t b/t/plugin/proxy-cache/memory.t index 66dece71e138..69c0ad30df7b 100644 --- a/t/plugin/proxy-cache/memory.t +++ b/t/plugin/proxy-cache/memory.t @@ -701,17 +701,7 @@ Cache-Control: only-if-cached } --- request GET /t ---- error_code: 200 ---- response_body -passed - - +--- response_body_like +.*err: invalid or empty cache_zone for cache_strategy: memory.* +--- error_code: 400 -=== TEST 37: hit route ---- request -GET /hello ---- grep_error_log eval -qr/no cache_zone provided/ ---- grep_error_log_out -no cache_zone provided -no cache_zone provided From 2c1d22e6c6349a0c58feac14817a2f71c626cd11 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Tue, 5 Sep 2023 14:01:14 +0530 Subject: [PATCH 5/9] fix lint err Signed-off-by: Ashish Tiwari --- apisix/plugins/proxy-cache/init.lua | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apisix/plugins/proxy-cache/init.lua b/apisix/plugins/proxy-cache/init.lua index 03a835a00c32..4c7ce65228bd 100644 --- a/apisix/plugins/proxy-cache/init.lua +++ b/apisix/plugins/proxy-cache/init.lua @@ -20,8 +20,6 @@ local disk_handler = require("apisix.plugins.proxy-cache.disk_handler") local util = require("apisix.plugins.proxy-cache.util") local core = require("apisix.core") local ipairs = ipairs -local ngx = ngx -local ngx_shared = ngx.shared local plugin_name = "proxy-cache" @@ -143,7 +141,8 @@ function _M.check_schema(conf) end end - -- For memory based cache, the default cache_zone cannot be used. cache_zone will also be set as default value in case when passed empty. + -- For memory based cache, the default cache_zone cannot be used. + -- cache_zone will also be set as default value in case when passed empty. if conf.cache_strategy == STRATEGY_MEMORY and conf.cache_zone == DEFAULT_CACHE_ZONE then return false, "invalid or empty cache_zone for cache_strategy: "..conf.cache_strategy end From ea08b27302ef684a6a4a2a6efcc379e030d1ee97 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Tue, 5 Sep 2023 16:56:31 +0530 Subject: [PATCH 6/9] fix lint error Signed-off-by: Ashish Tiwari --- apisix/plugins/proxy-cache/init.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugins/proxy-cache/init.lua b/apisix/plugins/proxy-cache/init.lua index 4c7ce65228bd..1f49e4904a11 100644 --- a/apisix/plugins/proxy-cache/init.lua +++ b/apisix/plugins/proxy-cache/init.lua @@ -141,7 +141,7 @@ function _M.check_schema(conf) end end - -- For memory based cache, the default cache_zone cannot be used. + -- For memory based cache, the default cache_zone cannot be used. -- cache_zone will also be set as default value in case when passed empty. if conf.cache_strategy == STRATEGY_MEMORY and conf.cache_zone == DEFAULT_CACHE_ZONE then return false, "invalid or empty cache_zone for cache_strategy: "..conf.cache_strategy From 9e5068f398541bd0da0fd5e2e95208088056adda Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Tue, 5 Sep 2023 16:59:59 +0530 Subject: [PATCH 7/9] reindex Signed-off-by: Ashish Tiwari --- t/plugin/proxy-cache/memory.t | 1 - 1 file changed, 1 deletion(-) diff --git a/t/plugin/proxy-cache/memory.t b/t/plugin/proxy-cache/memory.t index 69c0ad30df7b..c307a4073132 100644 --- a/t/plugin/proxy-cache/memory.t +++ b/t/plugin/proxy-cache/memory.t @@ -704,4 +704,3 @@ GET /t --- response_body_like .*err: invalid or empty cache_zone for cache_strategy: memory.* --- error_code: 400 - From eee6560415c12157f38e11288bda08ae310b1dc7 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Wed, 6 Sep 2023 11:17:46 +0530 Subject: [PATCH 8/9] Update t/plugin/proxy-cache/memory.t enhance test description Co-authored-by: Abhishek Choudhary --- t/plugin/proxy-cache/memory.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/plugin/proxy-cache/memory.t b/t/plugin/proxy-cache/memory.t index c307a4073132..e617f8a1b0fc 100644 --- a/t/plugin/proxy-cache/memory.t +++ b/t/plugin/proxy-cache/memory.t @@ -664,7 +664,7 @@ Cache-Control: only-if-cached -=== TEST 36: configure plugin without memory_cache zone +=== TEST 36: configure plugin without memory_cache zone for cache_strategy = memory --- config location /t { content_by_lua_block { From 148c46f91fba13c0e070d125552c3a66f68269de Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Wed, 13 Sep 2023 10:08:07 +0530 Subject: [PATCH 9/9] apply suggestions Signed-off-by: Ashish Tiwari --- apisix/plugins/proxy-cache/init.lua | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/apisix/plugins/proxy-cache/init.lua b/apisix/plugins/proxy-cache/init.lua index 4c7ce65228bd..918f755994ea 100644 --- a/apisix/plugins/proxy-cache/init.lua +++ b/apisix/plugins/proxy-cache/init.lua @@ -130,23 +130,26 @@ function _M.check_schema(conf) local found = false local local_conf = core.config.local_conf() if local_conf.apisix.proxy_cache then + local err = "cache_zone " .. conf.cache_zone .. " not found" for _, cache in ipairs(local_conf.apisix.proxy_cache.zones) do + -- cache_zone passed in plugin config matched one of the proxy_cache zones if cache.name == conf.cache_zone then - found = true + -- check for the mismatch between cache_strategy and corresponding cache zone + if (conf.cache_strategy == STRATEGY_MEMORY and cache.disk_path) or + (conf.cache_strategy == STRATEGY_DISK and not cache.disk_path) then + err = "invalid or empty cache_zone for cache_strategy: "..conf.cache_strategy + else + found = true + end + break end end if found == false then - return false, "cache_zone " .. conf.cache_zone .. " not found" + return false, err end end - -- For memory based cache, the default cache_zone cannot be used. - -- cache_zone will also be set as default value in case when passed empty. - if conf.cache_strategy == STRATEGY_MEMORY and conf.cache_zone == DEFAULT_CACHE_ZONE then - return false, "invalid or empty cache_zone for cache_strategy: "..conf.cache_strategy - end - return true end