change: global rules should not be executed on the internal api (#3396)

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
 

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,14 +287,19 @@ 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
     end
 
     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
 
 

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

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)

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 {