[apicast] use only instance interface of proxy

even though the proxy is instantiated on init
it is just a local properly of the apicast module

so it the future can be made per-request

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Bump s2i-openresty to 1.11.2.2-2 [PR #260](https://github.com/3scale/apicast/pull/260)
 - Echo API on port 8081 listens accepts any Host [PR #268](https://github.com/3scale/apicast/pull/268)
 - Always use DNS search scopes [PR #271](https://github.com/3scale/apicast/pull/271)
+- Reduce use of global objects [PR #273](https://github.com/3scale/apicast/pull/273)
 
 ### Added
 

apicast/src/apicast.lua

@@ -7,6 +7,7 @@ local math = math
 local setmetatable = setmetatable
 local env = require('resty.env')
 local reload_config = env.enabled('APICAST_RELOAD_CONFIG')
+local configuration_store = require('configuration_store')
 local user_agent = require('user_agent')
 
 local noop = function() end
@@ -34,36 +35,41 @@ end
 local mt = {
   __index = _M
 }
+--- This is called when APIcast boots the master process.
 function _M.new()
-  return setmetatable({ proxy = proxy.new() }, mt)
+  -- FIXME: this is really bad idea, this file is shared across all requests,
+  -- so that means sharing something in this module would be sharing it acros all requests
+  -- and in multi-tenant environment that would mean leaking information
+  local configuration = configuration_store.new()
+  return setmetatable({ proxy = proxy.new(configuration) }, mt)
 end
 
-function _M.init()
+function _M:init()
   user_agent.cache()
 
   math.randomseed(ngx.now())
   -- First calls to math.random after a randomseed tend to be similar; discard them
   for _=1,3 do math.random() end
 
   local config, err = configuration_loader.init()
-  local init = config and proxy.init(config)
+  local init = config and self.proxy:configure(config)
 
   if not init then
     handle_missing_configuration(err)
   end
 end
 
-local function refresh_config()
+local function refresh_config(p)
   local config, err = configuration_loader.boot()
 
   if config then
-    proxy.init(config)
+    p:configure(config)
   else
     ngx.log(ngx.ERR, 'failed to refresh configuration: ', err)
   end
 end
 
-function _M.init_worker()
+function _M:init_worker()
   local interval = tonumber(env.get('AUTO_UPDATE_INTERVAL'), 10) or 0
 
   local function schedule(...)
@@ -77,24 +83,24 @@ function _M.init_worker()
 
   local handler
 
-  handler = function (premature)
+  handler = function (premature, ...)
     if premature then return end
 
     ngx.log(ngx.INFO, 'auto updating configuration')
 
-    local updated, err = pcall(refresh_config)
+    local updated, err = pcall(refresh_config, ...)
 
     if updated then
       ngx.log(ngx.INFO, 'auto updating configuration finished successfuly')
     else
       ngx.log(ngx.ERR, 'auto updating configuration failed with: ', err)
     end
 
-    schedule(interval, handler)
+    schedule(interval, handler, ...)
   end
 
   if interval > 0 then
-    schedule(interval, handler)
+    schedule(interval, handler, self.proxy)
   end
 end
 
@@ -116,11 +122,11 @@ function _M:rewrite()
     p:configure(config)
   end
 
-  p.set_upstream(p.set_service(host))
+  p.set_upstream(p:set_service(host))
 end
 
-function _M.post_action()
-  proxy:post_action()
+function _M:post_action()
+  self.proxy:post_action()
 end
 
 function _M:access()

apicast/src/configuration_loader.lua

@@ -16,7 +16,7 @@ function _M.boot(host)
   return mock_loader.call() or file_loader.call() or remote_loader_v2.call() or remote_loader_v1.call(host) or error('missing configuration')
 end
 
-_M.save = mock_loader.save
+_M.mock = mock_loader.save
 
 -- Cosocket API is not available in the init_by_lua* context (see more here: https://github.com/openresty/lua-nginx-module#cosockets-not-available-everywhere)
 -- For this reason a new process needs to be started to download the configuration through 3scale API

apicast/src/management.lua

@@ -1,7 +1,7 @@
 local _M = {}
 
 local cjson = require('cjson')
-local provider = require('proxy')
+local proxy = require('module').proxy
 local router = require('router')
 local configuration_parser = require('configuration_parser')
 local configuration_loader = require('configuration_loader')
@@ -31,8 +31,8 @@ end
 
 function _M.status()
   -- TODO: this should be fixed for multi-tenant deployment
-  local has_configuration = provider.configuration.configured
-  local has_services = #(provider.configuration:all()) > 0
+  local has_configuration = proxy.configuration.configured
+  local has_services = #(proxy.configuration:all()) > 0
 
   if not has_configuration then
     return { status = 'error', error = 'not configured',  success = false }
@@ -44,7 +44,7 @@ function _M.status()
 end
 
 function _M.config()
-  local config = provider.configuration
+  local config = proxy.configuration
   local contents = cjson.encode(config.configured and { services = config:all() } or nil)
 
   ngx.header.content_type = 'application/json; charset=utf-8'
@@ -64,7 +64,7 @@ function _M.update_config()
   end
 
   local config = configuration_parser.decode(data)
-  provider.configure(config)
+  proxy:configure(config)
   -- TODO: respond with proper 304 Not Modified when config is the same
   local response = cjson.encode({ status = 'ok', config = config or cjson.null })
   ngx.header.content_type = 'application/json; charset=utf-8'
@@ -74,7 +74,7 @@ end
 function _M.delete_config()
   ngx.log(ngx.DEBUG, 'management config delete')
 
-  provider.configuration:reset()
+  proxy.configuration:reset()
   -- TODO: respond with proper 304 Not Modified when config is the same
   local response = cjson.encode({ status = 'ok', config = cjson.null })
   ngx.header.content_type = 'application/json; charset=utf-8'
@@ -90,7 +90,7 @@ function _M.boot()
 
   ngx.log(ngx.DEBUG, 'management boot config:' .. inspect(data))
 
-  provider.init(config)
+  proxy:configure(config)
 
   ngx.say(response)
 end

apicast/src/proxy.lua

@@ -24,25 +24,24 @@ local empty = {}
 local response_codes = env.enabled('APICAST_RESPONSE_CODES')
 local request_logs = env.enabled('APICAST_REQUEST_LOGS')
 
-local _M = {
-  -- FIXME: this is really bad idea, this file is shared across all requests,
-  -- so that means sharing something in this module would be sharing it acros all requests
-  -- and in multi-tenant environment that would mean leaking information
-  configuration = configuration_store.new()
-}
+local _M = { }
 
 local mt = {
   __index = _M
 }
 
 function _M.new(configuration)
   return setmetatable({
-    configuration = configuration
+    configuration = assert(configuration, 'missing proxy configuration')
   }, mt)
 end
 
 function _M:configure(contents)
-  if self and not contents then contents = self end
+  local configuration = self.configuration
+
+  if not configuration then
+    return nil, 'not initialized'
+  end
 
   local config, err = configuration_parser.parse(contents)
 
@@ -53,9 +52,6 @@ function _M:configure(contents)
     return nil, err
   end
 
-  -- FIXME: using global configuration interface
-  local configuration = self.configuration or _M.configuration
-
   if config then
     return configuration:store(config)
   end
@@ -71,8 +67,6 @@ function _M:configured(host)
   return next(hosts) and true
 end
 
-_M.init = function(config) return _M.configure(_M, config) end
-
 -- Error Codes
 local function error_no_credentials(service)
   ngx.log(ngx.INFO, 'no credentials provided for service ', service.id)
@@ -254,9 +248,9 @@ function _M.authorize(backend_version, service)
   end
 end
 
-function _M.set_service(host)
+function _M:set_service(host)
   host = host or ngx.var.host
-  local service = _M:find_service(host)
+  local service = self:find_service(host)
 
   if not service then
     error_service_not_found(host)
@@ -314,7 +308,7 @@ end
 
 function _M:call(host)
   host = host or ngx.var.host
-  local service = ngx.ctx.service or self.set_service(host)
+  local service = ngx.ctx.service or self:set_service(host)
 
   self:set_backend_upstream(service)
 

spec/configuration_loader_spec.lua

@@ -6,7 +6,7 @@ insulate('Configuration object', function()
 
     it('saves mock configuration', function()
       local config = { 'foo' }
-      configuration.save(config)
+      configuration.mock(config)
 
       assert.equal(config, mock_loader.config)
     end)

spec/proxy_spec.lua

@@ -1,9 +1,11 @@
-local proxy = require 'proxy'
 local configuration_store = require 'configuration_store'
 
 describe('Proxy', function()
+  local configuration, proxy
+
   before_each(function()
-    proxy.configuration:reset()
+    configuration = configuration_store.new()
+    proxy = require('proxy').new(configuration)
   end)
 
   it('has access function', function()
@@ -23,12 +25,10 @@ describe('Proxy', function()
 
   it('finds service by host', function()
     local example = { id = 42, hosts = { 'example.com'} }
-    local configuration = configuration_store.new()
-    local p = assert(proxy.new(configuration))
 
     configuration:add(example)
 
-    assert.same(example, p:find_service('example.com'))
+    assert.same(example, proxy:find_service('example.com'))
     assert.falsy(proxy:find_service('unknown'))
   end)
 
@@ -49,9 +49,9 @@ describe('Proxy', function()
     end)
 
     it('returns true when configured', function()
-      local configuration = configuration_store.new()
-      configuration:add({ id = 42, hosts = { 'example.com' } })
-      local p = assert(proxy.new(configuration))
+      local config = configuration_store.new()
+      config:add({ id = 42, hosts = { 'example.com' } })
+      local p = assert(proxy.new(config))
 
       assert.truthy(p:configured('example.com'))
     end)

t/001-management.t

@@ -22,7 +22,7 @@ When configuration is saved, readiness probe returns success.
   lua_package_path "$TEST_NGINX_LUA_PATH";
 
   init_by_lua_block {
-    require('proxy').configure({ services = { { id = 42 } } })
+    require('module').proxy:configure({ services = { { id = 42 } } })
   }
 --- config
 include $TEST_NGINX_MANAGEMENT_CONFIG;
@@ -53,7 +53,7 @@ Should respond with error status and a reason.
 --- http_config
   lua_package_path "$TEST_NGINX_LUA_PATH";
   init_by_lua_block {
-    require('proxy').configure({services = { }})
+    require('module').proxy:configure({services = { }})
   }
 --- config
   include $TEST_NGINX_MANAGEMENT_CONFIG;
@@ -86,7 +86,7 @@ Endpoint that dumps the original configuration.
   lua_package_path "$TEST_NGINX_LUA_PATH";
 
   init_by_lua_block {
-    require('proxy').configure({ services = { { id = 42 } } })
+    require('module').proxy:configure({ services = { { id = 42 } } })
   }
 --- config
 include $TEST_NGINX_MANAGEMENT_CONFIG;
@@ -148,7 +148,7 @@ env RESOLVER=127.0.0.1:1953;
 --- http_config
   lua_package_path "$TEST_NGINX_LUA_PATH";
   init_by_lua_block {
-      require('configuration_loader').save({ services = { { id = 42 } } })
+      require('module').proxy:configure({ services = { { id = 42 } } })
   }
 --- config
 include $TEST_NGINX_MANAGEMENT_CONFIG;
@@ -171,7 +171,7 @@ env RESOLVER=127.0.0.1:1953;
 --- http_config
   lua_package_path "$TEST_NGINX_LUA_PATH";
   init_by_lua_block {
-      require('configuration_loader').save({ services = { { id = 42 } } })
+      require('module').proxy:configure({ services = { { id = 42 } } })
   }
 --- config
 include $TEST_NGINX_MANAGEMENT_CONFIG;