Skip to content

Commit

Permalink
feat(conf) implement new 'plugins' property
Browse files Browse the repository at this point in the history
Problem
-------

All plugins bundled with Kong are loaded by default. There exists no
mechanism to disable any of the bundled plugins, which can yield
considerable performance improvements in the runloop in specific
workloads.

Solution
--------

Introduce a config option which controls which of the bundled plugins
are loaded into Kong.

Implementation
--------------

Introduce `plugins` config option and deprecate `custom_plugins` option.
Kong will load only plugins which are specified in `plugins` config
options. This also is inline in the goal of simplifying Kong's
configurations.

Plugins specified in `plugins` and `custom_plugins` are merged together
to avoid a break change but `custom_plugins` will be removed in future.

The new `plugins` options can take in a sugar value `bundled` which
loads all the plugins bundled with Kong.  This accomplishes  good
usability and flexibility.

From #3387
  • Loading branch information
hbagdi authored and thibaultcha committed Jun 15, 2018
1 parent 2380e24 commit 80acbf8
Show file tree
Hide file tree
Showing 26 changed files with 283 additions and 50 deletions.
36 changes: 30 additions & 6 deletions kong.conf.default
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,36 @@
# adjusted by the `log_level`
# directive.

#custom_plugins = # Comma-separated list of additional plugins
# this node should load.
# Use this property to load custom plugins
# that are not bundled with Kong.
# Plugins will be loaded from the
# `kong.plugins.{name}.*` namespace.
#plugins = bundled # Comma-separated list of plugins this node
# should load. By default, only plugins
# bundled in official distributions are
# loaded via the `bundled` keyword.
#
# Loading a plugin does not enable it by
# default, but only instructs Kong to load its
# source code, and allows to configure the
# plugin via the various related Admin API
# endpoints.
#
# The specified name(s) will be substituted as
# such in the Lua namespace:
# `kong.plugins.{name}.*`.
#
# When the `off` keyword is specified as the
# only value, no plugins will be loaded.
#
# `bundled` and plugin names can be mixed
# together, as the following examples suggest:
#
# - plugins = bundled, custom-auth
# > will load all bundled plugins and
# another plugin `custom-auth`
#
# - plugins = key-auth, custom-auth
# > will only load these two plugins
#
# - plugins = off
# > will not load any plugin

#anonymous_reports = on # Send anonymous usage data such as error
# stack traces to help improve Kong.
Expand Down
4 changes: 2 additions & 2 deletions kong/api/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ end


-- Loading plugins routes
if singletons.configuration and singletons.configuration.plugins then
for k in pairs(singletons.configuration.plugins) do
if singletons.configuration and singletons.configuration.loaded_plugins then
for k in pairs(singletons.configuration.loaded_plugins) do
local loaded, mod = utils.load_module_if_exists("kong.plugins." .. k .. ".api")

if loaded then
Expand Down
2 changes: 1 addition & 1 deletion kong/api/routes/kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ return {
pending = ngx.timer.pending_count()
},
plugins = {
available_on_server = singletons.configuration.plugins,
available_on_server = singletons.configuration.loaded_plugins,
enabled_in_cluster = distinct_plugins
},
lua_version = lua_version,
Expand Down
2 changes: 1 addition & 1 deletion kong/api/routes/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ return {
["/plugins/enabled"] = {
GET = function(self, dao_factory, helpers)
local enabled_plugins = setmetatable({}, cjson.empty_array_mt)
for k in pairs(singletons.configuration.plugins) do
for k in pairs(singletons.configuration.loaded_plugins) do
enabled_plugins[#enabled_plugins+1] = k
end
return helpers.responses.send_HTTP_OK {
Expand Down
33 changes: 27 additions & 6 deletions kong/conf_loader.lua
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ local CONF_INFERENCES = {
admin_error_log = {typ = "string"},
log_level = {enum = {"debug", "info", "notice", "warn",
"error", "crit", "alert", "emerg"}},
plugins = {typ = "array"},
custom_plugins = {typ = "array"},
anonymous_reports = {typ = "boolean"},
nginx_daemon = {typ = "ngx_boolean"},
Expand Down Expand Up @@ -627,13 +628,33 @@ local function load(path, custom_conf)

-- merge plugins
do
local custom_plugins = {}
for i = 1, #conf.custom_plugins do
local plugin_name = pl_stringx.strip(conf.custom_plugins[i])
custom_plugins[plugin_name] = true
local plugins = {}
if #conf.plugins > 0 and conf.plugins[1] ~= "off" then
for i = 1, #conf.plugins do
local plugin_name = pl_stringx.strip(conf.plugins[i])
if plugin_name ~= "off" then
if plugin_name == "bundled" then
plugins = tablex.merge(constants.BUNDLED_PLUGINS, plugins, true)
else
plugins[plugin_name] = true
end
end
end
end
conf.plugins = tablex.merge(constants.PLUGINS_AVAILABLE, custom_plugins, true)
setmetatable(conf.plugins, nil) -- remove Map mt

if conf.custom_plugins and #conf.custom_plugins > 0 then
log.warn("the 'custom_plugins' configuration property is deprecated, " ..
"use 'plugins' instead")

for i = 1, #conf.custom_plugins do
local plugin_name = pl_stringx.strip(conf.custom_plugins[i])
plugins[plugin_name] = true
end
end

conf.loaded_plugins = setmetatable(plugins, {
__tostring = function() return "" end,
})
end

-- nginx user directive
Expand Down
2 changes: 1 addition & 1 deletion kong/constants.lua
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ for _, plugin in ipairs(deprecated_plugins) do
end

return {
PLUGINS_AVAILABLE = plugin_map,
BUNDLED_PLUGINS = plugin_map,
DEPRECATED_PLUGINS = deprecated_plugin_map,
-- non-standard headers, specific to Kong
HEADERS = {
Expand Down
2 changes: 1 addition & 1 deletion kong/dao/factory.lua
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ function _M.new(kong_config, new_db)
daos = {},
additional_tables = {},
kong_config = kong_config,
plugin_names = kong_config.plugins or {}
plugin_names = kong_config.loaded_plugins or {}
}

local DB = require("kong.dao.db." .. self.db_type)
Expand Down
4 changes: 2 additions & 2 deletions kong/dao/schemas/plugins.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ local function load_config_schema(plugin_t)

-- singletons.configuration would be nil when plugin operations are
-- done through DAOs like in migrations or tests
if singletons.configuration and not singletons.configuration.plugins[plugin_name] then
if singletons.configuration and not singletons.configuration.loaded_plugins[plugin_name] then
return nil, "plugin '" .. plugin_name .. "' not enabled; " ..
"add it to the 'custom_plugins' configuration property"
"add it to the 'plugins' configuration property"
end

local loaded, plugin_schema = utils.load_module_if_exists("kong.plugins."
Expand Down
4 changes: 2 additions & 2 deletions kong/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ local function load_plugins(kong_conf, dao)

-- check all plugins in DB are enabled/installed
for plugin in pairs(in_db_plugins) do
if not kong_conf.plugins[plugin] then
if not kong_conf.loaded_plugins[plugin] then
return nil, plugin .. " plugin is in use but not enabled"
end
end

-- load installed plugins
for plugin in pairs(kong_conf.plugins) do
for plugin in pairs(kong_conf.loaded_plugins) do
if constants.DEPRECATED_PLUGINS[plugin] then
ngx.log(ngx.WARN, "plugin '", plugin, "' has been deprecated")
end
Expand Down
1 change: 1 addition & 0 deletions kong/templates/kong_defaults.lua
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ proxy_access_log = logs/access.log
proxy_error_log = logs/error.log
admin_access_log = logs/admin_access.log
admin_error_log = logs/error.log
plugins = bundled
custom_plugins = NONE
anonymous_reports = on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ describe("Admin API #" .. kong_config.database, function()
})
local body = assert.res_status(400, res)
local json = cjson.decode(body)
assert.same({ config = "plugin 'foo' not enabled; add it to the 'custom_plugins' configuration property" }, json)
assert.same({ config = "plugin 'foo' not enabled; add it to the 'plugins' configuration property" }, json)
end
end)
end)
Expand Down
27 changes: 20 additions & 7 deletions spec/01-unit/002-conf_loader_spec.lua
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
local conf_loader = require "kong.conf_loader"
local helpers = require "spec.helpers"
local tablex = require "pl.tablex"

describe("Configuration loader", function()
it("loads the defaults", function()
Expand Down Expand Up @@ -53,21 +54,33 @@ describe("Configuration loader", function()
it("returns a plugins table", function()
local constants = require "kong.constants"
local conf = assert(conf_loader())
assert.same(constants.PLUGINS_AVAILABLE, conf.plugins)
assert.same(constants.BUNDLED_PLUGINS, conf.loaded_plugins)
end)
it("loads custom plugins", function()
local conf = assert(conf_loader(nil, {
custom_plugins = "hello-world,my-plugin"
}))
assert.True(conf.plugins["hello-world"])
assert.True(conf.plugins["my-plugin"])
assert.True(conf.loaded_plugins["hello-world"])
assert.True(conf.loaded_plugins["my-plugin"])
end)
it("merges plugins and custom plugins", function()
local conf = assert(conf_loader(nil, {
plugins = "foo, bar",
custom_plugins = "baz,foobaz",
}))
assert.is_not_nil(conf.loaded_plugins)
assert.same(4, tablex.size(conf.loaded_plugins))
assert.True(conf.loaded_plugins["foo"])
assert.True(conf.loaded_plugins["bar"])
assert.True(conf.loaded_plugins["baz"])
assert.True(conf.loaded_plugins["foobaz"])
end)
it("loads custom plugins surrounded by spaces", function()
local conf = assert(conf_loader(nil, {
custom_plugins = " hello-world , another-one "
}))
assert.True(conf.plugins["hello-world"])
assert.True(conf.plugins["another-one"])
assert.True(conf.loaded_plugins["hello-world"])
assert.True(conf.loaded_plugins["another-one"])
end)
it("extracts flags, ports and listen ips from proxy_listen/admin_listen", function()
local conf = assert(conf_loader())
Expand Down Expand Up @@ -163,8 +176,8 @@ describe("Configuration loader", function()
it("overcomes penlight's list_delim option", function()
local conf = assert(conf_loader("spec/fixtures/to-strip.conf"))
assert.False(conf.pg_ssl)
assert.True(conf.plugins.foobar)
assert.True(conf.plugins["hello-world"])
assert.True(conf.loaded_plugins.foobar)
assert.True(conf.loaded_plugins["hello-world"])
end)
it("correctly parses values containing an octothorpe", function()
local conf = assert(conf_loader("spec/fixtures/to-strip.conf"))
Expand Down
6 changes: 3 additions & 3 deletions spec/01-unit/003-prefix_handler_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe("NGINX conf compiler", function()
describe("compile_kong_conf()", function()
it("compiles the Kong NGINX conf chunk", function()
local kong_nginx_conf = prefix_handler.compile_kong_conf(helpers.test_conf)
assert.matches("lua_package_path './?.lua;./?/init.lua;;;'", kong_nginx_conf, nil, true)
assert.matches("lua_package_path './spec/fixtures/custom_plugins/?.lua;;'", kong_nginx_conf, nil, true)
assert.matches("listen 0.0.0.0:9000;", kong_nginx_conf, nil, true)
assert.matches("listen 127.0.0.1:9001;", kong_nginx_conf, nil, true)
assert.matches("server_name kong;", kong_nginx_conf, nil, true)
Expand Down Expand Up @@ -434,8 +434,8 @@ describe("NGINX conf compiler", function()
assert(prefix_handler.prepare_prefix(conf))

local in_prefix_kong_conf = assert(conf_loader(tmp_config.kong_env))
assert.True(in_prefix_kong_conf.plugins.foo)
assert.True(in_prefix_kong_conf.plugins.bar)
assert.True(in_prefix_kong_conf.loaded_plugins.foo)
assert.True(in_prefix_kong_conf.loaded_plugins.bar)
end)

describe("ssl", function()
Expand Down
4 changes: 2 additions & 2 deletions spec/01-unit/014-plugins_order_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ describe("Plugins", function()
local conf = assert(conf_loader(nil, {
-- ensure we test the galileo priority even if galileo isn't enabled by
-- default anymore
custom_plugins = "galileo",
plugins = "bundled, galileo",
}))

plugins = {}

for plugin in pairs(conf.plugins) do
for plugin in pairs(conf.loaded_plugins) do
local handler = require("kong.plugins." .. plugin .. ".handler")
table.insert(plugins, {
name = plugin,
Expand Down
2 changes: 1 addition & 1 deletion spec/01-unit/015-plugins_version_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ describe("Plugins", function()

plugins = {}

for plugin in pairs(conf.plugins) do
for plugin in pairs(conf.loaded_plugins) do
local handler = require("kong.plugins." .. plugin .. ".handler")
table.insert(plugins, {
name = plugin,
Expand Down
10 changes: 10 additions & 0 deletions spec/02-integration/02-cmd/02-start_stop_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ describe("kong start/stop", function()
end)
end)

describe("using deprecated custom_plugin property" , function()
it("prints a warning to stderr", function()
local _, stderr, stdout = assert(helpers.kong_exec("start --conf " ..
"spec/fixtures/deprecated_custom_plugin.conf"))
assert.matches("Kong started", stdout, nil, true)
assert.matches("[warn] the 'custom_plugins' configuration property is " ..
"deprecated, use 'plugins' instead", stderr, nil, true)
end)
end)

describe("/etc/hosts resolving in CLI", function()
it("resolves #cassandra hostname", function()
assert(helpers.kong_exec("start --vv --run-migrations --conf " .. helpers.test_conf_path, {
Expand Down
2 changes: 1 addition & 1 deletion spec/02-integration/04-admin_api/02-apis_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1204,7 +1204,7 @@ pending("Admin API #" .. kong_config.database, function()
})
local body = assert.res_status(400, res)
local json = cjson.decode(body)
assert.same({ config = "plugin 'foo' not enabled; add it to the 'custom_plugins' configuration property" }, json)
assert.same({ config = "plugin 'foo' not enabled; add it to the 'plugins' configuration property" }, json)
end
end)
end)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ describe("Admin API (" .. strategy .. "): ", function()
})
local body = assert.res_status(400, res)
local json = cjson.decode(body)
assert.same({ config = "plugin 'foo' not enabled; add it to the 'custom_plugins' configuration property" }, json)
assert.same({ config = "plugin 'foo' not enabled; add it to the 'plugins' configuration property" }, json)
end
end)
end)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("Admin API post-processing", function()
setup(function()
assert(helpers.dao:run_migrations())
assert(helpers.start_kong {
custom_plugins = "admin-api-post-process, dummy"
plugins = "bundled, admin-api-post-process, dummy"
})

client = assert(helpers.admin_client())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ describe("Deprecated plugin API" , function()
})
local body = assert.res_status(400 , res)
local json = cjson.decode(body)
assert.is_equal("plugin 'admin-api-post-process' not enabled; add it to the 'custom_plugins' configuration property", json.config)
assert.is_equal("plugin 'admin-api-post-process' not enabled; add it to the 'plugins' configuration property", json.config)
end)
end)
end)

describe("deprecated enabled plugins" , function()
setup(function()
assert(helpers.start_kong({
custom_plugins = "admin-api-post-process"
plugins = "bundled, admin-api-post-process"
}))
client = helpers.admin_client()
end)
Expand Down
Loading

0 comments on commit 80acbf8

Please sign in to comment.