From f813dc821ed21db8942ec4a0a70f669a2c48b405 Mon Sep 17 00:00:00 2001 From: yuri1969 <1969yuri1969@gmail.com> Date: Thu, 4 Jan 2024 19:27:44 +0100 Subject: [PATCH] fix(script): no Docker options when not using Docker --- build.gradle | 4 + .../scripts/exec/AbstractExecScript.java | 2 + .../exec/scripts/models/DockerOptions.java | 2 + .../AbstractExecScriptValidation.java | 15 ++++ .../AbstractExecScriptValidator.java | 36 ++++++++ .../scripts/exec/AbstractExecScriptTest.java | 82 +++++++++++++++++++ 6 files changed, 141 insertions(+) create mode 100644 plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/AbstractExecScriptValidation.java create mode 100644 plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/validators/AbstractExecScriptValidator.java create mode 100644 plugin-script/src/test/java/io/kestra/plugin/scripts/exec/AbstractExecScriptTest.java diff --git a/build.gradle b/build.gradle index d23d6e25..f2fb557a 100644 --- a/build.gradle +++ b/build.gradle @@ -84,6 +84,10 @@ subprojects { } dependencies { + // lombok + testAnnotationProcessor "org.projectlombok:lombok:$lombokVersion" + testCompileOnly "org.projectlombok:lombok:$lombokVersion" + testAnnotationProcessor platform("io.micronaut:micronaut-bom:$micronautVersion") testAnnotationProcessor "io.micronaut:micronaut-inject-java" testImplementation platform("io.micronaut:micronaut-bom:$micronautVersion") diff --git a/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/AbstractExecScript.java b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/AbstractExecScript.java index 50b7f048..e2fcd7ef 100644 --- a/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/AbstractExecScript.java +++ b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/AbstractExecScript.java @@ -8,6 +8,7 @@ import io.kestra.plugin.scripts.exec.scripts.models.RunnerType; import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; import io.kestra.plugin.scripts.exec.scripts.runners.CommandsWrapper; +import io.kestra.plugin.scripts.exec.scripts.validations.AbstractExecScriptValidation; import io.swagger.v3.oas.annotations.media.Schema; import lombok.*; import lombok.experimental.SuperBuilder; @@ -22,6 +23,7 @@ @EqualsAndHashCode @Getter @NoArgsConstructor +@AbstractExecScriptValidation public abstract class AbstractExecScript extends Task implements RunnableTask, NamespaceFilesInterface, InputFilesInterface, OutputFilesInterface { @Builder.Default @Schema( diff --git a/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/models/DockerOptions.java b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/models/DockerOptions.java index aebe3c0c..d3ae2049 100644 --- a/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/models/DockerOptions.java +++ b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/models/DockerOptions.java @@ -4,6 +4,7 @@ import io.micronaut.core.annotation.Introspected; import io.swagger.v3.oas.annotations.media.Schema; import lombok.Builder; +import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.experimental.SuperBuilder; @@ -17,6 +18,7 @@ @NoArgsConstructor @Getter @Introspected +@EqualsAndHashCode public class DockerOptions { @Schema( title = "Docker api uri" diff --git a/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/AbstractExecScriptValidation.java b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/AbstractExecScriptValidation.java new file mode 100644 index 00000000..c6542714 --- /dev/null +++ b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/AbstractExecScriptValidation.java @@ -0,0 +1,15 @@ +package io.kestra.plugin.scripts.exec.scripts.validations; + +import io.kestra.plugin.scripts.exec.scripts.validations.validators.AbstractExecScriptValidator; + +import javax.validation.Constraint; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) +@Constraint(validatedBy = AbstractExecScriptValidator.class) +@Inherited +public @interface AbstractExecScriptValidation { + String message() default "invalid script ({validatedValue})"; +} diff --git a/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/validators/AbstractExecScriptValidator.java b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/validators/AbstractExecScriptValidator.java new file mode 100644 index 00000000..a51944c1 --- /dev/null +++ b/plugin-script/src/main/java/io/kestra/plugin/scripts/exec/scripts/validations/validators/AbstractExecScriptValidator.java @@ -0,0 +1,36 @@ +package io.kestra.plugin.scripts.exec.scripts.validations.validators; + +import io.kestra.plugin.scripts.exec.scripts.models.DockerOptions; +import io.kestra.plugin.scripts.exec.scripts.models.RunnerType; +import io.kestra.plugin.scripts.exec.scripts.validations.AbstractExecScriptValidation; +import io.micronaut.core.annotation.AnnotationValue; +import io.micronaut.core.annotation.Introspected; +import io.micronaut.core.annotation.NonNull; +import io.micronaut.core.annotation.Nullable; +import io.micronaut.validation.validator.constraints.ConstraintValidator; +import io.micronaut.validation.validator.constraints.ConstraintValidatorContext; +import jakarta.inject.Singleton; + +@Singleton +@Introspected +public class AbstractExecScriptValidator implements ConstraintValidator { + @Override + public boolean isValid( + @Nullable io.kestra.plugin.scripts.exec.AbstractExecScript value, + @NonNull AnnotationValue annotationMetadata, + @NonNull ConstraintValidatorContext context) { + if (value == null) { + return true; + } + + final DockerOptions defaultOptions = DockerOptions.builder().build(); + + if (value.getRunner() != RunnerType.DOCKER && value.getDocker() != null && !value.getDocker().equals(defaultOptions)) { + context.messageTemplate("invalid script: custom Docker options require the Docker runner"); + + return false; + } + + return true; + } +} diff --git a/plugin-script/src/test/java/io/kestra/plugin/scripts/exec/AbstractExecScriptTest.java b/plugin-script/src/test/java/io/kestra/plugin/scripts/exec/AbstractExecScriptTest.java new file mode 100644 index 00000000..e94985a3 --- /dev/null +++ b/plugin-script/src/test/java/io/kestra/plugin/scripts/exec/AbstractExecScriptTest.java @@ -0,0 +1,82 @@ +package io.kestra.plugin.scripts.exec; + +import io.kestra.core.models.validations.ModelValidator; +import io.kestra.core.runners.RunContext; +import io.kestra.plugin.scripts.exec.scripts.models.DockerOptions; +import io.kestra.plugin.scripts.exec.scripts.models.RunnerType; +import io.kestra.plugin.scripts.exec.scripts.models.ScriptOutput; +import io.micronaut.test.extensions.junit5.annotation.MicronautTest; +import jakarta.inject.Inject; +import lombok.Builder; +import lombok.Getter; +import lombok.experimental.SuperBuilder; +import org.apache.commons.lang3.NotImplementedException; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; + +@MicronautTest +class AbstractExecScriptTest { + @Inject + private ModelValidator modelValidator; + + @SuperBuilder + @Getter + public static class AbstractExecScriptCls extends AbstractExecScript { + @Builder.Default + protected DockerOptions docker = DockerOptions.builder().build(); + + @Override + public ScriptOutput run(RunContext runContext) throws Exception { + throw new NotImplementedException(); + } + } + + @Test + void validation() { + final AbstractExecScriptCls defaults = AbstractExecScriptCls.builder() + .id("unit-test") + .type(AbstractExecScriptCls.class.getName()) + .build(); + + assertThat(modelValidator.isValid(defaults).isEmpty(), is(true)); + + final AbstractExecScriptCls validDocker = AbstractExecScriptCls.builder() + .id("unit-test") + .type(AbstractExecScriptCls.class.getName()) + .docker(DockerOptions.builder() + .pullPolicy(DockerOptions.PullPolicy.IF_NOT_PRESENT) + .build() + ) + .runner(RunnerType.DOCKER) + .build(); + + assertThat(modelValidator.isValid(validDocker).isEmpty(), is(true)); + + final AbstractExecScriptCls validProcess = AbstractExecScriptCls.builder() + .id("unit-test") + .type(AbstractExecScriptCls.class.getName()) + .runner(RunnerType.PROCESS) + .build(); + + assertThat(modelValidator.isValid(validProcess).isEmpty(), is(true)); + + final AbstractExecScriptCls dockerPolicyWithProcessRunner = AbstractExecScriptCls.builder() + .id("unit-test") + .type(AbstractExecScriptCls.class.getName()) + .docker(DockerOptions.builder() + .pullPolicy(DockerOptions.PullPolicy.IF_NOT_PRESENT) + .build() + ) + .runner(RunnerType.PROCESS) + .build(); + + assertThat(modelValidator.isValid(dockerPolicyWithProcessRunner).isPresent(), is(true)); + assertThat( + modelValidator.isValid(dockerPolicyWithProcessRunner).get().getMessage(), + containsString(": invalid script: custom Docker options require the Docker runner") + ); + } +}