From 93600cdccd2a1af515850ae747e0aa766f7fc251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Nowak?= Date: Fri, 23 Feb 2024 17:06:11 +0100 Subject: [PATCH] chore(tests): remove redis>=v6 checks in tests There were a lot of checks for redis version being at least 6.0.0 It's been 4 years since redis 6.0.0 release and we don't test against lower versions anymore so these checks are no longer needed. KAG-2130 --- .../23-rate-limiting/05-integration_spec.lua | 242 ++++++++--------- .../05-integration_spec.lua | 244 ++++++++---------- 2 files changed, 218 insertions(+), 268 deletions(-) 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 1ec13be79001..7f0239aa4999 100644 --- a/spec/03-plugins/23-rate-limiting/05-integration_spec.lua +++ b/spec/03-plugins/23-rate-limiting/05-integration_spec.lua @@ -1,5 +1,4 @@ local helpers = require "spec.helpers" -local version = require "version" local cjson = require "cjson" local redis_helper = require "spec.helpers.redis_helper" @@ -23,7 +22,6 @@ describe("Plugin: rate-limiting (integration)", function() local client local bp local red - local red_version lazy_setup(function() bp = helpers.get_db_utils(nil, { @@ -33,7 +31,7 @@ describe("Plugin: rate-limiting (integration)", function() }, { "rate-limiting" }) - red, red_version = redis_helper.connect(REDIS_HOST, REDIS_PORT) + red = redis_helper.connect(REDIS_HOST, REDIS_PORT) end) lazy_teardown(function() @@ -77,10 +75,8 @@ describe("Plugin: rate-limiting (integration)", function() lazy_setup(function() red:flushall() - if red_version >= version("6.0.0") then - redis_helper.add_admin_user(red, REDIS_USER_VALID, REDIS_PASSWORD) - redis_helper.add_basic_user(red, REDIS_USER_INVALID, REDIS_PASSWORD) - end + redis_helper.add_admin_user(red, REDIS_USER_VALID, REDIS_PASSWORD) + redis_helper.add_basic_user(red, REDIS_USER_INVALID, REDIS_PASSWORD) bp = helpers.get_db_utils(nil, { "routes", @@ -135,56 +131,53 @@ describe("Plugin: rate-limiting (integration)", function() }, }) - if red_version >= version("6.0.0") then - local route3 = assert(bp.routes:insert { - hosts = { "redistest3.test" }, - }) - assert(bp.plugins:insert { - name = "rate-limiting", - route = { id = route3.id }, - config = { - minute = 2, -- Handle multiple tests - policy = "redis", - 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, - }, - }) - - local route4 = assert(bp.routes:insert { - hosts = { "redistest4.test" }, - }) - assert(bp.plugins:insert { - name = "rate-limiting", - route = { id = route4.id }, - config = { - minute = 1, - policy = "redis", - 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, + local route3 = assert(bp.routes:insert { + hosts = { "redistest3.test" }, + }) + assert(bp.plugins:insert { + name = "rate-limiting", + route = { id = route3.id }, + config = { + minute = 2, -- Handle multiple tests + policy = "redis", + 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, }, - }) - end + fault_tolerant = false, + }, + }) + local route4 = assert(bp.routes:insert { + hosts = { "redistest4.test" }, + }) + assert(bp.plugins:insert { + name = "rate-limiting", + route = { id = route4.id }, + config = { + minute = 1, + policy = "redis", + 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, + }, + }) assert(helpers.start_kong({ nginx_conf = "spec/fixtures/custom_nginx.template", @@ -195,10 +188,8 @@ describe("Plugin: rate-limiting (integration)", function() lazy_teardown(function() helpers.stop_kong() - if red_version >= version("6.0.0") then - redis_helper.remove_user(red, REDIS_USER_VALID) - redis_helper.remove_user(red, REDIS_USER_INVALID) - end + redis_helper.remove_user(red, REDIS_USER_VALID) + redis_helper.remove_user(red, REDIS_USER_INVALID) end) it("connection pool respects database setting", function() @@ -210,11 +201,10 @@ describe("Plugin: rate-limiting (integration)", function() assert.equal(0, tonumber(size_1)) assert.equal(0, tonumber(size_2)) - if red_version >= version("6.0.0") then - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - assert.equal(0, tonumber(size_3)) - end + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + assert.equal(0, tonumber(size_3)) local res = assert(client:send { method = "GET", @@ -239,11 +229,10 @@ describe("Plugin: rate-limiting (integration)", function() assert.equal(1, tonumber(size_1)) assert.equal(0, tonumber(size_2)) - if red_version >= version("6.0.0") then - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - assert.equal(0, tonumber(size_3)) - end + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + assert.equal(0, tonumber(size_3)) -- rate-limiting plugin will reuses the redis connection local res = assert(client:send { @@ -269,76 +258,63 @@ describe("Plugin: rate-limiting (integration)", function() assert.equal(1, tonumber(size_1)) assert.equal(1, tonumber(size_2)) - if red_version >= version("6.0.0") then - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - assert.equal(0, tonumber(size_3)) - end - - if red_version >= version("6.0.0") then - -- rate-limiting plugin will reuses the redis connection - local res = assert(client:send { - method = "GET", - path = "/status/200", - headers = { - ["Host"] = "redistest3.test" - } - }) - assert.res_status(200, res) - - -- Wait for async timer to increment the limit - - ngx.sleep(SLEEP_TIME) - - assert(red:select(REDIS_DB_1)) - size_1 = assert(red:dbsize()) - - assert(red:select(REDIS_DB_2)) - size_2 = assert(red:dbsize()) - - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - - -- TEST: All DBs should now have one hit, because the - -- plugin correctly chose to select the database it is - -- configured to hit - - assert.is_true(tonumber(size_1) > 0) - assert.is_true(tonumber(size_2) > 0) - assert.is_true(tonumber(size_3) > 0) - end + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + assert.equal(0, tonumber(size_3)) + + -- rate-limiting plugin will reuses the redis connection + local res = assert(client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "redistest3.test" + } + }) + assert.res_status(200, res) + + -- Wait for async timer to increment the limit + + ngx.sleep(SLEEP_TIME) + + assert(red:select(REDIS_DB_1)) + size_1 = assert(red:dbsize()) + + assert(red:select(REDIS_DB_2)) + size_2 = assert(red:dbsize()) + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + + -- TEST: All DBs should now have one hit, because the + -- plugin correctly chose to select the database it is + -- configured to hit + + assert.is_true(tonumber(size_1) > 0) + assert.is_true(tonumber(size_2) > 0) + assert.is_true(tonumber(size_3) > 0) end) it("authenticates and executes with a valid redis user having proper ACLs", function() - if red_version >= version("6.0.0") then - local res = assert(client:send { - method = "GET", - path = "/status/200", - headers = { - ["Host"] = "redistest3.test" - } - }) - assert.res_status(200, res) - else - ngx.log(ngx.WARN, "Redis v" .. tostring(red_version) .. " does not support ACL functions " .. - "'authenticates and executes with a valid redis user having proper ACLs' will be skipped") - end + local res = assert(client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "redistest3.test" + } + }) + assert.res_status(200, res) end) it("fails to rate-limit for a redis user with missing ACLs", function() - if red_version >= version("6.0.0") then - local res = assert(client:send { - method = "GET", - path = "/status/200", - headers = { - ["Host"] = "redistest4.test" - } - }) - assert.res_status(500, res) - else - ngx.log(ngx.WARN, "Redis v" .. tostring(red_version) .. " does not support ACL functions " .. - "'fails to rate-limit for a redis user with missing ACLs' will be skipped") - end + local res = assert(client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "redistest4.test" + } + }) + assert.res_status(500, res) end) end) end 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 d4e3cef0d0ba..2e17d1f196fb 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,11 +1,7 @@ local helpers = require "spec.helpers" -local version = require "version" local cjson = require "cjson" local redis_helper = require "spec.helpers.redis_helper" -local tostring = tostring - - local REDIS_HOST = helpers.redis_host local REDIS_PORT = helpers.redis_port local REDIS_SSL_PORT = helpers.redis_ssl_port @@ -27,7 +23,6 @@ describe("Plugin: rate-limiting (integration)", function() local client local bp local red - local red_version lazy_setup(function() -- only to run migrations @@ -38,7 +33,7 @@ describe("Plugin: rate-limiting (integration)", function() }, { "response-ratelimiting", }) - red, red_version = redis_helper.connect(REDIS_HOST, REDIS_PORT) + red = redis_helper.connect(REDIS_HOST, REDIS_PORT) end) lazy_teardown(function() @@ -80,10 +75,8 @@ describe("Plugin: rate-limiting (integration)", function() lazy_setup(function() red:flushall() - if red_version >= version("6.0.0") then - redis_helper.add_admin_user(red, REDIS_USER_VALID, REDIS_PASSWORD) - redis_helper.add_basic_user(red, REDIS_USER_INVALID, REDIS_PASSWORD) - end + redis_helper.add_admin_user(red, REDIS_USER_VALID, REDIS_PASSWORD) + redis_helper.add_basic_user(red, REDIS_USER_INVALID, REDIS_PASSWORD) bp = helpers.get_db_utils(nil, { "routes", @@ -137,55 +130,53 @@ describe("Plugin: rate-limiting (integration)", function() }, }) - if red_version >= version("6.0.0") then - local route3 = assert(bp.routes:insert { - hosts = { "redistest3.test" }, - }) - assert(bp.plugins:insert { - name = "response-ratelimiting", - route = { id = route3.id }, - config = { - policy = "redis", - 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, - limits = { video = { minute = 6 } }, + local route3 = assert(bp.routes:insert { + hosts = { "redistest3.test" }, + }) + assert(bp.plugins:insert { + name = "response-ratelimiting", + route = { id = route3.id }, + config = { + policy = "redis", + 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, }, - }) - - local route4 = assert(bp.routes:insert { - hosts = { "redistest4.test" }, - }) - assert(bp.plugins:insert { - name = "response-ratelimiting", - route = { id = route4.id }, - config = { - policy = "redis", - 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, - limits = { video = { minute = 6 } }, + fault_tolerant = false, + limits = { video = { minute = 6 } }, + }, + }) + + local route4 = assert(bp.routes:insert { + hosts = { "redistest4.test" }, + }) + assert(bp.plugins:insert { + name = "response-ratelimiting", + route = { id = route4.id }, + config = { + policy = "redis", + 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, }, - }) - end + fault_tolerant = false, + limits = { video = { minute = 6 } }, + }, + }) assert(helpers.start_kong({ nginx_conf = "spec/fixtures/custom_nginx.template", @@ -196,10 +187,8 @@ describe("Plugin: rate-limiting (integration)", function() lazy_teardown(function() helpers.stop_kong() - if red_version >= version("6.0.0") then - redis_helper.remove_user(red, REDIS_USER_VALID) - redis_helper.remove_user(red, REDIS_USER_INVALID) - end + redis_helper.remove_user(red, REDIS_USER_VALID) + redis_helper.remove_user(red, REDIS_USER_INVALID) end) it("connection pool respects database setting", function() @@ -211,11 +200,10 @@ describe("Plugin: rate-limiting (integration)", function() assert.equal(0, tonumber(size_1)) assert.equal(0, tonumber(size_2)) - if red_version >= version("6.0.0") then - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - assert.equal(0, tonumber(size_3)) - end + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + assert.equal(0, tonumber(size_3)) local res = assert(client:send { method = "GET", @@ -242,11 +230,10 @@ describe("Plugin: rate-limiting (integration)", function() assert.is_true(tonumber(size_1) > 0) assert.equal(0, tonumber(size_2)) - if red_version >= version("6.0.0") then - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - assert.equal(0, tonumber(size_3)) - end + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + assert.equal(0, tonumber(size_3)) -- response-ratelimiting plugin reuses the redis connection local res = assert(client:send { @@ -274,78 +261,65 @@ describe("Plugin: rate-limiting (integration)", function() assert.is_true(tonumber(size_1) > 0) assert.is_true(tonumber(size_2) > 0) - if red_version >= version("6.0.0") then - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - assert.equal(0, tonumber(size_3)) - end + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + assert.equal(0, tonumber(size_3)) -- response-ratelimiting plugin reuses the redis connection - if red_version >= version("6.0.0") then - local res = assert(client:send { - method = "GET", - path = "/response-headers?x-kong-limit=video=1", - headers = { - ["Host"] = "redistest3.test" - } - }) - assert.res_status(200, res) - assert.equal(6, tonumber(res.headers["x-ratelimit-limit-video-minute"])) - assert.equal(5, tonumber(res.headers["x-ratelimit-remaining-video-minute"])) - - -- Wait for async timer to increment the limit - - ngx.sleep(SLEEP_TIME) - - assert(red:select(REDIS_DB_1)) - size_1 = assert(red:dbsize()) - - assert(red:select(REDIS_DB_2)) - size_2 = assert(red:dbsize()) - - assert(red:select(REDIS_DB_3)) - local size_3 = assert(red:dbsize()) - - -- TEST: All DBs should now have one hit, because the - -- plugin correctly chose to select the database it is - -- configured to hit - - assert.is_true(tonumber(size_1) > 0) - assert.is_true(tonumber(size_2) > 0) - assert.is_true(tonumber(size_3) > 0) - end + local res = assert(client:send { + method = "GET", + path = "/response-headers?x-kong-limit=video=1", + headers = { + ["Host"] = "redistest3.test" + } + }) + assert.res_status(200, res) + assert.equal(6, tonumber(res.headers["x-ratelimit-limit-video-minute"])) + assert.equal(5, tonumber(res.headers["x-ratelimit-remaining-video-minute"])) + + -- Wait for async timer to increment the limit + + ngx.sleep(SLEEP_TIME) + + assert(red:select(REDIS_DB_1)) + size_1 = assert(red:dbsize()) + + assert(red:select(REDIS_DB_2)) + size_2 = assert(red:dbsize()) + + assert(red:select(REDIS_DB_3)) + local size_3 = assert(red:dbsize()) + + -- TEST: All DBs should now have one hit, because the + -- plugin correctly chose to select the database it is + -- configured to hit + + assert.is_true(tonumber(size_1) > 0) + assert.is_true(tonumber(size_2) > 0) + assert.is_true(tonumber(size_3) > 0) end) it("authenticates and executes with a valid redis user having proper ACLs", function() - if red_version >= version("6.0.0") then - local res = assert(client:send { - method = "GET", - path = "/status/200", - headers = { - ["Host"] = "redistest3.test" - } - }) - assert.res_status(200, res) - else - ngx.log(ngx.WARN, "Redis v" .. tostring(red_version) .. " does not support ACL functions " .. - "'authenticates and executes with a valid redis user having proper ACLs' will be skipped") - end + local res = assert(client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "redistest3.test" + } + }) + assert.res_status(200, res) end) it("fails to rate-limit for a redis user with missing ACLs", function() - if red_version >= version("6.0.0") then - local res = assert(client:send { - method = "GET", - path = "/status/200", - headers = { - ["Host"] = "redistest4.test" - } - }) - assert.res_status(500, res) - else - ngx.log(ngx.WARN, "Redis v" .. tostring(red_version) .. " does not support ACL functions " .. - "'fails to response rate-limit for a redis user with missing ACLs' will be skipped") - end + local res = assert(client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "redistest4.test" + } + }) + assert.res_status(500, res) end) end) end -- end for each strategy