[Improvement-18034][alert] Add configurable timeout for script alert plugin

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtils.java

@@ -18,6 +18,7 @@
 package org.apache.dolphinscheduler.plugin.alert.script;
 
 import java.io.IOException;
+import java.util.concurrent.TimeUnit;
 
 import lombok.extern.slf4j.Slf4j;
 
@@ -29,12 +30,13 @@ private ProcessUtils() {
     }
 
     /**
-     * executeScript
+     * executeScript with timeout
      *
+     * @param timeoutSeconds timeout in seconds, if <= 0 waits indefinitely
      * @param cmd cmd params
-     * @return exit code
+     * @return exit code, -1 if error, -2 if timeout
      */
-    static Integer executeScript(String... cmd) {
+    static Integer executeScript(long timeoutSeconds, String... cmd) {
 
         int exitCode = -1;
 
@@ -46,12 +48,53 @@ static Integer executeScript(String... cmd) {
 
             inputStreamGobbler.start();
             errorStreamGobbler.start();
-            return process.waitFor();
-        } catch (IOException | InterruptedException e) {
-            log.error("execute alert script error {}", e.getMessage());
+
+            if (timeoutSeconds > 0) {
+                boolean finished = process.waitFor(timeoutSeconds, TimeUnit.SECONDS);
+                if (!finished) {
+                    log.error("script execution timed out after {} seconds, destroying process", timeoutSeconds);
+                    process.destroyForcibly();
+                    closeProcessStreams(process);
+                    joinGobbler(inputStreamGobbler);
+                    joinGobbler(errorStreamGobbler);
+                    return -2;
+                }
+            } else {
+                process.waitFor();
+            }
+            int processExitCode = process.exitValue();
+            joinGobbler(inputStreamGobbler);
+            joinGobbler(errorStreamGobbler);
+            return processExitCode;
+        } catch (InterruptedException e) {
+            log.error("execute alert script interrupted {}", e.getMessage());
             Thread.currentThread().interrupt();
+        } catch (IOException e) {
+            log.error("execute alert script error {}", e.getMessage());
         }
 
         return exitCode;
     }
+
+    private static void closeProcessStreams(Process process) {
+        try {
+            process.getInputStream().close();
+        } catch (IOException e) {
+            log.warn("Failed to close process input stream after timeout", e);
+        }
+        try {
+            process.getErrorStream().close();
+        } catch (IOException e) {
+            log.warn("Failed to close process error stream after timeout", e);
+        }
+    }
+
+    private static void joinGobbler(StreamGobbler gobbler) {
+        try {
+            gobbler.interrupt();
+            gobbler.join(TimeUnit.SECONDS.toMillis(1));
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        }
+    }
 }

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactory.java

@@ -21,10 +21,12 @@
 import org.apache.dolphinscheduler.alert.api.AlertChannelFactory;
 import org.apache.dolphinscheduler.alert.api.AlertInputTips;
 import org.apache.dolphinscheduler.common.utils.JSONUtils;
+import org.apache.dolphinscheduler.spi.params.base.DataType;
 import org.apache.dolphinscheduler.spi.params.base.ParamsOptions;
 import org.apache.dolphinscheduler.spi.params.base.PluginParams;
 import org.apache.dolphinscheduler.spi.params.base.Validate;
 import org.apache.dolphinscheduler.spi.params.input.InputParam;
+import org.apache.dolphinscheduler.spi.params.input.number.InputNumberParam;
 import org.apache.dolphinscheduler.spi.params.radio.RadioParam;
 
 import java.util.Arrays;
@@ -66,7 +68,18 @@ public List<PluginParams> params() {
                 .addValidate(Validate.newBuilder().setRequired(true).build())
                 .build();
 
-        return Arrays.asList(scriptUserParam, scriptPathParam, scriptTypeParams);
+        InputNumberParam scriptTimeoutParam =
+                InputNumberParam.newBuilder(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT,
+                        ScriptParamsConstants.SCRIPT_TIMEOUT)
+                        .setValue(ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT)
+                        .addValidate(Validate.newBuilder()
+                                .setType(DataType.NUMBER.getDataType())
+                                .setRequired(false)
+                                .setMin(0D)
+                                .build())
+                        .build();
+
+        return Arrays.asList(scriptUserParam, scriptPathParam, scriptTypeParams, scriptTimeoutParam);
     }
 
     @Override

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptParamsConstants.java

@@ -31,6 +31,12 @@ public final class ScriptParamsConstants {
 
     static final String NAME_SCRIPT_USER_PARAMS = "userParams";
 
+    static final String SCRIPT_TIMEOUT = "$t('timeout')";
+
+    static final String NAME_SCRIPT_TIMEOUT = "timeout";
+
+    static final int DEFAULT_SCRIPT_TIMEOUT = 60;
+
     private ScriptParamsConstants() {
         throw new UnsupportedOperationException("This is a utility class and cannot be instantiated");
     }

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java

@@ -36,6 +36,7 @@ public final class ScriptSender {
     private final String scriptPath;
     private final String scriptType;
     private final String userParams;
+    private final long timeout;
 
     ScriptSender(Map<String, String> config) {
         scriptPath = StringUtils.isNotBlank(config.get(ScriptParamsConstants.NAME_SCRIPT_PATH))
@@ -47,6 +48,19 @@ public final class ScriptSender {
         userParams = StringUtils.isNotBlank(config.get(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS))
                 ? config.get(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS)
                 : "";
+        String timeoutConfig = config.get(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT);
+        if (StringUtils.isNotBlank(timeoutConfig)) {
+            long parsedTimeout = ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT;
+            try {
+                parsedTimeout = Long.parseLong(timeoutConfig);
+            } catch (NumberFormatException ex) {
+                log.warn("Invalid script timeout config value: '{}', using default: {}",
+                        timeoutConfig, ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT, ex);
+            }
+            timeout = parsedTimeout;
+        } else {
+            timeout = ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT;
+        }
     }
 
     AlertResult sendScriptAlert(String title, String content) {
@@ -108,13 +122,19 @@ private AlertResult executeShellScript(String title, String content) {
 
         String[] cmd = {"/bin/sh", "-c", scriptPath + ALERT_TITLE_OPTION + "'" + title + "'" + ALERT_CONTENT_OPTION
                 + "'" + content + "'" + ALERT_USER_PARAMS_OPTION + "'" + userParams + "'"};
-        int exitCode = ProcessUtils.executeScript(cmd);
+        int exitCode = ProcessUtils.executeScript(timeout, cmd);
 
         if (exitCode == 0) {
             alertResult.setSuccess(true);
             alertResult.setMessage("send script alert msg success");
             return alertResult;
         }
+        if (exitCode == -2) {
+            alertResult.setMessage("send script alert msg error, script execution timed out after " + timeout
+                    + " seconds");
+            log.error("send script alert msg error, script execution timed out after {} seconds", timeout);
+            return alertResult;
+        }
         alertResult.setMessage("send script alert msg error,exitCode is " + exitCode);
         log.info("send script alert msg error,exitCode is {}", exitCode);
         return alertResult;

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtilsTest.java

@@ -17,7 +17,10 @@
 
 package org.apache.dolphinscheduler.plugin.alert.script;
 
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
 
 /**
  * ProcessUtilsTest
@@ -32,8 +35,17 @@
     private String[] cmd = {"/bin/sh", "-c", shellFilPath + " -t 1"};
 
     @Test
+    @DisabledOnOs(OS.WINDOWS)
     public void testExecuteScript() {
-        int code = ProcessUtils.executeScript(cmd);
-        assert code != -1;
+        int code = ProcessUtils.executeScript(60, cmd);
+        Assertions.assertNotEquals(-1, code);
+    }
+
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    public void testExecuteScriptTimeout() {
+        String[] sleepCmd = {"/bin/sh", "-c", "sleep 30"};
+        int code = ProcessUtils.executeScript(2, sleepCmd);
+        Assertions.assertEquals(-2, code);
     }
 }

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactoryTest.java

@@ -34,7 +34,7 @@ public class ScriptAlertChannelFactoryTest {
     public void testGetParams() {
         ScriptAlertChannelFactory scriptAlertChannelFactory = new ScriptAlertChannelFactory();
         List<PluginParams> params = scriptAlertChannelFactory.params();
-        Assertions.assertEquals(3, params.size());
+        Assertions.assertEquals(4, params.size());
     }
 
     @Test

dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java

@@ -27,6 +27,8 @@
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
 
 /**
  * ScriptSenderTest
@@ -35,17 +37,18 @@
 
     private static final String rootPath = System.getProperty("user.dir");
     private static final String shellFilPath = rootPath + "/src/test/script/shell/scriptExample.sh";
-    private static Map<String, String> scriptConfig = new HashMap<>();
+    private Map<String, String> scriptConfig;
 
     @BeforeEach
     public void initScriptConfig() {
-
+        scriptConfig = new HashMap<>();
         scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TYPE, String.valueOf(ScriptType.SHELL.getDescp()));
         scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "userParams");
         scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_PATH, shellFilPath);
     }
 
     @Test
+    @DisabledOnOs(OS.WINDOWS)
     public void testScriptSenderTest() {
         ScriptSender scriptSender = new ScriptSender(scriptConfig);
         AlertResult alertResult;
@@ -56,6 +59,7 @@
     }
 
     @Test
+    @DisabledOnOs(OS.WINDOWS)
     public void testScriptSenderInjectionTest() {
         scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "' ; calc.exe ; '");
         ScriptSender scriptSender = new ScriptSender(scriptConfig);
@@ -64,6 +68,7 @@
     }
 
     @Test
+    @DisabledOnOs(OS.WINDOWS)
     public void testUserParamsNPE() {
         scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, null);
         ScriptSender scriptSender = new ScriptSender(scriptConfig);
@@ -82,6 +87,7 @@
     }
 
     @Test
+    @DisabledOnOs(OS.WINDOWS)
     public void testPathError() {
         scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_PATH, "/usr/sbin/abc");
         ScriptSender scriptSender = new ScriptSender(scriptConfig);
@@ -100,4 +106,30 @@
         assertFalse(alertResult.isSuccess());
     }
 
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    public void testDefaultTimeout() {
+        ScriptSender scriptSender = new ScriptSender(scriptConfig);
+        AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content");
+        Assertions.assertTrue(alertResult.isSuccess());
+    }
+
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    public void testCustomTimeout() {
+        scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, "30");
+        ScriptSender scriptSender = new ScriptSender(scriptConfig);
+        AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content");
+        Assertions.assertTrue(alertResult.isSuccess());
+    }
+
+    @Test
+    @DisabledOnOs(OS.WINDOWS)
+    public void testInvalidTimeoutFallsBackToDefault() {
+        scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, "notANumber");
+        ScriptSender scriptSender = new ScriptSender(scriptConfig);
+        AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content");
+        Assertions.assertTrue(alertResult.isSuccess());
+    }
+
 }