fix(core): fix StackOverflowError on plugin defaults (#4363)

Fix: #4363
kestra-io · Aug 1, 2024 · 02c8689 · 02c8689
1 parent 69a1e8a
commit 02c8689
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 11 deletions.
diff --git a/core/src/main/java/io/kestra/core/services/PluginDefaultService.java b/core/src/main/java/io/kestra/core/services/PluginDefaultService.java
@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.Lists;
 import io.kestra.core.models.Plugin;
 import io.kestra.core.models.executions.Execution;
@@ -165,20 +166,23 @@ private void addAliases(List<PluginDefault> allDefaults) {
         allDefaults.addAll(aliasedPluginDefault);
     }
 
-    private Object recursiveDefaults(Object object, Map<String, List<PluginDefault>> defaults) {
+    @VisibleForTesting
+    Object recursiveDefaults(Object object, Map<String, List<PluginDefault>> defaults) {
         if (object instanceof Map<?, ?> value) {
-            if (value.containsKey("type")) {
-                value = defaults(value, defaults);
-            }
-
-            return value
+            value = value
                 .entrySet()
                 .stream()
                 .map(e -> new AbstractMap.SimpleEntry<>(
                     e.getKey(),
                     recursiveDefaults(e.getValue(), defaults)
                 ))
                 .collect(HashMap::new, (m, v) -> m.put(v.getKey(), v.getValue()), HashMap::putAll);
+
+            if (value.containsKey("type")) {
+                value = defaults(value, defaults);
+            }
+
+            return value;
         } else if (object instanceof Collection<?> value) {
             return value
                 .stream()

diff --git a/core/src/test/java/io/kestra/core/services/PluginDefaultServiceTest.java b/core/src/test/java/io/kestra/core/services/PluginDefaultServiceTest.java
@@ -1,45 +1,104 @@
 package io.kestra.core.services;
 
 import com.google.common.collect.ImmutableMap;
+import io.kestra.core.junit.annotations.KestraTest;
 import io.kestra.core.models.annotations.Plugin;
 import io.kestra.core.models.conditions.ConditionContext;
-import io.kestra.core.models.flows.PluginDefault;
-import io.kestra.plugin.core.condition.ExpressionCondition;
 import io.kestra.core.models.executions.Execution;
 import io.kestra.core.models.flows.Flow;
+import io.kestra.core.models.flows.PluginDefault;
 import io.kestra.core.models.tasks.RunnableTask;
 import io.kestra.core.models.tasks.Task;
 import io.kestra.core.models.tasks.VoidOutput;
 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.plugin.core.trigger.Schedule;
 import io.kestra.core.runners.RunContext;
-import io.kestra.core.junit.annotations.KestraTest;
+import io.kestra.plugin.core.condition.ExpressionCondition;
+import io.kestra.plugin.core.trigger.Schedule;
+import jakarta.inject.Inject;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.ToString;
 import lombok.experimental.SuperBuilder;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.time.Duration;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import jakarta.inject.Inject;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.is;
 
 @KestraTest
 class PluginDefaultServiceTest {
+    private static final Map<String, Object> TEST_FLOW_AS_MAP = Map.of(
+        "id", "test",
+        "namespace", "type",
+        "tasks", List.of(
+            Map.of("id", "my-task", "type", "io.kestra.test")
+        )
+    );
+
     @Inject
     private PluginDefaultService pluginDefaultService;
 
+    @Test
+    void shouldInjectGivenDefaultsIncludingType() {
+        // Given
+        Map<String, List<PluginDefault>> defaults = Map.of(
+            "io.kestra.test",
+            List.of(new PluginDefault("io.kestra.test", false, Map.of("taskRunner", Map.of("type", "io.kestra.test"))))
+        );
+
+        // When
+        Object result = pluginDefaultService.recursiveDefaults(TEST_FLOW_AS_MAP, defaults);
+
+        // Then
+        Assertions.assertEquals(Map.of(
+            "id", "test",
+            "namespace", "type",
+            "tasks", List.of(
+                Map.of(
+                    "id", "my-task",
+                    "type", "io.kestra.test",
+                    "taskRunner", Map.of("type", "io.kestra.test")
+                )
+            )
+        ), result);
+    }
+
+    @Test
+    void shouldInjectGivenSimpleDefaults() {
+        // Given
+        Map<String, List<PluginDefault>> defaults = Map.of(
+            "io.kestra.test",
+            List.of(new PluginDefault("io.kestra.test", false, Map.of("default-key", "default-value")))
+        );
+
+        // When
+        Object result = pluginDefaultService.recursiveDefaults(TEST_FLOW_AS_MAP, defaults);
+
+        // Then
+        Assertions.assertEquals(Map.of(
+            "id", "test",
+            "namespace", "type",
+            "tasks", List.of(
+                Map.of(
+                    "id", "my-task",
+                    "type", "io.kestra.test",
+                    "default-key", "default-value"
+                )
+            )
+        ), result);
+    }
+
     @Test
     public void injectFlowAndGlobals() {
         DefaultTester task = DefaultTester.builder()