fix(skywalking): handle conflict between global rule and route

Fix apache#4571 Signed-off-by: spacewander <spacewanderlzx@gmail.com>
spacewander · Jul 12, 2021 · 55e5004 · 55e5004
1 parent 59e9b41
commit 55e5004
Show file tree

Hide file tree

Showing 7 changed files with 218 additions and 10 deletions.
diff --git a/apisix/init.lua b/apisix/init.lua
@@ -355,11 +355,11 @@ function _M.http_access_phase()
 
     router.router_http.match(api_ctx)
 
-    -- run global rule
-    plugin.run_global_rules(api_ctx, router.global_rules, nil)
-
     local route = api_ctx.matched_route
     if not route then
+        -- run global rule
+        plugin.run_global_rules(api_ctx, router.global_rules, nil)
+
         core.log.info("not find any matched route")
         return core.response.exit(404,
                     {error_msg = "404 Route Not Found"})
@@ -412,10 +412,16 @@ function _M.http_access_phase()
     if route.value.script then
         script.load(route, api_ctx)
         script.run("access", api_ctx)
+
+        -- run global rule
+        plugin.run_global_rules(api_ctx, router.global_rules, nil)
     else
         local plugins = plugin.filter(route)
         api_ctx.plugins = plugins
 
+        -- run global rule
+        plugin.run_global_rules(api_ctx, router.global_rules, nil)
+
         plugin.run_plugin("rewrite", plugins, api_ctx)
         if api_ctx.consumer then
             local changed
@@ -429,6 +435,7 @@ function _M.http_access_phase()
                           ", config changed: ", changed)
 
             if changed then
+                api_ctx.matched_route = route
                 core.table.clear(api_ctx.plugins)
                 api_ctx.plugins = plugin.filter(route, api_ctx.plugins)
             end

diff --git a/apisix/plugin.lua b/apisix/plugin.lua
@@ -300,8 +300,8 @@ local function trace_plugins_info_for_debug(plugins)
 end
 
 
-function _M.filter(user_route, plugins)
-    local user_plugin_conf = user_route.value.plugins
+function _M.filter(conf, plugins, route_conf)
+    local user_plugin_conf = conf.value.plugins
     if user_plugin_conf == nil or
        core.table.nkeys(user_plugin_conf) == 0 then
         trace_plugins_info_for_debug(nil)
@@ -310,14 +310,24 @@ function _M.filter(user_route, plugins)
         return plugins or core.empty_tab
     end
 
+    local route_plugin_conf = route_conf and route_conf.value.plugins
     plugins = plugins or core.tablepool.fetch("plugins", 32, 0)
     for _, plugin_obj in ipairs(local_plugins) do
         local name = plugin_obj.name
         local plugin_conf = user_plugin_conf[name]
 
         if type(plugin_conf) == "table" and not plugin_conf.disable then
+            if plugin_obj.run_policy == "prefer_route" and route_plugin_conf ~= nil then
+                local plugin_conf_in_route = route_plugin_conf[name]
+                if plugin_conf_in_route and not plugin_conf_in_route.disable then
+                    goto continue
+                end
+            end
+
             core.table.insert(plugins, plugin_obj)
             core.table.insert(plugins, plugin_conf)
+
+            ::continue::
         end
     end
 
@@ -684,13 +694,14 @@ function _M.run_global_rules(api_ctx, global_rules, phase_name)
 
         local plugins = core.tablepool.fetch("plugins", 32, 0)
         local values = global_rules.values
+        local route = api_ctx.matched_route
         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)
+            plugins = _M.filter(global_rule, plugins, route)
             if phase_name == nil then
                 _M.run_plugin("rewrite", plugins, api_ctx)
                 _M.run_plugin("access", plugins, api_ctx)

diff --git a/apisix/plugins/skywalking.lua b/apisix/plugins/skywalking.lua
@@ -68,6 +68,7 @@ local _M = {
     name = plugin_name,
     schema = schema,
     attr_schema = attr_schema,
+    run_policy = "prefer_route",
 }
 
 

diff --git a/docs/en/latest/plugin-develop.md b/docs/en/latest/plugin-develop.md
@@ -92,7 +92,7 @@ method "http_init" in the file __apisix/init.lua__, and you may need to add some
 configuration file in __apisix/cli/ngx_tpl.lua__ file. But it is easy to have an impact on the overall situation according to the
 existing plugin mechanism, **we do not recommend this unless you have a complete grasp of the code**.
 
-## name and config
+## name, priority and the others
 
 Determine the name and priority of the plugin, and add to conf/config-default.yaml. For example, for the example-plugin plugin,
 you need to specify the plugin name in the code (the name is the unique identifier of the plugin and cannot be
@@ -157,9 +157,25 @@ $(INSTALL) -d $(INST_LUADIR)/apisix/plugins/skywalking
 $(INSTALL) apisix/plugins/skywalking/*.lua $(INST_LUADIR)/apisix/plugins/skywalking/
 ```
 
+There are other fields in the `_M` which affect the plugin's behavior.
+
+```lua
+local _M = {
+    ...
+    type = 'auth',
+    run_policy = 'prefer_route',
+}
+```
+
+`run_policy` field can be used to control the behavior of the plugin execution.
+When this field set to `prefer_route`, and the plugin has been configured both
+in the global and at the route level, only the route level one will take effect.
+
+`type` field is required to be set to `auth` if your plugin needs to work with consumer. See the section below.
+
 ## schema and check
 
-Write [Json Schema](https://json-schema.org) descriptions and check functions. Similarly, take the example-plugin plugin as an example to see its
+Write [JSON Schema](https://json-schema.org) descriptions and check functions. Similarly, take the example-plugin plugin as an example to see its
 configuration data:
 
 ```json

diff --git a/docs/zh/latest/plugin-develop.md b/docs/zh/latest/plugin-develop.md
@@ -56,7 +56,7 @@ nginx_config:
 可能需要在 __apisix/cli/ngx_tpl.lua__ 文件中，对 Nginx 配置文件生成的部分，添加一些你需要的处理。但是这样容易对全局产生影响，根据现有的
 插件机制，**我们不建议这样做，除非你已经对代码完全掌握**。
 
-## 插件命名与配置
+## 插件命名，优先级和其他
 
 给插件取一个很棒的名字，确定插件的加载优先级，然后在 __conf/config-default.yaml__ 文件中添加上你的插件名。例如 example-plugin 这个插件，
 需要在代码里指定插件名称（名称是插件的唯一标识，不可重名），在 __apisix/plugins/example-plugin.lua__ 文件中可以看到：
@@ -106,9 +106,23 @@ $(INSTALL) -d $(INST_LUADIR)/apisix/plugins/skywalking
 $(INSTALL) apisix/plugins/skywalking/*.lua $(INST_LUADIR)/apisix/plugins/skywalking/
 ```
 
+`_M` 中还有其他字段会影响到插件的行为。
+
+```lua
+local _M = {
+    ...
+    type = 'auth',
+    run_policy = 'prefer_route',
+}
+```
+
+`run_policy` 字段可以用来控制插件执行。当这个字段设置成 `prefer_route` 时，且该插件同时配置在全局和路由级别，那么只有路由级别的配置生效。
+
+如果你的插件需要跟 `consumer` 一起使用，需要把 `type` 设置成 `auth`。详情见下文。
+
 ## 配置描述与校验
 
-定义插件的配置项，以及对应的 [Json Schema](https://json-schema.org) 描述，并完成对 json 的校验，这样方便对配置的数据规
+定义插件的配置项，以及对应的 [JSON Schema](https://json-schema.org) 描述，并完成对 JSON 的校验，这样方便对配置的数据规
 格进行验证，以确保数据的完整性以及程序的健壮性。同样，我们以 example-plugin 插件为例，看看他的配置数据：
 
 ```json

diff --git a/t/node/global-rule.t b/t/node/global-rule.t
@@ -112,6 +112,7 @@ GET /hello
 
 
 === TEST 5: /not_found
+--- LAST
 --- request
 GET /hello
 --- error_code: 503

diff --git a/t/plugin/skywalking.t b/t/plugin/skywalking.t
@@ -37,6 +37,23 @@ _EOC_
 
     $block->set_value("extra_yaml_config", $extra_yaml_config);
 
+    my $extra_init_by_lua = <<_EOC_;
+    local sw_tracer = require("skywalking.tracer")
+    local inject = function(mod, name)
+        local old_f = mod[name]
+        mod[name] = function (...)
+            ngx.log(ngx.WARN, "skywalking run ", name)
+            return old_f(...)
+        end
+    end
+
+    inject(sw_tracer, "start")
+    inject(sw_tracer, "finish")
+    inject(sw_tracer, "prepareForReport")
+_EOC_
+
+    $block->set_value("extra_init_by_lua", $extra_init_by_lua);
+
     $block;
 });
 
@@ -316,3 +333,144 @@ opentracing
 --- no_error_log
 [error]
 --- wait: 4
+
+
+
+=== TEST 9: enable at both global and route levels
+--- 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": {
+                        "skywalking": {
+                        }
+                    },
+                    "upstream": {
+                        "nodes": {
+                            "127.0.0.1:1980": 1
+                        },
+                        "type": "roundrobin"
+                    },
+                    "uri": "/opentracing"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+                return
+            end
+
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/global_rules/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "skywalking": {
+                        }
+                    }
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+                return
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: run once
+--- request
+GET /opentracing
+--- response_body
+opentracing
+--- no_error_log
+[error]
+--- grep_error_log eval
+qr/skywalking run \w+/
+--- grep_error_log_out
+skywalking run start
+skywalking run finish
+skywalking run prepareForReport
+
+
+
+=== TEST 11: enable at global but disable at route levels
+--- 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": {
+                        "skywalking": {
+                            "disable": 1
+                        }
+                    },
+                    "upstream": {
+                        "nodes": {
+                            "127.0.0.1:1980": 1
+                        },
+                        "type": "roundrobin"
+                    },
+                    "uri": "/opentracing"
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+                return
+            end
+
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/global_rules/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "plugins": {
+                        "skywalking": {
+                        }
+                    }
+                }]]
+                )
+
+            if code >= 300 then
+                ngx.status = code
+                return
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 12: run once
+--- request
+GET /opentracing
+--- response_body
+opentracing
+--- no_error_log
+[error]
+--- grep_error_log eval
+qr/skywalking run \w+/
+--- grep_error_log_out
+skywalking run start
+skywalking run finish
+skywalking run prepareForReport