feat: faas plugin refactoring with url path forwarding#5616
Conversation
5dad53a to
ca539fd
Compare
| if ctx.curr_req_matched and ctx.curr_req_matched["*"] then | ||
| local end_path = ctx.curr_req_matched["*"] | ||
|
|
||
| if path:sub(-1, -1) == "/" or end_path:sub(1, 1) == "/" then |
There was a problem hiding this comment.
Using :byte would be better.
BTW, it is not a good idea to refactor & add feature in the same PR.
There was a problem hiding this comment.
for byte you mean like this
if path:byte(-1, -1) == string.byte"/" or end_path:byte(1, 1) == string.byte"/" then
The PRs are interdependent and require one PR to be merged before another. Do you think I should split it up into 2 PRs?
There was a problem hiding this comment.
Like if path:byte(-1) == string.byte("/") or end_path:byte(1) == string.byte("/") then.
We can accept this one now.
Next time, please don't mix up refactor & new feature.
| local url_decoded = url.parse(conf.function_uri) | ||
| local path = url_decoded.path or "/" | ||
|
|
||
| if ctx.curr_req_matched and ctx.curr_req_matched["*"] then |
There was a problem hiding this comment.
We should use :ext instead of *
There was a problem hiding this comment.
Agree. I was using ** that's why the extra arg was received through curr_req_matched["*"]. Updated
| "function_uri": "http://localhost:8765/api" | ||
| } | ||
| }, | ||
| "uri": "/azure/**" |
There was a problem hiding this comment.
| "uri": "/azure/**" | |
| "uri": "/azure/*" |
The wildcard match only uses one *.
|
|
||
| ngx.say(body) | ||
|
|
||
| local code, _, body = t("/azure/httptrigger", "GET") |
There was a problem hiding this comment.
Let's add another test which contains multiple levels of paths, like /azure/http/trigger.
There was a problem hiding this comment.
As discussed offline the following cases have been covered
multiple levels of paths
multiple slashes in the paths
the wildcard matches an empty part, like /azure
|
|
||
|
|
||
|
|
||
| === TEST 11: check multilevel url path forwarding |
There was a problem hiding this comment.
Next time, we can use a table drive test for those cases. See
Line 498 in 7242216
3 participants
What this PR does / why we need it:
This PR is based on the idea of creating a single generic upstream where each faas plugin implements an authorization schema and perform the header processing.
GOAL: Reduce the same code over multiple plugins (mainly faas)
How developers should write new (faas) plugins:
request_processorfunction on the plugin lua file that take `plugin conf, ngx context and the URL params (that contains headers, body, urlpath, query params etc. etc.)". The function can perform a series of operations including update/adding the necessary authorization headers, performing encoding/decoding (base64 or anything) on the body.Pre-submission checklist: