# refactored the way to obtain the redis host to only use upstream {...} configs by leveraging lua-upstream-nginx-module module

Author: ddragosd

src/lua/api-gateway/redis/redisHealthCheck.lua

@@ -101,20 +101,25 @@ local function getHealthCheckForUpstream(upstreamName)
     return bits
 end
 
-local function getHealthyRedisNodeFromCache(dict_name)
+local function getHealthyRedisNodeFromCache(dict_name, upstream_name)
     local dict = ngx.shared[dict_name];
     local upstreamRedis
     if ( nil ~= dict ) then
-        upstreamRedis = dict:get("healthy_redis_upstream")
+        upstreamRedis = dict:get("healthy_redis_upstream:" .. tostring(upstream_name) )
     end
     return upstreamRedis
 end
 
-local function updateHealthyRedisNodeInCache(dict_name, upstreamRedis)
+local function updateHealthyRedisNodeInCache(dict_name, upstream_name, healthy_redis_host)
     local dict = ngx.shared[dict_name];
     if ( nil ~= dict ) then
-        dict:set("healthy_redis_upstream", upstreamRedis, 5)
+        ngx.log(ngx.DEBUG, "Saving a healthy redis host:", healthy_redis_host, " in cache:", dict_name, " for upstream:", upstream_name)
+        local exp_time_in_seconds = 5
+        dict:set("healthy_redis_upstream:" .. tostring(upstream_name), healthy_redis_host, exp_time_in_seconds)
+        return
     end
+
+    ngx.log(ngx.WARN, "Dictionary ", dict_name,  " doesn't seem to be set. Did you define one ? ")
 end
 
 local function getHostAndPortInUpstream(upstreamRedis)
@@ -132,29 +137,32 @@ end
 -- Get the redis node to use for read.
 -- Returns 3 values: <upstreamName , host, port >
 -- The difference between upstream and <host,port> is that the upstream may be just a string containing host:port
-function HealthCheck:getHealthyRedisNodeForRead()
+function HealthCheck:getHealthyRedisNode(upstream_name)
 
     -- get the Redis host and port from the local cache first
-    local redisToUse = getHealthyRedisNodeFromCache(self.shared_dict)
-    if ( nil ~= redisToUse) then
-        local host, port = getHostAndPortInUpstream(redisToUse)
-        return redisToUse, host, port
+    local healthy_redis_host = getHealthyRedisNodeFromCache(self.shared_dict, upstream_name)
+    if ( nil ~= healthy_redis_host) then
+        local host, port = getHostAndPortInUpstream(healthy_redis_host)
+        return healthy_redis_host, host, port
     end
 
+    ngx.log(ngx.DEBUG, "Looking up for a healthy redis node in upstream:", upstream_name)
     -- if the Redis host is not in the local cache get it from the upstream configuration
-    local redisUpstreamHealthResult = self:getRedisUpstreamHealthStatus()
+    local redisUpstreamHealthResult = getHealthCheckForUpstream(upstream_name)
 
     if(redisUpstreamHealthResult == nil) then
         ngx.log(ngx.ERR, "\n No upstream results found for redis!!! ")
         return nil
     end
 
     for key,value in ipairs(redisUpstreamHealthResult) do
+        -- return the first node found to be up.
+        -- TODO: save all the nodes that are up and return them using round-robin alg
         if(value == " up\n") then
-            redisToUse = redisUpstreamHealthResult[key-1]
-            updateHealthyRedisNodeInCache(self.shared_dict, redisToUse)
-            local host, port = getHostAndPortInUpstream(redisToUse)
-            return redisToUse, host, port
+            healthy_redis_host = redisUpstreamHealthResult[key-1]
+            updateHealthyRedisNodeInCache(self.shared_dict, upstream_name, healthy_redis_host)
+            local host, port = getHostAndPortInUpstream(healthy_redis_host)
+            return healthy_redis_host, host, port
         end
         if(value == " DOWN\n" and redisUpstreamHealthResult[key-1] ~= nil ) then
             ngx.log(ngx.WARN, "\n Redis node " .. tostring(redisUpstreamHealthResult[key-1]) .. " is down! Checking for backup nodes. ")
@@ -165,15 +173,4 @@ function HealthCheck:getHealthyRedisNodeForRead()
     return nil -- No redis nodes are up
 end
 
--- To get the health check results on all the nodes defined in the redis upstream.
--- The health check is performed by a worker using "resty.upstream.healthcheck" module.
--- The name of the redis read only upstream is used here.
-function HealthCheck:getRedisUpstreamHealthStatus()
-    -- TODO: make the name of the upstream configurable for reuse
-    local redisUpstreamStatus = getHealthCheckForUpstream("cache_read_only_backend")
-    return redisUpstreamStatus;
-end
-
-
-
 return HealthCheck

src/lua/api-gateway/validation/key/redisApiKeyValidator.lua

@@ -26,7 +26,6 @@
 -- Dependencies:
 --   1. ngx.var.key needs to be set
 --   2. ngx.var.service_id needs to be set
---   3. ngx.var.redis_backend needs to be set
 --
 -- User: ddascal
 -- Date: 11/22/13

src/lua/api-gateway/validation/oauth2/oauthTokenValidator.lua

@@ -29,9 +29,8 @@
 -- Dependencies:
 --   1. ngx.var.oauth_host needs to be set
 --   2. ngx.var.authtoken needs to be set
---   3. ngx.var.redis_backend needs to be set
---   4. location /validate-token defined
---   5. lua_shared_dict cachedOauthTokens 50m;
+--   3. location /validate-token defined
+--   4. lua_shared_dict cachedOauthTokens 50m;
 --
 -- Properties that can be set:
 --  1. oauth_token_scope
@@ -41,7 +40,6 @@
 
 local BaseValidator = require "api-gateway.validation.validator"
 local cjson = require "cjson"
-local redis = require "resty.redis"
 
 local _M = BaseValidator:new()
 

src/lua/api-gateway/validation/oauth2/userProfileValidator.lua

@@ -26,9 +26,8 @@
 -- Dependencies:
 --   1. ngx.var.oauth_host                      - optional var
 --   2. ngx.var.authtoken                       - required to be set
---   3. ngx.var.redis_backend                   - required
---   4. lua_shared_dict cachedOauthTokens 50m;  - required. The local shared dict to cache user profiles
---   5. ngx.ctx.oauth_token_expires_at          - optional. This is usually set by the oauthTokenValidator
+--   3. lua_shared_dict cachedOauthTokens 50m;  - required. The local shared dict to cache user profiles
+--   4. ngx.ctx.oauth_token_expires_at          - optional. This is usually set by the oauthTokenValidator
 --
 -- Properties that can be set by this validator:
 --  1. user_email

src/lua/api-gateway/validation/validator.lua

@@ -27,9 +27,8 @@
 -- Time: 18:01
 --
 -- Dependencies:
---   1. ngx.var.redis_backend needs to be set
---   2. ngx.var.redis_backend_rw needs to be set
---   2. ngx.var.disable_redis_replica_healthchecks OPTIONAL - it disables redis healthchecks
+--   1. api-gateway-redis upstream needs to be set
+--   2. api-gateway-redis-replica needs to be set
 --
 local base              = require "api-gateway.validation.base"
 local redis             = require "resty.redis"
@@ -38,39 +37,15 @@ local cjson             = require "cjson"
 local debug_mode        = ngx.config.debug
 
 -- redis endpoints are assumed to be global per GW node and therefore are read here
-local redis_RO_backend                      = "none"
-local redis_RW_backend                      = "none"
-local disable_redis_replica_healthchecks    = "none"
+local redis_RO_upstream                      = "api-gateway-redis-replica"
+local redis_RW_upstream                      = "api-gateway-redis"
 
 -- class to be used as a base class for all api-gateway validators --
 local BaseValidator = {}
 local redisHealthCheck = RedisHealthCheck:new({
     shared_dict = "cachedkeys"
 })
 
-local function get_redis_RO_backend()
-    if ( redis_RO_backend == "none" ) then
-        redis_RO_backend = ngx.var.redis_backend
-
-    end
-    return redis_RO_backend
-end
-
-local function get_redis_RW_backend()
-    if ( redis_RW_backend == "none" ) then
-        redis_RW_backend = ngx.var.redis_backend_rw
-    end
-    return redis_RW_backend
-end
-
-local function get_disable_redis_replica_healthchecks()
-    if ( disable_redis_replica_healthchecks == "none" ) then
-        disable_redis_replica_healthchecks = ngx.var.disable_redis_replica_healthchecks
-    end
-    return disable_redis_replica_healthchecks
-end
-
-
 function BaseValidator:new(o)
     local o = o or {}
     setmetatable(o, self)
@@ -103,26 +78,24 @@ function BaseValidator:setKeyInLocalCache(key, string_value, exptime, dict_name)
     end
 end
 
-function BaseValidator:getRedisUpstream()
-    if (get_disable_redis_replica_healthchecks() == "on") then
-        return get_redis_RO_backend(), 6379
-    end
-    local upstream, host, port = redisHealthCheck:getHealthyRedisNodeForRead()
-    ngx.log(ngx.DEBUG, "REDIS HOST:" .. tostring(host) .. ":" .. tostring(port))
+function BaseValidator:getRedisUpstream(upstream_name)
+    local n = upstream_name or redis_RO_upstream
+    local upstream, host, port = redisHealthCheck:getHealthyRedisNode(n)
+    ngx.log(ngx.DEBUG, "Obtained Redis Host:" .. tostring(host) .. ":" .. tostring(port), " from upstream:", n)
     if (nil ~= host and nil ~= port) then
         return host, port
     end
 
-    ngx.log(ngx.ERR, "Could not find a Redis upstream. Using default upstream: [" .. get_redis_RO_backend() .. ":6379]")
-    return get_redis_RO_backend(), 6379
+    ngx.log(ngx.ERR, "Could not find a Redis upstream.")
+    return nil,nil
 end
 
 -- retrieves a saved information from the Redis cache --
 -- the method uses HGET redis command --
 -- it returns the value of the key, when found in the cache, nil otherwise --
 function BaseValidator:getKeyFromRedis(key, hash_name)
     local redisread = redis:new()
-    local redis_host, redis_port = self:getRedisUpstream()
+    local redis_host, redis_port = self:getRedisUpstream(redis_RO_upstream)
     local ok, err = redisread:connect(redis_host, redis_port)
     if ok then
         local redis_key, selecterror = redisread:hget(key, hash_name)
@@ -132,7 +105,7 @@ function BaseValidator:getKeyFromRedis(key, hash_name)
             return redis_key
         end
     else
-        ngx.log(ngx.WARN, "Failed to read key " .. key .. " from Redis cache:", redis_host, ":", redis_port, ".Error:", err)
+        ngx.log(ngx.WARN, "Failed to read key " .. tostring(key) .. " from Redis cache:[", redis_host, ":", redis_port, "]. Error:", err)
     end
     return nil;
 end
@@ -142,7 +115,8 @@ end
 -- it retuns true if the information is saved in the cache, false otherwise --
 function BaseValidator:setKeyInRedis(key, hash_name, keyexpires, value)
     local rediss = redis:new()
-    local ok, err = rediss:connect(get_redis_RW_backend(), 6379)
+    local redis_host, redis_port = self:getRedisUpstream(redis_RW_upstream)
+    local ok, err = rediss:connect(redis_host, redis_port)
     if ok then
         --ngx.log(ngx.DEBUG, "WRITING IN REDIS JSON OBJ key=" .. key .. "=" .. value .. ",expiring in:" .. (keyexpires - (os.time() * 1000)) )
         rediss:init_pipeline()
@@ -157,7 +131,7 @@ function BaseValidator:setKeyInRedis(key, hash_name, keyexpires, value)
             ngx.log(ngx.WARN, "Failed to write the key [", key, "] in Redis. Error:", commit_err)
         end
     else
-        ngx.log(ngx.WARN, "Failed to save key:" .. key .. " into cache: ", get_redis_RW_backend(), ".Error:", err)
+        ngx.log(ngx.WARN, "Failed to save key:" .. tostring(key) .. " into cache: [", tostring(redis_host) .. ":" .. tostring(redis_port), "]. Error:", err)
     end
     return false;
 end

test/perl/api-gateway/validation/key/api_key.t

@@ -31,7 +31,7 @@ use Cwd qw(cwd);
 
 repeat_each(2);
 
-plan tests => repeat_each() * (blocks() * 4) + 10;
+plan tests => repeat_each() * (blocks() * 4) + 14;
 
 my $pwd = cwd();
 
@@ -47,11 +47,11 @@ our $HttpConfig = <<_EOC_;
         ngx.apiGateway = ngx.apiGateway or {}
         ngx.apiGateway.validation = require "api-gateway.validation.factory"
      ';
-    include "$pwd/conf.d/http.d/*.conf";
-    upstream cache_rw_backend {
+    lua_shared_dict cachedkeys 50m; # caches api-keys
+    upstream api-gateway-redis {
     	server 127.0.0.1:6379;
     }
-    upstream cache_read_only_backend { # Default config for redis health check test
+    upstream api-gateway-redis-replica { # Default config for redis health check test
         server 127.0.0.1:6379;
     }
 _EOC_
@@ -66,6 +66,8 @@ __DATA__
 --- http_config eval: $::HttpConfig
 --- config
         include ../../api-gateway/api_key_service.conf;
+        error_log ../test-logs/api_key_test1_error.log debug;
+
 --- more_headers
 X-Test: test
 --- request
@@ -81,6 +83,7 @@ POST /cache/api_key?key=k-123&service_id=s-123
 --- config
         include ../../api-gateway/api_key_service.conf;
         include ../../api-gateway/default_validators.conf;
+        error_log ../test-logs/api_key_test2_error.log debug;
 
         location /test-api-key {
             set $service_id s-123;
@@ -105,6 +108,8 @@ GET /test-api-key
 --- config
         include ../../api-gateway/api_key_service.conf;
         include ../../api-gateway/default_validators.conf;
+        error_log ../test-logs/api_key_test3_error.log debug;
+
         location /test-api-key {
             set $service_id s-123;
 
@@ -128,6 +133,7 @@ GET /test-api-key?api_key=ab123
 --- config
         include ../../api-gateway/api_key_service.conf;
         include ../../api-gateway/default_validators.conf;
+        error_log ../test-logs/api_key_test4_error.log debug;
 
         location /test-api-key {
             set $service_id s-123;
@@ -142,10 +148,13 @@ GET /test-api-key?api_key=ab123
         }
 --- pipelined_requests eval
 ["POST /cache/api_key?key=test-key-1234&service_id=s-123",
+"GET /test-api-key?api_key=test-key-1234",
 "GET /test-api-key?api_key=test-key-1234"]
 --- response_body eval
 ["+OK\r\n",
-"api-key is valid.\n"]
+"api-key is valid.\n",
+"api-key is valid.\n"
+]
 --- no_error_log
 
 
@@ -155,6 +164,7 @@ GET /test-api-key?api_key=ab123
         include ../../api-gateway/api_key_service.conf;
         include ../../api-gateway/default_validators.conf;
         error_log ../test-logs/api_key_test5_error.log debug;
+
         location /test-api-key-5 {
             set $service_id s-123;
 
@@ -184,6 +194,8 @@ GET /test-api-key?api_key=ab123
 --- config
         include ../../api-gateway/api_key_service.conf;
         include ../../api-gateway/default_validators.conf;
+        error_log ../test-logs/api_key_test6_error.log debug;
+
         location /test-api-key {
             set $service_id s-123;
 
@@ -210,11 +222,13 @@ GET /test-api-key?api_key=ab123
 [error]
 
 
-=== TEST 6: test api-key related field starting with capital H
+=== TEST 7: test api-key related field starting with capital H
 --- http_config eval: $::HttpConfig
 --- config
         include ../../api-gateway/api_key_service.conf;
         include ../../api-gateway/default_validators.conf;
+        error_log ../test-logs/api_key_test7_error.log debug;
+
         location /test-api-key {
             set $service_id hH-123;
 

test/perl/api-gateway/validation/oauth2/oauthTokenValidator.t

@@ -48,14 +48,14 @@ our $HttpConfig = <<_EOC_;
         ngx.apiGateway = ngx.apiGateway or {}
         ngx.apiGateway.validation = require "api-gateway.validation.factory"
     ';
-
+    lua_shared_dict cachedkeys 50m; # caches api-keys
     # dict used by OAuth validator to cache valid tokens
     lua_shared_dict cachedOauthTokens 50m;
 
-    upstream cache_rw_backend {
+    upstream api-gateway-redis {
     	server 127.0.0.1:6379;
     }
-    upstream cache_read_only_backend { # Default config for redis health check test
+    upstream api-gateway-redis-replica { # Default config for redis health check test
         server 127.0.0.1:6379;
     }
 _EOC_

test/perl/api-gateway/validation/oauth2/userProfileValidator.t

@@ -48,16 +48,16 @@ our $HttpConfig = <<_EOC_;
         ngx.apiGateway = ngx.apiGateway or {}
         ngx.apiGateway.validation = require "api-gateway.validation.factory"
     ';
-
+    lua_shared_dict cachedkeys 50m; # caches api-keys
     # dict used by OAuth validator to cache valid tokens
     lua_shared_dict cachedOauthTokens 50m;
     # dict used by User Profile validator to cache valid profiles
     lua_shared_dict cachedUserProfiles 50m;
 
-    upstream cache_rw_backend {
-    	server 127.0.0.1:6379;
+    upstream api-gateway-redis {
+        server 127.0.0.1:6379;
     }
-    upstream cache_read_only_backend { # Default config for redis health check test
+    upstream api-gateway-redis-replica { # Default config for redis health check test
         server 127.0.0.1:6379;
     }
 _EOC_