You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/10/20 11:15:16 UTC
[GitHub] [apisix] zhendongcmss opened a new pull request #5292: feat(proxy-rewrite): add method proxy
zhendongcmss opened a new pull request #5292:
URL: https://github.com/apache/apisix/pull/5292
### What this PR does / why we need it:
fix: https://github.com/apache/apisix/issues/5173
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734324916
##########
File path: docs/en/latest/plugins/proxy-rewrite.md
##########
@@ -39,6 +39,7 @@ The `proxy-rewrite` is an upstream proxy information rewriting plugin, which sup
| --------- | ------------- | ----------- | ------- | ----------------- | ------------------------------------------------------------ |
| scheme | string | optional | "http" | ["http", "https"] | Upstream new `schema` forwarding protocol. This option is deprecated. It's recommended to set the proxy `scheme` in the Upstream object's `scheme` field instead.|
| uri | string | optional | | | Upstream new `uri` forwarding address. Supports the use of [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). Variables must start with `$`, such as `$arg_name`. |
+| method | enum | optional | | ["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | rewrite the HTTP method.|
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734667627
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,149 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "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
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: set route(update rewrite method)
Review comment:
Thank you！
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734452571
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,149 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "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
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: set route(update rewrite method)
Review comment:
We need to hit the path in the test, like this one:
https://github.com/apache/apisix/blob/05b36d36176b7f06c04a1aed82b9987f6cb6996f/t/plugin/proxy-rewrite.t#L1336
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#issuecomment-950474243
>
>
> @spacewander Are there commands used on comment box to restart CICD chekcs. The CICD checks often fail due to other test cases.
I see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734176942
##########
File path: t/plugin/proxy-rewrite.t
##########
@@ -1342,3 +1342,115 @@ host: test.com:6443
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 46: set route(rewrite method)
Review comment:
Could you add the new test in proxy-rewrite2.t? We don't need to set `--- no_error_log` for tests in that file.
##########
File path: docs/en/latest/plugins/proxy-rewrite.md
##########
@@ -39,6 +39,7 @@ The `proxy-rewrite` is an upstream proxy information rewriting plugin, which sup
| --------- | ------------- | ----------- | ------- | ----------------- | ------------------------------------------------------------ |
| scheme | string | optional | "http" | ["http", "https"] | Upstream new `schema` forwarding protocol. This option is deprecated. It's recommended to set the proxy `scheme` in the Upstream object's `scheme` field instead.|
| uri | string | optional | | | Upstream new `uri` forwarding address. Supports the use of [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). Variables must start with `$`, such as `$arg_name`. |
+| method | enum["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | optional | | | rewrite the HTTP method.|
Review comment:
We need to put the enum value to the "valid" section, like the "scheme" row above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734306808
##########
File path: docs/en/latest/plugins/proxy-rewrite.md
##########
@@ -39,6 +39,7 @@ The `proxy-rewrite` is an upstream proxy information rewriting plugin, which sup
| --------- | ------------- | ----------- | ------- | ----------------- | ------------------------------------------------------------ |
| scheme | string | optional | "http" | ["http", "https"] | Upstream new `schema` forwarding protocol. This option is deprecated. It's recommended to set the proxy `scheme` in the Upstream object's `scheme` field instead.|
| uri | string | optional | | | Upstream new `uri` forwarding address. Supports the use of [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). Variables must start with `$`, such as `$arg_name`. |
+| method | enum | optional | | ["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | rewrite the HTTP method.|
Review comment:
The type should be string
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,149 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "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
Review comment:
There is no need to set `GET /t` & `no_error_log`. They are set in the header of the file.
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,149 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "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
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: set route(update rewrite method)
Review comment:
Need to check the upstream receives rewritten method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734176942
##########
File path: t/plugin/proxy-rewrite.t
##########
@@ -1342,3 +1342,115 @@ host: test.com:6443
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 46: set route(rewrite method)
Review comment:
~~Could you add the new test in proxy-rewrite2.t? We don't need to set `--- no_error_log` for tests in that file.~~
We can add the new test in proxy-rewrite3.t, with no_error_log set like https://github.com/apache/apisix/blob/2f250d53698b0fa617ea981d2d488ebd756bb655/t/plugin/cors2.t#L32
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734255958
##########
File path: t/plugin/proxy-rewrite.t
##########
@@ -1342,3 +1342,115 @@ host: test.com:6443
x-real-ip: 127.0.0.1
--- no_error_log
[error]
+
+
+
+=== TEST 46: set route(rewrite method)
Review comment:
will update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733482814
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -34,6 +34,13 @@ local schema = {
maxLength = 4096,
pattern = [[^\/.*]],
},
+ method = {
+ description = "proxy route method",
+ type = "string",
+ enum = {"GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS",
Review comment:
From curent code, parameters are rarelt used in schema, but this way reducing code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r735207337
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
Review comment:
We can remove "no_error_log" there
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
+[error]
+--- grep_error_log_out
+plugin_proxy_rewrite get method: POST
+
+
+
+=== TEST 3: set route(update rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "GET",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
Review comment:
Ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733263029
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -34,6 +34,11 @@ local schema = {
maxLength = 4096,
pattern = [[^\/.*]],
},
+ method = {
+ description = "proxy route method",
+ type = "string",
+ enum = {"GET", "POST", "PUT", "HEAD"}
Review comment:
See https://github.com/openresty/lua-nginx-module#http-method-constants, we may support more methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r735096375
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
+[error]
+--- grep_error_log_out
+plugin_proxy_rewrite get method: POST
+
+
+
+=== TEST 3: set route(update rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "GET",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
Review comment:
You are right, but TEST 1 already test this path(rewrite method). This test path for that this PR doesn't change default behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733479125
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -132,6 +139,51 @@ do
core.table.insert(upstream_names, name)
end
+ local switch = {
Review comment:
Good idea
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r735056588
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
+[error]
+--- grep_error_log_out
+plugin_proxy_rewrite get method: POST
+
+
+
+=== TEST 3: set route(update rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "GET",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
Review comment:
Should we use another method instead of `GET`, as `GET` is the target method used to proxy the upstream, we cannot be convinced if we use `GET` since the method doesn't change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#issuecomment-950473754
@spacewander Are there commands used on comment box to restart CICD chekcs. The CICD checks often fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r735208449
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
Review comment:
OK will remove.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734395428
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,149 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "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
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: set route(update rewrite method)
Review comment:
Hi @spacewander, I need help. The test cases are more difficult than code. Fellowing your suggestion, I logging upstream receiving method down, but test cases failed. How I get the upstream method in test case and comparing it with except string ?
In my PR, t/lib/server.lua
line 79 `ngx.say("method: ", ngx.req.get_method())`, but it say` "passed"`, no `"method: xxx"`. In my mind, ngx.say("xxx") return "xxx" in the response body.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733479125
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -132,6 +139,51 @@ do
core.table.insert(upstream_names, name)
end
+ local switch = {
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r735198301
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
+[error]
+--- grep_error_log_out
+plugin_proxy_rewrite get method: POST
+
+
+
+=== TEST 3: set route(update rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "GET",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
Review comment:
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss edited a comment on pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss edited a comment on pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#issuecomment-950473754
@spacewander Are there commands used on comment box to restart CICD chekcs. The CICD checks often fail due to other test cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733265200
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -34,6 +34,11 @@ local schema = {
maxLength = 4096,
pattern = [[^\/.*]],
},
+ method = {
+ description = "proxy route method",
+ type = "string",
+ enum = {"GET", "POST", "PUT", "HEAD"}
Review comment:
OK， will add them in next commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734310973
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,149 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "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
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: set route(update rewrite method)
Review comment:
We can log it down in https://github.com/apache/apisix/blob/a6f757e08f7433b996d7725be0a90de55169336d/t/lib/server.lua#L75
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733482814
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -34,6 +34,13 @@ local schema = {
maxLength = 4096,
pattern = [[^\/.*]],
},
+ method = {
+ description = "proxy route method",
+ type = "string",
+ enum = {"GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS",
Review comment:
From current code, parameters are rarelt used in schema, but this way reducing code.
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -34,6 +34,13 @@ local schema = {
maxLength = 4096,
pattern = [[^\/.*]],
},
+ method = {
+ description = "proxy route method",
+ type = "string",
+ enum = {"GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS",
Review comment:
From current code, parameters are rarely used in schema, but this way reducing code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734453407
##########
File path: t/lib/server.lua
##########
@@ -76,6 +76,7 @@ function _M.plugin_proxy_rewrite()
ngx.say("uri: ", ngx.var.uri)
ngx.say("host: ", ngx.var.host)
ngx.say("scheme: ", ngx.var.scheme)
+ ngx.say("method: ", ngx.req.get_method())
Review comment:
```suggestion
ngx.log(ngx.WARN, "plugin_proxy_rewrite get method: ", ngx.req.get_method())
```
We can use a log so the other tests won't fail because the response body is changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5292:
URL: https://github.com/apache/apisix/pull/5292
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r734247092
##########
File path: docs/en/latest/plugins/proxy-rewrite.md
##########
@@ -39,6 +39,7 @@ The `proxy-rewrite` is an upstream proxy information rewriting plugin, which sup
| --------- | ------------- | ----------- | ------- | ----------------- | ------------------------------------------------------------ |
| scheme | string | optional | "http" | ["http", "https"] | Upstream new `schema` forwarding protocol. This option is deprecated. It's recommended to set the proxy `scheme` in the Upstream object's `scheme` field instead.|
| uri | string | optional | | | Upstream new `uri` forwarding address. Supports the use of [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). Variables must start with `$`, such as `$arg_name`. |
+| method | enum["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | optional | | | rewrite the HTTP method.|
Review comment:
OK， Will update
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r735208752
##########
File path: t/plugin/proxy-rewrite3.t
##########
@@ -0,0 +1,157 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+log_level("info");
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->no_error_log && !$block->error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: set route(rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "POST",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
+[error]
+--- grep_error_log_out
+plugin_proxy_rewrite get method: POST
+
+
+
+=== TEST 3: set route(update rewrite 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,
+ [[{
+ "methods": ["GET"],
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "/plugin_proxy_rewrite",
+ "method": "GET",
+ "scheme": "http",
+ "host": "apisix.iresty.com"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/hello"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 4: hit route(upstream uri: should be /hello)
+--- request
+GET /hello
+--- no_error_log
Review comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#issuecomment-948278259
help review @spacewander
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733398484
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -34,6 +34,13 @@ local schema = {
maxLength = 4096,
pattern = [[^\/.*]],
},
+ method = {
+ description = "proxy route method",
+ type = "string",
+ enum = {"GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS",
Review comment:
The enum can be calculated from the {GET = ngx.HTTP_GET, ...} map
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -132,6 +139,51 @@ do
core.table.insert(upstream_names, name)
end
+ local switch = {
Review comment:
We can use a `{GET = ngx.HTTP_GET, ...}` to dismiss repeating `ngx.req.set_method`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#discussion_r733505023
##########
File path: apisix/plugins/proxy-rewrite.lua
##########
@@ -24,6 +24,19 @@ local re_sub = ngx.re.sub
local sub_str = string.sub
local str_find = core.string.find
+local switch_map = {GET = ngx.HTTP_GET, POST = ngx.HTTP_POST, PUT = ngx.HTTP_PUT,
+ HEAD = ngx.HTTP_HEAD, DELETE = ngx.HTTP_DELETE,
+ OPTIONS = ngx.HTTP_OPTIONS, MKCOL = ngx.HTTP_MKCOL,
+ COPY = ngx.HTTP_COPY, MOVE = ngx.HTTP_MOVE,
+ PROPFIND = ngx.HTTP_PROPFIND, LOCK = ngx.HTTP_LOCK,
+ UNLOCK = ngx.HTTP_UNLOCK, PATCH = ngx.HTTP_PATCH,
+ TRACE = ngx.HTTP_TRACE,
+ }
+local schema_method_enum = {}
+for key, _ in pairs(switch_map) do
Review comment:
```suggestion
for key in pairs(switch_map) do
```
##########
File path: docs/en/latest/plugins/proxy-rewrite.md
##########
@@ -39,6 +39,7 @@ The `proxy-rewrite` is an upstream proxy information rewriting plugin, which sup
| --------- | ------------- | ----------- | ------- | ----------------- | ------------------------------------------------------------ |
| scheme | string | optional | "http" | ["http", "https"] | Upstream new `schema` forwarding protocol. This option is deprecated. It's recommended to set the proxy `scheme` in the Upstream object's `scheme` field instead.|
| uri | string | optional | | | Upstream new `uri` forwarding address. Supports the use of [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). Variables must start with `$`, such as `$arg_name`. |
+| method | enum["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | optional | | | proxy route method to this method。|
Review comment:
```suggestion
| method | enum["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | optional | | | rewrite the HTTP method.|
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #5292: feat(proxy-rewrite): add method proxy
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #5292:
URL: https://github.com/apache/apisix/pull/5292#issuecomment-948439313
Should add some test cases to cover the changes @zhendongcmss .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org