From c7216b9cb0ebc15c9565d002af822845769bb9f1 Mon Sep 17 00:00:00 2001 From: Andres Gomez Ferrer Date: Fri, 2 Feb 2024 13:36:51 +0100 Subject: [PATCH 1/2] Escape $ inside the task name Signed-off-by: Andres Gomez Ferrer Fix format Signed-off-by: Andres Gomez Ferrer Apply suggestions from code review Co-authored-by: Honnix Signed-off-by: Andres Gomez Ferrer --- .../flyte/jflyte/utils/ProjectClosure.java | 6 ++++- .../jflyte/utils/ProjectClosureTest.java | 23 ++++++++++++++++++- 2 files changed, 27 insertions(+), 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 index 21ad821c..33d51852 100644 --- 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 launchPlans(); + private static final Escaper ESCAPER = Escapers.builder().addEscape('$', "\\$").build(); + ProjectClosure applyCustom(JFlyteCustom custom) { Map 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 index d7b270da..9c00ef06 100644 --- 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,21 @@ 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"; + Resources expectedResources = Resources.builder().build(); + + // 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 +774,15 @@ void testCollectDynamicWorkflowTasks() { private RunnableTask createRunnableTask( Resources expectedResources, List customJavaToolOptions) { + return createRunnableTask("my-test-task", expectedResources, customJavaToolOptions); + } + + private RunnableTask createRunnableTask( + String name, Resources expectedResources, List customJavaToolOptions) { return new RunnableTask() { @Override public String getName() { - return "my-test-task"; + return name; } @Override From bc1a22808f1a9b42cadd880619e6328bb3fa7c6e Mon Sep 17 00:00:00 2001 From: Andres Gomez Ferrer Date: Fri, 2 Feb 2024 14:14:24 +0100 Subject: [PATCH 2/2] fix spotbugs Signed-off-by: Andres Gomez Ferrer --- .../src/test/java/org/flyte/jflyte/utils/ProjectClosureTest.java | 1 - 1 file changed, 1 deletion(-) 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 index 9c00ef06..e7d56096 100644 --- a/jflyte-utils/src/test/java/org/flyte/jflyte/utils/ProjectClosureTest.java +++ b/jflyte-utils/src/test/java/org/flyte/jflyte/utils/ProjectClosureTest.java @@ -447,7 +447,6 @@ public void testCreateTaskTemplateForRunnableTask() { // given RunnableTask task = createRunnableTask("NameWith$AsTaskName", null, List.of()); String image = "my-image"; - Resources expectedResources = Resources.builder().build(); // when TaskTemplate result = ProjectClosure.createTaskTemplateForRunnableTask(task, image);