diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e2a46dd7..1cd71b702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Remove "$id" from the policy schema [PR #1525](https://github.com/3scale/APIcast/pull/1525) [THEESCALE-11610](https://issues.redhat.com/browse/THREESCALE-11610) - Fixed Financial-grade API (FAPI) policy not showing up in the admin portal [PR #1528](https://github.com/3scale/APIcast/pull/1528) [THREESCALE-11620](https://issues.redhat.com/browse/THREESCALE-11620) - Remove Conditional Policy from the UI [PR #1534](https://github.com/3scale/APIcast/pull/1534) [THREESCALE-6116](https://issues.redhat.com/browse/THREESCALE-6116) +- Remove redis connection error message from response body in edge limiting policy [PR #1537](https://github.com/3scale/APIcast/pull/1537) [THREESCALE-11701](https://issues.redhat.com/browse/THREESCALE-11701) ### Added diff --git a/gateway/src/apicast/threescale_utils.lua b/gateway/src/apicast/threescale_utils.lua index d634178f0..c3fd2b1fe 100644 --- a/gateway/src/apicast/threescale_utils.lua +++ b/gateway/src/apicast/threescale_utils.lua @@ -1,5 +1,6 @@ local sub = string.sub local tonumber = tonumber +local fmt = string.format local redis = require 'resty.redis' local env = require 'resty.env' @@ -152,22 +153,22 @@ function _M.connect_redis(options) local ok, err = red:connect(_M.resolve(host, port)) if not ok then - return nil, _M.error("failed to connect to redis on ", host, ":", port, ": ", err) + return nil, fmt("failed to connect to redis on %s:%d, err: %s",host, port, err) end if opts.password then - ok = red:auth(opts.password) + ok, err = red:auth(opts.password) if not ok then - return nil, _M.error("failed to auth on redis ", host, ":", port) + return nil, fmt("failed to auth on redis %s:%d, err: %s", host, port, err) end end if opts.db then - ok = red:select(opts.db) + ok, err = red:select(opts.db) if not ok then - return nil, _M.error("failed to select db ", opts.db, " on redis ", host, ":", port) + return nil, fmt("failed to select db %s on redis %s:%d, err: %s", opts.db, host, port, err) end end diff --git a/spec/policy/rate_limit/rate_limit_spec.lua b/spec/policy/rate_limit/rate_limit_spec.lua index f4dce6594..56e443293 100644 --- a/spec/policy/rate_limit/rate_limit_spec.lua +++ b/spec/policy/rate_limit/rate_limit_spec.lua @@ -35,6 +35,7 @@ describe('Rate limit policy', function() redis:flushdb() stub(ngx, 'exit') + stub(ngx, 'say') stub(ngx, 'sleep') stub(ngx, 'time', function() return 11111 end) @@ -138,8 +139,9 @@ describe('Rate limit policy', function() redis_url = 'redis://invalidhost.domain:'..redis_port..'/1' }) - assert.returns_error('failed to connect to redis on invalidhost.domain:6379: invalidhost.domain could not be resolved (3: Host not found)', rate_limit_policy:access(context)) + rate_limit_policy:access(context) + assert.spy(ngx.say).was_not_called_with('failed to connect to redis on invalidhost.domain:6379: invalidhost.domain could not be resolved (3: Host not found)') assert.spy(ngx.exit).was_called_with(500) end) diff --git a/spec/policy/rate_limit/redis_shdict_spec.lua b/spec/policy/rate_limit/redis_shdict_spec.lua index 8df6505cb..f4886bec7 100644 --- a/spec/policy/rate_limit/redis_shdict_spec.lua +++ b/spec/policy/rate_limit/redis_shdict_spec.lua @@ -10,6 +10,23 @@ describe('Redis Shared Dictionary', function() local redis local shdict + describe('new', function() + it('invalid redis url', function() + local _, err = redis_shdict.new{ host = "invalid", port = 6379 } + assert.match("failed to connect to redis on invalid:6379", err) + end) + + it('invalid redis auth', function() + local _, err = redis_shdict.new{ host = redis_host, port = redis_port, db=1 , password = "invalid"} + assert.match("failed to auth on redis ".. redis_host .. ":" .. redis_port ..", err: ERR AUTH called without any password configured for the default user. Are you sure your configuration is correct?", err) + end) + + it('invalid redis db', function() + local _, err = redis_shdict.new{ host = redis_host, port = redis_port, db = 1000} + assert.match("failed to select db 1000 on redis " .. redis_host ..":" .. redis_port .. ", err: ERR DB index is out of range", err) + end) + end) + before_each(function() local options = { host = redis_host, port = redis_port, db = 1 } redis = assert(ts.connect_redis(options)) @@ -18,6 +35,7 @@ describe('Redis Shared Dictionary', function() assert(redis:flushdb()) end) + describe('flush_all', function() it('removes all records', function() assert(redis:set('foo', 'bar')) diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t index 26f5ed649..8207837cc 100644 --- a/t/apicast-policy-rate-limit.t +++ b/t/apicast-policy-rate-limit.t @@ -213,8 +213,6 @@ Return 500 code. --- request GET / --- error_code: 500 ---- no_error_log -[error] --- error_log query for invalidhost finished with no answers @@ -1565,3 +1563,40 @@ location /transactions/authrep.xml { [error] --- error_log Requests over the limit. + + + +=== TEST 23: Invalid redis url and configuration_error set to log. +Return 200 code. +--- configuration +{ + "services" : [ + { + "id" : 42, + "proxy" : { + "policy_chain" : [ + { + "name" : "apicast.policy.rate_limit", + "configuration" : { + "connection_limiters" : [ + { + "key" : {"name" : "test3", "scope" : "global"}, + "conn" : 20, + "burst" : 10, + "delay" : 0.5 + } + ], + "redis_url" : "redis://invalidhost:$TEST_NGINX_REDIS_PORT/1", + "configuration_error": {"error_handling": "log"} + } + } + ] + } + } + ] +} +--- request +GET / +--- error_code: 200 +--- error_log +query for invalidhost finished with no answers