From 25da4623da80116bf4eece365313329da53d45f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Nowak?= Date: Fri, 12 Jan 2024 14:52:09 +0100 Subject: [PATCH] chore(rl): standarize redis configuration (#12301) * chore(rl): standarize redis configuration Rate-Limiting right now has new config structure that reuses common redis connection configuration. The same as ACME plugin. KAG-3388 * chore(response-rl): standarize redis configuration Response-RateLimiting right now has new config structure that reuses common redis connection configuration. The same as ACME and RateLimiting plugin. KAG-3388 --- ...dize-redis-conifguration-rate-limiting.yml | 3 + ...ardize-redis-conifguration-response-rl.yml | 3 + kong-3.6.0-0.rockspec | 4 + kong/clustering/compat/checkers.lua | 4 +- .../clustering/compat/redis_translation.lua | 23 ++ .../migrations/006_350_to_360.lua | 38 +++ .../plugins/rate-limiting/migrations/init.lua | 1 + kong/plugins/rate-limiting/policies/init.lua | 61 +++-- kong/plugins/rate-limiting/schema.lua | 77 +++++- .../clustering/compat/redis_translation.lua | 23 ++ .../migrations/001_350_to_360.lua | 38 +++ .../response-ratelimiting/migrations/init.lua | 1 + .../response-ratelimiting/policies/init.lua | 50 ++-- kong/plugins/response-ratelimiting/schema.lua | 96 +++++-- kong/tools/redis/schema.lua | 1 + .../04-admin_api/15-off_spec.lua | 10 - .../09-hybrid_mode/09-config-compat_spec.lua | 94 +++++++ .../01-request-debug_spec.lua | 8 +- .../23-rate-limiting/01-schema_spec.lua | 66 +++++ .../23-rate-limiting/02-policies_spec.lua | 10 +- .../23-rate-limiting/04-access_spec.lua | 216 +++++++++------- .../23-rate-limiting/05-integration_spec.lua | 193 +++++++++++--- .../01-schema_spec.lua | 74 +++++- .../04-access_spec.lua | 238 ++++++++++-------- .../05-integration_spec.lua | 200 ++++++++++++--- .../migrations/006_350_to_360_spec.lua | 72 ++++++ .../migrations/001_350_to_360_spec.lua | 74 ++++++ 27 files changed, 1325 insertions(+), 353 deletions(-) create mode 100644 changelog/unreleased/kong/standardize-redis-conifguration-rate-limiting.yml create mode 100644 changelog/unreleased/kong/standardize-redis-conifguration-response-rl.yml create mode 100644 kong/plugins/rate-limiting/clustering/compat/redis_translation.lua create mode 100644 kong/plugins/rate-limiting/migrations/006_350_to_360.lua create mode 100644 kong/plugins/response-ratelimiting/clustering/compat/redis_translation.lua create mode 100644 kong/plugins/response-ratelimiting/migrations/001_350_to_360.lua create mode 100644 spec/05-migration/plugins/rate-limiting/migrations/006_350_to_360_spec.lua create mode 100644 spec/05-migration/plugins/response-ratelimiting/migrations/001_350_to_360_spec.lua diff --git a/changelog/unreleased/kong/standardize-redis-conifguration-rate-limiting.yml b/changelog/unreleased/kong/standardize-redis-conifguration-rate-limiting.yml new file mode 100644 index 000000000000..3ea7788baffb --- /dev/null +++ b/changelog/unreleased/kong/standardize-redis-conifguration-rate-limiting.yml @@ -0,0 +1,3 @@ +message: "**Rate Limiting**: Standardize redis configuration across plugins. The redis configuration right now follows common schema that is shared across other plugins." +type: deprecation +scope: Plugin diff --git a/changelog/unreleased/kong/standardize-redis-conifguration-response-rl.yml b/changelog/unreleased/kong/standardize-redis-conifguration-response-rl.yml new file mode 100644 index 000000000000..45045bb4d7f7 --- /dev/null +++ b/changelog/unreleased/kong/standardize-redis-conifguration-response-rl.yml @@ -0,0 +1,3 @@ +message: "**Response-RateLimiting**: Standardize redis configuration across plugins. The redis configuration right now follows common schema that is shared across other plugins." +type: deprecation +scope: Plugin diff --git a/kong-3.6.0-0.rockspec b/kong-3.6.0-0.rockspec index 04dabfb1d75d..e9879c7394a0 100644 --- a/kong-3.6.0-0.rockspec +++ b/kong-3.6.0-0.rockspec @@ -372,15 +372,18 @@ build = { ["kong.plugins.rate-limiting.migrations.003_10_to_112"] = "kong/plugins/rate-limiting/migrations/003_10_to_112.lua", ["kong.plugins.rate-limiting.migrations.004_200_to_210"] = "kong/plugins/rate-limiting/migrations/004_200_to_210.lua", ["kong.plugins.rate-limiting.migrations.005_320_to_330"] = "kong/plugins/rate-limiting/migrations/005_320_to_330.lua", + ["kong.plugins.rate-limiting.migrations.006_350_to_360"] = "kong/plugins/rate-limiting/migrations/006_350_to_360.lua", ["kong.plugins.rate-limiting.expiration"] = "kong/plugins/rate-limiting/expiration.lua", ["kong.plugins.rate-limiting.handler"] = "kong/plugins/rate-limiting/handler.lua", ["kong.plugins.rate-limiting.schema"] = "kong/plugins/rate-limiting/schema.lua", ["kong.plugins.rate-limiting.daos"] = "kong/plugins/rate-limiting/daos.lua", ["kong.plugins.rate-limiting.policies"] = "kong/plugins/rate-limiting/policies/init.lua", ["kong.plugins.rate-limiting.policies.cluster"] = "kong/plugins/rate-limiting/policies/cluster.lua", + ["kong.plugins.rate-limiting.clustering.compat.redis_translation"] = "kong/plugins/rate-limiting/clustering/compat/redis_translation.lua", ["kong.plugins.response-ratelimiting.migrations"] = "kong/plugins/response-ratelimiting/migrations/init.lua", ["kong.plugins.response-ratelimiting.migrations.000_base_response_rate_limiting"] = "kong/plugins/response-ratelimiting/migrations/000_base_response_rate_limiting.lua", + ["kong.plugins.response-ratelimiting.migrations.001_350_to_360"] = "kong/plugins/response-ratelimiting/migrations/001_350_to_360.lua", ["kong.plugins.response-ratelimiting.handler"] = "kong/plugins/response-ratelimiting/handler.lua", ["kong.plugins.response-ratelimiting.access"] = "kong/plugins/response-ratelimiting/access.lua", ["kong.plugins.response-ratelimiting.header_filter"] = "kong/plugins/response-ratelimiting/header_filter.lua", @@ -388,6 +391,7 @@ build = { ["kong.plugins.response-ratelimiting.schema"] = "kong/plugins/response-ratelimiting/schema.lua", ["kong.plugins.response-ratelimiting.policies"] = "kong/plugins/response-ratelimiting/policies/init.lua", ["kong.plugins.response-ratelimiting.policies.cluster"] = "kong/plugins/response-ratelimiting/policies/cluster.lua", + ["kong.plugins.response-ratelimiting.clustering.compat.redis_translation"] = "kong/plugins/response-ratelimiting/clustering/compat/redis_translation.lua", ["kong.plugins.request-size-limiting.handler"] = "kong/plugins/request-size-limiting/handler.lua", ["kong.plugins.request-size-limiting.schema"] = "kong/plugins/request-size-limiting/schema.lua", diff --git a/kong/clustering/compat/checkers.lua b/kong/clustering/compat/checkers.lua index 4866d5fbda49..2cc89cba3821 100644 --- a/kong/clustering/compat/checkers.lua +++ b/kong/clustering/compat/checkers.lua @@ -27,7 +27,9 @@ local compatible_checkers = { function(config_table, dp_version, log_suffix) local has_update local redis_plugins_update = { - acme = require("kong.plugins.acme.clustering.compat.redis_translation").adapter + acme = require("kong.plugins.acme.clustering.compat.redis_translation").adapter, + ['rate-limiting'] = require("kong.plugins.rate-limiting.clustering.compat.redis_translation").adapter, + ['response-ratelimiting'] = require("kong.plugins.response-ratelimiting.clustering.compat.redis_translation").adapter } for _, plugin in ipairs(config_table.plugins or {}) do diff --git a/kong/plugins/rate-limiting/clustering/compat/redis_translation.lua b/kong/plugins/rate-limiting/clustering/compat/redis_translation.lua new file mode 100644 index 000000000000..051f2aba4b31 --- /dev/null +++ b/kong/plugins/rate-limiting/clustering/compat/redis_translation.lua @@ -0,0 +1,23 @@ +local function adapter(config_to_update) + if config_to_update.policy == "redis" then + config_to_update.redis_host = config_to_update.redis.host + config_to_update.redis_port = config_to_update.redis.port + config_to_update.redis_username = config_to_update.redis.username + config_to_update.redis_password = config_to_update.redis.password + config_to_update.redis_database = config_to_update.redis.database + config_to_update.redis_timeout = config_to_update.redis.timeout + config_to_update.redis_ssl = config_to_update.redis.ssl + config_to_update.redis_ssl_verify = config_to_update.redis.ssl_verify + config_to_update.redis_server_name = config_to_update.redis.server_name + + config_to_update.redis = nil + + return true + end + + return false +end + +return { + adapter = adapter +} diff --git a/kong/plugins/rate-limiting/migrations/006_350_to_360.lua b/kong/plugins/rate-limiting/migrations/006_350_to_360.lua new file mode 100644 index 000000000000..cc697f7cbeba --- /dev/null +++ b/kong/plugins/rate-limiting/migrations/006_350_to_360.lua @@ -0,0 +1,38 @@ +return { + postgres = { + up = [[ + DO $$ + BEGIN + UPDATE plugins + SET config = + config::jsonb + - 'redis_host' + - 'redis_port' + - 'redis_password' + - 'redis_username' + - 'redis_ssl' + - 'redis_ssl_verify' + - 'redis_server_name' + - 'redis_timeout' + - 'redis_database' + || jsonb_build_object( + 'redis', + jsonb_build_object( + 'host', config->'redis_host', + 'port', config->'redis_port', + 'password', config->'redis_password', + 'username', config->'redis_username', + 'ssl', config->'redis_ssl', + 'ssl_verify', config->'redis_ssl_verify', + 'server_name', config->'redis_server_name', + 'timeout', config->'redis_timeout', + 'database', config->'redis_database' + ) + ) + WHERE name = 'rate-limiting'; + EXCEPTION WHEN UNDEFINED_COLUMN OR UNDEFINED_TABLE THEN + -- Do nothing, accept existing state + END$$; + ]], + }, +} diff --git a/kong/plugins/rate-limiting/migrations/init.lua b/kong/plugins/rate-limiting/migrations/init.lua index 74c3d402d1e8..4b0c18fcca8a 100644 --- a/kong/plugins/rate-limiting/migrations/init.lua +++ b/kong/plugins/rate-limiting/migrations/init.lua @@ -3,4 +3,5 @@ return { "003_10_to_112", "004_200_to_210", "005_320_to_330", + "006_350_to_360", } diff --git a/kong/plugins/rate-limiting/policies/init.lua b/kong/plugins/rate-limiting/policies/init.lua index f372d6310a7d..1d5e3c68efb9 100644 --- a/kong/plugins/rate-limiting/policies/init.lua +++ b/kong/plugins/rate-limiting/policies/init.lua @@ -78,31 +78,48 @@ local sock_opts = {} 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, + } +end + + local function get_db_key(conf) + local redis_config = get_redis_configuration(conf) return fmt("%s:%d;%d", - conf.redis_host, - conf.redis_port, - conf.redis_database) + redis_config.host, + redis_config.port, + redis_config.database) end local function get_redis_connection(conf) local red = redis:new() - red:set_timeout(conf.redis_timeout) + local redis_config = get_redis_configuration(conf) + red:set_timeout(redis_config.timeout) - sock_opts.ssl = conf.redis_ssl - sock_opts.ssl_verify = conf.redis_ssl_verify - sock_opts.server_name = conf.redis_server_name + sock_opts.ssl = redis_config.ssl + sock_opts.ssl_verify = redis_config.ssl_verify + sock_opts.server_name = redis_config.server_name local db_key = get_db_key(conf) - -- use a special pool name only if redis_database is set to non-zero + -- use a special pool name only if redis_config.database is set to non-zero -- otherwise use the default pool name host:port - if conf.redis_database ~= 0 then + if redis_config.database ~= 0 then sock_opts.pool = db_key end - local ok, err = red:connect(conf.redis_host, conf.redis_port, + local ok, err = red:connect(redis_config.host, redis_config.port, sock_opts) if not ok then kong.log.err("failed to connect to Redis: ", err) @@ -116,16 +133,16 @@ local function get_redis_connection(conf) end if times == 0 then - if is_present(conf.redis_password) then + if is_present(redis_config.password) then local ok, err - if is_present(conf.redis_username) then + if is_present(redis_config.username) then ok, err = kong.vault.try(function(cfg) - return red:auth(cfg.redis_username, cfg.redis_password) - end, conf) + return red:auth(cfg.username, cfg.password) + end, redis_config) else ok, err = kong.vault.try(function(cfg) - return red:auth(cfg.redis_password) - end, conf) + return red:auth(cfg.password) + end, redis_config) end if not ok then kong.log.err("failed to auth Redis: ", err) @@ -133,11 +150,11 @@ local function get_redis_connection(conf) end end - if conf.redis_database ~= 0 then + if redis_config.database ~= 0 then -- Only call select first time, since we know the connection is shared -- between instances that use the same redis database - local ok, err = red:select(conf.redis_database) + local ok, err = red:select(redis_config.database) if not ok then kong.log.err("failed to change Redis database: ", err) return nil, db_key, err @@ -213,6 +230,8 @@ local plugin_sync_running = {} -- will be sync to Redis at most sync_rate interval local function rate_limited_sync(conf, sync_func) local cache_key = conf.__key__ or conf.__plugin_id or "rate-limiting" + local redis_config = get_redis_configuration(conf) + -- a timer is pending. The change will be picked up by the pending timer if plugin_sync_pending[cache_key] then return true @@ -231,7 +250,7 @@ local function rate_limited_sync(conf, sync_func) -- a "pending" state is never touched before the timer is started assert(plugin_sync_pending[cache_key]) - + local tries = 0 -- a timer is already running. -- the sleep time is picked to a seemingly reasonable value @@ -245,8 +264,8 @@ local function rate_limited_sync(conf, sync_func) kong.log.emerg("A Redis sync is blocked by a previous try. " .. "The previous try should have timed out but it didn't for unknown reasons.") end - - ngx.sleep(conf.redis_timeout / 2) + + ngx.sleep(redis_config.timeout / 2) tries = tries + 1 end diff --git a/kong/plugins/rate-limiting/schema.lua b/kong/plugins/rate-limiting/schema.lua index 1d9a9cdf55a0..18abb84f7ae3 100644 --- a/kong/plugins/rate-limiting/schema.lua +++ b/kong/plugins/rate-limiting/schema.lua @@ -1,5 +1,6 @@ local typedefs = require "kong.db.schema.typedefs" - +local redis_schema = require "kong.tools.redis.schema" +local deprecation = require "kong.deprecation" local SYNC_RATE_REALTIME = -1 @@ -91,6 +92,7 @@ return { { path = typedefs.path }, { 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 }, }, @@ -111,13 +113,20 @@ return { }, entity_checks = { { at_least_one_of = { "config.second", "config.minute", "config.hour", "config.day", "config.month", "config.year" } }, - { conditional = { + { conditional_at_least_one_of = { if_field = "config.policy", if_match = { eq = "redis" }, - then_field = "config.redis_host", then_match = { required = true }, + then_at_least_one_of = { "config.redis.host", "config.redis_host" }, + then_err = "must set one of %s when 'policy' is 'redis'", } }, - { conditional = { + { conditional_at_least_one_of = { if_field = "config.policy", if_match = { eq = "redis" }, - then_field = "config.redis_port", then_match = { required = true }, + then_at_least_one_of = { "config.redis.port", "config.redis_port" }, + then_err = "must set one of %s when 'policy' is 'redis'", + } }, + { conditional_at_least_one_of = { + 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'", } }, { conditional = { if_field = "config.limit_by", if_match = { eq = "header" }, @@ -127,9 +136,59 @@ return { if_field = "config.limit_by", if_match = { eq = "path" }, then_field = "config.path", then_match = { required = true }, } }, - { conditional = { - if_field = "config.policy", if_match = { eq = "redis" }, - then_field = "config.redis_timeout", 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 + } } }, } diff --git a/kong/plugins/response-ratelimiting/clustering/compat/redis_translation.lua b/kong/plugins/response-ratelimiting/clustering/compat/redis_translation.lua new file mode 100644 index 000000000000..051f2aba4b31 --- /dev/null +++ b/kong/plugins/response-ratelimiting/clustering/compat/redis_translation.lua @@ -0,0 +1,23 @@ +local function adapter(config_to_update) + if config_to_update.policy == "redis" then + config_to_update.redis_host = config_to_update.redis.host + config_to_update.redis_port = config_to_update.redis.port + config_to_update.redis_username = config_to_update.redis.username + config_to_update.redis_password = config_to_update.redis.password + config_to_update.redis_database = config_to_update.redis.database + config_to_update.redis_timeout = config_to_update.redis.timeout + config_to_update.redis_ssl = config_to_update.redis.ssl + config_to_update.redis_ssl_verify = config_to_update.redis.ssl_verify + config_to_update.redis_server_name = config_to_update.redis.server_name + + config_to_update.redis = nil + + return true + end + + return false +end + +return { + adapter = adapter +} diff --git a/kong/plugins/response-ratelimiting/migrations/001_350_to_360.lua b/kong/plugins/response-ratelimiting/migrations/001_350_to_360.lua new file mode 100644 index 000000000000..a67fe338e9e2 --- /dev/null +++ b/kong/plugins/response-ratelimiting/migrations/001_350_to_360.lua @@ -0,0 +1,38 @@ +return { + postgres = { + up = [[ + DO $$ + BEGIN + UPDATE plugins + SET config = + config::jsonb + - 'redis_host' + - 'redis_port' + - 'redis_password' + - 'redis_username' + - 'redis_ssl' + - 'redis_ssl_verify' + - 'redis_server_name' + - 'redis_timeout' + - 'redis_database' + || jsonb_build_object( + 'redis', + jsonb_build_object( + 'host', config->'redis_host', + 'port', config->'redis_port', + 'password', config->'redis_password', + 'username', config->'redis_username', + 'ssl', config->'redis_ssl', + 'ssl_verify', config->'redis_ssl_verify', + 'server_name', config->'redis_server_name', + 'timeout', config->'redis_timeout', + 'database', config->'redis_database' + ) + ) + WHERE name = 'response-ratelimiting'; + EXCEPTION WHEN UNDEFINED_COLUMN OR UNDEFINED_TABLE THEN + -- Do nothing, accept existing state + END$$; + ]], + }, +} diff --git a/kong/plugins/response-ratelimiting/migrations/init.lua b/kong/plugins/response-ratelimiting/migrations/init.lua index 8043ed0295b0..6ccfd21f7aa7 100644 --- a/kong/plugins/response-ratelimiting/migrations/init.lua +++ b/kong/plugins/response-ratelimiting/migrations/init.lua @@ -1,3 +1,4 @@ return { "000_base_response_rate_limiting", + "001_350_to_360", } diff --git a/kong/plugins/response-ratelimiting/policies/init.lua b/kong/plugins/response-ratelimiting/policies/init.lua index 6c6a5e82308c..16e8b3202058 100644 --- a/kong/plugins/response-ratelimiting/policies/init.lua +++ b/kong/plugins/response-ratelimiting/policies/init.lua @@ -25,6 +25,19 @@ local function is_present(str) return str and str ~= "" and str ~= null 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, + } +end local function get_service_and_route_ids(conf) conf = conf or {} @@ -53,22 +66,23 @@ end local sock_opts = {} local function get_redis_connection(conf) local red = redis:new() - red:set_timeout(conf.redis_timeout) + local redis_config = get_redis_configuration(conf) + red:set_timeout(redis_config.timeout) - sock_opts.ssl = conf.redis_ssl - sock_opts.ssl_verify = conf.redis_ssl_verify - sock_opts.server_name = conf.redis_server_name + sock_opts.ssl = redis_config.ssl + sock_opts.ssl_verify = redis_config.ssl_verify + sock_opts.server_name = redis_config.server_name - -- use a special pool name only if redis_database is set to non-zero + -- use a special pool name only if redis_config.database is set to non-zero -- otherwise use the default pool name host:port - if conf.redis_database ~= 0 then + if redis_config.database ~= 0 then sock_opts.pool = fmt( "%s:%d;%d", - conf.redis_host, - conf.redis_port, - conf.redis_database) + redis_config.host, + redis_config.port, + redis_config.database) end - local ok, err = red:connect(conf.redis_host, conf.redis_port, + local ok, err = red:connect(redis_config.host, redis_config.port, sock_opts) if not ok then kong.log.err("failed to connect to Redis: ", err) @@ -82,16 +96,16 @@ local function get_redis_connection(conf) end if times == 0 then - if is_present(conf.redis_password) then + if is_present(redis_config.password) then local ok, err - if is_present(conf.redis_username) then + if is_present(redis_config.username) then ok, err = kong.vault.try(function(cfg) - return red:auth(cfg.redis_username, cfg.redis_password) - end, conf) + return red:auth(cfg.username, cfg.password) + end, redis_config) else ok, err = kong.vault.try(function(cfg) - return red:auth(cfg.redis_password) - end, conf) + return red:auth(cfg.password) + end, redis_config) end if not ok then kong.log.err("failed to auth Redis: ", err) @@ -99,11 +113,11 @@ local function get_redis_connection(conf) end end - if conf.redis_database ~= 0 then + if redis_config.database ~= 0 then -- Only call select first time, since we know the connection is shared -- between instances that use the same redis database - local ok, err = red:select(conf.redis_database) + local ok, err = red:select(redis_config.database) if not ok then kong.log.err("failed to change Redis database: ", err) return nil, err diff --git a/kong/plugins/response-ratelimiting/schema.lua b/kong/plugins/response-ratelimiting/schema.lua index 2125bba8094c..b36b4948b619 100644 --- a/kong/plugins/response-ratelimiting/schema.lua +++ b/kong/plugins/response-ratelimiting/schema.lua @@ -1,5 +1,6 @@ local typedefs = require "kong.db.schema.typedefs" - +local redis_schema = require "kong.tools.redis.schema" +local deprecation = require "kong.deprecation" local ORDERED_PERIODS = { "second", "minute", "hour", "day", "month", "year" } @@ -94,6 +95,7 @@ return { default = true }, }, + { redis = redis_schema.config_schema }, { redis_host = typedefs.redis_host, }, @@ -202,29 +204,73 @@ return { }, }, entity_checks = { - { - conditional = { - if_field = "config.policy", - if_match = { eq = "redis" }, - then_field = "config.redis_host", - then_match = { required = true }, - } - }, - { - conditional = { - if_field = "config.policy", - if_match = { eq = "redis" }, - then_field = "config.redis_port", - then_match = { required = true }, - } - }, - { - conditional = { - if_field = "config.policy", - if_match = { eq = "redis" }, - then_field = "config.redis_timeout", - then_match = { required = true }, - } - }, + { conditional_at_least_one_of = { + 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'", + } }, + { conditional_at_least_one_of = { + 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'", + } }, + { conditional_at_least_one_of = { + 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'", + } }, + { 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: 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("response-ratelimiting: config.redis_database is deprecated, please use config.redis.database instead", + { after = "4.0", }) + end + + return true + end + } } }, } diff --git a/kong/tools/redis/schema.lua b/kong/tools/redis/schema.lua index e40a72532e7a..39f2c19b06d0 100644 --- a/kong/tools/redis/schema.lua +++ b/kong/tools/redis/schema.lua @@ -4,6 +4,7 @@ local DEFAULT_TIMEOUT = 2000 return { config_schema = { type = "record", + description = "Redis configuration", fields = { { host = typedefs.host }, { port = typedefs.port }, diff --git a/spec/02-integration/04-admin_api/15-off_spec.lua b/spec/02-integration/04-admin_api/15-off_spec.lua index 1f618e4cfec1..655a9e621bb4 100644 --- a/spec/02-integration/04-admin_api/15-off_spec.lua +++ b/spec/02-integration/04-admin_api/15-off_spec.lua @@ -2472,11 +2472,6 @@ R6InCcH2Wh8wSeY5AuDXvu2tv9g/PW9wIJmPuKSHMA== hide_client_headers = false, limit_by = "consumer", policy = "local", - redis_database = 0, - redis_port = 6379, - redis_ssl = false, - redis_ssl_verify = false, - redis_timeout = 2000, second = 2000, }, enabled = true, @@ -2551,11 +2546,6 @@ R6InCcH2Wh8wSeY5AuDXvu2tv9g/PW9wIJmPuKSHMA== hide_client_headers = false, limit_by = "consumer", policy = "local", - redis_database = 0, - redis_port = 6379, - redis_ssl = false, - redis_ssl_verify = false, - redis_timeout = 2000, second = 2000, }, consumer = username, diff --git a/spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua b/spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua index af3a0aaf404b..60b07225bd28 100644 --- a/spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua +++ b/spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua @@ -362,6 +362,100 @@ describe("CP/DP config compat transformations #" .. strategy, function() admin.plugins:remove({ id = acme.id }) end) end) + + describe("rate-limiting plugin", function() + it("translates standardized redis config to older rate-limiting structure", function() + -- [[ 3.6.x ]] -- + local rl = admin.plugins:insert { + name = "rate-limiting", + enabled = true, + config = { + minute = 300, + policy = "redis", + -- [[ new structure redis + redis = { + host = "localhost", + port = 57198, + username = "test", + password = "secret", + database = 2, + timeout = 1100, + ssl = true, + ssl_verify = true, + server_name = "example.test" + } + -- ]] + } + } + + local expected_rl_prior_36 = utils.cycle_aware_deep_copy(rl) + expected_rl_prior_36.config.redis = nil + expected_rl_prior_36.config.redis_host = "localhost" + expected_rl_prior_36.config.redis_port = 57198 + expected_rl_prior_36.config.redis_username = "test" + expected_rl_prior_36.config.redis_password = "secret" + expected_rl_prior_36.config.redis_database = 2 + expected_rl_prior_36.config.redis_timeout = 1100 + expected_rl_prior_36.config.redis_ssl = true + expected_rl_prior_36.config.redis_ssl_verify = true + expected_rl_prior_36.config.redis_server_name = "example.test" + + + do_assert(utils.uuid(), "3.5.0", expected_rl_prior_36) + + -- cleanup + admin.plugins:remove({ id = rl.id }) + end) + end) + + describe("response-ratelimiting plugin", function() + it("translates standardized redis config to older response-ratelimiting structure", function() + -- [[ 3.6.x ]] -- + local response_rl = admin.plugins:insert { + name = "response-ratelimiting", + enabled = true, + config = { + limits = { + video = { + minute = 300, + } + }, + policy = "redis", + -- [[ new structure redis + redis = { + host = "localhost", + port = 57198, + username = "test", + password = "secret", + database = 2, + timeout = 1100, + ssl = true, + ssl_verify = true, + server_name = "example.test" + } + -- ]] + } + } + + local expected_response_rl_prior_36 = utils.cycle_aware_deep_copy(response_rl) + expected_response_rl_prior_36.config.redis = nil + expected_response_rl_prior_36.config.redis_host = "localhost" + expected_response_rl_prior_36.config.redis_port = 57198 + expected_response_rl_prior_36.config.redis_username = "test" + expected_response_rl_prior_36.config.redis_password = "secret" + expected_response_rl_prior_36.config.redis_database = 2 + expected_response_rl_prior_36.config.redis_timeout = 1100 + expected_response_rl_prior_36.config.redis_ssl = true + expected_response_rl_prior_36.config.redis_ssl_verify = true + expected_response_rl_prior_36.config.redis_server_name = "example.test" + + + do_assert(utils.uuid(), "3.5.0", expected_response_rl_prior_36) + + -- cleanup + admin.plugins:remove({ id = response_rl.id }) + end) + end) end) end) end) diff --git a/spec/02-integration/21-request-debug/01-request-debug_spec.lua b/spec/02-integration/21-request-debug/01-request-debug_spec.lua index a507e4a80a00..8be19151782d 100644 --- a/spec/02-integration/21-request-debug/01-request-debug_spec.lua +++ b/spec/02-integration/21-request-debug/01-request-debug_spec.lua @@ -625,10 +625,12 @@ describe(desc, function() local plugin_id = setup_plugin(route_id, "rate-limiting", { second = 9999, policy = "redis", - redis_host = helpers.redis_host, - redis_port = helpers.redis_port, fault_tolerant = false, - redis_timeout = 10000, + redis = { + host = helpers.redis_host, + port = helpers.redis_port, + timeout = 10000, + } }) finally(function() diff --git a/spec/03-plugins/23-rate-limiting/01-schema_spec.lua b/spec/03-plugins/23-rate-limiting/01-schema_spec.lua index c5daa8ec3f77..517463b64100 100644 --- a/spec/03-plugins/23-rate-limiting/01-schema_spec.lua +++ b/spec/03-plugins/23-rate-limiting/01-schema_spec.lua @@ -1,3 +1,4 @@ +local helpers = require "spec.helpers" local schema_def = require "kong.plugins.rate-limiting.schema" local v = require("spec.helpers").validate_plugin_config_schema @@ -67,5 +68,70 @@ describe("Plugin: rate-limiting (schema)", function() assert.falsy(ok) assert.equal("required field missing", err.config.path) end) + + it("is limited by path but the path field is missing", function() + local config = { second = 10, limit_by = "path", path = nil } + local ok, err = v(config, schema_def) + assert.falsy(ok) + assert.equal("required field missing", err.config.path) + end) + + it("proper config validates with redis new structure", function() + local config = { + second = 10, + policy = "redis", + redis = { + host = helpers.redis_host, + port = helpers.redis_port, + database = 0, + username = "test", + password = "testXXX", + ssl = true, + ssl_verify = false, + timeout = 1100, + server_name = helpers.redis_ssl_sni, + } } + local ok, _, err = v(config, schema_def) + assert.truthy(ok) + assert.is_nil(err) + end) + + it("proper config validates with redis legacy structure", function() + local config = { + second = 10, + policy = "redis", + redis_host = helpers.redis_host, + redis_port = helpers.redis_port, + redis_database = 0, + redis_username = "test", + redis_password = "testXXX", + redis_ssl = true, + redis_ssl_verify = false, + redis_timeout = 1100, + redis_server_name = helpers.redis_ssl_sni, + } + local ok, _, err = v(config, schema_def) + assert.truthy(ok) + assert.is_nil(err) + end) + + it("verifies that redis required fields are supplied", function() + local config = { + second = 10, + policy = "redis", + redis = { + port = helpers.redis_port, + database = 0, + username = "test", + password = "testXXX", + ssl = true, + ssl_verify = false, + timeout = 1100, + server_name = helpers.redis_ssl_sni, + } } + local ok, err = v(config, schema_def) + assert.falsy(ok) + assert.contains("must set one of 'config.redis.host', 'config.redis_host' when 'policy' is 'redis'", err["@entity"]) + end) end) end) diff --git a/spec/03-plugins/23-rate-limiting/02-policies_spec.lua b/spec/03-plugins/23-rate-limiting/02-policies_spec.lua index b221da87582c..640de183f1db 100644 --- a/spec/03-plugins/23-rate-limiting/02-policies_spec.lua +++ b/spec/03-plugins/23-rate-limiting/02-policies_spec.lua @@ -195,9 +195,11 @@ describe("Plugin: rate-limiting (policies)", function() local conf = { route_id = uuid(), service_id = uuid(), - redis_host = helpers.redis_host, - redis_port = helpers.redis_port, - redis_database = 0, + redis = { + host = helpers.redis_host, + port = helpers.redis_port, + database = 0, + }, sync_rate = sync_rate, } @@ -205,7 +207,7 @@ describe("Plugin: rate-limiting (policies)", function() local red = require "resty.redis" local redis = assert(red:new()) redis:set_timeout(1000) - assert(redis:connect(conf.redis_host, conf.redis_port)) + assert(redis:connect(conf.redis.host, conf.redis.port)) redis:flushall() redis:close() end) diff --git a/spec/03-plugins/23-rate-limiting/04-access_spec.lua b/spec/03-plugins/23-rate-limiting/04-access_spec.lua index 9601d4deb243..ba128c616eef 100644 --- a/spec/03-plugins/23-rate-limiting/04-access_spec.lua +++ b/spec/03-plugins/23-rate-limiting/04-access_spec.lua @@ -474,13 +474,15 @@ describe(desc, function() limit_by = limit_by, path = test_path, -- only for limit_by = "path" header_name = test_header, -- only for limit_by = "header" - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) local auth_plugin @@ -545,13 +547,15 @@ if limit_by == "ip" then minute = 6, policy = policy, limit_by = "ip", - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -583,13 +587,15 @@ if limit_by == "ip" then minute = 6, policy = policy, limit_by = "ip", - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -657,13 +663,15 @@ if limit_by == "ip" then policy = policy, limit_by = "ip", hide_client_headers = true, - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -700,13 +708,15 @@ if limit_by == "ip" then limit_by = limit_by, path = test_path, header_name = test_header, - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -739,13 +749,15 @@ if limit_by == "ip" then second = 1, policy = policy, limit_by = "ip", - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -797,13 +809,15 @@ if limit_by == "ip" then policy = policy, limit_by = limit_by, path = test_path, - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + }, error_code = 404, error_message = "Fake Not Found", }, service) @@ -843,13 +857,15 @@ if limit_by == "service" then minute = 6, policy = policy, limit_by = "service", - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }) finally(function() @@ -887,13 +903,15 @@ if limit_by == "path" then policy = policy, limit_by = "path", path = test_path_1, - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -931,13 +949,15 @@ if limit_by == "header" then policy = policy, limit_by = "header", header_name = test_header_1, - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) finally(function() @@ -974,13 +994,15 @@ if limit_by == "consumer" or limit_by == "credential" then minute = 6, policy = policy, limit_by = limit_by, - redis_host = REDIS_HOST, - redis_port = ssl_conf.redis_port, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = ssl_conf.redis_ssl, - redis_ssl_verify = ssl_conf.redis_ssl_verify, - redis_server_name = ssl_conf.redis_server_name, + redis = { + host = REDIS_HOST, + port = ssl_conf.redis_port, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = ssl_conf.redis_ssl, + ssl_verify = ssl_conf.redis_ssl_verify, + server_name = ssl_conf.redis_server_name, + } }, service) local auth_plugin = setup_key_auth_plugin(admin_client, { key_names = { test_key_name }, @@ -1181,9 +1203,11 @@ if policy == "redis" then minute = 6, policy = "redis", limit_by = "ip", - redis_host = "127.0.0.1", - redis_port = 80, -- bad redis port - redis_ssl = false, + redis = { + host = "127.0.0.1", + port = 80, -- bad redis port + ssl = false, + }, fault_tolerant = false, }, service) @@ -1210,9 +1234,11 @@ if policy == "redis" then minute = 6, policy = "redis", limit_by = "ip", - redis_host = "127.0.0.1", - redis_port = 80, -- bad redis port - redis_ssl = false, + redis = { + host = "127.0.0.1", + port = 80, -- bad redis port + ssl = false, + }, fault_tolerant = true, }, service) @@ -1284,11 +1310,13 @@ describe(desc, function () minute = 6, policy = "redis", limit_by = "ip", - redis_host = REDIS_HOST, - redis_port = REDIS_PORT, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = false, + redis = { + host = REDIS_HOST, + port = REDIS_PORT, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = false, + }, sync_rate = 10, }, service) local red = redis_connect() @@ -1397,11 +1425,13 @@ describe(desc, function () minute = 6, policy = "local", limit_by = "credential", - redis_host = REDIS_HOST, - redis_port = REDIS_PORT, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, - redis_ssl = false, + redis = { + host = REDIS_HOST, + port = REDIS_PORT, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + ssl = false, + } }) local credential = setup_credential(admin_client, consumer, test_credential) diff --git a/spec/03-plugins/23-rate-limiting/05-integration_spec.lua b/spec/03-plugins/23-rate-limiting/05-integration_spec.lua index 4402c451325d..f2cc5b11329a 100644 --- a/spec/03-plugins/23-rate-limiting/05-integration_spec.lua +++ b/spec/03-plugins/23-rate-limiting/05-integration_spec.lua @@ -1,6 +1,7 @@ local helpers = require "spec.helpers" local redis = require "resty.redis" local version = require "version" +local cjson = require "cjson" local REDIS_HOST = helpers.redis_host @@ -70,7 +71,7 @@ describe("Plugin: rate-limiting (integration)", function() end) local strategies = { - no_ssl = { + no_ssl = { redis_port = REDIS_PORT, }, ssl_verify = { @@ -121,14 +122,16 @@ describe("Plugin: rate-limiting (integration)", function() config = { minute = 1, policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_database = REDIS_DB_1, - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + database = REDIS_DB_1, + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, sync_rate = with_sync_rate and SYNC_RATE or nil, }, }) @@ -142,14 +145,16 @@ describe("Plugin: rate-limiting (integration)", function() config = { minute = 1, policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_database = REDIS_DB_2, - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + database = REDIS_DB_2, + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, }, }) @@ -163,16 +168,18 @@ describe("Plugin: rate-limiting (integration)", function() config = { minute = 2, -- Handle multiple tests policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_username = REDIS_USER_VALID, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DB_3, -- ensure to not get a pooled authenticated connection by using a different db - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + username = REDIS_USER_VALID, + password = REDIS_PASSWORD, + database = REDIS_DB_3, -- ensure to not get a pooled authenticated connection by using a different db + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, }, }) @@ -185,16 +192,18 @@ describe("Plugin: rate-limiting (integration)", function() config = { minute = 1, policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_username = REDIS_USER_INVALID, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DB_4, -- ensure to not get a pooled authenticated connection by using a different db - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + username = REDIS_USER_INVALID, + password = REDIS_PASSWORD, + database = REDIS_DB_4, -- ensure to not get a pooled authenticated connection by using a different db + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, }, }) end @@ -353,8 +362,124 @@ describe("Plugin: rate-limiting (integration)", function() "'fails to rate-limit for a redis user with missing ACLs' will be skipped") end end) + end) + end -- for each redis strategy + + describe("creating rate-limiting plugins using api", function () + local route3, admin_client + + lazy_setup(function() + assert(helpers.start_kong({ + nginx_conf = "spec/fixtures/custom_nginx.template", + })) + + route3 = assert(bp.routes:insert { + hosts = { "redistest3.test" }, + }) + admin_client = helpers.admin_client() end) - end + + lazy_teardown(function() + if admin_client then + admin_client:close() + end + + helpers.stop_kong() + end) + + before_each(function() + helpers.clean_logfile() + end) + + local function delete_plugin(admin_client, plugin) + local res = assert(admin_client:send({ + method = "DELETE", + path = "/plugins/" .. plugin.id, + })) + + assert.res_status(204, res) + end + + it("allows to create a plugin with new redis configuration", function() + local res = assert(admin_client:send { + method = "POST", + route = { + id = route3.id + }, + path = "/plugins", + headers = { ["Content-Type"] = "application/json" }, + body = { + name = "rate-limiting", + config = { + minute = 100, + policy = "redis", + redis = { + host = helpers.redis_host, + port = helpers.redis_port, + username = "test1", + password = "testX", + database = 1, + timeout = 1100, + ssl = true, + ssl_verify = true, + server_name = "example.test", + }, + }, + }, + }) + + local json = cjson.decode(assert.res_status(201, res)) + delete_plugin(admin_client, json) + assert.logfile().has.no.line("rate-limiting: config.redis_host is deprecated, please use config.redis.host instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_port is deprecated, please use config.redis.port instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_password is deprecated, please use config.redis.password instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_username is deprecated, please use config.redis.username instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_ssl is deprecated, please use config.redis.ssl instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_server_name is deprecated, please use config.redis.server_name instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_timeout is deprecated, please use config.redis.timeout instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("rate-limiting: config.redis_database is deprecated, please use config.redis.database instead (deprecated after 4.0)", true) + end) + + it("allows to create a plugin with legacy redis configuration", function() + local res = assert(admin_client:send { + method = "POST", + route = { + id = route3.id + }, + path = "/plugins", + headers = { ["Content-Type"] = "application/json" }, + body = { + name = "rate-limiting", + config = { + minute = 100, + policy = "redis", + redis_host = "custom-host.example.test", + redis_port = 55000, + redis_username = "test1", + redis_password = "testX", + redis_database = 1, + redis_timeout = 1100, + redis_ssl = true, + redis_ssl_verify = true, + redis_server_name = "example.test", + }, + }, + }) + + local json = cjson.decode(assert.res_status(201, res)) + delete_plugin(admin_client, json) + assert.logfile().has.line("rate-limiting: config.redis_host is deprecated, please use config.redis.host instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_port is deprecated, please use config.redis.port instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_password is deprecated, please use config.redis.password instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_username is deprecated, please use config.redis.username instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_ssl is deprecated, please use config.redis.ssl instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_server_name is deprecated, please use config.redis.server_name instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_timeout is deprecated, please use config.redis.timeout instead (deprecated after 4.0)", true) + assert.logfile().has.line("rate-limiting: config.redis_database is deprecated, please use config.redis.database instead (deprecated after 4.0)", true) + end) + end) end end) diff --git a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua index 9455b197035e..6eca5929c307 100644 --- a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua @@ -1,5 +1,7 @@ local schema_def = require "kong.plugins.response-ratelimiting.schema" -local v = require("spec.helpers").validate_plugin_config_schema +local helpers = require "spec.helpers" +local v = helpers.validate_plugin_config_schema + local null = ngx.null @@ -59,5 +61,75 @@ describe("Plugin: response-rate-limiting (schema)", function() assert.falsy(ok) assert.equal("expected a record", err.config.limits) end) + + it("proper config validates with redis new structure", function() + local config = { + limits = { + video = { + second = 10 + } + }, + policy = "redis", + redis = { + host = helpers.redis_host, + port = helpers.redis_port, + database = 0, + username = "test", + password = "testXXX", + ssl = true, + ssl_verify = false, + timeout = 1100, + server_name = helpers.redis_ssl_sni, + } } + local ok, _, err = v(config, schema_def) + assert.truthy(ok) + assert.is_nil(err) + end) + + it("proper config validates with redis legacy structure", function() + local config = { + limits = { + video = { + second = 10 + } + }, + policy = "redis", + redis_host = helpers.redis_host, + redis_port = helpers.redis_port, + redis_database = 0, + redis_username = "test", + redis_password = "testXXX", + redis_ssl = true, + redis_ssl_verify = false, + redis_timeout = 1100, + redis_server_name = helpers.redis_ssl_sni, + } + local ok, _, err = v(config, schema_def) + assert.truthy(ok) + assert.is_nil(err) + end) + + it("verifies that redis required fields are supplied", function() + local config = { + limits = { + video = { + second = 10 + } + }, + policy = "redis", + redis = { + port = helpers.redis_port, + database = 0, + username = "test", + password = "testXXX", + ssl = true, + ssl_verify = false, + timeout = 1100, + server_name = helpers.redis_ssl_sni, + } } + local ok, err = v(config, schema_def) + assert.falsy(ok) + assert.contains("must set one of 'config.redis.host', 'config.redis_host' when 'policy' is 'redis'", err["@entity"]) + end) end) end) diff --git a/spec/03-plugins/24-response-rate-limiting/04-access_spec.lua b/spec/03-plugins/24-response-rate-limiting/04-access_spec.lua index a697444a19cf..4fb9ecb5d0f2 100644 --- a/spec/03-plugins/24-response-rate-limiting/04-access_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/04-access_spec.lua @@ -150,13 +150,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS } }, }, }) @@ -171,13 +173,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS*2, minute = ITERATIONS*4 }, image = { second = ITERATIONS } }, }, @@ -197,10 +201,12 @@ for _, strategy in helpers.each_strategy() do route = { id = route3.id }, config = { policy = policy, - redis_host = REDIS_HOST, - redis_port = REDIS_PORT, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = REDIS_PORT, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS - 3 } } }, }) @@ -211,13 +217,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS - 2 } }, }, }) @@ -232,13 +240,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS * 2 + 2 }, image = { second = ITERATIONS } @@ -256,13 +266,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, block_on_first_violation = true, limits = { video = { @@ -286,13 +298,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS, minute = ITERATIONS*2 }, image = { second = ITERATIONS-1 } }, } @@ -309,13 +323,15 @@ for _, strategy in helpers.each_strategy() do fault_tolerant = false, policy = policy, hide_client_headers = true, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS } }, } }) @@ -336,13 +352,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS } }, } }) @@ -363,13 +381,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS } }, } }) @@ -605,12 +625,14 @@ for _, strategy in helpers.each_strategy() do route = { id = route.id }, config = { policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + }, fault_tolerant = false, limits = { video = { second = ITERATIONS } }, } @@ -676,13 +698,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS } }, } }) @@ -717,13 +741,15 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DATABASE, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + database = REDIS_DATABASE, + }, limits = { video = { second = ITERATIONS } }, } }) @@ -786,12 +812,14 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + }, limits = { video = { second = ITERATIONS} }, } } @@ -805,12 +833,14 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = true, policy = policy, - redis_host = REDIS_HOST, - redis_port = redis_conf.redis_port, - redis_ssl = redis_conf.redis_ssl, - redis_ssl_verify = redis_conf.redis_ssl_verify, - redis_server_name = redis_conf.redis_server_name, - redis_password = REDIS_PASSWORD, + redis = { + host = REDIS_HOST, + port = redis_conf.redis_port, + ssl = redis_conf.redis_ssl, + ssl_verify = redis_conf.redis_ssl_verify, + server_name = redis_conf.redis_server_name, + password = REDIS_PASSWORD, + }, limits = { video = {second = ITERATIONS} } } } @@ -891,7 +921,10 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = false, policy = policy, - redis_host = "5.5.5.5", + redis = { + host = "5.5.5.5", + port = REDIS_PORT + }, limits = { video = { second = ITERATIONS } }, } } @@ -906,7 +939,10 @@ for _, strategy in helpers.each_strategy() do config = { fault_tolerant = true, policy = policy, - redis_host = "5.5.5.5", + redis = { + host = "5.5.5.5", + port = REDIS_PORT + }, limits = { video = { second = ITERATIONS } }, } } diff --git a/spec/03-plugins/24-response-rate-limiting/05-integration_spec.lua b/spec/03-plugins/24-response-rate-limiting/05-integration_spec.lua index 3c48b76a3c8d..1da10160c334 100644 --- a/spec/03-plugins/24-response-rate-limiting/05-integration_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/05-integration_spec.lua @@ -1,6 +1,7 @@ local helpers = require "spec.helpers" local redis = require "resty.redis" local version = require "version" +local cjson = require "cjson" local tostring = tostring @@ -122,14 +123,16 @@ describe("Plugin: rate-limiting (integration)", function() route = { id = route1.id }, config = { policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_database = REDIS_DB_1, - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + database = REDIS_DB_1, + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, limits = { video = { minute = 6 } }, }, }) @@ -142,14 +145,16 @@ describe("Plugin: rate-limiting (integration)", function() route = { id = route2.id }, config = { policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_database = REDIS_DB_2, - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + database = REDIS_DB_2, + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, limits = { video = { minute = 6 } }, }, }) @@ -163,16 +168,18 @@ describe("Plugin: rate-limiting (integration)", function() route = { id = route3.id }, config = { policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_username = REDIS_USER_VALID, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DB_3, - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + username = REDIS_USER_VALID, + password = REDIS_PASSWORD, + database = REDIS_DB_3, + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, limits = { video = { minute = 6 } }, }, }) @@ -185,16 +192,18 @@ describe("Plugin: rate-limiting (integration)", function() route = { id = route4.id }, config = { policy = "redis", - redis_host = REDIS_HOST, - redis_port = config.redis_port, - redis_username = REDIS_USER_INVALID, - redis_password = REDIS_PASSWORD, - redis_database = REDIS_DB_4, - redis_ssl = config.redis_ssl, - redis_ssl_verify = config.redis_ssl_verify, - redis_server_name = config.redis_server_name, + redis = { + host = REDIS_HOST, + port = config.redis_port, + username = REDIS_USER_INVALID, + password = REDIS_PASSWORD, + database = REDIS_DB_4, + ssl = config.redis_ssl, + ssl_verify = config.redis_ssl_verify, + server_name = config.redis_server_name, + timeout = 10000, + }, fault_tolerant = false, - redis_timeout = 10000, limits = { video = { minute = 6 } }, }, }) @@ -360,5 +369,130 @@ describe("Plugin: rate-limiting (integration)", function() end end) end) - end + end -- end for each strategy + + describe("creating rate-limiting plugins using api", function () + local route3, admin_client + + lazy_setup(function() + assert(helpers.start_kong({ + nginx_conf = "spec/fixtures/custom_nginx.template" + })) + + route3 = assert(bp.routes:insert { + hosts = { "redistest3.test" }, + }) + + admin_client = helpers.admin_client() + end) + + lazy_teardown(function() + if admin_client then + admin_client:close() + end + + helpers.stop_kong() + end) + + before_each(function() + helpers.clean_logfile() + end) + + local function delete_plugin(admin_client, plugin) + local res = assert(admin_client:send({ + method = "DELETE", + path = "/plugins/" .. plugin.id, + })) + + assert.res_status(204, res) + end + + it("allows to create a plugin with new redis configuration", function() + local res = assert(admin_client:send { + method = "POST", + route = { + id = route3.id + }, + path = "/plugins", + headers = { ["Content-Type"] = "application/json" }, + body = { + name = "response-ratelimiting", + config = { + limits = { + video = { + minute = 100, + } + }, + policy = "redis", + redis = { + host = helpers.redis_host, + port = helpers.redis_port, + username = "test1", + password = "testX", + database = 1, + timeout = 1100, + ssl = true, + ssl_verify = true, + server_name = "example.test", + }, + }, + }, + }) + + local json = cjson.decode(assert.res_status(201, res)) + delete_plugin(admin_client, json) + assert.logfile().has.no.line("response-ratelimiting: config.redis_host is deprecated, please use config.redis.host instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_port is deprecated, please use config.redis.port instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_password is deprecated, please use config.redis.password instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_username is deprecated, please use config.redis.username instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_ssl is deprecated, please use config.redis.ssl instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_server_name is deprecated, please use config.redis.server_name instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_timeout is deprecated, please use config.redis.timeout instead (deprecated after 4.0)", true) + assert.logfile().has.no.line("response-ratelimiting: config.redis_database is deprecated, please use config.redis.database instead (deprecated after 4.0)", true) + end) + + it("allows to create a plugin with legacy redis configuration", function() + local res = assert(admin_client:send { + method = "POST", + route = { + id = route3.id + }, + path = "/plugins", + headers = { ["Content-Type"] = "application/json" }, + body = { + name = "response-ratelimiting", + config = { + limits = { + video = { + minute = 100, + } + }, + policy = "redis", + redis_host = "custom-host.example.test", + redis_port = 55000, + redis_username = "test1", + redis_password = "testX", + redis_database = 1, + redis_timeout = 1100, + redis_ssl = true, + redis_ssl_verify = true, + redis_server_name = "example.test", + }, + }, + }) + + local json = cjson.decode(assert.res_status(201, res)) + delete_plugin(admin_client, json) + assert.logfile().has.line("response-ratelimiting: config.redis_host is deprecated, please use config.redis.host instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_port is deprecated, please use config.redis.port instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_password is deprecated, please use config.redis.password instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_username is deprecated, please use config.redis.username instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_ssl is deprecated, please use config.redis.ssl instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_ssl_verify is deprecated, please use config.redis.ssl_verify instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_server_name is deprecated, please use config.redis.server_name instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_timeout is deprecated, please use config.redis.timeout instead (deprecated after 4.0)", true) + assert.logfile().has.line("response-ratelimiting: config.redis_database is deprecated, please use config.redis.database instead (deprecated after 4.0)", true) + end) + end) end) diff --git a/spec/05-migration/plugins/rate-limiting/migrations/006_350_to_360_spec.lua b/spec/05-migration/plugins/rate-limiting/migrations/006_350_to_360_spec.lua new file mode 100644 index 000000000000..29ab4ff1228c --- /dev/null +++ b/spec/05-migration/plugins/rate-limiting/migrations/006_350_to_360_spec.lua @@ -0,0 +1,72 @@ + +local cjson = require "cjson" +local uh = require "spec.upgrade_helpers" + + +if uh.database_type() == 'postgres' then + describe("rate-limiting plugin migration", function() + lazy_setup(function() + assert(uh.start_kong()) + end) + + lazy_teardown(function () + assert(uh.stop_kong(nil, true)) + end) + + uh.setup(function () + local admin_client = assert(uh.admin_client()) + + local res = assert(admin_client:send { + method = "POST", + path = "/plugins/", + body = { + name = "rate-limiting", + config = { + minute = 200, + redis_host = "localhost", + redis_port = 57198, + redis_username = "test", + redis_password = "secret", + redis_ssl = true, + redis_ssl_verify = true, + redis_server_name = "test.example", + redis_timeout = 1100, + redis_database = 2, + } + }, + headers = { + ["Content-Type"] = "application/json" + } + }) + assert.res_status(201, res) + admin_client:close() + end) + + uh.new_after_up("has updated rate-limiting redis configuration", function () + local admin_client = assert(uh.admin_client()) + local res = assert(admin_client:send { + method = "GET", + path = "/plugins/" + }) + local body = cjson.decode(assert.res_status(200, res)) + assert.equal(1, #body.data) + assert.equal("rate-limiting", body.data[1].name) + local expected_config = { + minute = 200, + redis = { + host = "localhost", + port = 57198, + username = "test", + password = "secret", + ssl = true, + ssl_verify = true, + server_name = "test.example", + timeout = 1100, + database = 2, + } + } + assert.partial_match(expected_config, body.data[1].config) + admin_client:close() + end) + end) +end diff --git a/spec/05-migration/plugins/response-ratelimiting/migrations/001_350_to_360_spec.lua b/spec/05-migration/plugins/response-ratelimiting/migrations/001_350_to_360_spec.lua new file mode 100644 index 000000000000..d574bd9cfc78 --- /dev/null +++ b/spec/05-migration/plugins/response-ratelimiting/migrations/001_350_to_360_spec.lua @@ -0,0 +1,74 @@ + +local cjson = require "cjson" +local uh = require "spec.upgrade_helpers" + + +if uh.database_type() == 'postgres' then + describe("rate-limiting plugin migration", function() + lazy_setup(function() + assert(uh.start_kong()) + end) + + lazy_teardown(function () + assert(uh.stop_kong(nil, true)) + end) + + uh.setup(function () + local admin_client = assert(uh.admin_client()) + + local res = assert(admin_client:send { + method = "POST", + path = "/plugins/", + body = { + name = "response-ratelimiting", + config = { + limits = { + video = { + minute = 200, + } + }, + redis_host = "localhost", + redis_port = 57198, + redis_username = "test", + redis_password = "secret", + redis_timeout = 1100, + redis_database = 2, + } + }, + headers = { + ["Content-Type"] = "application/json" + } + }) + assert.res_status(201, res) + admin_client:close() + end) + + uh.new_after_up("has updated rate-limiting redis configuration", function () + local admin_client = assert(uh.admin_client()) + local res = assert(admin_client:send { + method = "GET", + path = "/plugins/" + }) + local body = cjson.decode(assert.res_status(200, res)) + assert.equal(1, #body.data) + assert.equal("response-ratelimiting", body.data[1].name) + local expected_config = { + limits = { + video = { + minute = 200, + } + }, + redis = { + host = "localhost", + port = 57198, + username = "test", + password = "secret", + timeout = 1100, + database = 2, + } + } + assert.partial_match(expected_config, body.data[1].config) + admin_client:close() + end) + end) +end