Skip to content

Commit

Permalink
fix(plugins): fix competing redis configs
Browse files Browse the repository at this point in the history
ACME, RateLimiting and Response-RateLimiting now use
the same redis configuration structure. The olds fields
were left in place to maintain backwards compatibility.
When resolving the configuration we looked into new fields
and if they were empty then fallback to legacy fields.
Unfortunately the new fields have their defaults as well
which get written into db - so at the time of plugin resolution
we'd have to implement complex logic to figure out if the
new value came from user or from defualt.

This approach removes the olds fields and uses shorthands
to maintain backwards compatibility.

KAG-3388
  • Loading branch information
nowNick authored and locao committed Jan 15, 2024
1 parent 7a25ad4 commit e1663b5
Show file tree
Hide file tree
Showing 12 changed files with 473 additions and 416 deletions.
6 changes: 5 additions & 1 deletion kong/db/schema/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,11 @@ function Schema:process_auto_fields(data, context, nulls, opts)
local new_values = sdata.func(value)
if new_values then
for k, v in pairs(new_values) do
data[k] = v
if type(v) == "table" then
data[k] = tablex.merge(data[k] or {}, v, true)
else
data[k] = v
end
end
end
end
Expand Down
76 changes: 34 additions & 42 deletions kong/plugins/acme/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,42 @@ local SHM_STORAGE_SCHEMA = {
local KONG_STORAGE_SCHEMA = {
}

-- deprecated old schema
local REDIS_LEGACY_SCHEMA_FIELDS = {
{ auth = { type = "string", referenceable = true, description = "The Redis password to use for authentication. " } },
{ ssl_server_name = typedefs.sni { required = false, description = "The expected server name for the SSL/TLS certificate presented by the Redis server." }},
{
namespace = {
type = "string",
description = "A namespace to prepend to all keys stored in Redis.",
required = true,
default = "",
len_min = 0,
custom_validator = validate_namespace
}
},
{ scan_count = { type = "number", required = false, default = 10, description = "The number of keys to return in Redis SCAN calls." } },
local LEGACY_SCHEMA_TRANSLATIONS = {
{ auth = {
type = "string",
func = function(value)
deprecation("acme: config.storage_config.redis.auth is deprecated, please use config.storage_config.redis.password instead",
{ after = "4.0", })
return { password = value }
end
}},
{ ssl_server_name = {
type = "string",
func = function(value)
deprecation("acme: config.storage_config.redis.ssl_server_name is deprecated, please use config.storage_config.redis.server_name instead",
{ after = "4.0", })
return { server_name = value }
end
}},
{ namespace = {
type = "string",
func = function(value)
deprecation("acme: config.storage_config.redis.namespace is deprecated, please use config.storage_config.redis.extra_options.namespace instead",
{ after = "4.0", })
return { extra_options = { namespace = value } }
end
}},
{ scan_count = {
type = "integer",
func = function(value)
deprecation("acme: config.storage_config.redis.scan_count is deprecated, please use config.storage_config.redis.extra_options.scan_count instead",
{ after = "4.0", })
return { extra_options = { scan_count = value } }
end
}},
}

local REDIS_STORAGE_SCHEMA = tablex.copy(redis_schema.config_schema.fields)
for _,v in ipairs(REDIS_LEGACY_SCHEMA_FIELDS) do
table.insert(REDIS_STORAGE_SCHEMA, v)
end

table.insert(REDIS_STORAGE_SCHEMA, { extra_options = {
description = "Custom ACME Redis options",
type = "record",
Expand Down Expand Up @@ -217,7 +231,7 @@ local schema = {
fields = {
{ shm = { type = "record", fields = SHM_STORAGE_SCHEMA, } },
{ kong = { type = "record", fields = KONG_STORAGE_SCHEMA, } },
{ redis = { type = "record", fields = REDIS_STORAGE_SCHEMA, } },
{ redis = { type = "record", fields = REDIS_STORAGE_SCHEMA, shorthand_fields = LEGACY_SCHEMA_TRANSLATIONS } },
{ consul = { type = "record", fields = CONSUL_STORAGE_SCHEMA, } },
{ vault = { type = "record", fields = VAULT_STORAGE_SCHEMA, } },
},
Expand Down Expand Up @@ -271,28 +285,6 @@ local schema = {
end
}
},
{ custom_entity_check = {
field_sources = { "config.storage_config.redis.namespace", "config.storage_config.redis.scan_count", "config.storage_config.redis.auth", "config.storage_config.redis.ssl_server_name" },
fn = function(entity)
if (entity.config.storage_config.redis.namespace or ngx.null) ~= ngx.null and entity.config.storage_config.redis.namespace ~= "" then
deprecation("acme: config.storage_config.redis.namespace is deprecated, please use config.storage_config.redis.extra_options.namespace instead",
{ after = "4.0", })
end
if (entity.config.storage_config.redis.scan_count or ngx.null) ~= ngx.null and entity.config.storage_config.redis.scan_count ~= 10 then
deprecation("acme: config.storage_config.redis.scan_count is deprecated, please use config.storage_config.redis.extra_options.scan_count instead",
{ after = "4.0", })
end
if (entity.config.storage_config.redis.auth or ngx.null) ~= ngx.null then
deprecation("acme: config.storage_config.redis.auth is deprecated, please use config.storage_config.redis.password instead",
{ after = "4.0", })
end
if (entity.config.storage_config.redis.ssl_server_name or ngx.null) ~= ngx.null then
deprecation("acme: config.storage_config.redis.ssl_server_name is deprecated, please use config.storage_config.redis.server_name instead",
{ after = "4.0", })
end
return true
end
} }
},
}

Expand Down
8 changes: 4 additions & 4 deletions kong/plugins/acme/storage/config_adapters/redis.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ local function redis_config_adapter(conf)
host = conf.host,
port = conf.port,
database = conf.database,
auth = conf.password or conf.auth, -- allow conf.auth until 4.0 version
auth = conf.password,
ssl = conf.ssl,
ssl_verify = conf.ssl_verify,
ssl_server_name = conf.server_name or conf.ssl_server_name, -- allow conf.ssl_server_name until 4.0 version
ssl_server_name = conf.server_name,

namespace = conf.extra_options.namespace or conf.namespace, -- allow conf.namespace until 4.0 version
scan_count = conf.extra_options.scan_count or conf.scan_count, -- allow conf.scan_count until 4.0 version
namespace = conf.extra_options.namespace,
scan_count = conf.extra_options.scan_count,
}
end

Expand Down
18 changes: 9 additions & 9 deletions kong/plugins/rate-limiting/policies/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ local EXPIRATION = require "kong.plugins.rate-limiting.expiration"

local function get_redis_configuration(plugin_conf)
return {
host = plugin_conf.redis.host or plugin_conf.redis_host,
port = plugin_conf.redis.port or plugin_conf.redis_port,
username = plugin_conf.redis.username or plugin_conf.redis_username,
password = plugin_conf.redis.password or plugin_conf.redis_password,
database = plugin_conf.redis.database or plugin_conf.redis_database,
timeout = plugin_conf.redis.timeout or plugin_conf.redis_timeout,
ssl = plugin_conf.redis.ssl or plugin_conf.redis_ssl,
ssl_verify = plugin_conf.redis.ssl_verify or plugin_conf.redis_ssl_verify,
server_name = plugin_conf.redis.server_name or plugin_conf.redis_server_name,
host = plugin_conf.redis.host,
port = plugin_conf.redis.port,
username = plugin_conf.redis.username,
password = plugin_conf.redis.password,
database = plugin_conf.redis.database,
timeout = plugin_conf.redis.timeout,
ssl = plugin_conf.redis.ssl,
ssl_verify = plugin_conf.redis.ssl_verify,
server_name = plugin_conf.redis.server_name,
}
end

Expand Down
153 changes: 81 additions & 72 deletions kong/plugins/rate-limiting/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -93,40 +93,103 @@ return {
{ policy = policy },
{ fault_tolerant = { description = "A boolean value that determines if the requests should be proxied even if Kong has troubles connecting a third-party data store. If `true`, requests will be proxied anyway, effectively disabling the rate-limiting function until the data store is working again. If `false`, then the clients will see `500` errors.", type = "boolean", required = true, default = true }, },
{ redis = redis_schema.config_schema },
{ redis_host = typedefs.host },
{ redis_port = typedefs.port({ default = 6379 }), },
{ redis_password = { description = "When using the `redis` policy, this property specifies the password to connect to the Redis server.", type = "string", len_min = 0, referenceable = true }, },
{ redis_username = { description = "When using the `redis` policy, this property specifies the username to connect to the Redis server when ACL authentication is desired.", type = "string", referenceable = true }, },
{ redis_ssl = { description = "When using the `redis` policy, this property specifies if SSL is used to connect to the Redis server.", type = "boolean", required = true, default = false, }, },
{ redis_ssl_verify = { description = "When using the `redis` policy with `redis_ssl` set to `true`, this property specifies it server SSL certificate is validated. Note that you need to configure the lua_ssl_trusted_certificate to specify the CA (or server) certificate used by your Redis server. You may also need to configure lua_ssl_verify_depth accordingly.", type = "boolean", required = true, default = false }, },
{ redis_server_name = typedefs.sni },
{ redis_timeout = { description = "When using the `redis` policy, this property specifies the timeout in milliseconds of any command submitted to the Redis server.", type = "number", default = 2000, }, },
{ redis_database = { description = "When using the `redis` policy, this property specifies the Redis database to use.", type = "integer", default = 0 }, },
{ hide_client_headers = { description = "Optionally hide informative response headers.", type = "boolean", required = true, default = false }, },
{ error_code = { description = "Set a custom error code to return when the rate limit is exceeded.", type = "number", default = 429, gt = 0 }, },
{ error_message = { description = "Set a custom error message to return when the rate limit is exceeded.", type = "string", default = "API rate limit exceeded" }, },
{ sync_rate = { description = "How often to sync counter data to the central data store. A value of -1 results in synchronous behavior.", type = "number", required = true, default = -1 }, },
},
custom_validator = validate_periods_order,
shorthand_fields = {
-- TODO: deprecated forms, to be removed in Kong 4.0
{ redis_host = {
type = "string",
func = function(value)
deprecation("rate-limiting: config.redis_host is deprecated, please use config.redis.host instead",
{ after = "4.0", })
return { redis = { host = value } }
end
} },
{ redis_port = {
type = "integer",
func = function(value)
deprecation("rate-limiting: config.redis_port is deprecated, please use config.redis.port instead",
{ after = "4.0", })
return { redis = { port = value } }
end
} },
{ redis_password = {
type = "string",
func = function(value)
deprecation("rate-limiting: config.redis_password is deprecated, please use config.redis.password instead",
{ after = "4.0", })
return { redis = { password = value } }
end
} },
{ redis_username = {
type = "string",
func = function(value)
deprecation("rate-limiting: config.redis_username is deprecated, please use config.redis.username instead",
{ after = "4.0", })
return { redis = { username = value } }
end
} },
{ redis_ssl = {
type = "boolean",
func = function(value)
deprecation("rate-limiting: config.redis_ssl is deprecated, please use config.redis.ssl instead",
{ after = "4.0", })
return { redis = { ssl = value } }
end
} },
{ redis_ssl_verify = {
type = "boolean",
func = function(value)
deprecation("rate-limiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead",
{ after = "4.0", })
return { redis = { ssl_verify = value } }
end
} },
{ redis_server_name = {
type = "string",
func = function(value)
deprecation("rate-limiting: config.redis_server_name is deprecated, please use config.redis.server_name instead",
{ after = "4.0", })
return { redis = { server_name = value } }
end
} },
{ redis_timeout = {
type = "integer",
func = function(value)
deprecation("rate-limiting: config.redis_timeout is deprecated, please use config.redis.timeout instead",
{ after = "4.0", })
return { redis = { timeout = value } }
end
} },
{ redis_database = {
type = "integer",
func = function(value)
deprecation("rate-limiting: config.redis_database is deprecated, please use config.redis.database instead",
{ after = "4.0", })
return { redis = { database = value } }
end
} },
},
},
},
},
entity_checks = {
{ at_least_one_of = { "config.second", "config.minute", "config.hour", "config.day", "config.month", "config.year" } },
{ conditional_at_least_one_of = {
{ conditional = {
if_field = "config.policy", if_match = { eq = "redis" },
then_at_least_one_of = { "config.redis.host", "config.redis_host" },
then_err = "must set one of %s when 'policy' is 'redis'",
then_field = "config.redis.host", then_match = { required = true },
} },
{ conditional_at_least_one_of = {
{ conditional = {
if_field = "config.policy", if_match = { eq = "redis" },
then_at_least_one_of = { "config.redis.port", "config.redis_port" },
then_err = "must set one of %s when 'policy' is 'redis'",
then_field = "config.redis.port", then_match = { required = true },
} },
{ conditional_at_least_one_of = {
{ conditional = {
if_field = "config.policy", if_match = { eq = "redis" },
then_at_least_one_of = { "config.redis.timeout", "config.redis_timeout" },
then_err = "must set one of %s when 'policy' is 'redis'",
then_field = "config.redis.timeout", then_match = { required = true },
} },
{ conditional = {
if_field = "config.limit_by", if_match = { eq = "header" },
Expand All @@ -136,59 +199,5 @@ return {
if_field = "config.limit_by", if_match = { eq = "path" },
then_field = "config.path", then_match = { required = true },
} },
{ custom_entity_check = {
field_sources = {
"config.redis_host",
"config.redis_port",
"config.redis_password",
"config.redis_username",
"config.redis_ssl",
"config.redis_ssl_verify",
"config.redis_server_name",
"config.redis_timeout",
"config.redis_database"
},
fn = function(entity)

if (entity.config.redis_host or ngx.null) ~= ngx.null then
deprecation("rate-limiting: config.redis_host is deprecated, please use config.redis.host instead",
{ after = "4.0", })
end
if (entity.config.redis_port or ngx.null) ~= ngx.null and entity.config.redis_port ~= 6379 then
deprecation("rate-limiting: config.redis_port is deprecated, please use config.redis.port instead",
{ after = "4.0", })
end
if (entity.config.redis_password or ngx.null) ~= ngx.null then
deprecation("rate-limiting: config.redis_password is deprecated, please use config.redis.password instead",
{ after = "4.0", })
end
if (entity.config.redis_username or ngx.null) ~= ngx.null then
deprecation("rate-limiting: config.redis_username is deprecated, please use config.redis.username instead",
{ after = "4.0", })
end
if (entity.config.redis_ssl or ngx.null) ~= ngx.null and entity.config.redis_ssl ~= false then
deprecation("rate-limiting: config.redis_ssl is deprecated, please use config.redis.ssl instead",
{ after = "4.0", })
end
if (entity.config.redis_ssl_verify or ngx.null) ~= ngx.null and entity.config.redis_ssl_verify ~= false then
deprecation("rate-limiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead",
{ after = "4.0", })
end
if (entity.config.redis_server_name or ngx.null) ~= ngx.null then
deprecation("rate-limiting: config.redis_server_name is deprecated, please use config.redis.server_name instead",
{ after = "4.0", })
end
if (entity.config.redis_timeout or ngx.null) ~= ngx.null and entity.config.redis_timeout ~= 2000 then
deprecation("rate-limiting: config.redis_timeout is deprecated, please use config.redis.timeout instead",
{ after = "4.0", })
end
if (entity.config.redis_database or ngx.null) ~= ngx.null and entity.config.redis_database ~= 0 then
deprecation("rate-limiting: config.redis_database is deprecated, please use config.redis.database instead",
{ after = "4.0", })
end

return true
end
} }
},
}
18 changes: 9 additions & 9 deletions kong/plugins/response-ratelimiting/policies/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ end

local function get_redis_configuration(plugin_conf)
return {
host = plugin_conf.redis.host or plugin_conf.redis_host,
port = plugin_conf.redis.port or plugin_conf.redis_port,
username = plugin_conf.redis.username or plugin_conf.redis_username,
password = plugin_conf.redis.password or plugin_conf.redis_password,
database = plugin_conf.redis.database or plugin_conf.redis_database,
timeout = plugin_conf.redis.timeout or plugin_conf.redis_timeout,
ssl = plugin_conf.redis.ssl or plugin_conf.redis_ssl,
ssl_verify = plugin_conf.redis.ssl_verify or plugin_conf.redis_ssl_verify,
server_name = plugin_conf.redis.server_name or plugin_conf.redis_server_name,
host = plugin_conf.redis.host,
port = plugin_conf.redis.port,
username = plugin_conf.redis.username,
password = plugin_conf.redis.password,
database = plugin_conf.redis.database,
timeout = plugin_conf.redis.timeout,
ssl = plugin_conf.redis.ssl,
ssl_verify = plugin_conf.redis.ssl_verify,
server_name = plugin_conf.redis.server_name,
}
end

Expand Down
Loading

0 comments on commit e1663b5

Please sign in to comment.