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/05/28 07:39:30 UTC
[GitHub] [apisix] keilon opened a new pull request #4332: feat: add real_ip_recursive to nginx.conf
keilon opened a new pull request #4332:
URL: https://github.com/apache/apisix/pull/4332
### What this PR does / why we need it:
Enable to set real_ip_recursive to nginx.conf. When apisix is behind multiple layer network, the real client ip may not be recognized. See also [http://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive](http://nginx.org/en/docs/http/ngx_http_realip_module.html#real_ip_recursive)
### Pre-submission checklist:
* [X] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [X] 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4332:
URL: https://github.com/apache/apisix/pull/4332#discussion_r641379541
##########
File path: t/cli/test_main.sh
##########
@@ -621,3 +621,10 @@ if ! grep "keepalive_timeout 6s;" conf/nginx.conf > /dev/null; then
fi
echo "passed: found the keepalive related parameter in nginx.conf"
+
+if ! grep "real_ip_recursive off;" conf/nginx.conf > /dev/null; then
+ echo "failed: 'real_ip_recursive off;' not in nginx.conf"
+ exit 1
+fi
+
+echo "passed: disable real_ip_recursive by default in nginx.conf"
Review comment:
Need another test to change its default value and check, like `client_max_body_size: 512m ` in my example.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] keilon commented on a change in pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
keilon commented on a change in pull request #4332:
URL: https://github.com/apache/apisix/pull/4332#discussion_r641393026
##########
File path: t/cli/test_main.sh
##########
@@ -621,3 +621,10 @@ if ! grep "keepalive_timeout 6s;" conf/nginx.conf > /dev/null; then
fi
echo "passed: found the keepalive related parameter in nginx.conf"
+
+if ! grep "real_ip_recursive off;" conf/nginx.conf > /dev/null; then
+ echo "failed: 'real_ip_recursive off;' not in nginx.conf"
+ exit 1
+fi
+
+echo "passed: disable real_ip_recursive by default in nginx.conf"
Review comment:
Fixed.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou removed a comment on pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
Firstsawyou removed a comment on pull request #4332:
URL: https://github.com/apache/apisix/pull/4332#issuecomment-850222126
We need to add corresponding test cases, https://github.com/apache/apisix/blob/master/t/cli/test_main.sh
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4332:
URL: https://github.com/apache/apisix/pull/4332#discussion_r641343710
##########
File path: apisix/cli/ngx_tpl.lua
##########
@@ -229,6 +229,10 @@ http {
real_ip_header {* http.real_ip_header *};
{% end %}
+ {% if http.real_ip_recursive then %}
+ real_ip_recursive {* http.real_ip_recursive *};
Review comment:
Need to add a test like: https://github.com/apache/apisix/blob/9a40fd7cb497974f9d7bd75f128eed5dd238cdc6/t/cli/test_main.sh#L382-L399
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on pull request #4332:
URL: https://github.com/apache/apisix/pull/4332#issuecomment-850222126
We need to add corresponding test cases, https://github.com/apache/apisix/blob/master/t/cli/test_main.sh
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4332:
URL: https://github.com/apache/apisix/pull/4332
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4332: feat: add real_ip_recursive to nginx.conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4332:
URL: https://github.com/apache/apisix/pull/4332#discussion_r641379541
##########
File path: t/cli/test_main.sh
##########
@@ -621,3 +621,10 @@ if ! grep "keepalive_timeout 6s;" conf/nginx.conf > /dev/null; then
fi
echo "passed: found the keepalive related parameter in nginx.conf"
+
+if ! grep "real_ip_recursive off;" conf/nginx.conf > /dev/null; then
+ echo "failed: 'real_ip_recursive off;' not in nginx.conf"
+ exit 1
+fi
+
+echo "passed: disable real_ip_recursive by default in nginx.conf"
Review comment:
Need a test to change its default value and check, like `client_max_body_size: 512m ` in my example.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org