bzp2010 commented on code in PR #12263: URL: https://github.com/apache/apisix/pull/12263#discussion_r2141800499
########## apisix/discovery/nacos/factory.lua: ########## @@ -0,0 +1,292 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- + +local require = require +local core = require("apisix.core") +local http = require('resty.http') +local ngx = ngx +local ngx_re = require('ngx.re') +local utils = require("apisix.discovery.nacos.utils") +local string = string +local string_sub = string.sub +local str_find = core.string.find +local ngx_timer_at = ngx.timer.at +local math_random = math.random +local ipairs = ipairs +local shdict_name = "nacos" +if ngx.config.subsystem == "stream" then + shdict_name = shdict_name .. "-stream" +end + +local nacos_dict = ngx.shared[shdict_name] +local NACOS_LOGIN_PATH = "/auth/login" +local NACOS_INSTANCE_PATH = "/ns/instance/list" +local _M = {} + + +local function _request(method, uri, params, headers, body, options) + local url = uri + if params ~= nil and params ~= {} then + url = uri .. "?" .. ngx.encode_args(params) + end + local httpc = http.new() + local timeout = options and options.timeout or {} + local connect_timeout = timeout.connect and timeout.connect * 1000 or 2000 + local read_timeout = timeout.read and timeout.read * 1000 or 2000 + local write_timeout = timeout.write and timeout.write * 1000 or 5000 + + httpc:set_timeouts(connect_timeout, read_timeout, write_timeout ) + local res, err = httpc:request_uri(url, { + method = method, + headers = headers, + body = body, + ssl_verify = false, Review Comment: What I mean is that hardcoding to disable TLS certificate validation should not be done. If it needs to default to `false`, then it should be configurable; otherwise, it should be hard-coded to `true`. Anyway, "no certificate validation at all times" isn't acceptable security-wise. I think we should do the right thing instead of pleasing the lazy. Not to mention that we already have `ca-certificates` installed for container deployment and APISIX will use it by default. ########## apisix/discovery/nacos/factory.lua: ########## @@ -0,0 +1,292 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- + +local require = require +local core = require("apisix.core") +local http = require('resty.http') +local ngx = ngx +local ngx_re = require('ngx.re') +local utils = require("apisix.discovery.nacos.utils") +local string = string +local string_sub = string.sub +local str_find = core.string.find +local ngx_timer_at = ngx.timer.at +local math_random = math.random +local ipairs = ipairs +local shdict_name = "nacos" +if ngx.config.subsystem == "stream" then + shdict_name = shdict_name .. "-stream" +end + +local nacos_dict = ngx.shared[shdict_name] +local NACOS_LOGIN_PATH = "/auth/login" +local NACOS_INSTANCE_PATH = "/ns/instance/list" +local _M = {} + + +local function _request(method, uri, params, headers, body, options) + local url = uri + if params ~= nil and params ~= {} then + url = uri .. "?" .. ngx.encode_args(params) + end + local httpc = http.new() + local timeout = options and options.timeout or {} + local connect_timeout = timeout.connect and timeout.connect * 1000 or 2000 + local read_timeout = timeout.read and timeout.read * 1000 or 2000 + local write_timeout = timeout.write and timeout.write * 1000 or 5000 + + httpc:set_timeouts(connect_timeout, read_timeout, write_timeout ) + local res, err = httpc:request_uri(url, { + method = method, + headers = headers, + body = body, + ssl_verify = false, + }) + + if not res then + return nil, err + end + + if not res.body or res.status ~= 200 then + return nil, 'status = ' .. res.status + end + + core.log.info("request to nacos, uri: ", url, "response: ", res.body) + + local data, err = core.json.decode(res.body) + if not data then + return nil, err + end + + return data +end + + +local function get_base_uri(hosts) + local host = hosts + -- TODO Add health check to get healthy nodes. + local url = host[math_random(#host)] + local auth_idx = core.string.rfind_char(url, '@') + local username, password + if auth_idx then + local protocol_idx = str_find(url, '://') + local protocol = string_sub(url, 1, protocol_idx + 2) + local user_and_password = string_sub(url, protocol_idx + 3, auth_idx - 1) + local arr = ngx_re.split(user_and_password, ':') + if #arr == 2 then + username = arr[1] + password = arr[2] + end + local other = string_sub(url, auth_idx + 1) + url = protocol .. other + end + return url, username, password +end + +local function request_login(self, host, username, password) + local params = { + ["username"] = username, + ["password"] = password, Review Comment: ditto https://github.com/apache/apisix/pull/12263/files#diff-eb100eda46f74a8793829cab2eab9974ec0f98edc6f312a91e0a31c4bab12889R147-R150 ########## apisix/discovery/nacos/init.lua: ########## @@ -15,410 +15,196 @@ -- limitations under the License. -- -local require = require -local local_conf = require('apisix.core.config_local').local_conf() -local http = require('resty.http') -local core = require('apisix.core') -local ipairs = ipairs -local type = type -local math = math -local math_random = math.random -local ngx = ngx -local ngx_re = require('ngx.re') -local ngx_timer_at = ngx.timer.at -local ngx_timer_every = ngx.timer.every -local string = string -local string_sub = string.sub -local str_byte = string.byte -local str_find = core.string.find -local log = core.log +local core = require("apisix.core") +local nacos_factory = require("apisix.discovery.nacos.factory") +local utils = require("apisix.discovery.nacos.utils") +local process = require("ngx.process") +local ipairs = ipairs +local require = require +local table = require("apisix.core.table") +local pcall = pcall +local pairs = pairs +local local_conf = require('apisix.core.config_local').local_conf() +local ngx = ngx + +local shdict_name = "nacos" +if ngx.config.subsystem == "stream" then + shdict_name = shdict_name .. "-stream" +end + +local nacos_dict = ngx.shared[shdict_name] +local OLD_CONFIG_ID = utils.old_config_id +local _M = {} +local nacos_clients = {} -local default_weight -local applications -local auth_path = 'auth/login' -local instance_list_path = 'ns/instance/list?healthyOnly=true&serviceName=' -local default_namespace_id = "public" -local default_group_name = "DEFAULT_GROUP" -local access_key -local secret_key - -local events -local events_list - - -local _M = {} - -local function discovery_nacos_callback(data, event, source, pid) - applications = data - log.notice("update local variable application, event is: ", event, - "source: ", source, "server pid:", pid, - ", application: ", core.json.encode(applications, true)) -end - - -local function request(request_uri, path, body, method, basic_auth) - local url = request_uri .. path - log.info('request url:', url) - local headers = {} - headers['Accept'] = 'application/json' - - if basic_auth then - headers['Authorization'] = basic_auth - end +function _M.nodes(service_name, discovery_args) + local ns_id = discovery_args and discovery_args.namespace_id or utils.default_namespace_id + local group_name = discovery_args and discovery_args.group_name or utils.default_group_name + local id = discovery_args and discovery_args.id or OLD_CONFIG_ID + local key = utils.generate_key(id, + ns_id, + group_name, + service_name) + local value = nacos_dict:get_stale(key) + local nodes = {} + if not value then + -- maximum waiting time: 5 seconds + local waiting_time = 5 + local step = 0.1 + local logged = false + while not value and waiting_time > 0 do + if not logged then + logged = true + end - if body and 'table' == type(body) then - local err - body, err = core.json.encode(body) - if not body then - return nil, 'invalid body : ' .. err + ngx.sleep(step) + waiting_time = waiting_time - step + value = nacos_dict:get_stale(key) end - headers['Content-Type'] = 'application/json' - end - - local httpc = http.new() - local timeout = local_conf.discovery.nacos.timeout - local connect_timeout = timeout.connect - local send_timeout = timeout.send - local read_timeout = timeout.read - log.info('connect_timeout:', connect_timeout, ', send_timeout:', send_timeout, - ', read_timeout:', read_timeout) - httpc:set_timeouts(connect_timeout, send_timeout, read_timeout) - local res, err = httpc:request_uri(url, { - method = method, - headers = headers, - body = body, - ssl_verify = true, - }) - if not res then - return nil, err - end - - if not res.body or res.status ~= 200 then - return nil, 'status = ' .. res.status - end - - local json_str = res.body - local data, err = core.json.decode(json_str) - if not data then - return nil, err - end - return data -end - - -local function get_url(request_uri, path) - return request(request_uri, path, nil, 'GET', nil) -end - - -local function post_url(request_uri, path, body) - return request(request_uri, path, body, 'POST', nil) -end - - -local function get_token_param(base_uri, username, password) - if not username or not password then - return '' - end - - local args = { username = username, password = password} - local data, err = post_url(base_uri, auth_path .. '?' .. ngx.encode_args(args), nil) - if err then - log.error('nacos login fail:', username, ' ', password, ' desc:', err) - return nil, err - end - return '&accessToken=' .. data.accessToken -end - - -local function get_namespace_param(namespace_id) - local param = '' - if namespace_id then - local args = {namespaceId = namespace_id} - param = '&' .. ngx.encode_args(args) end - return param -end - - -local function get_group_name_param(group_name) - local param = '' - if group_name then - local args = {groupName = group_name} - param = '&' .. ngx.encode_args(args) - end - return param -end - - -local function get_signed_param(group_name, service_name) - local param = '' - if access_key ~= '' and secret_key ~= '' then - local str_to_sign = ngx.now() * 1000 .. '@@' .. group_name .. '@@' .. service_name - local args = { - ak = access_key, - data = str_to_sign, - signature = ngx.encode_base64(ngx.hmac_sha1(secret_key, str_to_sign)) - } - param = '&' .. ngx.encode_args(args) + if not value then + core.log.error("nacos service not found: ", service_name) + return nodes end - return param -end + nodes = core.json.decode(value) -local function get_base_uri() - local host = local_conf.discovery.nacos.host - -- TODO Add health check to get healthy nodes. - local url = host[math_random(#host)] - local auth_idx = core.string.rfind_char(url, '@') - local username, password - if auth_idx then - local protocol_idx = str_find(url, '://') - local protocol = string_sub(url, 1, protocol_idx + 2) - local user_and_password = string_sub(url, protocol_idx + 3, auth_idx - 1) - local arr = ngx_re.split(user_and_password, ':') - if #arr == 2 then - username = arr[1] - password = arr[2] + local res = {} + for _, node in ipairs(nodes) do + if discovery_args then + if utils.match_metdata(node.metadata, discovery_args.metadata) then + core.table.insert(res, node) + end + else + core.table.insert(res, node) end - local other = string_sub(url, auth_idx + 1) - url = protocol .. other end - if local_conf.discovery.nacos.prefix then - url = url .. local_conf.discovery.nacos.prefix - end - - if str_byte(url, #url) ~= str_byte('/') then - url = url .. '/' - end - - return url, username, password + core.log.info("nacos service_name: ", service_name, " nodes: ", core.json.encode(res)) + return res end - -local function de_duplication(services, namespace_id, group_name, service_name, scheme) - for _, service in ipairs(services) do - if service.namespace_id == namespace_id and service.group_name == group_name - and service.service_name == service_name and service.scheme == scheme then - return true - end - end - return false +local function generate_new_config_from_old(discovery_conf) + local config = { + id = OLD_CONFIG_ID, + hosts = discovery_conf.host, + prefix = discovery_conf.prefix, + fetch_interval = discovery_conf.fetch_interval, + auth = { + access_key = discovery_conf.access_key, + secret_key = discovery_conf.secret_key, + }, + default_weight = discovery_conf.weight, + timeout = discovery_conf.timeout, + old_conf = true + } + return {config} end +function _M.init_worker() + local local_conf = require("apisix.core.config_local").local_conf(true) + local discovery_conf = local_conf.discovery and local_conf.discovery.nacos or {} -local function iter_and_add_service(services, values) - if not values then + if process.type() ~= "privileged agent" then return end - for _, value in core.config_util.iterate_values(values) do - local conf = value.value - if not conf then - goto CONTINUE - end - - local up - if conf.upstream then - up = conf.upstream - else - up = conf - end - - local namespace_id = (up.discovery_args and up.discovery_args.namespace_id) - or default_namespace_id - - local group_name = (up.discovery_args and up.discovery_args.group_name) - or default_group_name + local keep = {} + -- support old way + if discovery_conf.host then + discovery_conf = generate_new_config_from_old(discovery_conf) + end + for _, val in ipairs(discovery_conf) do + local id = val.id + local version = ngx.md5(core.json.encode(val, true)) + keep[id] = true - local dup = de_duplication(services, namespace_id, group_name, - up.service_name, up.scheme) - if dup then + -- The nacos config has not been changed. + if nacos_clients[id] and nacos_clients[id].version == version then goto CONTINUE end - if up.discovery_type == 'nacos' then - core.table.insert(services, { - service_name = up.service_name, - namespace_id = namespace_id, - group_name = group_name, - scheme = up.scheme, - }) + if nacos_clients[id] then + nacos_clients[id]:stop() end + local new_client = nacos_factory.new(val) + new_client:start() + nacos_clients[id] = new_client + ::CONTINUE:: end -end - -local function get_nacos_services() - local services = {} - -- here we use lazy load to work around circle dependency - local get_upstreams = require('apisix.upstream').upstreams - local get_routes = require('apisix.router').http_routes - local get_stream_routes = require('apisix.router').stream_routes - local get_services = require('apisix.http.service').services - local values = get_upstreams() - iter_and_add_service(services, values) - values = get_routes() - iter_and_add_service(services, values) - values = get_services() - iter_and_add_service(services, values) - values = get_stream_routes() - iter_and_add_service(services, values) - return services + for id, client in pairs(nacos_clients) do + -- The nacos config has been deleted. + if not keep[client.id] then + client:stop() + nacos_clients[id] = nil + end + end end -local function is_grpc(scheme) - if scheme == 'grpc' or scheme == 'grpcs' then - return true - end - return false +-- Now we use control plane to list the services +function _M.list_all_services() + return {} end -local function fetch_full_registry(premature) - if premature then - return +function _M.get_health_checkers() + local result = core.table.new(0, 4) + if nacos_clients == nil then + return result end - local up_apps = {} - local base_uri, username, password = get_base_uri() - local token_param, err = get_token_param(base_uri, username, password) - if err then - log.error('get_token_param error:', err) - if not applications then - applications = up_apps + for id in pairs(nacos_clients) do + local health_check = require("resty.healthcheck") + local list = health_check.get_target_list(id, "nacos") + if list then + result[id] = list end - return end - local infos = get_nacos_services() - if #infos == 0 then - applications = up_apps - return - end + return result +end - for _, service_info in ipairs(infos) do - local data, err - local namespace_id = service_info.namespace_id - local group_name = service_info.group_name - local scheme = service_info.scheme or '' - local namespace_param = get_namespace_param(service_info.namespace_id) - local group_name_param = get_group_name_param(service_info.group_name) - local signature_param = get_signed_param(service_info.group_name, service_info.service_name) - local query_path = instance_list_path .. service_info.service_name - .. token_param .. namespace_param .. group_name_param - .. signature_param - data, err = get_url(base_uri, query_path) - if err then - log.error('get_url:', query_path, ' err:', err) - goto CONTINUE - end +local cjson = require "cjson" - if not up_apps[namespace_id] then - up_apps[namespace_id] = {} - end +function _M.dump_data() + local applications = {} + local keys = nacos_dict:get_keys() or {} - if not up_apps[namespace_id][group_name] then - up_apps[namespace_id][group_name] = {} + for _, key in ipairs(keys) do + local parts = {} + for part in key:gmatch("[^/]+") do + table.insert(parts, part) end - for _, host in ipairs(data.hosts) do - local nodes = up_apps[namespace_id] - [group_name][service_info.service_name] - if not nodes then - nodes = {} - up_apps[namespace_id] - [group_name][service_info.service_name] = nodes - end - - local node = { - host = host.ip, - port = host.port, - weight = host.weight or default_weight, - } - - -- docs: https://github.com/yidongnan/grpc-spring-boot-starter/pull/496 - if is_grpc(scheme) and host.metadata and host.metadata.gRPC_port then - node.port = host.metadata.gRPC_port + if #parts == 4 then + local id, namespace_id, + group_name, service_name = parts[1], parts[2], parts[3], parts[4] + local data_str = nacos_dict:get(key) + + if data_str and data_str ~= "" then + -- Decode JSON string to Lua table + local success, data = pcall(cjson.decode, data_str) + if success then + applications[id] = applications[id] or {} + applications[id][namespace_id] = applications[id][namespace_id] or {} + applications[id][namespace_id][group_name] = applications[id] + [namespace_id][group_name] or {} + applications[id][namespace_id][group_name][service_name] = data + else + ngx.log(ngx.ERR, "failed to decode data for key ", key, ": ", data) + end end - - core.table.insert(nodes, node) end - - ::CONTINUE:: - end - local new_apps_md5sum = ngx.md5(core.json.encode(up_apps)) - local old_apps_md5sum = ngx.md5(core.json.encode(applications)) - if new_apps_md5sum == old_apps_md5sum then - return - end - applications = up_apps - local ok, err = events:post(events_list._source, events_list.updating, - applications) - if not ok then - log.error("post_event failure with ", events_list._source, - ", update application error: ", err) - end -end - - -function _M.nodes(service_name, discovery_args) - local namespace_id = discovery_args and - discovery_args.namespace_id or default_namespace_id - local group_name = discovery_args - and discovery_args.group_name or default_group_name - - local logged = false - -- maximum waiting time: 5 seconds - local waiting_time = 5 - local step = 0.1 - while not applications and waiting_time > 0 do - if not logged then - log.warn('wait init') - logged = true - end - ngx.sleep(step) - waiting_time = waiting_time - step - end - - if not applications or not applications[namespace_id] - or not applications[namespace_id][group_name] - then - return nil end - return applications[namespace_id][group_name][service_name] -end - -function _M.init_worker() - events = require("apisix.events") - events_list = events:event_list("discovery_nacos_update_application", - "updating") - - if 0 ~= ngx.worker.id() then - events:register(discovery_nacos_callback, events_list._source, - events_list.updating) - return - end - - default_weight = local_conf.discovery.nacos.weight - log.info('default_weight:', default_weight) - local fetch_interval = local_conf.discovery.nacos.fetch_interval - log.info('fetch_interval:', fetch_interval) - access_key = local_conf.discovery.nacos.access_key - secret_key = local_conf.discovery.nacos.secret_key - ngx_timer_at(0, fetch_full_registry) - ngx_timer_every(fetch_interval, fetch_full_registry) -end - - -function _M.dump_data() - return {config = local_conf.discovery.nacos, services = applications or {}} + return { + config = local_conf.discovery.nacos, Review Comment: This will contain `access_key` and `secret_key`. It does seem that it should not be presented to the API in plaintext. You can't ensure that the API will only allow native calls for security consistent with local files. The fact that this API (Control API) can be configured to allow access from any IP, and also does not (unsupported) turn on TLS, amounts to creating the possibility for a credential to be transmitted over the internet in plaintext. Moreover, what is the point of passing this configuration through the API data, as an authorized system administrator who can access the configuration file at any time, or drive configuration changes through some CI/CD system. The significance of providing this convenience via the API seems trivial. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org