diff --git a/plugin-script-groovy/src/main/java/io/kestra/plugin/scripts/groovy/Script.java b/plugin-script-groovy/src/main/java/io/kestra/plugin/scripts/groovy/Script.java index e545f191..08cb4fd9 100644 --- a/plugin-script-groovy/src/main/java/io/kestra/plugin/scripts/groovy/Script.java +++ b/plugin-script-groovy/src/main/java/io/kestra/plugin/scripts/groovy/Script.java @@ -11,6 +11,7 @@ import io.kestra.core.models.property.Property; import io.kestra.core.models.tasks.RunnableTask; import io.kestra.core.models.tasks.runners.TargetOS; +import io.kestra.core.models.tasks.runners.TaskRunner; import io.kestra.core.runners.FilesService; import io.kestra.core.runners.RunContext; import io.kestra.plugin.scripts.exec.AbstractExecScript; @@ -31,7 +32,12 @@ @NoArgsConstructor @Schema( title = "Execute a Groovy script inline with your Flow Code", - description = "Runs an inline Groovy script in the JVM and captures its output." + description = """ + Runs an inline Groovy script in the JVM and captures its output. + + On the Docker task runner, the container runs as `root` unless `taskRunner.user` is set explicitly, so it can read the mounted script. Set `taskRunner.user` to keep the image's own default user instead. Other task runner settings are preserved. + + With the Process or Kubernetes task runner, `groovy` must be installed on the worker or in the pod, respectively.""" ) @Plugin( examples = { @@ -94,7 +100,9 @@ protected DockerOptions injectDefaults(RunContext runContext, DockerOptions orig builder.image(runContext.render(this.getContainerImage()).as(String.class).orElse(null)); } - builder.user("root"); + if (original.getUser() == null) { + builder.user("root"); + } return builder.build(); } @@ -102,6 +110,12 @@ protected DockerOptions injectDefaults(RunContext runContext, DockerOptions orig public ScriptOutput run(RunContext runContext) throws Exception { CommandsWrapper commands = this.commands(runContext); + // The Groovy image user may not be able to read the mounted script. + TaskRunner taskRunner = commands.getTaskRunner(); + if (taskRunner instanceof Docker docker && docker.getUser() == null) { + commands = commands.withTaskRunner(docker.toBuilder().user("root").build()); + } + Map inputFiles = FilesService.inputFiles(runContext, commands.getTaskRunner().additionalVars(runContext, commands), this.getInputFiles()); Path relativeScriptPath = runContext.workingDir().path().relativize(runContext.workingDir().createTempFile(".groovy")); inputFiles.put( @@ -115,12 +129,6 @@ public ScriptOutput run(RunContext runContext) throws Exception { .withInterpreter(this.interpreter) .withBeforeCommands(beforeCommands) .withBeforeCommandsWithOptions(true) - .withTaskRunner( - // because of, we are mounting a volume and the uid running Docker is not 1000, so it should run as user root (-u root). - Docker.builder() - .user("root") - .build() - ) .withCommands( Property.ofValue( List.of( @@ -131,4 +139,4 @@ public ScriptOutput run(RunContext runContext) throws Exception { .withTargetOS(os) .run(); } -} \ No newline at end of file +} diff --git a/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/CommandsTest.java b/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/CommandsTest.java index 2ee31631..85e18907 100644 --- a/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/CommandsTest.java +++ b/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/CommandsTest.java @@ -2,17 +2,12 @@ import java.io.InputStream; import java.io.InputStreamReader; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.List; -import java.util.Set; import java.util.UUID; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; -import com.github.dockerjava.api.DockerClient; import com.google.common.collect.ImmutableMap; import com.google.common.io.CharStreams; @@ -28,20 +23,19 @@ import io.kestra.core.utils.TestsUtils; import io.kestra.plugin.scripts.exec.scripts.models.DockerOptions; import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; -import io.kestra.plugin.scripts.runner.docker.DockerService; import jakarta.inject.Inject; import jakarta.inject.Named; import reactor.core.publisher.Flux; +import static io.kestra.plugin.scripts.groovy.GroovyTestImages.buildNonRootTestImage; +import static io.kestra.plugin.scripts.groovy.GroovyTestImages.removeTestImage; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; @KestraTest public class CommandsTest { - private static final long IMAGE_BUILD_TIMEOUT_MINUTES = 5; - @Inject RunContextFactory runContextFactory; @@ -85,11 +79,15 @@ void outputFilesOnNonRootImage() throws Exception { .allowWarning(true) .containerImage(Property.ofValue(nonRootImage)) .outputFiles(Property.ofValue(List.of("out.txt"))) - .commands(Property.ofValue(List.of( - "id", - "ls -ld .", - "groovy -e 'new File(\"out.txt\").text = \"hello\"'" - ))) + .commands( + Property.ofValue( + List.of( + "id", + "ls -ld .", + "groovy -e 'new File(\"out.txt\").text = \"hello\"'" + ) + ) + ) .build(); RunContext runContext = TestsUtils.mockRunContext(runContextFactory, groovyCommands, ImmutableMap.of()); @@ -120,11 +118,15 @@ void outputFilesOnNonRootImageLegacyDockerProperty() throws Exception { .allowWarning(true) .docker(DockerOptions.builder().image(nonRootImage).build()) .outputFiles(Property.ofValue(List.of("out.txt"))) - .commands(Property.ofValue(List.of( - "id", - "ls -ld .", - "groovy -e 'new File(\"out.txt\").text = \"hello\"'" - ))) + .commands( + Property.ofValue( + List.of( + "id", + "ls -ld .", + "groovy -e 'new File(\"out.txt\").text = \"hello\"'" + ) + ) + ) .build(); RunContext runContext = TestsUtils.mockRunContext(runContextFactory, groovyCommands, ImmutableMap.of()); @@ -144,31 +146,4 @@ void outputFilesOnNonRootImageLegacyDockerProperty() throws Exception { } } - private void buildNonRootTestImage(RunContext runContext, String tag) throws Exception { - Path buildContext = Files.createTempDirectory("groovy-non-root-image"); - Files.writeString(buildContext.resolve("Dockerfile"), """ - FROM groovy:jdk21 - USER root - RUN groupadd -g 5000 kestratest && useradd -u 5000 -g 5000 -m kestratest - USER kestratest - """); - - try (DockerClient dockerClient = DockerService.client(runContext, null, null, null, null)) { - dockerClient.buildImageCmd(buildContext.resolve("Dockerfile").toFile()) - .withTags(Set.of(tag)) - .start() - .awaitImageId(IMAGE_BUILD_TIMEOUT_MINUTES, TimeUnit.MINUTES); - } finally { - Files.deleteIfExists(buildContext.resolve("Dockerfile")); - Files.deleteIfExists(buildContext); - } - } - - private void removeTestImage(RunContext runContext, String tag) { - try (DockerClient dockerClient = DockerService.client(runContext, null, null, null, null)) { - dockerClient.removeImageCmd(tag).withForce(true).exec(); - } catch (Exception ignored) { - // best-effort cleanup of the throwaway test image - } - } -} \ No newline at end of file +} diff --git a/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/GroovyTestImages.java b/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/GroovyTestImages.java new file mode 100644 index 00000000..f5d722da --- /dev/null +++ b/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/GroovyTestImages.java @@ -0,0 +1,46 @@ +package io.kestra.plugin.scripts.groovy; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import com.github.dockerjava.api.DockerClient; + +import io.kestra.core.runners.RunContext; +import io.kestra.plugin.scripts.runner.docker.DockerService; + +final class GroovyTestImages { + private static final long IMAGE_BUILD_TIMEOUT_MINUTES = 5; + + private GroovyTestImages() { + } + + static void buildNonRootTestImage(RunContext runContext, String tag) throws Exception { + Path buildContext = Files.createTempDirectory("groovy-non-root-image"); + Files.writeString(buildContext.resolve("Dockerfile"), """ + FROM groovy:jdk21 + USER root + RUN groupadd -g 5000 kestratest && useradd -u 5000 -g 5000 -m kestratest + USER kestratest + """); + + try (DockerClient dockerClient = DockerService.client(runContext, null, null, null, null)) { + dockerClient.buildImageCmd(buildContext.resolve("Dockerfile").toFile()) + .withTags(Set.of(tag)) + .start() + .awaitImageId(IMAGE_BUILD_TIMEOUT_MINUTES, TimeUnit.MINUTES); + } finally { + Files.deleteIfExists(buildContext.resolve("Dockerfile")); + Files.deleteIfExists(buildContext); + } + } + + static void removeTestImage(RunContext runContext, String tag) { + try (DockerClient dockerClient = DockerService.client(runContext, null, null, null, null)) { + dockerClient.removeImageCmd(tag).withForce(true).exec(); + } catch (Exception ignored) { + // best-effort cleanup of the throwaway test image + } + } +} diff --git a/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/ScriptTest.java b/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/ScriptTest.java index 11048008..0f44cc39 100644 --- a/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/ScriptTest.java +++ b/plugin-script-groovy/src/test/java/io/kestra/plugin/scripts/groovy/ScriptTest.java @@ -1,12 +1,19 @@ package io.kestra.plugin.scripts.groovy; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.file.Path; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.concurrent.CopyOnWriteArrayList; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import com.google.common.collect.ImmutableMap; +import com.google.common.io.CharStreams; import io.kestra.core.junit.annotations.KestraTest; import io.kestra.core.models.executions.LogEntry; @@ -15,13 +22,24 @@ import io.kestra.core.queues.QueueInterface; import io.kestra.core.runners.RunContext; import io.kestra.core.runners.RunContextFactory; +import io.kestra.core.storages.StorageInterface; +import io.kestra.core.tenant.TenantService; import io.kestra.core.utils.TestsUtils; +import io.kestra.plugin.core.runner.Process; +import io.kestra.plugin.scripts.exec.scripts.models.DockerOptions; +import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; +import io.kestra.plugin.scripts.runner.docker.Docker; +import io.kestra.plugin.scripts.runner.docker.PullPolicy; +import groovy.lang.GroovyShell; import jakarta.inject.Inject; import jakarta.inject.Named; import reactor.core.publisher.Flux; +import static io.kestra.plugin.scripts.groovy.GroovyTestImages.buildNonRootTestImage; +import static io.kestra.plugin.scripts.groovy.GroovyTestImages.removeTestImage; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; @KestraTest @@ -30,6 +48,9 @@ public class ScriptTest { @Inject RunContextFactory runContextFactory; + @Inject + StorageInterface storageInterface; + @Inject @Named(QueueFactoryInterface.WORKERTASKLOG_NAMED) private QueueInterface logQueue; @@ -55,4 +76,85 @@ void script() throws Exception { receive.blockLast(); assertThat(List.copyOf(logs).stream().anyMatch(log -> log.getMessage() != null && log.getMessage().contains("Kestra is amazing!")), is(true)); } -} \ No newline at end of file + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void outputFilesOnNonRootImage(boolean legacyDocker) throws Exception { + runOnNonRootImage(legacyDocker, null, "0:0"); + } + + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void preservesExplicitDockerUser(boolean legacyDocker) throws Exception { + // Keep root's access to the mounted script, but use a distinct group to detect an overridden user. + runOnNonRootImage(legacyDocker, "root:5000", "0:5000"); + } + + private void runOnNonRootImage(boolean legacyDocker, String user, String expectedIdentity) throws Exception { + String image = "kestra-test/groovy-script-non-root:" + UUID.randomUUID(); + var builder = Script.builder() + .id("groovy-script-" + UUID.randomUUID()) + .type(Script.class.getName()) + .allowWarning(true) + .containerImage(Property.ofValue(image)) + .outputFiles(Property.ofValue(List.of("out.txt"))) + .script(Property.ofValue(""" + assert NetworkInterface.getByName('eth0') == null + def uid = ['id', '-u'].execute().text.trim() + def gid = ['id', '-g'].execute().text.trim() + new File('out.txt').text = uid + ':' + gid + """)); + if (legacyDocker) { + builder.docker(DockerOptions.builder().image(image).user(user).networkMode("none").build()); + } else { + builder.taskRunner( + Docker.builder() + .type(Docker.class.getName()) + .user(user) + .networkMode("none") + .pullPolicy(Property.ofValue(PullPolicy.NEVER)) + .build() + ); + } + var script = builder.build(); + RunContext runContext = TestsUtils.mockRunContext(runContextFactory, script, Map.of()); + + buildNonRootTestImage(runContext, image); + try { + assertOutput(script.run(runContext), expectedIdentity); + } finally { + removeTestImage(runContext, image); + } + } + + @Test + void preservesProcessRunner() throws Exception { + String java = Path.of(System.getProperty("java.home"), "bin", "java").toString(); + String groovy = Path.of(GroovyShell.class.getProtectionDomain().getCodeSource().getLocation().toURI()).toString(); + var script = Script.builder() + .id("groovy-script-" + UUID.randomUUID()) + .type(Script.class.getName()) + .taskRunner(Process.builder().type(Process.class.getName()).build()) + .beforeCommands( + Property.ofValue( + List.of( + "groovy() { '" + java.replace("'", "'\"'\"'") + "' -cp '" + groovy.replace("'", "'\"'\"'") + "' groovy.ui.GroovyMain \"$@\"; }" + ) + ) + ) + .outputFiles(Property.ofValue(List.of("out.txt"))) + .script(Property.ofValue("new File('out.txt').text = 'hello'")) + .build(); + RunContext runContext = TestsUtils.mockRunContext(runContextFactory, script, Map.of()); + + assertOutput(script.run(runContext), "hello"); + } + + private void assertOutput(ScriptOutput output, String expected) throws Exception { + assertThat(output.getExitCode(), is(0)); + assertThat(output.getOutputFiles(), hasKey("out.txt")); + try (InputStream input = storageInterface.get(TenantService.MAIN_TENANT, null, output.getOutputFiles().get("out.txt"))) { + assertThat(CharStreams.toString(new InputStreamReader(input)), is(expected)); + } + } +}