Skip to content

Commit

Permalink
fix(plugins): add more context in error message or log (#84)
Browse files Browse the repository at this point in the history
  • Loading branch information
wklken authored Nov 8, 2024
1 parent 96e68c5 commit ae74265
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 12 deletions.
21 changes: 12 additions & 9 deletions src/apisix/plugins/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
上下文注入,优先级:18000 ~ 19000

- bk-legacy-invalid-params # priority: 18880 # 用于兼容老版本 go1.16 使用 `;` 作为 query string 分隔符
- bk-opentelemetry # priority: 18870 # 这个插件用于 opentelemetry, 需要尽量精准统计全局的耗时,同时需要注入 trace_id/span_id 作为后面所有插件自定义 opentelemetry 上报的 trace_id 即 parent span_id
- bk-opentelemetry # priority: 18870 # 这个插件用于 opentelemetry, 需要尽量精准统计全局的耗时,同时需要注入 trace_id/span_id 作为后面所有插件自定义 opentelemetry 上报的 trace_id 即 parent span_id (abandonned, will be replaced by another plugin)
- bk-not-found-handler # priority: 18860 # 该插件仅适用于由 operator 创建的默认根路由,用以规范化 404 消息。该插件以较高优先级结束请求返回 404 错误信息
- bk-request-id # priority: 18850
- bk-stage-context # priority: 18840
Expand Down Expand Up @@ -63,17 +63,20 @@ proxy 预处理:17000 ~ 17500
- bk-resource-header-rewrite # priority: 17420
- bk-mock # priority: 17150

响应后处理
官方插件

- bk-response-check # priority: 153
- bk-time-cost # priority: 150
- bk-debug # priority: 145
- bk-error-wrapper # priority: 0 # 该插件应默认应用于所有路由
- fault-injection # priority: 11000
- request-validation # priority: 2800
- api-breaker # priority: 1005
- prometheus # priority: 500
- file-logger (priority update) # priority: 399

默认:优先级
响应后处理

- prometheus # priority: 500
- file-logger (priority update) # priority: 399
- bk-response-check # priority: 153
- bk-time-cost # priority: 150
- bk-debug # priority: 145
- bk-error-wrapper # priority: 0 # 该插件应默认应用于所有路由

## 插件开发

Expand Down
1 change: 1 addition & 0 deletions src/apisix/plugins/bk-auth-verify.lua
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ local function get_auth_params_from_header(ctx)

local auth_params = core.json.decode(authorization)
if type(auth_params) ~= "table" then
core.log.error("the invalid X-Bkapi-Authorization: ", core.json.delay_encode(authorization))
return nil, "request header X-Bkapi-Authorization is not a valid JSON"
end

Expand Down
11 changes: 8 additions & 3 deletions src/apisix/plugins/bk-permission.lua
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ function _M.access(conf, ctx)

-- 0. no permission records, return app_no_permission
if pl_types.is_empty(data) then
return errorx.exit_with_apigw_err(ctx, errorx.new_app_no_permission():with_field("reason", reason_no_perm), _M)
local reason = reason_no_perm .. ", bk_app_code=" .. app_code
return errorx.exit_with_apigw_err(
ctx, errorx.new_app_no_permission():with_field("reason", reason), _M
)
end

local now = ngx_time()
Expand All @@ -145,8 +148,9 @@ function _M.access(conf, ctx)
-- if only has one record, means expired
if pl_tablex.size(data) == 1 then
-- expired: permission has expired
local reason = reason_perm_expired .. ", type=gateway_permission, bk_app_code=" .. app_code
return errorx.exit_with_apigw_err(
ctx, errorx.new_app_no_permission():with_field("reason", reason_perm_expired), _M
ctx, errorx.new_app_no_permission():with_field("reason", reason), _M
)
end

Expand All @@ -161,8 +165,9 @@ function _M.access(conf, ctx)
return
else
-- expired: permission has expired
local reason = reason_perm_expired .. ", type=resource_permission, bk_app_code=" .. app_code
return errorx.exit_with_apigw_err(
ctx, errorx.new_app_no_permission():with_field("reason", reason_perm_expired), _M
ctx, errorx.new_app_no_permission():with_field("reason", reason), _M
)
end
end
Expand Down
9 changes: 9 additions & 0 deletions src/apisix/tests/test-bk-permission.lua
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ describe(

local code = plugin.access(conf, ctx)
assert.is_equal(403, code)
assert.is_equal(ctx.var.bk_apigw_error.error.message,
'App has no permission to the resource [reason="no permission, bk_app_code=bk-app-code"]')
assert.stub(cache_fallback.get_with_fallback).was_called(1)
end
)
Expand All @@ -184,6 +186,9 @@ describe(

local code = plugin.access(conf, ctx)
assert.is_equal(403, code)
-- the last return in access
assert.is_equal(ctx.var.bk_apigw_error.error.message,
'App has no permission to the resource [reason="no permission"]')
assert.stub(cache_fallback.get_with_fallback).was_called(1)
end
)
Expand All @@ -197,6 +202,8 @@ describe(

local code = plugin.access(conf, ctx)
assert.is_equal(403, code)
assert.is_equal(ctx.var.bk_apigw_error.error.message,
'App has no permission to the resource [reason="permission has expired, type=gateway_permission, bk_app_code=bk-app-code"]')
assert.stub(cache_fallback.get_with_fallback).was_called(1)
end
)
Expand All @@ -223,6 +230,8 @@ describe(

local code = plugin.access(conf, ctx)
assert.is_equal(403, code)
assert.is_equal(ctx.var.bk_apigw_error.error.message,
'App has no permission to the resource [reason="permission has expired, type=resource_permission, bk_app_code=bk-app-code"]')
assert.stub(cache_fallback.get_with_fallback).was_called(1)
end
)
Expand Down

0 comments on commit ae74265

Please sign in to comment.