Merge pull request #1564 from tkan145/THREESCALE-12133-optimize-configuration-reload

THREESCALE-12133 optimize configuration reload

Author: tkan145

gateway/src/apicast/configuration_loader/remote_v2.lua

@@ -102,33 +102,36 @@ local function service_config_endpoint(portal_endpoint, service_id, env, version
   )
 end
 
+local function get_oidc_issuer_endpoint(proxy_content)
+  return proxy_content.proxy and proxy_content.proxy.oidc_issuer_endpoint
+end
+
 local function parse_proxy_configs(self, proxy_configs)
   local config = { services = array(), oidc = array() }
 
   for i, proxy_conf in ipairs(proxy_configs) do
     local proxy_config = proxy_conf.proxy_config
+    local content = proxy_config.content
 
-    -- Copy the config because parse_service have side-effects. It adds
-    -- liquid templates in some policies and those cannot be encoded into a
-    -- JSON. We should get rid of these side effects.
-    local original_proxy_config = deepcopy(proxy_config)
-
-    local service = configuration.parse_service(proxy_config.content)
+    config.services[i] = content
 
+    local issuer_endpoint = get_oidc_issuer_endpoint(content)
+    local oidc
+    if issuer_endpoint then
+      oidc = self.oidc:call(issuer_endpoint, self.ttl)
+    end
     -- We always assign a oidc to the service, even an empty one with the
     -- service_id, if not on APICAST_SERVICES_LIST will fail on filtering
-    local oidc = self:oidc_issuer_configuration(service)
     if not oidc then
       oidc = {}
     end
 
     -- deepcopy because this can be cached, and we want to have a deepcopy to
     -- avoid issues with service_id
     local oidc_copy = deepcopy(oidc)
-    oidc_copy.service_id = service.id
+    oidc_copy.service_id = tostring(content.id)
 
     config.oidc[i] = oidc_copy
-    config.services[i] = original_proxy_config.content
   end
   return cjson.encode(config)
 end
@@ -451,10 +454,6 @@ function _M:services()
   return services
 end
 
-function _M:oidc_issuer_configuration(service)
-  return self.oidc:call(service.oidc.issuer_endpoint, self.ttl)
-end
-
 function _M:config(service, environment, version, service_regexp_filter)
   local http_client = self.http_client
 
@@ -482,20 +481,22 @@ function _M:config(service, environment, version, service_regexp_filter)
 
   if res.status == 200 then
     local proxy_config = cjson.decode(res.body).proxy_config
-
-    -- Copy the config because parse_service have side-effects. It adds
-    -- liquid templates in some policies and those cannot be encoded into a
-    -- JSON. We should get rid of these side effects.
-    local original_proxy_config = deepcopy(proxy_config)
+    local content = proxy_config.content
 
     local config_service = configuration.parse_service(proxy_config.content)
     if service_regexp_filter and not config_service:match_host(service_regexp_filter) then
       return nil, "Service filtered out because APICAST_SERVICES_FILTER_BY_URL"
     end
 
-    original_proxy_config.oidc = self:oidc_issuer_configuration(config_service)
+    local issuer_endpoint = get_oidc_issuer_endpoint(content)
+    local oidc
+
+    if issuer_endpoint then
+      oidc = self.oidc:call(issuer_endpoint, self.ttl)
+    end
 
-    return original_proxy_config
+    proxy_config.oidc = oidc
+    return proxy_config
   else
     return nil, status_code_error(res)
   end

gateway/src/apicast/policy_chain.lua

@@ -193,12 +193,15 @@ function _M:add_policy(name, version, ...)
     end
 end
 
+local default_policy_order_check = PolicyOrderChecker.new(policy_manifests_loader.get_all())
+
 -- Checks if there are any policies placed in the wrong place in the chain.
 -- It doesn't return anything, it prints error messages when there's a problem.
 function _M:check_order(manifests)
-    PolicyOrderChecker.new(
-        manifests or policy_manifests_loader.get_all()
-    ):check(self)
+  if manifests then
+    return PolicyOrderChecker.new(manifests):check(self)
+  end
+  default_policy_order_check:check(self)
 end
 
 local function call_chain(phase_name)

gateway/src/apicast/policy_config_validator.lua

@@ -3,8 +3,35 @@
 -- Validates a policy configuration against a policy config JSON schema.
 
 local jsonschema = require('jsonschema')
+local lrucache = require("resty.lrucache")
 
-local _M = { }
+local cached_validator = lrucache.new(100)
+
+local _M = {
+  _VERSION=0.1
+}
+
+local function create_validator(schema)
+  local ok, res = pcall(jsonschema.generate_validator, schema)
+  if ok then
+    return res
+  end
+
+  return nil, res
+end
+
+local function get_validator(schema)
+  local validator, err = cached_validator:get(schema)
+  if not validator then
+    validator, err = create_validator(schema)
+    if not validator then
+      return nil, err
+    end
+    cached_validator:set(schema, validator)
+  end
+
+  return validator, nil
+end
 
 --- Validate a policy configuration
 -- Checks if a policy configuration is valid according to the given schema.
@@ -13,7 +40,10 @@ local _M = { }
 -- @treturn boolean True if the policy configuration is valid. False otherwise.
 -- @treturn string Error message only when the policy config is invalid.
 function _M.validate_config(config, config_schema)
-  local validator = jsonschema.generate_validator(config_schema or {})
+  local validator, err = get_validator(config_schema or {})
+  if not validator then
+    return false, err
+  end
   return validator(config or {})
 end
 

gateway/src/apicast/policy_loader.lua

@@ -16,6 +16,11 @@ local concat = table.concat
 local setmetatable = setmetatable
 local pcall = pcall
 
+local isempty = require('table.isempty')
+
+-- Module-level cache storage (one per worker process)
+local manifests_cache = {}
+
 local _M = {}
 
 local resty_env = require('resty.env')
@@ -70,8 +75,26 @@ local function lua_load_path(load_path)
   return format('%s/?.lua', load_path)
 end
 
+-- Get a cached manifest by policy name and version
+-- @tparam string name The policy name
+-- @tparam string version The policy version
+-- @treturn table|nil The cached manifest table, or nil if not cached
+local function get_cached_manifest(name, version)
+  local manifests = manifests_cache[name]
+  if manifests then
+    for _, manifest in ipairs(manifests) do
+      if version == manifest.version then
+        return manifest
+      end
+    end
+  end
+end
+
 local function load_manifest(name, version, path)
-  local manifest = read_manifest(path)
+  local manifest = get_cached_manifest(name, version)
+  if not manifest then
+    manifest = read_manifest(path)
+  end
 
   if manifest then
       if manifest.version ~= version then
@@ -110,8 +133,8 @@ end
 function _M:load_path(name, version, paths)
   local failures = {}
 
-  for _, path in ipairs(paths or self.policy_load_paths()) do
-    local manifest, load_path = load_manifest(name, version, format('%s/%s/%s', path, name, version) )
+  if version == 'builtin' then
+    local manifest, load_path = load_manifest(name, version, format('%s/%s', self.builtin_policy_load_path(), name) )
 
     if manifest then
       return load_path, manifest.configuration
@@ -120,8 +143,8 @@ function _M:load_path(name, version, paths)
     end
   end
 
-  if version == 'builtin' then
-    local manifest, load_path = load_manifest(name, version, format('%s/%s', self.builtin_policy_load_path(), name) )
+  for _, path in ipairs(paths or self.policy_load_paths()) do
+    local manifest, load_path = load_manifest(name, version, format('%s/%s/%s', path, name, version) )
 
     if manifest then
       return load_path, manifest.configuration
@@ -130,6 +153,7 @@ function _M:load_path(name, version, paths)
     end
   end
 
+
   return nil, nil, failures
 end
 
@@ -173,9 +197,15 @@ end
 -- Returns all the policy modules
 function _M:get_all()
   local policy_modules = {}
+  local manifests
 
-  local policy_manifests_loader = require('apicast.policy_manifests_loader')
-  local manifests = policy_manifests_loader.get_all()
+  if isempty(manifests_cache) then
+    local policy_manifests_loader = require('apicast.policy_manifests_loader')
+    manifests = policy_manifests_loader.get_all()
+    manifests_cache = manifests
+  else
+    manifests = manifests_cache
+  end
 
   for policy_name, policy_manifests in pairs(manifests) do
     for _, manifest in ipairs(policy_manifests) do

spec/configuration_loader/remote_v2_spec.lua

@@ -651,7 +651,7 @@ UwIDAQAB
     it('does not crash on empty issuer', function()
       local service = { oidc = { issuer_endpoint = '' }}
 
-      assert.falsy(loader:oidc_issuer_configuration(service))
+      assert.falsy(loader.oidc:call(service.oidc.issuer_endpoint, 0))
     end)
   end)