Merge pull request #900 from 3scale/3.3-stable

Merge 3.3-stable into master

CHANGELOG.md

@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+## [3.3.0-cr1] - 2018-09-14
+
 ### Fixed
 
 - Set default errlog level when `APICAST_LOG_LEVEL` is empty [PR #868](https://github.com/3scale/apicast/pull/868)
@@ -69,7 +71,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - The `scope` of the Rate Limit policy is `service` by default [PR #704](https://github.com/3scale/apicast/pull/704)
 - 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)
+- Use forked `resty.limit.count` that uses increments instead of decrements [PR #758](https://github.com/3scale/apicast/pull/758), [PR 843](https://github.com/3scale/apicast/pull/843)
 - 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 #714](https://github.com/3scale/apicast/pull/714)
 - Call `init` and `init_worker` on all available policies regardless they are used or not [PR #770](https://github.com/3scale/apicast/pull/770)
@@ -492,7 +494,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 - Major rewrite using JSON configuration instead of code generation.
 
-[Unreleased]: https://github.com/3scale/apicast/compare/v3.3.0-beta2...HEAD
+[Unreleased]: https://github.com/3scale/apicast/compare/v3.3.0-cr1...HEAD
 [2.0.0]: https://github.com/3scale/apicast/compare/v0.2...v2.0.0
 [3.0.0-alpha1]: https://github.com/3scale/apicast/compare/v2.0.0...v3.0.0-alpha1
 [3.0.0-alpha2]: https://github.com/3scale/apicast/compare/v3.0.0-alpha1...v3.0.0-alpha2
@@ -517,3 +519,4 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 [3.2.1]: https://github.com/3scale/apicast/compare/v3.2.0...v3.2.1
 [3.3.0-beta1]: https://github.com/3scale/apicast/compare/v3.2.1...v3.3.0-beta1
 [3.3.0-beta2]: https://github.com/3scale/apicast/compare/v3.3.0-beta1...v3.3.0-beta2
+[3.3.0-cr1]: https://github.com/3scale/apicast/compare/v3.3.0-beta2...v3.3.0-cr1

gateway/src/apicast/policy/3scale_referrer/apicast-policy.json

@@ -1,7 +1,7 @@
 {
   "$schema": "http://apicast.io/policy-v1/schema#manifest#",
   "name": "3scale Referrer",
-  "summary": "Sends the 'Referer' to 3scale backend so it can be validated",
+  "summary": "Sends the 'Referer' to 3scale backend so it can be validated.",
   "description": "Sends the 'Referer' to 3scale backend so it can be validated",
   "version": "builtin",
   "configuration": {

gateway/src/apicast/policy/conditional/apicast-policy.json

@@ -1,9 +1,10 @@
 {
   "$schema": "http://apicast.io/policy-v1/schema#manifest#",
   "name": "Conditional policy",
-  "summary": "Executes a policy chain conditionally",
+  "summary": "Executes a policy chain conditionally.",
   "description": [
-    "Evaluates a condition, and when it's true, it calls its policy chain."
+    "Evaluates a condition, and when it's true, it calls its policy chain. ",
+    "This policy cannot be configured from the 3scale UI."
   ],
   "version": "builtin",
   "configuration": {

gateway/src/apicast/policy/default_credentials/apicast-policy.json

@@ -1,7 +1,7 @@
 {
   "$schema": "http://apicast.io/policy-v1/schema#manifest#",
   "name": "Anonymous access",
-  "summary": "Provides default credentials for unauthenticated requests",
+  "summary": "Provides default credentials for unauthenticated requests.",
   "description":
     ["This policy allows to expose a service without authentication. ",
      "It can be useful, for example, for legacy apps that cannot be adapted to ",

gateway/src/apicast/policy/logging/apicast-policy.json

@@ -1,7 +1,7 @@
 {
   "$schema": "http://apicast.io/policy-v1/schema#manifest#",
   "name": "Logging",
-  "summary": "Controls logging",
+  "summary": "Controls logging.",
   "description": [
     "Controls logging. It allows to enable and disable access logs per ",
     "service."

gateway/src/apicast/policy/rewrite_url_captures/apicast-policy.json

@@ -1,7 +1,7 @@
 {
   "$schema": "http://apicast.io/policy-v1/schema#manifest#",
   "name": "URL rewriting with captures",
-  "summary": "Captures arguments in a URL and rewrites the URL using them",
+  "summary": "Captures arguments in a URL and rewrites the URL using them.",
   "description":
     ["Captures arguments in a URL and rewrites the URL using those arguments. ",
      "For example, we can specify a matching rule with arguments like ",

gateway/src/apicast/version.lua

@@ -1 +1 @@
-return "3.3.0-beta2"
+return "3.3.0"

gateway/src/resty/limit/count-inc.lua

@@ -33,6 +33,17 @@ function _M.incoming(self, key, commit)
         if not count then
             return nil, err
         end
+
+        if count > limit then
+            count, err = dict:incr(key, -1)
+
+            if not count then
+                return nil, err
+            end
+
+            return nil, "rejected"
+        end
+
     else
         count = (dict:get(key) or 0) + 1
     end

spec/policy/rate_limit/rate_limit_spec.lua

@@ -194,7 +194,7 @@ describe('Rate limit policy', function()
           assert(rate_limit_policy:access(context))
           assert.returns_error('limits exceeded', rate_limit_policy:access(context))
 
-          assert.equal('2', redis:get('11110_fixed_window_test3'))
+          assert.equal('1', redis:get('11110_fixed_window_test3'))
           assert.spy(ngx.exit).was_called_with(429)
         end)
 
@@ -211,7 +211,7 @@ describe('Rate limit policy', function()
           assert(rate_limit_policy:access(ctx))
           assert.returns_error('limits exceeded', rate_limit_policy:access(ctx))
 
-          assert.equal('2', redis:get('11110_fixed_window_test3'))
+          assert.equal('1', redis:get('11110_fixed_window_test3'))
           assert.spy(ngx.exit).was_called_with(429)
         end)
 
@@ -232,7 +232,7 @@ describe('Rate limit policy', function()
           assert(rate_limit_policy:access(context))
           assert.returns_error('limits exceeded', rate_limit_policy:access(context))
 
-          assert.equal('2', redis:get('11110_fixed_window_' .. test_host))
+          assert.equal('1', redis:get('11110_fixed_window_' .. test_host))
           assert.spy(ngx.exit).was_called_with(429)
         end)
 

spec/resty/limit/count-inc_spec.lua

@@ -0,0 +1,152 @@
+local _M = require 'resty.limit.count-inc'
+
+local shdict_mt = {
+  __index = {
+    get = function(t, k) return rawget(t, k) end,
+    set = function(t, k, v) rawset(t, k , v); return true end,
+    incr = function(t, key, inc, init, _)
+      local value = t:get(key) or init
+      if not value then return nil, 'not found' end
+
+      t:set(key, value + inc)
+      return t:get(key)
+    end,
+  }
+}
+local function shdict()
+  return setmetatable({ }, shdict_mt)
+end
+
+describe('resty.limit.count-inc', function()
+
+  before_each(function()
+    ngx.shared.limiter = mock(shdict(), true)
+  end)
+
+  describe('.new', function()
+    describe('using correct shdict', function()
+      it('returns limiter', function()
+        local lim = _M.new('limiter', 1, 1)
+        assert.same(1, lim.limit)
+        assert.same(1, lim.window)
+      end)
+    end)
+
+    describe('using incorrect shdict', function()
+      it('returns error', function()
+        local _, err = _M.new('incorrect', 1, 1)
+        assert.same('shared dict not found', err)
+      end)
+    end)
+  end)
+
+  describe('.incoming', function()
+    local lim
+
+    before_each(function()
+      lim = _M.new('limiter', 1, 1)
+    end)
+
+    describe('when commit is true', function()
+      describe('if count is less than limit', function()
+        it('returns zero and remaining', function()
+          local delay, err = lim:incoming('tmp', true)
+          assert.same(0, delay)
+          assert.same(0, err)
+        end)
+
+        describe('and incr method fails', function()
+          it('returns error', function()
+            ngx.shared.limiter.incr = function()
+              return nil, 'something'
+            end
+            local delay, err = lim:incoming('tmp', true)
+            assert.is_nil(delay)
+            assert.same('something', err)
+          end)
+        end)
+      end)
+
+      describe('if count is greater than limit', function()
+        it('return rejected', function()
+          lim:incoming('tmp', true)
+          local delay, err = lim:incoming('tmp', true)
+          assert.is_nil(delay)
+          assert.same('rejected', err)
+        end)
+
+        describe('and incr method fails', function()
+          it('returns error', function()
+            ngx.shared.limiter.incr = function(t, key, inc, init, _)
+              local value = t:get(key) or init
+              if not value then return nil, 'not found' end
+              if inc == -1 then return nil, 'something' end
+              t:set(key, value + inc)
+              return t:get(key)
+            end
+            lim:incoming('tmp', true)
+            local delay, err = lim:incoming('tmp', true)
+            assert.is_nil(delay)
+            assert.same('something', err)
+          end)
+        end)
+      end)
+    end)
+
+    describe('when commit is false', function()
+      describe('if count is less than limit', function()
+        it('returns zero and remaining', function()
+          local delay, err = lim:incoming('tmp', false)
+          assert.same(0, delay)
+          assert.same(0, err)
+        end)
+      end)
+
+      describe('if count is greater than limit', function()
+        it('return rejected', function()
+          lim:incoming('tmp', true)
+          local delay, err = lim:incoming('tmp', false)
+          assert.is_nil(delay)
+          assert.same('rejected', err)
+        end)
+      end)
+    end)
+
+  end)
+
+  describe('.uncommit', function()
+    local lim
+
+    before_each(function()
+      lim = _M.new('limiter', 1, 1)
+    end)
+
+    describe('when incr method succeeds', function()
+      it('returns remaining', function()
+        lim:incoming('tmp', true)
+        local delay = lim:uncommit('tmp')
+        assert.same(1, delay)
+      end)
+    end)
+
+    describe('when incr method fails', function()
+      describe('if key is not found', function()
+        it('returns remaining', function()
+          local delay = lim:uncommit('tmp')
+          assert.same(1, delay)
+        end)
+      end)
+
+      it('returns error', function()
+        lim:incoming('tmp', true)
+        ngx.shared.limiter.incr = function()
+          return nil, 'something'
+        end
+        local delay, err = lim:uncommit('tmp')
+        assert.is_nil(delay)
+        assert.same('something', err)
+      end)
+    end)
+  end)
+
+end)