diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fad83caf..86591ac39 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/gateway/src/apicast/policy/url_rewriting/url_rewriting.lua b/gateway/src/apicast/policy/url_rewriting/url_rewriting.lua index 66bab37a7..4b2bd7237 100644 --- a/gateway/src/apicast/policy/url_rewriting/url_rewriting.lua +++ b/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 diff --git a/spec/policy/url_rewriting/url_rewriting_spec.lua b/spec/policy/url_rewriting/url_rewriting_spec.lua index a1855a69b..02c81bee4 100644 --- a/spec/policy/url_rewriting/url_rewriting_spec.lua +++ b/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) diff --git a/t/apicast-policy-url-rewriting.t b/t/apicast-policy-url-rewriting.t index e99c2f942..8facb9789 100644 --- a/t/apicast-policy-url-rewriting.t +++ b/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" } ]