Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(proxy-cache): keep cache_method same with nginx's proxy_cache_methods #4814

Merged
merged 2 commits into from
Aug 15, 2021
Merged

fix(proxy-cache): keep cache_method same with nginx's proxy_cache_methods #4814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6f004ee
fix(proxy-cache): keep cache_method same with nginx's proxy_cache_met…
agile6v Aug 12, 2021
89757d4
Update proxy_cache_methods in APISIX.pm
agile6v Aug 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apisix/cli/ngx_tpl.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ http {
proxy_cache $upstream_cache_zone;
proxy_cache_valid any {% if proxy_cache.cache_ttl then %} {* proxy_cache.cache_ttl *} {% else %} 10s {% end %};
proxy_cache_min_uses 1;
proxy_cache_methods GET HEAD;
proxy_cache_methods GET HEAD POST;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nginx by default doesn't enable the cache for POST. Why change this behavior here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't affect the default behavior of APISIX, which means that user can specify the POST method on APISIX.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agile6v
Could you sync it to APISIX.pm?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to cache a POST request?

The only I can think of for caching POST requests is that some internal specification forces the consistent use of POST requests……

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't affect the default behavior of APISIX, which means that user can specify the POST method on APISIX.

Here you change the Nginx template, without a configurable setting. This is a change for default havior.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agile6v
Could you sync it to APISIX.pm?

done

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to cache a POST request?

The only I can think of for caching POST requests is that some internal specification forces the consistent use of POST requests……

Yes. This is a very rare scenario. The users don't enable the method on APISIX, it doesn't take effect. If users enable it, they need to know what they are doing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't affect the default behavior of APISIX, which means that user can specify the POST method on APISIX.

Here you change the Nginx template, without a configurable setting. This is a change for default havior.

Whether to enable is here (cache_method)

proxy_cache_lock_timeout 5s;
proxy_cache_use_stale off;
proxy_cache_key $upstream_cache_key;
Expand Down
6 changes: 5 additions & 1 deletion apisix/plugins/proxy-cache.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ local schema = {
cache_method = {
type = "array",
minItems = 1,
items = core.schema.method_schema,
items = {
description = "supported http method",
type = "string",
enum = {"GET", "POST", "HEAD"},
},
uniqueItems = true,
default = {"GET", "HEAD"},
},
Expand Down
2 changes: 1 addition & 1 deletion docs/en/latest/plugins/proxy-cache.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ The proxy-cache plugin, which provides the ability to cache upstream response da
| cache_zone | string | optional | disk_cache_one | | Specify which cache area to use, each cache area can be configured with different paths. In addition, cache areas can be predefined in conf/config.yaml file. When the default value is not used, the specified cache area is inconsistent with the pre-defined cache area in the conf/config.yaml file, and the cache is invalid. |
| cache_key | array[string] | optional | ["$host", "$request_uri"] | | key of a cache, can use variables. For example: ["$host", "$uri", "-cache-id"] |
| cache_bypass | array[string] | optional | | | Whether to skip cache retrieval. That is, do not look for data in the cache. It can use variables, and note that cache data retrieval will be skipped when the value of this attribute is not empty or not '0'. For example: ["$arg_bypass"] |
| cache_method | array[string] | optional | ["GET", "HEAD"] | ["GET", "POST", "PUT", "DELETE", "PATCH", "HEAD","OPTIONS", "CONNECT", "TRACE"] | Decide whether to be cached according to the request method |
| cache_method | array[string] | optional | ["GET", "HEAD"] | ["GET", "POST", "HEAD"] | Decide whether to be cached according to the request method |
| cache_http_status | array[integer] | optional | [200, 301, 404] | [200, 599] | Decide whether to be cached according to the upstream response status |
| hide_cache_headers | boolean | optional | false | | Whether to return the Expires and Cache-Control response headers to the client, |
| no_cache | array[string] | optional | | | Whether to cache data, it can use variables, and note that the data will not be cached when the value of this attribute is not empty or not '0'. |
Expand Down
2 changes: 1 addition & 1 deletion docs/zh/latest/plugins/proxy-cache.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ title: proxy-cache
| cache_zone | string | 可选 | disk_cache_one | | 指定使用哪个缓存区域，不同的缓存区域可以配置不同的路径，在 conf/config.yaml 文件中可以预定义使用的缓存区域。当不使用默认值时，指定的缓存区域与 conf/config.yaml 文件中预定义的缓存区域不一致，缓存无效。 |
| cache_key | array[string] | 可选 | ["$host", "$request_uri"] | | 缓存key，可以使用变量。例如：["$host", "$uri", "-cache-id"] |
| cache_bypass | array[string] | 可选 | | | 是否跳过缓存检索，即不在缓存中查找数据，可以使用变量，需要注意当此参数的值不为空或非'0'时将会跳过缓存的检索。例如：["$arg_bypass"] |
| cache_method | array[string] | 可选 | ["GET", "HEAD"] | ["GET", "POST", "PUT", "DELETE", "PATCH", "HEAD","OPTIONS", "CONNECT", "TRACE"] | 根据请求method决定是否需要缓存 |
| cache_method | array[string] | 可选 | ["GET", "HEAD"] | ["GET", "POST", "HEAD"] | 根据请求method决定是否需要缓存 |
| cache_http_status | array[integer] | 可选 | [200, 301, 404] | [200, 599] | 根据响应码决定是否需要缓存 |
| hide_cache_headers | boolean | 可选 | false | | 是否将 Expires 和 Cache-Control 响应头返回给客户端 |
| no_cache | array[string] | 可选 | | | 是否缓存数据，可以使用变量，需要注意当此参数的值不为空或非'0'时将不会缓存数据 |
Expand Down
2 changes: 1 addition & 1 deletion t/APISIX.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ _EOC_
proxy_cache \$upstream_cache_zone;
proxy_cache_valid any 10s;
proxy_cache_min_uses 1;
proxy_cache_methods GET HEAD;
proxy_cache_methods GET HEAD POST;
proxy_cache_lock_timeout 5s;
proxy_cache_use_stale off;
proxy_cache_key \$upstream_cache_key;
Expand Down
44 changes: 44 additions & 0 deletions t/plugin/proxy-cache.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,3 +737,47 @@ Cache-Control: bar
Expires: any
--- no_error_log
[error]



=== TEST 27: sanity check (invalid method for cache_method)
--- 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_zone": "disk_cache_one",
"cache_bypass": ["$arg_bypass"],
"cache_method": ["GET", "PUT"],
"cache_http_status": [200],
"hide_cache_headers": true,
"no_cache": ["$arg_no_cache"]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body eval
qr/failed to check the configuration of plugin proxy-cache err/
--- no_error_log
[error]