You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by sp...@apache.org on 2020/10/04 05:12:36 UTC
[apisix] branch master updated: bugfix: when service works with
upstream that contains a domain name,
the merged configuration should not always changing. (#2322)
This is an automated email from the ASF dual-hosted git repository.
spacewander 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 c5dcced bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. (#2322)
c5dcced is described below
commit c5dcced1cf90e2ffa5b73583784fc9bd34706aac
Author: YuanSheng Wang <me...@gmail.com>
AuthorDate: Sun Oct 4 13:12:28 2020 +0800
bugfix: when service works with upstream that contains a domain name, the merged configuration should not always changing. (#2322)
---
apisix/init.lua | 102 ++++++------
apisix/upstream.lua | 25 +--
t/plugin/key-auth-upstream-domain-node.t | 258 +++++++++++++++++++++++++++++++
3 files changed, 307 insertions(+), 78 deletions(-)
diff --git a/apisix/init.lua b/apisix/init.lua
index 9f8b38b..780757f 100644
--- a/apisix/init.lua
+++ b/apisix/init.lua
@@ -240,7 +240,7 @@ local function compare_upstream_node(old_t, new_t)
end
-local function parse_domain_in_up(up, api_ctx)
+local function parse_domain_in_up(up)
local nodes = up.value.nodes
local new_nodes, err = parse_domain_for_nodes(nodes)
if not new_nodes then
@@ -253,20 +253,17 @@ local function parse_domain_in_up(up, api_ctx)
return up
end
- if not up.modifiedIndex_org then
- up.modifiedIndex_org = up.modifiedIndex
- end
- up.modifiedIndex = up.modifiedIndex_org .. "#" .. ngx_now()
-
- up.dns_value = core.table.clone(up.value)
- up.dns_value.nodes = new_nodes
+ local up_new = core.table.clone(up)
+ up_new.modifiedIndex = up.modifiedIndex .. "#" .. ngx_now()
+ up_new.dns_value = core.table.clone(up.value)
+ up_new.dns_value.nodes = new_nodes
core.log.info("resolve upstream which contain domain: ",
- core.json.delay_encode(up))
- return up
+ core.json.delay_encode(up_new))
+ return up_new
end
-local function parse_domain_in_route(route, api_ctx)
+local function parse_domain_in_route(route)
local nodes = route.value.upstream.nodes
local new_nodes, err = parse_domain_for_nodes(nodes)
if not new_nodes then
@@ -279,22 +276,14 @@ local function parse_domain_in_route(route, api_ctx)
return route
end
- if not route.modifiedIndex_org then
- route.modifiedIndex_org = route.modifiedIndex
- end
- route.modifiedIndex = route.modifiedIndex_org .. "#" .. ngx_now()
- api_ctx.conf_version = route.modifiedIndex
+ local route_new = core.table.clone(route)
+ route_new.modifiedIndex = route.modifiedIndex .. "#" .. ngx_now()
- route.dns_value = core.table.deepcopy(route.value)
- route.dns_value.upstream.nodes = new_nodes
+ route_new.dns_value = core.table.deepcopy(route.value)
+ route_new.dns_value.upstream.nodes = new_nodes
core.log.info("parse route which contain domain: ",
core.json.delay_encode(route))
- return route
-end
-
-
-local function return_direct(...)
- return ...
+ return route_new
end
@@ -428,25 +417,15 @@ function _M.http_access_phase()
-- the `api_ctx.conf_version` is different after we called
-- `parse_domain_in_up`, need to recreate the cache by new
-- `api_ctx.conf_version`
- local parsed_upstream, err = lru_resolved_domain(upstream,
- upstream.modifiedIndex, return_direct, nil)
+ local err
+ upstream, err = lru_resolved_domain(upstream,
+ upstream.modifiedIndex,
+ parse_domain_in_up,
+ upstream)
if err then
core.log.error("failed to get resolved upstream: ", err)
return core.response.exit(500)
end
-
- if not parsed_upstream then
- parsed_upstream, err = parse_domain_in_up(upstream)
- if err then
- core.log.error("failed to reolve domain in upstream: ",
- err)
- return core.response.exit(500)
- end
-
- lru_resolved_domain(upstream, upstream.modifiedIndex,
- return_direct, parsed_upstream)
- end
-
end
if upstream.value.enable_websocket then
@@ -457,37 +436,37 @@ function _M.http_access_phase()
api_ctx.pass_host = upstream.value.pass_host
api_ctx.upstream_host = upstream.value.upstream_host
end
+
+ core.log.info("parsed upstream: ", core.json.delay_encode(upstream))
+ api_ctx.matched_upstream = upstream.dns_value or upstream.value
end
else
if route.has_domain then
- local parsed_route, err = lru_resolved_domain(route, api_ctx.conf_version,
- return_direct, nil)
+ local err
+ route, err = lru_resolved_domain(route, api_ctx.conf_version,
+ parse_domain_in_route, route)
if err then
core.log.error("failed to get resolved route: ", err)
return core.response.exit(500)
end
- if not parsed_route then
- route, err = parse_domain_in_route(route, api_ctx)
- if err then
- core.log.error("failed to reolve domain in route: ", err)
- return core.response.exit(500)
- end
-
- lru_resolved_domain(route, api_ctx.conf_version,
- return_direct, route)
- end
+ api_ctx.matched_route = route
end
- if route.value.upstream and route.value.upstream.enable_websocket then
+ local route_val = route.value
+ if route_val.upstream and route_val.upstream.enable_websocket then
enable_websocket = true
end
- if route.value.upstream and route.value.upstream.pass_host then
- api_ctx.pass_host = route.value.upstream.pass_host
- api_ctx.upstream_host = route.value.upstream.upstream_host
+ if route_val.upstream and route_val.upstream.pass_host then
+ api_ctx.pass_host = route_val.upstream.pass_host
+ api_ctx.upstream_host = route_val.upstream.upstream_host
end
+
+ api_ctx.matched_upstream = (route.dns_value and
+ route.dns_value.upstream)
+ or route_val.upstream
end
if enable_websocket then
@@ -580,6 +559,12 @@ function _M.grpc_access_phase()
api_ctx.conf_id = route.value.id
end
+ -- todo: support upstream id
+
+ api_ctx.matched_upstream = (route.dns_value and
+ route.dns_value.upstream)
+ or route.value.upstream
+
local plugins = core.tablepool.fetch("plugins", 32, 0)
api_ctx.plugins = plugin.filter(route, plugins)
@@ -816,13 +801,18 @@ function _M.stream_preread_phase()
api_ctx.plugins = plugin.stream_filter(matched_route, plugins)
-- core.log.info("valid plugins: ", core.json.delay_encode(plugins, true))
+ api_ctx.matched_upstream = matched_route.value.upstream
api_ctx.conf_type = "stream/route"
api_ctx.conf_version = matched_route.modifiedIndex
api_ctx.conf_id = matched_route.value.id
run_plugin("preread", plugins, api_ctx)
- set_upstream(matched_route, api_ctx)
+ local ok, err = set_upstream(matched_route, api_ctx)
+ if not ok then
+ core.log.error("failed to set upstream: ", err)
+ return ngx_exit(1)
+ end
end
diff --git a/apisix/upstream.lua b/apisix/upstream.lua
index dabb303..0ce1f2b 100644
--- a/apisix/upstream.lua
+++ b/apisix/upstream.lua
@@ -59,32 +59,13 @@ function _M.set_by_route(route, api_ctx)
return true
end
- local up_id = route.value.upstream_id
- if up_id then
- if not upstreams then
- return false, "need to create a etcd instance for fetching "
- .. "upstream information"
- end
-
- local up_obj = upstreams:get(tostring(up_id))
- if not up_obj then
- return false, "failed to find upstream by id: " .. up_id
- end
- core.log.info("upstream: ", core.json.delay_encode(up_obj))
-
- local up_conf = up_obj.dns_value or up_obj.value
- set_directly(api_ctx, up_conf.type .. "#upstream_" .. up_id,
- up_obj.modifiedIndex, up_conf, up_obj)
- return true
- end
-
- local up_conf = (route.dns_value and route.dns_value.upstream)
- or route.value.upstream
+ local up_conf = api_ctx.matched_upstream
if not up_conf then
return false, "missing upstream configuration in Route or Service"
end
+ -- core.log.info("up_conf: ", core.json.delay_encode(up_conf, true))
- set_directly(api_ctx, up_conf.type .. "#route_" .. route.value.id,
+ set_directly(api_ctx, up_conf.type .. "#upstream_" .. tostring(up_conf),
api_ctx.conf_version, up_conf, route)
return true
end
diff --git a/t/plugin/key-auth-upstream-domain-node.t b/t/plugin/key-auth-upstream-domain-node.t
new file mode 100644
index 0000000..f2f1589
--- /dev/null
+++ b/t/plugin/key-auth-upstream-domain-node.t
@@ -0,0 +1,258 @@
+#
+# 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.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: create consumer
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "key-auth": {
+ "key": "auth-one"
+ }
+ }
+ }]]
+ )
+
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 2: set service and enabled plugin `key-auth`
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/services/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "key-auth": {}
+ },
+ "desc": "new service"
+ }]]
+ )
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 3: create route with plugin `limit-req`(upstream node contains domain)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-req": {
+ "rate": 1,
+ "burst": 0,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "www.apple.com:80": 1
+ },
+ "pass_host": "node",
+ "type": "roundrobin"
+ },
+ "service_id": 1,
+ "uri": "/index.html"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 4: hit route 3 times
+--- config
+location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local headers = {
+ ["User-Agent"] = "curl/7.68.0",
+ ["apikey"] = "auth-one",
+ }
+
+ for i = 1, 3 do
+ local code, body = t.test('/index.html',
+ ngx.HTTP_GET,
+ "",
+ nil,
+ headers
+ )
+ ngx.say("return: ", code)
+ end
+ }
+}
+--- request
+GET /t
+--- response_body
+return: 301
+return: 503
+return: 503
+--- no_error_log
+[error]
+--- timeout: 5
+
+
+
+=== TEST 5: set upstream
+--- 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,
+ [[{
+ "nodes": {
+ "www.apple.com:80": 1
+ },
+ "pass_host": "node",
+ "type": "roundrobin"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 6: create route with plugin `limit-req`, and bind upstream via id
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "limit-req": {
+ "rate": 1,
+ "burst": 0,
+ "rejected_code": 503,
+ "key": "remote_addr"
+ }
+ },
+ "upstream_id": 1,
+ "service_id": 1,
+ "uri": "/index.html"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: hit route 3 times
+--- config
+location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local t = require("lib.test_admin")
+ local headers = {
+ ["User-Agent"] = "curl/7.68.0",
+ ["apikey"] = "auth-one",
+ }
+
+ for i = 1, 3 do
+ local code, body = t.test('/index.html',
+ ngx.HTTP_GET,
+ "",
+ nil,
+ headers
+ )
+ ngx.say("return: ", code)
+ end
+ }
+}
+--- request
+GET /t
+--- response_body
+return: 301
+return: 503
+return: 503
+--- no_error_log
+[error]
+--- timeout: 5