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,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

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
 

www/caddy/src/opnsense/mvc/app/controllers/OPNsense/Caddy/forms/general.xml

@@ -64,7 +64,7 @@
             <label>Grace Period</label>
             <type>text</type>
             <hint>10</hint>
-            <help><![CDATA[Defines the grace period for shutting down Caddy during a reload in seconds. During the grace period, no new connections are accepted, idle connections are closed, and active connections are impatiently waited upon to finish their requests. If clients do not finish their requests within the grace period, the server will be forcefully terminated to allow the reload to complete and free up resources. This can influence how long "Apply" of new configurations take, since Caddy waits for all open connections to close.]]></help>
+            <help><![CDATA[Defines the grace period for shutting down Caddy during a reload in seconds. If clients do not finish their requests within the grace period, the server will be forcefully terminated to allow the reload to complete and free up resources. This can influence how long "Apply" of new configurations take, since Caddy waits for all open connections to close. If the grace period is over and Caddy is unresponsive, there will be a forced kill and service restart.]]></help>
         </field>
     </tab>
     <tab id="general-logsettings" description="Log Settings">

www/caddy/src/opnsense/mvc/app/models/OPNsense/Caddy/Caddy.xml

@@ -95,8 +95,8 @@
             <GracePeriod type="IntegerField">
                 <Default>10</Default>
                 <MinimumValue>1</MinimumValue>
-                <MaximumValue>3600</MaximumValue>
-                <ValidationMessage>Please enter a valid Grace Period between 1 and 3600 seconds.</ValidationMessage>
+                <MaximumValue>20</MaximumValue>
+                <ValidationMessage>Please enter a valid Grace Period between 1 and 20 seconds.</ValidationMessage>
                 <Required>Y</Required>
             </GracePeriod>
             <HttpVersion type="OptionField">

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"]: