From f16a8f00e1f97a6691eb7ed66a8b0e7819160b7e Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Fri, 19 Jun 2020 21:16:28 +0800 Subject: [PATCH 1/3] bugfix: the id can be string object, which contains `^[a-zA-Z0-9-_]+$`. --- apisix/admin/global_rules.lua | 2 ++ apisix/admin/routes.lua | 2 ++ apisix/admin/services.lua | 4 +-- apisix/admin/ssl.lua | 43 ++--------------------- apisix/admin/stream_routes.lua | 10 +++--- apisix/admin/upstreams.lua | 4 +-- apisix/schema_def.lua | 3 ++ t/admin/global-rules.t | 33 ++++++++++++++++++ t/admin/routes.t | 63 ++++++++++++++++++++++++++++++++++ t/admin/services.t | 31 +++++++++++++++++ t/admin/ssl.t | 59 +++++++++++++++++++++++++++++++ t/admin/stream-routes.t | 63 ++++++++++++++++++++++++++++++++++ t/admin/upstream.t | 61 +++++++++++++++++++++++++++++++- 13 files changed, 327 insertions(+), 51 deletions(-) diff --git a/apisix/admin/global_rules.lua b/apisix/admin/global_rules.lua index a29d9539de8a..a768012f9960 100644 --- a/apisix/admin/global_rules.lua +++ b/apisix/admin/global_rules.lua @@ -43,6 +43,8 @@ local function check_conf(id, conf, need_id) return nil, {error_msg = "wrong route id"} end + conf.id = id + core.log.info("schema: ", core.json.delay_encode(core.schema.global_rule)) core.log.info("conf : ", core.json.delay_encode(conf)) local ok, err = core.schema.check(core.schema.global_rule, conf) diff --git a/apisix/admin/routes.lua b/apisix/admin/routes.lua index 6a14ce4d9432..2ce284be71c9 100644 --- a/apisix/admin/routes.lua +++ b/apisix/admin/routes.lua @@ -45,6 +45,8 @@ local function check_conf(id, conf, need_id) return nil, {error_msg = "wrong route id"} end + conf.id = id + core.log.info("schema: ", core.json.delay_encode(core.schema.route)) core.log.info("conf : ", core.json.delay_encode(conf)) local ok, err = core.schema.check(core.schema.route, conf) diff --git a/apisix/admin/services.lua b/apisix/admin/services.lua index 7bebd3b158f4..c10a215fd610 100644 --- a/apisix/admin/services.lua +++ b/apisix/admin/services.lua @@ -20,7 +20,6 @@ local schema_plugin = require("apisix.admin.plugins").check_schema local upstreams = require("apisix.admin.upstreams") local tostring = tostring local ipairs = ipairs -local tonumber = tonumber local type = type @@ -47,6 +46,7 @@ local function check_conf(id, conf, need_id) return nil, {error_msg = "wrong service id"} end + conf.id = id core.log.info("schema: ", core.json.delay_encode(core.schema.service)) core.log.info("conf : ", core.json.delay_encode(conf)) @@ -55,7 +55,7 @@ local function check_conf(id, conf, need_id) return nil, {error_msg = "invalid configuration: " .. err} end - if need_id and not tonumber(id) then + if need_id and not id then return nil, {error_msg = "wrong type of service id"} end diff --git a/apisix/admin/ssl.lua b/apisix/admin/ssl.lua index ccf3047a2aea..6d77117911f9 100644 --- a/apisix/admin/ssl.lua +++ b/apisix/admin/ssl.lua @@ -46,6 +46,8 @@ local function check_conf(id, conf, need_id) return nil, {error_msg = "wrong ssl id"} end + conf.id = id + core.log.info("schema: ", core.json.delay_encode(core.schema.ssl)) core.log.info("conf : ", core.json.delay_encode(conf)) local ok, err = core.schema.check(core.schema.ssl, conf) @@ -53,47 +55,6 @@ local function check_conf(id, conf, need_id) return nil, {error_msg = "invalid configuration: " .. err} end - local upstream_id = conf.upstream_id - if upstream_id then - local key = "/upstreams/" .. upstream_id - local res, err = core.etcd.get(key) - if not res then - return nil, {error_msg = "failed to fetch upstream info by " - .. "upstream id [" .. upstream_id .. "]: " - .. err} - end - - if res.status ~= 200 then - return nil, {error_msg = "failed to fetch upstream info by " - .. "upstream id [" .. upstream_id .. "], " - .. "response code: " .. res.status} - end - end - - local service_id = conf.service_id - if service_id then - local key = "/services/" .. service_id - local res, err = core.etcd.get(key) - if not res then - return nil, {error_msg = "failed to fetch service info by " - .. "service id [" .. service_id .. "]: " - .. err} - end - - if res.status ~= 200 then - return nil, {error_msg = "failed to fetch service info by " - .. "service id [" .. service_id .. "], " - .. "response code: " .. res.status} - end - end - - if conf.plugins then - local ok, err = schema_plugin(conf.plugins) - if not ok then - return nil, {error_msg = err} - end - end - return need_id and id or true end diff --git a/apisix/admin/stream_routes.lua b/apisix/admin/stream_routes.lua index e806da5e01d6..969f775164e6 100644 --- a/apisix/admin/stream_routes.lua +++ b/apisix/admin/stream_routes.lua @@ -31,17 +31,19 @@ local function check_conf(id, conf, need_id) id = id or conf.id if need_id and not id then - return nil, {error_msg = "missing stream stream route id"} + return nil, {error_msg = "missing stream route id"} end if not need_id and id then - return nil, {error_msg = "wrong stream stream route id, do not need it"} + return nil, {error_msg = "wrong stream route id, do not need it"} end if need_id and conf.id and tostring(conf.id) ~= tostring(id) then - return nil, {error_msg = "wrong stream stream route id"} + return nil, {error_msg = "wrong stream route id"} end + conf.id = id + core.log.info("schema: ", core.json.delay_encode(core.schema.stream_route)) core.log.info("conf : ", core.json.delay_encode(conf)) local ok, err = core.schema.check(core.schema.stream_route, conf) @@ -129,7 +131,7 @@ end function _M.delete(id) if not id then - return 400, {error_msg = "missing stream stream route id"} + return 400, {error_msg = "missing stream route id"} end local key = "/stream_routes/" .. id diff --git a/apisix/admin/upstreams.lua b/apisix/admin/upstreams.lua index b7d2a04f96a6..f09093ec8aae 100644 --- a/apisix/admin/upstreams.lua +++ b/apisix/admin/upstreams.lua @@ -100,9 +100,7 @@ local function check_conf(id, conf, need_id) end -- let schema check id - if id and not conf.id then - conf.id = id - end + conf.id = id core.log.info("schema: ", core.json.delay_encode(core.schema.upstream)) core.log.info("conf : ", core.json.delay_encode(conf)) diff --git a/apisix/schema_def.lua b/apisix/schema_def.lua index ab75e62d7e43..d580be6982ed 100644 --- a/apisix/schema_def.lua +++ b/apisix/schema_def.lua @@ -481,6 +481,7 @@ _M.upstream = upstream_schema _M.ssl = { type = "object", properties = { + id = id_schema, cert = { type = "string", minLength = 128, maxLength = 64*1024 }, @@ -533,6 +534,7 @@ _M.proto = { _M.global_rule = { type = "object", properties = { + id = id_schema, plugins = plugins_schema }, required = {"plugins"}, @@ -543,6 +545,7 @@ _M.global_rule = { _M.stream_route = { type = "object", properties = { + id = id_schema, remote_addr = remote_addr_def, server_addr = { description = "server IP", diff --git a/t/admin/global-rules.t b/t/admin/global-rules.t index ee1f10c33985..9676299e6dff 100644 --- a/t/admin/global-rules.t +++ b/t/admin/global-rules.t @@ -309,3 +309,36 @@ GET /t {"error_msg":"invalid configuration: property \"plugins\" is required"} --- no_error_log [error] + + + +=== TEST 9: string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/global_rules/a-b-c-ABC_0123', + ngx.HTTP_PUT, + [[{ + "plugins": { + "limit-count": { + "count": 2, + "time_window": 60, + "rejected_code": 503, + "key": "remote_addr" + } + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] diff --git a/t/admin/routes.t b/t/admin/routes.t index 32e674f955c5..42f58d7e57dd 100644 --- a/t/admin/routes.t +++ b/t/admin/routes.t @@ -1772,3 +1772,66 @@ GET /t passed --- no_error_log [error] + + + +=== TEST 48: string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/a-b-c-ABC_0123', + ngx.HTTP_PUT, + [[{ + "upstream": { + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + }, + "uri": "/index.html" + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 49: invalid string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/*invalid', + ngx.HTTP_PUT, + [[{ + "upstream": { + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + }, + "uri": "/index.html" + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- error_code: 400 +--- no_error_log +[error] diff --git a/t/admin/services.t b/t/admin/services.t index 73ce783b01cd..546e2ce03b55 100644 --- a/t/admin/services.t +++ b/t/admin/services.t @@ -928,3 +928,34 @@ GET /t passed --- no_error_log [error] + + + +=== TEST 27: invalid string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/services/*invalid', + ngx.HTTP_PUT, + [[{ + "upstream": { + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- error_code: 400 +--- no_error_log +[error] diff --git a/t/admin/ssl.t b/t/admin/ssl.t index 57eb69e8f13d..309a582057d2 100644 --- a/t/admin/ssl.t +++ b/t/admin/ssl.t @@ -353,3 +353,62 @@ GET /t passed --- no_error_log [error] + + + +=== TEST 10: string id +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin") + + local ssl_cert = t.read_file("conf/cert/apisix.crt") + local ssl_key = t.read_file("conf/cert/apisix.key") + local data = {cert = ssl_cert, key = ssl_key, sni = "test.com"} + + local code, body = t.test('/apisix/admin/ssl/a-b-c-ABC_0123', + ngx.HTTP_PUT, + core.json.encode(data) + ) + if code > 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 11: invalid id +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin") + + local ssl_cert = t.read_file("conf/cert/apisix.crt") + local ssl_key = t.read_file("conf/cert/apisix.key") + local data = {cert = ssl_cert, key = ssl_key, sni = "test.com"} + + local code, body = t.test('/apisix/admin/ssl/*invalid', + ngx.HTTP_PUT, + core.json.encode(data) + ) + if code > 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- error_code: 400 +--- no_error_log +[error] diff --git a/t/admin/stream-routes.t b/t/admin/stream-routes.t index 24b5e5ef0331..dcdd6bda95b5 100644 --- a/t/admin/stream-routes.t +++ b/t/admin/stream-routes.t @@ -297,3 +297,66 @@ GET /t [delete] code: 200 message: passed --- no_error_log [error] + + + +=== TEST 8: string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/stream_routes/a-b-c-ABC_0123', + ngx.HTTP_PUT, + [[{ + "remote_addr": "127.0.0.1", + "upstream": { + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 9: invalid string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/stream_routes/*invalid', + ngx.HTTP_PUT, + [[{ + "remote_addr": "127.0.0.1", + "upstream": { + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- error_code: 400 +--- no_error_log +[error] diff --git a/t/admin/upstream.t b/t/admin/upstream.t index 5d263dd11b27..28aac0d19f22 100644 --- a/t/admin/upstream.t +++ b/t/admin/upstream.t @@ -780,7 +780,7 @@ passed "127.0.0.1:8081": 3, "127.0.0.1:8082": 0 } - }]], + }]], [[{ "node": { "value": { @@ -1279,3 +1279,62 @@ GET /t passed --- no_error_log [error] + + + +=== TEST 39: string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/upstreams/a-b-c-ABC_0123', + ngx.HTTP_PUT, + [[{ + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 40: invalid string id +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/upstreams/*invalid', + ngx.HTTP_PUT, + [[{ + "nodes": { + "127.0.0.1:8080": 1 + }, + "type": "roundrobin" + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- request +GET /t +--- error_code: 400 +--- response_body +{"error_msg":"invalid configuration: property \"id\" validation failed: object matches none of the requireds"} +--- no_error_log +[error] From fb4013bf0d92bdb13e2bec86a115b1bc3cb2cc9e Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Sat, 20 Jun 2020 08:44:05 +0800 Subject: [PATCH 2/3] change: remove useless local cache. --- apisix/admin/ssl.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/apisix/admin/ssl.lua b/apisix/admin/ssl.lua index 6d77117911f9..6d9307d95d1d 100644 --- a/apisix/admin/ssl.lua +++ b/apisix/admin/ssl.lua @@ -15,7 +15,6 @@ -- limitations under the License. -- local core = require("apisix.core") -local schema_plugin = require("apisix.admin.plugins").check_schema local tostring = tostring local aes = require "resty.aes" local ngx_encode_base64 = ngx.encode_base64 From 5b9b4706128f7534b5e82adad756dccb90049bb5 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Sat, 20 Jun 2020 09:45:09 +0800 Subject: [PATCH 3/3] test: clean data in etcd. --- t/admin/global-rules.t | 23 +++++++++++++++++++++++ t/admin/routes.t | 25 ++++++++++++++++++++++++- t/admin/ssl.t | 31 ++++++++++++++++++++++++++++++- t/admin/stream-routes.t | 25 ++++++++++++++++++++++++- t/admin/upstream.t | 25 ++++++++++++++++++++++++- 5 files changed, 125 insertions(+), 4 deletions(-) diff --git a/t/admin/global-rules.t b/t/admin/global-rules.t index 9676299e6dff..2cda952f205c 100644 --- a/t/admin/global-rules.t +++ b/t/admin/global-rules.t @@ -342,3 +342,26 @@ GET /t passed --- no_error_log [error] + + + +=== TEST 10: string id(DELETE) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/global_rules/a-b-c-ABC_0123', + ngx.HTTP_DELETE + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] diff --git a/t/admin/routes.t b/t/admin/routes.t index 42f58d7e57dd..be9feb6a1b4e 100644 --- a/t/admin/routes.t +++ b/t/admin/routes.t @@ -1807,7 +1807,30 @@ passed -=== TEST 49: invalid string id +=== TEST 49: string id(delete) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/a-b-c-ABC_0123', + ngx.HTTP_DELETE + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 50: invalid string id --- config location /t { content_by_lua_block { diff --git a/t/admin/ssl.t b/t/admin/ssl.t index 309a582057d2..e93ca1971174 100644 --- a/t/admin/ssl.t +++ b/t/admin/ssl.t @@ -386,7 +386,36 @@ passed -=== TEST 11: invalid id +=== TEST 11: string id(delete) +--- config + location /t { + content_by_lua_block { + local core = require("apisix.core") + local t = require("lib.test_admin") + + local ssl_cert = t.read_file("conf/cert/apisix.crt") + local ssl_key = t.read_file("conf/cert/apisix.key") + local data = {cert = ssl_cert, key = ssl_key, sni = "test.com"} + + local code, body = t.test('/apisix/admin/ssl/a-b-c-ABC_0123', + ngx.HTTP_DELETE + ) + if code > 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 12: invalid id --- config location /t { content_by_lua_block { diff --git a/t/admin/stream-routes.t b/t/admin/stream-routes.t index dcdd6bda95b5..5bd36ff33613 100644 --- a/t/admin/stream-routes.t +++ b/t/admin/stream-routes.t @@ -332,7 +332,30 @@ passed -=== TEST 9: invalid string id +=== TEST 9: string id(delete) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/stream_routes/a-b-c-ABC_0123', + ngx.HTTP_DELETE + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 10: invalid string id --- config location /t { content_by_lua_block { diff --git a/t/admin/upstream.t b/t/admin/upstream.t index 28aac0d19f22..d1167ab80418 100644 --- a/t/admin/upstream.t +++ b/t/admin/upstream.t @@ -1311,7 +1311,30 @@ passed -=== TEST 40: invalid string id +=== TEST 40: string id(delete) +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/upstreams/a-b-c-ABC_0123', + ngx.HTTP_DELETE + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 41: invalid string id --- config location /t { content_by_lua_block {