nic-6443 commented on code in PR #14018:
URL: https://github.com/apache/apisix/pull/14018#discussion_r4218380746
##########
apisix/discovery/consul/client.lua:
##########
@@ -120,6 +120,9 @@ local function get_opts(consul_server, is_catalog)
local opts = {
host = consul_server.host,
port = consul_server.port,
+ ssl = consul_server.ssl,
+ ssl_verify = true,
+ sni_host = consul_server.host,
connect_timeout = consul_server.connect_timeout,
read_timeout = consul_server.read_timeout,
default_args = {
Review Comment:
Agreed, lua-resty-consul ignores them without `ssl`, but there's no reason
to rely on that. Moved the three TLS fields into a small `set_tls_opts()`
helper that only sets them for https servers, used by both `get_opts` and
`fetch_services_from_server`. Done in fc4cddf.
##########
apisix/discovery/consul/client.lua:
##########
@@ -303,6 +310,9 @@ function _M.fetch_services_from_server(consul_server,
options)
local consul_client = resty_consul:new({
host = consul_server.host,
port = consul_server.port,
+ ssl = consul_server.ssl,
+ ssl_verify = true,
+ sni_host = consul_server.host,
connect_timeout = consul_server.connect_timeout,
read_timeout = consul_server.read_timeout,
default_args = {
Review Comment:
Same fix as the `get_opts` thread: `fetch_services_from_server` now goes
through `set_tls_opts()`, so plain http servers get no TLS fields. fc4cddf.
##########
apisix/discovery/consul/client.lua:
##########
@@ -249,15 +252,19 @@ function _M.format_consul_params(consul_conf)
local consul_server_list = core.table.new(0, #servers)
for _, v in pairs(servers) do
- local scheme, host, port, path = unpack(http.parse_uri(nil, v))
- if scheme ~= "http" then
- return nil, "only support consul http schema address, eg:
http://address:port";
+ -- parse_uri only accepts http and https, any other scheme yields nil
+ local scheme, host, port, path = unpack(http.parse_uri(nil, v) or {})
+ if scheme ~= "http" and scheme ~= "https" then
+ return nil, "only support consul http or https schema address, "
+ .. "eg: http://address:port or https://address:port";
elseif path ~= "/" or core.string.has_suffix(v, '/') then
- return nil, "invalid consul server address, the valid format:
http://address:port";
+ return nil, "invalid consul server address, the valid format: "
+ .. "http://address:port or https://address:port";
Review Comment:
Good catch. The parse error from `parse_uri` is now returned as `invalid
consul server address: <err>, the valid format: ...`, so a malformed address
(including any non-http(s) scheme, which `parse_uri` doesn't match) reports the
real reason. Test updated accordingly. fc4cddf.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
