From 688786e30c4aa02ed697023e3e754279a39f9b5d Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 11:00:54 +0200
Subject: [PATCH 1/7] spec/policy/rate_limit: fix organization of describe
 blocks

The "when conditions are defined" block was incorrectly placed under the
"using redis" one.
---
 spec/policy/rate_limit/rate_limit_spec.lua | 290 ++++++++++-----------
 1 file changed, 145 insertions(+), 145 deletions(-)

diff --git a/spec/policy/rate_limit/rate_limit_spec.lua b/spec/policy/rate_limit/rate_limit_spec.lua
index 9eaec2827..b0a232c9e 100644
--- a/spec/policy/rate_limit/rate_limit_spec.lua
+++ b/spec/policy/rate_limit/rate_limit_spec.lua
@@ -324,192 +324,192 @@ describe('Rate limit policy', function()
           assert.spy(ngx.sleep).was_called_with(match.is_gt(0.001))
         end)
       end)
+    end)
 
-      describe('when conditions are defined', function()
-        local true_condition = {
-          operations = {
-            { left = '1', op = '==', right = '1' }
-          }
+    describe('when conditions are defined', function()
+      local true_condition = {
+        operations = {
+          { left = '1', op = '==', right = '1' }
         }
+      }
 
-        local false_condition = {
-          operations = {
-            { left = '1', op = '==', right = '2' }
-          }
+      local false_condition = {
+        operations = {
+          { left = '1', op = '==', right = '2' }
         }
+      }
 
-        describe('and the type of the limit is "connection"', function()
-          it('applies the limit when the condition is true', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              connection_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  conn = 1, burst = 0, delay = 0.5,
-                  condition = true_condition
-                }
+      describe('and the type of the limit is "connection"', function()
+        it('applies the limit when the condition is true', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            connection_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                conn = 1, burst = 0, delay = 0.5,
+                condition = true_condition
               }
-            })
+            }
+          })
 
-            assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
 
-            assert.returns_error('limits exceeded', rate_limit_policy:access(context))
-            assert.spy(ngx.exit).was_called_with(429)
-          end)
+          assert.returns_error('limits exceeded', rate_limit_policy:access(context))
+          assert.spy(ngx.exit).was_called_with(429)
+        end)
 
-          it('does not apply the limit when the condition is false', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              connection_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  conn = 1, burst = 0, delay = 0.5,
-                  condition = false_condition
-                }
+        it('does not apply the limit when the condition is false', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            connection_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                conn = 1, burst = 0, delay = 0.5,
+                condition = false_condition
               }
-            })
+            }
+          })
 
-            -- Limit is 1. Make 2 requests and check that they're not limited
-            assert(rate_limit_policy:access(context))
-            assert(rate_limit_policy:access(context))
-          end)
+          -- Limit is 1. Make 2 requests and check that they're not limited
+          assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
         end)
+      end)
 
-        describe('and the type of the limit is "leaky_bucket"', function()
-          it('applies the limit when the condition is true', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              leaky_bucket_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  rate = 1,
-                  burst = 0,
-                  condition = true_condition
-                }
+      describe('and the type of the limit is "leaky_bucket"', function()
+        it('applies the limit when the condition is true', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            leaky_bucket_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                rate = 1,
+                burst = 0,
+                condition = true_condition
               }
-            })
+            }
+          })
 
-            assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
 
-            assert.returns_error('limits exceeded', rate_limit_policy:access(context))
-            assert.spy(ngx.exit).was_called_with(429)
-          end)
+          assert.returns_error('limits exceeded', rate_limit_policy:access(context))
+          assert.spy(ngx.exit).was_called_with(429)
+        end)
 
-          it('does not apply the limit when the condition is false', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              leaky_bucket_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  rate = 1,
-                  burst = 0,
-                  condition = false_condition
-                }
+        it('does not apply the limit when the condition is false', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            leaky_bucket_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                rate = 1,
+                burst = 0,
+                condition = false_condition
               }
-            })
+            }
+          })
 
-            -- Limit is 1. Make 2 requests and check that they're not limited
-            assert(rate_limit_policy:access(context))
-            assert(rate_limit_policy:access(context))
-          end)
+          -- Limit is 1. Make 2 requests and check that they're not limited
+          assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
         end)
+      end)
 
-        describe('and the type of the limits is "fixed_window"', function()
-          it('applies the limit when the condition is true', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              fixed_window_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  count = 1,
-                  window = 10,
-                  condition = true_condition
-                }
+      describe('and the type of the limits is "fixed_window"', function()
+        it('applies the limit when the condition is true', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            fixed_window_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                count = 1,
+                window = 10,
+                condition = true_condition
               }
-            })
+            }
+          })
 
-            assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
 
-            assert.returns_error('limits exceeded', rate_limit_policy:access(context))
-            assert.spy(ngx.exit).was_called_with(429)
-          end)
+          assert.returns_error('limits exceeded', rate_limit_policy:access(context))
+          assert.spy(ngx.exit).was_called_with(429)
+        end)
 
-          it('does not apply the limit when the condition is false', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              fixed_window_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  count = 1,
-                  window = 10,
-                  condition = false_condition
-                }
+        it('does not apply the limit when the condition is false', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            fixed_window_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                count = 1,
+                window = 10,
+                condition = false_condition
               }
-            })
+            }
+          })
 
-            -- Limit is 1. Make 2 requests and check that they're not limited
-            assert(rate_limit_policy:access(context))
-            assert(rate_limit_policy:access(context))
-          end)
+          -- Limit is 1. Make 2 requests and check that they're not limited
+          assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
         end)
+      end)
 
-        describe('and there are several limits applied', function()
-          it('denies access when the condition of any limit is false', function()
-            local rate_limit_policy = RateLimitPolicy.new({
-              leaky_bucket_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  rate = 1,
-                  burst = 0,
-                  condition = false_condition
-                }
-              },
-              fixed_window_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  count = 1,
-                  window = 10,
-                  condition = true_condition
-                }
-              },
-              connection_limiters = {
-                {
-                  key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
-                  conn = 1, burst = 0, delay = 0.5,
-                  condition = true_condition
-                }
+      describe('and there are several limits applied', function()
+        it('denies access when the condition of any limit is false', function()
+          local rate_limit_policy = RateLimitPolicy.new({
+            leaky_bucket_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                rate = 1,
+                burst = 0,
+                condition = false_condition
+              }
+            },
+            fixed_window_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                count = 1,
+                window = 10,
+                condition = true_condition
+              }
+            },
+            connection_limiters = {
+              {
+                key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+                conn = 1, burst = 0, delay = 0.5,
+                condition = true_condition
               }
-            })
+            }
+          })
 
-            assert(rate_limit_policy:access(context))
+          assert(rate_limit_policy:access(context))
 
-            assert.returns_error('limits exceeded', rate_limit_policy:access(context))
-            assert.spy(ngx.exit).was_called_with(429)
-          end)
+          assert.returns_error('limits exceeded', rate_limit_policy:access(context))
+          assert.spy(ngx.exit).was_called_with(429)
         end)
       end)
     end)
+  end)
 
-    describe('.log', function()
-      it('success in leaving', function()
-        local rate_limit_policy = RateLimitPolicy.new({
-          connection_limiters = {
-            { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 }
-          },
-          redis_url = redis_url
-        })
+  describe('.log', function()
+    it('success in leaving', function()
+      local rate_limit_policy = RateLimitPolicy.new({
+        connection_limiters = {
+          { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 }
+        },
+        redis_url = redis_url
+      })
 
-        assert(rate_limit_policy:access(context))
+      assert(rate_limit_policy:access(context))
 
-        assert(rate_limit_policy:log())
-      end)
+      assert(rate_limit_policy:log())
+    end)
 
-      it('success when redis url is empty', function()
-        local rate_limit_policy = RateLimitPolicy.new({
-          connection_limiters = {
-            { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 }
-          },
-          redis_url = ''
-        })
+    it('success when redis url is empty', function()
+      local rate_limit_policy = RateLimitPolicy.new({
+        connection_limiters = {
+          { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 }
+        },
+        redis_url = ''
+      })
 
-        assert(rate_limit_policy:access(context))
+      assert(rate_limit_policy:access(context))
 
-        assert(rate_limit_policy:log())
-      end)
+      assert(rate_limit_policy:log())
     end)
   end)
 end)

From 6a7b476c68ddb9c4e443737e3c4f4223228cd07b Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 11:27:36 +0200
Subject: [PATCH 2/7] policy/rate_limit: add matches operation to the schema

---
 gateway/src/apicast/policy/rate_limit/apicast-policy.json | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gateway/src/apicast/policy/rate_limit/apicast-policy.json b/gateway/src/apicast/policy/rate_limit/apicast-policy.json
index 69c074c45..17bd383ff 100644
--- a/gateway/src/apicast/policy/rate_limit/apicast-policy.json
+++ b/gateway/src/apicast/policy/rate_limit/apicast-policy.json
@@ -63,8 +63,9 @@
             "type": "string"
           },
           "op": {
+            "description": "Operation to apply. The matches op supports PCRE (Perl compatible regular expressions)",
             "type": "string",
-            "enum": ["==", "!="]
+            "enum": ["==", "!=", "matches"]
           },
           "right": {
             "type": "string"

From 6880e45fc7afe5ca761e70a0e270f1c99ce5dbf8 Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 11:28:01 +0200
Subject: [PATCH 3/7] spec/policy/rate_limit: test condition with 'matches' op

---
 spec/policy/rate_limit/rate_limit_spec.lua | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/spec/policy/rate_limit/rate_limit_spec.lua b/spec/policy/rate_limit/rate_limit_spec.lua
index b0a232c9e..e831aa137 100644
--- a/spec/policy/rate_limit/rate_limit_spec.lua
+++ b/spec/policy/rate_limit/rate_limit_spec.lua
@@ -483,6 +483,53 @@ describe('Rate limit policy', function()
         end)
       end)
     end)
+
+    it('accepts "matches" as an operation in the conditions', function()
+      local condition_with_matches = {
+        operations = {
+          {
+            left = '{{ uri }}',
+            left_type = 'liquid',
+            op = 'matches',
+            right = '/v1/.*/something/.*',
+            right_type = 'plain'
+          }
+        }
+      }
+
+      local rate_limit_policy = RateLimitPolicy.new({
+        fixed_window_limiters = {
+          {
+            key = { name = 'limit_key', name_type = 'plain', scope = 'global' },
+            count = 2,
+            window = 10,
+            condition = condition_with_matches
+          }
+        }
+      })
+
+      -- We are going to make 3 requests that match the url pattern defined
+      -- above. As the limit is set to 2, only the third one should fail.
+
+      ngx_variable.available_context:revert()
+      stub(ngx_variable, 'available_context', function()
+        return { uri = '/v1/aaa/something/bbb' }
+      end)
+      assert(rate_limit_policy:access({}))
+
+      ngx_variable.available_context:revert()
+      stub(ngx_variable, 'available_context', function()
+        return { uri = '/v1/ccc/something/ddd' }
+      end)
+      assert(rate_limit_policy:access({}))
+
+      ngx_variable.available_context:revert()
+      stub(ngx_variable, 'available_context', function()
+        return { uri = '/v1/eee/something/fff' }
+      end)
+      assert.returns_error('limits exceeded', rate_limit_policy:access({}))
+      assert.spy(ngx.exit).was_called_with(429)
+    end)
   end)
 
   describe('.log', function()

From 5711fc60829efc0cf921bb48157daed6b49eaa68 Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 11:38:29 +0200
Subject: [PATCH 4/7] t/apicast-policy-rate-limit: test condition with
 'matches' op

---
 t/apicast-policy-rate-limit.t | 56 +++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t
index 1fc3d833b..5e0ae2f7e 100644
--- a/t/apicast-policy-rate-limit.t
+++ b/t/apicast-policy-rate-limit.t
@@ -1281,3 +1281,59 @@ making 3 requests and checking that only the last one is rejected.
 [200, 200, 200, 429]
 --- no_error_log
 [error]
+
+=== TEST 21: condition with "matches" operation
+This test makes 3 requests that match the URL pattern defined in the
+limit. The limit is set to 2. Only the third one should fail.
+--- http_config
+  include $TEST_NGINX_UPSTREAM_CONFIG;
+  lua_package_path "$TEST_NGINX_LUA_PATH";
+
+  init_by_lua_block {
+    require "resty.core"
+    ngx.shared.limiter:flush_all()
+    require('apicast.configuration_loader').mock({
+      services = {
+        {
+          id = 42,
+          proxy = {
+            policy_chain = {
+              {
+                name = "apicast.policy.rate_limit",
+                configuration = {
+                  fixed_window_limiters = {
+                    {
+                      key = { name = "test21_key_1" },
+                      count = 2,
+                      window = 60,
+                      condition = {
+                        operations = {
+                          {
+                            left = "{{ uri }}",
+                            left_type = "liquid",
+                            op = "matches",
+                            right = "/v1/.*/something/.*",
+                            right_type = "plain"
+                          }
+                        }
+                      }
+                    }
+                  },
+                  limits_exceeded_error = { status_code = 429 }
+                }
+              }
+            }
+          }
+        }
+      }
+    })
+  }
+  lua_shared_dict limiter 1m;
+--- config
+  include $TEST_NGINX_APICAST_CONFIG;
+--- pipelined_requests eval
+["GET /v1/aaa/something/bbb", "GET /v1/ccc/something/ddd", "GET /v1/eee/something/fff"]
+--- error_code eval
+[200, 200, 429]
+--- no_error_log
+[error]

From 5d74461029cda9d3d686e2a9c3d5d5df3a315c5e Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 11:40:51 +0200
Subject: [PATCH 5/7] t/apicast-policy-rate-limit: delete unnecessary redis
 flush

Redis is not used in this test. Looks like a copy-paste issue.
---
 t/apicast-policy-rate-limit.t | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t
index 5e0ae2f7e..fd1b6f5ba 100644
--- a/t/apicast-policy-rate-limit.t
+++ b/t/apicast-policy-rate-limit.t
@@ -1276,9 +1276,9 @@ making 3 requests and checking that only the last one is rejected.
 --- config
   include $TEST_NGINX_APICAST_CONFIG;
 --- pipelined_requests eval
-["GET /flush_redis", "GET /", "GET /", "GET /"]
+["GET /", "GET /", "GET /"]
 --- error_code eval
-[200, 200, 200, 429]
+[200, 200, 429]
 --- no_error_log
 [error]
 

From b736480df664d12f085257e02d415ccad001b420 Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 11:51:44 +0200
Subject: [PATCH 6/7] rate_limit/README: add example with matches

---
 .../src/apicast/policy/rate_limit/README.md   | 34 ++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/gateway/src/apicast/policy/rate_limit/README.md b/gateway/src/apicast/policy/rate_limit/README.md
index 9457eb8bf..63e175549 100644
--- a/gateway/src/apicast/policy/rate_limit/README.md
+++ b/gateway/src/apicast/policy/rate_limit/README.md
@@ -6,7 +6,8 @@
 - [**Liquid templating**](#liquid-templating)
 - [**Requests over limits**](#requests-over-limits)
 - [**Per-gateway vs shared**](#per-gateway-vs-shared)
-- [**Complete config example**](#complete-config-example)
+- [**Limits with conditions**](#limits-with-conditions)
+- [**Complete config example**](#complete-config-examples)
 
 
 ## Description
@@ -141,6 +142,37 @@ To use Redis, we just need to provide the `redis_url` attribute in the config
 of the policy: `"redis_url": "redis://a_host:6379"`
 
 
+## Limits with conditions
+
+The policy allows to define limits with conditions. The limit will be applied
+only when the condition is true. The following example shows how to define a
+limit that applies only when the path of the request matches
+`/v1/.*/something/.*`.
+
+```json
+{
+  "key": {
+    "name": "limit_path"
+  },
+  "count": 10,
+  "window": 60,
+  "condition": {
+    "operations": [
+      {
+        "left": "{{ uri }}",
+        "left_type": "liquid",
+        "op": "matches",
+        "right": "/v1/.*/something/.*",
+        "right_type": "plain"
+      }
+    ]
+  }
+}
+```
+
+Apart from "matches", the policy also supports operations with "==" and "!=".
+
+
 ## Complete config examples
 
 ```json

From b1693ed6fe8dcc04c818740b6716451ab76aa552 Mon Sep 17 00:00:00 2001
From: David Ortiz <z.david.ortiz@gmail.com>
Date: Wed, 29 May 2019 12:01:11 +0200
Subject: [PATCH 7/7] CHANGELOG: add support for the matches op in the rate
 limit policy

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ea710617e..19f636b5b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Add the option to obtain client_id from any JWT claim [THREESCALE-2264](https://issues.jboss.org/browse/THREESCALE-2264) [PR #1034](https://github.com/3scale/APIcast/pull/1034)
 - Added `APICAST_PATH_ROUTING_ONLY` variable that allows to perform path-based routing without falling back to the default host-based routing [PR #1035](https://github.com/3scale/APIcast/pull/1035), [THREESCALE-1150](https://issues.jboss.org/browse/THREESCALE-1150)
 - Added the option to manage access based on method on Keycloak Policy. [THREESCALE-2236](https://issues.jboss.org/browse/THREESCALE-2236) [PR #1039](https://github.com/3scale/APIcast/pull/1039)
+- The Rate Limit policy now supports conditions defined with the "matches" operation. [PR #1051](https://github.com/3scale/APIcast/pull/1051), [https://issues.jboss.org/browse/THREESCALE-2590](https://issues.jboss.org/browse/THREESCALE-2590)
 
 ### Fixed