From 19d77029436647d88d0533d9be12a28b284056c2 Mon Sep 17 00:00:00 2001 From: HarshXAI <113297734+IamPossible007@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:35:25 +0530 Subject: [PATCH 1/3] feat(php): add ScriptTrigger and CommandsTrigger Mirror the dotnet/perl polling trigger pattern for the PHP plugin: KV-backed edge mode, exitCondition validation, and integration tests. --- AGENTS.md | 2 + .../plugin/scripts/php/CommandsTrigger.java | 306 +++++++++++++++++ .../plugin/scripts/php/ScriptTrigger.java | 311 ++++++++++++++++++ .../scripts/php/CommandsTriggerTest.java | 117 +++++++ .../plugin/scripts/php/EdgeStateTest.java | 235 +++++++++++++ .../php/ScriptTriggerEvaluateTest.java | 82 +++++ .../plugin/scripts/php/ScriptTriggerTest.java | 77 +++++ 7 files changed, 1130 insertions(+) create mode 100644 plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java create mode 100644 plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java create mode 100644 plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java create mode 100644 plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java create mode 100644 plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java create mode 100644 plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java diff --git a/AGENTS.md b/AGENTS.md index 4601ab03..b744d61c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -105,7 +105,9 @@ This is a **multi-module** plugin with 19 submodules: **plugin-script-php:** - `io.kestra.plugin.scripts.php.Commands` +- `io.kestra.plugin.scripts.php.CommandsTrigger` - `io.kestra.plugin.scripts.php.Script` +- `io.kestra.plugin.scripts.php.ScriptTrigger` **plugin-script-powershell:** - `io.kestra.plugin.scripts.powershell.Commands` diff --git a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java new file mode 100644 index 00000000..54555b81 --- /dev/null +++ b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java @@ -0,0 +1,306 @@ +package io.kestra.plugin.scripts.php; + +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import io.kestra.core.models.annotations.Example; +import io.kestra.core.models.annotations.Plugin; +import io.kestra.core.models.annotations.PluginProperty; +import io.kestra.core.models.conditions.ConditionContext; +import io.kestra.core.models.executions.Execution; +import io.kestra.core.models.property.Property; +import io.kestra.core.models.tasks.RunnableTaskException; +import io.kestra.core.models.tasks.runners.TaskException; +import io.kestra.core.models.triggers.AbstractTrigger; +import io.kestra.core.models.triggers.PollingTriggerInterface; +import io.kestra.core.models.triggers.TriggerContext; +import io.kestra.core.models.triggers.TriggerOutput; +import io.kestra.core.models.triggers.TriggerService; +import io.kestra.core.runners.RunContext; +import io.kestra.core.storages.kv.KVStore; +import io.kestra.core.storages.kv.KVValueAndMetadata; +import io.kestra.plugin.scripts.exec.TriggerRunContext; +import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; + +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotNull; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.ToString; +import lombok.experimental.SuperBuilder; + +@SuperBuilder +@ToString +@EqualsAndHashCode +@Getter +@NoArgsConstructor +@Schema( + title = "Trigger a flow when PHP commands match a condition", + description = """ + Polls by running PHP commands in a script container (default image php) and emits when exitCondition matches. \ + Edge mode (the default) emits only on a transition from not matching to matching, remembering the previous result in the namespace KV store. \ + Polls every 60s by default. Accepts 'exit N' or a regex (fallback substring) matched against emitted vars; a failed run has no vars, so only 'exit N' can match a failure. + """ +) +@Plugin( + examples = { + @Example( + title = "Trigger when commands fail with an implicit error (exit 1).", + full = true, + code = """ + id: php_commands_trigger + namespace: company.team + + triggers: + - id: commands_failure + type: io.kestra.plugin.scripts.php.CommandsTrigger + interval: PT60S + exitCondition: "exit 1" + edge: true + containerImage: php + commands: + - php -r 'exit(1);' + + tasks: + - id: log + type: io.kestra.plugin.core.log.Log + message: "Triggered with exitCode={{ trigger.exitCode }} (condition={{ trigger.condition }})" + """ + ) + } +) +public class CommandsTrigger extends AbstractTrigger + implements PollingTriggerInterface, TriggerOutput { + + private static final String DEFAULT_IMAGE = "php"; + private static final Pattern EXIT_CONDITION_PATTERN = Pattern.compile("^\\s*exit\\s+(\\d+)\\s*$", Pattern.CASE_INSENSITIVE); + + @Schema( + title = "Docker image used to execute the commands", + description = """ + Container image used by the underlying Commands task to run PHP commands. + Defaults to 'php'. + """ + ) + @Builder.Default + @PluginProperty(group = "execution") + protected Property containerImage = Property.ofValue(DEFAULT_IMAGE); + + @Schema( + title = "PHP commands to execute", + description = "Commands executed on each poll (same semantics as the PHP Commands task)." + ) + @NotNull + @PluginProperty(group = "main") + protected Property> commands; + + @Schema( + title = "Condition to match", + description = """ + Condition evaluated after each commands execution. The trigger emits an event only when this condition matches. + + Supported forms: + - 'exit N' (example: 'exit 1'): matches when the process exit code equals N. + - Any other string: treated as a regex (or substring if regex is invalid) matched against the + task's 'vars' (when commands emit ::{"outputs":...}::). On a failed run no vars are available + to match against, so only an 'exit N' condition can match a failure. + """ + ) + @NotNull + @PluginProperty(group = "main") + protected Property exitCondition; + + @Schema( + title = "Check interval", + description = "Interval between polling evaluations." + ) + @Builder.Default + @PluginProperty(group = "execution") + private final Duration interval = Duration.ofSeconds(60); + + @Schema( + title = "Edge trigger mode", + description = """ + If true (default), the trigger emits only on a transition from 'not matching' to 'matching' \ + (anti-spam). The previous result is kept in the namespace KV store, keyed by flow and trigger id. \ + If false, the trigger emits on every poll where the condition matches. + """ + ) + @Builder.Default + @PluginProperty(group = "advanced") + protected Property edge = Property.ofValue(true); + + @Override + public Optional evaluate(ConditionContext conditionContext, TriggerContext context) throws Exception { + RunContext runContext = conditionContext.getRunContext(); + boolean renderedEdge = runContext.render(this.edge).as(Boolean.class).orElse(true); + + Output out; + try { + out = runOnce(runContext); + } catch (Exception e) { + runContext.logger().warn("Trigger evaluation failed, returning empty result to avoid blocking the scheduler", e); + return Optional.empty(); + } + + boolean matched = matchesCondition(out); + + boolean emit = shouldEmit(runContext, context, renderedEdge, matched); + + if (!emit) { + return Optional.empty(); + } + + return Optional.of(TriggerService.generateExecution(this, conditionContext, context, out)); + } + + boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, boolean matched) throws Exception { + if (!edge) { + return matched; + } + + // A polling trigger is rebuilt from the flow definition (and serialized to a worker) on + // every poll, so the previous result cannot live in a field. It is kept in the namespace + // KV store instead and advanced on every poll. + KVStore kvStore = runContext.namespaceKv(context.getNamespace()); + String key = edgeStateKey(context); + + boolean previouslyMatched = kvStore.getValue(key) + .map(value -> Boolean.parseBoolean(String.valueOf(value.value()))) + .orElse(false); + + // Skip the write on a poll that repeats the same result, so a condition that stays true + // for hours does not rewrite the same value every interval. + if (matched != previouslyMatched) { + kvStore.put(key, new KVValueAndMetadata(null, matched)); + } + + return matched && !previouslyMatched; + } + + // Length prefixed so that the pairs ("a-b", "c") and ("a", "b-c") can never share a key. + // Flow and trigger ids only use characters that are valid in a KV key. + static String edgeStateKey(TriggerContext context) { + return "trigger-edge-" + context.getFlowId().length() + "-" + context.getFlowId() + "-" + context.getTriggerId(); + } + + private Output runOnce(RunContext runContext) throws Exception { + Commands task = Commands.builder() + .id(this.getId()) + .type(Commands.class.getName()) + .containerImage(this.containerImage) + .commands(this.commands) + .build(); + + String renderedCondition = runContext.render(this.exitCondition).as(String.class) + .filter(condition -> !condition.isBlank()) + .orElseThrow(() -> new IllegalArgumentException("exitCondition must render to a non-empty value")); + + try { + ScriptOutput taskOutput = task.run(TriggerRunContext.forEmbeddedTask(runContext, task)); + Integer exitCode = safeExitCode(taskOutput); + Map vars = safeVars(taskOutput); + + return new Output(Instant.now(), renderedCondition, exitCode, vars); + } catch (RunnableTaskException e) { + ExtractedFailure failure = extractFailure(e); + return new Output(Instant.now(), renderedCondition, failure.exitCode, null); + } + } + + boolean matchesCondition(Output out) { + String cond = out.getCondition() == null ? "" : out.getCondition().trim(); + + Matcher exitMatcher = EXIT_CONDITION_PATTERN.matcher(cond); + if (exitMatcher.matches()) { + int expected = Integer.parseInt(exitMatcher.group(1)); + return out.getExitCode() != null && out.getExitCode() == expected; + } + + String haystack = buildHaystack(out); + if (haystack.isEmpty() || cond.isEmpty()) { + return false; + } + + try { + return Pattern.compile(cond).matcher(haystack).find(); + } catch (Exception invalidRegex) { + return haystack.contains(cond); + } + } + + private String buildHaystack(Output out) { + if (out.getVars() == null || out.getVars().isEmpty()) { + return ""; + } + return out.getVars().toString(); + } + + private Integer safeExitCode(ScriptOutput taskOutput) { + try { + return taskOutput.getExitCode(); + } catch (Exception ignored) { + return null; + } + } + + private Map safeVars(ScriptOutput taskOutput) { + try { + return taskOutput.getVars(); + } catch (Exception ignored) { + return null; + } + } + + private record ExtractedFailure(Integer exitCode) { + } + + private ExtractedFailure extractFailure(RunnableTaskException e) { + Integer exitCode = null; + + Throwable cur = e.getCause(); + while (cur != null) { + if (cur instanceof TaskException te) { + exitCode = te.getExitCode(); + break; + } + cur = cur.getCause(); + } + + return new ExtractedFailure(exitCode); + } + + @Data + @AllArgsConstructor + public static class Output implements io.kestra.core.models.tasks.Output { + @Schema(title = "Timestamp of the event that fired the trigger") + private Instant timestamp; + + @Schema( + title = "Rendered condition", + description = "Rendered value of the exitCondition property for this poll." + ) + private String condition; + + @Schema( + title = "Commands exit code", + description = "Exit code returned by the PHP process (may be null if not available)." + ) + private Integer exitCode; + + @Schema( + title = "Commands vars", + description = "Vars produced by the task (e.g. via ::{\"outputs\":{...}}:: convention)." + ) + private Map vars; + } +} diff --git a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java new file mode 100644 index 00000000..0af1c14b --- /dev/null +++ b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java @@ -0,0 +1,311 @@ +package io.kestra.plugin.scripts.php; + +import java.time.Duration; +import java.time.Instant; +import java.util.Map; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import io.kestra.core.models.annotations.Example; +import io.kestra.core.models.annotations.Plugin; +import io.kestra.core.models.annotations.PluginProperty; +import io.kestra.core.models.enums.MonacoLanguages; +import io.kestra.core.models.conditions.ConditionContext; +import io.kestra.core.models.executions.Execution; +import io.kestra.core.models.property.Property; +import io.kestra.core.models.tasks.RunnableTaskException; +import io.kestra.core.models.tasks.runners.TaskException; +import io.kestra.core.models.triggers.AbstractTrigger; +import io.kestra.core.models.triggers.PollingTriggerInterface; +import io.kestra.core.models.triggers.TriggerContext; +import io.kestra.core.models.triggers.TriggerOutput; +import io.kestra.core.models.triggers.TriggerService; +import io.kestra.core.runners.RunContext; +import io.kestra.core.storages.kv.KVStore; +import io.kestra.core.storages.kv.KVValueAndMetadata; +import io.kestra.plugin.scripts.exec.TriggerRunContext; +import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; + +import io.swagger.v3.oas.annotations.media.Schema; +import jakarta.validation.constraints.NotNull; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.ToString; +import lombok.experimental.SuperBuilder; + +@SuperBuilder +@ToString +@EqualsAndHashCode +@Getter +@NoArgsConstructor +@Schema( + title = "Trigger on PHP script condition", + description = """ + Polls by running an inline PHP script in a container (default image php) and emits when exitCondition matches. \ + Edge mode (the default) emits only on a transition from not matching to matching, remembering the previous result in the namespace KV store. \ + Polls every 60s by default. Accepts 'exit N' or a regex (fallback substring) matched against emitted vars; a failed run has no vars, so only 'exit N' can match a failure. + """ +) +@Plugin( + examples = { + @Example( + title = "Trigger when the script fails with an implicit error (exit 1).", + full = true, + code = """ + id: php_script_trigger + namespace: company.team + + triggers: + - id: script_failure + type: io.kestra.plugin.scripts.php.ScriptTrigger + interval: PT60S + exitCondition: "exit 1" + edge: true + containerImage: php + script: | + { + + private static final String DEFAULT_IMAGE = "php"; + private static final Pattern EXIT_CONDITION_PATTERN = Pattern.compile("^\\s*exit\\s+(\\d+)\\s*$", Pattern.CASE_INSENSITIVE); + + @Schema( + title = "Container image for script execution", + description = """ + Image used by the Script task to run the inline PHP script; defaults to 'php'. + Provide an image that includes the PHP runtime and any required extensions. + """ + ) + @Builder.Default + @PluginProperty(group = "execution") + protected Property containerImage = Property.ofValue(DEFAULT_IMAGE); + + @Schema( + title = "Inline PHP script", + description = """ + Multi-line script executed on each poll, with the same semantics as the PHP Script task. + """ + ) + @NotNull + @PluginProperty(language = MonacoLanguages.PHP, group = "main") + protected Property script; + + @Schema( + title = "Condition to match", + description = """ + Rendered condition evaluated after each execution; the trigger emits only when it matches. + 'exit N' compares the exit code, otherwise the string is used as a regex (or substring fallback) \ + against emitted vars (from ::{"outputs":...}::). On a failed run no vars are available to match \ + against, so only an 'exit N' condition can match a failure. + """ + ) + @NotNull + @PluginProperty(group = "main") + protected Property exitCondition; + + @Schema( + title = "Check interval", + description = """ + Interval between polls; default PT60S. The scheduler uses this to schedule the next evaluation. + """ + ) + @Builder.Default + @PluginProperty(group = "execution") + private final Duration interval = Duration.ofSeconds(60); + + @Schema( + title = "Edge trigger mode", + description = """ + When true (default), emit only on a transition from not matching to matching, so a condition that \ + stays true does not fire on every poll. The previous result is kept in the namespace KV store, keyed \ + by flow and trigger id. When false, emit on every poll that matches. + """ + ) + @Builder.Default + @PluginProperty(group = "advanced") + protected Property edge = Property.ofValue(true); + + @Override + public Optional evaluate(ConditionContext conditionContext, TriggerContext context) throws Exception { + RunContext runContext = conditionContext.getRunContext(); + boolean renderedEdge = runContext.render(this.edge).as(Boolean.class).orElse(true); + + Output out; + try { + out = runOnce(runContext); + } catch (Exception e) { + runContext.logger().warn("Trigger evaluation failed, returning empty result to avoid blocking the scheduler", e); + return Optional.empty(); + } + + boolean matched = matchesCondition(out); + + boolean emit = shouldEmit(runContext, context, renderedEdge, matched); + + if (!emit) { + return Optional.empty(); + } + + return Optional.of(TriggerService.generateExecution(this, conditionContext, context, out)); + } + + boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, boolean matched) throws Exception { + if (!edge) { + return matched; + } + + // A polling trigger is rebuilt from the flow definition (and serialized to a worker) on + // every poll, so the previous result cannot live in a field. It is kept in the namespace + // KV store instead and advanced on every poll. + KVStore kvStore = runContext.namespaceKv(context.getNamespace()); + String key = edgeStateKey(context); + + boolean previouslyMatched = kvStore.getValue(key) + .map(value -> Boolean.parseBoolean(String.valueOf(value.value()))) + .orElse(false); + + // Skip the write on a poll that repeats the same result, so a condition that stays true + // for hours does not rewrite the same value every interval. + if (matched != previouslyMatched) { + kvStore.put(key, new KVValueAndMetadata(null, matched)); + } + + return matched && !previouslyMatched; + } + + // Length prefixed so that the pairs ("a-b", "c") and ("a", "b-c") can never share a key. + // Flow and trigger ids only use characters that are valid in a KV key. + static String edgeStateKey(TriggerContext context) { + return "trigger-edge-" + context.getFlowId().length() + "-" + context.getFlowId() + "-" + context.getTriggerId(); + } + + private Output runOnce(RunContext runContext) throws Exception { + Script task = Script.builder() + .id(this.getId()) + .type(Script.class.getName()) + .containerImage(this.containerImage) + .script(this.script) + .build(); + + String renderedExitCondition = runContext.render(this.exitCondition).as(String.class) + .filter(condition -> !condition.isBlank()) + .orElseThrow(() -> new IllegalArgumentException("exitCondition must render to a non-empty value")); + + try { + ScriptOutput taskOutput = task.run(TriggerRunContext.forEmbeddedTask(runContext, task)); + Integer exitCode = safeExitCode(taskOutput); + Map vars = safeVars(taskOutput); + + return new Output(Instant.now(), renderedExitCondition, exitCode, vars); + } catch (RunnableTaskException e) { + ExtractedFailure failure = extractFailure(e); + return new Output(Instant.now(), renderedExitCondition, failure.exitCode, null); + } + } + + boolean matchesCondition(Output out) { + String cond = out.getCondition() == null ? "" : out.getCondition().trim(); + + Matcher exitMatcher = EXIT_CONDITION_PATTERN.matcher(cond); + if (exitMatcher.matches()) { + int expected = Integer.parseInt(exitMatcher.group(1)); + return out.getExitCode() != null && out.getExitCode() == expected; + } + + String haystack = buildHaystack(out); + if (haystack.isEmpty() || cond.isEmpty()) { + return false; + } + + try { + return Pattern.compile(cond).matcher(haystack).find(); + } catch (Exception invalidRegex) { + return haystack.contains(cond); + } + } + + private String buildHaystack(Output out) { + if (out.getVars() == null || out.getVars().isEmpty()) { + return ""; + } + return out.getVars().toString(); + } + + private Integer safeExitCode(ScriptOutput taskOutput) { + try { + return taskOutput.getExitCode(); + } catch (Exception ignored) { + return null; + } + } + + private Map safeVars(ScriptOutput taskOutput) { + try { + return taskOutput.getVars(); + } catch (Exception ignored) { + return null; + } + } + + private record ExtractedFailure(Integer exitCode) { + } + + private ExtractedFailure extractFailure(RunnableTaskException e) { + Integer exitCode = null; + + Throwable cur = e.getCause(); + while (cur != null) { + if (cur instanceof TaskException te) { + exitCode = te.getExitCode(); + break; + } + cur = cur.getCause(); + } + + return new ExtractedFailure(exitCode); + } + + @Data + @AllArgsConstructor + public static class Output implements io.kestra.core.models.tasks.Output { + @Schema(title = "Timestamp of the event that fired the trigger") + private Instant timestamp; + + @Schema( + title = "Rendered condition", + description = "Rendered value of the exitCondition property for this poll." + ) + private String condition; + + @Schema( + title = "Script exit code", + description = "Exit code returned by the PHP process (may be null if not available)." + ) + private Integer exitCode; + + @Schema( + title = "Script vars", + description = """ + Vars produced by the task (e.g. via ::{"outputs":{...}}:: convention). This is the main structured + way to evaluate non-exit conditions on successful runs. + """ + ) + private Map vars; + } +} diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java new file mode 100644 index 00000000..149fdfa2 --- /dev/null +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java @@ -0,0 +1,117 @@ +package io.kestra.plugin.scripts.php; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.junit.jupiter.api.Test; + +import io.kestra.core.junit.annotations.KestraTest; +import io.kestra.core.models.executions.Execution; +import io.kestra.core.models.property.Property; +import io.kestra.core.runners.RunContextFactory; +import io.kestra.core.serializers.JacksonMapper; +import io.kestra.core.utils.IdUtils; +import io.kestra.core.utils.TestsUtils; + +import jakarta.inject.Inject; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; + +@KestraTest +class CommandsTriggerTest { + @Inject + private RunContextFactory runContextFactory; + + @Test + void commandsTrigger_shouldTriggerOnImplicitFailureExit1() throws Exception { + CommandsTrigger trigger = CommandsTrigger.builder() + .id("commands-trigger-" + IdUtils.create()) + .type(CommandsTrigger.class.getName()) + .exitCondition(Property.ofValue("exit 1")) + .edge(Property.ofValue(true)) + .containerImage(Property.ofValue("php")) + .commands(Property.ofValue(List.of("php -r 'exit(1);'"))) + .build(); + + var context = TestsUtils.mockTrigger(runContextFactory, trigger); + Optional execution = trigger.evaluate(context.getKey(), context.getValue()); + + assertThat(execution.isPresent(), is(true)); + + Map triggerVars = execution.get().getTrigger().getVariables(); + assertThat("condition should be present", triggerVars.get("condition"), is("exit 1")); + assertThat("exitCode should be present", triggerVars.get("exitCode"), notNullValue()); + assertThat("exitCode should be 1", triggerVars.get("exitCode"), is(1)); + assertThat("timestamp should be present", triggerVars.get("timestamp"), notNullValue()); + } + + @Test + void commandsTrigger_shouldTriggerOnStdoutMatchUsingStructuredOutputs() throws Exception { + CommandsTrigger trigger = CommandsTrigger.builder() + .id("commands-stdout-match-trigger-" + IdUtils.create()) + .type(CommandsTrigger.class.getName()) + .exitCondition(Property.ofValue("toto")) + .edge(Property.ofValue(true)) + .containerImage(Property.ofValue("php")) + .commands(Property.ofValue(List.of("echo '::{\"outputs\":{\"listing\":\"toto\"}}::'"))) + .build(); + + var context = TestsUtils.mockTrigger(runContextFactory, trigger); + Optional execution = trigger.evaluate(context.getKey(), context.getValue()); + + assertThat(execution.isPresent(), is(true)); + + Map triggerVars = execution.get().getTrigger().getVariables(); + assertThat("condition should be present", triggerVars.get("condition"), is("toto")); + assertThat("exitCode should be present", triggerVars.get("exitCode"), notNullValue()); + assertThat("exitCode should be 0", triggerVars.get("exitCode"), is(0)); + assertThat("timestamp should be present", triggerVars.get("timestamp"), notNullValue()); + assertThat("vars should be present", triggerVars.get("vars"), notNullValue()); + } + + @Test + void commandsTrigger_edgeModeShouldSuppressSecondEmission() throws Exception { + CommandsTrigger trigger = CommandsTrigger.builder() + .id("commands-edge-trigger-" + IdUtils.create()) + .type(CommandsTrigger.class.getName()) + .exitCondition(Property.ofValue("exit 1")) + .edge(Property.ofValue(true)) + .containerImage(Property.ofValue("php")) + .commands(Property.ofValue(List.of("php -r 'exit(1);'"))) + .build(); + + var context = TestsUtils.mockTrigger(runContextFactory, trigger); + Optional first = trigger.evaluate(context.getKey(), context.getValue()); + assertThat("First evaluation should fire", first.isPresent(), is(true)); + + // The second poll runs on a copy that went through the worker's serialize/deserialize round trip. + CommandsTrigger nextPoll = JacksonMapper.ofJson().readValue(JacksonMapper.ofJson().writeValueAsString(trigger), CommandsTrigger.class); + context = TestsUtils.mockTrigger(runContextFactory, nextPoll); + Optional second = nextPoll.evaluate(context.getKey(), context.getValue()); + assertThat("Edge mode should suppress repeated emission", second.isPresent(), is(false)); + } + + @Test + void commandsTrigger_shouldMatchRegexAgainstStructuredOutputs() throws Exception { + CommandsTrigger trigger = CommandsTrigger.builder() + .id("commands-regex-trigger-" + IdUtils.create()) + .type(CommandsTrigger.class.getName()) + .exitCondition(Property.ofValue("status=\\w+")) + .edge(Property.ofValue(true)) + .containerImage(Property.ofValue("php")) + .commands(Property.ofValue(List.of("echo '::{\"outputs\":{\"status\":\"status=ready\"}}::'"))) + .build(); + + var context = TestsUtils.mockTrigger(runContextFactory, trigger); + Optional execution = trigger.evaluate(context.getKey(), context.getValue()); + + assertThat("Regex condition should match", execution.isPresent(), is(true)); + + Map triggerVars = execution.get().getTrigger().getVariables(); + assertThat("exitCode should be 0", triggerVars.get("exitCode"), is(0)); + assertThat("vars should be present", triggerVars.get("vars"), notNullValue()); + } +} diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java new file mode 100644 index 00000000..329f36c7 --- /dev/null +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java @@ -0,0 +1,235 @@ +package io.kestra.plugin.scripts.php; + +import java.time.ZonedDateTime; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import io.kestra.core.junit.annotations.KestraTest; +import io.kestra.core.models.property.Property; +import io.kestra.core.models.triggers.Trigger; +import io.kestra.core.models.triggers.TriggerContext; +import io.kestra.core.runners.RunContext; +import io.kestra.core.runners.RunContextFactory; +import io.kestra.core.serializers.JacksonMapper; +import io.kestra.core.storages.kv.KVStore; +import io.kestra.core.utils.IdUtils; +import io.kestra.core.utils.TestsUtils; + +import jakarta.inject.Inject; + +import static io.kestra.core.tenant.TenantService.MAIN_TENANT; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; + +/** + * Edge mode keeps its previous result in the namespace KV store. None of these tests need Docker: + * they drive shouldEmit directly, so only the state handling is under test. + * + * The KV store used by the test config is a local folder that survives between runs, so every + * test uses a unique trigger id. + */ +@KestraTest +class EdgeStateTest { + @Inject + private RunContextFactory runContextFactory; + + private static ScriptTrigger scriptTrigger(String id) { + return ScriptTrigger.builder() + .id(id) + .type(ScriptTrigger.class.getName()) + .exitCondition(Property.ofValue("exit 1")) + .script(Property.ofValue("unused")) + .build(); + } + + private static CommandsTrigger commandsTrigger(String id) { + return CommandsTrigger.builder() + .id(id) + .type(CommandsTrigger.class.getName()) + .exitCondition(Property.ofValue("exit 1")) + .commands(Property.ofValue(List.of("unused"))) + .build(); + } + + // The scheduler hands every poll a freshly deserialized trigger, so each poll here does the same. + private static T freshCopy(T trigger, Class type) throws Exception { + return JacksonMapper.ofJson().readValue(JacksonMapper.ofJson().writeValueAsString(trigger), type); + } + + private static TriggerContext contextFor(String flowId, String triggerId, String namespace) { + return Trigger.builder() + .triggerId(triggerId) + .flowId(flowId) + .tenantId(MAIN_TENANT) + .namespace(namespace) + .date(ZonedDateTime.now()) + .build(); + } + + @Test + void scriptTrigger_edgeEmitsOnlyOnTransitionAcrossFreshInstances() throws Exception { + ScriptTrigger original = scriptTrigger("script-edge-" + IdUtils.create()); + var mock = TestsUtils.mockTrigger(runContextFactory, original); + RunContext runContext = mock.getKey().getRunContext(); + TriggerContext context = mock.getValue(); + + boolean[] polls = { false, true, true, true, false, false, true }; + boolean[] expected = { false, true, false, false, false, false, true }; + + for (int i = 0; i < polls.length; i++) { + boolean emit = freshCopy(original, ScriptTrigger.class).shouldEmit(runContext, context, true, polls[i]); + assertThat("poll " + i + " (matched=" + polls[i] + ")", emit, is(expected[i])); + } + } + + @Test + void commandsTrigger_edgeEmitsOnlyOnTransitionAcrossFreshInstances() throws Exception { + CommandsTrigger original = commandsTrigger("commands-edge-" + IdUtils.create()); + var mock = TestsUtils.mockTrigger(runContextFactory, original); + RunContext runContext = mock.getKey().getRunContext(); + TriggerContext context = mock.getValue(); + + boolean[] polls = { false, true, true, true, false, false, true }; + boolean[] expected = { false, true, false, false, false, false, true }; + + for (int i = 0; i < polls.length; i++) { + boolean emit = freshCopy(original, CommandsTrigger.class).shouldEmit(runContext, context, true, polls[i]); + assertThat("poll " + i + " (matched=" + polls[i] + ")", emit, is(expected[i])); + } + } + + @Test + void scriptTrigger_edgeDisabledEmitsOnEveryMatchAndKeepsNoState() throws Exception { + ScriptTrigger original = scriptTrigger("script-noedge-" + IdUtils.create()); + var mock = TestsUtils.mockTrigger(runContextFactory, original); + RunContext runContext = mock.getKey().getRunContext(); + TriggerContext context = mock.getValue(); + + assertThat(original.shouldEmit(runContext, context, false, true), is(true)); + assertThat(original.shouldEmit(runContext, context, false, true), is(true)); + assertThat(original.shouldEmit(runContext, context, false, false), is(false)); + + assertThat( + "edge=false must not write to the KV store", + runContext.namespaceKv(context.getNamespace()).getValue(ScriptTrigger.edgeStateKey(context)).isPresent(), + is(false) + ); + } + + @Test + void commandsTrigger_edgeDisabledEmitsOnEveryMatchAndKeepsNoState() throws Exception { + CommandsTrigger original = commandsTrigger("commands-noedge-" + IdUtils.create()); + var mock = TestsUtils.mockTrigger(runContextFactory, original); + RunContext runContext = mock.getKey().getRunContext(); + TriggerContext context = mock.getValue(); + + assertThat(original.shouldEmit(runContext, context, false, true), is(true)); + assertThat(original.shouldEmit(runContext, context, false, true), is(true)); + assertThat(original.shouldEmit(runContext, context, false, false), is(false)); + + assertThat( + "edge=false must not write to the KV store", + runContext.namespaceKv(context.getNamespace()).getValue(CommandsTrigger.edgeStateKey(context)).isPresent(), + is(false) + ); + } + + @Test + void scriptTrigger_stateIsScopedToFlowAndTrigger() throws Exception { + String triggerId = "script-scope-" + IdUtils.create(); + ScriptTrigger trigger = scriptTrigger(triggerId); + var mock = TestsUtils.mockTrigger(runContextFactory, trigger); + RunContext runContext = mock.getKey().getRunContext(); + String namespace = mock.getValue().getNamespace(); + + TriggerContext flowA = contextFor("flow-a", triggerId, namespace); + TriggerContext flowB = contextFor("flow-b", triggerId, namespace); + TriggerContext otherTriggerInFlowA = contextFor("flow-a", triggerId + "-other", namespace); + + assertThat(trigger.shouldEmit(runContext, flowA, true, true), is(true)); + assertThat("same trigger id in another flow has its own state", trigger.shouldEmit(runContext, flowB, true, true), is(true)); + assertThat("another trigger in the same flow has its own state", trigger.shouldEmit(runContext, otherTriggerInFlowA, true, true), is(true)); + + assertThat(trigger.shouldEmit(runContext, flowA, true, true), is(false)); + assertThat(trigger.shouldEmit(runContext, flowB, true, true), is(false)); + } + + @Test + void commandsTrigger_stateIsScopedToFlowAndTrigger() throws Exception { + String triggerId = "commands-scope-" + IdUtils.create(); + CommandsTrigger trigger = commandsTrigger(triggerId); + var mock = TestsUtils.mockTrigger(runContextFactory, trigger); + RunContext runContext = mock.getKey().getRunContext(); + String namespace = mock.getValue().getNamespace(); + + TriggerContext flowA = contextFor("flow-a", triggerId, namespace); + TriggerContext flowB = contextFor("flow-b", triggerId, namespace); + TriggerContext otherTriggerInFlowA = contextFor("flow-a", triggerId + "-other", namespace); + + assertThat(trigger.shouldEmit(runContext, flowA, true, true), is(true)); + assertThat("same trigger id in another flow has its own state", trigger.shouldEmit(runContext, flowB, true, true), is(true)); + assertThat("another trigger in the same flow has its own state", trigger.shouldEmit(runContext, otherTriggerInFlowA, true, true), is(true)); + + assertThat(trigger.shouldEmit(runContext, flowA, true, true), is(false)); + assertThat(trigger.shouldEmit(runContext, flowB, true, true), is(false)); + } + + @Test + void edgeStateKey_doesNotCollideWhenIdsShareHyphens() { + // "a-b" + "c" and "a" + "b-c" would both read "a-b-c" without the length prefix. + TriggerContext first = contextFor("a-b", "c", "company.team"); + TriggerContext second = contextFor("a", "b-c", "company.team"); + + assertThat(ScriptTrigger.edgeStateKey(first), is(not(ScriptTrigger.edgeStateKey(second)))); + assertThat(CommandsTrigger.edgeStateKey(first), is(not(CommandsTrigger.edgeStateKey(second)))); + } + + @Test + void edgeStateKey_isAValidKvKey() { + String key = ScriptTrigger.edgeStateKey(contextFor("my_flow-1", "my_trigger-2", "company.team")); + + assertThat(key.matches("[a-zA-Z0-9][a-zA-Z0-9._-]*"), is(true)); + } + + @Test + void scriptTrigger_doesNotRewriteTheEntryWhenThePollRepeatsTheSameResult() throws Exception { + ScriptTrigger trigger = scriptTrigger("script-nowrite-" + IdUtils.create()); + var mock = TestsUtils.mockTrigger(runContextFactory, trigger); + RunContext runContext = mock.getKey().getRunContext(); + TriggerContext context = mock.getValue(); + KVStore kvStore = runContext.namespaceKv(context.getNamespace()); + String key = ScriptTrigger.edgeStateKey(context); + + trigger.shouldEmit(runContext, context, true, true); + int versionAfterFirstWrite = kvStore.get(key).orElseThrow().version(); + + trigger.shouldEmit(runContext, context, true, true); + trigger.shouldEmit(runContext, context, true, true); + assertThat("repeating the same result should not rewrite the entry", kvStore.get(key).orElseThrow().version(), is(versionAfterFirstWrite)); + + trigger.shouldEmit(runContext, context, true, false); + assertThat("an actual transition should still write", kvStore.get(key).orElseThrow().version(), is(versionAfterFirstWrite + 1)); + } + + @Test + void commandsTrigger_doesNotRewriteTheEntryWhenThePollRepeatsTheSameResult() throws Exception { + CommandsTrigger trigger = commandsTrigger("commands-nowrite-" + IdUtils.create()); + var mock = TestsUtils.mockTrigger(runContextFactory, trigger); + RunContext runContext = mock.getKey().getRunContext(); + TriggerContext context = mock.getValue(); + KVStore kvStore = runContext.namespaceKv(context.getNamespace()); + String key = CommandsTrigger.edgeStateKey(context); + + trigger.shouldEmit(runContext, context, true, true); + int versionAfterFirstWrite = kvStore.get(key).orElseThrow().version(); + + trigger.shouldEmit(runContext, context, true, true); + trigger.shouldEmit(runContext, context, true, true); + assertThat("repeating the same result should not rewrite the entry", kvStore.get(key).orElseThrow().version(), is(versionAfterFirstWrite)); + + trigger.shouldEmit(runContext, context, true, false); + assertThat("an actual transition should still write", kvStore.get(key).orElseThrow().version(), is(versionAfterFirstWrite + 1)); + } +} diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java new file mode 100644 index 00000000..08b328e7 --- /dev/null +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java @@ -0,0 +1,82 @@ +package io.kestra.plugin.scripts.php; + +import java.util.Map; +import java.util.Optional; + +import org.junit.jupiter.api.Test; + +import io.kestra.core.junit.annotations.KestraTest; +import io.kestra.core.models.executions.Execution; +import io.kestra.core.models.property.Property; +import io.kestra.core.runners.RunContextFactory; +import io.kestra.core.serializers.JacksonMapper; +import io.kestra.core.utils.IdUtils; +import io.kestra.core.utils.TestsUtils; + +import jakarta.inject.Inject; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; + +/** + * Runs the real ScriptTrigger#evaluate end to end, which needs Docker like the other script tests. + */ +@KestraTest +class ScriptTriggerEvaluateTest { + private static final String IMAGE = "php"; + + @Inject + private RunContextFactory runContextFactory; + + private static ScriptTrigger trigger(String prefix, String script) { + return ScriptTrigger.builder() + .id(prefix + "-" + IdUtils.create()) + .type(ScriptTrigger.class.getName()) + .exitCondition(Property.ofValue("exit 1")) + .edge(Property.ofValue(true)) + .containerImage(Property.ofValue(IMAGE)) + .script(Property.ofValue(script)) + .build(); + } + + @Test + void scriptTrigger_shouldEmitWhenExitCodeMatches() throws Exception { + ScriptTrigger trigger = trigger("script-exit1", " execution = trigger.evaluate(context.getKey(), context.getValue()); + + assertThat(execution.isPresent(), is(true)); + + Map triggerVars = execution.get().getTrigger().getVariables(); + assertThat(triggerVars.get("condition"), is("exit 1")); + assertThat(triggerVars.get("exitCode"), is(1)); + assertThat(triggerVars.get("timestamp"), notNullValue()); + } + + @Test + void scriptTrigger_shouldStayQuietWhenConditionDoesNotMatch() throws Exception { + ScriptTrigger trigger = trigger("script-exit0", " execution = trigger.evaluate(context.getKey(), context.getValue()); + + assertThat(execution.isPresent(), is(false)); + } + + @Test + void scriptTrigger_edgeModeShouldSuppressSecondEmissionAcrossFreshInstances() throws Exception { + ScriptTrigger trigger = trigger("script-edge", " first = trigger.evaluate(context.getKey(), context.getValue()); + assertThat("first poll should fire", first.isPresent(), is(true)); + + // The next poll runs on a copy that went through the worker's serialize/deserialize round trip. + ScriptTrigger nextPoll = JacksonMapper.ofJson().readValue(JacksonMapper.ofJson().writeValueAsString(trigger), ScriptTrigger.class); + context = TestsUtils.mockTrigger(runContextFactory, nextPoll); + Optional second = nextPoll.evaluate(context.getKey(), context.getValue()); + assertThat("edge mode should suppress the repeat", second.isPresent(), is(false)); + } +} diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java new file mode 100644 index 00000000..922cc8c8 --- /dev/null +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java @@ -0,0 +1,77 @@ +package io.kestra.plugin.scripts.php; + +import java.time.Instant; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +/** + * Unit tests for ScriptTrigger's condition-matching logic. + * + * Calls the real, package-private ScriptTrigger#matchesCondition directly (same pattern as + * plugin-script-dotnet's ScriptTriggerTest), so a change to the production method is + * what these tests actually exercise, not a separately maintained copy of its logic. + */ +class ScriptTriggerTest { + + private final ScriptTrigger trigger = ScriptTrigger.builder().build(); + + private ScriptTrigger.Output output(String condition, Integer exitCode, Map vars) { + return new ScriptTrigger.Output(Instant.now(), condition, exitCode, vars); + } + + @ParameterizedTest + @CsvSource( + { + "exit 0, 0, true", + "exit 1, 1, true", + "EXIT 1, 1, true", + "exit 0, 1, false", + "exit 1, 127, false", + "exit 42, 42, true", + } + ) + void exitCodeCondition(String condition, int exitCode, boolean expected) { + assertThat(trigger.matchesCondition(output(condition, exitCode, null)), is(expected)); + } + + @Test + void exitCondition_nullExitCode_doesNotMatch() { + assertThat(trigger.matchesCondition(output("exit 1", null, null)), is(false)); + } + + @Test + void substringMatch_inVars() { + assertThat(trigger.matchesCondition(output("toto", 0, Map.of("listing", "toto"))), is(true)); + } + + @Test + void noMatch_whenSubstringAbsentFromVars() { + assertThat(trigger.matchesCondition(output("toto", 0, Map.of("listing", "something_else"))), is(false)); + } + + @Test + void regexMatch_inVars() { + assertThat(trigger.matchesCondition(output("status=\\w+", 0, Map.of("status", "status=ready"))), is(true)); + } + + @Test + void noMatch_emptyHaystack() { + assertThat(trigger.matchesCondition(output("something", 0, null)), is(false)); + } + + @Test + void noMatch_emptyCondition() { + assertThat(trigger.matchesCondition(output("", 0, Map.of("k", "v"))), is(false)); + } + + @Test + void nullCondition_doesNotMatch() { + assertThat(trigger.matchesCondition(output(null, 0, Map.of("k", "v"))), is(false)); + } +} From 8befe84f9ed974b1ba4c1206d9c7ef72987823b9 Mon Sep 17 00:00:00 2001 From: HarshXAI <113297734+IamPossible007@users.noreply.github.com> Date: Fri, 2 Oct 2026 18:57:15 +0530 Subject: [PATCH 2/3] fix(php): address ScriptTrigger review on PR #457 Validate exitCondition before execution, narrow swallowed errors, document runner limits, trim comments, and extend trigger unit tests. --- AGENTS.md | 1 + .../plugin/scripts/php/CommandsTrigger.java | 71 +++++++--------- .../plugin/scripts/php/ScriptTrigger.java | 77 ++++++++---------- .../php/CommandsTriggerConditionTest.java | 50 ++++++++++++ .../scripts/php/CommandsTriggerTest.java | 1 - .../plugin/scripts/php/EdgeStateTest.java | 8 -- .../php/ScriptTriggerEvaluateTest.java | 81 ++++++++++++++++++- .../plugin/scripts/php/ScriptTriggerTest.java | 33 ++++++-- 8 files changed, 213 insertions(+), 109 deletions(-) create mode 100644 plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerConditionTest.java diff --git a/AGENTS.md b/AGENTS.md index b744d61c..1f5d4e7a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -108,6 +108,7 @@ This is a **multi-module** plugin with 19 submodules: - `io.kestra.plugin.scripts.php.CommandsTrigger` - `io.kestra.plugin.scripts.php.Script` - `io.kestra.plugin.scripts.php.ScriptTrigger` + **plugin-script-powershell:** - `io.kestra.plugin.scripts.powershell.Commands` diff --git a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java index 54555b81..d37987ae 100644 --- a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java +++ b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java @@ -8,6 +8,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import io.kestra.core.exceptions.IllegalVariableEvaluationException; import io.kestra.core.models.annotations.Example; import io.kestra.core.models.annotations.Plugin; import io.kestra.core.models.annotations.PluginProperty; @@ -24,9 +25,9 @@ import io.kestra.core.runners.RunContext; import io.kestra.core.storages.kv.KVStore; import io.kestra.core.storages.kv.KVValueAndMetadata; +import io.kestra.plugin.scripts.exec.ExitConditionRegex; import io.kestra.plugin.scripts.exec.TriggerRunContext; import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; - import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.constraints.NotNull; import lombok.AllArgsConstructor; @@ -48,7 +49,8 @@ description = """ Polls by running PHP commands in a script container (default image php) and emits when exitCondition matches. \ Edge mode (the default) emits only on a transition from not matching to matching, remembering the previous result in the namespace KV store. \ - Polls every 60s by default. Accepts 'exit N' or a regex (fallback substring) matched against emitted vars; a failed run has no vars, so only 'exit N' can match a failure. + Polls every 60s by default. Accepts 'exit N' or a regex (fallback substring) matched against emitted vars; a failed run has no vars, so only 'exit N' can match a failure. \ + Only containerImage, commands, interval, edge, and exitCondition are configurable here; the Commands task's taskRunner, env, beforeCommands, inputFiles, and namespaceFiles are not exposed on this trigger. """ ) @Plugin( @@ -143,12 +145,15 @@ If true (default), the trigger emits only on a transition from 'not matching' to public Optional evaluate(ConditionContext conditionContext, TriggerContext context) throws Exception { RunContext runContext = conditionContext.getRunContext(); boolean renderedEdge = runContext.render(this.edge).as(Boolean.class).orElse(true); + String renderedExitCondition = renderExitCondition(runContext); Output out; try { - out = runOnce(runContext); + out = runOnce(runContext, renderedExitCondition); + } catch (IllegalArgumentException | IllegalVariableEvaluationException e) { + throw e; } catch (Exception e) { - runContext.logger().warn("Trigger evaluation failed, returning empty result to avoid blocking the scheduler", e); + runContext.logger().warn("Trigger execution failed, returning empty result to avoid blocking the scheduler", e); return Optional.empty(); } @@ -168,9 +173,6 @@ boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, return matched; } - // A polling trigger is rebuilt from the flow definition (and serialized to a worker) on - // every poll, so the previous result cannot live in a field. It is kept in the namespace - // KV store instead and advanced on every poll. KVStore kvStore = runContext.namespaceKv(context.getNamespace()); String key = edgeStateKey(context); @@ -178,8 +180,6 @@ boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, .map(value -> Boolean.parseBoolean(String.valueOf(value.value()))) .orElse(false); - // Skip the write on a poll that repeats the same result, so a condition that stays true - // for hours does not rewrite the same value every interval. if (matched != previouslyMatched) { kvStore.put(key, new KVValueAndMetadata(null, matched)); } @@ -187,13 +187,15 @@ boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, return matched && !previouslyMatched; } - // Length prefixed so that the pairs ("a-b", "c") and ("a", "b-c") can never share a key. - // Flow and trigger ids only use characters that are valid in a KV key. static String edgeStateKey(TriggerContext context) { return "trigger-edge-" + context.getFlowId().length() + "-" + context.getFlowId() + "-" + context.getTriggerId(); } - private Output runOnce(RunContext runContext) throws Exception { + private String renderExitCondition(RunContext runContext) throws IllegalVariableEvaluationException { + return ScriptTrigger.renderExitCondition(runContext, this.exitCondition); + } + + private Output runOnce(RunContext runContext, String renderedExitCondition) throws Exception { Commands task = Commands.builder() .id(this.getId()) .type(Commands.class.getName()) @@ -201,19 +203,13 @@ private Output runOnce(RunContext runContext) throws Exception { .commands(this.commands) .build(); - String renderedCondition = runContext.render(this.exitCondition).as(String.class) - .filter(condition -> !condition.isBlank()) - .orElseThrow(() -> new IllegalArgumentException("exitCondition must render to a non-empty value")); - try { ScriptOutput taskOutput = task.run(TriggerRunContext.forEmbeddedTask(runContext, task)); - Integer exitCode = safeExitCode(taskOutput); - Map vars = safeVars(taskOutput); - return new Output(Instant.now(), renderedCondition, exitCode, vars); + return new Output(Instant.now(), renderedExitCondition, taskOutput.getExitCode(), taskOutput.getVars()); } catch (RunnableTaskException e) { ExtractedFailure failure = extractFailure(e); - return new Output(Instant.now(), renderedCondition, failure.exitCode, null); + return new Output(Instant.now(), renderedExitCondition, failure.exitCode, null); } } @@ -222,8 +218,12 @@ boolean matchesCondition(Output out) { Matcher exitMatcher = EXIT_CONDITION_PATTERN.matcher(cond); if (exitMatcher.matches()) { - int expected = Integer.parseInt(exitMatcher.group(1)); - return out.getExitCode() != null && out.getExitCode() == expected; + try { + long expected = Long.parseLong(exitMatcher.group(1)); + return out.getExitCode() != null && out.getExitCode() == expected; + } catch (NumberFormatException e) { + return false; + } } String haystack = buildHaystack(out); @@ -231,11 +231,7 @@ boolean matchesCondition(Output out) { return false; } - try { - return Pattern.compile(cond).matcher(haystack).find(); - } catch (Exception invalidRegex) { - return haystack.contains(cond); - } + return ExitConditionRegex.find(cond, haystack); } private String buildHaystack(Output out) { @@ -245,22 +241,6 @@ private String buildHaystack(Output out) { return out.getVars().toString(); } - private Integer safeExitCode(ScriptOutput taskOutput) { - try { - return taskOutput.getExitCode(); - } catch (Exception ignored) { - return null; - } - } - - private Map safeVars(ScriptOutput taskOutput) { - try { - return taskOutput.getVars(); - } catch (Exception ignored) { - return null; - } - } - private record ExtractedFailure(Integer exitCode) { } @@ -299,7 +279,10 @@ public static class Output implements io.kestra.core.models.tasks.Output { @Schema( title = "Commands vars", - description = "Vars produced by the task (e.g. via ::{\"outputs\":{...}}:: convention)." + description = """ + Vars produced by the task (e.g. via ::{"outputs":{...}}:: convention). May contain sensitive values \ + from command output; treat trigger payloads accordingly. + """ ) private Map vars; } diff --git a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java index 0af1c14b..daeb6fd3 100644 --- a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java +++ b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java @@ -7,11 +7,12 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import io.kestra.core.exceptions.IllegalVariableEvaluationException; import io.kestra.core.models.annotations.Example; import io.kestra.core.models.annotations.Plugin; import io.kestra.core.models.annotations.PluginProperty; -import io.kestra.core.models.enums.MonacoLanguages; import io.kestra.core.models.conditions.ConditionContext; +import io.kestra.core.models.enums.MonacoLanguages; import io.kestra.core.models.executions.Execution; import io.kestra.core.models.property.Property; import io.kestra.core.models.tasks.RunnableTaskException; @@ -24,9 +25,9 @@ import io.kestra.core.runners.RunContext; import io.kestra.core.storages.kv.KVStore; import io.kestra.core.storages.kv.KVValueAndMetadata; +import io.kestra.plugin.scripts.exec.ExitConditionRegex; import io.kestra.plugin.scripts.exec.TriggerRunContext; import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; - import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.constraints.NotNull; import lombok.AllArgsConstructor; @@ -48,7 +49,8 @@ description = """ Polls by running an inline PHP script in a container (default image php) and emits when exitCondition matches. \ Edge mode (the default) emits only on a transition from not matching to matching, remembering the previous result in the namespace KV store. \ - Polls every 60s by default. Accepts 'exit N' or a regex (fallback substring) matched against emitted vars; a failed run has no vars, so only 'exit N' can match a failure. + Polls every 60s by default. Accepts 'exit N' or a regex (fallback substring) matched against emitted vars; a failed run has no vars, so only 'exit N' can match a failure. \ + Only containerImage, script, interval, edge, and exitCondition are configurable here; the Script task's taskRunner, env, beforeCommands, inputFiles, and namespaceFiles are not exposed on this trigger. """ ) @Plugin( @@ -145,12 +147,15 @@ When true (default), emit only on a transition from not matching to matching, so public Optional evaluate(ConditionContext conditionContext, TriggerContext context) throws Exception { RunContext runContext = conditionContext.getRunContext(); boolean renderedEdge = runContext.render(this.edge).as(Boolean.class).orElse(true); + String renderedExitCondition = renderExitCondition(runContext); Output out; try { - out = runOnce(runContext); + out = runOnce(runContext, renderedExitCondition); + } catch (IllegalArgumentException | IllegalVariableEvaluationException e) { + throw e; } catch (Exception e) { - runContext.logger().warn("Trigger evaluation failed, returning empty result to avoid blocking the scheduler", e); + runContext.logger().warn("Trigger execution failed, returning empty result to avoid blocking the scheduler", e); return Optional.empty(); } @@ -170,9 +175,6 @@ boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, return matched; } - // A polling trigger is rebuilt from the flow definition (and serialized to a worker) on - // every poll, so the previous result cannot live in a field. It is kept in the namespace - // KV store instead and advanced on every poll. KVStore kvStore = runContext.namespaceKv(context.getNamespace()); String key = edgeStateKey(context); @@ -180,8 +182,6 @@ boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, .map(value -> Boolean.parseBoolean(String.valueOf(value.value()))) .orElse(false); - // Skip the write on a poll that repeats the same result, so a condition that stays true - // for hours does not rewrite the same value every interval. if (matched != previouslyMatched) { kvStore.put(key, new KVValueAndMetadata(null, matched)); } @@ -189,13 +189,21 @@ boolean shouldEmit(RunContext runContext, TriggerContext context, boolean edge, return matched && !previouslyMatched; } - // Length prefixed so that the pairs ("a-b", "c") and ("a", "b-c") can never share a key. - // Flow and trigger ids only use characters that are valid in a KV key. static String edgeStateKey(TriggerContext context) { return "trigger-edge-" + context.getFlowId().length() + "-" + context.getFlowId() + "-" + context.getTriggerId(); } - private Output runOnce(RunContext runContext) throws Exception { + static String renderExitCondition(RunContext runContext, Property exitCondition) throws IllegalVariableEvaluationException { + return runContext.render(exitCondition).as(String.class) + .filter(condition -> !condition.isBlank()) + .orElseThrow(() -> new IllegalArgumentException("exitCondition must render to a non-empty value")); + } + + private String renderExitCondition(RunContext runContext) throws IllegalVariableEvaluationException { + return renderExitCondition(runContext, this.exitCondition); + } + + private Output runOnce(RunContext runContext, String renderedExitCondition) throws Exception { Script task = Script.builder() .id(this.getId()) .type(Script.class.getName()) @@ -203,16 +211,10 @@ private Output runOnce(RunContext runContext) throws Exception { .script(this.script) .build(); - String renderedExitCondition = runContext.render(this.exitCondition).as(String.class) - .filter(condition -> !condition.isBlank()) - .orElseThrow(() -> new IllegalArgumentException("exitCondition must render to a non-empty value")); - try { ScriptOutput taskOutput = task.run(TriggerRunContext.forEmbeddedTask(runContext, task)); - Integer exitCode = safeExitCode(taskOutput); - Map vars = safeVars(taskOutput); - return new Output(Instant.now(), renderedExitCondition, exitCode, vars); + return new Output(Instant.now(), renderedExitCondition, taskOutput.getExitCode(), taskOutput.getVars()); } catch (RunnableTaskException e) { ExtractedFailure failure = extractFailure(e); return new Output(Instant.now(), renderedExitCondition, failure.exitCode, null); @@ -224,8 +226,12 @@ boolean matchesCondition(Output out) { Matcher exitMatcher = EXIT_CONDITION_PATTERN.matcher(cond); if (exitMatcher.matches()) { - int expected = Integer.parseInt(exitMatcher.group(1)); - return out.getExitCode() != null && out.getExitCode() == expected; + try { + long expected = Long.parseLong(exitMatcher.group(1)); + return out.getExitCode() != null && out.getExitCode() == expected; + } catch (NumberFormatException e) { + return false; + } } String haystack = buildHaystack(out); @@ -233,11 +239,7 @@ boolean matchesCondition(Output out) { return false; } - try { - return Pattern.compile(cond).matcher(haystack).find(); - } catch (Exception invalidRegex) { - return haystack.contains(cond); - } + return ExitConditionRegex.find(cond, haystack); } private String buildHaystack(Output out) { @@ -247,22 +249,6 @@ private String buildHaystack(Output out) { return out.getVars().toString(); } - private Integer safeExitCode(ScriptOutput taskOutput) { - try { - return taskOutput.getExitCode(); - } catch (Exception ignored) { - return null; - } - } - - private Map safeVars(ScriptOutput taskOutput) { - try { - return taskOutput.getVars(); - } catch (Exception ignored) { - return null; - } - } - private record ExtractedFailure(Integer exitCode) { } @@ -302,8 +288,9 @@ public static class Output implements io.kestra.core.models.tasks.Output { @Schema( title = "Script vars", description = """ - Vars produced by the task (e.g. via ::{"outputs":{...}}:: convention). This is the main structured - way to evaluate non-exit conditions on successful runs. + Vars produced by the task (e.g. via ::{"outputs":{...}}:: convention). May contain sensitive values \ + from script output; treat trigger payloads accordingly. This is the main structured way to evaluate \ + non-exit conditions on successful runs. """ ) private Map vars; diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerConditionTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerConditionTest.java new file mode 100644 index 00000000..c88eb804 --- /dev/null +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerConditionTest.java @@ -0,0 +1,50 @@ +package io.kestra.plugin.scripts.php; + +import io.kestra.plugin.scripts.exec.ExitConditionRegex; + +import java.time.Instant; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; + +class CommandsTriggerConditionTest { + + private final CommandsTrigger trigger = CommandsTrigger.builder().build(); + + private CommandsTrigger.Output output(String condition, Integer exitCode, Map vars) { + return new CommandsTrigger.Output(Instant.now(), condition, exitCode, vars); + } + + @ParameterizedTest + @CsvSource({ + "exit 0, 0, true", + "exit 1, 1, true", + "exit 1, 0, false", + }) + void exitCodeCondition(String condition, int exitCode, boolean expected) { + assertThat(trigger.matchesCondition(output(condition, exitCode, null)), is(expected)); + } + + @Test + void regexCondition_delegatesToHelperAndReturnsItsResult() { + String condition = "status=\\w+"; + String haystack = "{status=status=ready}"; + var out = output(condition, 0, Map.of("status", "status=ready")); + + try (var helper = mockStatic(ExitConditionRegex.class)) { + helper.when(() -> ExitConditionRegex.find(condition, haystack)).thenReturn(true); + + assertThat(trigger.matchesCondition(out), is(true)); + + helper.verify(() -> ExitConditionRegex.find(condition, haystack), times(1)); + helper.verifyNoMoreInteractions(); + } + } +} diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java index 149fdfa2..21258476 100644 --- a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java @@ -87,7 +87,6 @@ void commandsTrigger_edgeModeShouldSuppressSecondEmission() throws Exception { Optional first = trigger.evaluate(context.getKey(), context.getValue()); assertThat("First evaluation should fire", first.isPresent(), is(true)); - // The second poll runs on a copy that went through the worker's serialize/deserialize round trip. CommandsTrigger nextPoll = JacksonMapper.ofJson().readValue(JacksonMapper.ofJson().writeValueAsString(trigger), CommandsTrigger.class); context = TestsUtils.mockTrigger(runContextFactory, nextPoll); Optional second = nextPoll.evaluate(context.getKey(), context.getValue()); diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java index 329f36c7..4aece291 100644 --- a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/EdgeStateTest.java @@ -23,13 +23,6 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -/** - * Edge mode keeps its previous result in the namespace KV store. None of these tests need Docker: - * they drive shouldEmit directly, so only the state handling is under test. - * - * The KV store used by the test config is a local folder that survives between runs, so every - * test uses a unique trigger id. - */ @KestraTest class EdgeStateTest { @Inject @@ -53,7 +46,6 @@ private static CommandsTrigger commandsTrigger(String id) { .build(); } - // The scheduler hands every poll a freshly deserialized trigger, so each poll here does the same. private static T freshCopy(T trigger, Class type) throws Exception { return JacksonMapper.ofJson().readValue(JacksonMapper.ofJson().writeValueAsString(trigger), type); } diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java index 08b328e7..8376eea6 100644 --- a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java @@ -2,14 +2,17 @@ import java.util.Map; import java.util.Optional; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import io.kestra.core.junit.annotations.KestraTest; import io.kestra.core.models.executions.Execution; import io.kestra.core.models.property.Property; import io.kestra.core.runners.RunContextFactory; import io.kestra.core.serializers.JacksonMapper; +import io.kestra.core.storages.kv.KVStore; import io.kestra.core.utils.IdUtils; import io.kestra.core.utils.TestsUtils; @@ -18,10 +21,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assertions.assertThrows; -/** - * Runs the real ScriptTrigger#evaluate end to end, which needs Docker like the other script tests. - */ @KestraTest class ScriptTriggerEvaluateTest { private static final String IMAGE = "php"; @@ -40,8 +41,64 @@ private static ScriptTrigger trigger(String prefix, String script) { .build(); } + @Test + void evaluate_blankExitCondition_propagatesIllegalArgumentException() throws Exception { + ScriptTrigger trigger = ScriptTrigger.builder() + .id("blank-exit-" + IdUtils.create()) + .type(ScriptTrigger.class.getName()) + .exitCondition(Property.ofValue(" ")) + .script(Property.ofValue(" trigger.evaluate(context.getKey(), context.getValue()); + + assertThrows(IllegalArgumentException.class, evaluate); + } + + @Test + void evaluate_executionFailure_doesNotAdvanceEdgeState() throws Exception { + ScriptTrigger trigger = trigger("script-exec-fail", " execution = trigger.evaluate(mock.getKey(), mock.getValue()); + + if (execution.isEmpty()) { + assertThat(kvStore.getValue(key).isPresent(), is(false)); + } + } + + @Test + void evaluate_edgeDisabledEmitsOnEveryMatchWhenDockerWorks() throws Exception { + if (!dockerAvailable()) { + return; + } + + ScriptTrigger trigger = ScriptTrigger.builder() + .id("edge-off-" + IdUtils.create()) + .type(ScriptTrigger.class.getName()) + .exitCondition(Property.ofValue("exit 0")) + .edge(Property.ofValue(false)) + .containerImage(Property.ofValue(IMAGE)) + .script(Property.ofValue(" first = trigger.evaluate(context.getKey(), context.getValue()); + Optional second = trigger.evaluate(context.getKey(), context.getValue()); + + assertThat(first.isPresent(), is(true)); + assertThat(second.isPresent(), is(true)); + } + @Test void scriptTrigger_shouldEmitWhenExitCodeMatches() throws Exception { + if (!dockerAvailable()) { + return; + } + ScriptTrigger trigger = trigger("script-exit1", " first = trigger.evaluate(context.getKey(), context.getValue()); assertThat("first poll should fire", first.isPresent(), is(true)); - // The next poll runs on a copy that went through the worker's serialize/deserialize round trip. ScriptTrigger nextPoll = JacksonMapper.ofJson().readValue(JacksonMapper.ofJson().writeValueAsString(trigger), ScriptTrigger.class); context = TestsUtils.mockTrigger(runContextFactory, nextPoll); Optional second = nextPoll.evaluate(context.getKey(), context.getValue()); assertThat("edge mode should suppress the repeat", second.isPresent(), is(false)); } + + private static boolean dockerAvailable() { + try { + var process = new ProcessBuilder("docker", "info").start(); + return process.waitFor(10, TimeUnit.SECONDS) && process.exitValue() == 0; + } catch (Exception e) { + return false; + } + } } diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java index 922cc8c8..e95923e6 100644 --- a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerTest.java @@ -1,5 +1,7 @@ package io.kestra.plugin.scripts.php; +import io.kestra.plugin.scripts.exec.ExitConditionRegex; + import java.time.Instant; import java.util.Map; @@ -9,14 +11,9 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; -/** - * Unit tests for ScriptTrigger's condition-matching logic. - * - * Calls the real, package-private ScriptTrigger#matchesCondition directly (same pattern as - * plugin-script-dotnet's ScriptTriggerTest), so a change to the production method is - * what these tests actually exercise, not a separately maintained copy of its logic. - */ class ScriptTriggerTest { private final ScriptTrigger trigger = ScriptTrigger.builder().build(); @@ -45,6 +42,11 @@ void exitCondition_nullExitCode_doesNotMatch() { assertThat(trigger.matchesCondition(output("exit 1", null, null)), is(false)); } + @Test + void exitCodeCondition_overflowingInteger_doesNotMatch() { + assertThat(trigger.matchesCondition(output("exit 9999999999999", 0, null)), is(false)); + } + @Test void substringMatch_inVars() { assertThat(trigger.matchesCondition(output("toto", 0, Map.of("listing", "toto"))), is(true)); @@ -74,4 +76,21 @@ void noMatch_emptyCondition() { void nullCondition_doesNotMatch() { assertThat(trigger.matchesCondition(output(null, 0, Map.of("k", "v"))), is(false)); } + + @Test + void regexCondition_delegatesToHelperAndReturnsItsResult() { + String condition = "status=\\w+"; + String haystack = "{status=status=ready}"; + var out = output(" " + condition + " ", 0, Map.of("status", "status=ready")); + + try (var helper = mockStatic(ExitConditionRegex.class)) { + helper.when(() -> ExitConditionRegex.find(condition, haystack)).thenReturn(true, false); + + assertThat(trigger.matchesCondition(out), is(true)); + assertThat(trigger.matchesCondition(out), is(false)); + + helper.verify(() -> ExitConditionRegex.find(condition, haystack), times(2)); + helper.verifyNoMoreInteractions(); + } + } } From 464737fd3d941115281c2a7ea34bec06bb650c67 Mon Sep 17 00:00:00 2001 From: HarshXAI <113297734+IamPossible007@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:15:26 +0530 Subject: [PATCH 3/3] fix(php): address second review on PR #457 Rethrow infrastructure errors from triggers, harden evaluate tests, document triggers in package doc, and use visible Docker skips. --- .../plugin/scripts/php/CommandsTrigger.java | 11 +--- .../plugin/scripts/php/ScriptTrigger.java | 11 +--- .../doc/io.kestra.plugin.scripts.php.md | 23 +++++++- .../scripts/php/CommandsTriggerTest.java | 5 ++ .../plugin/scripts/php/DockerTestSupport.java | 18 ++++++ .../php/ScriptTriggerEvaluateTest.java | 58 ++++++++++--------- 6 files changed, 79 insertions(+), 47 deletions(-) create mode 100644 plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/DockerTestSupport.java diff --git a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java index d37987ae..70a0f385 100644 --- a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java +++ b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/CommandsTrigger.java @@ -146,16 +146,7 @@ public Optional evaluate(ConditionContext conditionContext, TriggerCo RunContext runContext = conditionContext.getRunContext(); boolean renderedEdge = runContext.render(this.edge).as(Boolean.class).orElse(true); String renderedExitCondition = renderExitCondition(runContext); - - Output out; - try { - out = runOnce(runContext, renderedExitCondition); - } catch (IllegalArgumentException | IllegalVariableEvaluationException e) { - throw e; - } catch (Exception e) { - runContext.logger().warn("Trigger execution failed, returning empty result to avoid blocking the scheduler", e); - return Optional.empty(); - } + Output out = runOnce(runContext, renderedExitCondition); boolean matched = matchesCondition(out); diff --git a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java index daeb6fd3..20fc1a48 100644 --- a/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java +++ b/plugin-script-php/src/main/java/io/kestra/plugin/scripts/php/ScriptTrigger.java @@ -148,16 +148,7 @@ public Optional evaluate(ConditionContext conditionContext, TriggerCo RunContext runContext = conditionContext.getRunContext(); boolean renderedEdge = runContext.render(this.edge).as(Boolean.class).orElse(true); String renderedExitCondition = renderExitCondition(runContext); - - Output out; - try { - out = runOnce(runContext, renderedExitCondition); - } catch (IllegalArgumentException | IllegalVariableEvaluationException e) { - throw e; - } catch (Exception e) { - runContext.logger().warn("Trigger execution failed, returning empty result to avoid blocking the scheduler", e); - return Optional.empty(); - } + Output out = runOnce(runContext, renderedExitCondition); boolean matched = matchesCondition(out); diff --git a/plugin-script-php/src/main/resources/doc/io.kestra.plugin.scripts.php.md b/plugin-script-php/src/main/resources/doc/io.kestra.plugin.scripts.php.md index d3739090..d61cdfe5 100644 --- a/plugin-script-php/src/main/resources/doc/io.kestra.plugin.scripts.php.md +++ b/plugin-script-php/src/main/resources/doc/io.kestra.plugin.scripts.php.md @@ -4,10 +4,31 @@ Run PHP scripts inside a container as flow steps. ## Common properties -`containerImage` defaults to a PHP image. `taskRunner` defaults to Docker and can be overridden for other execution environments. +`containerImage` defaults to the `php` image on `Script` and `Commands` tasks. `taskRunner` defaults to Docker on those tasks and can be overridden for other execution environments. The polling triggers below do not expose `taskRunner`, `env`, `beforeCommands`, `inputFiles`, or `namespaceFiles`; only `containerImage`, the script or commands, `interval`, `edge`, and `exitCondition` are configurable on the trigger. ## Tasks `Script` runs inline PHP code defined in the `script` property — best for short, flow-specific logic. `Commands` runs shell commands (e.g., `php main.php`) against script files; use it when your code lives in [namespace files](https://kestra.io/docs/concepts/namespace-files) shared across flows, or is cloned from a Git repository with a preceding `Clone` task. Manage dependencies with Composer — include a `composer.json` via namespace files and run `composer install` in `beforeCommands`. For projects without Composer dependencies, a plain PHP image is sufficient. + +## Triggers + +### ScriptTrigger + +Polls on an interval by running an inline PHP script the same way the `Script` task does, and starts an execution when `exitCondition` matches. The script runs in a fresh container on every poll, so keep it quick. + +Required properties: +- `script`: inline PHP script body +- `exitCondition`: either `exit N`, which matches when the script exits with code N, or a regex (with substring fallback) matched against the vars the script emits with `::{"outputs":{...}}::` + +Optional: +- `interval`: time between polls, defaults to `PT60S` +- `edge`: defaults to `true`, so the trigger fires only when the condition changes from not matching to matching. The previous result is kept in the namespace KV store under a key starting with `trigger-edge-`. Set to `false` to fire on every matching poll +- `containerImage`: defaults to `php` + +The trigger outputs are available as `{{ trigger.timestamp }}`, `{{ trigger.condition }}`, `{{ trigger.exitCode }}` and `{{ trigger.vars }}`. A failed run has no vars, so only an `exit N` condition can match a failure. + +### CommandsTrigger + +Same behavior as `ScriptTrigger`, but runs a list of shell commands the way the `Commands` task does. Required properties are `commands` and `exitCondition`; `interval`, `edge` and `containerImage` work as above. diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java index 21258476..8e855ad4 100644 --- a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/CommandsTriggerTest.java @@ -19,6 +19,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; @KestraTest class CommandsTriggerTest { @@ -27,6 +28,7 @@ class CommandsTriggerTest { @Test void commandsTrigger_shouldTriggerOnImplicitFailureExit1() throws Exception { + assumeTrue(DockerTestSupport.dockerAvailable()); CommandsTrigger trigger = CommandsTrigger.builder() .id("commands-trigger-" + IdUtils.create()) .type(CommandsTrigger.class.getName()) @@ -50,6 +52,7 @@ void commandsTrigger_shouldTriggerOnImplicitFailureExit1() throws Exception { @Test void commandsTrigger_shouldTriggerOnStdoutMatchUsingStructuredOutputs() throws Exception { + assumeTrue(DockerTestSupport.dockerAvailable()); CommandsTrigger trigger = CommandsTrigger.builder() .id("commands-stdout-match-trigger-" + IdUtils.create()) .type(CommandsTrigger.class.getName()) @@ -74,6 +77,7 @@ void commandsTrigger_shouldTriggerOnStdoutMatchUsingStructuredOutputs() throws E @Test void commandsTrigger_edgeModeShouldSuppressSecondEmission() throws Exception { + assumeTrue(DockerTestSupport.dockerAvailable()); CommandsTrigger trigger = CommandsTrigger.builder() .id("commands-edge-trigger-" + IdUtils.create()) .type(CommandsTrigger.class.getName()) @@ -95,6 +99,7 @@ void commandsTrigger_edgeModeShouldSuppressSecondEmission() throws Exception { @Test void commandsTrigger_shouldMatchRegexAgainstStructuredOutputs() throws Exception { + assumeTrue(DockerTestSupport.dockerAvailable()); CommandsTrigger trigger = CommandsTrigger.builder() .id("commands-regex-trigger-" + IdUtils.create()) .type(CommandsTrigger.class.getName()) diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/DockerTestSupport.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/DockerTestSupport.java new file mode 100644 index 00000000..b8ece74c --- /dev/null +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/DockerTestSupport.java @@ -0,0 +1,18 @@ +package io.kestra.plugin.scripts.php; + +import java.util.concurrent.TimeUnit; + +final class DockerTestSupport { + + private DockerTestSupport() { + } + + static boolean dockerAvailable() { + try { + var process = new ProcessBuilder("docker", "info").start(); + return process.waitFor(10, TimeUnit.SECONDS) && process.exitValue() == 0; + } catch (Exception e) { + return false; + } + } +} diff --git a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java index 8376eea6..986e7348 100644 --- a/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java +++ b/plugin-script-php/src/test/java/io/kestra/plugin/scripts/php/ScriptTriggerEvaluateTest.java @@ -2,8 +2,6 @@ import java.util.Map; import java.util.Optional; -import java.util.concurrent.TimeUnit; - import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; @@ -22,6 +20,8 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assumptions.assumeFalse; +import static org.junit.jupiter.api.Assumptions.assumeTrue; @KestraTest class ScriptTriggerEvaluateTest { @@ -58,23 +58,43 @@ void evaluate_blankExitCondition_propagatesIllegalArgumentException() throws Exc @Test void evaluate_executionFailure_doesNotAdvanceEdgeState() throws Exception { - ScriptTrigger trigger = trigger("script-exec-fail", " execution = trigger.evaluate(mock.getKey(), mock.getValue()); - if (execution.isEmpty()) { - assertThat(kvStore.getValue(key).isPresent(), is(false)); - } + assertThat(execution.isPresent(), is(false)); + assertThat(kvStore.getValue(key).isPresent(), is(false)); + } + + @Test + void evaluate_infrastructureFailure_propagatesWithoutAdvancingEdgeState() throws Exception { + assumeFalse(DockerTestSupport.dockerAvailable()); + + ScriptTrigger trigger = trigger("script-infra-fail", " trigger.evaluate(mock.getKey(), mock.getValue())); + assertThat(kvStore.getValue(key).isPresent(), is(false)); } @Test void evaluate_edgeDisabledEmitsOnEveryMatchWhenDockerWorks() throws Exception { - if (!dockerAvailable()) { - return; - } + assumeTrue(DockerTestSupport.dockerAvailable()); ScriptTrigger trigger = ScriptTrigger.builder() .id("edge-off-" + IdUtils.create()) @@ -95,9 +115,7 @@ void evaluate_edgeDisabledEmitsOnEveryMatchWhenDockerWorks() throws Exception { @Test void scriptTrigger_shouldEmitWhenExitCodeMatches() throws Exception { - if (!dockerAvailable()) { - return; - } + assumeTrue(DockerTestSupport.dockerAvailable()); ScriptTrigger trigger = trigger("script-exit1", "