diff --git a/kong/plugins/rate-limiting/handler.lua b/kong/plugins/rate-limiting/handler.lua index 8553a7e9c6a..177c1abb0c2 100644 --- a/kong/plugins/rate-limiting/handler.lua +++ b/kong/plugins/rate-limiting/handler.lua @@ -26,8 +26,9 @@ local function increment(api_id, identifier, current_timestamp, value) -- Increment metrics for all periods if the request goes through local _, stmt_err = dao.ratelimiting_metrics:increment(api_id, identifier, current_timestamp, value) if stmt_err then - return responses.send_HTTP_INTERNAL_SERVER_ERROR(stmt_err) + return false, stmt_err end + return true end local function increment_async(premature, api_id, identifier, current_timestamp, value) @@ -46,7 +47,7 @@ local function get_usage(api_id, identifier, current_timestamp, limits) for name, limit in pairs(limits) do local current_metric, err = dao.ratelimiting_metrics:find_one(api_id, identifier, current_timestamp, name) if err then - return responses.send_HTTP_INTERNAL_SERVER_ERROR(err) + return nil, nil, err end -- What is the current usage for the configured limit name? @@ -79,22 +80,33 @@ function RateLimitingHandler:access(conf) local identifier = get_identifier() local api_id = ngx.ctx.api.id local is_async = conf.async + local is_continue_on_error = conf.continue_on_error -- Load current metric for configured period conf.async = nil - local usage, stop = get_usage(api_id, identifier, current_timestamp, conf) - - -- Adding headers - for k, v in pairs(usage) do - ngx.header[constants.HEADERS.RATELIMIT_LIMIT.."-"..k] = v.limit - ngx.header[constants.HEADERS.RATELIMIT_REMAINING.."-"..k] = math.max(0, (stop == nil or stop == k) and v.remaining - 1 or v.remaining) -- -increment_value for this current request + conf.continue_on_error = nil + local usage, stop, err = get_usage(api_id, identifier, current_timestamp, conf) + if err then + if is_continue_on_error then + ngx.log(ngx.ERR, "failed to get usage: ", tostring(err)) + else + return responses.send_HTTP_INTERNAL_SERVER_ERROR(err) + end end - -- If limit is exceeded, terminate the request - if stop then - return responses.send(429, "API rate limit exceeded") - end + if usage then + -- Adding headers + for k, v in pairs(usage) do + ngx.header[constants.HEADERS.RATELIMIT_LIMIT.."-"..k] = v.limit + ngx.header[constants.HEADERS.RATELIMIT_REMAINING.."-"..k] = math.max(0, (stop == nil or stop == k) and v.remaining - 1 or v.remaining) -- -increment_value for this current request + end + -- If limit is exceeded, terminate the request + if stop then + return responses.send(429, "API rate limit exceeded") + end + end + -- Increment metrics for all periods if the request goes through if is_async then local ok, err = ngx.timer.at(0, increment_async, api_id, identifier, current_timestamp, 1) @@ -102,7 +114,14 @@ function RateLimitingHandler:access(conf) ngx.log(ngx.ERR, "failed to create timer: ", err) end else - increment(api_id, identifier, current_timestamp, 1) + local _, err = increment(api_id, identifier, current_timestamp, 1) + if err then + if is_continue_on_error then + ngx.log(ngx.ERR, "failed to increment: ", tostring(err)) + else + return responses.send_HTTP_INTERNAL_SERVER_ERROR(err) + end + end end end diff --git a/kong/plugins/rate-limiting/schema.lua b/kong/plugins/rate-limiting/schema.lua index 3e821e1670d..979173fa45b 100644 --- a/kong/plugins/rate-limiting/schema.lua +++ b/kong/plugins/rate-limiting/schema.lua @@ -9,7 +9,8 @@ return { day = { type = "number" }, month = { type = "number" }, year = { type = "number" }, - async = { type = "boolean", default = false } + async = { type = "boolean", default = false }, + continue_on_error = { type = "boolean", default = false } }, self_check = function(schema, plugin_t, dao, is_update) local ordered_periods = { "second", "minute", "hour", "day", "month", "year"} diff --git a/kong/plugins/response-ratelimiting/access.lua b/kong/plugins/response-ratelimiting/access.lua index 3559f31e524..96635aad5a8 100644 --- a/kong/plugins/response-ratelimiting/access.lua +++ b/kong/plugins/response-ratelimiting/access.lua @@ -24,7 +24,7 @@ local function get_current_usage(api_id, identifier, current_timestamp, limits) for lk, lv in pairs(v) do -- Iterare over periods local current_metric, err = dao.response_ratelimiting_metrics:find_one(api_id, identifier, current_timestamp, lk, k) if err then - return responses.send_HTTP_INTERNAL_SERVER_ERROR(err) + return false, err end local current_usage = current_metric and current_metric.value or 0 @@ -54,7 +54,15 @@ function _M.execute(conf) ngx.ctx.identifier = identifier -- For later use -- Load current metric for configured period - local usage = get_current_usage(api_id, identifier, current_timestamp, conf.limits) + local usage, err = get_current_usage(api_id, identifier, current_timestamp, conf.limits) + if err then + if conf.continue_on_error then + ngx.log(ngx.ERR, "failed to get usage: ", tostring(err)) + return + else + return responses.send_HTTP_INTERNAL_SERVER_ERROR(err) + end + end ngx.ctx.usage = usage -- For later use end diff --git a/kong/plugins/response-ratelimiting/handler.lua b/kong/plugins/response-ratelimiting/handler.lua index 47bd9250b9b..8aee410d4ce 100644 --- a/kong/plugins/response-ratelimiting/handler.lua +++ b/kong/plugins/response-ratelimiting/handler.lua @@ -22,7 +22,7 @@ function ResponseRateLimitingHandler:header_filter(conf) end function ResponseRateLimitingHandler:log(conf) - if not ngx.ctx.stop_log then + if not ngx.ctx.stop_log and ngx.ctx.usage then ResponseRateLimitingHandler.super.log(self) log.execute(ngx.ctx.api.id, ngx.ctx.identifier, ngx.ctx.current_timestamp, ngx.ctx.increments, ngx.ctx.usage) end diff --git a/kong/plugins/response-ratelimiting/header_filter.lua b/kong/plugins/response-ratelimiting/header_filter.lua index 05e0414d67d..87c21139a60 100644 --- a/kong/plugins/response-ratelimiting/header_filter.lua +++ b/kong/plugins/response-ratelimiting/header_filter.lua @@ -32,7 +32,8 @@ function _M.execute(conf) ngx.ctx.increments = increments local usage = ngx.ctx.usage -- Load current usage - + if not usage then return end + local stop for limit_name, v in pairs(usage) do for period_name, lv in pairs(usage[limit_name]) do diff --git a/kong/plugins/response-ratelimiting/schema.lua b/kong/plugins/response-ratelimiting/schema.lua index a5f1e05742c..b90bc1675d2 100644 --- a/kong/plugins/response-ratelimiting/schema.lua +++ b/kong/plugins/response-ratelimiting/schema.lua @@ -37,6 +37,7 @@ end return { fields = { header_name = { type = "string", default = "x-kong-limit" }, + continue_on_error = { type = "boolean", default = false }, limits = { type = "table", schema = { flexible = true, diff --git a/spec/plugins/rate-limiting/access_spec.lua b/spec/plugins/rate-limiting/access_spec.lua index e10d21512da..fd872fbd9ba 100644 --- a/spec/plugins/rate-limiting/access_spec.lua +++ b/spec/plugins/rate-limiting/access_spec.lua @@ -17,7 +17,7 @@ end describe("RateLimiting Plugin", function() - setup(function() + local function prepare_db() spec_helper.prepare_db() spec_helper.insert_fixtures { api = { @@ -25,7 +25,9 @@ describe("RateLimiting Plugin", function() { name = "tests-rate-limiting2", request_host = "test4.com", upstream_url = "http://mockbin.com" }, { name = "tests-rate-limiting3", request_host = "test5.com", upstream_url = "http://mockbin.com" }, { name = "tests-rate-limiting4", request_host = "test6.com", upstream_url = "http://mockbin.com" }, - { name = "tests-rate-limiting5", request_host = "test7.com", upstream_url = "http://mockbin.com" } + { name = "tests-rate-limiting5", request_host = "test7.com", upstream_url = "http://mockbin.com" }, + { name = "tests-rate-limiting6", request_host = "test8.com", upstream_url = "http://mockbin.com" }, + { name = "tests-rate-limiting7", request_host = "test9.com", upstream_url = "http://mockbin.com" } }, consumer = { { custom_id = "provider_123" }, @@ -38,16 +40,20 @@ describe("RateLimiting Plugin", function() { name = "rate-limiting", config = { minute = 6 }, __api = 2 }, { name = "rate-limiting", config = { minute = 3, hour = 5 }, __api = 3 }, { name = "rate-limiting", config = { minute = 33 }, __api = 4 }, - { name = "rate-limiting", config = { minute = 6, async = true }, __api = 5 } + { name = "rate-limiting", config = { minute = 6, async = true }, __api = 5 }, + { name = "rate-limiting", config = { minute = 6, continue_on_error = false }, __api = 6 }, + { name = "rate-limiting", config = { minute = 6, continue_on_error = true }, __api = 7 } }, keyauth_credential = { { key = "apikey122", __consumer = 1 }, { key = "apikey123", __consumer = 2 } } } + end + setup(function() + prepare_db() spec_helper.start_kong() - wait() end) @@ -148,7 +154,6 @@ describe("RateLimiting Plugin", function() end) describe("Async increment", function() - it("should increment asynchronously", function() -- Default rate-limiting plugin for this API says 6/minute local limit = 6 @@ -167,6 +172,63 @@ describe("RateLimiting Plugin", function() assert.are.equal(429, status) assert.are.equal("API rate limit exceeded", body.message) end) + end) + + describe("Continue on error", function() + + local session, err, configuration + + setup(function() + local cassandra = require "cassandra" + local TEST_CONF = spec_helper.get_env().conf_file + local env = spec_helper.get_env(TEST_CONF) + configuration = env.configuration + session, err = cassandra.spawn_session { + shm = "ratelimiting_specs", + keyspace = configuration.dao_config.keyspace, + contact_points = configuration.dao_config.contact_points + } + assert.falsy(err) + end) + after_each(function() + session:execute("DROP KEYSPACE "..configuration.dao_config.keyspace) + prepare_db() + end) + + teardown(function() + session:shutdown() + end) + + it("should not continue if an error occurs", function() + local _, status, headers = http_client.get(STUB_GET_URL, {}, {host = "test8.com"}) + assert.are.equal(200, status) + assert.are.same(tostring(6), headers["x-ratelimit-limit-minute"]) + assert.are.same(tostring(5), headers["x-ratelimit-remaining-minute"]) + + -- Simulate an error on the database + session:execute("DROP TABLE ratelimiting_metrics") + + -- Make another request + local res, status, _ = http_client.get(STUB_GET_URL, {}, {host = "test8.com"}) + assert.equal("An unexpected error occurred", cjson.decode(res).message) + assert.are.equal(500, status) + end) + + it("should continue if an error occurs", function() + local _, status, headers = http_client.get(STUB_GET_URL, {}, {host = "test9.com"}) + assert.are.equal(200, status) + assert.falsy(headers["x-ratelimit-limit-minute"]) + assert.falsy(headers["x-ratelimit-remaining-minute"]) + + -- Simulate an error on the database + session:execute("DROP TABLE ratelimiting_metrics") + + -- Make another request + local _, status, headers = http_client.get(STUB_GET_URL, {}, {host = "test9.com"}) + assert.are.equal(200, status) + assert.falsy(headers["x-ratelimit-limit-minute"]) + assert.falsy(headers["x-ratelimit-remaining-minute"]) + end) end) end) diff --git a/spec/plugins/response-ratelimiting/access_spec.lua b/spec/plugins/response-ratelimiting/access_spec.lua index 2821641014b..82e4ce280ce 100644 --- a/spec/plugins/response-ratelimiting/access_spec.lua +++ b/spec/plugins/response-ratelimiting/access_spec.lua @@ -1,6 +1,7 @@ local spec_helper = require "spec.spec_helpers" local http_client = require "kong.tools.http_client" local timestamp = require "kong.tools.timestamp" +local cjson = require "cjson" local PROXY_URL = spec_helper.PROXY_URL local SLEEP_VALUE = "0.5" @@ -17,13 +18,15 @@ end describe("RateLimiting Plugin", function() - setup(function() + local function prepare_db() spec_helper.prepare_db() spec_helper.insert_fixtures { api = { { name = "tests-response-ratelimiting1", request_host = "test1.com", upstream_url = "http://httpbin.org/" }, { name = "tests-response-ratelimiting2", request_host = "test2.com", upstream_url = "http://httpbin.org/" }, - { name = "tests-response-ratelimiting3", request_host = "test3.com", upstream_url = "http://httpbin.org/" } + { name = "tests-response-ratelimiting3", request_host = "test3.com", upstream_url = "http://httpbin.org/" }, + { name = "tests-response-ratelimiting4", request_host = "test4.com", upstream_url = "http://httpbin.org/" }, + { name = "tests-response-ratelimiting5", request_host = "test5.com", upstream_url = "http://httpbin.org/" } }, consumer = { { custom_id = "consumer_123" }, @@ -35,7 +38,9 @@ describe("RateLimiting Plugin", function() { name = "response-ratelimiting", config = { limits = { video = { minute = 6, hour = 10 }, image = { minute = 4 } } }, __api = 2 }, { name = "key-auth", config = {key_names = {"apikey"}, hide_credentials = true}, __api = 3 }, { name = "response-ratelimiting", config = { limits = { video = { minute = 6 } } }, __api = 3 }, - { name = "response-ratelimiting", config = { limits = { video = { minute = 2 } } }, __api = 3, __consumer = 1 } + { name = "response-ratelimiting", config = { limits = { video = { minute = 2 } } }, __api = 3, __consumer = 1 }, + { name = "response-ratelimiting", config = { continue_on_error = false, limits = { video = { minute = 6 } } }, __api = 4 }, + { name = "response-ratelimiting", config = { continue_on_error = true, limits = { video = { minute = 6 } } }, __api = 5 } }, keyauth_credential = { { key = "apikey123", __consumer = 1 }, @@ -43,9 +48,11 @@ describe("RateLimiting Plugin", function() { key = "apikey125", __consumer = 3 } } } + end + setup(function() + prepare_db() spec_helper.start_kong() - wait() end) @@ -54,7 +61,6 @@ describe("RateLimiting Plugin", function() end) describe("Without authentication (IP address)", function() - it("should get blocked if exceeding limit", function() -- Default ratelimiting plugin for this API says 6/minute local limit = 6 @@ -73,7 +79,6 @@ describe("RateLimiting Plugin", function() end) - it("should handle multiple limits", function() for i = 1, 3 do local _, status, headers = http_client.get(PROXY_URL.."/response-headers", {["x-kong-limit"] = "video=2, image = 1"}, {host = "test2.com"}) @@ -95,13 +100,10 @@ describe("RateLimiting Plugin", function() assert.are.equal("4", headers["x-ratelimit-remaining-video-hour"]) assert.are.equal("1", headers["x-ratelimit-remaining-image-minute"]) end) - end) describe("With authentication", function() - describe("Default plugin", function() - it("should get blocked if exceeding limit and a per consumer setting", function() -- Default ratelimiting plugin for this API says 6/minute local limit = 2 @@ -158,7 +160,64 @@ describe("RateLimiting Plugin", function() assert.are.equal("0", headers["x-ratelimit-remaining-video-minute"]) assert.are.equal("6", headers["x-ratelimit-limit-video-minute"]) end) + end) + end) + + describe("Continue on error", function() + + local session, err, configuration + + setup(function() + local cassandra = require "cassandra" + local TEST_CONF = spec_helper.get_env().conf_file + local env = spec_helper.get_env(TEST_CONF) + configuration = env.configuration + session, err = cassandra.spawn_session { + shm = "response_ratelimiting_specs", + keyspace = configuration.dao_config.keyspace, + contact_points = configuration.dao_config.contact_points + } + assert.falsy(err) + end) + + after_each(function() + session:execute("DROP KEYSPACE "..configuration.dao_config.keyspace) + prepare_db() + end) + + teardown(function() + session:shutdown() + end) + + it("should not continue if an error occurs", function() + local _, status, headers = http_client.get(PROXY_URL.."/response-headers", {["x-kong-limit"] = "video=1"}, {host = "test4.com"}) + assert.are.equal(200, status) + assert.are.same('6', headers["x-ratelimit-limit-video-minute"]) + assert.are.same('5', headers["x-ratelimit-remaining-video-minute"]) + + -- Simulate an error on the database + session:execute("DROP TABLE response_ratelimiting_metrics") + + -- Make another request + local res, status, _ = http_client.get(PROXY_URL.."/response-headers", {["x-kong-limit"] = "video=1"}, {host = "test4.com"}) + assert.equal("An unexpected error occurred", cjson.decode(res).message) + assert.are.equal(500, status) + end) + it("should continue if an error occurs", function() + local _, status, headers = http_client.get(PROXY_URL.."/response-headers", {["x-kong-limit"] = "video=1"}, {host = "test5.com"}) + assert.are.equal(200, status) + assert.falsy(headers["x-ratelimit-limit-video-minute"]) + assert.falsy(headers["x-ratelimit-remaining-video-minute"]) + + -- Simulate an error on the database + session:execute("DROP TABLE response_ratelimiting_metrics") + + -- Make another request + local _, status, headers = http_client.get(PROXY_URL.."/response-headers", {["x-kong-limit"] = "video=1"}, {host = "test5.com"}) + assert.are.equal(200, status) + assert.falsy(headers["x-ratelimit-limit-video-minute"]) + assert.falsy(headers["x-ratelimit-remaining-video-minute"]) end) end)