From 0868eed571482df0a49a4dae2f92be40d2b2264e Mon Sep 17 00:00:00 2001 From: Carlo Palmieri Date: Mon, 4 May 2020 10:43:58 +0200 Subject: [PATCH 1/5] Fix: Path base routing match query args Fix THREESCALE-5149 --- gateway/src/apicast/policy/find_service/path_based_finder.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gateway/src/apicast/policy/find_service/path_based_finder.lua b/gateway/src/apicast/policy/find_service/path_based_finder.lua index 7e2150930..048d7755c 100644 --- a/gateway/src/apicast/policy/find_service/path_based_finder.lua +++ b/gateway/src/apicast/policy/find_service/path_based_finder.lua @@ -1,5 +1,6 @@ local mapping_rules_matcher = require 'apicast.mapping_rules_matcher' local escape = require("resty.http.uri_escape") +local pretty = require("pl.pretty") local _M = {} @@ -8,6 +9,7 @@ function _M.find_service(config_store, host) local services = config_store:find_by_host(host) local method = ngx.req.get_method() local uri = escape.escape_uri(ngx.var.uri) + local args = ngx.req.get_uri_args() for s=1, #services do local service = services[s] @@ -17,7 +19,7 @@ function _M.find_service(config_store, host) if hosts[h] == host then local name = service.system_name or service.id ngx.log(ngx.DEBUG, 'service ', name, ' matched host ', hosts[h]) - local matches = mapping_rules_matcher.matches(method, uri, {}, service.rules) + local matches = mapping_rules_matcher.matches(method, uri, args, service.rules) -- matches() also returns the index of the first rule that matched. -- As a future optimization, in the part of the code that calculates -- the usage, we could use this to avoid trying to match again all the From 683446c79387444c113cf37818d00bc421504bf4 Mon Sep 17 00:00:00 2001 From: Carlo Palmieri Date: Wed, 6 May 2020 11:39:37 +0200 Subject: [PATCH 2/5] added test for path based routing with query args --- t/apicast-path-routing.t | 76 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/t/apicast-path-routing.t b/t/apicast-path-routing.t index 05b803539..36d4ead61 100644 --- a/t/apicast-path-routing.t +++ b/t/apicast-path-routing.t @@ -294,3 +294,79 @@ Host: one [200, 200, 200] --- no_error_log [error] + +=== TEST 5: multi service configuration with path based routing and query args in mapping rules +--- env eval +('APICAST_PATH_ROUTING' => '1') +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 2, + "proxy": { + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api-backend/foo/", + "hosts": [ + "same" + ], + "backend_authentication_type": "service_token", + "backend_authentication_value": "service-one", + "proxy_rules": [ + { + "pattern": "/one/test", + "http_method": "GET", + "metric_system_name": "one", + "delta": 1 + } + ] + } + }, + { + "id": 21, + "backend_version": 2, + "proxy": { + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api-backend/bar/", + "hosts": [ + "same" + ], + "backend_authentication_type": "service_token", + "backend_authentication_value": "service-two", + "proxy_rules": [ + { + "pattern": "/two/test?foo={bar}", + "http_method": "GET", + "metric_system_name": "two", + "delta": 2, + "querystring_parameters": { + "foo": "{bar}" + } + } + ] + } + } + ] +} +--- backend + location /transactions/authrep.xml { + content_by_lua_block { ngx.exit(200) } + } +--- upstream + location ~ /api-backend(/.+) { + echo 'yay, api backend: $1'; + } +--- request eval +[ + "GET /two/test?app_id=two-id&app_key=two-key&foo=bar", + "GET /two/test?app_id=two-id&app_key=two-key" +] +--- more_headers eval +["Host: same", "Host: same"] +--- response_body eval +[ + "yay, api backend: /bar/two/test\x{0a}", + "No Mapping Rule matched" +] +--- error_code eval +[200, 404] +--- no_error_log +[error] \ No newline at end of file From fe252510d639e79ecfda4d7f14f794871402caaf Mon Sep 17 00:00:00 2001 From: Carlo Palmieri Date: Wed, 6 May 2020 11:41:43 +0200 Subject: [PATCH 3/5] removed unused library --- gateway/src/apicast/policy/find_service/path_based_finder.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/gateway/src/apicast/policy/find_service/path_based_finder.lua b/gateway/src/apicast/policy/find_service/path_based_finder.lua index 048d7755c..2b71242b8 100644 --- a/gateway/src/apicast/policy/find_service/path_based_finder.lua +++ b/gateway/src/apicast/policy/find_service/path_based_finder.lua @@ -1,6 +1,5 @@ local mapping_rules_matcher = require 'apicast.mapping_rules_matcher' local escape = require("resty.http.uri_escape") -local pretty = require("pl.pretty") local _M = {} From 852fbf05c33b48274b0a7ef34eb7534c23cd9516 Mon Sep 17 00:00:00 2001 From: Carlo Palmieri Date: Wed, 6 May 2020 11:49:31 +0200 Subject: [PATCH 4/5] added PR 1190 to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95a6ad5c1..1fff793cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fixed issues with liquid replaces [THREESCALE-4937](https://issues.jboss.org/browse/THREESCALE-4937) [PR #1185](https://github.com/3scale/APIcast/pull/1185) - Fixed issues with HTTPS_PROXY and large bodies [THREESCALE-3863](https://issues.jboss.org/browse/THREESCALE-3863) [PR #1191](https://github.com/3scale/APIcast/pull/1191) - +- Fixed issues with path routing and query args [THREESCALE-5149](https://issues.redhat.com/browse/THREESCALE-5149) [PR #1190](https://github.com/3scale/APIcast/pull/1190) ### Added From 1daeb1500113ca68aaef71f759066ac638c887bd Mon Sep 17 00:00:00 2001 From: Carlo Palmieri Date: Wed, 13 May 2020 13:36:51 +0200 Subject: [PATCH 5/5] Added unittest test for path based routing with query args --- .../find_service/path_based_finder_spec.lua | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/spec/policy/find_service/path_based_finder_spec.lua b/spec/policy/find_service/path_based_finder_spec.lua index cfcbcbb25..e409c88af 100644 --- a/spec/policy/find_service/path_based_finder_spec.lua +++ b/spec/policy/find_service/path_based_finder_spec.lua @@ -100,5 +100,77 @@ describe('PathBasedFinder', function() assert.is_nil(service_found) end) + + it('returns the service if it matches the path, the host and the querystring params', function() + ngx.var = { uri = '/abc' } + stub(ngx.req, 'get_uri_args', function() return { foo = 'bar' } end) + stub(ngx.req, 'get_method', function() return 'GET' end) + + local host = 'example.com' + + local service_not_matching = Configuration.parse_service({ + id = 1, + proxy = { + hosts = { host }, + proxy_rules = { { pattern = '/dont_match', http_method = 'GET', + metric_system_name = 'hits', delta = 2 } } + } + }) + + local service_matching = Configuration.parse_service({ + id = 2, + proxy = { + hosts = { host }, + proxy_rules = { { pattern = '/abc', + querystring_parameters = { foo = '{bar}' }, + http_method = 'GET', metric_system_name = 'hits', + delta = 2 } } + } + }) + + local services = { service_not_matching, service_matching } + local config_store = ConfigurationStore.new(nil, config_store_opts) + config_store:store({ services = services }) + + local service_found = PathBasedFinder.find_service(config_store, host) + + assert.equal(service_matching, service_found) + end) + + it('returns nil if it matches the path, the host but not the querystring params', function() + ngx.var = { uri = '/abc' } + stub(ngx.req, 'get_uri_args', function() return { } end) + stub(ngx.req, 'get_method', function() return 'GET' end) + + local host = 'example.com' + + local service_not_matching = Configuration.parse_service({ + id = 1, + proxy = { + hosts = { host }, + proxy_rules = { { pattern = '/dont_match', http_method = 'GET', + metric_system_name = 'hits', delta = 2 } } + } + }) + + local service_matching = Configuration.parse_service({ + id = 2, + proxy = { + hosts = { host }, + proxy_rules = { { pattern = '/abc', + querystring_parameters = { foo = 'bar' }, + http_method = 'GET', metric_system_name = 'hits', + delta = 2 } } + } + }) + + local services = { service_not_matching, service_matching } + local config_store = ConfigurationStore.new(nil, config_store_opts) + config_store:store({ services = services }) + + local service_found = PathBasedFinder.find_service(config_store, host) + + assert.is_nil(service_found) + end) end) end)