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

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

kong-3.6.0-0.rockspec

@@ -372,12 +372,14 @@ 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/clustering/compat/checkers.lua

@@ -27,7 +27,8 @@ 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,
       }
 
       for _, plugin in ipairs(config_table.plugins or {}) do

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
+}

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$$;
+      ]],
+    },
+}

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",
 }

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,28 +133,28 @@ 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)
         return nil, db_key, err
       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
 

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
+    } }
   },
 }

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 },

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,