diff --git a/core/src/main/java/io/kestra/core/services/PluginDefaultService.java b/core/src/main/java/io/kestra/core/services/PluginDefaultService.java index 4bbe85afc4..27bccf20ce 100644 --- a/core/src/main/java/io/kestra/core/services/PluginDefaultService.java +++ b/core/src/main/java/io/kestra/core/services/PluginDefaultService.java @@ -19,20 +19,21 @@ import io.kestra.core.serializers.JacksonMapper; import io.kestra.core.serializers.YamlFlowParser; import io.kestra.core.utils.MapUtils; import io.micronaut.core.annotation.Nullable; +import jakarta.annotation.PostConstruct; import lombok.extern.slf4j.Slf4j; import org.slf4j.Logger; import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import java.util.stream.Stream; + import jakarta.inject.Inject; import jakarta.inject.Named; import jakarta.inject.Singleton; import jakarta.validation.ConstraintViolationException; -import static io.kestra.core.utils.Rethrow.throwConsumer; - @Singleton @Slf4j public class PluginDefaultService { @@ -61,6 +62,22 @@ public class PluginDefaultService { private AtomicBoolean warnOnce = new AtomicBoolean(false); + @PostConstruct + void validateGlobalPluginDefault() { + List mergedDefaults = new ArrayList<>(); + if (taskGlobalDefault != null && taskGlobalDefault.getDefaults() != null) { + mergedDefaults.addAll(taskGlobalDefault.getDefaults()); + } + + if (pluginGlobalDefault != null && pluginGlobalDefault.getDefaults() != null) { + mergedDefaults.addAll(pluginGlobalDefault.getDefaults()); + } + + mergedDefaults.stream() + .flatMap(pluginDefault -> this.validateDefault(pluginDefault).stream()) + .forEach(violation -> log.error("Invalid plugin default configuration: {}", violation)); + } + /** * @param flow the flow to extract default * @return list of {@code PluginDefault} ordered by most important first @@ -74,7 +91,7 @@ public class PluginDefaultService { if (taskGlobalDefault != null && taskGlobalDefault.getDefaults() != null) { if (warnOnce.compareAndSet(false, true)) { - log.warn("Global Task Defaults are deprecated, please use Global Plugin Defaults instead via the 'kestra.plugins.defaults' property."); + log.warn("Global Task Defaults are deprecated, please use Global Plugin Defaults instead via the 'kestra.plugins.defaults' configuration property."); } list.addAll(taskGlobalDefault.getDefaults()); } @@ -162,6 +179,36 @@ public class PluginDefaultService { return yamlFlowParser.parse(flowAsMap, Flow.class, false); } + /** + * Validate a plugin default by comparing its properties with the getters of the plugin class. + *

+ * If the plugin default type is unknown, + * validation will be disabled as we cannot differentiate between a prefix or an unknown type. + */ + public List validateDefault(PluginDefault pluginDefault) { + Class classByIdentifier = pluginRegistry.findClassByIdentifier(pluginDefault.getType()); + if (classByIdentifier == null) { + // this can either be a prefix or a non-existing plugin, in both cases we cannot validate in detail + return Collections.emptyList(); + } + + Set pluginDefaultProperties = pluginDefault.getValues().keySet(); + List pluginProperties = Stream.of(classByIdentifier.getMethods()) + .filter(method -> method.getName().startsWith("get") || method.getName().startsWith("if")) + .map(method -> { + if (method.getName().startsWith("get")) { + return method.getName().substring(3).toLowerCase(); + } + return method.getName().substring(2).toLowerCase(); + }) + .toList(); + + return pluginDefaultProperties.stream() + .filter(property -> !pluginProperties.contains(property.toLowerCase())) + .map(property -> "No property '" + property + "' exists in plugin '" + pluginDefault.getType() + "'") + .toList(); + } + private void addAliases(List allDefaults) { List aliasedPluginDefault = allDefaults.stream() .map(pluginDefault -> { diff --git a/core/src/main/java/io/kestra/core/validations/validator/PluginDefaultValidator.java b/core/src/main/java/io/kestra/core/validations/validator/PluginDefaultValidator.java index 1828cdcd4a..684e93092f 100644 --- a/core/src/main/java/io/kestra/core/validations/validator/PluginDefaultValidator.java +++ b/core/src/main/java/io/kestra/core/validations/validator/PluginDefaultValidator.java @@ -1,12 +1,14 @@ package io.kestra.core.validations.validator; import io.kestra.core.models.flows.PluginDefault; +import io.kestra.core.services.PluginDefaultService; 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.Inject; import jakarta.inject.Singleton; import io.kestra.core.validations.PluginDefaultValidation; @@ -18,6 +20,9 @@ import java.util.Map; @Singleton @Introspected public class PluginDefaultValidator implements ConstraintValidator { + @Inject + private PluginDefaultService pluginDefaultService; + @Override public boolean isValid(@Nullable PluginDefault value, @NonNull AnnotationValue annotationMetadata, @NonNull ConstraintValidatorContext context) { if (value == null) { @@ -27,28 +32,31 @@ public class PluginDefaultValidator implements ConstraintValidator violations = new ArrayList<>(); if (value.getValues() == null) { - violations.add("Null values map found"); + violations.add("No 'values' provided"); addConstraintViolation(context, violations); return false; } if (value.getType() == null) { - violations.add("No type provided"); + violations.add("No 'type' provided"); } // Check if the "values" map is empty for (Map.Entry entry : value.getValues().entrySet()) { if (entry.getValue() == null) { - violations.add("Null value found in values with key " + entry.getKey()); + violations.add("No value provided for key '" + entry.getKey() + "'"); } } + List strings = pluginDefaultService.validateDefault(value); + if(!strings.isEmpty()) { + violations.addAll(strings); + } + if (!violations.isEmpty()) { addConstraintViolation(context, violations); - return false; } else { - return true; } } diff --git a/core/src/test/java/io/kestra/core/validations/PluginDefaultValidationTest.java b/core/src/test/java/io/kestra/core/validations/PluginDefaultValidationTest.java index e1ff9939f9..3345676fe5 100644 --- a/core/src/test/java/io/kestra/core/validations/PluginDefaultValidationTest.java +++ b/core/src/test/java/io/kestra/core/validations/PluginDefaultValidationTest.java @@ -7,6 +7,8 @@ import jakarta.inject.Inject; import jakarta.validation.ConstraintViolationException; import org.junit.jupiter.api.Test; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import static org.hamcrest.MatcherAssert.assertThat; @@ -18,7 +20,7 @@ class PluginDefaultValidationTest { private ModelValidator modelValidator; @Test - void nullValue() { + void nullValuesShouldViolate() { PluginDefault pluginDefault = PluginDefault.builder() .type("io.kestra.tests") .build(); @@ -28,4 +30,54 @@ class PluginDefaultValidationTest { assertThat(validate.isPresent(), is(true)); } + @Test + void nullPropertiesShouldViolate() { + Map props = new HashMap<>(); + props.put("nullProperty", null); + PluginDefault pluginDefault = PluginDefault.builder() + .type("io.kestra.tests") + .values(props) + .build(); + + Optional validate = modelValidator.isValid(pluginDefault); + + assertThat(validate.isPresent(), is(true)); + } + + @Test + void unknownPropertyShouldViolate() { + PluginDefault pluginDefault = PluginDefault.builder() + .type("io.kestra.plugin.core.log.Log") + .values(Map.of("not", "existing")) + .build(); + + Optional validate = modelValidator.isValid(pluginDefault); + + assertThat(validate.isPresent(), is(true)); + } + + @Test + void unknownPropertyOnUnknownPluginShouldPass() { + PluginDefault pluginDefault = PluginDefault.builder() + .type("io.kestra.plugin.core.log") + .values(Map.of("not", "existing")) + .build(); + + Optional validate = modelValidator.isValid(pluginDefault); + + assertThat(validate.isEmpty(), is(true)); + } + + @Test + void validShouldPass() { + PluginDefault pluginDefault = PluginDefault.builder() + .type("io.kestra.plugin.core.log.Log") + .values(Map.of("level", "WARN")) + .build(); + + Optional validate = modelValidator.isValid(pluginDefault); + + assertThat(validate.isEmpty(), is(true)); + } + }