Merge pull request #703 from Hitachi/improve-rate-limit-policy-retry

Rate Limit policy to take into account changes in the config

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

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

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,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)
 
@@ -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)

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)
     }
   }