feat: proxy_rewrite support miltiple regex pattern matching by fengxsong · Pull Request #9194 · apache/apisix

fengxsong · 2023-03-29T10:07:22Z

Description

As explained in #9179, this PR is a preliminary implementation. The following changes are included:

~~a new property that named none_match_abort, when none_match_abort=true and none of regex_uri matched, apisix will return 404 directly, for compatibility it defaults to false~~;
remove the length limit of field regex_uri, but it must have a even length;

With this feature, we can now configure regex_uri like

......
      name: proxy-rewrite
      enable: true
      config:
        regex_uri:
        - "/v1/user/([:A-Za-z0-9_-]+)/follow"
        - "/api/v1/user/$1/follow"
        - "/v1/user/([:A-Za-z0-9_-]+)/test"
        - "/api/v1/user/$1/test"
        # - can add more regex patterns here
......

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change (Not yet, I will update the document when this proposal is accepted)
I have verified that this change is backward compatible

juststillthinking · 2023-03-30T06:42:20Z

Hi @fengxsong, Test cases are needed!

fengxsong · 2023-03-30T09:11:53Z

Hi @fengxsong, Test cases are needed!

done

fengxsong · 2023-04-12T03:19:09Z

ping @monkeyDluffy6017

juststillthinking · 2023-04-12T07:11:56Z

I will check this later, THX @fengxsong

fengxsong · 2023-04-13T05:07:03Z

Is the ci failure caused by this code change? https://github.com/apache/apisix/actions/runs/4685205482/jobs/8302219899?pr=9194

kingluo · 2023-04-13T08:58:15Z

@fengxsong please add doc to describe this feature.

kingluo · 2023-04-13T10:43:19Z

 | method                      | string        | False    |         | ["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | Rewrites the HTTP method.                                                                                                                                                                                                                                                                                                                                                                                                                                                    |
-| regex_uri                   | array[string] | False    |         |                                                                                                                                        | New upstream forwarding address. Regular expressions can be used to match the URL from client. If it matches, the URL template is forwarded to the Upstream otherwise, the URL from the client is forwarded. When both `uri` and `regex_uri` are configured, `uri` is used first. For example, `[" ^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3"]`. Here, the first element is the regular expression to match and the second element is the URL template forwarded to the Upstream. |
+| none_match_abort                      | boolean        | False    |    `false`     |  | Abort with 404 status code when none of regex_uri matched, useful when making only those routes that match regular expression patterns public.                                                                                                                                                                                                                                                                                                                                                                                                                                                    |
+| regex_uri                   | array[string] | False    |         |                                                                                                                                        | New upstream forwarding address. Regular expressions can be used to match the URL from client. If it matches, the URL template is forwarded to the Upstream otherwise, the URL from the client is forwarded if `none_match_abort=false`. When both `uri` and `regex_uri` are configured, `uri` is used first. For example, `["^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3", ^/theothers/(.*)/(.*)", "/theothers/$1-$2"]`. Here, the first and the third elements are the regular expressions to match, the second and the fourth elements are the URL templates forwarded to the Upstream. Length of this property must be a even number. |


It's better to declare it supports multiple patterns for matching and substitution. It will iterate all patterns one by one until a successful match and substitution.

soulbird · 2023-04-14T07:49:50Z

+GET /test/plugin/proxy/rewrite/hello HTTP/1.1
+--- http_config
+    server {
+        listen 8125;


There are many URIs for testing in lib.server, usually, it is not necessary to start the server separately

I think he's following the other test cases in this test case file

soulbird · 2023-04-14T07:56:32Z


-        upstream_uri = uri
+        if ctx.proxy_rewrite_regex_uri_captures == nil and conf.none_match_abort then
+            return 404, { error_msg = "404 Route Not Found" }


This error message is wrong, but the rewrite was not successful, not that the route did not match successfully

soulbird · 2023-04-14T07:58:33Z

I don't understand why the none_match_abort parameter should be added, the proxy-rewrite plugin should not be able to block the request

fengxsong · 2023-04-14T08:36:41Z

I don't understand why the none_match_abort parameter should be added, the proxy-rewrite plugin should not be able to block the request

There are some scenarios we don't want to expose all of our APIs, for example.

      match:
        hosts:
        - echo.example.local
        paths:
        - /v1/user/*
      name: echo
      plugins:
      - config:
          none_match_abort: true
          regex_uri:
          - /v1/user/([:A-Za-z0-9_-]+)/follow
          - /api/v1/user/$1/follow
          - /v1/user/([:A-Za-z0-9_-]+)/comment
          - /api/v1/user/$1/comment
        enable: true
        name: proxy-rewrite

we can only expose /v1/user/([:A-Za-z0-9_-]+)/follow and /v1/user/([:A-Za-z0-9_-]+)/comment, and hide the rest API endpoints.

From the point of view of ability classification, none_match_abort shouldn't be here. so this might needs more discussions.

juststillthinking · 2023-05-05T01:50:43Z

@fengxsong please fix the ci

kingluo

rebase to the latest master, because proxy-rewrite.lua has merged new changes.

Signed-off-by: fengxsong <[email protected]>

* upstream/master: (42 commits) fix: add `rust-check` target to the makefile for deps pre-checking (apache#9453) test: remove duplicate `proto` unit test file and tweak test case title (apache#9445) feat: proxy-mirror add grpc and grpcs support (apache#9388) test: replace all httpbin upstream (apache#9452) fix: 404 error in ci test (apache#9447) fix: body-transformer plugin return raw body anytime (apache#9446) fix(ci): fips: ensure apisix compiles with openssl3 (apache#9427) chore(deps): bump actions/stale from 7 to 8 (apache#9159) chore(deps): bump actions/setup-node from 3.5.1 to 3.6.0 (apache#8625) chore(test): split the access log of apisix and test server (apache#8819) docs: fix grammar (apache#9395) fix(wolf-rbac): other plugin in consumer not effective when consumer used wolf-rbac plugin (apache#9287) (apache#9298) docs: use shell instead of Python to configure protos resources (apache#9414) docs: add guide to authenticate websocket connections (apache#9369) docs: updated Stream Proxy doc (apache#9367) feat: proxy_rewrite support miltiple regex pattern matching (apache#9194) feat: route-level MTLS (apache#9322) ci: tcp logger with real logger server (apache#9392) fix: always parse domain when host is domain name (apache#9332) docs: CHANGELOG.md Chinese docs lint (apache#9411) ...

fengxsong force-pushed the feat_proxy_rewrite_multiple_regex_uris branch from 61850a1 to fbe5af4 Compare March 30, 2023 01:07

fengxsong marked this pull request as ready for review March 30, 2023 01:17

juststillthinking requested review from kingluo and soulbird April 6, 2023 09:18

kingluo reviewed Apr 12, 2023

View reviewed changes

Comment thread apisix/plugins/proxy-rewrite.lua Outdated

Comment thread apisix/plugins/proxy-rewrite.lua Outdated

kingluo reviewed Apr 13, 2023

View reviewed changes

Comment thread apisix/plugins/proxy-rewrite.lua

kingluo requested review from juststillthinking and leslie-tsang April 13, 2023 08:57

kingluo reviewed Apr 13, 2023

View reviewed changes

fengxsong force-pushed the feat_proxy_rewrite_multiple_regex_uris branch from b930aba to 2e9dff3 Compare April 13, 2023 13:09

soulbird reviewed Apr 14, 2023

View reviewed changes

fengxsong force-pushed the feat_proxy_rewrite_multiple_regex_uris branch from 2e9dff3 to 1bca225 Compare April 14, 2023 08:58

juststillthinking reviewed Apr 24, 2023

View reviewed changes

Comment thread apisix/plugins/proxy-rewrite.lua Outdated