Skip to content

Commit

Permalink
Merge pull request #38 from TencentBlueKing/master
Browse files Browse the repository at this point in the history
backport: bk-header-rewrite bugfix and component error wrap
  • Loading branch information
wklken authored Aug 9, 2023
2 parents 12730ff + 5496f0c commit e3fa198
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 32 deletions.
5 changes: 2 additions & 3 deletions src/apisix/editions/ee/plugins/bk-components/bklogin.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
-- We undertake not to change the open source license (MIT license) applicable
-- to the current version of the project delivered to anyone in the future.
--

local pl_types = require("pl.types")
local http = require("resty.http")
local core = require("apisix.core")
Expand All @@ -36,7 +35,7 @@ local _M = {
function _M.get_username_by_bk_token(bk_token)
if pl_types.is_empty(_M.host) then
return {
err = "login host is not configured.",
err = "server error: login host is not configured.",
}
end

Expand All @@ -63,7 +62,7 @@ function _M.get_username_by_bk_token(bk_token)
if result == nil then
core.log.error(string_format("failed to request %s, err: %s", url, _err))
return {
err = string_format("failed to request %s, %s", url, _err),
err = string_format("failed to request third-party api, url: %s, err: %s", url, _err),
}
end

Expand Down
5 changes: 2 additions & 3 deletions src/apisix/editions/ee/plugins/bk-components/ssm.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
-- We undertake not to change the open source license (MIT license) applicable
-- to the current version of the project delivered to anyone in the future.
--

local pl_types = require("pl.types")
local http = require("resty.http")
local core = require("apisix.core")
Expand All @@ -42,7 +41,7 @@ end

function _M.verify_access_token(access_token)
if pl_types.is_empty(_M.host) then
return nil, "ssm host is not configured."
return nil, "server error: ssm host is not configured."
end

local url = bk_core.url.url_single_joining_slash(_M.host, VERIFY_ACCESS_TOKEN_URL)
Expand All @@ -69,7 +68,7 @@ function _M.verify_access_token(access_token)
local result, _err = bk_components_utils.parse_response(res, err, true)
if result == nil then
core.log.error(string_format("failed to request %s, err: %s", url, _err))
return nil, string_format("failed to request %s, %s", url, _err)
return nil, string_format("failed to request third-party api, url: %s, err: %s", url, _err)
end

if result.code ~= 0 then
Expand Down
28 changes: 22 additions & 6 deletions src/apisix/plugins/bk-auth-validate.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
-- We undertake not to change the open source license (MIT license) applicable
-- to the current version of the project delivered to anyone in the future.
--

-- # bk-auth-validate
--
-- Validate current request and return an error response if the validation is required
Expand All @@ -29,7 +28,7 @@
-- be skipped.
--
-- This plugin depends on:
-- * bk-resource-auth: To determine whether a verified data is necessary.
-- * bk-resource-context: To determine whether a verified data is necessary.
-- * bk-auth-verify: Get the verified bk_app and bk_user objects.
-- * bk-verified-user-exempted-apps: Get the whitelist configurations of current gateway.
--
Expand Down Expand Up @@ -94,8 +93,8 @@ end

local function validate_user(bk_resource_id, bk_resource_auth, user, app, verified_user_exempted_apps)
if (not bk_resource_auth:get_verified_user_required() or
is_app_exempted_from_verified_user(app:get_app_code(), bk_resource_id, verified_user_exempted_apps) or
bk_resource_auth:get_skip_user_verification()) then
is_app_exempted_from_verified_user(app:get_app_code(), bk_resource_id, verified_user_exempted_apps) or
bk_resource_auth:get_skip_user_verification()) then
return nil
end

Expand All @@ -106,6 +105,14 @@ local function validate_user(bk_resource_id, bk_resource_auth, user, app, verifi
return user.valid_error_message
end

---@param err string
---@return boolean
local function is_server_error(err)
return core.string.has_prefix(err, "failed to request third-party api") or
core.string.has_prefix(err, "server error: ") or
core.string.has_suffix(err, "please contact the API Gateway developer to handle")
end

function _M.rewrite(conf, ctx) -- luacheck: no unused
-- Return directly if "bk-resource-auth" is not loaded by checking "bk_resource_auth"
if ctx.var.bk_resource_auth == nil then
Expand All @@ -114,22 +121,31 @@ function _M.rewrite(conf, ctx) -- luacheck: no unused

local err = validate_app(ctx.var.bk_resource_auth, ctx.var.bk_app)
if err ~= nil then
return errorx.exit_with_apigw_err(ctx, errorx.new_invalid_args():with_field("reason", err), _M)
if is_server_error(err) then
return errorx.exit_with_apigw_err(ctx, errorx.new_internal_server_error():with_field("reason", err), _M)
else
return errorx.exit_with_apigw_err(ctx, errorx.new_invalid_args():with_field("reason", err), _M)
end
end

err = validate_user(
ctx.var.bk_resource_id, ctx.var.bk_resource_auth, ctx.var.bk_user, ctx.var.bk_app,
ctx.var.verified_user_exempted_apps
)
if err ~= nil then
return errorx.exit_with_apigw_err(ctx, errorx.new_invalid_args():with_field("reason", err), _M)
if is_server_error(err) then
return errorx.exit_with_apigw_err(ctx, errorx.new_internal_server_error():with_field("reason", err), _M)
else
return errorx.exit_with_apigw_err(ctx, errorx.new_invalid_args():with_field("reason", err), _M)
end
end
end

if _TEST then -- luacheck: ignore
_M._is_app_exempted_from_verified_user = is_app_exempted_from_verified_user
_M._validate_app = validate_app
_M._validate_user = validate_user
_M._is_server_error = is_server_error
end

return _M
3 changes: 1 addition & 2 deletions src/apisix/plugins/bk-auth-verify.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
-- We undertake not to change the open source license (MIT license) applicable
-- to the current version of the project delivered to anyone in the future.
--

-- # bk-auth-verify
--
-- This plugin sets the authentication-related properties to the current context, including
Expand All @@ -27,7 +26,7 @@
-- the plugin use anonymous application and user objects.
--
-- This plugin depends on:
-- * bk-resource-auth: To determine whether the user verification should be skipped.
-- * bk-resource-context: To determine whether the user verification should be skipped.
--
local pl_types = require("pl.types")
local core = require("apisix.core")
Expand Down
19 changes: 9 additions & 10 deletions src/apisix/plugins/bk-components/bkauth.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
-- We undertake not to change the open source license (MIT license) applicable
-- to the current version of the project delivered to anyone in the future.
--

local pl_types = require("pl.types")
local http = require("resty.http")
local core = require("apisix.core")
Expand Down Expand Up @@ -45,7 +44,7 @@ function _M.verify_app_secret(app_code, app_secret)
return {
existed = false,
verified = false,
err = "bkauth host is not configured.",
err = "server error: bkauth host is not configured.",
}
end

Expand Down Expand Up @@ -75,7 +74,7 @@ function _M.verify_app_secret(app_code, app_secret)
return {
existed = false,
verified = false,
err = string_format("failed to request %s, %s", url, err),
err = string_format("failed to request third-party api, url: %s, err: %s", url, err),
}
end

Expand All @@ -92,7 +91,7 @@ function _M.verify_app_secret(app_code, app_secret)
return {
existed = false,
verified = false,
err = string_format("failed to request %s, response is not valid json", url),
err = string_format("failed to request third-party api, response is not valid json, url: %s", url),
}
end

Expand All @@ -113,7 +112,7 @@ end
function _M.list_app_secrets(app_code)
if pl_types.is_empty(_M.host) then
return {
err = "bkauth host is not configured.",
err = "server error: bkauth host is not configured.",
}
end

Expand All @@ -136,7 +135,7 @@ function _M.list_app_secrets(app_code)
if not (res and res.body) then
core.log.error(string_format("failed to request %s, err: %s", url, err))
return {
err = string_format("failed to request %s, %s", url, err),
err = string_format("failed to request third-party api, url: %s, err: %s", url, err),
}
end

Expand All @@ -150,7 +149,7 @@ function _M.list_app_secrets(app_code)
local result = core.json.decode(res.body)
if result == nil then
return {
err = string_format("failed to request %s, response is not valid json", url),
err = string_format("failed to request third-party api, response is not valid json, url: %s", url),
}
end

Expand All @@ -172,7 +171,7 @@ end

function _M.verify_access_token(access_token)
if pl_types.is_empty(_M.host) then
return nil, "bkauth host is not configured."
return nil, "server error: bkauth host is not configured."
end

local url = bk_core.url.url_single_joining_slash(_M.host, VERIFY_ACCESS_TOKEN_URL)
Expand All @@ -199,12 +198,12 @@ function _M.verify_access_token(access_token)

if not (res and res.body) then
core.log.error(string_format("failed to request %s, err: %s", url, err))
return nil, string_format("failed to request %s, %s", url, err)
return nil, string_format("failed to request third-party api, url: %s, err: %s", url, err)
end

local result = core.json.decode(res.body)
if result == nil then
return nil, string_format("failed to request %s, response is not valid json", url)
return nil, string_format("failed to request third-party api, response is not valid json, url: %s", url)
end

if result.code ~= 0 or res.status ~= 200 then
Expand Down
3 changes: 0 additions & 3 deletions src/apisix/plugins/bk-stage-header-rewrite.lua
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ local schema = {
properties = {
add = {
type = "object",
minProperties = 1,
patternProperties = {
["^[^:]+$"] = {
oneOf = {
Expand All @@ -51,7 +50,6 @@ local schema = {
},
set = {
type = "object",
minProperties = 1,
patternProperties = {
["^[^:]+$"] = {
oneOf = {
Expand All @@ -63,7 +61,6 @@ local schema = {
},
remove = {
type = "array",
minItems = 1,
items = {
type = "string",
-- "Referer"
Expand Down
54 changes: 53 additions & 1 deletion src/apisix/t/bk-stage-header-rewrite.t
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ passed
"bk-stage-header-rewrite": {
"set": {
"X-Api-Version": "v2"
}
},
"remove": []
}
},
"upstream": {
Expand Down Expand Up @@ -354,3 +355,54 @@ uri: /uri/plugin_proxy_rewrite
host: localhost
x-api-engine: APISIX
x-real-ip: 127.0.0.1

=== TEST 13: remove header set empty
--- 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": {
"bk-proxy-rewrite": {
"uri": "/uri/plugin_proxy_rewrite"
},
"bk-stage-header-rewrite": {
"add": {},
"set": {},
"remove": ["X-Api-Test"]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed

=== TEST 14: remove header set empty ok
--- request
GET /hello HTTP/1.1
--- more_headers
X-Api-Test: foo
X-Api-Engine: bar
--- response_body
uri: /uri/plugin_proxy_rewrite
host: localhost
x-api-engine: bar
x-real-ip: 127.0.0.1
8 changes: 6 additions & 2 deletions src/apisix/tests/bk-rate-limit/test-init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ describe(
"allow_degradation=true", function()
conf.allow_degradation = true

local code = ratelimit.rate_limit(conf, ctx, "rate_limit", "key", 1, 60)
local code = ratelimit.rate_limit(
conf, ctx, "rate_limit_with_create_limit_obj_error", "key-1", 1, 60
)
assert.is_nil(code)
end
)
Expand All @@ -102,7 +104,9 @@ describe(
"allow_degradation=false", function()
conf.allow_degradation = false

local code = ratelimit.rate_limit(conf, ctx, "rate_limit", "key", 1, 60)
local code = ratelimit.rate_limit(
conf, ctx, "rate_limit_with_create_limit_obj_error", "key-2", 1, 60
)
assert.is_equal(code, 500)
end
)
Expand Down
Loading

0 comments on commit e3fa198

Please sign in to comment.