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

[WIP] bugfix: check plugin schema when sync data #1875

Closed
wants to merge 18 commits into from
Closed

[WIP] bugfix: check plugin schema when sync data #1875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
553384c
fix: add check plugin's schema in sync data
Jul 20, 2020
408ecb9
fix: add check plugin's schema in sync data
Jul 20, 2020
45fd6ba
feat: add check plugin's schema test
Jul 20, 2020
f695a98
refactor: move config-center-yaml to config-center/yaml
Jul 20, 2020
60a7e6c
fix: fix test case of route-plugin
wrl96 Jul 20, 2020
b589c64
fix: add check plugin's schema in sync data
Jul 20, 2020
bdec513
fix: add check plugin's schema in sync data
Jul 20, 2020
3dfbf27
feat: add check plugin's schema test
Jul 20, 2020
c5d5a75
refactor: move config-center-yaml to config-center/yaml
Jul 20, 2020
026e2bb
fix: fix test case of route-plugin
wrl96 Jul 20, 2020
ce93175
Merge branch 'fix-1604' of https://github.com/wrl96/incubator-apisix …
wrl96 Jul 21, 2020
022f827
fix: set pairs in local
wrl96 Jul 21, 2020
2b5aef7
Merge remote-tracking branch 'origin/fix-1604' into fix-1604
wrl96 Jul 21, 2020
48b9c4a
feat: add test casesof etcd
wrl96 Jul 21, 2020
2212d9e
feat: add test cases of etcd
wrl96 Jul 21, 2020
ee95248
feat: add test cases of etcd
wrl96 Jul 21, 2020
9778b7d
fix: spell error
wrl96 Jul 21, 2020
b3c1b3d
fix: test case error
wrl96 Jul 21, 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
51 changes: 51 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 @@ -25,6 +25,7 @@ local exiting = ngx.worker.exiting
local insert_tab = table.insert
local type = type
local ipairs = ipairs
local pairs = pairs
local setmetatable = setmetatable
local ngx_sleep = ngx.sleep
local ngx_timer_at = ngx.timer.at
Expand All @@ -49,6 +50,16 @@ local mt = {
end
}

local disable_schema = {
type = "object",
properties = {
disable = {type = "boolean", enum={true}}
},
required = {"disable"}
}
local local_plugins
local stream_local_plugins

local function readdir(etcd_cli, key)
if not etcd_cli then
return nil, nil, "not inited"
Expand Down Expand Up @@ -166,6 +177,39 @@ local function sync_data(self)
", it shoud be a object")
end

-- check plugins schema
local plugins_conf = item.plugins
if plugins_conf then
for name, plugin_conf in pairs(plugins_conf) do
Copy link
Member

Choose a reason for hiding this comment

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

check the plugin conf in this way, it is not a good way.

config_etcd.lua is a basic component, it should not know the plugin.

I think we can export a new option when creating new config_etcd object, then we can add more check logic in filter_func .

Example:

core.config.new("/routes", {
    automatic = true,
    item_schema = core.schema.route,
    filter_func = function ()
        -- your code
    end
    filter = filter,
})

local plugin_obj = local_plugins[name]
if plugin_obj then
log.info("check plugin scheme, name: ", name, ", configurations: ",
json.delay_encode(plugin_conf, true))
else
plugin_obj = stream_local_plugins[name]
if plugin_obj then
log.info("check stream plugin scheme, name: ", name,
": ", json.delay_encode(plugin_conf, true))
else
log.error("unknown plugin [" .. name .. "]")
return false
end
end

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

if data_valid and self.item_schema then
data_valid, err = check_schema(self.item_schema, item.value)
if not data_valid then
Expand Down Expand Up @@ -466,5 +510,12 @@ function _M.server_version(self)
return read_etcd_version(self.etcd_cli)
end

function _M.init_plugins(plugins, is_stream)
if is_stream then
stream_local_plugins = plugins
else
local_plugins = plugins
end
end

return _M
54 changes: 52 additions & 2 deletions apisix/core/config_yaml.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ local exiting = ngx.worker.exiting
local insert_tab = table.insert
local type = type
local ipairs = ipairs
local pairs = pairs
local setmetatable = setmetatable
local ngx_sleep = ngx.sleep
local ngx_timer_at = ngx.timer.at
Expand Down Expand Up @@ -55,9 +56,18 @@ local mt = {
end
}

local disable_schema = {
type = "object",
properties = {
disable = {type = "boolean", enum={true}}
},
required = {"disable"}
}
local local_plugins
local stream_local_plugins

local apisix_yaml
local apisix_yaml_ctime
local apisix_yaml
local apisix_yaml_ctime
local function read_apisix_yaml(premature, pre_mtime)
if premature then
return
Expand Down Expand Up @@ -162,6 +172,39 @@ local function sync_data(self)
", it shoud be a object")
end

-- check plugins schema
local plugins_conf = item.plugins
if plugins_conf then
for name, plugin_conf in pairs(plugins_conf) do
local plugin_obj = local_plugins[name]
if plugin_obj then
log.info("check plugin scheme, name: ", name, ", configurations: ",
json.delay_encode(plugin_conf, true))
else
plugin_obj = stream_local_plugins[name]
if plugin_obj then
log.info("check stream plugin scheme, name: ", name,
": ", json.delay_encode(plugin_conf, true))
else
log.error("unknown plugin [" .. name .. "]")
return false
end
end

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

local key = item.id or "arr_" .. i
local conf_item = {value = item, modifiedIndex = apisix_yaml_ctime,
key = "/" .. self.key .. "/" .. key}
Expand Down Expand Up @@ -324,5 +367,12 @@ function _M.init_worker()
ngx.timer.every(1, read_apisix_yaml)
end

function _M.init_plugins(plugins, is_stream)
if is_stream then
stream_local_plugins = plugins
else
local_plugins = plugins
end
end

return _M
2 changes: 2 additions & 0 deletions apisix/plugin.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ local function load()

_M.load_times = _M.load_times + 1
core.log.info("load plugin times: ", _M.load_times)
core.config.init_plugins(local_plugins_hash, false)
return true
end

Expand Down Expand Up @@ -165,6 +166,7 @@ local function load_stream()
core.log.info("stream plugins: ",
core.json.delay_encode(stream_local_plugins, true))
core.log.info("load stream plugin times: ", _M.stream_load_times)
core.config.init_plugins(stream_local_plugins_hash, true)
return true
end

Expand Down
146 changes: 146 additions & 0 deletions t/config-center/etcd/route-plugin.t
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
BEGIN {
$ENV{"ETCD_ENABLE_AUTH"} = "true"
}

use t::APISIX 'no_plan';

repeat_each(1);
no_long_string();
no_shuffle();
no_root_location();
log_level("info");

# Authentication is enabled at etcd and credentials are set
system('etcdctl --endpoints="http://127.0.0.1:2379" -u root:5tHkHhYkjr6cQY user add root:5tHkHhYkjr6cQY');
system('etcdctl --endpoints="http://127.0.0.1:2379" -u root:5tHkHhYkjr6cQY auth enable');
system('etcdctl --endpoints="http://127.0.0.1:2379" -u root:5tHkHhYkjr6cQY role revoke --path "/*" -rw guest');

run_tests;

# Authentication is disabled at etcd & guest access is granted
system('etcdctl --endpoints="http://127.0.0.1:2379" -u root:5tHkHhYkjr6cQY auth disable');
system('etcdctl --endpoints="http://127.0.0.1:2379" -u root:5tHkHhYkjr6cQY role grant --path "/*" -rw guest');

__DATA__

=== TEST 1: set route with plugin
--- config
location /t {
content_by_lua_block {
local core = require("apisix.core")
local conf = {
["uri"] = "/hello",
["plugins"] = {
["proxy-rewrite"] = {
["uri"] = "/uri/plugin_proxy_rewrite",
["headers"] = {
["X-Api-Version"] = "v2"
}
}
},
["upstream"] = {
["nodes"] = {
["127.0.0.1:1980"] = 1
},
["type"] = "roundrobin"
}
}
local res, err = core.etcd.push("/routes", conf)
if not res then
core.log.error("failed to post route[/routes] to etcd")
ngx.exit(code)
end
ngx.say("done")
}
}
--- request
GET /t
--- response_body
done
--- no_error_log
[error]



=== TEST 2: route with plugin
--- request
GET /hello
--- more_headers
X-Api-Version:v1
--- response_body
uri: /uri/plugin_proxy_rewrite
host: localhost
x-api-version: v2
x-real-ip: 127.0.0.1
--- no_error_log
[error]



=== TEST 3: set route with invalid plugin
--- config
location /t {
content_by_lua_block {
local core = require("apisix.core")
local sub_str = string.sub
local res, err = core.etcd.get("/routes")
if not res then
core.log.error("failed to get route[/routes] from etcd: ", err)
end
local key = sub_str(res.body.node.nodes[1].key, 8)
local conf = {
["uri"] = "/hello",
["plugins"] = {
["proxy-rewrite"] = {
["uri"] = "/uri/plugin_proxy_rewrite",
["headers"] = {
[""] = ""
}
}
},
["upstream"] = {
["nodes"] = {
["127.0.0.1:1980"] = 1
},
["type"] = "roundrobin"
}
}
local res, err = core.etcd.set(key, conf)
if not res then
core.log.error("failed to put route[/routes] to etcd")
ngx.exit(code)
end
ngx.say("done")
}
}
--- request
GET /t
--- response_body
done
--- no_error_log
[error]



=== TEST 4: route with invalid plugin
--- request
GET /hello
--- error_code: 404
--- error_log
failed to check the configuration of plugin proxy-rewrite err: invalid type as header value, context: ngx.timer
Loading