From 13f3139fbb2b5ba7208278fe412d093020a6362a Mon Sep 17 00:00:00 2001 From: nic-chen <33000667+nic-chen@users.noreply.github.com> Date: Wed, 10 Feb 2021 12:01:58 +0800 Subject: [PATCH] change: global rules should not be executed on the internal api (#3396) --- apisix/api_router.lua | 13 +++-- apisix/init.lua | 108 +++++---------------------------------- apisix/plugin.lua | 74 +++++++++++++++++++++++++++ conf/config-default.yaml | 2 + t/node/global-rule.t | 56 +++++++++++++++++++- 5 files changed, 153 insertions(+), 100 deletions(-) diff --git a/apisix/api_router.lua b/apisix/api_router.lua index f382258dc974..e68b261db449 100644 --- a/apisix/api_router.lua +++ b/apisix/api_router.lua @@ -16,6 +16,7 @@ -- local require = require local router = require("apisix.utils.router") +local apisix_router = require("apisix.router") local plugin_mod = require("apisix.plugin") local ip_restriction = require("apisix.plugins.ip-restriction") local core = require("apisix.core") @@ -103,7 +104,7 @@ function fetch_api_router() core.table.insert(routes, { methods = route.methods, paths = route.uri, - handler = function (api_ctx) + handler = function (api_ctx, skip_global_rule) local code, body local metadata = plugin_mod.plugin_metadata(name) @@ -121,6 +122,12 @@ function fetch_api_router() end end + if not skip_global_rule then + plugin_mod.run_global_rules(api_ctx, + apisix_router.global_rules, "access") + api_ctx.global_rules = apisix_router.global_rules + end + code, body = route.handler(api_ctx) if code or body then core.response.exit(code, body) @@ -146,7 +153,7 @@ function _M.has_route_not_under_apisix() end -function _M.match(api_ctx) +function _M.match(api_ctx, skip_global_rule) local api_router = core.lrucache.global("api_router", plugin_mod.load_times, fetch_api_router) if not api_router then core.log.error("failed to fetch valid api router") @@ -156,7 +163,7 @@ function _M.match(api_ctx) core.table.clear(match_opts) match_opts.method = api_ctx.var.request_method - local ok = api_router:dispatch(api_ctx.var.uri, match_opts, api_ctx) + local ok = api_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, skip_global_rule) return ok end diff --git a/apisix/init.lua b/apisix/init.lua index 28a363f85ac0..38b69cea1999 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -16,7 +16,6 @@ -- local require = require local core = require("apisix.core") -local config_util = require("apisix.core.config_util") local plugin = require("apisix.plugin") local script = require("apisix.script") local service_fetch = require("apisix.http.service").get @@ -127,48 +126,6 @@ function _M.http_init_worker() end -local function run_plugin(phase, plugins, api_ctx) - api_ctx = api_ctx or ngx.ctx.api_ctx - if not api_ctx then - return - end - - plugins = plugins or api_ctx.plugins - if not plugins or #plugins == 0 then - return api_ctx - end - - if phase ~= "log" - and phase ~= "header_filter" - and phase ~= "body_filter" - then - for i = 1, #plugins, 2 do - local phase_func = plugins[i][phase] - if phase_func then - local code, body = phase_func(plugins[i + 1], api_ctx) - if code or body then - if code >= 400 then - core.log.warn(plugins[i].name, " exits with http status code ", code) - end - - core.response.exit(code, body) - end - end - end - return api_ctx - end - - for i = 1, #plugins, 2 do - local phase_func = plugins[i][phase] - if phase_func then - phase_func(plugins[i + 1], api_ctx) - end - end - - return api_ctx -end - - function _M.http_ssl_phase() local ngx_ctx = ngx.ctx local api_ctx = ngx_ctx.api_ctx @@ -318,31 +275,6 @@ function _M.http_access_phase() core.ctx.set_vars_meta(api_ctx) - -- load and run global rule - if router.global_rules and router.global_rules.values - and #router.global_rules.values > 0 then - local plugins = core.tablepool.fetch("plugins", 32, 0) - local values = router.global_rules.values - for _, global_rule in config_util.iterate_values(values) do - api_ctx.conf_type = "global_rule" - api_ctx.conf_version = global_rule.modifiedIndex - api_ctx.conf_id = global_rule.value.id - - core.table.clear(plugins) - api_ctx.plugins = plugin.filter(global_rule, plugins) - run_plugin("rewrite", plugins, api_ctx) - run_plugin("access", plugins, api_ctx) - end - - core.tablepool.release("plugins", plugins) - api_ctx.plugins = nil - api_ctx.conf_type = nil - api_ctx.conf_version = nil - api_ctx.conf_id = nil - - api_ctx.global_rules = router.global_rules - end - local uri = api_ctx.var.uri if local_conf.apisix and local_conf.apisix.delete_uri_tail_slash then if str_byte(uri, #uri) == str_byte("/") then @@ -355,7 +287,8 @@ function _M.http_access_phase() if router.api.has_route_not_under_apisix() or core.string.has_prefix(uri, "/apisix/") then - local matched = router.api.match(api_ctx) + local skip = local_conf and local_conf.apisix.global_rule_skip_internal_api + local matched = router.api.match(api_ctx, skip) if matched then return end @@ -363,6 +296,10 @@ function _M.http_access_phase() router.router_http.match(api_ctx) + -- run global rule + plugin.run_global_rules(api_ctx, router.global_rules, "access") + api_ctx.global_rules = router.global_rules + local route = api_ctx.matched_route if not route then core.log.info("not find any matched route") @@ -476,7 +413,7 @@ function _M.http_access_phase() local plugins = plugin.filter(route) api_ctx.plugins = plugins - run_plugin("rewrite", plugins, api_ctx) + plugin.run_plugin("rewrite", plugins, api_ctx) if api_ctx.consumer then local changed route, changed = plugin.merge_consumer_route( @@ -493,7 +430,7 @@ function _M.http_access_phase() api_ctx.plugins = plugin.filter(route, api_ctx.plugins) end end - run_plugin("access", plugins, api_ctx) + plugin.run_plugin("access", plugins, api_ctx) end if route.value.service_protocol == "grpc" then @@ -541,33 +478,12 @@ local function common_phase(phase_name) return end - if api_ctx.global_rules then - local orig_conf_type = api_ctx.conf_type - local orig_conf_version = api_ctx.conf_version - local orig_conf_id = api_ctx.conf_id - - local plugins = core.tablepool.fetch("plugins", 32, 0) - local values = api_ctx.global_rules.values - for _, global_rule in config_util.iterate_values(values) do - api_ctx.conf_type = "global_rule" - api_ctx.conf_version = global_rule.modifiedIndex - api_ctx.conf_id = global_rule.value.id - - core.table.clear(plugins) - plugins = plugin.filter(global_rule, plugins) - run_plugin(phase_name, plugins, api_ctx) - end - core.tablepool.release("plugins", plugins) - - api_ctx.conf_type = orig_conf_type - api_ctx.conf_version = orig_conf_version - api_ctx.conf_id = orig_conf_id - end + plugin.run_global_rules(api_ctx, api_ctx.global_rules, phase_name) if api_ctx.script_obj then script.run(phase_name, api_ctx) else - run_plugin(phase_name, nil, api_ctx) + plugin.run_plugin(phase_name, nil, api_ctx) end return api_ctx @@ -825,7 +741,7 @@ function _M.stream_preread_phase() api_ctx.conf_version = matched_route.modifiedIndex api_ctx.conf_id = matched_route.value.id - run_plugin("preread", plugins, api_ctx) + plugin.run_plugin("preread", plugins, api_ctx) local code, err = set_upstream(matched_route, api_ctx) if code then @@ -850,7 +766,7 @@ end function _M.stream_log_phase() core.log.info("enter stream_log_phase") -- core.ctx.release_vars(api_ctx) - run_plugin("log") + plugin.run_plugin("log") end diff --git a/apisix/plugin.lua b/apisix/plugin.lua index 9241a6bc3fa5..54bdf473cb00 100644 --- a/apisix/plugin.lua +++ b/apisix/plugin.lua @@ -640,4 +640,78 @@ function _M.stream_plugin_checker(item) end +function _M.run_plugin(phase, plugins, api_ctx) + api_ctx = api_ctx or ngx.ctx.api_ctx + if not api_ctx then + return + end + + plugins = plugins or api_ctx.plugins + if not plugins or #plugins == 0 then + return api_ctx + end + + if phase ~= "log" + and phase ~= "header_filter" + and phase ~= "body_filter" + then + for i = 1, #plugins, 2 do + local phase_func = plugins[i][phase] + if phase_func then + local code, body = phase_func(plugins[i + 1], api_ctx) + if code or body then + if code >= 400 then + core.log.warn(plugins[i].name, " exits with http status code ", code) + end + + core.response.exit(code, body) + end + end + end + return api_ctx + end + + for i = 1, #plugins, 2 do + local phase_func = plugins[i][phase] + if phase_func then + phase_func(plugins[i + 1], api_ctx) + end + end + + return api_ctx +end + + +function _M.run_global_rules(api_ctx, global_rules, phase_name) + if global_rules and global_rules.values + and #global_rules.values > 0 then + local orig_conf_type = api_ctx.conf_type + local orig_conf_version = api_ctx.conf_version + local orig_conf_id = api_ctx.conf_id + + local plugins = core.tablepool.fetch("plugins", 32, 0) + local values = global_rules.values + for _, global_rule in config_util.iterate_values(values) do + api_ctx.conf_type = "global_rule" + api_ctx.conf_version = global_rule.modifiedIndex + api_ctx.conf_id = global_rule.value.id + + core.table.clear(plugins) + plugins = _M.filter(global_rule, plugins) + if phase_name == "access" then + _M.run_plugin("rewrite", plugins, api_ctx) + _M.run_plugin("access", plugins, api_ctx) + else + _M.run_plugin(phase_name, plugins, api_ctx) + end + end + core.tablepool.release("plugins", plugins) + + api_ctx.conf_type = orig_conf_type + api_ctx.conf_version = orig_conf_version + api_ctx.conf_id = orig_conf_id + end +end + + return _M diff --git a/conf/config-default.yaml b/conf/config-default.yaml index c625f342a765..885007d7a7bb 100644 --- a/conf/config-default.yaml +++ b/conf/config-default.yaml @@ -89,6 +89,8 @@ apisix: role: viewer delete_uri_tail_slash: false # delete the '/' at the end of the URI + global_rule_skip_internal_api: true # does not run global rule in internal apis + # api that path starts with "/apisix" is considered to be internal api router: http: 'radixtree_uri' # radixtree_uri: match route by uri(base on radixtree) # radixtree_host_uri: match route by host + uri(base on radixtree) diff --git a/t/node/global-rule.t b/t/node/global-rule.t index d2bc9fac20f4..ccf97ab24b78 100644 --- a/t/node/global-rule.t +++ b/t/node/global-rule.t @@ -120,7 +120,61 @@ GET /hello -=== TEST 6: delete global rule +=== TEST 6: skip global rule for internal api (not limited) +--- yaml_config +plugins: + - limit-count + - node-status +--- request +GET /apisix/status +--- error_code: 200 +--- no_error_log +[error] + + + +=== TEST 7: skip global rule for internal api - 2 (not limited) +--- yaml_config +plugins: + - limit-count + - node-status +--- request +GET /apisix/status +--- error_code: 200 +--- no_error_log +[error] + + + +=== TEST 8: skip global rule for internal api - 3 (not limited) +--- yaml_config +plugins: + - limit-count + - node-status +--- request +GET /apisix/status +--- error_code: 200 +--- no_error_log +[error] + + + +=== TEST 9: doesn't skip global rule for internal api (should limit) +--- yaml_config +apisix: + global_rule_skip_internal_api: false +plugins: + - limit-count + - node-status +--- request +GET /apisix/status +--- error_code: 503 +--- no_error_log +[error] + + + +=== TEST 10: delete global rule --- config location /t { content_by_lua_block {