Merge pull request #891 from 3scale/merge-ngx-variable-in-templatestring

Merge what's exposed by 'ngx_variable' in TemplateString:render()

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - When the 3scale referrer filters was enabled, cached requests were not handled correctly [PR #875](https://github.com/3scale/apicast/issues/875)
 - Invalid SNI when connecting to 3scale backend over HTTPS [THREESCALE-1269](https://issues.jboss.org/browse/THREESCALE-1269)
 - Fix handling --pid and --signal on the CLI [PR #880](https://github.com/3scale/apicast/pull/880)
+- Some policies did not have access to the vars exposed when using Liquid (`uri`, `path`, etc.) [PR #891](https://github.com/3scale/apicast/pull/891)
 
 ### Added
 

gateway/src/apicast/policy/rate_limit/rate_limit.lua

@@ -7,7 +7,6 @@ local resty_limit_count = require('resty.limit.count-inc')
 
 local ngx_semaphore = require "ngx.semaphore"
 local limit_traffic = require "resty.limit.traffic"
-local ngx_variable = require ('apicast.policy.ngx_variable')
 local redis_shdict = require('redis_shdict')
 
 local Condition = require('apicast.policy.conditional.condition')
@@ -101,10 +100,6 @@ local function build_condition(config_condition)
   )
 end
 
-local function liquid_context(policy_context)
-  return ngx_variable.available_context(policy_context)
-end
-
 local function build_limiters_and_keys(type, limiters, redis, error_settings, context)
   local res_limiters = {}
   local res_keys = {}
@@ -120,16 +115,14 @@ local function build_limiters_and_keys(type, limiters, redis, error_settings, co
 
     lim.dict = redis or lim.dict
 
-    lim.condition_is_true = function(policy_context)
+    lim.condition_is_true = function(context)
       local condition = build_condition(limiter.condition)
-      local liquid_ctx = liquid_context(policy_context)
-      return condition:evaluate(liquid_ctx)
+      return condition:evaluate(context)
     end
 
     insert(res_limiters, lim)
 
-    local key = limiter.template_string:render(
-      ngx_variable.available_context(context))
+    local key = limiter.template_string:render(context)
     if limiter.key.scope == "global" then
       key = format("%s_%s", type, key)
     else

gateway/src/apicast/template_string.lua

@@ -9,6 +9,8 @@ local setmetatable = setmetatable
 local pairs = pairs
 local format = string.format
 
+local ngx_variable = require('apicast.policy.ngx_variable')
+
 local LiquidTemplateString = {}
 local liquid_template_string_mt = { __index = LiquidTemplateString }
 
@@ -72,8 +74,10 @@ function LiquidTemplateString.new(string)
 end
 
 function LiquidTemplateString:render(context)
+  local available_context = ngx_variable.available_context(context)
+
   return self.template:render(
-    LiquidInterpreterContext:new(context),
+    LiquidInterpreterContext:new(available_context),
     liquid_filter_set,
     liquid_resource_limit
   )

spec/policy/conditional/operation_spec.lua

@@ -1,6 +1,12 @@
 local Operation = require('apicast.policy.conditional.operation')
+local ngx_variable = require('apicast.policy.ngx_variable')
 
 describe('Operation', function()
+  before_each(function()
+    -- avoid stubbing all the ngx.var.* and ngx.req.* in the available context
+    stub(ngx_variable, 'available_context', function(context) return context end)
+  end)
+
   describe('.new', function()
     it('raises error with an unsupported operation', function()
       local res, err = pcall(Operation.new, '1', 'plain', '<>', '1', 'plain')

spec/policy/headers/headers_spec.lua

@@ -1,6 +1,12 @@
 local HeadersPolicy = require('apicast.policy.headers')
+local ngx_variable = require 'apicast.policy.ngx_variable'
 
 describe('Headers policy', function()
+  before_each(function()
+    -- avoid stubbing all the ngx.var.* and ngx.req.* in the available context
+    stub(ngx_variable, 'available_context', function(context) return context end)
+  end)
+
   -- Apply the operations to the same header in all the tests for simplicity
   local header = 'test_header'
 

spec/policy/keycloak_role_check/keycloak_role_check_spec.lua

@@ -1,10 +1,14 @@
 local KeycloakRoleCheckPolicy = require('apicast.policy.keycloak_role_check')
+local ngx_variable = require('apicast.policy.ngx_variable')
 
 describe('Keycloak Role check policy', function()
 
   before_each(function()
     ngx.header = {}
     stub(ngx, 'print')
+
+    -- avoid stubbing all the ngx.var.* and ngx.req.* in the available context
+    stub(ngx_variable, 'available_context', function(context) return context end)
   end)
 
   describe('.access', function()

spec/policy/url_rewriting/url_rewriting_spec.lua

@@ -1,4 +1,5 @@
 local URLRewriting = require('apicast.policy.url_rewriting')
+local ngx_variable = require('apicast.policy.ngx_variable')
 
 -- Mock QueryParams module. In these tests we are only interested in checking
 -- that a QueryParams instance is called with the appropriate params. We do
@@ -16,6 +17,9 @@ describe('URL rewriting policy', function()
 
   before_each(function ()
     stub(QueryParams, 'new', function() return mocked_query_params end)
+
+    -- avoid stubbing all the ngx.var.* and ngx.req.* in the available context
+    stub(ngx_variable, 'available_context', function(context) return context end)
   end)
 
   describe('.rewrite', function()

spec/template_string_spec.lua

@@ -1,6 +1,13 @@
 local TemplateString = require 'apicast.template_string'
+local ngx_variable = require 'apicast.policy.ngx_variable'
+local LinkedList = require('apicast.linked_list')
 
 describe('template string', function()
+  before_each(function()
+    -- Stub the available context to avoid depending on ngx.var.*
+    stub(ngx_variable, 'available_context', function(context) return context end)
+  end)
+
   it('renders plain text', function()
     local plain_text_template = TemplateString.new('{{ a_key }}', 'plain')
     assert.equals('{{ a_key }}', plain_text_template:render())
@@ -11,6 +18,16 @@ describe('template string', function()
     assert.equals('a_value', liquid_template:render({ a_key = 'a_value' }))
   end)
 
+  it('when rendering liquid, it can use the vars exposed in ngx_variable', function()
+    stub(ngx_variable, 'available_context', function(policies_context)
+      local exposed = { a_key_exposed_in_ngx_var = 'a_value' }
+      return LinkedList.readonly(exposed, policies_context)
+    end)
+
+    local liquid_template = TemplateString.new('{{ a_key_exposed_in_ngx_var }}', 'liquid')
+    assert.equals('a_value', liquid_template:render({}))
+  end)
+
   it('can apply liquid filters', function()
     local liquid_template = TemplateString.new('{{ "something" | md5 }}', 'liquid')
     assert.equals(ngx.md5('something'), liquid_template:render({}))

t/apicast-policy-url-rewriting.t

@@ -624,3 +624,60 @@ yay, api backend
 --- error_code: 200
 --- no_error_log
 [error]
+
+=== TEST 12: modify query args using liquid and one of the vars exposed
+The goal of this test is to check that we can use liquid with the vars that are
+not in the policies context, but are exposed by default (uri, host, etc.).
+We are going to use "uri" in this case.
+--- backend
+  location /transactions/authrep.xml {
+    content_by_lua_block {
+      local expected = "service_token=token-value&service_id=42&usage%5Bhits%5D=2&user_key=uk"
+      require('luassert').same(ngx.decode_args(expected), ngx.req.get_uri_args(0))
+    }
+  }
+--- configuration
+{
+  "services": [
+    {
+      "id": 42,
+      "backend_version":  1,
+      "backend_authentication_type": "service_token",
+      "backend_authentication_value": "token-value",
+      "proxy": {
+        "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/",
+        "proxy_rules": [
+          { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 }
+        ],
+        "policy_chain": [
+          {
+            "name": "apicast.policy.url_rewriting",
+            "configuration": {
+              "query_args_commands": [
+                { "op": "push", "arg": "new_arg", "value": "{{ uri }}", "value_type": "liquid" }
+              ]
+            }
+          },
+          { "name": "apicast.policy.apicast" }
+        ]
+      }
+    }
+  ]
+}
+--- upstream
+  location / {
+     content_by_lua_block {
+     ngx.log(ngx.WARN, 'request: ', ngx.var.request)
+
+       require('luassert').are.equal('GET /abc?user_key=uk&new_arg=%2Fabc HTTP/1.1',
+                                     ngx.var.request)
+       ngx.say('yay, api backend');
+     }
+  }
+--- request
+GET /abc?user_key=uk
+--- response_body
+yay, api backend
+--- error_code: 200
+--- no_error_log
+[error]