JSON conditions for conditional policy

CHANGELOG.md

@@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `GC` module that implements a workaround to be able to define `__gc` on tables [PR #790](https://github.com/3scale/apicast/pull/790)
 - Policies can define `__gc` metamethod that gets called when they are garbage collected to do cleanup [PR #688](https://github.com/3scale/apicast/pull/688)
 - Keycloak Role Check policy [PR #773](https://github.com/3scale/apicast/pull/773)
-- Conditional policy. This policy includes a condition and a policy chain, and only executes the chain when the condition is true [PR #812](https://github.com/3scale/apicast/pull/812)
+- Conditional policy. This policy includes a condition and a policy chain, and only executes the chain when the condition is true [PR #812](https://github.com/3scale/apicast/pull/812), [PR #814](https://github.com/3scale/apicast/pull/814)
 
 ### Changed
 

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

@@ -8,10 +8,73 @@
   "version": "builtin",
   "configuration": {
     "type": "object",
+    "definitions": {
+      "operation": {
+        "type": "object",
+        "$id": "#/definitions/operation",
+        "properties": {
+          "left": {
+            "type": "string"
+          },
+          "op": {
+            "type": "string",
+            "enum": ["==", "!="]
+          },
+          "right": {
+            "type": "string"
+          },
+          "left_type": {
+            "description": "How to evaluate 'left'",
+            "type": "string",
+            "default": "plain",
+            "oneOf": [
+              {
+                "enum": ["plain"],
+                "title": "Evaluate 'left' as plain text."
+              },
+              {
+                "enum": ["liquid"],
+                "title": "Evaluate 'left' as liquid."
+              }
+            ]
+          },
+          "right_type": {
+            "description": "How to evaluate 'right'",
+            "type": "string",
+            "default": "plain",
+            "oneOf": [
+              {
+                "enum": ["plain"],
+                "title": "Evaluate 'right' as plain text."
+              },
+              {
+                "enum": ["liquid"],
+                "title": "Evaluate 'right' as liquid."
+              }
+            ]
+          }
+        },
+        "required": ["left", "op", "right"]
+      }
+    },
     "properties": {
       "condition": {
-        "description": "condition to be evaluated",
-        "type": "string"
+        "description": "conditions to be evaluated",
+        "type": "object",
+        "properties": {
+          "operations": {
+            "type": "array",
+            "items": {
+              "$ref": "#/definitions/operation"
+            },
+            "minItems": 1
+          },
+          "combine_op": {
+            "type": "string",
+            "enum": ["and", "or"],
+            "default": "and"
+          }
+        }
       },
       "policy_chain": {
         "description": "the policy chain to execute when the condition is true",

gateway/src/apicast/policy/conditional/condition.lua

@@ -0,0 +1,58 @@
+local setmetatable = setmetatable
+local ipairs = ipairs
+local assert = assert
+
+local _M = {}
+
+local mt = { __index = _M }
+
+local default_combine_op = 'and'
+
+local function all_true(operations, context)
+  for _, operation in ipairs(operations) do
+    if not operation:evaluate(context) then
+      return false
+    end
+  end
+
+  return true
+end
+
+local function at_least_one_true(operations, context)
+  for _, operation in ipairs(operations) do
+    if operation:evaluate(context) then
+      return true
+    end
+  end
+
+  return false
+end
+
+local evaluate_func = {
+  ['and'] = all_true,
+  ['or'] = at_least_one_true,
+  ['true'] = function() return true end
+}
+
+function _M.new(operations, combine_op)
+  local self = setmetatable({}, mt)
+
+  if not operations then
+    -- If there's nothing to evaluate, return true.
+    self.evaluate_func = evaluate_func['true']
+  else
+    self.evaluate_func = evaluate_func[combine_op or default_combine_op]
+  end
+
+  assert(self.evaluate_func, 'Unsupported combine op')
+
+  self.operations = operations
+
+  return self
+end
+
+function _M:evaluate(context)
+  return self.evaluate_func(self.operations, context)
+end
+
+return _M

gateway/src/apicast/policy/conditional/conditional.lua

@@ -1,7 +1,12 @@
+local ipairs = ipairs
+local insert = table.insert
+
 local policy = require('apicast.policy')
 local policy_phases = require('apicast.policy').phases
 local PolicyChain = require('apicast.policy_chain')
-local Engine = require('apicast.policy.conditional.engine')
+local Condition = require('apicast.policy.conditional.condition')
+local Operation = require('apicast.policy.conditional.operation')
+local ngx_variable = require('apicast.policy.ngx_variable')
 
 local _M = policy.new('Conditional policy')
 
@@ -23,25 +28,41 @@ local function build_policy_chain(chain)
   return PolicyChain.new(policies)
 end
 
+local function build_operations(config_ops)
+  local res = {}
+
+  for _, op in ipairs(config_ops) do
+    insert(res, Operation.new(op.left, op.left_type, op.op, op.right, op.right_type))
+  end
+
+  return res
+end
+
 function _M.new(config)
   local self = new(config)
-  self.condition = config.condition or true
+
+  config.condition = config.condition or {}
+  self.condition = Condition.new(
+    build_operations(config.condition.operations),
+    config.condition.combine_op
+  )
+
   self.policy_chain = build_policy_chain(config.policy_chain)
   return self
 end
 
-local function condition_is_true(condition)
-  return Engine.evaluate(condition)
-end
-
 function _M:export()
   return self.policy_chain:export()
 end
 
 -- Forward policy phases to chain
 for _, phase in policy_phases() do
   _M[phase] = function(self, context)
-    if condition_is_true(self.condition) then
+    local condition_is_true = self.condition:evaluate(
+      ngx_variable.available_context(context)
+    )
+
+    if condition_is_true then
       ngx.log(ngx.DEBUG, 'Condition met in conditional policy')
       self.policy_chain[phase](self.policy_chain, context)
     else

gateway/src/apicast/policy/conditional/operation.lua

@@ -0,0 +1,49 @@
+local setmetatable = setmetatable
+local assert = assert
+local tostring = tostring
+
+local TemplateString = require 'apicast.template_string'
+
+local default_value_type = 'plain'
+
+local _M = {}
+
+local mt = { __index = _M }
+
+local function create_template(value, type)
+  return TemplateString.new(value, type or default_value_type)
+end
+
+local evaluate_func = {
+  ['=='] = function(left, right) return tostring(left) == tostring(right) end,
+  ['!='] = function(left, right) return tostring(left) ~= tostring(right) end
+}
+
+--- Initialize an operation
+-- @tparam string left Left operand
+-- @tparam[opt] string left_type How to evaluate the left operand ('plain' or
+--   'liquid'). Default: 'plain'.
+-- @tparam string op Operation
+-- @tparam[opt] string right Right operand
+-- @tparam string right_type How to evaluate the right operand ('plain' or
+--   'liquid'). Default: 'plain'.
+function _M.new(left, left_type, op, right, right_type)
+  local self = setmetatable({}, mt)
+
+  self.evaluate_func = evaluate_func[op]
+  assert(self.evaluate_func, 'Unsupported operation')
+
+  self.templated_left = create_template(left, left_type)
+  self.templated_right = create_template(right, right_type)
+
+  return self
+end
+
+function _M:evaluate(context)
+  local left_value = self.templated_left:render(context)
+  local right_value = self.templated_right:render(context)
+
+  return self.evaluate_func(left_value, right_value)
+end
+
+return _M

spec/policy/conditional/condition_spec.lua

@@ -0,0 +1,89 @@
+local Condition = require('apicast.policy.conditional.condition')
+local Operation = require('apicast.policy.conditional.operation')
+
+describe('Engine', function()
+  describe('.new', function()
+    it('raises error with an unsupported operation', function()
+      local res, err = pcall(Condition.new, {}, '<>')
+
+      assert.is_falsy(res)
+      assert.is_truthy(err)
+    end)
+  end)
+
+  describe('.evaluate', function()
+    it('combines operations with "and"', function()
+      local true_condition = Condition.new(
+        {
+          Operation.new('1', 'plain', '==', '1', 'plain'),
+          Operation.new('2', 'plain', '==', '2', 'plain'),
+          Operation.new('3', 'plain', '==', '3', 'plain')
+        },
+        'and'
+      )
+
+      assert.is_true(true_condition:evaluate())
+
+      local false_condition = Condition.new(
+        {
+          Operation.new('1', 'plain', '==', '1', 'plain'),
+          Operation.new('2', 'plain', '==', '20', 'plain'),
+          Operation.new('3', 'plain', '==', '3', 'plain')
+        },
+        'and'
+      )
+
+      assert.is_false(false_condition:evaluate())
+    end)
+
+    it('combines operations with "or"', function()
+      local true_condition = Condition.new(
+        {
+          Operation.new('1', 'plain', '==', '10', 'plain'),
+          Operation.new('2', 'plain', '==', '20', 'plain'),
+          Operation.new('3', 'plain', '==', '3', 'plain')
+        },
+        'or'
+      )
+
+      assert.is_true(true_condition:evaluate())
+
+      local false_condition = Condition.new(
+        {
+          Operation.new('1', 'plain', '==', '10', 'plain'),
+          Operation.new('2', 'plain', '==', '20', 'plain'),
+          Operation.new('3', 'plain', '==', '30', 'plain')
+        },
+        'or'
+      )
+
+      assert.is_false(false_condition:evaluate())
+    end)
+
+    it('combines operations with "and" by default', function()
+      local true_condition = Condition.new(
+        {
+          Operation.new('1', 'plain', '==', '1', 'plain'),
+          Operation.new('2', 'plain', '==', '2', 'plain'),
+          Operation.new('3', 'plain', '==', '3', 'plain')
+        }
+      )
+
+      assert.is_true(true_condition:evaluate())
+
+      local false_condition = Condition.new(
+        {
+          Operation.new('1', 'plain', '==', '1', 'plain'),
+          Operation.new('2', 'plain', '==', '20', 'plain'),
+          Operation.new('3', 'plain', '==', '3', 'plain')
+        }
+      )
+
+      assert.is_false(false_condition:evaluate())
+    end)
+
+    it('returns true when there are no operations', function()
+      assert.is_true(Condition.new():evaluate())
+    end)
+  end)
+end)

spec/policy/conditional/conditional_spec.lua

@@ -1,12 +1,23 @@
 local ConditionalPolicy = require('apicast.policy.conditional')
 local Policy = require('apicast.policy')
 local PolicyChain = require('apicast.policy_chain')
-local Engine = require('apicast.policy.conditional.engine')
+local ngx_variable = require('apicast.policy.ngx_variable')
 
 describe('Conditional policy', function()
   local test_policy_chain
   local context
-  local condition = "request_path == '/some_path'"
+
+  local true_condition = {
+    operations = {
+      { left = '{{ var_1 }}', left_type = 'liquid', op = '==', right = '1' }
+    }
+  }
+
+  local false_condition = {
+    operations = {
+      { left = '1', op = '==', right = '2' }
+    }
+  }
 
   before_each(function()
     test_policy_chain = PolicyChain.build({})
@@ -15,16 +26,14 @@ describe('Conditional policy', function()
       test_policy_chain[phase] = spy.new(function() end)
     end
 
+    stub(ngx_variable, 'available_context').returns({ var_1 = '1' })
+
     context = {}
   end)
 
   describe('when the condition is true', function()
-    before_each(function()
-      stub(Engine, 'evaluate').returns(true)
-    end)
-
     it('forwards the policy phases (except init and init_worker) to the chain', function()
-      local conditional = ConditionalPolicy.new({ condition = condition })
+      local conditional = ConditionalPolicy.new({ condition = true_condition })
 
       -- .new() will try to load the chain, set it here to avoid that and
       -- control which one to use.
@@ -45,12 +54,8 @@ describe('Conditional policy', function()
   end)
 
   describe('when the condition is false', function()
-    before_each(function()
-      stub(Engine, 'evaluate').returns(false)
-    end)
-
     it('does not forward the policy phases to the chain', function()
-      local conditional = ConditionalPolicy.new({ condition = condition })
+      local conditional = ConditionalPolicy.new({ condition = false_condition })
       conditional.policy_chain = test_policy_chain
 
       for _, phase in Policy.phases() do
@@ -67,7 +72,7 @@ describe('Conditional policy', function()
 
       stub(test_policy_chain, 'export').returns(exported_by_chain)
 
-      local conditional = ConditionalPolicy.new({ condition = condition })
+      local conditional = ConditionalPolicy.new({ condition = true_condition })
       conditional.policy_chain = test_policy_chain
 
       assert.same(exported_by_chain, conditional:export())

spec/policy/conditional/operation_spec.lua

@@ -0,0 +1,61 @@
+local Operation = require('apicast.policy.conditional.operation')
+
+describe('Operation', function()
+  describe('.new', function()
+    it('raises error with an unsupported operation', function()
+      local res, err = pcall(Operation.new, '1', 'plain', '<>', '1', 'plain')
+
+      assert.is_falsy(res)
+      assert.is_truthy(err)
+    end)
+  end)
+
+  describe('.evaluate', function()
+    it('evaluates ==', function()
+      assert.is_true(Operation.new('1', 'plain', '==', '1', 'plain'):evaluate({}))
+      assert.is_false(Operation.new('1', 'plain', '==', '2', 'plain'):evaluate({}))
+    end)
+
+    it('evaluates !=', function()
+      assert.is_true(Operation.new('1', 'plain', '!=', '2', 'plain'):evaluate({}))
+      assert.is_false(Operation.new('1', 'plain', '!=', '1', 'plain'):evaluate({}))
+    end)
+
+    it('evaluates values as plain text by default', function()
+      assert.is_true(Operation.new('1', nil, '==', '1', nil):evaluate({}))
+      assert.is_false(Operation.new('1', nil, '==', '2', nil):evaluate({}))
+    end)
+
+    it('evaluates liquid when indicated in the types', function()
+      local context = { var_1 = '1', var_2 = '2' }
+
+      local res_true = Operation.new(
+        '{{ var_1 }}', 'liquid', '==', '1', 'plain'
+      ):evaluate(context)
+
+      assert.is_true(res_true)
+
+      local res_false = Operation.new(
+        '{{ var_1 }}', 'liquid', '==', '{{ var_2 }}', 'liquid'
+      ):evaluate(context)
+
+      assert.is_false(res_false)
+    end)
+
+    it('evaluates comparison ops without differentiating types', function()
+      local context = { var_1 = 1 }
+
+      local eq_int_and_string = Operation.new(
+        '{{ var_1 }}', 'liquid', '==', '1', 'plain'
+      ):evaluate(context)
+
+      assert.is_true(eq_int_and_string)
+
+      local not_eq_int_and_string = Operation.new(
+        '{{ var_1 }}', 'liquid', '!=', '1', 'plain'
+      ):evaluate(context)
+
+      assert.is_false(not_eq_int_and_string)
+    end)
+  end)
+end)

t/apicast-policy-conditional.t

@@ -20,7 +20,17 @@ phases it runs, so we can use that to verify it was executed.
           {
             "name": "apicast.policy.conditional",
             "configuration": {
-              "condition": "request_path == \"/log\"",
+              "condition": {
+                "operations": [
+                  {
+                    "left": "{{ uri }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "/log",
+                    "right_type": "plain"
+                  }
+                ]
+              },
               "policy_chain": [
                 {
                   "name": "apicast.policy.phase_logger"
@@ -61,7 +71,17 @@ phases it runs, so we can use that to verify that it was not executed.
           {
             "name": "apicast.policy.conditional",
             "configuration": {
-              "condition": "request_path == \"/log\"",
+              "condition": {
+                "operations": [
+                  {
+                    "left": "{{ uri }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "/log",
+                    "right_type": "plain"
+                  }
+                ]
+              },
               "policy_chain": [
                 {
                   "name": "apicast.policy.phase_logger"
@@ -85,3 +105,58 @@ GET / HTTP/1.1
 --- no_error_log
 [error]
 running phase: rewrite
+
+=== TEST 3: Combine several operations in the condition
+--- configuration
+{
+  "services": [
+    {
+      "id": 42,
+      "proxy": {
+        "policy_chain": [
+          {
+            "name": "apicast.policy.conditional",
+            "configuration": {
+              "condition": {
+                "operations": [
+                  {
+                    "left": "{{ uri }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "/log",
+                    "right_type": "plain"
+                  },
+                  {
+                    "left": "{{ service.id }}",
+                    "left_type": "liquid",
+                    "op": "==",
+                    "right": "42",
+                    "right_type": "plain"
+                  }
+                ],
+                "combine_op": "and"
+              },
+              "policy_chain": [
+                {
+                  "name": "apicast.policy.phase_logger"
+                }
+              ]
+            }
+          },
+          {
+            "name": "apicast.policy.echo"
+          }
+        ]
+      }
+    }
+  ]
+}
+--- request
+GET /log
+--- response_body
+GET /log HTTP/1.1
+--- error_code: 200
+--- no_error_log
+[error]
+--- error_log chomp
+running phase: rewrite