diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e32c870f..874ab2f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +## [3.2.0-rc2] - 2018-05-11 + +### Added + +- Default value for the `caching_type` attribute of the caching policy config schema [#691](https://github.com/3scale/apicast/pull/691), [THREESCALE-845](https://issues.jboss.org/browse/THREESCALE-845) + +### Fixed + +- Fixed set of valid values for the exit param of the Echo policy [PR #684](https://github.com/3scale/apicast/pull/684/) + +### Changed + +- The schema of the rate-limit policy has been adapted so it can be rendered by `react-jsonschema-form`, a library used in the 3scale UI. This is a breaking change. [PR #696](https://github.com/3scale/apicast/pull/696), [THREESCALE-888](https://issues.jboss.org/browse/THREESCALE-888) +- The upstream policy now performs the rule matching in the rewrite phase. This allows combining it with the URL rewriting policy – upstream policy regex will be matched against the original path if upstream policy is placed before URL rewriting in the policy chain, and against the rewritten path otherwise [PR #690](https://github.com/3scale/apicast/pull/690), [THREESCALE-852](https://issues.jboss.org/browse/THREESCALE-852) + ## [3.2.0-rc1] - 2018-04-24 ### Added @@ -382,7 +397,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed - Major rewrite using JSON configuration instead of code generation. -[Unreleased]: https://github.com/3scale/apicast/compare/v3.2.0-rc1...HEAD +[Unreleased]: https://github.com/3scale/apicast/compare/v3.2.0-rc2...HEAD [2.0.0]: https://github.com/3scale/apicast/compare/v0.2...v2.0.0 [3.0.0-alpha1]: https://github.com/3scale/apicast/compare/v2.0.0...v3.0.0-alpha1 [3.0.0-alpha2]: https://github.com/3scale/apicast/compare/v3.0.0-alpha1...v3.0.0-alpha2 @@ -402,3 +417,4 @@ and this project adheres to [Semantic Versioning](http://semver.org/). [3.2.0-beta2]: https://github.com/3scale/apicast/compare/v3.2.0-beta1...v3.2.0-beta2 [3.2.0-beta3]: https://github.com/3scale/apicast/compare/v3.2.0-beta2...v3.2.0-beta3 [3.2.0-rc1]: https://github.com/3scale/apicast/compare/v3.2.0-beta3...v3.2.0-rc1 +[3.2.0-rc2]: https://github.com/3scale/apicast/compare/v3.2.0-rc1...v3.2.0-rc2 diff --git a/doc/policies.md b/doc/policies.md index c0138032d..13f77a8d8 100644 --- a/doc/policies.md +++ b/doc/policies.md @@ -67,12 +67,6 @@ mapping rules of APIcast will be applied against the rewritten path. However, if the URL policy appears after the APIcast one, the mapping rules will be applied against the original path. -Another example, suppose that we combine the upstream policy with the URL -rewriting one. The upstream policy acts on the content phase, whereas the URL -rewriting one acts on the rewrite phase. This means that the upstream policy -will always take into account the rewritten path instead of the original one, -regardless of the position of the policies in the chain. - ### Types There are two types of policy chains in APIcast: per-service chains and a diff --git a/examples/policies/rate_limit_configuration.lua b/examples/policies/rate_limit_configuration.lua index 9f730de82..7ae6ed892 100644 --- a/examples/policies/rate_limit_configuration.lua +++ b/examples/policies/rate_limit_configuration.lua @@ -1,26 +1,28 @@ local policy_chain = require('apicast.policy_chain').default() local rate_limit_policy = require('apicast.policy.rate_limit').new({ - limiters = { - { - name = "connections", - key = {name = "limit1"}, - conn = 20, - burst = 10, - delay = 0.5 + connection_limiters = { + { + key = {name = "limit1"}, + conn = 20, + burst = 10, + delay = 0.5 + } }, - { - name = "leaky_bucket", - key = {name = "limit2"}, - rate = 18, - burst = 9 + leaky_bucket_limiters = { + { + key = {name = "limit2"}, + rate = 18, + burst = 9 + } + }, + fixed_window_limiters = { + { + key = {name = "limit3"}, + count = 10, + window = 10 + } }, - { - name = "fixed_window", - key = {name = "limit3"}, - count = 10, - window = 10 - }}, redis_url = "redis://localhost:6379/1" }) diff --git a/gateway/src/apicast/policy/caching/apicast-policy.json b/gateway/src/apicast/policy/caching/apicast-policy.json index b52701a17..928a3a5d0 100644 --- a/gateway/src/apicast/policy/caching/apicast-policy.json +++ b/gateway/src/apicast/policy/caching/apicast-policy.json @@ -42,7 +42,8 @@ "enum": ["none"], "title": "Disable caching." } - ] + ], + "default": "none" } } } diff --git a/gateway/src/apicast/policy/echo/apicast-policy.json b/gateway/src/apicast/policy/echo/apicast-policy.json index 30e693d56..ccb0385e4 100644 --- a/gateway/src/apicast/policy/echo/apicast-policy.json +++ b/gateway/src/apicast/policy/echo/apicast-policy.json @@ -22,7 +22,7 @@ "title": "Interrupt the processing of the request." }, { - "enum": ["set"], + "enum": ["phase"], "title": "Skip only the rewrite phase." } ] diff --git a/gateway/src/apicast/policy/rate_limit/apicast-policy.json b/gateway/src/apicast/policy/rate_limit/apicast-policy.json index be933890b..9a7e0a1df 100644 --- a/gateway/src/apicast/policy/rate_limit/apicast-policy.json +++ b/gateway/src/apicast/policy/rate_limit/apicast-policy.json @@ -7,214 +7,196 @@ "configuration": { "type": "object", "properties": { - "limiters": { - "description": "List of limiters to be applied", + "connection_limiters": { "type": "array", "items": { - "anyOf": [{ - "type": "object", - "properties": { - "name": { - "type": "string", - "enum": ["connections"], - "description": "Limiting request concurrency (or concurrent connections)" - }, - "key": { - "description": "The key corresponding to the limiter object", - "type": "object", - "properties": { - "name": { - "type": "string", - "description": "The name of the key, must be unique in the scope" - }, - "scope": { - "type": "string", - "description": "Scope of the key", - "default": "global", - "oneOf": [{ - "enum": ["global"], - "description": "Global scope, affecting to all services" - }, { - "enum": ["service"], - "description": "Service scope, affecting to one service" - }] - }, - "service_name": { - "type": "string", - "description": "Name of service, necessary for service scope" - } + "type": "object", + "properties": { + "key": { + "description": "The key corresponding to the limiter object", + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "The name of the key, must be unique in the scope" + }, + "scope": { + "type": "string", + "description": "Scope of the key", + "default": "global", + "oneOf": [{ + "enum": ["global"], + "description": "Global scope, affecting to all services" + }, { + "enum": ["service"], + "description": "Service scope, affecting to one service" + }] + }, + "service_name": { + "type": "string", + "description": "Name of service, necessary for service scope" } - }, - "conn": { - "type": "integer", - "description": "The maximum number of concurrent requests allowed", - "exclusiveminimum": 0 - }, - "burst": { - "type": "integer", - "description": "The number of excessive concurrent requests (or connections) allowed to be delayed", - "minimum": 0 - }, - "delay": { - "type": "number", - "description": "The default processing latency of a typical connection (or request)", - "exclusiveMinimum": 0 } + }, + "conn": { + "type": "integer", + "description": "The maximum number of concurrent requests allowed", + "exclusiveminimum": 0 + }, + "burst": { + "type": "integer", + "description": "The number of excessive concurrent requests (or connections) allowed to be delayed", + "minimum": 0 + }, + "delay": { + "type": "number", + "description": "The default processing latency of a typical connection (or request)", + "exclusiveMinimum": 0 } - }, { - "type": "object", - "properties": { - "name": { - "type": "string", - "enum": ["leaky_bucket"], - "description": "Limiting request rate" - }, - "key": { - "description": "The key corresponding to the limiter object", - "type": "object", - "properties": { - "name": { - "type": "string", - "description": "The name of the key, must be unique in the scope" - }, - "scope": { - "type": "string", - "description": "Scope of the key", - "default": "global", - "oneOf": [{ - "enum": ["global"], - "description": "Global scope, affecting to all services" - }, { - "enum": ["service"], - "description": "Service scope, affecting to one service" - }] - }, - "service_name": { - "type": "string", - "description": "Name of service, necessary for service scope" - } + } + } + }, + "leaky_bucket_limiters": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "The key corresponding to the limiter object", + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "The name of the key, must be unique in the scope" + }, + "scope": { + "type": "string", + "description": "Scope of the key", + "default": "global", + "oneOf": [{ + "enum": ["global"], + "description": "Global scope, affecting to all services" + }, { + "enum": ["service"], + "description": "Service scope, affecting to one service" + }] + }, + "service_name": { + "type": "string", + "description": "Name of service, necessary for service scope" } - }, - "rate": { - "type": "integer", - "description": "The specified request rate (number per second) threshold", - "exclusiveMinimum": 0 - }, - "burst": { - "type": "integer", - "description": "The number of excessive requests per second allowed to be delayed", - "minimum": 0 } + }, + "rate": { + "type": "integer", + "description": "The specified request rate (number per second) threshold", + "exclusiveMinimum": 0 + }, + "burst": { + "type": "integer", + "description": "The number of excessive requests per second allowed to be delayed", + "minimum": 0 } - }, { - "type": "object", - "properties": { - "name": { - "type": "string", - "enum": ["fixed_window"], - "description": "Limiting request counts" - }, - "key": { - "description": "The key corresponding to the limiter object", - "type": "object", - "properties": { - "name": { - "type": "string", - "description": "The name of the key, must be unique in the scope" - }, - "scope": { - "type": "string", - "description": "Scope of the key", - "default": "global", - "oneOf": [{ - "enum": ["global"], - "description": "Global scope, affecting to all services" - }, { - "enum": ["service"], - "description": "Service scope, affecting to one service" - }] - }, - "service_name": { - "type": "string", - "description": "Name of service, necessary for service scope" - } + } + } + }, + "fixed_window_limiters": { + "type": "array", + "items": { + "type": "object", + "properties": { + "key": { + "description": "The key corresponding to the limiter object", + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "The name of the key, must be unique in the scope" + }, + "scope": { + "type": "string", + "description": "Scope of the key", + "default": "global", + "oneOf": [{ + "enum": ["global"], + "description": "Global scope, affecting to all services" + }, { + "enum": ["service"], + "description": "Service scope, affecting to one service" + }] + }, + "service_name": { + "type": "string", + "description": "Name of service, necessary for service scope" } - }, - "count": { - "type": "integer", - "description": "The specified number of requests threshold", - "exclusiveMinimum": 0 - }, - "window": { - "type": "integer", - "description": "The time window in seconds before the request count is reset", - "exclusiveMinimum": 0 } + }, + "count": { + "type": "integer", + "description": "The specified number of requests threshold", + "exclusiveMinimum": 0 + }, + "window": { + "type": "integer", + "description": "The time window in seconds before the request count is reset", + "exclusiveMinimum": 0 } - }] + } } }, "redis_url": { "description": "URL of Redis", "type": "string" }, - "error_settings": { - "description": "List of error settings", - "type": "array", - "items": { - "anyOf": [{ - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": ["limits_exceeded"], - "description": "The error setting when requests over the limit" - }, - "status_code": { - "type": "integer", - "description": "The status code when requests over the limit", - "default": 429 - }, - "error_handling": { - "type": "string", - "description": "How to handle an error", - "default": "exit", - "oneOf": [{ - "enum": ["exit"], - "description": "Respond with an error" - }, { - "enum": ["log"], - "description": "Let the request go through and only output logs" - }] - } - } - }, { - "type": "object", - "properties": { - "type": { - "type": "string", - "enum": ["configuration_issue"], - "description": "The error setting when there is some configuration issue" - }, - "status_code": { - "type": "integer", - "description": "The status code when there is some configuration issue", - "default": 500 + "limits_exceeded_error": { + "type": "object", + "properties": { + "status_code": { + "type": "integer", + "description": "The status code when requests over the limit", + "default": 429 + }, + "error_handling": { + "type": "string", + "description": "How to handle an error", + "default": "exit", + "oneOf": [{ + "enum": ["exit"], + "description": "Respond with an error" + }, { + "enum": ["log"], + "description": "Let the request go through and only output logs" + }] + } + } + }, + "configuration_error": { + "type": "object", + "properties": { + "status_code": { + "type": "integer", + "description": "The status code when there is some configuration issue", + "default": 500 + }, + "error_handling": { + "type": "string", + "description": "How to handle an error", + "default": "exit", + "oneOf": [ + { + "enum": [ + "exit" + ], + "description": "Respond with an error" }, - "error_handling": { - "type": "string", - "description": "How to handle an error", - "default": "exit", - "oneOf": [{ - "enum": ["exit"], - "description": "Respond with an error" - }, { - "enum": ["log"], - "description": "Let the request go through and only output logs" - }] + { + "enum": [ + "log" + ], + "description": "Let the request go through and only output logs" } - } - }] + ] + } } } } diff --git a/gateway/src/apicast/policy/rate_limit/rate_limit.lua b/gateway/src/apicast/policy/rate_limit/rate_limit.lua index e839f8944..bba13c1a2 100644 --- a/gateway/src/apicast/policy/rate_limit/rate_limit.lua +++ b/gateway/src/apicast/policy/rate_limit/rate_limit.lua @@ -12,6 +12,10 @@ local tonumber = tonumber local next = next local shdict_key = 'limiter' +local insert = table.insert +local ipairs = ipairs +local unpack = table.unpack + local new = _M.new local traffic_limiters = { @@ -78,37 +82,75 @@ local function error(error_settings, type) end end -local function init_error_settings(config_error_settings) +local function init_error_settings(limits_exceeded_error, configuration_error) local error_settings = default_error_settings - if config_error_settings then - for _, error_setting in pairs(config_error_settings) do - if error_setting.type then - if error_setting.status_code then - error_settings[error_setting.type]["status_code"] = error_setting.status_code - end - if error_setting.error_handling then - error_settings[error_setting.type]["error_handling"] = error_setting.error_handling - end - end + + if limits_exceeded_error then + if limits_exceeded_error.status_code then + error_settings.limits_exceeded.status_code = limits_exceeded_error.status_code + end + + if limits_exceeded_error.error_handling then + error_settings.limits_exceeded.error_handling = limits_exceeded_error.error_handling end end + + if configuration_error then + if configuration_error.status_code then + error_settings.configuration_issue.status_code = configuration_error.status_code + end + + if configuration_error.error_handling then + error_settings.configuration_issue.error_handling = configuration_error.error_handling + end + end + return error_settings end +local function build_limiters_and_keys(type, limiters, redis, error_settings) + local res_limiters = {} + local res_keys = {} + + for _, limiter in ipairs(limiters) do + local lim, initerr = traffic_limiters[type](limiter) + if not lim then + ngx.log(ngx.ERR, "unknown limiter: ", type, ", err: ", initerr) + error(error_settings, "configuration_issue") + return + end + + lim.dict = redis or lim.dict + + insert(res_limiters, lim) + + local key + if limiter.key.scope == "service" then + key = limiter.key.service_name.."_"..type.."_"..limiter.key.name + else + key = type.."_"..limiter.key.name + end + + insert(res_keys, key) + end + + return res_limiters, res_keys +end + function _M.new(config) local self = new() self.config = config or {} - self.limiters = config.limiters + self.connection_limiters = config.connection_limiters or {} + self.leaky_bucket_limiters = config.leaky_bucket_limiters or {} + self.fixed_window_limiters = config.fixed_window_limiters or {} self.redis_url = config.redis_url - self.error_settings = init_error_settings(config.error_settings) + self.error_settings = init_error_settings( + config.limits_exceeded_error, config.configuration_error) return self end function _M:access() - local limiters = {} - local keys = {} - local red if self.redis_url then local rederr @@ -120,27 +162,29 @@ function _M:access() end end - for _, limiter in ipairs(self.limiters) do - local lim, initerr = traffic_limiters[limiter.name](limiter) - if not lim then - ngx.log(ngx.ERR, "unknown limiter: ", limiter.name, ", err: ", initerr) - error(self.error_settings, "configuration_issue") - return - end + local conn_limiters, conn_keys = build_limiters_and_keys( + 'connections', self.connection_limiters, red, self.error_settings) - lim.dict = red or lim.dict + local leaky_bucket_limiters, leaky_bucket_keys = build_limiters_and_keys( + 'leaky_bucket', self.leaky_bucket_limiters, red, self.error_settings) - table.insert(limiters, lim) + local fixed_window_limiters, fixed_window_keys = build_limiters_and_keys( + 'fixed_window', self.fixed_window_limiters, red, self.error_settings) - local key - if limiter.key.scope == "service" then - key = limiter.key.service_name.."_"..limiter.name.."_"..limiter.key.name - else - key = limiter.name.."_"..limiter.key.name + local limiters = {} + local limiter_groups = { conn_limiters, leaky_bucket_limiters, fixed_window_limiters } + for _, limiter_group in ipairs(limiter_groups) do + if #limiter_group > 0 then + insert(limiters, unpack(limiter_group)) end + end - table.insert(keys, key) - + local keys = {} + local keys_groups = { conn_keys, leaky_bucket_keys, fixed_window_keys } + for _, keys_group in ipairs(keys_groups) do + if #keys_group > 0 then + insert(keys, unpack(keys_group)) + end end local states = {} diff --git a/gateway/src/apicast/policy/upstream/apicast-policy.json b/gateway/src/apicast/policy/upstream/apicast-policy.json index 94c0624af..f09fccf24 100644 --- a/gateway/src/apicast/policy/upstream/apicast-policy.json +++ b/gateway/src/apicast/policy/upstream/apicast-policy.json @@ -4,10 +4,10 @@ "summary": "Allows to modify the upstream URL of the request based on its path.", "description": ["This policy allows to modify the upstream URL (scheme, host and port) of the request based on its path. ", - "It accepts regular expressions that, when matched against the request path, ", - "replaces it with a given string. \n", + "It accepts regular expressions and, when matched against the request path, ", + "replaces the upstream URL with a given string. \n", "When combined with the APIcast policy, the upstream policy should be ", - "placed before it in the policy chain"], + "placed before it in the policy chain."], "version": "builtin", "configuration": { "type": "object", diff --git a/gateway/src/apicast/policy/upstream/upstream.lua b/gateway/src/apicast/policy/upstream/upstream.lua index 35b9a8dee..6fbd58833 100644 --- a/gateway/src/apicast/policy/upstream/upstream.lua +++ b/gateway/src/apicast/policy/upstream/upstream.lua @@ -65,20 +65,29 @@ function _M.new(config) return self end -function _M:content(context) +function _M:rewrite(context) local req_uri = ngx.var.uri for _, rule in ipairs(self.rules) do if match(req_uri, rule.regex) then ngx.log(ngx.DEBUG, 'upstream policy uri: ', req_uri, ' regex: ', rule.regex, ' match: true') - context.upstream_changed = true - return change_upstream(rule.url) - elseif ngx.config.debug then + context.new_upstream = rule.url + break + else ngx.log(ngx.DEBUG, 'upstream policy uri: ', req_uri, ' regex: ', rule.regex, ' match: false') end end end +function _M.content(_, context) + local new_upstream = context.new_upstream + + if new_upstream then + context.upstream_changed = true + return change_upstream(new_upstream) + end +end + function _M.balancer(_, context) if context.upstream_changed then balancer.call() diff --git a/gateway/src/apicast/version.lua b/gateway/src/apicast/version.lua index 70b64a88a..c14562c47 100644 --- a/gateway/src/apicast/version.lua +++ b/gateway/src/apicast/version.lua @@ -1 +1 @@ -return "3.2.0-rc1" +return "3.2.0-rc2" diff --git a/spec/policy/echo/echo_spec.lua b/spec/policy/echo/echo_spec.lua new file mode 100644 index 000000000..e1b31f5b9 --- /dev/null +++ b/spec/policy/echo/echo_spec.lua @@ -0,0 +1,28 @@ +local EchoPolicy = require('apicast.policy.echo') + +describe('Echo policy', function() + describe('.rewrite', function() + describe('when configured with exit=request', function() + it('stops processing the request and returns with the configured status', function() + local ngx_exit_spy = spy.on(ngx, 'exit') + local status = 200 + local echo = EchoPolicy.new({ status = status, exit = 'request' }) + + echo:rewrite() + + assert.spy(ngx_exit_spy).was_called_with(status) + end) + end) + + describe('when configured with exit=phase', function() + it('skips the current phase', function() + local ngx_exit_spy = spy.on(ngx, 'exit') + local echo = EchoPolicy.new({ status = 200, exit = 'phase' }) + + echo:rewrite() + + assert.spy(ngx_exit_spy).was_called_with(0) + end) + end) + end) +end) diff --git a/spec/policy/rate_limit/rate_limit_spec.lua b/spec/policy/rate_limit/rate_limit_spec.lua index 8a34457b5..840e81008 100644 --- a/spec/policy/rate_limit/rate_limit_spec.lua +++ b/spec/policy/rate_limit/rate_limit_spec.lua @@ -57,10 +57,14 @@ describe('Rate limit policy', function() describe('.access', function() it('success with multiple limiters', function() local config = { - limiters = { - {name = "connections", key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5}, - {name = "leaky_bucket", key = {name = 'test2'}, rate = 18, burst = 9}, - {name = "fixed_window", key = {name = 'test3'}, count = 10, window = 10} + connection_limiters = { + { key = { name = 'test1' }, conn = 20, burst = 10, delay = 0.5 } + }, + leaky_bucket_limiters = { + { key = { name = 'test2' }, rate = 18, burst = 9 } + }, + fixed_window_limiters = { + { key = {name = 'test3'}, count = 10, window = 10 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } @@ -70,10 +74,14 @@ describe('Rate limit policy', function() it('no redis url', function() local config = { - limiters = { - {name = "connections", key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5}, - {name = "leaky_bucket", key = {name = 'test2'}, rate = 18, burst = 9}, - {name = "fixed_window", key = {name = 'test3'}, count = 10, window = 10} + connection_limiters = { + { key = { name = 'test1' }, conn = 20, burst = 10, delay = 0.5 } + }, + leaky_bucket_limiters = { + { key = { name = 'test2' }, rate = 18, burst = 9 } + }, + fixed_window_limiters = { + { key = { name = 'test3' }, count = 10, window = 10 } } } local rate_limit_policy = RateLimitPolicy.new(config) @@ -82,8 +90,8 @@ describe('Rate limit policy', function() it('invalid redis url', function() local config = { - limiters = { - {name = "connections", key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5} + connection_limiters = { + { key = { name = 'test1' }, conn = 20, burst = 10, delay = 0.5 } }, redis_url = 'redis://invalidhost:'..redis_port..'/1' } @@ -94,8 +102,8 @@ describe('Rate limit policy', function() it('rejected (conn)', function() local config = { - limiters = { - {name = "connections", key = {name = 'test1'}, conn = 1, burst = 0, delay = 0.5} + connection_limiters = { + { key = { name = 'test1' }, conn = 1, burst = 0, delay = 0.5 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } @@ -107,8 +115,8 @@ describe('Rate limit policy', function() it('rejected (req)', function() local config = { - limiters = { - {name = "leaky_bucket", key = {name = 'test2'}, rate = 1, burst = 0} + leaky_bucket_limiters = { + { key = { name = 'test2' }, rate = 1, burst = 0 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } @@ -120,8 +128,8 @@ describe('Rate limit policy', function() it('rejected (count)', function() local config = { - limiters = { - {name = "fixed_window", key = {name = 'test3'}, count = 1, window = 10} + fixed_window_limiters = { + { key = { name = 'test3' }, count = 1, window = 10 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } @@ -133,8 +141,8 @@ describe('Rate limit policy', function() it('delay (conn)', function() local config = { - limiters = { - {name = "connections", key = {name = 'test1'}, conn = 1, burst = 1, delay = 2} + connection_limiters = { + { key = {name = 'test1'}, conn = 1, burst = 1, delay = 2 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } @@ -146,8 +154,8 @@ describe('Rate limit policy', function() it('delay (req)', function() local config = { - limiters = { - {name = "leaky_bucket", key = {name = 'test2', scope = 'global'}, rate = 1, burst = 1} + leaky_bucket_limiters = { + { key = { name = 'test2', scope = 'global' }, rate = 1, burst = 1 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } @@ -159,10 +167,9 @@ describe('Rate limit policy', function() it('delay (req) service scope', function() local config = { - limiters = { + leaky_bucket_limiters = { { - name = "leaky_bucket", - key = {name = 'test4', scope = 'service', service_name = 'bank_A'}, + key = { name = 'test4', scope = 'service', service_name = 'bank_A' }, rate = 1, burst = 1 } @@ -179,8 +186,8 @@ describe('Rate limit policy', function() describe('.log', function() it('success in leaving', function() local config = { - limiters = { - {name = "connections", key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5} + connection_limiters = { + { key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } diff --git a/spec/policy/upstream/upstream_spec.lua b/spec/policy/upstream/upstream_spec.lua index 3a31724d4..9a7a49abd 100644 --- a/spec/policy/upstream/upstream_spec.lua +++ b/spec/policy/upstream/upstream_spec.lua @@ -2,29 +2,28 @@ local balancer = require('apicast.balancer') local UpstreamPolicy = require('apicast.policy.upstream') describe('Upstream policy', function() - describe('.content', function() - -- Set request URI and matched/non-matched upstream used in all the tests - local test_req_uri = 'http://example.com/' - local test_upstream_matched_host = 'localhost' - local test_upstream_matched = string.format("http://%s/a_path:8080", - test_upstream_matched_host) - local test_upstream_matched_proxy_pass = 'http://upstream/a_path:8080' - local test_upstream_not_matched = 'http://localhost/a_path:80' - - local context = {} -- Context shared between policies - - before_each(function() - context = {} - - -- Set headers_sent to false, otherwise, the upstream is not changed - ngx.headers_sent = false - - -- ngx functions and vars - ngx.var = { uri = test_req_uri } - stub(ngx.req, 'set_header') - stub(ngx, 'exec') - end) + -- Set request URI and matched/non-matched upstream used in all the tests + local test_req_uri = 'http://example.com/' + local test_upstream_matched_host = 'localhost' + local test_upstream_matched = string.format("http://%s/a_path:8080", + test_upstream_matched_host) + local test_upstream_matched_proxy_pass = 'http://upstream/a_path:8080' + local test_upstream_not_matched = 'http://localhost/a_path:80' + + local test_upstream_matched_url = { + host = 'localhost', + path = '/a_path:8080', + scheme = 'http' + } + + local context -- Context shared between policies + + before_each(function() + context = {} + ngx.var = { uri = test_req_uri } + end) + describe('.rewrite', function() describe('when there is a rule that matches the request URI', function() local config_with_a_matching_rule = { rules = { @@ -35,18 +34,9 @@ describe('Upstream policy', function() local upstream = UpstreamPolicy.new(config_with_a_matching_rule) - it('changes the upstream according to that rule', function() - upstream:content(context) - - assert.equals(test_upstream_matched_proxy_pass, ngx.var.proxy_pass) - assert.stub(ngx.req.set_header).was_called_with('Host', - test_upstream_matched_host) - assert.stub(ngx.exec).was_called_with('@upstream') - end) - - it('marks in the context that the upstream has changed', function() - upstream:content(context) - assert.is_truthy(context.upstream_changed) + it('stores in the context the URL of that rule', function() + upstream:rewrite(context) + assert.same(test_upstream_matched_url, context.new_upstream) end) end) @@ -61,7 +51,45 @@ describe('Upstream policy', function() local upstream = UpstreamPolicy.new(config_with_several_matching_rules) - it('changes the upstream according to the first rule that matches', function() + it('stores in the context the URL of the 1st rule that matches', function() + upstream:rewrite(context) + assert.same(test_upstream_matched_url, context.new_upstream) + end) + end) + + describe('when there are no rules that match the request URI', function() + local config_without_matching_rules = { + rules = { + { regex = '/i_dont_match', url = test_upstream_not_matched } + } + } + + local upstream = UpstreamPolicy.new(config_without_matching_rules) + + it('does not store a URL in the context', function() + upstream:rewrite(context) + assert.is_nil(context.new_upstream) + end) + end) + end) + + describe('.content', function() + before_each(function() + -- Set headers_sent to false, otherwise, the upstream is not changed + ngx.headers_sent = false + + stub(ngx.req, 'set_header') + stub(ngx, 'exec') + end) + + describe('when there is a new upstream in the context', function() + before_each(function() + context.new_upstream = test_upstream_matched_url + end) + + it('changes the upstream', function() + local upstream = UpstreamPolicy.new({}) + upstream:content(context) assert.equals(test_upstream_matched_proxy_pass, ngx.var.proxy_pass) @@ -71,21 +99,19 @@ describe('Upstream policy', function() end) it('marks in the context that the upstream has changed', function() + local upstream = UpstreamPolicy.new({}) upstream:content(context) assert.is_truthy(context.upstream_changed) end) end) - describe('when there are no rules that match the request URI', function() - local config_without_matching_rules = { - rules = { - { regex = '/i_dont_match', url = test_upstream_not_matched } - } - } - - local upstream = UpstreamPolicy.new(config_without_matching_rules) + describe('when there is not a new upstream in the context', function() + before_each(function() + context.new_upstream = nil + end) it('does not change the upstream', function() + local upstream = UpstreamPolicy.new({}) upstream:content(context) assert.is_nil(ngx.var.proxy_pass) @@ -94,6 +120,7 @@ describe('Upstream policy', function() end) it('does not mark in the context that the upstream has changed', function() + local upstream = UpstreamPolicy.new({}) upstream:content(context) assert.is_falsy(context.upstream_changed) end) diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t index 6934e2192..375ccca14 100644 --- a/t/apicast-policy-rate-limit.t +++ b/t/apicast-policy-rate-limit.t @@ -28,9 +28,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test1", scope = "service", service_name = "service_C"}, conn = 1, burst = 1, @@ -43,9 +42,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test1", scope = "service", service_name = "service_C"}, conn = 1, burst = 1, @@ -106,9 +104,8 @@ Return 500 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test3"}, conn = 20, burst = 10, @@ -151,9 +148,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test4"}, conn = 1, burst = 0, @@ -161,17 +157,14 @@ Return 200 code. } }, redis_url = "redis://$TEST_NGINX_REDIS_HOST:$TEST_NGINX_REDIS_PORT/1", - error_settings = { - {type = "limits_exceeded", error_handling = "log"} - } + limits_exceeded_error = { error_handling = "log" } } }, { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test4"}, conn = 1, burst = 0, @@ -179,9 +172,7 @@ Return 200 code. } }, redis_url = "redis://$TEST_NGINX_REDIS_HOST:$TEST_NGINX_REDIS_PORT/1", - error_settings = { - {type = "limits_exceeded", error_handling = "log"} - } + limits_exceeded_error = { error_handling = "log" } } } } @@ -231,9 +222,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test5"}, conn = 20, burst = 10, @@ -277,22 +267,23 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + leaky_bucket_limiters = { { - name = "leaky_bucket", key = {name = "test6_1"}, rate = 20, burst = 10 - }, + } + }, + connection_limiters = { { - name = "connections", key = {name = "test6_2"}, conn = 20, burst = 10, delay = 0.5 - }, + } + }, + fixed_window_limiters = { { - name = "fixed_window", key = {name = "test6_3"}, count = 20, window = 10 @@ -351,9 +342,8 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test7"}, conn = 1, burst = 0, @@ -366,9 +356,8 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test7"}, conn = 1, burst = 0, @@ -427,18 +416,15 @@ Return 503 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + leaky_bucket_limiters = { { - name = "leaky_bucket", key = {name = "test8"}, rate = 1, burst = 0 } }, redis_url = "redis://$TEST_NGINX_REDIS_HOST:$TEST_NGINX_REDIS_PORT/1", - error_settings = { - {type = "limits_exceeded", status_code = 503} - } + limits_exceeded_error = { status_code = 503 } } } } @@ -490,18 +476,15 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + fixed_window_limiters = { { - name = "fixed_window", key = {name = "test9", scope = "global"}, count = 1, window = 10 } }, redis_url = "redis://$TEST_NGINX_REDIS_HOST:$TEST_NGINX_REDIS_PORT/1", - error_settings = { - {type = "limits_exceeded", status_code = 429} - } + limits_exceeded_error = { status_code = 429 } } } } @@ -553,9 +536,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test10"}, conn = 1, burst = 1, @@ -568,9 +550,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test10"}, conn = 1, burst = 1, @@ -631,9 +612,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + leaky_bucket_limiters = { { - name = "leaky_bucket", key = {name = "test11"}, rate = 1, burst = 1 @@ -693,9 +673,8 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test12"}, conn = 1, burst = 0, @@ -707,9 +686,8 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test12"}, conn = 1, burst = 0, @@ -753,17 +731,14 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + leaky_bucket_limiters = { { - name = "leaky_bucket", key = {name = "test13"}, rate = 1, burst = 0 } }, - error_settings = { - {type = "limits_exceeded", error_handling = "exit"} - } + limits_exceeded_error = { error_handling = "exit" } } } } @@ -800,9 +775,8 @@ Return 429 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + fixed_window_limiters = { { - name = "fixed_window", key = {name = "test14"}, count = 1, window = 10 @@ -844,9 +818,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test15"}, conn = 1, burst = 1, @@ -858,9 +831,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + connection_limiters = { { - name = "connections", key = {name = "test15"}, conn = 1, burst = 1, @@ -908,9 +880,8 @@ Return 200 code. { name = "apicast.policy.rate_limit", configuration = { - limiters = { + leaky_bucket_limiters = { { - name = "leaky_bucket", key = {name = "test16"}, rate = 1, burst = 1 diff --git a/t/apicast-policy-upstream.t b/t/apicast-policy-upstream.t index fb787d39d..295f710f8 100644 --- a/t/apicast-policy-upstream.t +++ b/t/apicast-policy-upstream.t @@ -418,3 +418,123 @@ MHcCAQEEIFCV3VwLEFKz9+yTR5vzonmLPYO/fUvZiMVU1Hb11nN8oAoGCCqGSM49 AwEHoUQDQgAEhkmo6Xp/9W9cGaoGFU7TaBFXOUkZxYbGXQfxyZZucIQPt89+4r1c bx0wVEzbYK5wRb7UiWhvvvYDltIzsD75vg== -----END EC PRIVATE KEY----- + +=== TEST 9: before the URL rewriting policy in the chain +Check that the upstream policy matches the original path of the request +instead of the rewritten one. +--- 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://example.com:80/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ], + "policy_chain": [ + { + "name": "apicast.policy.upstream", + "configuration": { + "rules": [ { "regex": "/original", "url": "http://test:$TEST_NGINX_SERVER_PORT" } ] + } + }, + { + "name": "apicast.policy.url_rewriting", + "configuration": { + "commands": [ + { "op": "gsub", "regex": "/original", "replace": "/rewritten" } + ] + } + }, + { "name": "apicast.policy.apicast" } + ] + } + } + ] +} +--- upstream + location /rewritten { + content_by_lua_block { + require('luassert').are.equal('GET /rewritten?user_key=uk&a_param=a_value HTTP/1.1', + ngx.var.request) + ngx.say('yay, api backend'); + } + } +--- request +GET /original?user_key=uk&a_param=a_value +--- response_body +yay, api backend +--- error_code: 200 +--- no_error_log +[error] + +=== TEST 10: after the URL rewriting policy in the chain +Check that the upstream policy matches the rewritten path of the request +instead of the original one. +--- 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://example.com:80/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ], + "policy_chain": [ + { + "name": "apicast.policy.url_rewriting", + "configuration": { + "commands": [ + { "op": "gsub", "regex": "/original", "replace": "/rewritten" } + ] + } + }, + { + "name": "apicast.policy.upstream", + "configuration": { + "rules": [ { "regex": "/rewritten", "url": "http://test:$TEST_NGINX_SERVER_PORT" } ] + } + }, + { "name": "apicast.policy.apicast" } + ] + } + } + ] +} +--- upstream + location /rewritten { + content_by_lua_block { + require('luassert').are.equal('GET /rewritten?user_key=uk&a_param=a_value HTTP/1.1', + ngx.var.request) + ngx.say('yay, api backend'); + } + } +--- request +GET /original?user_key=uk&a_param=a_value +--- response_body +yay, api backend +--- error_code: 200 +--- no_error_log +[error]