Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pr/1794 2 #1941

Merged
merged 7 commits into from
Jan 5, 2017
Merged

Pr/1794 2 #1941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
762b579
task(rate-limiting) Allow connecting to non-zero Redis databases (#17…
logronoide Nov 5, 2016
c6c12ab
Merge remote-tracking branch 'upstream/next' into task-1726-2
logronoide Nov 16, 2016
ff0c8b3
Added support for non-zero redis databases to response-ratelimiting p…
logronoide Nov 16, 2016
97f727a
Fixing conflicts
subnetmarco Jan 4, 2017
a8649c8
tests
subnetmarco Jan 4, 2017
752d347
wip
subnetmarco Jan 4, 2017
cd09ed9
wip
subnetmarco Jan 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions kong/plugins/rate-limiting/policies/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ return {
end
end

if conf.redis_database ~= nil and conf.redis_database > 0 then
local ok, err = red:select(conf.redis_database)
if not ok then
ngx_log(ngx.ERR, "failed to change Redis database: ", err)
return nil, err
end
end

local periods = timestamp.get_timestamps(current_timestamp)
for period, period_date in pairs(periods) do
local cache_key = get_local_key(api_id, identifier, period_date, period)
Expand Down
3 changes: 2 additions & 1 deletion kong/plugins/rate-limiting/schema.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ return {
redis_host = { type = "string" },
redis_port = { type = "number", default = 6379 },
redis_password = { type = "string" },
redis_timeout = { type = "number", default = 2000 }
redis_timeout = { type = "number", default = 2000 },
redis_database = { type = "number", default = 0 }
},
self_check = function(schema, plugin_t, dao, is_update)
local ordered_periods = { "second", "minute", "hour", "day", "month", "year"}
Expand Down
8 changes: 8 additions & 0 deletions kong/plugins/response-ratelimiting/policies/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ return {
end
end

if conf.redis_database ~= nil and conf.redis_database > 0 then
local ok, err = red:select(conf.redis_database)
if not ok then
ngx_log(ngx.ERR, "failed to change Redis database: ", err)
return nil, err
end
end

local periods = timestamp.get_timestamps(current_timestamp)
for period, period_date in pairs(periods) do
local cache_key = get_local_key(api_id, identifier, period_date, name, period)
Expand Down
1 change: 1 addition & 0 deletions kong/plugins/response-ratelimiting/schema.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ return {
redis_port = { type = "number", default = 6379 },
redis_password = { type = "string" },
redis_timeout = { type = "number", default = 2000 },
redis_database = { type = "number", default = 0 },
block_on_first_violation = { type = "boolean", default = false},
limits = { type = "table",
schema = {
Expand Down
54 changes: 33 additions & 21 deletions spec/03-plugins/98-rate-limiting/04-access_spec.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ local timestamp = require "kong.tools.timestamp"
local REDIS_HOST = "127.0.0.1"
local REDIS_PORT = 6379
local REDIS_PASSWORD = ""
local REDIS_DATABASE = 0

local SLEEP_TIME = 1

Expand Down Expand Up @@ -35,6 +36,11 @@ local function flush_redis()
end
end

local ok, err = red:select(REDIS_DATABASE)
if not ok then
error("failed to change Redis database: ", err)
end

red:flushall()
red:close()
end
Expand Down Expand Up @@ -81,7 +87,8 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
fault_tolerant = false,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE
}
})

Expand All @@ -99,7 +106,8 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
policy = policy,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE
}
})

Expand All @@ -121,7 +129,8 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
policy = policy,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE
}
})
assert(helpers.dao.plugins:insert {
Expand All @@ -134,7 +143,8 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
policy = policy,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE
}
})

Expand All @@ -156,7 +166,8 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
policy = policy,
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD
redis_password = REDIS_PASSWORD,
redis_database = REDIS_DATABASE
}
})
end)
Expand All @@ -178,7 +189,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
describe("Without authentication (IP address)", function()
it("blocks if exceeding limit", function()
for i = 1, 6 do
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
Expand All @@ -194,7 +205,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
end

-- Additonal request, while limit is 6/minute
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
Expand All @@ -212,7 +223,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
}

for i = 1, 3 do
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
Expand All @@ -229,7 +240,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.are.same(limits.hour - i, tonumber(res.headers["x-ratelimit-remaining-hour"]))
end

local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
Expand All @@ -246,7 +257,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
describe("API-specific plugin", function()
it("blocks if exceeding limit", function()
for i = 1, 6 do
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey123",
headers = {
Expand All @@ -262,7 +273,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
end

-- Third query, while limit is 2/minute
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey123",
headers = {
Expand All @@ -273,7 +284,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.are.equal([[{"message":"API rate limit exceeded"}]], body)

-- Using a different key of the same consumer works
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey333",
headers = {
Expand All @@ -286,7 +297,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
describe("Plugin customized for specific consumer", function()
it("blocks if exceeding limit", function()
for i = 1, 8 do
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey122",
headers = {
Expand All @@ -301,7 +312,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.are.same(8 - i, tonumber(res.headers["x-ratelimit-remaining-minute"]))
end

local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey122",
headers = {
Expand All @@ -313,7 +324,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
end)
it("blocks if the only rate-limiting plugin existing is per consumer and not per API", function()
for i = 1, 6 do
local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey122",
headers = {
Expand All @@ -328,7 +339,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.are.same(6 - i, tonumber(res.headers["x-ratelimit-remaining-minute"]))
end

local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/?apikey=apikey122",
headers = {
Expand Down Expand Up @@ -507,15 +518,16 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
redis_host = REDIS_HOST,
redis_port = REDIS_PORT,
redis_password = REDIS_PASSWORD,
fault_tolerant = false
fault_tolerant = false,
redis_database = REDIS_DATABASE
}
})
end)

it("expires a counter", function()
local periods = timestamp.get_timestamps()

local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
Expand All @@ -530,7 +542,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.are.same(5, tonumber(res.headers["x-ratelimit-remaining-minute"]))

if policy == "local" then
local res = assert(admin_client:send {
local res = assert(helpers.admin_client():send {
method = "GET",
path = "/cache/"..string.format("ratelimit:%s:%s:%s:%s", api.id, "127.0.0.1", periods.minute, "minute"),
query = { cache = "shm" },
Expand All @@ -541,7 +553,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do

ngx.sleep(61) -- Wait for counter to expire

local res = assert(client:send {
local res = assert(helpers.proxy_client():send {
method = "GET",
path = "/status/200/",
headers = {
Expand All @@ -556,7 +568,7 @@ for i, policy in ipairs({"local", "cluster", "redis"}) do
assert.are.same(5, tonumber(res.headers["x-ratelimit-remaining-minute"]))

if policy == "local" then
local res = assert(admin_client:send {
local res = assert(helpers.admin_client():send {
method = "GET",
path = "/cache/"..string.format("ratelimit:%s:%s:%s:%s", api.id, "127.0.0.1", periods.minute, "minute"),
query = { cache = "shm" },
Expand Down
Loading