Merge pull request #20862 from vespa-engine/bjorncs/internal-validators

Support additional model validators

7 files changed, 23 insertions, 9 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java
index bd8fefdda2e..8e2945432b9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java
@@ -24,6 +24,7 @@ import com.yahoo.config.provision.TransientException;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.vespa.config.VespaVersion;
 import com.yahoo.vespa.model.application.validation.Validation;
+import com.yahoo.vespa.model.application.validation.Validator;
 import com.yahoo.yolean.Exceptions;
 import org.xml.sax.SAXException;
 
@@ -50,11 +51,13 @@ public class VespaModelFactory implements ModelFactory {
     private final Zone zone;
     private final Clock clock;
     private final Version version;
+    private final List<Validator> additionalValidators;
 
     /** Creates a factory for Vespa models for this version of the source */
     @Inject
     public VespaModelFactory(ComponentRegistry<ConfigModelPlugin> pluginRegistry,
                              ComponentRegistry<MlModelImporter> modelImporters,
+                             ComponentRegistry<Validator> additionalValidators,
                              Zone zone) {
         this.version = new Version(VespaVersion.major, VespaVersion.minor, VespaVersion.micro);
         List<ConfigModelBuilder> modelBuilders = new ArrayList<>();
@@ -66,6 +69,7 @@ public class VespaModelFactory implements ModelFactory {
         this.configModelRegistry = new MapConfigModelRegistry(modelBuilders);
         this.modelImporters = modelImporters.allComponents();
         this.zone = zone;
+        this.additionalValidators = List.copyOf(additionalValidators.allComponents());
 
         this.clock = Clock.systemUTC();
     }
@@ -91,6 +95,7 @@ public class VespaModelFactory implements ModelFactory {
             this.configModelRegistry = configModelRegistry;
         }
         this.modelImporters = Collections.emptyList();
+        this.additionalValidators = List.of();
         this.zone = zone;
         this.clock = clock;
     }
@@ -197,7 +202,7 @@ public class VespaModelFactory implements ModelFactory {
 
     private List<ConfigChangeAction> validateModel(VespaModel model, DeployState deployState, ValidationParameters validationParameters) {
         try {
-            return Validation.validate(model, validationParameters, deployState);
+            return new Validation(additionalValidators).validate(model, validationParameters, deployState);
         } catch (ValidationOverrides.ValidationException e) {
             if (deployState.isHosted() && zone.environment().isManuallyDeployed())
                 deployState.getDeployLogger().logApplicationPackage(Level.WARNING,
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
index 08dc73a1bd0..36503ba4bae 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
@@ -50,6 +50,13 @@ import static java.util.stream.Collectors.toList;
  */
 public class Validation {
 
+    private final List<Validator> additionalValidators;
+
+    public Validation() { this(List.of()); }
+
+    /** Create instance taking additional validators (e.g for cloud applications) */
+    public Validation(List<Validator> additionalValidators) { this.additionalValidators = additionalValidators; }
+
     /**
      * Validates the model supplied, and if there already exists a model for the application validates changes
      * between the previous and current model
@@ -57,7 +64,7 @@ public class Validation {
      * @return a list of required changes needed to make this configuration live
      * @throws ValidationOverrides.ValidationException if the change fails validation
      */
-    public static List<ConfigChangeAction> validate(VespaModel model, ValidationParameters validationParameters, DeployState deployState) {
+    public List<ConfigChangeAction> validate(VespaModel model, ValidationParameters validationParameters, DeployState deployState) {
         if (validationParameters.checkRouting()) {
             new RoutingValidator().validate(model, deployState);
             new RoutingSelectorValidator().validate(model, deployState);
@@ -80,6 +87,8 @@ public class Validation {
         new QuotaValidator().validate(model, deployState);
         new UriBindingsValidator().validate(model, deployState);
 
+        additionalValidators.forEach(v -> v.validate(model, deployState));
+
         List<ConfigChangeAction> result = Collections.emptyList();
         if (deployState.getProperties().isFirstTimeDeployment()) {
             validateFirstTimeDeployment(model, deployState);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java
index 689f1c70b6e..85050aa0cf9 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java
@@ -105,7 +105,7 @@ public class ComplexAttributeFieldsValidatorTestCase {
         DeployState deployState = createDeployState(servicesXml(), schema);
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
         ValidationParameters validationParameters = new ValidationParameters(CheckRouting.FALSE);
-        Validation.validate(model, validationParameters, deployState);
+        new Validation().validate(model, validationParameters, deployState);
     }
 
     private static DeployState createDeployState(String servicesXml, String schema) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidatorTest.java
index 16bff341c0b..93d42e07dd8 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidatorTest.java
@@ -37,7 +37,7 @@ public class ValidationOverridesValidatorTest {
 
         var deployState = createDeployState(validationOverridesXml);
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
-        Validation.validate(model, new ValidationParameters(), deployState);
+        new Validation().validate(model, new ValidationParameters(), deployState);
     }
 
     @Test
@@ -58,7 +58,7 @@ public class ValidationOverridesValidatorTest {
         try {
             var deployState = createDeployState(validationOverridesXml);
             VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
-            Validation.validate(model, new ValidationParameters(), deployState);
+            new Validation().validate(model, new ValidationParameters(), deployState);
             fail("Did not get expected exception");
         } catch (IllegalArgumentException e) {
             assertEquals(message, e.getMessage());
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java
index bf4d066e454..7f8bca825d2 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTestCase.java
@@ -242,7 +242,7 @@ public class VespaModelTestCase {
                 .build();
         DeployState deployState = builder.deployLogger(logger).applicationPackage(app).build();
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
-        Validation.validate(model, new ValidationParameters(ValidationParameters.IgnoreValidationErrors.TRUE), deployState);
+        new Validation().validate(model, new ValidationParameters(ValidationParameters.IgnoreValidationErrors.TRUE), deployState);
         assertFalse(logger.msgs.isEmpty());
     }
 
@@ -312,7 +312,7 @@ public class VespaModelTestCase {
                 .deployLogger(logger)
                 .build();
         VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
-        Validation.validate(model, new ValidationParameters(), deployState);
+        new Validation().validate(model, new ValidationParameters(), deployState);
         assertContainsWarning(logger.msgs, "Directory searchdefinitions/ should not be used for schemas, use schemas/ instead");
     }
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithFilePkg.java b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithFilePkg.java
index 351b080eb5b..565e4c7c076 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithFilePkg.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithFilePkg.java
@@ -66,7 +66,7 @@ public class VespaModelCreatorWithFilePkg {
             // is constructed in a special way and cannot always be validated in
             // this step for unit tests)
             ValidationParameters validationParameters = new ValidationParameters(IgnoreValidationErrors.TRUE, FailOnIncompatibleChange.TRUE, CheckRouting.FALSE);
-            Validation.validate(model, validationParameters, deployState);
+            new Validation().validate(model, validationParameters, deployState);
             return model;
         } catch (Exception e) {
             throw e instanceof RuntimeException ? (RuntimeException) e : new RuntimeException(e);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithMockPkg.java b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithMockPkg.java
index 66dc63bbb02..d777fefbbe5 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithMockPkg.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/utils/VespaModelCreatorWithMockPkg.java
@@ -75,7 +75,7 @@ public class VespaModelCreatorWithMockPkg {
                 // is constructed in a special way and cannot always be validated in
                 // this step for unit tests)
                 ValidationParameters validationParameters = new ValidationParameters(CheckRouting.FALSE);
-                configChangeActions = Validation.validate(model, validationParameters, deployState);
+                configChangeActions = new Validation().validate(model, validationParameters, deployState);
             }
             return model;
         } catch (Exception e) {