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 9bcca2c fix(skywalking): handle conflict between global rule and
route (#4589)
9bcca2c is described below
commit 9bcca2c1f2677d934337ca40bec28923ba4aed43
Author: 罗泽轩 <[email protected]>
AuthorDate: Wed Jul 14 10:56:13 2021 +0800
fix(skywalking): handle conflict between global rule and route (#4589)
Fix #4571
Signed-off-by: spacewander <[email protected]>
---
apisix/init.lua | 11 ++-
apisix/plugin.lua | 17 ++++-
apisix/plugins/skywalking.lua | 1 +
docs/en/latest/plugin-develop.md | 20 ++++-
docs/zh/latest/plugin-develop.md | 18 ++++-
t/plugin/skywalking.t | 158 +++++++++++++++++++++++++++++++++++++++
6 files changed, 215 insertions(+), 10 deletions(-)
diff --git a/apisix/init.lua b/apisix/init.lua
index 3fc3ac4..7c2f092 100644
--- 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"})
@@ -409,9 +409,13 @@ function _M.http_access_phase()
api_ctx.route_id = route.value.id
api_ctx.route_name = route.value.name
+ -- run global rule
+ plugin.run_global_rules(api_ctx, router.global_rules, nil)
+
if route.value.script then
script.load(route, api_ctx)
script.run("access", api_ctx)
+
else
local plugins = plugin.filter(route)
api_ctx.plugins = plugins
@@ -429,6 +433,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
index 7dead03..c8824df 100644
--- 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
index 41fe814..94c7215 100644
--- 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
index 0708428..0f81940 100644
--- 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.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
index fedb29a..1a428f3 100644
--- 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.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/plugin/skywalking.t b/t/plugin/skywalking.t
index e58fb69..5c5f03f 100644
--- 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
