Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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