[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r364561012 ## File path: doc/architecture-design-cn.md ## @@ -338,43 +338,89 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -X PUT -d ' 更多细节可以参考[健康检查的文档](health-check.md)。 下面是几个使用不同`hash_on`类型的配置示例： +1 Consumer +创建一个consumer对象: ```shell -curl http://127.0.0.1:9080/apisix/admin/upstreams/1 -X PUT -d ' +curl http://127.0.0.1:9080/apisix/admin/consumers -X PUT -d ` Review comment: get 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support
more flexible ways
URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363282685
##
File path: lua/apisix/admin/upstreams.lua
##
@@ -45,23 +98,17 @@ local function check_conf(id, conf, need_id)
if need_id and conf.id and tostring(conf.id) ~= tostring(id) then
return nil, {error_msg = "wrong upstream id"}
end
-
core.log.info("schema: ", core.json.delay_encode(core.schema.upstream))
core.log.info("conf : ", core.json.delay_encode(conf))
-local ok, err = core.schema.check(core.schema.upstream, conf)
+local ok, err = check_upstream_conf(conf)
if not ok then
-return nil, {error_msg = "invalid configuration: " .. err}
+return nil, err
Review comment:
```lua
local function check_upstream_conf(conf)
local ok, err = core.schema.check(core.schema.upstream, conf)
if not ok then
return false, {error_msg = "invalid configuration: " .. err}
end
...
```
I move to this function. May be `check_upstream_conf` function return error
string is better,
unified error handling for `check_conf` function.
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
With regards,
Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support
more flexible ways
URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363280855
##
File path: lua/apisix/admin/upstreams.lua
##
@@ -28,6 +28,59 @@ local _M = {
}
+local function get_chash_key_schema(hash_on)
+if not hash_on then
+return nil, "hash_on is nil"
+end
+
+if hash_on == "vars" then
+return core.schema.upstream_hash_vars_schema
+end
+
+if hash_on == "header" or hash_on == "cookie" then
+return core.schema.upstream_hash_header_schema
+end
+
+if hash_on == "consumer" then
+return nil, nil
+end
+
+return nil, "invalid hash_on type " .. hash_on
+end
+
+
+local function check_upstream_conf(conf)
+local ok, err = core.schema.check(core.schema.upstream, conf)
+if not ok then
+return false, {error_msg = "invalid configuration: " .. err}
+end
+
+if conf.type == "chash" then
Review comment:
ok, 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
With regards,
Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support
more flexible ways
URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363278721
##
File path: lua/apisix/balancer.lua
##
@@ -151,11 +184,11 @@ local function create_server_picker(upstream, checker)
end
local picker = resty_chash:new(nodes)
-local key = upstream.key
return {
upstream = upstream,
get = function (ctx)
-local id = picker:find(ctx.var[key])
+local chash_key = create_chash_hash_key(ctx, upstream)
Review comment:
ok, is better
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
With regards,
Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363278540 ## File path: lua/apisix/balancer.lua ## @@ -122,6 +123,38 @@ local function fetch_healthchecker(upstream, healthcheck_parent, version) end +local function create_chash_hash_key(ctx, upstream) +local key = upstream.key +local hash_on = upstream.hash_on Review comment: get 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363278370 ## File path: lua/apisix/balancer.lua ## @@ -122,6 +123,38 @@ local function fetch_healthchecker(upstream, healthcheck_parent, version) end +local function create_chash_hash_key(ctx, upstream) +local key = upstream.key +local hash_on = upstream.hash_on +local chash_key + +-- hash_on is nil, compatible with old version, default vars +if not hash_on then +hash_on = "vars" +end + +if hash_on == "consumer" then +chash_key = ctx.consumer_id +elseif hash_on == "vars" then +chash_key = ctx.var[key] +elseif hash_on == "header" then +chash_key = ngx.req.get_headers()[key] Review comment: get 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support
more flexible ways
URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363278222
##
File path: t/admin/upstream.t
##
@@ -980,3 +980,223 @@ GET /t
passed
--- no_error_log
[error]
+
+
+
+=== TEST 30: type chash, hash_on: vars
+--- config
+location /t {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+"key": "arg_device_id",
+"nodes": {
+"127.0.0.1:8080": 1
+},
+"type": "chash",
+"hash_on": "vars",
+"desc": "new chash upstream"
+}]]
+)
+
+ngx.status = code
+ngx.say(body)
+}
+}
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 31: type chash, hash_on: header, header name with '_',
underscores_in_headers on
+--- config
+location /t {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+"key": "custom_header",
+"nodes": {
+"127.0.0.1:8080": 1
+},
+"type": "chash",
+"hash_on": "header",
+"desc": "new chash upstream"
+}]]
+)
+
+ngx.status = code
+ngx.say(body)
+}
+}
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 32: type chash, hash_on: header, header name with invalid character
+--- config
+location /t {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+"key": "$#^@",
+"nodes": {
+"127.0.0.1:8080": 1
+},
+"type": "chash",
+"hash_on": "header",
+"desc": "new chash upstream"
+}]]
+)
+
+ngx.status = code
+ngx.print(body)
+}
+}
+--- request
+GET /t
+--- error_code: 400
+--- response_body
+{"error_msg":"invalid configuration: failed to match pattern
\"^[a-zA-Z0-9-_]+$\" with \"$#^@\""}
+--- no_error_log
+[error]
+
+
+
+=== TEST 33: type chash, hash_on: cookie
+--- config
+location /t {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+"key": "custom_cookie",
+"nodes": {
+"127.0.0.1:8080": 1
+},
+"type": "chash",
+"hash_on": "cookie",
+"desc": "new chash upstream"
+}]]
+)
+
+ngx.status = code
+ngx.say(body)
+}
+}
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 34: type chash, hash_on: cookie, cookie name with invalid character
+--- config
+location /t {
+content_by_lua_block {
+local t = require("lib.test_admin").test
+local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+"key": "$#^@abc",
+"nodes": {
+"127.0.0.1:8080": 1
+},
+"type": "chash",
+"hash_on": "cookie",
+"desc": "new chash upstream"
+}]]
+)
+
+ngx.status = code
+ngx.print(body)
+}
+}
+--- request
+GET /t
+--- error_code: 400
+--- response_body
+{"error_msg":"invalid configuration: failed to match pattern
\"^[a-zA-Z0-9-_]+$\" with \"$#^@abc\""}
+--- no_error_log
+[error]
+
+
+
+=== TEST 35: type chash, hash_on: consumer, don't need upstream key
Review comment:
when hash_on is consumer, key is invalid.
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
With regards,
Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363278286 ## File path: t/admin/services.t ## @@ -751,3 +751,35 @@ GET /t passed --- no_error_log [error] + + + +=== TEST 22: set service(id: 1) and upstream(type:chash, default hash_on: vars, missing key) Review comment: get 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support
more flexible ways
URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363277303
##
File path: doc/architecture-design-cn.md
##
@@ -334,9 +335,46 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -X PUT
-d '
}
}'
```
-
更多细节可以参考[健康检查的文档](health-check.md)。
+下面是几个使用不同`hash_on`类型的配置示例：
+```shell
+curl http://127.0.0.1:9080/apisix/admin/upstreams/1 -X PUT -d '
+{
+"type": "chash",
+"hash_on":"consumer",
+"nodes": {
+"127.0.0.1:80": 1,
+"127.0.0.2:80": 2,
+"foo.com:80": 3
+}
+}'
+
+curl http://127.0.0.1:9080/apisix/admin/upstreams/1 -X PUT -d '
+{
+"type": "chash",
+"hash_on":"header",
+"key": "custom-header-name",
Review comment:
get
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
With regards,
Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363277042 ## File path: doc/architecture-design-cn.md ## @@ -237,7 +237,8 @@ APISIX 的 Upstream 除了基本的复杂均衡算法选择外，还支持对上 |--- |-|--| |type|必需|`roundrobin` 支持权重的负载，`chash` 一致性哈希，两者是二选一的| |nodes |必需|哈希表，内部元素的 key 是上游机器地址列表，格式为`地址 + Port`，其中地址部分可以是 IP 也可以是域名，比如 `192.168.1.100:80`、`foo.com:80`等。value 则是节点的权重，特别的，当权重值为 `0` 有特殊含义，通常代表该上游节点失效，永远不希望被选中。| -|key |必需|该选项只有类型是 `chash` 才有效。根据 `key` 来查找对应的 node `id`，相同的 `key` 在同一个对象中，永远返回相同 id，目前支持的 Nginx 内置变量有 `uri, server_name, server_addr, request_uri, remote_port, remote_addr, query_string, host, hostname, arg_***`，其中 `arg_***` 是来自URL的请求参数，[Nginx 变量列表](http://nginx.org/en/docs/varindex.html)| +|hash_on |可选|该选项只有类型是 `chash` 才有效。支持的类型有`vars`（Nginx内置变量），`header`（自定义header），`cookie`，`consumer`，默认值为`vars`| +|key |必需|该选项只有类型是 `chash` 才有效，需要配合`hash_on`来使用，通过`hash_on`和`key`来查找对应的 node `id`。`hash_on`设为`vars`时，`key`为必传参数，目前支持的 Nginx 内置变量有 `uri, server_name, server_addr, request_uri, remote_port, remote_addr, query_string, host, hostname, arg_***`，其中 `arg_***` 是来自URL的请求参数，[Nginx 变量列表](http://nginx.org/en/docs/varindex.html)；`hash_on`设为`header`时, `key`为必传参数， 自定义的`header name`；`hash_on`设为`cookie`时, `key`为必传参数， 自定义的`cookie name`；`hash_on`设为`consumer`时，`key`不需要设置，为空，此时哈希算法采用的`key`为认证通过的`consumer_id`| Review comment: I don't think this is necessary, because only authenticated requests can reach upstream. 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363277095 ## File path: doc/architecture-design-cn.md ## @@ -334,9 +335,46 @@ curl http://127.0.0.1:9080/apisix/admin/routes/1 -X PUT -d ' } }' ``` - 更多细节可以参考[健康检查的文档](health-check.md)。 +下面是几个使用不同`hash_on`类型的配置示例： +```shell +curl http://127.0.0.1:9080/apisix/admin/upstreams/1 -X PUT -d ' Review comment: get 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363275347 ## File path: doc/architecture-design-cn.md ## @@ -237,7 +237,8 @@ APISIX 的 Upstream 除了基本的复杂均衡算法选择外，还支持对上 |--- |-|--| |type|必需|`roundrobin` 支持权重的负载，`chash` 一致性哈希，两者是二选一的| |nodes |必需|哈希表，内部元素的 key 是上游机器地址列表，格式为`地址 + Port`，其中地址部分可以是 IP 也可以是域名，比如 `192.168.1.100:80`、`foo.com:80`等。value 则是节点的权重，特别的，当权重值为 `0` 有特殊含义，通常代表该上游节点失效，永远不希望被选中。| -|key |必需|该选项只有类型是 `chash` 才有效。根据 `key` 来查找对应的 node `id`，相同的 `key` 在同一个对象中，永远返回相同 id，目前支持的 Nginx 内置变量有 `uri, server_name, server_addr, request_uri, remote_port, remote_addr, query_string, host, hostname, arg_***`，其中 `arg_***` 是来自URL的请求参数，[Nginx 变量列表](http://nginx.org/en/docs/varindex.html)| +|hash_on |可选|该选项只有类型是 `chash` 才有效。支持的类型有`vars`（Nginx内置变量），`header`（自定义header），`cookie`，`consumer`，默认值为`vars`| Review comment: get 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 With regards, Apache Git Services
[GitHub] [incubator-apisix] codjust commented on a change in pull request #1022: Feature/chash key support more flexible ways
codjust commented on a change in pull request #1022: Feature/chash key support
more flexible ways
URL: https://github.com/apache/incubator-apisix/pull/1022#discussion_r363275316
##
File path: conf/config.yaml
##
@@ -67,6 +67,7 @@ nginx_config: # config for render the
template to genarate n
client_header_timeout: 60s # timeout for reading client request
header, then 408 (Request Time-out) error is returned to the client
client_body_timeout: 60s # timeout for reading client request body,
then 408 (Request Time-out) error is returned to the client
send_timeout: 10s # timeout for transmitting a response to
the client.then the connection is closed
+underscores_in_headers: "on" # default enables the use of underscores in
client request header fields
Review comment:
I need to validate the header name and cookie name .
```lua
_M.upstream_hash_header_schema = {
type = "string",
pattern = [[^[a-zA-Z0-9-_]+$]]
}
```
if underscores_in_headers not on, header name with `_` character is not
allow.
Maybe we can turn underscores on by default, how about 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
With regards,
Apache Git Services
