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

Jump to bottom

fix: validate plugin configuration in the DP #2856

Merged
merged 3 commits into from
Nov 28, 2020
Merged

fix: validate plugin configuration in the DP #2856

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2308af8
fix: validate plugin configuration in the DP
spacewander Nov 26, 2020
0e00766
tweak test expect for new jsonschema lib
spacewander Nov 27, 2020
4aa3fe5
style
spacewander Nov 27, 2020
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
66 changes: 4 additions & 62 deletions apisix/admin/plugins.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@
--
local require = require
local core = require("apisix.core")
local local_plugins = require("apisix.plugin").plugins_hash
local stream_local_plugins = require("apisix.plugin").stream_plugins_hash
local pairs = pairs
local check_schema = require("apisix.plugin").check_schema
local stream_check_schema = require("apisix.plugin").stream_check_schema
local ipairs = ipairs
local pcall = pcall
local type = type
local table_sort = table.sort
local table_insert = table.insert
local get_uri_args = ngx.req.get_uri_args
Expand All @@ -30,68 +28,12 @@ local _M = {}


function _M.check_schema(plugins_conf, schema_type)
for name, plugin_conf in pairs(plugins_conf) do
core.log.info("check plugin scheme, name: ", name, ", configurations: ",
core.json.delay_encode(plugin_conf, true))
if type(plugin_conf) ~= "table" then
return false, "invalid plugin conf " ..
core.json.encode(plugin_conf, true) ..
" for plugin [" .. name .. "]"
end

local plugin_obj = local_plugins[name]
if not plugin_obj then
return false, "unknown plugin [" .. name .. "]"
end

if plugin_obj.check_schema then
local disable = plugin_conf.disable
plugin_conf.disable = nil

local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
if not ok then
return false, "failed to check the configuration of plugin "
.. name .. " err: " .. err
end

plugin_conf.disable = disable
end
end

return true
return check_schema(plugins_conf, schema_type, false)
end


function _M.stream_check_schema(plugins_conf, schema_type)
for name, plugin_conf in pairs(plugins_conf) do
core.log.info("check stream plugin scheme, name: ", name,
": ", core.json.delay_encode(plugin_conf, true))
if type(plugin_conf) ~= "table" then
return false, "invalid plugin conf " ..
core.json.encode(plugin_conf, true) ..
" for plugin [" .. name .. "]"
end

local plugin_obj = stream_local_plugins[name]
if not plugin_obj then
return false, "unknown plugin [" .. name .. "]"
end

if plugin_obj.check_schema then
local disable = plugin_conf.disable
plugin_conf.disable = nil

local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
if not ok then
return false, "failed to check the configuration of "
.. "stream plugin [" .. name .. "]: " .. err
end

plugin_conf.disable = disable
end
end

return true
return stream_check_schema(plugins_conf, schema_type, false)
end


Expand Down
28 changes: 28 additions & 0 deletions apisix/core/config_etcd.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,14 @@ local function sync_data(self)
end
end

if data_valid and self.checker then
data_valid, err = self.checker(item.value)
if not data_valid then
log.error("failed to check item data of [", self.key,
"] err:", err, " ,val: ", json.delay_encode(item.value))
end
end

if data_valid then
changed = true
insert_tab(self.values, item)
Expand Down Expand Up @@ -256,6 +264,14 @@ local function sync_data(self)
end
end

if data_valid and self.checker then
data_valid, err = self.checker(item.value)
if not data_valid then
log.error("failed to check item data of [", self.key,
"] err:", err, " ,val: ", json.delay_encode(item.value))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad indentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

end
end

if data_valid then
changed = true
insert_tab(self.values, item)
Expand Down Expand Up @@ -349,6 +365,16 @@ local function sync_data(self)
return false, "failed to check item data of ["
.. self.key .. "] err:" .. err
end

if self.checker then
local ok, err = self.checker(res.value)
if not ok then
self:upgrade_version(res.modifiedIndex)

return false, "failed to check item data of ["
.. self.key .. "] err:" .. err
end
end
end

self:upgrade_version(res.modifiedIndex)
Expand Down Expand Up @@ -538,13 +564,15 @@ function _M.new(key, opts)
local filter_fun = opts and opts.filter
local timeout = opts and opts.timeout
local single_item = opts and opts.single_item
local checker = opts and opts.checker

local obj = setmetatable({
etcd_cli = nil,
etcd_conf = etcd_conf,
key = key and prefix .. key,
automatic = automatic,
item_schema = item_schema,
checker = checker,
sync_times = 0,
running = true,
conf_version = 0,
Expand Down
24 changes: 21 additions & 3 deletions apisix/core/config_yaml.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,14 @@ local function sync_data(self)
log.error("failed to check item data of [", self.key,
"] err:", err, " ,val: ", json.delay_encode(item))
end

if data_valid and self.checker then
data_valid, err = self.checker(item)
if not data_valid then
log.error("failed to check item data of [", self.key,
"] err:", err, " ,val: ", json.delay_encode(item))
end
end
end

if data_valid then
Expand All @@ -180,8 +188,8 @@ local function sync_data(self)
if type(item) ~= "table" then
data_valid = false
log.error("invalid item data of [", self.key .. "/" .. id,
"], val: ", json.delay_encode(item),
", it shoud be a object")
"], val: ", json.delay_encode(item),
", it shoud be a object")
end

local key = item.id or "arr_" .. i
Expand All @@ -192,7 +200,15 @@ local function sync_data(self)
data_valid, err = check_schema(self.item_schema, item)
if not data_valid then
log.error("failed to check item data of [", self.key,
"] err:", err, " ,val: ", json.delay_encode(item))
"] err:", err, " ,val: ", json.delay_encode(item))
end
end

if data_valid and self.checker then
data_valid, err = self.checker(item)
if not data_valid then
log.error("failed to check item data of [", self.key,
"] err:", err, " ,val: ", json.delay_encode(item))
end
end

Expand Down Expand Up @@ -287,6 +303,7 @@ function _M.new(key, opts)
local item_schema = opts and opts.item_schema
local filter_fun = opts and opts.filter
local single_item = opts and opts.single_item
local checker = opts and opts.checker

-- like /routes and /upstreams, remove first char `/`
if key then
Expand All @@ -296,6 +313,7 @@ function _M.new(key, opts)
local obj = setmetatable({
automatic = automatic,
item_schema = item_schema,
checker = checker,
sync_times = 0,
running = true,
conf_version = 0,
Expand Down
2 changes: 2 additions & 0 deletions apisix/http/router/radixtree_host_uri.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
local require = require
local router = require("resty.radixtree")
local core = require("apisix.core")
local plugin_checker = require("apisix.plugin").plugin_checker
local ipairs = ipairs
local type = type
local error = error
Expand Down Expand Up @@ -160,6 +161,7 @@ function _M.init_worker(filter)
user_routes, err = core.config.new("/routes", {
automatic = true,
item_schema = core.schema.route,
checker = plugin_checker,
filter = filter,
})
if not user_routes then
Expand Down
2 changes: 2 additions & 0 deletions apisix/http/router/radixtree_uri.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
local require = require
local router = require("resty.radixtree")
local core = require("apisix.core")
local plugin_checker = require("apisix.plugin").plugin_checker
local ipairs = ipairs
local type = type
local error = error
Expand Down Expand Up @@ -116,6 +117,7 @@ function _M.init_worker(filter)
user_routes, err = core.config.new("/routes", {
automatic = true,
item_schema = core.schema.route,
checker = plugin_checker,
filter = filter,
})
if not user_routes then
Expand Down
2 changes: 2 additions & 0 deletions apisix/http/service.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
-- limitations under the License.
--
local core = require("apisix.core")
local plugin_checker = require("apisix.plugin").plugin_checker
local ipairs = ipairs
local services
local error = error
Expand Down Expand Up @@ -87,6 +88,7 @@ function _M.init_worker()
services, err = core.config.new("/services", {
automatic = true,
item_schema = core.schema.service,
checker = plugin_checker,
filter = filter,
})
if not services then
Expand Down
98 changes: 98 additions & 0 deletions apisix/plugin.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,4 +498,102 @@ function _M.get(name)
end


local function check_schema(plugins_conf, schema_type, skip_disabled_plugin)
for name, plugin_conf in pairs(plugins_conf) do
core.log.info("check plugin scheme, name: ", name, ", configurations: ",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: scheme => schema.

core.json.delay_encode(plugin_conf, true))
if type(plugin_conf) ~= "table" then
return false, "invalid plugin conf " ..
core.json.encode(plugin_conf, true) ..
" for plugin [" .. name .. "]"
end

local plugin_obj = local_plugins_hash[name]
if not plugin_obj then
if skip_disabled_plugin then
goto CONTINUE
else
return false, "unknown plugin [" .. name .. "]"
end
end

if plugin_obj.check_schema then
local disable = plugin_conf.disable
plugin_conf.disable = nil

local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
if not ok then
return false, "failed to check the configuration of plugin "
.. name .. " err: " .. err
end

plugin_conf.disable = disable
end

::CONTINUE::
end

return true
end
_M.check_schema = check_schema


local function stream_check_schema(plugins_conf, schema_type, skip_disabled_plugin)
for name, plugin_conf in pairs(plugins_conf) do
core.log.info("check stream plugin scheme, name: ", name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

": ", core.json.delay_encode(plugin_conf, true))
if type(plugin_conf) ~= "table" then
return false, "invalid plugin conf " ..
core.json.encode(plugin_conf, true) ..
" for plugin [" .. name .. "]"
end

local plugin_obj = stream_local_plugins_hash[name]
if not plugin_obj then
if skip_disabled_plugin then
goto CONTINUE
else
return false, "unknown plugin [" .. name .. "]"
end
end

if plugin_obj.check_schema then
local disable = plugin_conf.disable
plugin_conf.disable = nil

local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
if not ok then
return false, "failed to check the configuration of "
.. "stream plugin [" .. name .. "]: " .. err
end

plugin_conf.disable = disable
end

::CONTINUE::
end

return true
end
_M.stream_check_schema = stream_check_schema


function _M.plugin_checker(item)
if item.plugins then
return check_schema(item.plugins, nil, true)
end

return true
end


function _M.stream_plugin_checker(item)
if item.plugins then
return stream_check_schema(item.plugins, nil, true)
end

return true
end


return _M
13 changes: 12 additions & 1 deletion apisix/plugins/openid-connect.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ local _M = {
}

function _M.check_schema(conf)
if conf.ssl_verify == "no" then
-- we used to set 'ssl_verify' to "no"
conf.ssl_verify = false
end

local ok, err = core.schema.check(schema, conf)
if not ok then
return false, err
Expand All @@ -63,7 +68,9 @@ function _M.check_schema(conf)
conf.scope = "openid"
end
if not conf.ssl_verify then
conf.ssl_verify = "no"
-- we need to use a boolean default value here
-- so that the schema can pass check in the DP
conf.ssl_verify = false
end
if not conf.timeout then
conf.timeout = 3
Expand Down Expand Up @@ -144,6 +151,10 @@ function _M.access(plugin_conf, ctx)
if not conf.redirect_uri then
conf.redirect_uri = ctx.var.request_uri
end
if not conf.ssl_verify then
-- openidc use "no" to disable ssl verification
conf.ssl_verify = "no"
end

local response, err
if conf.introspection_endpoint or conf.public_key then
Expand Down
Loading