From a4634da49e563e6bbdbb7a41bb0e00a539dfc8bd Mon Sep 17 00:00:00 2001 From: YuanSheng Wang Date: Mon, 17 Jun 2019 15:05:19 +0800 Subject: [PATCH] feature: used json schema to check the configuration of `limit-count` plugin. (#116) * feature: used json schema to check the configuration of `limit-count` plugin. * travis: if there was any errors, exist directly. --- .travis.yml | 2 +- lua/apisix/admin/plugins.lua | 33 ++++ lua/apisix/admin/routes.lua | 15 ++ lua/apisix/admin/services.lua | 15 ++ lua/apisix/plugin.lua | 25 +-- lua/apisix/plugins/limit-count.lua | 3 +- t/plugin/limit-count.t | 247 +++++++++++++++++++++++++++++ 7 files changed, 329 insertions(+), 11 deletions(-) create mode 100644 lua/apisix/admin/plugins.lua diff --git a/.travis.yml b/.travis.yml index ea1f7b92..309d096f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -40,5 +40,5 @@ script: - sudo apisix init_etcd - cd test-nginx && (sudo cpanm . > build.log 2>&1 || (cat build.log && exit 1)) && cd .. - export PATH=$OPENRESTY_PREFIX/nginx/sbin:$OPENRESTY_PREFIX/luajit/bin:$PATH - - make check + - make check || exit 1 - make test diff --git a/lua/apisix/admin/plugins.lua b/lua/apisix/admin/plugins.lua new file mode 100644 index 00000000..2d498c96 --- /dev/null +++ b/lua/apisix/admin/plugins.lua @@ -0,0 +1,33 @@ +local core = require("apisix.core") +local local_plugins = require("apisix.plugin").plugins_hash +local pairs = pairs + + +local _M = { + version = 0.1, +} + + +function _M.check_schema(plugins_conf) + 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)) + local plugin_obj = local_plugins[name] + if not plugin_obj then + return false, "unknow plugin [" .. name .. "]" + end + + if plugin_obj.check_schema then + local ok, err = plugin_obj.check_schema(plugin_conf) + if not ok then + return false, "failed to check the configuration of plugin " + .. name .. " err: " .. err + end + end + end + + return true +end + + +return _M diff --git a/lua/apisix/admin/routes.lua b/lua/apisix/admin/routes.lua index ce792f47..51b3974a 100644 --- a/lua/apisix/admin/routes.lua +++ b/lua/apisix/admin/routes.lua @@ -1,4 +1,5 @@ local core = require("apisix.core") +local schema_plugin = require("apisix.admin.plugins").check_schema local tostring = tostring @@ -63,6 +64,13 @@ function _M.put(uri_segs, conf) end end + if conf.plugins then + local ok, err = schema_plugin(conf.plugins) + if not ok then + return 400, {error_msg = err} + end + end + local key = "/" .. resource .. "/" .. id local res, err = core.etcd.set(key, conf) if not res then @@ -135,6 +143,13 @@ function _M.post(uri_segs, conf) end end + if conf.plugins then + local ok, err = schema_plugin(conf.plugins) + if not ok then + return 400, {error_msg = err} + end + end + local key = "/" .. uri_segs[4] -- core.log.info("key: ", key) local res, err = core.etcd.push(key, conf) diff --git a/lua/apisix/admin/services.lua b/lua/apisix/admin/services.lua index 76ade8be..3637a34d 100644 --- a/lua/apisix/admin/services.lua +++ b/lua/apisix/admin/services.lua @@ -1,4 +1,5 @@ local core = require("apisix.core") +local schema_plugin = require("apisix.admin.plugins").check_schema local tostring = tostring @@ -46,6 +47,13 @@ function _M.put(uri_segs, conf) end end + if conf.plugins then + local ok, err = schema_plugin(conf.plugins) + if not ok then + return 400, {error_msg = err} + end + end + local key = "/services/" .. id core.log.info("key: ", key) local res, err = core.etcd.set(key, conf) @@ -121,6 +129,13 @@ function _M.post(uri_segs, conf) end end + if conf.plugins then + local ok, err = schema_plugin(conf.plugins) + if not ok then + return 400, {error_msg = err} + end + end + local key = "/services" local res, err = core.etcd.push(key, conf) if not res then diff --git a/lua/apisix/plugin.lua b/lua/apisix/plugin.lua index e8a2caf3..ea7c3fb0 100644 --- a/lua/apisix/plugin.lua +++ b/lua/apisix/plugin.lua @@ -7,13 +7,15 @@ local pcall = pcall local ipairs = ipairs local pairs = pairs local type = type -local local_supported_plugins = {} +local local_plugins = {} +local local_plugins_hash = {} local _M = { version = 0.1, load_times = 0, - plugins = local_supported_plugins, + plugins = local_plugins, + plugins_hash = local_plugins_hash, } @@ -23,7 +25,8 @@ end local function load() - core.table.clear(local_supported_plugins) + core.table.clear(local_plugins) + core.table.clear(local_plugins_hash) local plugin_names = core.config.local_conf().plugins if not plugin_names then @@ -50,7 +53,7 @@ local function load() else plugin.name = name - insert_tab(local_supported_plugins, plugin) + insert_tab(local_plugins, plugin) end if plugin.init then @@ -59,12 +62,16 @@ local function load() end -- sort by plugin's priority - if #local_supported_plugins > 1 then - sort_tab(local_supported_plugins, sort_plugin) + if #local_plugins > 1 then + sort_tab(local_plugins, sort_plugin) + end + + for _, plugin in ipairs(local_plugins) do + local_plugins_hash[plugin.name] = plugin end _M.load_times = _M.load_times + 1 - return local_supported_plugins + return local_plugins end _M.load = load @@ -100,13 +107,13 @@ end function _M.filter(user_route, plugins) - plugins = plugins or core.table.new(#local_supported_plugins * 2, 0) + plugins = plugins or core.table.new(#local_plugins * 2, 0) local user_plugin_conf = user_route.value.plugins if user_plugin_conf == nil then return plugins end - for _, plugin_obj in ipairs(local_supported_plugins) do + for _, plugin_obj in ipairs(local_plugins) do local name = plugin_obj.name local plugin_conf = user_plugin_conf[name] diff --git a/lua/apisix/plugins/limit-count.lua b/lua/apisix/plugins/limit-count.lua index 92669dc1..9c1b942a 100644 --- a/lua/apisix/plugins/limit-count.lua +++ b/lua/apisix/plugins/limit-count.lua @@ -9,8 +9,9 @@ local schema = { count = {type = "integer", minimum = 0}, time_window = {type = "integer", minimum = 0}, key = {type = "string", enum = {"remote_addr"}}, - rejected_code = {type = "integer", minimum = 200}, + rejected_code = {type = "integer", minimum = 200, maximum = 600}, }, + additionalProperties = false, required = {"count", "time_window", "key", "rejected_code"} } diff --git a/t/plugin/limit-count.t b/t/plugin/limit-count.t index 41afd60d..0c95d440 100644 --- a/t/plugin/limit-count.t +++ b/t/plugin/limit-count.t @@ -113,3 +113,250 @@ passed [404, 200, 404, 200, 503] --- no_error_log [error] + + + +=== TEST 6: invalid route: missing key +--- 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": { + "limit-count": { + "count": 2, + "time_window": 60, + "rejected_code": 503 + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin limit-count err: invalid \"required\" in docuement at pointer \"#\""} +--- no_error_log +[error] + + + +=== TEST 7: invalid route: wrong count +--- 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": { + "limit-count": { + "count": -100, + "time_window": 60, + "rejected_code": 503, + "key": "remote_addr" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin limit-count err: invalid \"minimum\" in docuement at pointer \"#\/count\""} +--- no_error_log +[error] + + + +=== TEST 8: invalid route: wrong count + POST method +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_POST, + [[{ + "plugins": { + "limit-count": { + "count": -100, + "time_window": 60, + "rejected_code": 503, + "key": "remote_addr" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin limit-count err: invalid \"minimum\" in docuement at pointer \"#\/count\""} +--- no_error_log +[error] + + + +=== TEST 9: invalid service: missing key +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/services/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "limit-count": { + "count": 2, + "time_window": 60, + "rejected_code": 503 + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin limit-count err: invalid \"required\" in docuement at pointer \"#\""} +--- no_error_log +[error] + + + +=== TEST 10: invalid service: wrong count +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/services/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "limit-count": { + "count": -100, + "time_window": 60, + "rejected_code": 503, + "key": "remote_addr" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin limit-count err: invalid \"minimum\" in docuement at pointer \"#\/count\""} +--- no_error_log +[error] + + + +=== TEST 11: invalid service: wrong count + POST method +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/services/1', + ngx.HTTP_POST, + [[{ + "plugins": { + "limit-count": { + "count": -100, + "time_window": 60, + "rejected_code": 503, + "key": "remote_addr" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin limit-count err: invalid \"minimum\" in docuement at pointer \"#\/count\""} +--- no_error_log +[error]