Skip to content

Commit

Permalink
fix(skywalking): handle conflict between global rule and route
Browse files Browse the repository at this point in the history
Fix #4571

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
  • Loading branch information
spacewander committed Jul 12, 2021
1 parent 59e9b41 commit e49fd24
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 10 deletions.
11 changes: 8 additions & 3 deletions apisix/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
17 changes: 14 additions & 3 deletions apisix/plugin.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions apisix/plugins/skywalking.lua
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ local _M = {
name = plugin_name,
schema = schema,
attr_schema = attr_schema,
run_policy = "prefer_route",
}


Expand Down
20 changes: 18 additions & 2 deletions docs/en/latest/plugin-develop.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
18 changes: 16 additions & 2 deletions docs/zh/latest/plugin-develop.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ nginx_config:
可能需要在 __apisix/cli/ngx_tpl.lua__ 文件中,对 Nginx 配置文件生成的部分,添加一些你需要的处理。但是这样容易对全局产生影响,根据现有的
插件机制,**我们不建议这样做,除非你已经对代码完全掌握**。
## 插件命名与配置
## 插件命名,优先级和其他
给插件取一个很棒的名字,确定插件的加载优先级,然后在 __conf/config-default.yaml__ 文件中添加上你的插件名。例如 example-plugin 这个插件,
需要在代码里指定插件名称(名称是插件的唯一标识,不可重名),在 __apisix/plugins/example-plugin.lua__ 文件中可以看到:
Expand Down Expand Up @@ -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
Expand Down
158 changes: 158 additions & 0 deletions t/plugin/skywalking.t
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down Expand Up @@ -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

0 comments on commit e49fd24

Please sign in to comment.