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.

Sign up for GitHub

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

backport: fix no valid ip found 502 #55

Merged
merged 3 commits into from
Oct 10, 2023
Merged

backport: fix no valid ip found 502 #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec2789f
remove verify by bkauth access_token (#50)
alex-smile Oct 9, 2023
3ff0005
fix(build/patches): add patch for parse_domain_for_nodes in apisix/ut…
wklken Oct 10, 2023
cc90084
fix(build/patches): patch err no_valid_ip_found as 502 (#54)
wklken Oct 10, 2023
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
22 changes: 14 additions & 8 deletions src/apisix/editions/ee/plugins/bk-cache/access-token.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
--
local core = require("apisix.core")
local access_token_define = require("apisix.plugins.bk-define.access-token")
local bkauth_component = require("apisix.plugins.bk-components.bkauth")
-- local bkauth_component = require("apisix.plugins.bk-components.bkauth")
local ssm_component = require("apisix.plugins.bk-components.ssm")

local ACCESS_TOKEN_CACHE_TTL = 600
Expand All @@ -34,12 +34,15 @@ local access_token_lrucache = core.lrucache.new(
local _M = {}

local function get_access_token(access_token)
local bkauth_token, err = bkauth_component.verify_access_token(access_token)
if bkauth_token ~= nil then
return {
token = access_token_define.new(bkauth_token.bk_app_code, bkauth_token.username, bkauth_token.expires_in),
}
end
local err
err = "authentication based on access_token is not supported"

-- local bkauth_token, err = bkauth_component.verify_access_token(access_token)
-- if bkauth_token ~= nil then
-- return {
-- token = access_token_define.new(bkauth_token.bk_app_code, bkauth_token.username, bkauth_token.expires_in)
-- }
-- end

if ssm_component.is_configured() then
local ssm_token
Expand All @@ -58,7 +61,10 @@ end

function _M.get_access_token(access_token)
local key = access_token
local result = access_token_lrucache(key, nil, get_access_token, access_token)
local result, err = access_token_lrucache(key, nil, get_access_token, access_token)
if result == nil then
return nil, err
end
return result.token, result.err
end

Expand Down
3 changes: 1 addition & 2 deletions src/apisix/editions/ee/plugins/bk-cache/bk-token.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ function _M.get_username_by_bk_token(bk_token)
local result, err = bk_token_lrucache(key, nil, bklogin_component.get_username_by_bk_token, bk_token)
if result == nil then
return nil, err
else
return result.username, result.error_message
end
return result.username, result.error_message
end

return _M
71 changes: 15 additions & 56 deletions src/apisix/editions/ee/tests/bk-cache/test-access-token.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,23 @@
-- 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 access_token_cache = require("apisix.plugins.bk-cache.access-token")
local bkauth_component = require("apisix.plugins.bk-components.bkauth")
local ssm_component = require("apisix.plugins.bk-components.ssm")
local uuid = require("resty.jit-uuid")

describe(
"access_token cache", function()

local bkauth_verify_access_token_result
local bkauth_verify_access_token_err
local ssm_verify_access_token_result
local ssm_verify_access_token_err
local ssm_is_configured

before_each(
function()
bkauth_verify_access_token_result = nil
bkauth_verify_access_token_err = nil
ssm_verify_access_token_result = nil
ssm_verify_access_token_err = nil
ssm_is_configured = false

stub(
bkauth_component, "verify_access_token", function()
return bkauth_verify_access_token_result, bkauth_verify_access_token_err
end
)

stub(
ssm_component, "verify_access_token", function()
return ssm_verify_access_token_result, ssm_verify_access_token_err
Expand All @@ -60,7 +48,6 @@ describe(

after_each(
function()
bkauth_component.verify_access_token:revert()
ssm_component.verify_access_token:revert()
ssm_component.is_configured:revert()
end
Expand All @@ -69,30 +56,7 @@ describe(
context(
"local get_access_token", function()
it(
"bkauth verify ok", function()
bkauth_verify_access_token_result = {
bk_app_code = "my-app",
username = "admin",
expires_in = 10,
}
bkauth_verify_access_token_err = nil

local result = access_token_cache._get_access_token("fake-access-token")
assert.is_same(
result.token, {
app_code = "my-app",
user_id = "admin",
expires_in = 10,
}
)
assert.is_nil(result.err)
end
)

it(
"bkauth verify fail, ssm verify ok", function()
bkauth_verify_access_token_result = nil
bkauth_verify_access_token_err = "bkauth err"
"ssm verify ok", function()
ssm_verify_access_token_result = {
bk_app_code = "my-foo",
username = "kitty",
Expand All @@ -114,9 +78,7 @@ describe(
)

it(
"bkauth verify fail, ssm verify fail", function()
bkauth_verify_access_token_result = nil
bkauth_verify_access_token_err = "bkauth error"
"ssm verify fail, and is configured", function()
ssm_verify_access_token_result = nil
ssm_verify_access_token_err = "ssm error"
ssm_is_configured = true
Expand All @@ -128,23 +90,19 @@ describe(
)

it(
"bkauth verify fail, ssm is not configured", function()
bkauth_verify_access_token_result = nil
bkauth_verify_access_token_err = "bkauth error"
"ssm verify fail, but not configured", function()
ssm_verify_access_token_result = nil
ssm_verify_access_token_err = "ssm error"
ssm_is_configured = nil

local result = access_token_cache._get_access_token("fake-access-token")
assert.is_nil(result.token)
assert.is_equal(result.err, "bkauth error")
assert.is_equal(result.err, "authentication based on access_token is not supported")
end
)

it(
"bkauth verify fail, ssm is not configured", function()
bkauth_verify_access_token_result = nil
bkauth_verify_access_token_err = "bkauth error"
"ssm verify ok, but not configured", function()
ssm_verify_access_token_result = {
bk_app_code = "my-foo",
username = "kitty",
Expand All @@ -155,7 +113,7 @@ describe(

local result = access_token_cache._get_access_token("fake-access-token")
assert.is_nil(result.token)
assert.is_equal(result.err, "bkauth error")
assert.is_equal(result.err, "authentication based on access_token is not supported")
end
)
end
Expand All @@ -165,7 +123,8 @@ describe(
"get_access_token", function()
it(
"get access_token from cache, ok", function()
bkauth_verify_access_token_result = {
ssm_is_configured = true
ssm_verify_access_token_result = {
bk_app_code = "my-app",
username = "admin",
expires_in = 100,
Expand All @@ -181,15 +140,15 @@ describe(
}
)
assert.is_nil(err)
assert.stub(bkauth_component.verify_access_token).was_called_with(access_token)
assert.stub(ssm_component.verify_access_token).was_called_with(access_token)

-- get from cache
access_token_cache.get_access_token(access_token)
assert.stub(bkauth_component.verify_access_token).was_called(1)
assert.stub(ssm_component.verify_access_token).was_called(1)

-- get from func
access_token_cache.get_access_token(uuid.generate_v4())
assert.stub(bkauth_component.verify_access_token).was_called(2)
assert.stub(ssm_component.verify_access_token).was_called(2)
end
)

Expand All @@ -199,20 +158,20 @@ describe(
bkauth_verify_access_token_err = "bkauth error"
ssm_verify_access_token_result = nil
ssm_verify_access_token_err = "ssm error"
ssm_is_configured = false
ssm_is_configured = true

local access_token = uuid.generate_v4()
local result, err = access_token_cache.get_access_token(access_token)
assert.is_nil(result)
assert.is_equal(err, "bkauth error")
assert.is_not_nil(err)

-- get from cache
access_token_cache.get_access_token(access_token)
assert.stub(bkauth_component.verify_access_token).was_called(1)
assert.stub(ssm_component.verify_access_token).was_called(1)

-- get from func
access_token_cache.get_access_token(uuid.generate_v4())
assert.stub(bkauth_component.verify_access_token).was_called(2)
assert.stub(ssm_component.verify_access_token).was_called(2)
end
)
end
Expand Down
19 changes: 19 additions & 0 deletions src/build/patches/002_upstream_parse_domain_for_nodes.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
diff --git a/apisix/utils/upstream.lua b/apisix/utils/upstream.lua
index c39d4cce..5d23ce49 100644
--- a/apisix/utils/upstream.lua
+++ b/apisix/utils/upstream.lua
@@ -82,6 +82,14 @@ local function parse_domain_for_nodes(nodes)
core.table.insert(new_nodes, node)
end
end
+
+ -- patch for: https://github.com/apache/apisix/issues/10093#issuecomment-1738381865
+ if #new_nodes == 0 then
+ local err = "no valid ip found"
+ core.log.error("parse domain for nodes: ", core.json.delay_encode(nodes), " error: ", err)
+ return nil, err
+ end
+
return new_nodes
end
_M.parse_domain_for_nodes = parse_domain_for_nodes
17 changes: 17 additions & 0 deletions src/build/patches/003_patch_no_valid_ip_found_502.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
diff --git a/apisix/init.lua b/apisix/init.lua
index 388af426..9899c332 100644
--- a/apisix/init.lua
+++ b/apisix/init.lua
@@ -431,6 +431,12 @@ function _M.handle_upstream(api_ctx, route, enable_websocket)
route, err = parse_domain_in_route(route)
if err then
core.log.error("failed to get resolved route: ", err)
+
+ -- if the dns resolve get no valid ips, return 502 , treat it as `pick_server` fail
+ if err == "no valid ip found" then
+ return core.response.exit(502)
+ end
+
return core.response.exit(500)
end