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 2022/05/04 09:06:04 UTC
[GitHub] [apisix] kwanhur opened a new pull request, #6981: feat(ops): handle real_ip_from CIDR format
kwanhur opened a new pull request, #6981:
URL: https://github.com/apache/apisix/pull/6981
### Description
Handle `real_ip_from` CIDR format
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
* Use `validate_cidr_or_ip` to check
* Add `create_loopback_ip_matcher` in `apisix/core/ip.lua`
* Use loopback ip matcher to validate ip whether a loopback address
Fixes # (issue)
https://github.com/apache/apisix/blob/cc88a8db0ba64e1cb351e389ca5ec64c516073d6/apisix/cli/ops.lua#L279-L281
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] 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
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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] tzssangglass commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1119200148
I think we can't use the `create_ip_matcher` function of the `ip` module until apisix is running.
--
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] tzssangglass commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1119198885
A bit tricky.
As I recall cjson is only loaded after APISIX is run (it belongs to the OpenResty runtime). During the startup phase, the cli module, you have to reference three functions of the ip module, one of which will necessarily load json.
--
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] kwanhur commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
kwanhur commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1119530728
What about `djson` instead of `cjson`?
https://github.com/apache/apisix/blob/2800a06f00ff9010d893eb8c4b9c94359cc3a96b/rockspec/apisix-master-0.rockspec#L61
--
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] tzssangglass commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1119681916
> What about `djson` instead of `cjson`?
just try
--
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] kwanhur commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
kwanhur commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1118609878
I need some help @tokers @tzssangglass
--
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 #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #6981:
URL: https://github.com/apache/apisix/pull/6981
--
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] kwanhur commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
kwanhur commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1118593461
I found the `cjson.so` place on `/usr/local/openresty-debug/lualib/` directory, but not it the `package.cpath`.
--
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] kwanhur commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
kwanhur commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1119702106
After a trial, `lua-resty-ipmatcher` not good on `apisix.cli.ops` because it requires `lua-resty-core` runtime :-(
[lua-libcidr-ffi](https://github.com/GUI/lua-libcidr-ffi) maybe a choice, it bases on [libcidr](http://www.over-yonder.net/~fullermd/projects/libcidr) a C library.
--
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 diff in pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6981:
URL: https://github.com/apache/apisix/pull/6981#discussion_r867483065
##########
t/cli/test_validate_config.sh:
##########
@@ -167,11 +167,79 @@ plugins:
nginx_config:
http:
real_ip_from:
- - "127.0.0.2"
+ - "128.0.0.2"
' > conf/config.yaml
out=$(make init 2>&1 || true)
-if ! echo "$out" | grep "missing '127.0.0.1' in the nginx_config.http.real_ip_from for plugin batch-requests"; then
+if ! echo "$out" | grep "missing loopback or unspecified in the nginx_config.http.real_ip_from for plugin batch-requests"; then
+ echo "failed: should check the realip configuration for batch-requests"
+ exit 1
+fi
+
+echo "passed: check the realip configuration for batch-requests"
+
+echo '
+plugins:
+- batch-requests
+nginx_config:
+ http:
+ real_ip_from:
+ - "127.0.0.1"
+' > conf/config.yaml
+
+out=$(make init 2>&1 || true)
+if echo "$out" | grep "missing loopback or unspecified in the nginx_config.http.real_ip_from for plugin batch-requests"; then
+ echo "failed: should check the realip configuration for batch-requests"
+ exit 1
+fi
+
+echo "passed: check the realip configuration for batch-requests"
Review Comment:
For multi checks we only need to put one `passed` at the end, like:
https://github.com/apache/apisix/blob/0d1280cde7fcd463f2e86941935e053561bcf331/t/cli/test_validate_config.sh#L149-L162
--
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] kwanhur commented on pull request #6981: feat(ops): handle real_ip_from CIDR format
Posted by GitBox <gi...@apache.org>.
kwanhur commented on PR #6981:
URL: https://github.com/apache/apisix/pull/6981#issuecomment-1117201701
`cjson` not in `apisix-master-0.rockspec`, any missing it? @spacewander
Using `apisix/core/ip.lua`, here require `apisix/core/json` where original `cjson` reference.
Test case failed cause at `cjson`
```plaintext
/apisix/bin/apisix init
/usr/local/openresty-debug/luajit/bin/luajit ./apisix/cli/apisix.lua init
/usr/local/openresty-debug/luajit/bin/luajit: ./apisix/core/json.lua:22: module 'cjson.safe' not found:
no field package.preload['cjson.safe']
no file '/apisix/cjson/safe/init.lua'
no file '/apisix/deps/share/lua/5.1/cjson/safe/init.lua'
no file '/apisix/deps/share/lua/5.1/cjson/safe.lua'
no file '/usr/local/apisix/deps/share/lua/5.1/cjson/safe.lua'
no file './cjson/safe.lua'
```
--
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