diff --git a/CHANGELOG.md b/CHANGELOG.md index c20f62fd7..ef36c7850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Headers Policy: don't render template string when delete header [PR #1586](https://github.com/3scale/APIcast/pull/1586) - 3scale Batcher Policy: replace regex with string operations [PR #1583](https://github.com/3scale/APIcast/pull/1583) - Proxy/Upstream Connection: setup configuration in init phase - [PR #1602](https://github.com/3scale/APIcast/pull/1602) +- Reduce memory when reloading with large configurations [PR #1608](https://github.com/3scale/APIcast/pull/1608) ### Fixed - Correct FAPI header to `x-fapi-interaction-id` [PR #1557](https://github.com/3scale/APIcast/pull/1557) [THREESCALE-11957](https://issues.redhat.com/browse/THREESCALE-11957) diff --git a/gateway/libexec/boot.lua b/gateway/libexec/boot.lua index 325a4b5a2..5c448ea78 100644 --- a/gateway/libexec/boot.lua +++ b/gateway/libexec/boot.lua @@ -9,4 +9,7 @@ require('apicast.loader') local configuration = require 'apicast.configuration_loader' local config = configuration.boot() +if type(config) == 'table' then + config = require('cjson').encode(config) +end ngx.say(config) diff --git a/gateway/src/apicast/configuration_loader/oidc.lua b/gateway/src/apicast/configuration_loader/oidc.lua index daeb033c3..eb9f19e34 100644 --- a/gateway/src/apicast/configuration_loader/oidc.lua +++ b/gateway/src/apicast/configuration_loader/oidc.lua @@ -61,7 +61,11 @@ function _M.call(...) config.oidc = oidc - return cjson.encode(config), select(2, ...) + -- Return the decoded table instead of re-encoding to JSON. + -- configuration_parser.decode() passes tables through untouched, so + -- downstream configuration_parser.parse() won't need to re-decode it + -- either. + return config, select(2, ...) else return ... end diff --git a/gateway/src/apicast/configuration_loader/remote_v2.lua b/gateway/src/apicast/configuration_loader/remote_v2.lua index 776d0f406..74e9b652c 100644 --- a/gateway/src/apicast/configuration_loader/remote_v2.lua +++ b/gateway/src/apicast/configuration_loader/remote_v2.lua @@ -133,7 +133,7 @@ local function parse_proxy_configs(self, proxy_configs) config.oidc[i] = oidc_copy end - return cjson.encode(config) + return config end local function parse_resp_body(self, resp_body) @@ -207,7 +207,7 @@ function _M:index_per_service() configs[i] = nil end - return cjson.encode(configs) + return configs end function _M:index_custom_path(host) diff --git a/gateway/src/apicast/mapping_rule.lua b/gateway/src/apicast/mapping_rule.lua index adc6eec0c..a1edd73ce 100644 --- a/gateway/src/apicast/mapping_rule.lua +++ b/gateway/src/apicast/mapping_rule.lua @@ -23,10 +23,14 @@ local _M = { } local mt = { __index = _M } +local empty_table = {} local function hash_to_array(hash) - local array = {} + if not hash or next(hash) == nil then + return empty_table + end + local array = {} for k,v in pairs(hash or {}) do insert(array, { k, v }) end @@ -86,32 +90,26 @@ local function matches_querystring_params(params, args) end local function matches_uri(rule_pattern, uri) - return re_match(uri, format("^%s", rule_pattern), 'oj') + return re_match(uri, rule_pattern, 'oj') end local function new(http_method, pattern, params, querystring_params, metric, delta, last, owner_id, owner_type) - local self = setmetatable({}, mt) - - local querystring_parameters = hash_to_array(querystring_params) - - self.method = http_method - self.pattern = pattern - self.regexpified_pattern = regexpify(pattern) - self.parameters = params - self.system_name = metric or error('missing metric name of rule') - self.delta = delta - self.last = last or false + local self = { + querystring_parameters = hash_to_array(querystring_params), + method = http_method, + pattern = pattern, + regexpified_pattern = format("^%s", regexpify(pattern)), + parameters = params, + system_name = metric or error('missing metric name of rule'), + delta = delta, + last = last or false + } if owner_type == BackendAPIconst then self.owner_id = owner_id end - - self.querystring_params = function(args) - return matches_querystring_params(querystring_parameters, args) - end - - return self + return setmetatable(self, mt) end --- Initializes a mapping rule from a proxy rule of the service configuration. @@ -153,7 +151,7 @@ end function _M:matches(method, uri, args) local match = (self.method == self.any_method or self.method == method) and matches_uri(self.regexpified_pattern, uri) and - self.querystring_params(args) + matches_querystring_params(self.querystring_parameters, args) -- match can be nil. Convert to boolean. return match == true diff --git a/spec/configuration_loader/oidc_spec.lua b/spec/configuration_loader/oidc_spec.lua index 3310b5875..0d1058d20 100644 --- a/spec/configuration_loader/oidc_spec.lua +++ b/spec/configuration_loader/oidc_spec.lua @@ -45,7 +45,9 @@ describe('OIDC Configuration loader', function() end) it('forwards all parameters', function() - assert.same({'{"oidc":[]}', 'one', 'two'}, { loader.call('{}', 'one', 'two')}) + local result = { loader.call('{}', 'one', 'two') } + assert.same({ oidc = {} }, result[1]) + assert.same({ 'one', 'two' }, { result[2], result[3] }) end) it('gets openid configuration', function() @@ -96,7 +98,7 @@ describe('OIDC Configuration loader', function() ] } ]]) - assert.same(expected_oidc, cjson.decode(oidc)) + assert.same(expected_oidc, oidc) end) -- This is a regression test. cjson crashed when parsing a config where @@ -161,7 +163,7 @@ describe('OIDC Configuration loader', function() ] } ]]) - assert.same(expected_oidc, cjson.decode(oidc)) + assert.same(expected_oidc, oidc) end) it('handles OIDC discovery failure gracefully without crashing', function() @@ -201,7 +203,7 @@ describe('OIDC Configuration loader', function() local result = loader.call(cjson.encode(config)) assert.is_not_nil(result) - local decoded = cjson.decode(result) + local decoded = result assert.equals(2, #decoded.oidc) -- First service should have error @@ -265,7 +267,7 @@ describe('OIDC Configuration loader', function() local result = loader.call(cjson.encode(config)) assert.is_not_nil(result) - local decoded = cjson.decode(result) + local decoded = result assert.equals(1, #decoded.oidc) assert.equals(99, decoded.oidc[1].service_id) -- assert.is_not_nil(decoded.oidc[1].error) diff --git a/spec/configuration_loader/remote_v2_spec.lua b/spec/configuration_loader/remote_v2_spec.lua index f9b806991..6352afb6e 100644 --- a/spec/configuration_loader/remote_v2_spec.lua +++ b/spec/configuration_loader/remote_v2_spec.lua @@ -396,9 +396,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - assert.equals(2, #(cjson.decode(config).services)) + assert.equals(2, #(config.services)) end) it('does not crash on error when getting services', function() @@ -447,9 +447,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - assert.equals(1, #(cjson.decode(config).services)) + assert.equals(1, #(config.services)) end) describe("When using APICAST_SERVICES_FILTER_BY_URL", function() @@ -503,9 +503,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - local res_services = cjson.decode(config).services + local res_services = config.services assert.equals(1, #res_services) assert.equals(1, res_services[1].id) end) @@ -516,9 +516,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - local res_services = cjson.decode(config).services + local res_services = config.services assert.equals(2, #res_services) assert.equals(1, res_services[1].id) assert.equals(2, res_services[2].id) @@ -531,9 +531,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - local res_services = cjson.decode(config).services + local res_services = config.services assert.equals(2, #res_services) assert.equals(1, res_services[1].id) assert.equals(2, res_services[2].id) @@ -546,9 +546,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - local res_services = cjson.decode(config).services + local res_services = config.services assert.equals(2, #res_services) assert.equals(1, res_services[1].id) assert.equals(2, res_services[2].id) @@ -560,9 +560,9 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - local res_services = cjson.decode(config).services + local res_services = config.services assert.equals(2, #res_services) assert.equals(1, res_services[1].id) assert.equals(2, res_services[2].id) @@ -665,17 +665,16 @@ UwIDAQAB local config = assert(loader:index_per_service()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(11, #result_config.services) - assert.equals(11, #result_config.oidc) + assert.equals(11, #config.services) + assert.equals(11, #config.oidc) assert.same({ id_token_signing_alg_values_supported = { 'RS256' }, issuer = 'https://idp.example.com/auth/realms/foo', jwks_uri = 'https://idp.example.com/auth/realms/foo/jwks' - }, result_config.oidc[11].config) - assert.same('https://idp.example.com/auth/realms/foo', result_config.oidc[11].issuer) + }, config.oidc[11].config) + assert.same('https://idp.example.com/auth/realms/foo', config.oidc[11].issuer) assert.same({ ['3g-I9PWt6NrznPLcbE4zZrakXar27FDKEpqRPlD2i2Y'] = { e = 'AQAB', kid = '3g-I9PWt6NrznPLcbE4zZrakXar27FDKEpqRPlD2i2Y', @@ -692,7 +691,7 @@ Bw2ns0fQOZZRjWFRVh8BjkVdqa4vCAb6zw8hpR1y9uSNG+fqUAPHy5IYQaD8k8QX UwIDAQAB -----END PUBLIC KEY----- ]], } - }, result_config.oidc[11].keys) + }, config.oidc[11].keys) end) end) @@ -725,12 +724,11 @@ UwIDAQAB local config = assert(loader:index_custom_path()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) it('returns configuration for all services with host', function() @@ -749,12 +747,11 @@ UwIDAQAB local config = assert(loader:index_custom_path('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) it('returns nil and an error if the config is not a valid', function() @@ -818,18 +815,17 @@ UwIDAQAB local config = assert(loader:index_custom_path('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) assert.same({ id_token_signing_alg_values_supported = { 'RS256' }, issuer = 'https://idp.example.com/auth/realms/foo', jwks_uri = 'https://idp.example.com/auth/realms/foo/jwks' - }, result_config.oidc[1].config) - assert.same('https://idp.example.com/auth/realms/foo', result_config.oidc[1].issuer) + }, config.oidc[1].config) + assert.same('https://idp.example.com/auth/realms/foo', config.oidc[1].issuer) assert.same({ ['3g-I9PWt6NrznPLcbE4zZrakXar27FDKEpqRPlD2i2Y'] = { e = 'AQAB', kid = '3g-I9PWt6NrznPLcbE4zZrakXar27FDKEpqRPlD2i2Y', @@ -846,7 +842,7 @@ Bw2ns0fQOZZRjWFRVh8BjkVdqa4vCAb6zw8hpR1y9uSNG+fqUAPHy5IYQaD8k8QX UwIDAQAB -----END PUBLIC KEY----- ]], } - }, result_config.oidc[1].keys) + }, config.oidc[1].keys) end) it('returns configuration from master endpoint', function() @@ -865,12 +861,11 @@ UwIDAQAB local config = assert(loader:index_custom_path('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) end) @@ -918,12 +913,11 @@ UwIDAQAB local config = assert(loader:index()) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) it('returns configuration for all services with host', function() @@ -942,12 +936,11 @@ UwIDAQAB local config = assert(loader:index('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) it('returns nil and an error if the config is not a valid', function() @@ -1011,18 +1004,17 @@ UwIDAQAB local config = assert(loader:index('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) assert.same({ id_token_signing_alg_values_supported = { 'RS256' }, issuer = 'https://idp.example.com/auth/realms/foo', jwks_uri = 'https://idp.example.com/auth/realms/foo/jwks' - }, result_config.oidc[1].config) - assert.same('https://idp.example.com/auth/realms/foo', result_config.oidc[1].issuer) + }, config.oidc[1].config) + assert.same('https://idp.example.com/auth/realms/foo', config.oidc[1].issuer) assert.same({ ['3g-I9PWt6NrznPLcbE4zZrakXar27FDKEpqRPlD2i2Y'] = { e = 'AQAB', kid = '3g-I9PWt6NrznPLcbE4zZrakXar27FDKEpqRPlD2i2Y', @@ -1039,7 +1031,7 @@ Bw2ns0fQOZZRjWFRVh8BjkVdqa4vCAb6zw8hpR1y9uSNG+fqUAPHy5IYQaD8k8QX UwIDAQAB -----END PUBLIC KEY----- ]], } - }, result_config.oidc[1].keys) + }, config.oidc[1].keys) end) it('returns configuration from admin portal endpoint', function() @@ -1058,12 +1050,11 @@ UwIDAQAB local config = assert(loader:index('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) it('retuns configurations from multiple pages', function() @@ -1094,10 +1085,9 @@ UwIDAQAB local config = loader:index() assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - local result_config = cjson.decode(config) - assert.equals(2*PROXY_CONFIGS_PER_PAGE + 51, #result_config.services) + assert.equals(2*PROXY_CONFIGS_PER_PAGE + 51, #config.services) end) end) @@ -1137,8 +1127,8 @@ UwIDAQAB local config = assert(loader:call()) assert.truthy(config) - assert.equals('string', type(config)) - assert.equals(1, #(cjson.decode(config).services)) + assert.equals('table', type(config)) + assert.equals(1, #(config.services)) end) it('with custom path call index_custom_path', function() @@ -1157,12 +1147,11 @@ UwIDAQAB local config = assert(loader:call('foobar.example.com')) assert.truthy(config) - assert.equals('string', type(config)) + assert.equals('table', type(config)) - result_config = cjson.decode(config) - assert.equals(1, #result_config.services) - assert.equals(1, #result_config.oidc) - assert.same('2', result_config.oidc[1].service_id) + assert.equals(1, #config.services) + assert.equals(1, #config.oidc) + assert.same('2', config.oidc[1].service_id) end) it('by default call index', function() @@ -1180,8 +1169,8 @@ UwIDAQAB local config = assert(loader:call("foobar.example.com")) assert.truthy(config) - assert.equals('string', type(config)) - assert.equals(1, #(cjson.decode(config).services)) + assert.equals('table', type(config)) + assert.equals(1, #(config.services)) end) end) end) diff --git a/t/configuration-loading-boot-remote.t b/t/configuration-loading-boot-remote.t index 541077cf4..9732280e0 100644 --- a/t/configuration-loading-boot-remote.t +++ b/t/configuration-loading-boot-remote.t @@ -29,7 +29,7 @@ env PATH; location = /t { content_by_lua_block { local loader = require('apicast.configuration_loader.remote_v2') - ngx.say(assert(loader:call())) + ngx.say(assert(require('cjson').encode(loader:call()))) } } @@ -55,7 +55,7 @@ env PATH; location = /t { content_by_lua_block { local loader = require('apicast.configuration_loader.remote_v2') - ngx.say(assert(loader:call())) + ngx.say(assert(require('cjson').encode(loader:call()))) } } @@ -102,7 +102,7 @@ echo ' GET /t --- exit_code: 200 -=== TEST 4: retrieve config with liquid values using THREESCALE_PORTAL_ENDPOINT with path +=== TEST 3: retrieve config with liquid values using THREESCALE_PORTAL_ENDPOINT with path should not fail --- main_config env THREESCALE_PORTAL_ENDPOINT=http://127.0.0.1:$TEST_NGINX_SERVER_PORT/config; @@ -115,7 +115,7 @@ env PATH; location = /t { content_by_lua_block { local loader = require('apicast.configuration_loader.remote_v2') - ngx.say(assert(loader:call())) + ngx.say(assert(require('cjson').encode(loader:call()))) } }