Merge pull request #1537 from tkan145/THREESCALE-11701-redis-error

[THREESCALE-11701] Remove redis connection error message from response body in edge limiting policy

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
 

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
 

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)
 

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 <password> 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'))

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