From f6ca479f7a420e945bb7ea48b60a43f4eaf7034f Mon Sep 17 00:00:00 2001 From: Keith Walsh Date: Fri, 7 Jun 2024 11:07:28 -0400 Subject: [PATCH 1/2] Ensure we don't schedule reloading configuration when reloading is disabled Details of the issue as well as local reproducers are documented in https://github.com/3scale/APIcast/issues/1467. This change ensures that when `boot.init_worker` checks to see whether or not the configuration needs to be reloaded, it also checks to confirm that the `boot.ttl()` value set from `APICAST_CONFIGURATION_CACHE` is a positive number, in order to avoid an immediate and infinite recursive loop of refreshing the configuration. This should still preserve the initial fix [1] where this regression appears to have been introduced, as we should never schedule reloading with the configuration settings of: ``` APICAST_CONFIGURATION_LOADER: boot APICAST_CONFIGURATION_CACHE: -1 ``` We should also never have a situation where we'd need to account for `interval = 0` since we fail out in `boot.init` in that case [2]. Tested locally as documented in the issue, however based on the comment in the initial fix PR, I've not added any tests in code. Happy to do so if there's now a good way to accomplish that! [1] https://github.com/3scale/APIcast/pull/1399 [2] https://github.com/3scale/APIcast/blob/7e7eaf6f1d584c78d999c0d09f5b65203161c402/gateway/src/apicast/configuration_loader.lua#L146-L149 --- gateway/src/apicast/configuration_loader.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway/src/apicast/configuration_loader.lua b/gateway/src/apicast/configuration_loader.lua index f1eff77e4..200e70322 100644 --- a/gateway/src/apicast/configuration_loader.lua +++ b/gateway/src/apicast/configuration_loader.lua @@ -209,7 +209,7 @@ function boot.init_worker(configuration) -- When a worker process is (re-)spawned, -- it will start working with fresh (according the ttl semantics) configuration local boot_reserved_hosts = configuration:find_by_host(boot_reserved_domain, false) - if(#boot_reserved_hosts == 0) + if(#boot_reserved_hosts == 0 and interval > 0) then -- the boot configuration has expired, load fresh config ngx.log(ngx.INFO, 'boot time configuration has expired') From 0975fd07dcfdaff8b2c5116a421b19e8c05988cc Mon Sep 17 00:00:00 2001 From: Keith Walsh Date: Mon, 24 Jun 2024 07:51:05 -0400 Subject: [PATCH 2/2] Update to more readable fix based on PR review feedback Simplifies the conditional to reload configuration based on the interval set by `APICAST_CONFIGURATION_CACHE`. Also conditionally sets the interval to schedule. --- gateway/src/apicast/configuration_loader.lua | 23 +++++++------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/gateway/src/apicast/configuration_loader.lua b/gateway/src/apicast/configuration_loader.lua index 200e70322..9f4fb7470 100644 --- a/gateway/src/apicast/configuration_loader.lua +++ b/gateway/src/apicast/configuration_loader.lua @@ -204,22 +204,15 @@ function boot.init_worker(configuration) schedule(interval, handler, ...) end - -- Check whether the reserved boot configuration is fresh or stale. - -- If it is stale, refresh configuration - -- When a worker process is (re-)spawned, - -- it will start working with fresh (according the ttl semantics) configuration - local boot_reserved_hosts = configuration:find_by_host(boot_reserved_domain, false) - if(#boot_reserved_hosts == 0 and interval > 0) - then - -- the boot configuration has expired, load fresh config - ngx.log(ngx.INFO, 'boot time configuration has expired') - -- ngx.socket.tcp is not available at the init or init_worker phases, - -- it needs to be scheduled (with delay = 0) - schedule(0, handler, configuration) - elseif(interval > 0) - then + if interval > 0 then + -- Check whether the reserved boot configuration is fresh or stale. + -- If it is stale, refresh configuration + -- When a worker process is (re-)spawned, + -- it will start working with fresh (according the ttl semantics) configuration + local boot_reserved_hosts = configuration:find_by_host(boot_reserved_domain, false) ngx.log(ngx.DEBUG, 'schedule new configuration loading') - schedule(interval, handler, configuration) + local curr_interval = #boot_reserved_hosts == 0 and 0 or interval + schedule(curr_interval, handler, configuration) else ngx.log(ngx.DEBUG, 'no scheduling for configuration loading') end