fix(balancer): quote ipv6 address #7594
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tokers
reviewed
Aug 2, 2022
docs/zh/latest/admin-api.md
Outdated
| @@ -532,7 +532,7 @@ APISIX 的 Upstream 除了基本的负载均衡算法选择外，还支持对上 | |||
| | 名字 | 可选项 | 类型 | 说明 | 示例 | | |||
| | -------------- | ---------------------------------- | -------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------ | | |||
| | type | 必需 | 枚举 | 负载均衡算法 | | | | |||
| | nodes | 必需，不能和 `service_name` 一起用 | Node | 哈希表或数组。当它是哈希表时，内部元素的 key 是上游机器地址列表，格式为`地址 +（可选的）端口`，其中地址部分可以是 IP 也可以是域名，比如 `192.168.1.100:80`、`foo.com:80`等。value 则是节点的权重。当它是数组时，数组中每个元素都是一个哈希表，其中包含 `host`、`weight` 以及可选的 `port`、`priority`。`nodes` 可以为空，这通常用作占位符。客户端命中这样的上游会返回 502。 | `192.168.1.100:80` | | |||
| | nodes | 必需，不能和 `service_name` 一起用 | Node | 哈希表或数组。当它是哈希表时，内部元素的 key 是上游机器地址列表，格式为`地址 +（可选的）端口`，其中地址部分可以是 IP 也可以是域名，比如 `192.168.1.100:80`、`foo.com:80`等。如果地址部分是 IPv6 地址，则必须用中括号将其括起来。value 则是节点的权重。当它是数组时，数组中每个元素都是一个哈希表，其中包含 `host`、`weight` 以及可选的 `port`、`priority`。`nodes` 可以为空，这通常用作占位符。客户端命中这样的上游会返回 502。 | `192.168.1.100:80`, `[::1]:80` | | |||
There was a problem hiding this comment.
We quote the IPv6 address in the code if it doesn't wrap by the double quotes, but why here do we ask users to add the double quotes?
There was a problem hiding this comment.
There are two types of nodes:
If it's an array, then user have to specify address and port seperately, we could quote the ipv6 address with square brackets if not quoted yet.
However, if it's a hash table, the key format is "address:port", then if user do not quote the ipv6 address, we could not tell the address and port from it. For example, ::1:1980 and ::1 are both valid ipv6 address, how to know whether 1980 is a port or not?
There was a problem hiding this comment.
::1 is a valid format as we claim the port is optional. So it's the user's choice to quote it or not. Would be better to use suggest instead of must in the doc.
There was a problem hiding this comment.
Or we can use must when the user needs to use a port, like 如果地址部分是 IPv6 地址加端口，则必须用中括号将 IPv6 地址括起来。
spacewander
reviewed
Aug 2, 2022
docs/en/latest/admin-api.md
Outdated
| @@ -524,7 +524,7 @@ In addition to the equalization algorithm selections, Upstream also supports pas | |||
| | Name | Optional | Description | Example | | |||
| | --------------------------- | ------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | | |||
| | type | required | Load balancing algorithm to be used. | | | |||
| | nodes | required, can't be used with `service_name` | IP addresses (with optional ports) of the Upstream nodes represented as a hash table or an array. In the hash table, the key is the IP address and the value is the weight of the node for the load balancing algorithm. In the array, each item is a hash table with keys `host`, `weight`, and the optional `port` and `priority`. Empty nodes are treated as placeholders and clients trying to access this Upstream will receive a 502 response. | `192.168.1.100:80` | | |||
| | nodes | required, can't be used with `service_name` | IP addresses (with optional ports) of the Upstream nodes represented as a hash table or an array. If the IP address is IPv6 address, it (without port) must be quoted with square brackets. In the hash table, the key is the IP address and the value is the weight of the node for the load balancing algorithm. In the array, each item is a hash table with keys `host`, `weight`, and the optional `port` and `priority`. Empty nodes are treated as placeholders and clients trying to access this Upstream will receive a 502 response. | `192.168.1.100:80`, `[::1]:80` | | |||
There was a problem hiding this comment.
IP addresses (with optional ports) of the Upstream nodes represented as a hash table or an array. If the IP address is IPv6 address, it (without port) must be quoted with square brackets.
The quote is not needed in the array format? Only when using in hash format, and the host contains a port, the quote is required to distinguish it between ::1:1980 and [::1]:1980.
spacewander
reviewed
Aug 2, 2022
t/node/upstream-ipv6.t
Outdated
|
|
||
|
|
||
|
|
||
| === TEST 7: set upstream(id: 1) |
There was a problem hiding this comment.
Would be better to use a precise test name instead of repeating set upstream(id: 1). The first test in a group of tests should contain a title that describes what is tested.
tzssangglass
previously approved these changes
Aug 2, 2022
spacewander
reviewed
Aug 3, 2022
docs/en/latest/admin-api.md
Outdated
| @@ -524,7 +524,7 @@ In addition to the equalization algorithm selections, Upstream also supports pas | |||
| | Name | Optional | Description | Example | | |||
| | --------------------------- | ------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------ | | |||
| | type | required | Load balancing algorithm to be used. | | | |||
| | nodes | required, can't be used with `service_name` | IP addresses (with optional ports) of the Upstream nodes represented as a hash table or an array. If the IP address is IPv6 address, it (without port) must be quoted with square brackets. In the hash table, the key is the IP address and the value is the weight of the node for the load balancing algorithm. In the array, each item is a hash table with keys `host`, `weight`, and the optional `port` and `priority`. Empty nodes are treated as placeholders and clients trying to access this Upstream will receive a 502 response. | `192.168.1.100:80`, `[::1]:80` | | |||
| | nodes | required, can't be used with `service_name` | IP addresses (with optional ports) of the Upstream nodes represented as a hash table or an array. For hash table case, if the key is IPv6 address with port, then the IPv6 address must be quoted with square brackets. In the hash table, the key is the IP address and the value is the weight of the node for the load balancing algorithm. In the array, each item is a hash table with keys `host`, `weight`, and the optional `port` and `priority`. Empty nodes are treated as placeholders and clients trying to access this Upstream will receive a 502 response. | `192.168.1.100:80`, `[::1]:80` | | |||
There was a problem hiding this comment.
For hash table case, if the key is IPv6 address with port, then the IPv6 address must be quoted with square brackets.
Could we move this sentence back so the English version is similar with the Chinese version?
spacewander
approved these changes
Aug 4, 2022
tzssangglass
approved these changes
Aug 4, 2022
Liu-Junlin
pushed a commit
to Liu-Junlin/apisix
that referenced
this pull request
Nov 4, 2022
spacewander
pushed a commit
to spacewander/incubator-apisix
that referenced
this pull request
Nov 9, 2022
spacewander
pushed a commit
that referenced
this pull request
Nov 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7563
Description
quote ipv6 address with square brackets before picking server.
Fixes #7563
Checklist