Repository: kestra-io/kestra
Issue: Discrepencies between manual and trigger execution in variabe handling
Labels: bug

### 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):


Issue comments:
Seems like the `replace` method doesn't handle string/datetime very well

![image](https://github.com/kestra-io/kestra/assets/46634684/61f28a37-3156-42ec-9a21-aedecbc13adc)

Reopened because the fix has been reverted in https://github.com/kestra-io/kestra/pull/2754
closing as with the render function (non-recursive behavior), both manual and scheduled executions generate identical results:

```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: {{ render(vars.period) }}
      {{ render(vars.period) | replace({'-': ''}) }}
      {{ render(vars.period | replace({'-': ''})) }}
      {{ render(vars.period) | replace({'2023': 'toto'}) }}
      {{ render(vars.period | replace({'2023': 'toto'})) }}

triggers:
  - id: schedule
    type: io.kestra.core.models.triggers.types.Schedule
    cron: "*/1 * * * *"
```

@Ben8t please open a new issue if you see any usage patterns not being feasible with the new behavior in 0.14.0

Pull request review comments:
`true` is the default so you can revert this change
```suggestion
    public String render(String inline, boolean preprocessVariables) throws IllegalVariableEvaluationException {
```

I think it's better to call it preprocessed as it's what you do and variable are rendered anyway as we are recursive.
Revert as it's the default behaviour
I think all changes inside the RunContext can be reverted as the new functions are not used anywhere.
Just keep it simple for now
```suggestion
    public String recursiveRender(String inline, Map<String, Object> variables, Boolean preprocessVariables) throws IllegalVariableEvaluationException {
```
Please add an infinite loop test and assert that you get the result that preceeds the infinite loop turn
(var1: {{var2}}, var2: {{var3}}, var3: {{var1}})
=> render({{var1 | replace({'3': '4'})}}) => {{var4}} 
you can change the test as it's purely logic and not easily understandable but you get it

Patch that resolved the issue:
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 2d93dc724b..abffe19a25 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 InputStream getTaskStateFile(String state, String name, Boolean isNamespa
         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 95fcd42dc2..62e8da2c8f 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.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.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.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 VariableRenderer(ApplicationContext applicationContext, @Nullable Variabl
     }
 
     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 String recursiveRender(String inline, Map<String, Object> variables) thro
             return uuid;
         });
 
+        // pre-process variables
+        if (preprocessVariables) {
+            variables = this.preprocessVariables(variables);
+        }
+
         boolean isSame = false;
         String current = "";
         PebbleTemplate compiledTemplate;
@@ -131,28 +139,43 @@ public String recursiveRender(String inline, Map<String, Object> variables) thro
                 }
 
                 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 IllegalVariableEvaluationException properPebbleException(PebbleException
         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 Map<String, Object> render(Map<String, Object> in, Map<String, Object> va
     }
 
     @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 e3c63d8f3b..9a02057c4e 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 @@ void simple() throws IllegalVariableEvaluationException {
         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 076ed9b4a7..b28f0635c0 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.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 @@ void inputsLarge() throws TimeoutException {
     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 @@ void variables() throws TimeoutException {
         );
         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 d74c3cfde9..9d26477f02 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.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 edae4e9957..281177dbdf 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 e8af120b10..32ccf4d7ef 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