Repository: kestra-io/kestra (java)

Issue:
Discrepencies between manual and trigger execution in variabe handling
### Explain the bug

Depending if the execution is run manually or via a trigger, the `vars.period` variable doesn't have the same behavior; especially on the `replace` Pebble method:

```yaml
id: copine_voyage_troubleshoot
namespace: dev

variables:
  period: "{{ schedule.date ?? execution.startDate | dateAdd(-1, 'DAYS') | date('yyyy-MM') }}"

tasks:

  - id: debug
    type: io.kestra.core.tasks.log.Log
    message: "Period: {{ vars.period }}, {{ vars.period | replace({'-': ''}) }}, {{ vars.period | replace({'2023': 'toto'}) }}"

triggers:
  - id: schedule
    type: io.kestra.core.models.triggers.types.Schedule
    cron: "0 12 1 * *"
    backfill:
      start: 2023-08-01T00:00:00Z
```

When running manually (working fine, except for the "toto" replacement):
![image](https://github.com/kestra-io/kestra/assets/46634684/4102a0a7-675f-49bb-b560-e752e3ed4558)

When running from trigger (here backfill schedule):

![image](https://github.com/kestra-io/kestra/assets/46634684/c68c1e3c-880b-4eb3-8c36-145fd6e70498)


### Environment Information

- Kestra Version: 0.13.0
- Operating System and Java Version (if not using Kestra Docker image):


Pull request:
fix(): prerender variables
close #2585 


Patch:
diff --git a/core/src/main/java/io/kestra/core/runners/RunContext.java b/core/src/main/java/io/kestra/core/runners/RunContext.java
index 2d93dc724..abffe19a2 100644
--- a/core/src/main/java/io/kestra/core/runners/RunContext.java
+++ b/core/src/main/java/io/kestra/core/runners/RunContext.java
@@ -598,7 +598,7 @@ public class RunContext {
         URI uri = URI.create(this.taskStateFilePathPrefix(state, isNamespace, useTaskRun));
         URI resolve = uri.resolve(uri.getPath() + "/" + name);
 
-       return this.storageInterface.get(getTenantId(), resolve);
+        return this.storageInterface.get(getTenantId(), resolve);
     }
 
     public URI putTaskStateFile(byte[] content, String state, String name) throws IOException {
diff --git a/core/src/main/java/io/kestra/core/runners/VariableRenderer.java b/core/src/main/java/io/kestra/core/runners/VariableRenderer.java
index 95fcd42dc..62e8da2c8 100644
--- a/core/src/main/java/io/kestra/core/runners/VariableRenderer.java
+++ b/core/src/main/java/io/kestra/core/runners/VariableRenderer.java
@@ -5,11 +5,6 @@ import com.github.jknack.handlebars.Handlebars;
 import com.github.jknack.handlebars.HandlebarsException;
 import com.github.jknack.handlebars.Template;
 import com.github.jknack.handlebars.helper.*;
-import io.pebbletemplates.pebble.PebbleEngine;
-import io.pebbletemplates.pebble.error.AttributeNotFoundException;
-import io.pebbletemplates.pebble.error.PebbleException;
-import io.pebbletemplates.pebble.extension.AbstractExtension;
-import io.pebbletemplates.pebble.template.PebbleTemplate;
 import io.kestra.core.exceptions.IllegalVariableEvaluationException;
 import io.kestra.core.runners.handlebars.VariableRendererPlugins;
 import io.kestra.core.runners.handlebars.helpers.*;
@@ -18,6 +13,15 @@ import io.kestra.core.runners.pebble.JsonWriter;
 import io.kestra.core.runners.pebble.PebbleLruCache;
 import io.micronaut.context.ApplicationContext;
 import io.micronaut.context.annotation.ConfigurationProperties;
+import io.micronaut.core.annotation.Nullable;
+import io.pebbletemplates.pebble.PebbleEngine;
+import io.pebbletemplates.pebble.error.AttributeNotFoundException;
+import io.pebbletemplates.pebble.error.PebbleException;
+import io.pebbletemplates.pebble.extension.AbstractExtension;
+import io.pebbletemplates.pebble.template.PebbleTemplate;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+import lombok.Getter;
 
 import java.io.IOException;
 import java.io.StringWriter;
@@ -26,11 +30,6 @@ import java.util.*;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import io.micronaut.core.annotation.Nullable;
-import jakarta.inject.Inject;
-import jakarta.inject.Singleton;
-import lombok.Getter;
-
 @Singleton
 public class VariableRenderer {
     private static final Pattern RAW_PATTERN = Pattern.compile("\\{%[-]*\\s*raw\\s*[-]*%\\}(.*?)\\{%[-]*\\s*endraw\\s*[-]*%\\}");
@@ -93,6 +92,10 @@ public class VariableRenderer {
     }
 
     public String recursiveRender(String inline, Map<String, Object> variables) throws IllegalVariableEvaluationException {
+        return recursiveRender(inline, variables, true);
+    }
+
+    public String recursiveRender(String inline, Map<String, Object> variables, Boolean preprocessVariables) throws IllegalVariableEvaluationException {
         if (inline == null) {
             return null;
         }
@@ -111,6 +114,11 @@ public class VariableRenderer {
             return uuid;
         });
 
+        // pre-process variables
+        if (preprocessVariables) {
+            variables = this.preprocessVariables(variables);
+        }
+
         boolean isSame = false;
         String current = "";
         PebbleTemplate compiledTemplate;
@@ -131,28 +139,43 @@ public class VariableRenderer {
                 }
 
                 try {
-                    Template  template = handlebars.compileInline(currentTemplate);
+                    Template template = handlebars.compileInline(currentTemplate);
                     current = template.apply(variables);
                 } catch (HandlebarsException | IOException hbE) {
                     throw new IllegalVariableEvaluationException(
-                        "Pebble evaluation failed with '" + e.getMessage() +  "' " +
-                        "and Handlebars fallback failed also  with '" + hbE.getMessage() + "'" ,
+                        "Pebble evaluation failed with '" + e.getMessage() + "' " +
+                            "and Handlebars fallback failed also  with '" + hbE.getMessage() + "'",
                         e
                     );
                 }
             }
 
+
             isSame = currentTemplate.equals(current);
             currentTemplate = current;
         }
 
         // post-process raw tags
-        for(var entry: replacers.entrySet()) {
+        for (var entry : replacers.entrySet()) {
             current = current.replace(entry.getKey(), entry.getValue());
         }
         return current;
     }
 
+    public Map<String, Object> preprocessVariables(Map<String, Object> variables) throws IllegalVariableEvaluationException {
+        Map<String, Object> currentVariables = variables;
+        Map<String, Object> previousVariables;
+        boolean isSame = false;
+
+        while (!isSame) {
+            previousVariables = currentVariables;
+            currentVariables = this.render(currentVariables, variables, false);
+            isSame = previousVariables.equals(currentVariables);
+        }
+
+        return currentVariables;
+    }
+
     public IllegalVariableEvaluationException properPebbleException(PebbleException e) {
         if (e instanceof AttributeNotFoundException current) {
             return new IllegalVariableEvaluationException(
@@ -166,17 +189,27 @@ public class VariableRenderer {
         return new IllegalVariableEvaluationException(e);
     }
 
+    // By default, render() will render variables
+    // so we keep the previous behavior
     public String render(String inline, Map<String, Object> variables) throws IllegalVariableEvaluationException {
-        return this.recursiveRender(inline, variables);
+        return this.recursiveRender(inline, variables, true);
     }
 
+    public String render(String inline, Map<String, Object> variables, boolean preprocessVariables) throws IllegalVariableEvaluationException {
+        return this.recursiveRender(inline, variables, preprocessVariables);
+    }
 
+    // Default behavior
     public Map<String, Object> render(Map<String, Object> in, Map<String, Object> variables) throws IllegalVariableEvaluationException {
+        return render(in, variables, true);
+    }
+
+    public Map<String, Object> render(Map<String, Object> in, Map<String, Object> variables, boolean preprocessVariables) throws IllegalVariableEvaluationException {
         Map<String, Object> map = new HashMap<>();
 
         for (Map.Entry<String, Object> r : in.entrySet()) {
-            String key = this.render(r.getKey(), variables);
-            Object value = renderObject(r.getValue(), variables).orElse(r.getValue());
+            String key = this.render(r.getKey(), variables, preprocessVariables);
+            Object value = renderObject(r.getValue(), variables, preprocessVariables).orElse(r.getValue());
 
             map.putIfAbsent(
                 key,
@@ -188,23 +221,25 @@ public class VariableRenderer {
     }
 
     @SuppressWarnings({"unchecked", "rawtypes"})
-    private Optional<Object> renderObject(Object object, Map<String, Object> variables) throws IllegalVariableEvaluationException {
+    private Optional<Object> renderObject(Object object, Map<String, Object> variables, boolean preprocessVariables) throws IllegalVariableEvaluationException {
         if (object instanceof Map) {
-            return Optional.of(this.render((Map) object, variables));
+            return Optional.of(this.render((Map) object, variables, preprocessVariables));
         } else if (object instanceof Collection) {
-            return Optional.of(this.renderList((List) object, variables));
+            return Optional.of(this.renderList((List) object, variables, preprocessVariables));
         } else if (object instanceof String) {
-            return Optional.of(this.render((String) object, variables));
+            return Optional.of(this.render((String) object, variables, preprocessVariables));
+        } else if (object == null) {
+            return Optional.empty();
         }
 
-        return Optional.empty();
+        return Optional.of(object);
     }
 
-    public List<Object> renderList(List<Object> list, Map<String, Object> variables) throws IllegalVariableEvaluationException {
+    private List<Object> renderList(List<Object> list, Map<String, Object> variables, boolean preprocessVariables) throws IllegalVariableEvaluationException {
         List<Object> result = new ArrayList<>();
 
         for (Object inline : list) {
-            this.renderObject(inline, variables)
+            this.renderObject(inline, variables, preprocessVariables)
                 .ifPresent(result::add);
         }
 


Test patch:
diff --git a/core/src/test/java/io/kestra/core/models/hierarchies/FlowGraphTest.java b/core/src/test/java/io/kestra/core/models/hierarchies/FlowGraphTest.java
index e3c63d8f3..9a02057c4 100644
--- a/core/src/test/java/io/kestra/core/models/hierarchies/FlowGraphTest.java
+++ b/core/src/test/java/io/kestra/core/models/hierarchies/FlowGraphTest.java
@@ -35,8 +35,8 @@ class FlowGraphTest extends AbstractMemoryRunnerTest {
         Flow flow = this.parse("flows/valids/return.yaml");
         FlowGraph flowGraph = GraphUtils.flowGraph(flow, null);
 
-        assertThat(flowGraph.getNodes().size(), is(5));
-        assertThat(flowGraph.getEdges().size(), is(4));
+        assertThat(flowGraph.getNodes().size(), is(6));
+        assertThat(flowGraph.getEdges().size(), is(5));
         assertThat(flowGraph.getClusters().size(), is(0));
 
         assertThat(((AbstractGraphTask) flowGraph.getNodes().get(2)).getTask().getId(), is("date"));
diff --git a/core/src/test/java/io/kestra/core/runners/RunContextTest.java b/core/src/test/java/io/kestra/core/runners/RunContextTest.java
index 076ed9b4a..b28f0635c 100644
--- a/core/src/test/java/io/kestra/core/runners/RunContextTest.java
+++ b/core/src/test/java/io/kestra/core/runners/RunContextTest.java
@@ -30,6 +30,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.*;
 
 @Property(name = "kestra.tasks.tmp-dir.path", value = "/tmp/sub/dir/tmp/")
@@ -112,7 +113,7 @@ class RunContextTest extends AbstractMemoryRunnerTest {
     void variables() throws TimeoutException {
         Execution execution = runnerUtils.runOne(null, "io.kestra.tests", "return");
 
-        assertThat(execution.getTaskRunList(), hasSize(3));
+        assertThat(execution.getTaskRunList(), hasSize(4));
 
         assertThat(
             ZonedDateTime.from(ZonedDateTime.parse((String) execution.getTaskRunList().get(0).getOutputs().get("value"))),
@@ -120,6 +121,7 @@ class RunContextTest extends AbstractMemoryRunnerTest {
         );
         assertThat(execution.getTaskRunList().get(1).getOutputs().get("value"), is("task-id"));
         assertThat(execution.getTaskRunList().get(2).getOutputs().get("value"), is("return"));
+        assertThat((String) execution.getTaskRunList().get(3).getOutputs().get("value"), containsString("toto"));
     }
 
     @Test
diff --git a/core/src/test/java/io/kestra/core/runners/pebble/PebbleVariableRendererTest.java b/core/src/test/java/io/kestra/core/runners/pebble/PebbleVariableRendererTest.java
index d74c3cfde..9d26477f0 100644
--- a/core/src/test/java/io/kestra/core/runners/pebble/PebbleVariableRendererTest.java
+++ b/core/src/test/java/io/kestra/core/runners/pebble/PebbleVariableRendererTest.java
@@ -5,13 +5,13 @@ import io.kestra.core.exceptions.IllegalVariableEvaluationException;
 import io.kestra.core.runners.VariableRenderer;
 import io.kestra.core.utils.Rethrow;
 import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
 import org.junit.jupiter.api.Test;
 
 import java.time.ZonedDateTime;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
-import jakarta.inject.Inject;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
diff --git a/core/src/test/resources/flows/templates/with-template.yaml b/core/src/test/resources/flows/templates/with-template.yaml
index edae4e995..281177dbd 100644
--- a/core/src/test/resources/flows/templates/with-template.yaml
+++ b/core/src/test/resources/flows/templates/with-template.yaml
@@ -10,7 +10,7 @@ inputs:
 
 variables:
   var-1: var-1
-  var-2: '{{ var-1 }}'
+  var-2: "{{ 'var-1' }}"
 
 taskDefaults:
   - type: io.kestra.core.tasks.log.Log
diff --git a/core/src/test/resources/flows/valids/return.yaml b/core/src/test/resources/flows/valids/return.yaml
index e8af120b1..32ccf4d7e 100644
--- a/core/src/test/resources/flows/valids/return.yaml
+++ b/core/src/test/resources/flows/valids/return.yaml
@@ -1,6 +1,9 @@
 id: return
 namespace: io.kestra.tests
 
+variables:
+  period: "{{ schedule.date ?? execution.startDate }}"
+
 tasks:
 - id: date
   type: io.kestra.core.tasks.debugs.Return
@@ -10,4 +13,7 @@ tasks:
   format: "{{task.id}}"
 - id: flow-id
   type: io.kestra.core.tasks.debugs.Return
-  format: "{{flow.id}}"
\ No newline at end of file
+  format: "{{flow.id}}"
+- id: variables
+  type: io.kestra.core.tasks.debugs.Return
+  format: "{{ vars.period | replace({':': 'toto'}) }}"
\ No newline at end of file