Rewrite by verb 1130 #1187
Rewrite by verb 1130 #1187
Conversation
|
Hi @pimg Thanks for your contribution. Have been super busy days here, I'll review this policy asap, but I do not forget it. Regards |
| @@ -107,6 +144,57 @@ | |||
| ], | |||
| "default": "plain" | |||
| } | |||
| }, | |||
| "methods": { | |||
There was a problem hiding this comment.
It'll be nice if a new definition for methods can be done, we're trying to implement that because make things easier to follow and less verbose. As a example:
https://github.com/3scale/APIcast/blob/master/gateway/src/apicast/policy/jwt_claim_check/apicast-policy.json#L13
Reference
https://json-schema.org/understanding-json-schema/structuring.html
| -- Returns true if no Method is provided in the config for backwardscompatibility | ||
| local function should_apply(methods) | ||
| local request_method = ngx.req.get_method() | ||
| local next = next |
There was a problem hiding this comment.
I would move this next to the top of the file.
|
|
||
| local should_apply_command = true | ||
|
|
||
| if methods ~= nil and next(methods) ~= nil then |
There was a problem hiding this comment.
This code can be a bit simpler, what about:
local function should_apply(methods)
local request_method = ngx.req.get_method()
if methods == nil and next(methods) == nil then
return true
end
for _,v in pairs(methods) do
if v == request_method then
return true
end
end
return false
end
There was a problem hiding this comment.
Hi Eloy thanks for the review comments, I'm working to resolve them.
One question: should't if methods == nil and next(methods) == nil then
be: if methods == nil or next(methods) == nil then
(So or instead of and) because in the case of and when methods == nil next(methods) will throw an error? Or am I missing something here?
| @@ -28,6 +28,30 @@ local function substitute(func, subject, regex, replace, options) | |||
| return new_uri, num_changes > 0 | |||
| end | |||
|
|
|||
| -- Returns true if the Method of the request is in the methods of the command meaning the rewrite rule should be applied | |||
| -- Returns true if no Method is provided in the config for backwardscompatibility | |||
| local function should_apply(methods) | |||
There was a problem hiding this comment.
What about if we change the function name to is_match_method(methods)
…d and refactored should_apply method to is_match_method
Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>
eloycoto
force-pushed
the
rewrite-by-verb-1130
branch
from
d21ff32
to
fb3ee3a
Compare
This resolves #1130 by taking an array of HTTP methods in the config for either the commands or the query_args_command. When the one or HTTP methods are applied in the config the rewrite rule only executes if the request method matches one in the config. If no methods are provided in the config the rewrite command will execute on all request methods.