[GitHub] [apisix-dashboard] liuxiran commented on issue #516: [Question] what does the option select item mean in the consumer-restriction plugin
liuxiran commented on issue #516: URL: https://github.com/apache/apisix-dashboard/issues/516#issuecomment-698737213 OK，just added a task list in apisix2082 :) 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: us...@infra.apache.org
[GitHub] [apisix] liuxiran commented on issue #2082: request help: Update plugin's schema
liuxiran commented on issue #2082: URL: https://github.com/apache/apisix/issues/2082#issuecomment-698736589 - [ ] consumer-restriction 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
nic-chen commented on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698734341 > > do you remember that change it? @Yiyiyimu > > > > I know the reason. > > > > When we create a new etcd v3 object, it depends on `cosocket` feature. so we have to move this code block into `timer` land. it is not supports `cosocket` feature in `init` and `init_worker` phase. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] membphis edited a comment on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
membphis edited a comment on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698730792 > do you remember that change it? @Yiyiyimu I know the reason. When we create a new etcd v3 object, it depends on `cosocket` feature. so we have to move this code block into `timer` land. it is not supports `cosocket` feature in `init` and `init_worker` phase. 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen edited a comment on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
nic-chen edited a comment on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698726024 I checked the old version. we created a etcd cli only once here: https://github.com/apache/apisix/blob/7f64ba810ef4ea9a105f7c3e8a83ac54e47ad58e/apisix/core/config_etcd.lua#L434-L446 and then we could reuse it. I think we could use that mode. do you remember that change it? @Yiyiyimu 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
membphis commented on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698731969 I just updated the initialization process and now it only initializes once. https://github.com/apache/apisix/pull/2312/commits/bd2d95038d29593c1b205fc4516768ea94bd4976 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
membphis commented on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698730792 > do you remember that change it? @Yiyiyimu I know the reason. When we create a new etcd v3 object, it depends on `cosocket` feature. so we have to move this code block into `timer` land. 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
nic-chen commented on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698726024 I checked the old version. we created a etcd cli only once here: https://github.com/apache/apisix/blob/7f64ba810ef4ea9a105f7c3e8a83ac54e47ad58e/apisix/core/config_etcd.lua#L434-L446 and then we could reuse it. I think we could use that mode. do you remember that change it? @Yiyiyimu 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: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
Yiyiyimu commented on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698707008 @membphis Sure that's my mistake. Thank you for the fix!! 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
membphis commented on pull request #2312: URL: https://github.com/apache/apisix/pull/2312#issuecomment-698706596 @Yiyiyimu please take a look at this issue, it is a bugfix 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: us...@infra.apache.org
[GitHub] [apisix] membphis opened a new pull request #2312: bugfix: create the etcd object in `xpcall`, it may fail, the return v…
membphis opened a new pull request #2312: URL: https://github.com/apache/apisix/pull/2312 …alues of `etcd.new` should be `res, err`. fix issue: #2310 ### What this PR does / why we need it: ### 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? 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: us...@infra.apache.org
[GitHub] [apisix] gy09535 opened a new pull request #2311: fix: https://github.com/apache/apisix/issues/2257
gy09535 opened a new pull request #2311: URL: https://github.com/apache/apisix/pull/2311 ### What this PR does / why we need it: ### Pre-submission checklist: * [x] Did you explain what problem does this PR solve? Or what new features have been added? * [x] Have you added corresponding test cases? * [ ] Have you modified the corresponding document? * [x] Is this PR backward compatible? 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: us...@infra.apache.org
[GitHub] [apisix] gy09535 edited a comment on issue #2257: bug: http_to_https ret_code 307 not working
gy09535 edited a comment on issue #2257: URL: https://github.com/apache/apisix/issues/2257#issuecomment-698704288 > @gy09535 > I think you mean 308 but not 307: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308 > Code 307 is 'Temporary'. yeah, should be 308 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: us...@infra.apache.org
[GitHub] [apisix] gy09535 commented on issue #2257: bug: http_to_https ret_code 307 not working
gy09535 commented on issue #2257: URL: https://github.com/apache/apisix/issues/2257#issuecomment-698704288 > @gy09535 > I think you mean 308 but not 307: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308 > Code 307 is 'Temporary'. yeah， should be 308 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: us...@infra.apache.org
[GitHub] [apisix] Firstsawyou opened a new issue #2310: bug: failed to match any routes
Firstsawyou opened a new issue #2310:
URL: https://github.com/apache/apisix/issues/2310
### Issue description
Before the `etcd` service is closed, the `apisix` request is normal. After
the `etcd` service is turned off and turned on again, the `apisix` request
reports 404 ({"error_msg":"failed to match any routes"}).
### Environment
* apisix version (cmd: `apisix version`): 1.5
* OS: centos 7
### Minimal test code / Steps to reproduce the issue
1.Add route and configure upstream information
curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
"methods": ["GET"],
"uri": "/index.html",
"upstream": {
"type": "roundrobin",
"nodes": {
"127.0.0.1:9081": 1
}
},
"desc": "upstream node"
}'
2. Access to 'route' is normal
3.Turn off the etcd service and turn it on again
4.Exception in accessing 'route' again
### What's the actual result? (including assertion message & call stack if
applicable)
curl -i http://127.0.0.1:9080/index.html
HTTP/1.1 404 Not Found
Date: Fri, 25 Sep 2020 03:36:10 GMT
Content-Type: text/html; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Server: APISIX/1.5
{"error_msg":"failed to match any routes"}
error log:
2020/09/25 10:37:07 [error] 16527#16527: *28 lua entry thread aborted:
runtime error: ...r_service-id_route-id/apisix/apisix/core/config_etcd.lua:421:
attempt to concatenate local 'err' (a nil value)
stack traceback:
coroutine 0:
...r_service-id_route-id/apisix/apisix/core/config_etcd.lua: in
function <...r_service-id_route-id/apisix/apisix/core/config_etcd.lua:414>,
context: ngx.timer
2020/09/25 10:37:07 [error] 16527#16527: *30 lua entry thread aborted:
runtime error: ...r_service-id_route-id/apisix/apisix/core/config_etcd.lua:421:
attempt to concatenate local 'err' (a nil value)
stack traceback:
coroutine 0:
...r_service-id_route-id/apisix/apisix/core/config_etcd.lua: in
function <...r_service-id_route-id/apisix/apisix/core/config_etcd.lua:414>,
context: ngx.timer
### What's the expected result?
curl -i http://127.0.0.1:9080/index.html
HTTP/1.1 200
..
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:
us...@infra.apache.org
[GitHub] [apisix] liuhengloveyou edited a comment on issue #2257: bug: http_to_https ret_code 307 not working
liuhengloveyou edited a comment on issue #2257: URL: https://github.com/apache/apisix/issues/2257#issuecomment-698702015 If the user has explicitly configured `ret_code`, we should not change it. Unless the configuration is wrong, we should indicate the range in the documentation. I think the range of ` should` be `400 > ret_code >= 300`? 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: us...@infra.apache.org
[GitHub] [apisix] spacewander commented on issue #2257: bug: http_to_https ret_code 307 not working
spacewander commented on issue #2257: URL: https://github.com/apache/apisix/issues/2257#issuecomment-698702658 @gy09535 I think you mean 308 but not 307: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308 Code 307 is 'Temporary'. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #516: [Question] what does the option select item mean in the consumer-restriction plugin
juzhiyuan commented on issue #516: URL: https://github.com/apache/apisix-dashboard/issues/516#issuecomment-698699627 Just like this one https://github.com/apache/apisix/issues/2082#issue-681772868 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #516: [Question] what does the option select item mean in the consumer-restriction plugin
juzhiyuan commented on issue #516: URL: https://github.com/apache/apisix-dashboard/issues/516#issuecomment-698699504 This need to modify its schema Let's put this in TODO list. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] liuxiran opened a new issue #516: [Question] what does the option select item mean in the consumer-restriction plugin
liuxiran opened a new issue #516: URL: https://github.com/apache/apisix-dashboard/issues/516 Please answer these questions before submitting your issue. - Why do you submit this issue? - [ ] Question or discussion - [ ] Bug - [ ] Requirements - [ ] Feature or performance improvement - [ ] Other ___ ### Question - What do you want to know? I'm confused about the option select item in the consumer-restriction plugin  what does it mean? ___ ### Bug - Which version of Apache APISIX Dashboard, OS, and Browser? - What happened? If possible, provide a way to reproduce the error. ___ ### Requirement or improvement - Please describe your requirements or improvement suggestions. 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: us...@infra.apache.org
[GitHub] [apisix] gy09535 edited a comment on issue #2257: bug: http_to_https ret_code 307 not working
gy09535 edited a comment on issue #2257: URL: https://github.com/apache/apisix/issues/2257#issuecomment-698682866 I thinks when we send redirect to 301 ,some browser default change the request to get, so we should add some logical to handle post request , when the request is post we should send the ret_code to 307, according to https://github.com/apache/apisix/blob/master/apisix/plugins/redirect.lua#L136 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: us...@infra.apache.org
[GitHub] [apisix] gy09535 commented on issue #2257: bug: http_to_https ret_code 307 not working
gy09535 commented on issue #2257: URL: https://github.com/apache/apisix/issues/2257#issuecomment-698682866 I thinks when we send redirect to 301 ,some browser default change the request to get, so we should add some logical to handle post request , when the request is post we should send the ret_code to 307 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: us...@infra.apache.org
[GitHub] [apisix] tokers commented on issue #2303: Proposal: inptroduce traffic split plugin
tokers commented on issue #2303: URL: https://github.com/apache/apisix/issues/2303#issuecomment-698676957 > Good job > It will be better if you can send this proposal to mailing list too OK, i will. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] liuxiran opened a new pull request #514: fix: add a config item APISIX_DEBUG_URL to support online debug
liuxiran opened a new pull request #514: URL: https://github.com/apache/apisix-dashboard/pull/514 Please answer these questions before submitting a pull request - Why submit this pull request? - [ ] Bug fix - [ ] New feature provided - [ ] Improve performance - Related issues fix: #499 ___ ### Bugfix - Description - How to fix? Add a config item APISIX_DEBUG_URL, which should be configed the accessible apisix address ___ ### New feature or improvement - Describe the details and related test reports. 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494699403
##
File path: apisix/plugins/hmac-auth.lua
##
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
-local diff = abs(ngx_time() - params.timestamp)
-core.log.info("timestamp diff: ", diff)
-if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+local time = ngx.parse_http_time(params.date)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2306: 1.fix generation of uuid;
membphis commented on a change in pull request #2306:
URL: https://github.com/apache/apisix/pull/2306#discussion_r494698342
##
File path: apisix/init.lua
##
@@ -107,6 +107,10 @@ function _M.http_init_worker()
require("apisix.debug").init_worker()
require("apisix.upstream").init_worker()
+-- need call seed
+local uuid = require("resty.jit-uuid")
+uuid.seed()
Review comment:
you can print more log for confirming why it works badly. we need to
know the reason.
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:
us...@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on pull request #2304: fix: Update error message when Route doesn't exist。
liuhengloveyou commented on pull request #2304: URL: https://github.com/apache/apisix/pull/2304#issuecomment-698666356 > @liuhengloveyou please check the commit mesg again, contains Chinese chars. updated 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: us...@infra.apache.org
[GitHub] [apisix] uptree commented on a change in pull request #2305: fix: kafka-logger plugin sent to the same partition everytime
uptree commented on a change in pull request #2305: URL: https://github.com/apache/apisix/pull/2305#discussion_r494686819 ## File path: apisix/plugins/kafka-logger.lua ## @@ -82,6 +84,11 @@ local function send_kafka_data(conf, log_message) end broker_config["request_timeout"] = conf.timeout * 1000 +broker_config["partitioner"] = function(key, num, correlation_id) Review comment: we can use "TEST 4" and "TEST 5" of kafka-logger.t test it，and number of partitions for a kafka topic >= 2. 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: us...@infra.apache.org
[GitHub] [apisix] uptree commented on a change in pull request #2305: fix: kafka-logger plugin sent to the same partition everytime
uptree commented on a change in pull request #2305: URL: https://github.com/apache/apisix/pull/2305#discussion_r49469 ## File path: apisix/plugins/kafka-logger.lua ## @@ -82,6 +84,11 @@ local function send_kafka_data(conf, log_message) end broker_config["request_timeout"] = conf.timeout * 1000 +broker_config["partitioner"] = function(key, num, correlation_id) Review comment: > It will be better if you can add test cases to cover. we can use "TEST 4" and "TEST 5" of kafka-logger.t test it，and number of partitions for a kafka topic >= 2. 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: us...@infra.apache.org
[GitHub] [apisix] uptree commented on a change in pull request #2305: fix: kafka-logger plugin sent to the same partition everytime
uptree commented on a change in pull request #2305: URL: https://github.com/apache/apisix/pull/2305#discussion_r494686819 ## File path: apisix/plugins/kafka-logger.lua ## @@ -82,6 +84,11 @@ local function send_kafka_data(conf, log_message) end broker_config["request_timeout"] = conf.timeout * 1000 +broker_config["partitioner"] = function(key, num, correlation_id) Review comment: we can use "TEST 4" and "TEST 5" of kafka-logger.t test it，and number of partitions for a kafka topic >= 2. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] liuxiran commented on a change in pull request #512: fix: update valify rules about whether enabled auth plugin in create/…
liuxiran commented on a change in pull request #512:
URL: https://github.com/apache/apisix-dashboard/pull/512#discussion_r494464572
##
File path: src/pages/Consumer/Create.tsx
##
@@ -68,8 +68,18 @@ const Page: React.FC = (props) => {
setStep(nextStep);
});
} else if (nextStep === 3) {
- const isValid = Object.keys(plugins).some((name) =>
name.includes('auth'));
- if (!isValid) {
+ const authPluginNames = [
Review comment:
got it~thanks, already done
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:
us...@infra.apache.org
[GitHub] [apisix] membphis opened a new pull request #2309: feat(http-logger): support for specified the log formats via admin API
membphis opened a new pull request #2309: URL: https://github.com/apache/apisix/pull/2309 This reverts commit 13b09683400e41529258fe21f013668880a6a5ba. ### What this PR does / why we need it: related PR: https://github.com/apache/apisix/pull/2307 let us discuss it in this PR. ### 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? 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan edited a comment on issue #511: using dashboard for a while , the token expired
juzhiyuan edited a comment on issue #511: URL: https://github.com/apache/apisix-dashboard/issues/511#issuecomment-698431701 There have 2 ways to implement this feature: 1. Implement this feature on the backend, so if the token should expire is rely on some cache DB, e.g Redis. 2. JWT with Refresh logic, every refresh action will generate a new token. ## Reference [1] https://developers.weixin.qq.com/doc/oplatform/Website_App/WeChat_Login/Authorized_Interface_Calling_UnionID.html 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #511: using dashboard for a while , the token expired
juzhiyuan commented on issue #511: URL: https://github.com/apache/apisix-dashboard/issues/511#issuecomment-698431701 There have 2 ways to implement this feature: 1. Implement this feature on the backend, so if the token should expire is rely on some cache DB, e.g Redis. 2. JWT with Refresh logic, just like Wechat OAuth[1] does. [1] https://developers.weixin.qq.com/doc/oplatform/Website_App/WeChat_Login/Authorized_Interface_Calling_UnionID.html 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: us...@infra.apache.org
[GitHub] [apisix] membphis merged pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis merged pull request #2307: URL: https://github.com/apache/apisix/pull/2307 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: us...@infra.apache.org
[apisix] branch master updated: Revert "feat(http-logger): support for specified the log formats via admin API (#2294)" (#2307)
This is an automated email from the ASF dual-hosted git repository.
membphis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push:
new 13b0968 Revert "feat(http-logger): support for specified the log
formats via admin API (#2294)" (#2307)
13b0968 is described below
commit 13b09683400e41529258fe21f013668880a6a5ba
Author: Wen Ming
AuthorDate: Thu Sep 24 23:50:22 2020 +0800
Revert "feat(http-logger): support for specified the log formats via admin
API (#2294)" (#2307)
This reverts commit 89f89f30044e12c04b67508623d0b4a85cb77709.
---
apisix/core/table.lua| 2 +-
apisix/plugin.lua| 16
apisix/plugins/http-logger.lua | 83 +++-
bin/apisix | 2 +-
doc/zh-cn/plugins/http-logger.md | 28 +--
t/node/route-domain-with-local-dns.t | 4 +-
t/node/route-domain.t| 6 +-
t/plugin/http-logger-log-format.t| 147 ---
8 files changed, 17 insertions(+), 271 deletions(-)
diff --git a/apisix/core/table.lua b/apisix/core/table.lua
index 2324840..4ad92ca 100644
--- a/apisix/core/table.lua
+++ b/apisix/core/table.lua
@@ -26,6 +26,7 @@ local ngx_re = require("ngx.re")
local _M = {
+version = 0.2,
new = new_tab,
clear = require("table.clear"),
nkeys = nkeys,
@@ -34,7 +35,6 @@ local _M = {
sort= table.sort,
clone = require("table.clone"),
isarray = require("table.isarray"),
-empty_tab = {},
}
diff --git a/apisix/plugin.lua b/apisix/plugin.lua
index c2d40ce..8c49883 100644
--- a/apisix/plugin.lua
+++ b/apisix/plugin.lua
@@ -25,7 +25,6 @@ local type = type
local local_plugins = core.table.new(32, 0)
local ngx = ngx
local tostring = tostring
-local error = error
local local_plugins_hash= core.table.new(0, 32)
local stream_local_plugins = core.table.new(32, 0)
local stream_local_plugins_hash = core.table.new(0, 32)
@@ -392,21 +391,6 @@ end
function _M.init_worker()
_M.load()
-
-local plugin_metadatas, err = core.config.new("/plugin_metadata",
-{automatic = true}
-)
-if not plugin_metadatas then
-error("failed to create etcd instance for fetching /plugin_metadatas :
"
- .. err)
-end
-
-_M.plugin_metadatas = plugin_metadatas
-end
-
-
-function _M.plugin_metadata(name)
-return _M.plugin_metadatas:get(name)
end
diff --git a/apisix/plugins/http-logger.lua b/apisix/plugins/http-logger.lua
index a483ab1..4694b60 100644
--- a/apisix/plugins/http-logger.lua
+++ b/apisix/plugins/http-logger.lua
@@ -14,25 +14,16 @@
-- See the License for the specific language governing permissions and
-- limitations under the License.
--
-
+local core = require("apisix.core")
+local log_util = require("apisix.utils.log-util")
local batch_processor = require("apisix.utils.batch-processor")
-local log_util= require("apisix.utils.log-util")
-local core= require("apisix.core")
-local http= require("resty.http")
-local url = require("net.url")
-local plugin = require("apisix.plugin")
-local ngx = ngx
-local tostring = tostring
-local pairs= pairs
-local ipairs = ipairs
-
-
local plugin_name = "http-logger"
+local ngx = ngx
+local tostring = tostring
+local http = require "resty.http"
+local url = require "net.url"
local buffers = {}
-local lru_log_format = core.lrucache.new({
-ttl = 300, count = 512
-})
-
+local ipairs = ipairs
local schema = {
type = "object",
@@ -54,24 +45,11 @@ local schema = {
}
-local metadata_schema = {
-type = "object",
-properties = {
-log_format = {
-type = "object",
-default = {},
-},
-},
-additionalProperties = false,
-}
-
-
local _M = {
version = 0.1,
priority = 410,
name = plugin_name,
schema = schema,
-metadata_schema = metadata_schema,
}
@@ -139,51 +117,8 @@ local function send_http_data(conf, log_message)
end
-local function gen_log_format(metadata)
-local log_format = core.table.empty_tab
-if not metadata or
- not metadata.value or
- not metadata.value.log_format or
- core.table.nkeys(metadata.value.log_format) == 0
-then
-return log_format
-end
-
-for k, var_name in pairs(metadata.value.log_format) do
-if var_name:sub(1, 1) == "$" then
-log_format[k] = {true, var_name:sub(2)}
-else
-log_format[k] = {false, var_name}
-end
-end
-core.log.info("log_format: ", core.json.delay_encode(log_format))
-return log_format
-end
-
-
-function _M.log(conf, ctx)
-local metadata = plugin.plugin_metadata(plugin_name)
-core.log.info("metadata: ", core.json.delay_encode(metadata))
-
-local entry
-
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #500: Failed to compile dashboard
juzhiyuan commented on issue #500: URL: https://github.com/apache/apisix-dashboard/issues/500#issuecomment-698428810 Still don't know how to resolve this issue :( @MrLightSpeed0 Could you please provide a detailed installation steps? so I could try to reproduce this issue. 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on issue #2308: request help: Provide the ability to view plug-in types
membphis commented on issue #2308: URL: https://github.com/apache/apisix/issues/2308#issuecomment-698427606 that is a useful feature, we can let APISIX support this 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: us...@infra.apache.org
[GitHub] [apisix] juzhiyuan commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
juzhiyuan commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698426741 > > Got it, we could revert it first, once most of the members agree to accept it, we could do a re-revert. > > I do not agree with it now. if we want to revert, we should to confirm the old PR is wong. Revert that PR is not because THAT feature has something wrong, but just because there has some arguments about merging that PR, once most of the members accpet it, we could re-revert 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] gxthrj opened a new issue #2308: request help: Provide the ability to view plug-in types
gxthrj opened a new issue #2308: URL: https://github.com/apache/apisix/issues/2308 ### Issue description some auth plugins has a type named "auth", Identifies that this plugin is used for authentication  I need to know the type of plugin, so can make some judgments based on the type. ### Environment * apisix version: 1.5 * OS: 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on issue #2303: Proposal: inptroduce traffic split plugin
moonming commented on issue #2303: URL: https://github.com/apache/apisix/issues/2303#issuecomment-698422004 Good job It will be better if you can send this proposal to mailing list too 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
moonming commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698415397 > > Got it, we could revert it first, once most of the members agree to accept it, we could do a re-revert. > > > > I do not agree with it now. if we want to revert, we should to confirm the old PR is wong. Revert this pr is not because of this feature, but because this operation will affect review. 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698404228 > @moonming Different routes may have different log formats that is not a good idea, another reason: we also need to collect the access log of request when not matched any route. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] nic-chen commented on issue #500: Failed to compile dashboard
nic-chen commented on issue #500: URL: https://github.com/apache/apisix-dashboard/issues/500#issuecomment-698364579 > Hello you said the dial tcp error was related to my network. I will check on that. that is the for question #3 (build with docker). what about #2, could you please help on that? copy it here for your quick reference: > > #2. I tried to build dashboard, go build needs to import following: > "github.com/apisix/manager-api/conf" > "github.com/apisix/manager-api/log" > "github.com/apisix/manager-api/route" > However I kept getting the errors saying those cannot found. I even tried to do, say: go get github.com/apisix/manager-api/conf, etc. they could not be found. I searched the github and unable to find them. Please advise on this. `github.com/apisix/manager-api` is `apisix-dashboard/api` please take a look at : https://github.com/apache/apisix-dashboard/blob/master/api/go.mod#L1 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
nic-chen commented on a change in pull request #2301: URL: https://github.com/apache/apisix/pull/2301#discussion_r494339821 ## File path: doc/plugins/hmac-auth.md ## @@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13 ## Test Plugin ### generate signature: -The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string` +The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string` 1. **HTTP Method** : Refers to the GET, PUT, POST and other request methods defined in the HTTP protocol, and must be in all uppercase. 2. **HTTP URI** : `HTTP URI` requirements must start with "/", those that do not start with "/" need to be added, and the empty path is "/". -3. **canonical_query_string** :`canonical_query_string` is the result of encoding the `query` in the URL (`query` is the string "key1 = valve1 & key2 = valve2" after the "?" in the URL). -4. **signed_headers_string** :`signed_headers_string` is the result of obtaining the fields specified by the client from the request header and concatenating the strings in order. +3. **HTTP Date** : Date and time string in GMT format. 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: us...@infra.apache.org
[GitHub] [apisix] ShiningRush commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
ShiningRush commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698360162 We can concern a meaningful name, `plugin_metadata` is just a global configuration cross all plugins, not real `meta data`. 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
nic-chen commented on a change in pull request #2301: URL: https://github.com/apache/apisix/pull/2301#discussion_r494339665 ## File path: t/plugin/custom_hmac_auth.t ## @@ -210,17 +211,17 @@ X-APISIX-HMAC-ACCESS-KEY: sdf -=== TEST 8: verify: invalid timestamp +=== TEST 8: verify: Invalid GMT format time --- request GET /hello --- more_headers X-APISIX-HMAC-SIGNATURE: asdf X-APISIX-HMAC-ALGORITHM: hmac-sha256 -X-APISIX-HMAC-TIMESTAMP: 112 +Date: Thu, 24 Sep 2020 06:39:52 GMT Review comment: not sensitive. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] membphis commented on issue #511: using dashboard for a while , the token expired
membphis commented on issue #511: URL: https://github.com/apache/apisix-dashboard/issues/511#issuecomment-698360031 Is the refresh of the token done on the front end? Or `manage API`? 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698356108 > Got it, we could revert it first, once most of the members agree to accept it, we could do a re-revert. I do not agree with it now. if we want to revert, we should to confirm the old PR is wong. 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307:
URL: https://github.com/apache/apisix/pull/2307#issuecomment-698354733
> I think it may be easier to understand if it is called `ShareData` or
other meaningful name.
that is the another PR which implemented the `plugin_metadata`:
https://github.com/apache/apisix/pull/2268
`/apisix/plugin_metadata/http-logger` -> `{log_format: {}}`
this value only can be used in plugin `http-logger`, it can not be used for
other plugin.
so I think `ShareData` is not a good name for this case.
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:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698350706 > The branch was merged without replying to the PMC member’s question [#2294 (comment)](https://github.com/apache/apisix/pull/2294#issuecomment-698108183) [#2294 (comment)](https://github.com/apache/apisix/pull/2294#discussion_r494134861) , so I ask to revert this PR. have to say sorry for missing the comments. I submitted lots of commits when I was developing. As a result, I did not see your message on the PR :( . 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698348564 @ShiningRush > I am also curious about what scenarios need metadata instead of plugin's option. Here is the answer: https://github.com/apache/apisix/pull/2307#issuecomment-698337991 BTW, here is the old way to support the default value for log-rotate plugin: https://github.com/apache/apisix/blob/master/conf/config-default.yaml#L199-L202 this way is not firendly: 1. local file, if we want to update it, we need to update the file and reload APISIX instance. 2. no schema, the user may set a wong value. 3. each plugin needs to check the default value if it is right by itself. but the `/apisix/plugin_metadata` fixed all of them, it is a standard way to set a default value for each plugin. 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: us...@infra.apache.org
[GitHub] [apisix] ShiningRush commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
ShiningRush commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698343525 I got it, this is a feature for sharing data in the same type of plugin. I think it is useful for users. For example, a plugin needs to use the redis connection string, but I don’t want to configure it in each plugin.this function is very helpful at this time. However, the name may cause some misunderstandings. I think it may be easier to understand if it is called `ShareData` or other meaningful name. 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2294: feat(http-logger): support for specified the log formats via admin API
membphis commented on a change in pull request #2294:
URL: https://github.com/apache/apisix/pull/2294#discussion_r494311698
##
File path: bin/apisix
##
@@ -1025,7 +1025,7 @@ local function init_etcd(show_output)
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
- "/proto"}) do
+ "/proto", "/plugin_metadata"}) do
Review comment:
this PR has been merged. we can talk in here:
https://github.com/apache/apisix/pull/2307#issuecomment-698337991
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:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698339660 > https://github.com/apache/apisix/pull/2294#discussion_r494134861 > why we need this PR? https://github.com/apache/apisix/pull/2294#issue-492080706 I have wrote the description for this PR. please take a look first. And I posted the use case. 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
membphis commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698337991 > https://github.com/apache/apisix/pull/2294#issuecomment-698108183 sorry for missing them. I reply to you here. > why not add log_format in http-logger plugin directly? the `plugin_metadata` can be the default value of plugin, and it supports a way we can control it by admin api. > Different routes may have different log formats That is not a good way. Like `Nginx`, only one `access.log`, all of the requests will generate access log to `access.log`. I do not think our user need this feature. 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
moonming commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698337447 @juzhiyuan @ShiningRush I think we should revert this PR first, then revert this revert if needed. This has nothing to do with the feature, but about how to merge PR. I think it is impolite to ignore the comments and merge directly. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on pull request #512: fix: update valify rules about whether enabled auth plugin in create/…
juzhiyuan commented on pull request #512: URL: https://github.com/apache/apisix-dashboard/pull/512#issuecomment-698336453 I noticed the response from that snapshot, yes, we only allow to enable 1 plugin. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #512: fix: update valify rules about whether enabled auth plugin in create/…
juzhiyuan commented on a change in pull request #512:
URL: https://github.com/apache/apisix-dashboard/pull/512#discussion_r494302019
##
File path: src/pages/Consumer/Create.tsx
##
@@ -68,8 +68,18 @@ const Page: React.FC = (props) => {
setStep(nextStep);
});
} else if (nextStep === 3) {
- const isValid = Object.keys(plugins).some((name) =>
name.includes('auth'));
- if (!isValid) {
+ const authPluginNames = [
+'openid-connect',
+'basic-auth',
+'jwt-auth',
+'key-auth',
+'authz-keycloak',
+ ];
+ const currentAuthPlugin = Object.keys(plugins).filter((plugin) =>
+authPluginNames.includes(plugin),
+ );
+ const currentAuthPluginLen = currentAuthPlugin.length;
+ if (currentAuthPluginLen > 1 || currentAuthPluginLen === 0) {
Review comment:
Maybe there have some mistakes here? I think this message will only
popup when there has no 1 auth plugin :D
##
File path: src/pages/Consumer/Create.tsx
##
@@ -68,8 +68,18 @@ const Page: React.FC = (props) => {
setStep(nextStep);
});
} else if (nextStep === 3) {
- const isValid = Object.keys(plugins).some((name) =>
name.includes('auth'));
- if (!isValid) {
+ const authPluginNames = [
Review comment:
oh my mistake, you could import an plugin array from
https://github.com/api7/dashboard-components/blob/master/packages/plugin/src/data.tsx#L4
##
File path: src/pages/Consumer/Create.tsx
##
@@ -68,8 +68,18 @@ const Page: React.FC = (props) => {
setStep(nextStep);
});
} else if (nextStep === 3) {
- const isValid = Object.keys(plugins).some((name) =>
name.includes('auth'));
- if (!isValid) {
+ const authPluginNames = [
Review comment:
```js
import {PLUGIN_MAPPER_SOURCE} from '@api7-dashboard/plugin;
```
https://github.com/api7/dashboard-components/blob/master/packages/plugin/src/data.tsx#L37
##
File path: src/pages/Consumer/Create.tsx
##
@@ -68,8 +68,18 @@ const Page: React.FC = (props) => {
setStep(nextStep);
});
} else if (nextStep === 3) {
- const isValid = Object.keys(plugins).some((name) =>
name.includes('auth'));
- if (!isValid) {
+ const authPluginNames = [
+'openid-connect',
+'basic-auth',
+'jwt-auth',
+'key-auth',
+'authz-keycloak',
+ ];
+ const currentAuthPlugin = Object.keys(plugins).filter((plugin) =>
+authPluginNames.includes(plugin),
+ );
+ const currentAuthPluginLen = currentAuthPlugin.length;
+ if (currentAuthPluginLen > 1 || currentAuthPluginLen === 0) {
Review comment:
let's invite @membphis @moonming to check this condition :D
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:
us...@infra.apache.org
[GitHub] [apisix] ShiningRush commented on a change in pull request #2294: feat(http-logger): support for specified the log formats via admin API
ShiningRush commented on a change in pull request #2294:
URL: https://github.com/apache/apisix/pull/2294#discussion_r494304368
##
File path: bin/apisix
##
@@ -1025,7 +1025,7 @@ local function init_etcd(show_output)
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
- "/proto"}) do
+ "/proto", "/plugin_metadata"}) do
Review comment:
I am also curious about what scenarios need metadata instead of plugin's
option.
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:
us...@infra.apache.org
[GitHub] [apisix] ShiningRush commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
ShiningRush commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698334232 We can continue to discuss its necessity in the original PR If it is a non-essential function point, we could revert 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] zlm0125 commented on a change in pull request #2306: 1.fix generation of uuid;
zlm0125 commented on a change in pull request #2306:
URL: https://github.com/apache/apisix/pull/2306#discussion_r494302867
##
File path: apisix/init.lua
##
@@ -107,6 +107,10 @@ function _M.http_init_worker()
require("apisix.debug").init_worker()
require("apisix.upstream").init_worker()
+-- need call seed
+local uuid = require("resty.jit-uuid")
+uuid.seed()
Review comment:
But in k8s, it not work very well.
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:
us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #513: Dashboard build failed on import /manager-api step
juzhiyuan commented on issue #513: URL: https://github.com/apache/apisix-dashboard/issues/513#issuecomment-698331940 duplicate with https://github.com/apache/apisix-dashboard/issues/500 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan closed issue #513: Dashboard build failed on import /manager-api step
juzhiyuan closed issue #513: URL: https://github.com/apache/apisix-dashboard/issues/513 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] juzhiyuan commented on issue #500: Failed to compile dashboard
juzhiyuan commented on issue #500: URL: https://github.com/apache/apisix-dashboard/issues/500#issuecomment-698331203 > Hello you said the dial tcp error was related to my network. I will check on that. that is the for question #3 (build with docker). what about #2, could you please help on that? copy it here for your quick reference: > > #2. I tried to build dashboard, go build needs to import following: > "github.com/apisix/manager-api/conf" > "github.com/apisix/manager-api/log" > "github.com/apisix/manager-api/route" > However I kept getting the errors saying those cannot found. I even tried to do, say: go get github.com/apisix/manager-api/conf, etc. they could not be found. I searched the github and unable to find them. Please advise on this. @nic-chen Please take a look at this one. 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: us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2306: 1.fix generation of uuid;
membphis commented on a change in pull request #2306:
URL: https://github.com/apache/apisix/pull/2306#discussion_r494299962
##
File path: apisix/init.lua
##
@@ -107,6 +107,10 @@ function _M.http_init_worker()
require("apisix.debug").init_worker()
require("apisix.upstream").init_worker()
+-- need call seed
+local uuid = require("resty.jit-uuid")
+uuid.seed()
Review comment:
https://github.com/apache/apisix/blob/master/apisix/init.lua#L72
We have set the seed with a better way. So we do not need to set it agian.
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:
us...@infra.apache.org
[GitHub] [apisix] juzhiyuan commented on pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
juzhiyuan commented on pull request #2307: URL: https://github.com/apache/apisix/pull/2307#issuecomment-698330736 Not sure if that PR is needed, maybe there should ping @membphis. 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: us...@infra.apache.org
[GitHub] [apisix] juzhiyuan commented on a change in pull request #2294: feat(http-logger): support for specified the log formats via admin API
juzhiyuan commented on a change in pull request #2294:
URL: https://github.com/apache/apisix/pull/2294#discussion_r494297836
##
File path: bin/apisix
##
@@ -1025,7 +1025,7 @@ local function init_etcd(show_output)
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
- "/proto"}) do
+ "/proto", "/plugin_metadata"}) do
Review comment:
I would recommend using `/plugins/{name}/metadat` in a RESTful way if
this PR is needed..
##
File path: bin/apisix
##
@@ -1025,7 +1025,7 @@ local function init_etcd(show_output)
for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
"/plugins", "/consumers", "/node_status",
"/ssl", "/global_rules", "/stream_routes",
- "/proto"}) do
+ "/proto", "/plugin_metadata"}) do
Review comment:
I would recommend using `/plugins/{name}/metadata` in a RESTful way if
this PR is needed..
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:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2265: doc: `consumer-restriction` plug-in document changes
membphis commented on a change in pull request #2265:
URL: https://github.com/apache/apisix/pull/2265#discussion_r494297020
##
File path: doc/zh-cn/plugins/consumer-restriction.md
##
@@ -87,31 +93,138 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H
'X-API-KEY: edd1c9f034335f13
}'
```
-## 测试插件
+**测试插件**
jack1 访问:
```shell
-$ curl -u jack2019:123456 http://127.0.0.1:9080/index.html
+curl -u jack2019:123456 http://127.0.0.1:9080/index.html -i
HTTP/1.1 200 OK
...
```
jack2 访问:
```shell
-$ curl -u jack2020:123456 http://127.0.0.1:9080/index.html -i
+curl -u jack2020:123456 http://127.0.0.1:9080/index.html -i
HTTP/1.1 403 Forbidden
...
-{"message":"You are not allowed"}
+{"message":"The consumer_name is forbidden."}
+```
+
+### 如何限制 `service_id`
+`service_id`方式需要与授权插件一起配合使用，这里以key-auth授权插件为例。
+
+1、创建两个 service
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/services/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"desc": "new service 001"
+}'
+
+curl http://127.0.0.1:9080/apisix/admin/services/2 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"desc": "new service 002"
+}'
+```
+
+2、在 `consumer` 上绑定 `consumer-restriction` 插件(需要与一个授权插件配合才能绑定),并添加 `service_id`
白名单列表
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"username": "new_consumer",
+"plugins": {
+"key-auth": {
+"key": "auth-jack"
+},
+"consumer-restriction": {
+ "type": "service_id",
+"whitelist": [
+"1"
+],
+"rejected_code": 403
+}
+}
+}'
+```
+
+3、在 route 上开启 `key-auth` 插件并绑定 `service_id` 为`1`
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"uri": "/index.html",
+"upstream": {
+"type": "roundrobin",
+"nodes": {
+"127.0.0.1:1980": 1
+}
+},
+"service_id": 1,
+"plugins": {
+ "key-auth": {
+}
+}
+}'
+```
+
+**测试插件**
Review comment:
Add a brief description of what is expected.
##
File path: doc/zh-cn/plugins/consumer-restriction.md
##
@@ -87,31 +93,138 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H
'X-API-KEY: edd1c9f034335f13
}'
```
-## 测试插件
+**测试插件**
jack1 访问:
```shell
-$ curl -u jack2019:123456 http://127.0.0.1:9080/index.html
+curl -u jack2019:123456 http://127.0.0.1:9080/index.html -i
HTTP/1.1 200 OK
...
```
jack2 访问:
```shell
-$ curl -u jack2020:123456 http://127.0.0.1:9080/index.html -i
+curl -u jack2020:123456 http://127.0.0.1:9080/index.html -i
HTTP/1.1 403 Forbidden
...
-{"message":"You are not allowed"}
+{"message":"The consumer_name is forbidden."}
+```
+
+### 如何限制 `service_id`
+`service_id`方式需要与授权插件一起配合使用，这里以key-auth授权插件为例。
+
+1、创建两个 service
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/services/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"desc": "new service 001"
+}'
+
+curl http://127.0.0.1:9080/apisix/admin/services/2 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"upstream": {
+"nodes": {
+"127.0.0.1:1980": 1
+},
+"type": "roundrobin"
+},
+"desc": "new service 002"
+}'
+```
+
+2、在 `consumer` 上绑定 `consumer-restriction` 插件(需要与一个授权插件配合才能绑定),并添加 `service_id`
白名单列表
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"username": "new_consumer",
+"plugins": {
+"key-auth": {
+"key": "auth-jack"
+},
+"consumer-restriction": {
+ "type": "service_id",
+"whitelist": [
+"1"
+],
+"rejected_code": 403
+}
+}
+}'
+```
+
+3、在 route 上开启 `key-auth` 插件并绑定 `service_id` 为`1`
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"uri": "/index.html",
+"upstream": {
+"type": "roundrobin",
+"nodes": {
+"127.0.0.1:1980": 1
+}
+},
+"service_id": 1,
+"plugins": {
+ "key-auth": {
+}
+}
+}'
+```
+
+**测试插件**
+
+```shell
+curl http://127.0.0.1:9080/index.html -H 'apikey: auth-jack' -i
+HTTP/1.1 200 OK
+...
+```
+
+4、在 route 上开启 `key-auth` 插件并绑定 `service_id` 为`2`
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"uri":
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494293766
##
File path: apisix/plugins/hmac-auth.lua
##
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
-local diff = abs(ngx_time() - params.timestamp)
-core.log.info("timestamp diff: ", diff)
-if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+local time = ngx.parse_http_time(params.date)
Review comment:
This makes it easy to copy requests, bypassing the `clock_skew` option.
I don't think it can be empty here.
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:
us...@infra.apache.org
[GitHub] [apisix] moonming opened a new pull request #2307: Revert "feat(http-logger): support for specified the log formats via …
moonming opened a new pull request #2307: URL: https://github.com/apache/apisix/pull/2307 …admin API (#2294)" This reverts commit 89f89f30044e12c04b67508623d0b4a85cb77709. The branch was merged without replying to the PMC member’s question https://github.com/apache/apisix/pull/2294#issuecomment-698108183 https://github.com/apache/apisix/pull/2294#discussion_r494134861 , so I ask to revert this PR. ### What this PR does / why we need it: ### 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? 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: us...@infra.apache.org
[GitHub] [apisix] membphis merged pull request #2294: feat(http-logger): support for specified the log formats via admin API
membphis merged pull request #2294: URL: https://github.com/apache/apisix/pull/2294 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] MrLightSpeed0 opened a new issue #513: Dashboard build failed on import /manager-api step
MrLightSpeed0 opened a new issue #513: URL: https://github.com/apache/apisix-dashboard/issues/513 Please answer these questions before submitting your issue. - Why do you submit this issue? - [X] Question or discussion - [ ] Bug - [ ] Requirements - [ ] Feature or performance improvement - [ ] Other ___ ### Question This is related to ticket number 500, someone closed it but I didnt get the answer. copying it here for reference: #2. I tried to build dashboard, go build needs to import following: "github.com/apisix/manager-api/conf" "github.com/apisix/manager-api/log" "github.com/apisix/manager-api/route" However I kept getting the errors saying those cannot found. I even tried to do, say: go get github.com/apisix/manager-api/conf, etc. they could not be found. I searched the github and unable to find them. Please advise on this. Thank you ___ ### Bug - Which version of Apache APISIX Dashboard, OS, and Browser? - What happened? If possible, provide a way to reproduce the error. ___ ### Requirement or improvement - Please describe your requirements or improvement suggestions. 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: us...@infra.apache.org
[apisix] branch master updated (3f9685f -> 89f89f3)
This is an automated email from the ASF dual-hosted git repository.
membphis pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/apisix.git.
from 3f9685f feat(http-logger): support to concat multiple log with
separator. (#2286)
add 89f89f3 feat(http-logger): support for specified the log formats via
admin API (#2294)
No new revisions were added by this update.
Summary of changes:
apisix/core/table.lua | 2 +-
apisix/plugin.lua | 16 +++
apisix/plugins/http-logger.lua | 83 +--
bin/apisix | 2 +-
doc/zh-cn/plugins/http-logger.md | 28 -
t/node/route-domain-with-local-dns.t | 4 +-
t/node/route-domain.t | 6 +-
.../http-logger-log-format.t} | 117 +
8 files changed, 172 insertions(+), 86 deletions(-)
copy t/{node/remote-addr.t => plugin/http-logger-log-format.t} (55%)
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feature: support `consumer_name` as key of `limit-req` plugin.
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494284498
##
File path: apisix/plugins/limit-req.lua
##
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
-local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+local key
+if conf.key == "consumer_name" then
+if not ctx.consumer_id then
+core.log.error("Missing consumer's username.")
Review comment:
sorry, it's fixed now.
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:
us...@infra.apache.org
[GitHub] [apisix-dashboard] MrLightSpeed0 commented on issue #500: Failed to compile dashboard
MrLightSpeed0 commented on issue #500: URL: https://github.com/apache/apisix-dashboard/issues/500#issuecomment-698318131 Hello you said the dial tcp error was related to my network. I will check on that. that is the for question #3 (build with docker). what about #2, could you please help on that? copy it here for your quick reference: #2. I tried to build dashboard, go build needs to import following: "github.com/apisix/manager-api/conf" "github.com/apisix/manager-api/log" "github.com/apisix/manager-api/route" However I kept getting the errors saying those cannot found. I even tried to do, say: go get github.com/apisix/manager-api/conf, etc. they could not be found. I searched the github and unable to find them. Please advise on this. 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
moonming commented on a change in pull request #2301: URL: https://github.com/apache/apisix/pull/2301#discussion_r494270999 ## File path: doc/plugins/hmac-auth.md ## @@ -40,7 +40,7 @@ The `consumer` then adds its key to request header to verify its request. | access_key | string| required| | | Different `consumer` objects should have different values, and it should be unique. If different consumers use the same `access_key`, a request matching exception will occur. | | secret_key | string| required| | | Use as a pair with `access_key`. | | algorithm | string| optional| "hmac-sha256" | ["hmac-sha1", "hmac-sha256", "hmac-sha512"] | Encryption algorithm. | -| clock_skew | integer | optional| 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking timestamp | +| clock_skew | integer | optional| 300 | | The clock skew allowed by the signature in seconds. For example, if the time is allowed to skew by 10 seconds, then it should be set to `10`. especially, `0` means not checking `HTTP Date` | Review comment: `HTTP Date` -> `Date` ## File path: doc/plugins/hmac-auth.md ## @@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13 ## Test Plugin ### generate signature: -The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string` +The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + HTTP Date + signed_headers_string` Review comment: HTTP Date -> Date ## File path: t/plugin/custom_hmac_auth.t ## @@ -210,17 +211,17 @@ X-APISIX-HMAC-ACCESS-KEY: sdf -=== TEST 8: verify: invalid timestamp +=== TEST 8: verify: Invalid GMT format time --- request GET /hello --- more_headers X-APISIX-HMAC-SIGNATURE: asdf X-APISIX-HMAC-ALGORITHM: hmac-sha256 -X-APISIX-HMAC-TIMESTAMP: 112 +Date: Thu, 24 Sep 2020 06:39:52 GMT Review comment: Need to test whether uppercase and lowercase letters are sensitive ## File path: doc/plugins/hmac-auth.md ## @@ -83,12 +83,13 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f13 ## Test Plugin ### generate signature: -The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP URI + canonical_query_string + access_key + timestamp + signed_headers_string` +The calculation formula of the signature is `signature = HMAC-SHAx-HEX(secret_key, signing_string)`. From the formula, it can be seen that in order to obtain the signature, two parameters, `SECRET_KEY` and `signing_STRING`, are required. Where secret_key is configured by the corresponding consumer, the calculation formula of `signing_STRING` is `signing_string = HTTP Method + HTTP
[GitHub] [apisix] moonming commented on a change in pull request #2270: feature: support `consumer_name` as key of `limit-req` plugin.
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494267003
##
File path: apisix/plugins/limit-req.lua
##
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
-local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+local key
+if conf.key == "consumer_name" then
+if not ctx.consumer_id then
+core.log.error("Missing consumer's username.")
Review comment:
still not fix
##
File path: apisix/plugins/limit-req.lua
##
@@ -67,7 +67,17 @@ function _M.access(conf, ctx)
return 500
end
-local key = (ctx.var[conf.key] or "") .. ctx.conf_type .. ctx.conf_version
+local key
+if conf.key == "consumer_name" then
+if not ctx.consumer_id then
+core.log.error("Missing consumer's username.")
Review comment:
take a look:
https://github.com/apache/apisix/pull/2270#discussion_r494019356
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:
us...@infra.apache.org
[GitHub] [apisix] zlm0125 opened a new pull request #2306: 1.fix generation of uuid;
zlm0125 opened a new pull request #2306: URL: https://github.com/apache/apisix/pull/2306 Change-Id: Iff73ad95f87a81bc66ecc26e9ab250a8ca41ebf6 ### What this PR does / why we need it: ### 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? 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494265478
##
File path: doc/plugins/limit-req.md
##
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together
with the authorization plugin. Here, the key-auth authorization plugin is taken
as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"username": "consumer_jack",
+"plugins": {
+"key-auth": {
+"key": "auth-jack"
+},
+"limit-req": {
+"rate": 1,
+"burst": 1,
+"rejected_code": 403,
+"key": "consumer_name"
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2305: fix: kafka-logger plugin sent to the same partition everytime
moonming commented on a change in pull request #2305: URL: https://github.com/apache/apisix/pull/2305#discussion_r494259548 ## File path: apisix/plugins/kafka-logger.lua ## @@ -82,6 +84,11 @@ local function send_kafka_data(conf, log_message) end broker_config["request_timeout"] = conf.timeout * 1000 +broker_config["partitioner"] = function(key, num, correlation_id) Review comment: It will be better if you can add test cases to cover. 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2304: fix: Update error message when Route doesn't exist。
moonming commented on pull request #2304: URL: https://github.com/apache/apisix/pull/2304#issuecomment-698299746 @liuhengloveyou please check the commit mesg again, contains Chinese chars. 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: us...@infra.apache.org
[GitHub] [apisix-dashboard] liuxiran opened a new pull request #512: fix: update valify rules about whether enabled auth plugin in create/…
liuxiran opened a new pull request #512: URL: https://github.com/apache/apisix-dashboard/pull/512 …update a consumer Please answer these questions before submitting a pull request - Why submit this pull request? - [ ] Bug fix - [ ] New feature provided - [ ] Improve performance - Related issues fix: #509 ___ ### Bugfix - Description - How to fix? update valify rules to create/update a consumer must enable one of the five auth plugins. (when enables two or above auth plugins , it will return a error from Apisix: `only one auth plugin is allowed`   ) ___ ### New feature or improvement - Describe the details and related test reports. 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: us...@infra.apache.org
[GitHub] [apisix] liuhengloveyou commented on pull request #2304: fix: Update error message when Route doesn't exist。
liuhengloveyou commented on pull request #2304: URL: https://github.com/apache/apisix/pull/2304#issuecomment-698255684 PS: whether the error message needs to be prefixed or not, to distinguish between an apisix error and a business error. 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494199196
##
File path: apisix/plugins/hmac-auth.lua
##
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
-local diff = abs(ngx_time() - params.timestamp)
-core.log.info("timestamp diff: ", diff)
-if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+local time = ngx.parse_http_time(params.date)
Review comment:
we allow it to be empty here
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:
us...@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2304: "failed to match any routes" to "404 Route Not Found"
membphis commented on pull request #2304: URL: https://github.com/apache/apisix/pull/2304#issuecomment-698249241 bad title and commit log style  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: us...@infra.apache.org
[GitHub] [apisix] uptree opened a new pull request #2305: fix: kafka-logger plugin sent to the same partition everytime
uptree opened a new pull request #2305: URL: https://github.com/apache/apisix/pull/2305 ### What this PR does / why we need it: 1. fix: kafka-logger plugin sent to the same partition everytime 2. random select partition_id for init kafka producer ### 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? * [ ] Have you modified the corresponding document? * [x] Is this PR backward compatible? 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: us...@infra.apache.org
[GitHub] [apisix] liuhengloveyou opened a new pull request #2304: "failed to match any routes" to "404 Route Not Found"
liuhengloveyou opened a new pull request #2304: URL: https://github.com/apache/apisix/pull/2304 ### What this PR does / why we need it: Fixes #2302 ### 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? * [ ] Have you modified the corresponding document? * [x] Is this PR backward compatible? 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: us...@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #2279: feat: Add labels for upstream object
tokers commented on pull request #2279: URL: https://github.com/apache/apisix/pull/2279#issuecomment-698245062 @imjoey @gxthrj @nic-chen @membphis @moonming I have submitted an issue about traffic split https://github.com/apache/apisix/issues/2303. 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: us...@infra.apache.org
[GitHub] [apisix] tokers opened a new issue #2303: Proposal: inptroduce traffic split plugin
tokers opened a new issue #2303:
URL: https://github.com/apache/apisix/issues/2303
### Background
I observed the Pull Request https://github.com/apache/apisix/pull/2279 adds
labels for `upstream` object, which inspires me that if we attach labels for
each node in `upstream`, then we can implement fancy traffic split feature.
### Design
Traffic split, which means to setup several route rules and schedule
requests by respecting these rules to specified upstream (or a subset nodes in
a single upstream). With this feature, we can support the [Canary
Release](https://martinfowler.com/bliki/CanaryRelease.html#:~:text=Canary%20release%20is%20a%20technique,making%20it%20available%20to%20everybody.),
[Blue Green
Release](https://docs.cloudfoundry.org/devguide/deploy-apps/blue-green.html#:~:text=Blue%2Dgreen%20deployment%20is%20a,live%20and%20Green%20is%20idle.)
for backend applications when they are releasing, it's useful to reduce the
downtime when something fault happens.
But it's not a good idea to introduce these concepts into APISIX directly,
since features like Blue Green Release is closer to business, not the
infrastructure, so what we can do is introducing the concept traffic split , to
abstract all the business logics into the traffic split rules (route rules),
and it can be implemented as a plugin.
Both the Istio and [SMI](https://github.com/servicemeshinterface/smi-spec)
support traffic split by `VirtualService` and `TrafficSplit` CRD respectively,
although the latter is not perfect (at lease for now), we still can be aware of
the similarity between them.
In general, we need two parts to define a traffic split rule, match and
route, the `match` defines the conditions that needed to judge wether a request
is eligible to apply the `route`, for instance, a `match` might be "the method
of HTTP request must be `GET`, and the parameter `id` must be equal to `1006`.
From APISIX's point of view, match conditions can be scoped to HTTP, TLS, so
conditions can vary, like URI, method, headers, SNI and even other APISIX
instance-scoped variables (hostname, env and etc).
The APISIX Route already defines a fantastic match mechanism which is
similar with the traffic split. But that not means we don't need to embed the
match part in traffic split plugin, instead, we can reuse the match mechanism
in Route as it's own match mechanism to further split requests that hit the
same Route. See https://github.com/api7/lua-resty-radixtree#new for more
details about Route match.
The route, decides the ultimate upstream that a request will go, either
specifying the upstream name or with a label selector to filter an eligible
subset from that upstream. What's more, multiple upstreams can be specified,
each with a non-negative weight to support probabilistic selection. For now,
node in APISIX Upstream doesn't contain labels attribute, so we can implement
it by the metadata map in node indirectly.
```json
# route requests to the nodes that has label release=canary in fake_upstream,
{
"route": [
{
"upstream_id": "1",
"labels": {
"release": "canary"
}
}
]
}
# route 75% requests to upstream 2 and other 25% to upstream 3.
{
"route": [
{
"upstream_id": "2",
"weight": 75
},
{
"upstream_id": "3",
"weight": 25
}
]
}
```
### Examples
Weighted Blue Green Release
Say we have two upstreams `app-blue` (id: 1) and `app-green` (id: 2), which
represent the old and new releases for `app` respectively. Now we want to route
10% requests which UA is Android to `app-green`.
```json
[
{
"match": [
{
"vars": [
[ "http_user_agent", "~~", "Android"]
]
}
],
"route": [
{
"upstream_id": 2,
"weight": 10
}
]
},
{
"route": [
{
"upstream_id": 1,
"weight": 90
}
]
}
]
```
### Canary Release in a single upstream
Say we updated a node in upstream `app` (id: 3), and this node have a unique
label `release=canary`, other nodes in `app` has label `release=stable`, now we
want to route requests which parameter `user_id` is between [13, 133] to this
node.
```json
[
{
"match": [
{
"vars": [
[ "arg_user_id", ">=", 13],
[ "arg_user_id", "<=", 133]
]
}
],
"route": [
{
[GitHub] [apisix] membphis commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
membphis commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494187478
##
File path: apisix/plugins/hmac-auth.lua
##
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
-local diff = abs(ngx_time() - params.timestamp)
-core.log.info("timestamp diff: ", diff)
-if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+local time = ngx.parse_http_time(params.date)
Review comment:
if the `time` is nil, I think we should throw an error message and write
an error log
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:
us...@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
Firstsawyou commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494181950
##
File path: doc/plugins/limit-req.md
##
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together
with the authorization plugin. Here, the key-auth authorization plugin is taken
as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"username": "consumer_jack",
+"plugins": {
+"key-auth": {
+"key": "auth-jack"
+},
+"limit-req": {
+"rate": 1,
+"burst": 1,
+"rejected_code": 403,
+"key": "consumer_name"
Review comment:
Sorry, my response is not clear. When bound to the `consumer`, the `key`
is optional, and when the `key` is other values (for example:`remote_addr`), it
can be used normally.
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:
us...@infra.apache.org
[GitHub] [apisix-dashboard] idbeta opened a new issue #511: using dashboard for a while , the token expired
idbeta opened a new issue #511: URL: https://github.com/apache/apisix-dashboard/issues/511 Please answer these questions before submitting your issue. - Why do you submit this issue? - [*] Question or discussion - [ ] Bug - [*] Requirements - [ ] Feature or performance improvement - [ ] Other when i login with admin account and using dashboard for a while , the token expired, i think it's not user-friendly... 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: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on issue #2295: request help: Too many connections when using etcdv3
Yiyiyimu commented on issue #2295: URL: https://github.com/apache/apisix/issues/2295#issuecomment-698216556 > how many connections ? > does the number grow over time? Yes the number grow over time. It could grow to 1000+ in my environment. 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: us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
nic-chen commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494154952
##
File path: apisix/plugins/hmac-auth.lua
##
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
-local diff = abs(ngx_time() - params.timestamp)
-core.log.info("timestamp diff: ", diff)
-if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+local time = ngx.parse_http_time(params.date)
Review comment:
it hasn't a err here. if params.date is empty string or other invalid
string, it will return nil.
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:
us...@infra.apache.org
[GitHub] [apisix] Firstsawyou commented on a change in pull request #2301: feat: replace timestamp with date and time in GMT format in plugin `hmac-auth`
Firstsawyou commented on a change in pull request #2301:
URL: https://github.com/apache/apisix/pull/2301#discussion_r494148437
##
File path: apisix/plugins/hmac-auth.lua
##
@@ -246,10 +246,13 @@ local function validate(ctx, params)
core.log.info("clock_skew: ", conf.clock_skew)
if conf.clock_skew and conf.clock_skew > 0 then
-local diff = abs(ngx_time() - params.timestamp)
-core.log.info("timestamp diff: ", diff)
-if diff > conf.clock_skew then
- return nil, {message = "Invalid timestamp"}
+local time = ngx.parse_http_time(params.date)
Review comment:
The value obtained here may be empty, we need to perform error handling,
right?
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:
us...@infra.apache.org
[GitHub] [apisix] nic-chen commented on issue #2295: request help: Too many connections when using etcdv3
nic-chen commented on issue #2295: URL: https://github.com/apache/apisix/issues/2295#issuecomment-698207960 @Yiyiyimu how many connections ? does the number grow over time? 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: us...@infra.apache.org
[GitHub] [apisix] huzhewei commented on issue #2289: request help: stream proxy didn't work
huzhewei commented on issue #2289: URL: https://github.com/apache/apisix/issues/2289#issuecomment-698204147 @Yiyiyimu Thanks~ @membphis Could you please give me a hand? 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: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on issue #2289: request help: stream proxy didn't work
Yiyiyimu commented on issue #2289: URL: https://github.com/apache/apisix/issues/2289#issuecomment-698202205 Sorry I'm not familiar with this part. ping @membphis 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: us...@infra.apache.org
[GitHub] [apisix] moonming commented on a change in pull request #2270: feat: The "limit-req" plugin adds the "consumer_name" method to limit the request speed
moonming commented on a change in pull request #2270:
URL: https://github.com/apache/apisix/pull/2270#discussion_r494136610
##
File path: doc/plugins/limit-req.md
##
@@ -104,6 +106,78 @@ Server: APISIX web server
This means that the limit req plugin is in effect.
+### How to enable on the `consumer`
+
+To enable the `limit-req` plugin on the consumer, it needs to be used together
with the authorization plugin. Here, the key-auth authorization plugin is taken
as an example.
+
+1. Bind the `limit-req` plugin to the consumer
+
+```shell
+curl http://127.0.0.1:9080/apisix/admin/consumers -H 'X-API-KEY:
edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+"username": "consumer_jack",
+"plugins": {
+"key-auth": {
+"key": "auth-jack"
+},
+"limit-req": {
+"rate": 1,
+"burst": 1,
+"rejected_code": 403,
+"key": "consumer_name"
Review comment:
> No, the 'key' enumeration type has no default value.
Does this answer have anything to do with my question?
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:
us...@infra.apache.org
