Escape $ inside the task name (#283)

* Escape $ inside the task name Signed-off-by: Andres Gomez Ferrer <[email protected]> Fix format Signed-off-by: Andres Gomez Ferrer <[email protected]> Apply suggestions from code review Co-authored-by: Honnix <[email protected]> Signed-off-by: Andres Gomez Ferrer <[email protected]> * fix spotbugs Signed-off-by: Andres Gomez Ferrer <[email protected]> --------- Signed-off-by: Andres Gomez Ferrer <[email protected]> Co-authored-by: Andres Gomez Ferrer <[email protected]> Co-authored-by: Honnix <[email protected]>
flyteorg · Feb 2, 2024 · 8d4976e · 8d4976e
1 parent e1a1837
commit 8d4976e
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 2 deletions.
diff --git a/jflyte-utils/src/main/java/org/flyte/jflyte/utils/ProjectClosure.java b/jflyte-utils/src/main/java/org/flyte/jflyte/utils/ProjectClosure.java
@@ -28,6 +28,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import com.google.common.escape.Escaper;
+import com.google.common.escape.Escapers;
 import com.google.protobuf.ByteString;
 import java.io.File;
 import java.io.IOException;
@@ -92,6 +94,8 @@ public abstract class ProjectClosure {
 
   public abstract Map<LaunchPlanIdentifier, LaunchPlan> launchPlans();
 
+  private static final Escaper ESCAPER = Escapers.builder().addEscape('$', "\\$").build();
+
   ProjectClosure applyCustom(JFlyteCustom custom) {
     Map<TaskIdentifier, TaskSpec> rewrittenTaskSpecs =
         mapValues(taskSpecs(), x -> applyCustom(x, custom));
@@ -483,7 +487,7 @@ static TaskTemplate createTaskTemplateForRunnableTask(RunnableTask task, String
                     "jflyte",
                     "execute",
                     "--task",
-                    task.getName(),
+                    ESCAPER.escape(task.getName()),
                     "--inputs",
                     "{{.input}}",
                     "--outputPrefix",

diff --git a/jflyte-utils/src/test/java/org/flyte/jflyte/utils/ProjectClosureTest.java b/jflyte-utils/src/test/java/org/flyte/jflyte/utils/ProjectClosureTest.java
@@ -30,6 +30,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -441,6 +442,20 @@ public void testCreateTaskTemplateForRunnableTask() {
     assertThat(result.type(), equalTo("java-task"));
   }
 
+  @Test
+  public void testCreateTaskTemplateForRunnableTaskWith$Name() {
+    // given
+    RunnableTask task = createRunnableTask("NameWith$AsTaskName", null, List.of());
+    String image = "my-image";
+
+    // when
+    TaskTemplate result = ProjectClosure.createTaskTemplateForRunnableTask(task, image);
+
+    // then
+    assertTrue(
+        result.container().args().stream().anyMatch(s -> s.contains("NameWith\\$AsTaskName")));
+  }
+
   @Test
   public void testCreateTaskTemplateForRunnableTaskWithResources() {
     // given
@@ -758,10 +773,15 @@ void testCollectDynamicWorkflowTasks() {
 
   private RunnableTask createRunnableTask(
       Resources expectedResources, List<String> customJavaToolOptions) {
+    return createRunnableTask("my-test-task", expectedResources, customJavaToolOptions);
+  }
+
+  private RunnableTask createRunnableTask(
+      String name, Resources expectedResources, List<String> customJavaToolOptions) {
     return new RunnableTask() {
       @Override
       public String getName() {
-        return "my-test-task";
+        return name;
       }
 
       @Override