From bd009e41c3031453009da074225a307565d1a45d Mon Sep 17 00:00:00 2001 From: Monviech <79600909+Monviech@users.noreply.github.com> Date: Fri, 27 Sep 2024 19:48:51 +0200 Subject: [PATCH] www/caddy: Force caddy to restart if a reload takes too long (#4261) * www/caddy: Patch behavior of caddy hanging during a reload or restart in some circumstances. This will avoid caddy waiting indefinitely when the NTLM module is active. After the Grace Period is over, there is a hard kill and restart. Since the template already regenerated the configuration, the new one will be used when Caddy starts. * www/caddy: Bump revision and add changelog --- www/caddy/Makefile | 1 + www/caddy/pkg-descr | 1 + .../OPNsense/Caddy/forms/general.xml | 2 +- .../mvc/app/models/OPNsense/Caddy/Caddy.xml | 4 +- .../scripts/OPNsense/Caddy/caddy_control.py | 52 ++++++++++++++++--- 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/www/caddy/Makefile b/www/caddy/Makefile index 2581e24c6..00b7379f0 100644 --- a/www/caddy/Makefile +++ b/www/caddy/Makefile @@ -1,5 +1,6 @@ PLUGIN_NAME= caddy PLUGIN_VERSION= 1.7.1 +PLUGIN_REVISION= 1 PLUGIN_DEPENDS= caddy-custom PLUGIN_COMMENT= Modern Reverse Proxy with Automatic HTTPS, Dynamic DNS and Layer4 Routing PLUGIN_MAINTAINER= cedrik@pischem.com diff --git a/www/caddy/pkg-descr b/www/caddy/pkg-descr index 6585ef223..47d8458bb 100644 --- a/www/caddy/pkg-descr +++ b/www/caddy/pkg-descr @@ -25,6 +25,7 @@ Plugin Changelog * Cleanup: Layer4, Domain and Handle dialogues have been cleaned up, some options are now hidden in advanced mode * Fix: Layer4 default ports did not render due to regression in previous version * Fix: Invert in Access Lists did not render due to regression in previous version +* Fix: When Apply takes longer than 20 seconds, Caddy will be forcefully restarted 1.7.0 diff --git a/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/forms/general.xml b/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/forms/general.xml index 3b87e2cc1..43afbb0ad 100644 --- a/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/forms/general.xml +++ b/www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/forms/general.xml @@ -64,7 +64,7 @@ text 10 - + diff --git a/www/caddy/src/opnsense/mvc/app/models/OPNsense/Caddy/Caddy.xml b/www/caddy/src/opnsense/mvc/app/models/OPNsense/Caddy/Caddy.xml index d04bda3df..c4ae85caf 100644 --- a/www/caddy/src/opnsense/mvc/app/models/OPNsense/Caddy/Caddy.xml +++ b/www/caddy/src/opnsense/mvc/app/models/OPNsense/Caddy/Caddy.xml @@ -95,8 +95,8 @@ 10 1 - 3600 - Please enter a valid Grace Period between 1 and 3600 seconds. + 20 + Please enter a valid Grace Period between 1 and 20 seconds. Y diff --git a/www/caddy/src/opnsense/scripts/OPNsense/Caddy/caddy_control.py b/www/caddy/src/opnsense/scripts/OPNsense/Caddy/caddy_control.py index add576358..8acc99f2b 100755 --- a/www/caddy/src/opnsense/scripts/OPNsense/Caddy/caddy_control.py +++ b/www/caddy/src/opnsense/scripts/OPNsense/Caddy/caddy_control.py @@ -29,14 +29,41 @@ import subprocess import json import sys +import os +import signal +import time + + +def kill_and_start_caddy(pidfile): + """ + Caddy can fail to reload in rare circumstances when + persistent keepalive connections are open with the NTLM + module active + """ + if os.path.exists(pidfile): + try: + with open(pidfile, 'r') as f: + pid = int(f.read().strip()) + + os.kill(pid, signal.SIGKILL) + time.sleep(2) + subprocess.run(["service", "caddy", "start"], check=True) + except Exception as e: + print(f"Error: {str(e)}") + else: + subprocess.run(["service", "caddy", "start"], check=True) def run_service_command(service_action, action_message): + """ + Includes special actions like a validation and + timeouts that force a restart when caddy is unresponsive + """ result = {"message": action_message} + pidfile = "/var/run/caddy/caddy.pid" if service_action == "validate": try: - # Validate the Caddyfile with explicit --config flag, capturing both stdout and stderr validation_output = subprocess.check_output( ["caddy", "validate", "--config", "/usr/local/etc/caddy/Caddyfile"], stderr=subprocess.STDOUT, text=True) @@ -44,16 +71,27 @@ def run_service_command(service_action, action_message): result["status"] = "ok" result["message"] = "Caddy configuration is valid." else: - # Search for the specific error message error_msg = next((line for line in validation_output.split('\n') if line.startswith("Error:")), "Caddy configuration is not valid.") result["status"] = "failed" result["message"] = error_msg except subprocess.CalledProcessError as e: - # Extracting only the specific "Error: ..." line from the output error_msg = next((line for line in e.output.split('\n') if line.startswith("Error:")), "Validation failed.") result["status"] = "failed" result["message"] = error_msg + elif service_action in ["stop", "restart", "reloadssl"]: + try: + proc = subprocess.Popen(["service", "caddy", service_action]) + try: + proc.wait(timeout=20) + result["status"] = "ok" + except subprocess.TimeoutExpired: + kill_and_start_caddy(pidfile) + result["status"] = "ok" + result["message"] = f"{service_action.capitalize()} took too long, Caddy was forcefully restarted." + except subprocess.CalledProcessError as e: + result["status"] = "failed" + result["message"] = str(e) else: try: subprocess.run(["service", "caddy", service_action], check=True) @@ -71,14 +109,12 @@ actions = { "stop": "stop", "restart": "restart", "reload": "reloadssl", - # Reloadssl reloads even if the config in the Caddyfile is unchanged, using an extra command of the rc.d script, - # forcing certificates in the filesystem to be reloaded. - "validate": "validate" # Validate action + "validate": "validate" } if __name__ == "__main__": if len(sys.argv) > 1: - action = sys.argv[1] # Get the action from the command-line argument + action = sys.argv[1] if action in actions: cmd_action = action service_action = actions[action] @@ -86,7 +122,7 @@ if __name__ == "__main__": # Call setup script for 'validate' and 'reloadssl' actions. This is needed because the setup script triggers # the caddy_certs.php script, which exports all certificates into the filesystem. Caddy reloads certificates - # when reloadssl is used. Because it is a non standard command, the caddy_setup script will not be triggered + # when reloadssl is used. Because it is a non-standard command, the caddy_setup script will not be triggered # in /etc/rc.conf.d/caddy. The validate command needs it to make sure all certificates are in the filesystem, # because otherwise the validation fails. if service_action in ["validate", "reloadssl"]: