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.

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

Rate Limit policy to take into account changes in the config #703

Merged
merged 5 commits into from
Jun 15, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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)

### Fixed

Expand Down
6 changes: 6 additions & 0 deletions gateway/src/apicast/policy/rate_limit/rate_limit.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down
13 changes: 9 additions & 4 deletions spec/policy/rate_limit/rate_limit_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -168,23 +169,26 @@ 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
})

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)

Expand All @@ -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)
Expand Down
28 changes: 17 additions & 11 deletions t/apicast-policy-rate-limit.t
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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')
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down