Merge pull request #641 from 3scale/fix-bug-respect-manifest-url-rewriting

Fix bug in URL rewriting policy that makes it ignore a field of its own manifest [THREESCALE-731]

Author: davidor

CHANGELOG.md

@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Error loading policy chain configuration JSON with null value [PR #626](https://github.com/3scale/apicast/pull/626)
 - Splitted `resolv.conf` in lines,to avoid commented lines  [PR #618](https://github.com/3scale/apicast/pull/618)
 - Avoid `nameserver` repetion from `RESOLVER` variable and `resolv.conf` file [PR #636](https://github.com/3scale/apicast/pull/636)
+- Bug in URL rewriting policy that ignored the `commands` attribute in the policy manifest [PR #641](https://github.com/3scale/apicast/pull/641)
 
 ## Added
 

gateway/src/apicast/policy/url_rewriting/url_rewriting.lua

@@ -58,12 +58,12 @@ end
 --     the URL, it will be the last command applied.
 function _M.new(config)
   local self = new()
-  self.config = config or {}
+  self.commands = (config and config.commands) or {}
   return self
 end
 
 function _M:rewrite()
-  for _, command in ipairs(self.config) do
+  for _, command in ipairs(self.commands) do
     local rewritten = apply_rewrite_command(command)
 
     if rewritten and command['break'] then

spec/policy/url_rewriting/url_rewriting_spec.lua

@@ -12,7 +12,9 @@ describe('URL rewriting policy', function()
 
     it('can rewrite URLs using sub', function()
       local config_with_sub = {
-        { op = 'sub', regex = 'to_be_replaced', replace = 'new' }
+        commands = {
+          { op = 'sub', regex = 'to_be_replaced', replace = 'new' }
+        }
       }
       local url_rewriting = URLRewriting.new(config_with_sub)
 
@@ -24,7 +26,9 @@ describe('URL rewriting policy', function()
 
     it('can rewrite URLs using gsub', function()
       local config_with_gsub = {
-        { op = 'gsub', regex = 'to_be_replaced', replace = 'new' }
+        commands = {
+          { op = 'gsub', regex = 'to_be_replaced', replace = 'new' }
+        }
       }
       local url_rewriting = URLRewriting.new(config_with_gsub)
 
@@ -35,9 +39,11 @@ describe('URL rewriting policy', function()
 
     it('applies the commands in order', function()
       local config_with_several_ops = {
-        { op = 'gsub', regex = 'to_be_replaced', replace = 'abc' },
-        { op = 'gsub', regex = 'abc', replace = 'def' },
-        { op = 'gsub', regex = 'def', replace = 'ghi' }
+        commands = {
+          { op = 'gsub', regex = 'to_be_replaced', replace = 'abc' },
+          { op = 'gsub', regex = 'abc', replace = 'def' },
+          { op = 'gsub', regex = 'def', replace = 'ghi' }
+        }
       }
       local url_rewriting = URLRewriting.new(config_with_several_ops)
 
@@ -48,8 +54,10 @@ describe('URL rewriting policy', function()
 
     it('when there is a break, stops at the first match', function()
       local config_with_break = {
-        { op = 'gsub', regex = 'to_be_replaced', replace = 'abc', ['break'] = '1' },
-        { op = 'gsub', regex = 'abc', replace = 'def' } -- Not applied
+        commands = {
+          { op = 'gsub', regex = 'to_be_replaced', replace = 'abc', ['break'] = '1' },
+          { op = 'gsub', regex = 'abc', replace = 'def' } -- Not applied
+        }
       }
       local url_rewriting = URLRewriting.new(config_with_break)
 
@@ -60,7 +68,9 @@ describe('URL rewriting policy', function()
 
     it('accepts options for the regexes, same as ngx.req.{sub, gsub}', function()
       local config_with_regex_opts = {
-        { op = 'gsub', regex = 'TO_BE_REPLACED', replace = 'new', options = 'i' }
+        commands = {
+          { op = 'gsub', regex = 'TO_BE_REPLACED', replace = 'new', options = 'i' }
+        }
       }
       local url_rewriting = URLRewriting.new(config_with_regex_opts)
 

t/apicast-policy-url-rewriting.t

@@ -29,10 +29,11 @@ __DATA__
           { "name": "apicast.policy.apicast" },
           {
             "name": "apicast.policy.url_rewriting",
-            "configuration":
-              [
+            "configuration": {
+              "commands": [
                 { "op": "sub", "regex": "original", "replace": "new" }
               ]
+            }
           }
         ]
       }
@@ -78,10 +79,11 @@ yay, api backend
           { "name": "apicast.policy.apicast" },
           {
             "name": "apicast.policy.url_rewriting",
-            "configuration":
-              [
+            "configuration": {
+              "commands": [
                 { "op": "gsub", "regex": "original", "replace": "new" }
               ]
+            }
           }
         ]
       }
@@ -128,12 +130,13 @@ Substitutions are applied in the order specified.
           { "name": "apicast.policy.apicast" },
           {
             "name": "apicast.policy.url_rewriting",
-            "configuration":
-              [
+            "configuration": {
+              "commands": [
                 { "op": "gsub", "regex": "aaa", "replace": "bbb", "options": "i" },
                 { "op": "sub", "regex": "bbb", "replace": "ccc" },
                 { "op": "sub", "regex": "ccc", "replace": "ddd" }
               ]
+            }
           }
         ]
       }
@@ -182,13 +185,14 @@ We need to test 2 things:
           { "name": "apicast.policy.apicast" },
           {
             "name": "apicast.policy.url_rewriting",
-            "configuration":
-              [
+            "configuration": {
+              "commands": [
                 { "op": "sub", "regex": "does_not_match", "replace": "a", "break": true },
                 { "op": "sub", "regex": "aaa", "replace": "bbb" },
                 { "op": "sub", "regex": "bbb", "replace": "ccc", "break": true },
                 { "op": "sub", "regex": "ccc", "replace": "ddd" }
               ]
+            }
           }
         ]
       }
@@ -236,10 +240,11 @@ rules.
         "policy_chain": [
           {
             "name": "apicast.policy.url_rewriting",
-            "configuration":
-              [
+            "configuration": {
+              "commands": [
                 { "op": "sub", "regex": "original", "replace": "new" }
               ]
+            }
           },
           { "name": "apicast.policy.apicast" }
         ]