diff --git a/CHANGELOG.md b/CHANGELOG.md index 12aa8a7a8..31a8b76ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Decoded JWTs are now exposed in the policies context by the APIcast policy [PR #718](https://github.com/3scale/apicast/pull/718) - Upgraded OpenResty to 1.13.6.2, uses OpenSSL 1.1 [PR #733](https://github.com/3scale/apicast/pull/733) - Use forked `resty.limit.count` that uses increments instead of decrements [PR #758](https://github.com/3scale/apicast/pull/758) +- Rate Limit policy to take into account changes in the config [PR #703](https://github.com/3scale/apicast/pull/703) - The regular expression for mapping rules has been changed, so that special characters are accepted in the wildcard values for path [PR #717](https://github.com/3scale/apicast/pull/714) ### Fixed diff --git a/gateway/src/apicast/policy/rate_limit/rate_limit.lua b/gateway/src/apicast/policy/rate_limit/rate_limit.lua index 57f17f85e..eb75e497c 100644 --- a/gateway/src/apicast/policy/rate_limit/rate_limit.lua +++ b/gateway/src/apicast/policy/rate_limit/rate_limit.lua @@ -86,6 +86,7 @@ local function build_limiters_and_keys(type, limiters, redis, error_settings, co for _, limiter in ipairs(limiters) do local lim, initerr = traffic_limiters[type](limiter) + if not lim then ngx.log(ngx.ERR, "unknown limiter: ", type, ", err: ", initerr) error(error_settings, "configuration_issue") @@ -103,6 +104,11 @@ local function build_limiters_and_keys(type, limiters, redis, error_settings, co else key = format("%s_%s_%s", context.service.id, type, key) end + if type == "fixed_window" then + local time = ngx.time() + local window = limiter.window + key = format("%d_%s", time - (time % window), key) + end insert(res_keys, key) end diff --git a/spec/policy/rate_limit/rate_limit_spec.lua b/spec/policy/rate_limit/rate_limit_spec.lua index 7bbd7a781..08cb19d8e 100644 --- a/spec/policy/rate_limit/rate_limit_spec.lua +++ b/spec/policy/rate_limit/rate_limit_spec.lua @@ -43,6 +43,7 @@ describe('Rate limit policy', function() setup(function() ngx_exit_spy = spy.on(ngx, 'exit') ngx_sleep_spy = spy.on(ngx, 'sleep') + stub(ngx, 'time', function() return 11111 end) end) before_each(function() @@ -168,15 +169,17 @@ describe('Rate limit policy', function() rate_limit_policy:access(context) rate_limit_policy:access(context) + local redis_key = redis:keys('11110_fixed_window_test3')[1] + assert.equal('2', redis:get(redis_key)) assert.spy(ngx_exit_spy).was_called_with(429) - assert.equal('2', redis:get('fixed_window_test3')) end) it('rejected (count), name_type is liquid', function() local ctx = { service = { id = 5 }, var_in_context = 'test3' } local rate_limit_policy = RateLimitPolicy.new({ fixed_window_limiters = { - { key = { name = '{{ var_in_context }}', name_type = 'liquid', scope = 'global' }, count = 1, window = 10 } + { key = { name = '{{ var_in_context }}', name_type = 'liquid', scope = 'global' }, + count = 1, window = 10 } }, redis_url = redis_url }) @@ -184,7 +187,8 @@ describe('Rate limit policy', function() rate_limit_policy:access(ctx) rate_limit_policy:access(ctx) - assert.equal('2', redis:get('fixed_window_test3')) + local redis_key = redis:keys('11110_fixed_window_test3')[1] + assert.equal('2', redis:get(redis_key)) assert.spy(ngx_exit_spy).was_called_with(429) end) @@ -199,7 +203,8 @@ describe('Rate limit policy', function() rate_limit_policy:access(context) rate_limit_policy:access(context) - assert.equal('2', redis:get('fixed_window_test3')) + local redis_key = redis:keys('11110_fixed_window_test3')[1] + assert.equal('2', redis:get(redis_key)) assert.spy(ngx_exit_spy).was_called_with(429) end) end) diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t index bcb250cc3..044c378d3 100644 --- a/t/apicast-policy-rate-limit.t +++ b/t/apicast-policy-rate-limit.t @@ -83,7 +83,7 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("42_connections_test1") + redis:del('42_connections_test1') } } @@ -159,7 +159,7 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("42_connections_test2") + redis:del('42_connections_test2') } } @@ -277,7 +277,7 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("connections_test4") + redis:del('connections_test4') } } @@ -394,7 +394,8 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("leaky_bucket_test6_1", "connections_test6_2", "fixed_window_test6_3") + local redis_key = redis:keys('*_fixed_window_test6_3')[1] + redis:del('leaky_bucket_test6_1', 'connections_test6_2', redis_key) } } @@ -469,7 +470,7 @@ Return 429 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("connections_test7") + redis:del('connections_test7') } } @@ -529,7 +530,7 @@ Return 503 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("leaky_bucket_test8") + redis:del('leaky_bucket_test8') } } @@ -589,7 +590,8 @@ Return 429 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("fixed_window_test9") + local redis_key = redis:keys('*_fixed_window_test9')[1] + redis:del(redis_key) } } @@ -667,7 +669,7 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("connections_test10") + redis:del('connections_test10') } } @@ -728,7 +730,7 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("leaky_bucket_test11") + redis:del('leaky_bucket_test11') } } @@ -1065,7 +1067,9 @@ so only the third call returns 429. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("fixed_window_test17_1", "fixed_window_test17_2") + local redis_key1 = redis:keys('*_fixed_window_test17_1')[1] + local redis_key2 = redis:keys('*_fixed_window_test17_2')[1] + redis:del(redis_key1, redis_key2) } } @@ -1153,7 +1157,9 @@ so only the third call returns 429. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("fixed_window_localhost/test18_1", "fixed_window_localhost/test18_2") + local redis_key1 = redis:keys('*_fixed_window_localhost/test18_1')[1] + local redis_key2 = redis:keys('*_fixed_window_localhost/test18_2')[1] + redis:del(redis_key1, redis_key2) } }