diff --git a/CHANGELOG.md b/CHANGELOG.md index 63fda8b8f..4d8116e31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Allow to use capture function in liquid templates. [PR #1107](https://github.com/3scale/APIcast/pull/1107), [THREESCALE-1911](https://issues.jboss.org/browse/THREESCALE-1911) - OAuth 2.0 MTLS policy [PR #1101](https://github.com/3scale/APIcast/pull/1101) [Issue #1003](https://github.com/3scale/APIcast/issues/1003) - Add an option to enable keepalive_timeout on gateway [THREESCALE-2886](https://issues.jboss.org/browse/THREESCALE-2886) [PR #1106](https://github.com/3scale/APIcast/pull/1106) -- Added a new replace path option in routing policy [THREESCALE-3512](https://issues.jboss.org/browse/THREESCALE-3512) [PR #1119](https://github.com/3scale/APIcast/pull/1119) [PR #1121](https://github.com/3scale/APIcast/pull/1121) +- Added a new replace path option in routing policy [THREESCALE-3512](https://issues.jboss.org/browse/THREESCALE-3512) [PR #1119](https://github.com/3scale/APIcast/pull/1119) [PR #1121](https://github.com/3scale/APIcast/pull/1121) [PR #1122](https://github.com/3scale/APIcast/pull/1122) ### Fixed diff --git a/gateway/src/apicast/policy/routing/upstream_selector.lua b/gateway/src/apicast/policy/routing/upstream_selector.lua index a7196cf8a..3f355e8e6 100644 --- a/gateway/src/apicast/policy/routing/upstream_selector.lua +++ b/gateway/src/apicast/policy/routing/upstream_selector.lua @@ -33,7 +33,7 @@ function _M.select(_, rules, context) upstream:use_host_header(rule.host_header) end if rule.replace_path then - upstream:set_path(rule.replace_path:render(context)) + upstream:append_path(rule.replace_path:render(context)) -- Set uri as nil if not will be appended to the upstream ngx.req.set_uri("/") end diff --git a/gateway/src/apicast/upstream.lua b/gateway/src/apicast/upstream.lua index 4c820ce23..8889a87d1 100644 --- a/gateway/src/apicast/upstream.lua +++ b/gateway/src/apicast/upstream.lua @@ -117,6 +117,20 @@ function _M:set_path(path) self.uri.path, self.uri.query = url_helper.split_path(path) end +function _M:append_path(path) + local tmp_path, tmp_query = url_helper.split_path(path) + if not self.uri.path then + self.uri.path = "/" + end + self.uri.path = resty_url.join(self.uri.path, tmp_path) + + -- If query is already present, do not need to add more. + if tmp_query and tmp_query ~= "" then + return + end + self.uri.query = tmp_query +end + --- Rewrite request Host header to what is provided in the argument or in the URL. function _M:rewrite_request() local uri = self.uri diff --git a/spec/policy/routing/upstream_selector_spec.lua b/spec/policy/routing/upstream_selector_spec.lua index 30e87e3e0..5f9736f42 100644 --- a/spec/policy/routing/upstream_selector_spec.lua +++ b/spec/policy/routing/upstream_selector_spec.lua @@ -142,7 +142,7 @@ describe('UpstreamSelector', function() before_each(function() stub(ngx_variable, 'available_context', function(context) return context end) stub(ngx.req, 'set_uri', function(_) return true end) - stub(apicast_upstream, 'set_path', function(_) return true end) + stub(apicast_upstream, 'append_path', function(_) return true end) end) it('is not set', function() @@ -170,8 +170,8 @@ describe('UpstreamSelector', function() assert.spy(ngx.req.set_uri).was_called() assert.spy(ngx.req.set_uri).was.called_with("/") - assert.spy(apicast_upstream.set_path).was_called() - assert.spy(apicast_upstream.set_path).was.called_with(upstream, "") + assert.spy(apicast_upstream.append_path).was_called() + assert.spy(apicast_upstream.append_path).was.called_with(upstream, "") end) it('is correctly set', function() @@ -187,8 +187,8 @@ describe('UpstreamSelector', function() assert.spy(ngx.req.set_uri).was_called() assert.spy(ngx.req.set_uri).was.called_with("/") - assert.spy(apicast_upstream.set_path).was_called() - assert.spy(apicast_upstream.set_path).was.called_with(upstream, "bar") + assert.spy(apicast_upstream.append_path).was_called() + assert.spy(apicast_upstream.append_path).was.called_with(upstream, "bar") end) end) end) diff --git a/spec/upstream_spec.lua b/spec/upstream_spec.lua index 6947e571e..8a677026c 100644 --- a/spec/upstream_spec.lua +++ b/spec/upstream_spec.lua @@ -66,6 +66,22 @@ describe('Upstream', function() end) end) + + describe(':append_path', function() + it('return valid path when is not set', function() + local up = Upstream.new('http://host:8090') + up:append_path("/test") + assert.same(up.uri.path, "/test") + end) + + it('leading slash is removed', function() + local up = Upstream.new('http://host:8090/') + up:append_path("/test/") + assert.same(up.uri.path, "/test/") + end) + + end) + local function stub_ngx_request() ngx.var = { } diff --git a/t/apicast-policy-routing.t b/t/apicast-policy-routing.t index 7194eba55..2babf3379 100644 --- a/t/apicast-policy-routing.t +++ b/t/apicast-policy-routing.t @@ -1741,3 +1741,56 @@ yay, api backend --- error_code: 200 --- no_error_log [error] + +=== TEST 27: replace path does not overwrite url base path. +--- configuration +{ + "services": [ + { + "id": 42, + "proxy": { + "policy_chain": [ + { + "name": "apicast.policy.routing", + "configuration": { + "rules": [ + { + "url": "http://test:$TEST_NGINX_SERVER_PORT/basePath/", + "replace_path": "{{original_request.path }}-test", + "condition": { + "operations": [ + { + "match": "liquid", + "liquid_value": "{{ original_request.path }}", + "op": "matches", + "value": "bridge" + } + ] + } + } + ] + } + }, + { + "name": "apicast.policy.echo" + } + ] + } + } + ] +} +--- upstream + location ~* /basePath/bridge-1-test$ { + content_by_lua_block { + local args = ngx.req.get_uri_args() + require('luassert').equals('foo', args["one"]) + ngx.say('yay, api backend'); + } + } +--- request +GET /bridge-1?one=foo +--- response_body +yay, api backend +--- error_code: 200 +--- no_error_log +[error]