diff options
75 files changed, 727 insertions, 737 deletions
diff --git a/config-model-api/abi-spec.json b/config-model-api/abi-spec.json index 79283641074..e1081ebec99 100644 --- a/config-model-api/abi-spec.json +++ b/config-model-api/abi-spec.json @@ -786,10 +786,8 @@ "public void <init>(java.util.List)", "public void invalid(java.util.Map, java.time.Instant)", "public void invalid(com.yahoo.config.application.api.ValidationId, java.lang.String, java.time.Instant)", - "public java.util.Optional invalidException(java.util.Map, java.time.Instant)", "public boolean allows(java.lang.String, java.time.Instant)", "public boolean allows(com.yahoo.config.application.api.ValidationId, java.time.Instant)", - "public void validate(java.time.Instant, java.util.function.Consumer)", "public boolean validate(java.time.Instant)", "public java.lang.String xmlForm()", "public static java.lang.String toAllowMessage(com.yahoo.config.application.api.ValidationId)", diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java index ab722dfb919..7b52d825473 100644 --- a/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java +++ b/config-model-api/src/main/java/com/yahoo/config/application/api/ValidationOverrides.java @@ -19,7 +19,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.Consumer; import java.util.stream.Collectors; /** @@ -50,7 +49,10 @@ public class ValidationOverrides { /** Throws a ValidationException unless all given validation is overridden at this time */ public void invalid(Map<ValidationId, ? extends Collection<String>> messagesByValidationId, Instant now) { - invalidException(messagesByValidationId, now).ifPresent(e -> { throw e; }); + Map<ValidationId, Collection<String>> disallowed = new HashMap<>(messagesByValidationId); + disallowed.keySet().removeIf(id -> allows(id, now)); + if ( ! disallowed.isEmpty()) + throw new ValidationException(disallowed); } /** Throws a ValidationException unless this validation is overridden at this time */ @@ -59,21 +61,6 @@ public class ValidationOverrides { throw new ValidationException(validationId, message); } - public Optional<ValidationException> invalidException(Map<ValidationId, ? extends Collection<String>> messagesByValidationId, Instant now) { - Map<ValidationId, Collection<String>> disallowed = new HashMap<>(messagesByValidationId); - disallowed.keySet().removeIf(id -> allows(id, now)); - - if (disallowed.size() == 1 && disallowed.values().iterator().next().size() == 1) // Single-message form if possible. - return Optional.of(new ValidationException(disallowed.keySet().iterator().next(), - disallowed.values().iterator().next().iterator().next())); - - if ( ! disallowed.isEmpty()) - return Optional.of(new ValidationException(disallowed)); - - return Optional.empty(); - } - - // TODO: remove after 8.284 is gone public boolean allows(String validationIdString, Instant now) { Optional<ValidationId> validationId = ValidationId.from(validationIdString); if (validationId.isEmpty()) return false; // unknown id -> not allowed @@ -90,17 +77,12 @@ public class ValidationOverrides { } /** Validates overrides (checks 'until' date') */ - public void validate(Instant now, Consumer<String> reporter) { + public boolean validate(Instant now) { for (Allow override : overrides) { if (now.plus(Duration.ofDays(30)).isBefore(override.until)) - reporter.accept("validation-overrides is invalid: " + override + - " is too far in the future: Max 30 days is allowed"); + throw new IllegalArgumentException("validation-overrides is invalid: " + override + + " is too far in the future: Max 30 days is allowed"); } - } - - /** Validates overrides (checks 'until' date') */ - public boolean validate(Instant now) { - validate(now, message -> { throw new IllegalArgumentException(message); }); return false; } diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java index 417d7b2ff36..b1b7928c67a 100644 --- a/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java +++ b/config-model-api/src/test/java/com/yahoo/config/application/api/ValidationOverrideTest.java @@ -60,7 +60,7 @@ public class ValidationOverrideTest { try { ValidationOverrides overrides = ValidationOverrides.fromXml(new StringReader(validationOverrides)); Instant now = ManualClock.at("2000-01-01T23:59:00"); - overrides.allows(ValidationId.indexingChange, now); + overrides.allows("indexing-change", now); overrides.validate(now); Assert.fail("Expected validation interval override validation validation failure"); } @@ -80,7 +80,7 @@ public class ValidationOverrideTest { try { ValidationOverrides overrides = ValidationOverrides.fromXml(new StringReader(validationOverrides)); Instant now = ManualClock.at("2000-01-01T23:59:00"); - overrides.allows(ValidationId.indexingChange, now); + overrides.allows("indexing-change", now); overrides.validate(now); Assert.fail("Expected validation interval override validation validation failure"); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java index 12c482f3fdb..cc574db2454 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AbstractBundleValidator.java @@ -7,7 +7,7 @@ import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.path.Path; import com.yahoo.text.XML; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NodeList; @@ -20,7 +20,6 @@ import java.io.IOException; import java.io.StringReader; import java.nio.file.Paths; import java.util.Optional; -import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -32,47 +31,36 @@ import java.util.regex.Pattern; * * @author bjorncs */ -public abstract class AbstractBundleValidator implements Validator { +public abstract class AbstractBundleValidator extends Validator { - protected interface JarContext { - void illegal(String error); - void illegal(String error, Throwable cause); - DeployState deployState(); - static JarContext of(Context context) { - return new JarContext() { - @Override public void illegal(String error) { context.illegal(error); } - @Override public void illegal(String error, Throwable cause) { context.illegal(error, cause); } - @Override public DeployState deployState() { return context.deployState(); } - }; - } - } - - protected abstract void validateManifest(JarContext context, JarFile jar, Manifest mf); - protected abstract void validatePomXml(JarContext context, JarFile jar, Document pom); + protected abstract void validateManifest(DeployState state, JarFile jar, Manifest mf); + protected abstract void validatePomXml(DeployState state, JarFile jar, Document pom); @Override - public final void validate(Context context) { - ApplicationPackage app = context.deployState().getApplicationPackage(); - for (ComponentInfo info : app.getComponentsInfo(context.deployState().getVespaVersion())) { + public final void validate(VespaModel model, DeployState state) { + ApplicationPackage app = state.getApplicationPackage(); + for (ComponentInfo info : app.getComponentsInfo(state.getVespaVersion())) { Path path = Path.fromString(info.getPathRelativeToAppDir()); try { - context.deployState().getDeployLogger() + state.getDeployLogger() .log(Level.FINE, String.format("Validating bundle at '%s'", path)); JarFile jarFile = new JarFile(app.getFileReference(path)); - validateJarFile(JarContext.of(context), jarFile); + validateJarFile(state, jarFile); } catch (IOException e) { - context.illegal("Failed to validate JAR file '" + path.last() + "'", e); + throw new IllegalArgumentException( + "Failed to validate JAR file '" + path.last() + "'", e); } } } - final void validateJarFile(JarContext context, JarFile jar) throws IOException { + final void validateJarFile(DeployState state, JarFile jar) throws IOException { Manifest manifest = jar.getManifest(); if (manifest == null) { - context.illegal("Non-existing or invalid manifest in " + filename(jar)); + throw new IllegalArgumentException("Non-existing or invalid manifest in " + filename(jar)); } - validateManifest(context, jar, manifest); - getPomXmlContent(context::illegal, context.deployState().getDeployLogger(), jar).ifPresent(pom -> validatePomXml(context, jar, pom)); + validateManifest(state, jar, manifest); + getPomXmlContent(state.getDeployLogger(), jar) + .ifPresent(pom -> validatePomXml(state, jar, pom)); } protected final String filename(JarFile jarFile) { return Paths.get(jarFile.getName()).getFileName().toString(); } @@ -101,7 +89,7 @@ public abstract class AbstractBundleValidator implements Validator { } private static final Pattern POM_FILE_LOCATION = Pattern.compile("META-INF/maven/.+?/.+?/pom.xml"); - public Optional<Document> getPomXmlContent(BiConsumer<String, Throwable> context, DeployLogger logger, JarFile jar) { + public Optional<Document> getPomXmlContent(DeployLogger deployLogger, JarFile jar) { return jar.stream() .filter(f -> POM_FILE_LOCATION.matcher(f.getName()).matches()) .findFirst() @@ -112,13 +100,13 @@ public abstract class AbstractBundleValidator implements Validator { .parse(new InputSource(new StringReader(text))); } catch (SAXException e) { String message = String.format("Unable to parse pom.xml from %s", filename(jar)); - logger.log(Level.SEVERE, message); - context.accept(message, e); + deployLogger.log(Level.SEVERE, message); + throw new RuntimeException(message, e); } catch (IOException e) { - logger.log(Level.INFO, + deployLogger.log(Level.INFO, String.format("Unable to read '%s' from '%s'", f.getName(), jar.getName())); + return null; } - return null; }); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidator.java index aee9ca83b08..ee37157902c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidator.java @@ -1,8 +1,9 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.CloudName; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.http.AccessControl; import com.yahoo.vespa.model.container.http.Http; @@ -14,29 +15,29 @@ import java.util.logging.Level; * * @author mortent */ -public class AccessControlFilterExcludeValidator implements Validator { +public class AccessControlFilterExcludeValidator extends Validator { @Override - public void validate(Context context) { - if (!context.deployState().isHosted() || context.deployState().zone().system().isPublic()) return; - if (context.deployState().getProperties().allowDisableMtls()) return; - context.model().getContainerClusters().forEach((id, cluster) -> { + public void validate(VespaModel model, DeployState deployState) { + if (!deployState.isHosted() || deployState.zone().system().isPublic()) return; + if (deployState.getProperties().allowDisableMtls()) return; + model.getContainerClusters().forEach((id, cluster) -> { Http http = cluster.getHttp(); if (http != null) { if (http.getAccessControl().isPresent()) { - verifyNoExclusions(id, http.getAccessControl().get(), context); + verifyNoExclusions(id, http.getAccessControl().get(), deployState); } } }); } - private void verifyNoExclusions(String clusterId, AccessControl accessControl, Context context) { + private void verifyNoExclusions(String clusterId, AccessControl accessControl, DeployState deployState) { if (!accessControl.excludedBindings().isEmpty()) { String message = "Application cluster %s excludes paths from access control, this is not allowed and should be removed.".formatted(clusterId); - if (context.deployState().zone().cloud().name().equals(CloudName.AWS)) { - context.illegal(message); + if (deployState.zone().cloud().name().equals(CloudName.AWS)) { + throw new IllegalArgumentException(message); } else { - context.deployState().getDeployLogger().log(Level.WARNING, message); + deployState.getDeployLogger().log(Level.WARNING, message); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidator.java index cd6212051cf..8ea0155dd04 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidator.java @@ -1,7 +1,8 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.component.chain.Chain; import com.yahoo.vespa.model.container.http.AccessControl; import com.yahoo.vespa.model.container.http.Filter; @@ -13,26 +14,26 @@ import com.yahoo.vespa.model.container.http.Http; * * @author bjorncs */ -public class AccessControlFilterValidator implements Validator { +public class AccessControlFilterValidator extends Validator { @Override - public void validate(Context context) { - context.model().getContainerClusters().forEach((id, cluster) -> { + public void validate(VespaModel model, DeployState deployState) { + model.getContainerClusters().forEach((id, cluster) -> { Http http = cluster.getHttp(); if (http != null) { if (http.getAccessControl().isPresent()) { - verifyAccessControlFilterPresent(context, http); + verifyAccessControlFilterPresent(http); } } }); } - private static void verifyAccessControlFilterPresent(Context context, Http http) { + private static void verifyAccessControlFilterPresent(Http http) { FilterChains filterChains = http.getFilterChains(); Chain<Filter> chain = filterChains.allChains().getComponent(AccessControl.ACCESS_CONTROL_CHAIN_ID); if (chain.getInnerComponents().isEmpty()) { // No access control filter configured - it's up to a config model plugin to provide an implementation of an access control filter. - context.illegal("The 'access-control' feature is not available in open-source Vespa."); + throw new IllegalArgumentException("The 'access-control' feature is not available in open-source Vespa."); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java index 0a23e25e432..d877e58e158 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/BundleValidator.java @@ -27,8 +27,10 @@ import java.util.regex.Pattern; */ public class BundleValidator extends AbstractBundleValidator { + public BundleValidator() {} + @Override - protected void validateManifest(JarContext reporter, JarFile jar, Manifest mf) { + protected void validateManifest(DeployState state, JarFile jar, Manifest mf) { // Check for required OSGI headers Attributes attributes = mf.getMainAttributes(); HashSet<String> mfAttributes = new HashSet<>(); @@ -39,22 +41,23 @@ public class BundleValidator extends AbstractBundleValidator { "Bundle-ManifestVersion", "Bundle-Name", "Bundle-SymbolicName", "Bundle-Version"); for (String header : requiredOSGIHeaders) { if (!mfAttributes.contains(header)) { - reporter.illegal("Required OSGI header '" + header + "' was not found in manifest in '" + filename(jar) + "'"); + throw new IllegalArgumentException("Required OSGI header '" + header + + "' was not found in manifest in '" + filename(jar) + "'"); } } if (attributes.getValue("Bundle-Version").endsWith(".SNAPSHOT")) { - log(reporter.deployState(), Level.WARNING, + log(state, Level.WARNING, "Deploying snapshot bundle " + filename(jar) + ".\nTo use this bundle, you must include the " + "qualifier 'SNAPSHOT' in the version specification in services.xml."); } if (attributes.getValue("Import-Package") != null) { - validateImportedPackages(reporter.deployState(), jar, mf); + validateImportedPackages(state, jar, mf); } } - @Override protected void validatePomXml(JarContext reporter, JarFile jar, Document pom) { } + @Override protected void validatePomXml(DeployState state, JarFile jar, Document pom) {} private void validateImportedPackages(DeployState state, JarFile jar, Manifest manifest) { Map<DeprecatedProvidedBundle, List<String>> deprecatedPackagesInUse = new HashMap<>(); @@ -70,7 +73,7 @@ public class BundleValidator extends AbstractBundleValidator { }); deprecatedPackagesInUse.forEach((artifact, packagesInUse) -> { log(state, Level.WARNING, "JAR file '%s' imports the packages %s from '%s'. \n%s", - filename(jar), packagesInUse, artifact.name, artifact.description); + filename(jar), packagesInUse, artifact.name, artifact.description); }); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java index d59a76d5804..3b50412c44f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java @@ -2,12 +2,14 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.ApplicationPackage; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.io.IOUtils; import com.yahoo.io.reader.NamedReader; import com.yahoo.path.Path; import com.yahoo.security.X509CertificateUtils; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; +import java.io.IOException; import java.security.cert.X509Certificate; import java.util.HashSet; import java.util.List; @@ -16,23 +18,23 @@ import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; -public class CloudDataPlaneFilterValidator implements Validator { +public class CloudDataPlaneFilterValidator extends Validator { private static final Logger log = Logger.getLogger(CloudDataPlaneFilterValidator.class.getName()); @Override - public void validate(Context context) { - if (!context.deployState().isHosted()) return; - if (!context.deployState().zone().system().isPublic()) return; + public void validate(VespaModel model, DeployState deployState) { + if (!deployState.isHosted()) return; + if (!deployState.zone().system().isPublic()) return; - validateUniqueCertificates(context); + validateUniqueCertificates(deployState); } - private void validateUniqueCertificates(Context context) { - List<NamedReader> certFiles = context.deployState().getApplicationPackage().getFiles(ApplicationPackage.SECURITY_DIR, ".pem"); + private void validateUniqueCertificates(DeployState deployState) { + List<NamedReader> certFiles = deployState.getApplicationPackage().getFiles(ApplicationPackage.SECURITY_DIR, ".pem"); Map<String, List<X509Certificate>> configuredCertificates = certFiles.stream() - .collect(Collectors.toMap(NamedReader::getName, reader -> readCertificates(context, reader))); + .collect(Collectors.toMap(NamedReader::getName, CloudDataPlaneFilterValidator::readCertificates)); Set<X509Certificate> duplicates = new HashSet<>(); Set<X509Certificate> globalUniqueCerts = new HashSet<>(); @@ -51,21 +53,19 @@ public class CloudDataPlaneFilterValidator implements Validator { .map(p -> ApplicationPackage.SECURITY_DIR.append(p).getRelative()) .sorted() .toList(); - context.illegal("Duplicate certificate(s) detected in files: %s. Certificate subject of duplicates: %s" - .formatted(filesWithDuplicates.toString(), - duplicates.stream().map(cert -> cert.getSubjectX500Principal().getName()).toList().toString())); + throw new IllegalArgumentException("Duplicate certificate(s) detected in files: %s. Certificate subject of duplicates: %s" + .formatted(filesWithDuplicates.toString(), + duplicates.stream().map(cert -> cert.getSubjectX500Principal().getName()).toList().toString())); } } - private static List<X509Certificate> readCertificates(Context context, NamedReader reader) { + private static List<X509Certificate> readCertificates(NamedReader reader) { try { return X509CertificateUtils.certificateListFromPem(IOUtils.readAll(reader)); - } catch (Exception e) { + } catch (IOException e) { log.warning("Exception reading certificate list from application package. File: %s, exception message: %s" .formatted(reader.getName(), e.getMessage())); - context.illegal("Error reading certificates from application package", e); - return List.of(); + throw new RuntimeException("Error reading certificates from application package", e); } } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java index 4e4c8c2916c..1ddbf4453ae 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidator.java @@ -1,7 +1,9 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + package com.yahoo.vespa.model.application.validation; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.http.ConnectorFactory; import com.yahoo.vespa.model.container.http.ssl.DefaultSslProvider; @@ -14,12 +16,12 @@ import com.yahoo.vespa.model.container.http.ssl.HostedSslConnectorFactory; * * @author bjorncs */ -public class CloudHttpConnectorValidator implements Validator { +public class CloudHttpConnectorValidator extends Validator { @Override - public void validate(Context context) { - if (!context.deployState().isHostedTenantApplication(context.model().getAdmin().getApplicationType())) return; + public void validate(VespaModel model, DeployState state) { + if (!state.isHostedTenantApplication(model.getAdmin().getApplicationType())) return; - context.model().getContainerClusters().forEach((__, cluster) -> { + model.getContainerClusters().forEach((__, cluster) -> { var http = cluster.getHttp(); if (http == null) return; var illegalConnectors = http.getHttpServer().stream().flatMap(s -> s.getConnectorFactories().stream() @@ -27,7 +29,7 @@ public class CloudHttpConnectorValidator implements Validator { .map(cf -> "%s@%d".formatted(cf.getName(), cf.getListenPort())) .toList(); if (illegalConnectors.isEmpty()) return; - context.illegal( + throw new IllegalArgumentException( ("Adding additional or modifying existing HTTPS connectors is not allowed for Vespa Cloud applications." + " Violating connectors: %s. See https://cloud.vespa.ai/en/security/whitepaper, " + "https://cloud.vespa.ai/en/security/guide#data-plane.") diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidator.java index 0ed49bb96f1..935c3baddd2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidator.java @@ -1,34 +1,33 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.http.HttpFilterChain; +import java.util.Comparator; import java.util.TreeSet; import java.util.logging.Level; import java.util.stream.Collectors; -import static java.util.Comparator.comparing; - /** * Validates that only allowed-listed cloud applications can set up user-specified filter chains * * @author bjorncs */ -public class CloudUserFilterValidator implements Validator { +public class CloudUserFilterValidator extends Validator { @Override - public void validate(Context context) { - if (!context.deployState().isHostedTenantApplication(context.model().getAdmin().getApplicationType())) return; - if (context.deployState().getProperties().allowUserFilters()) return; - record Violation(String cluster, String chain) { } - var violations = new TreeSet<Violation>(comparing(Violation::chain).thenComparing(Violation::cluster)); - for (var cluster : context.model().getContainerClusters().values()) { + public void validate(VespaModel model, DeployState state) { + if (!state.isHostedTenantApplication(model.getAdmin().getApplicationType())) return; + if (state.getProperties().allowUserFilters()) return; + var violations = new TreeSet<Violation>(); + for (var cluster : model.getContainerClusters().values()) { if (cluster.getHttp() == null) continue; for (var chain : cluster.getHttp().getFilterChains().allChains().allComponents()) { if (chain.type() == HttpFilterChain.Type.USER) { var msg = "Found filter chain violation - chain '%s' in cluster '%s'".formatted(cluster.name(), chain.id()); - context.deployState().getDeployLogger().log(Level.WARNING, msg); + state.getDeployLogger().log(Level.WARNING, msg); violations.add(new Violation(cluster.name(), chain.id())); } } @@ -38,7 +37,14 @@ public class CloudUserFilterValidator implements Validator { .map(v -> "chain '%s' in cluster '%s'".formatted(v.chain(), v.cluster())) .collect(Collectors.joining(", ", "[", "]")); var msg = ("HTTP filter chains are currently not supported in Vespa Cloud (%s)").formatted(violationsStr); - context.illegal(msg); + throw new IllegalArgumentException(msg); + } + + private record Violation(String cluster, String chain) implements Comparable<Violation> { + @Override + public int compareTo(Violation other) { + return Comparator.comparing(Violation::chain).thenComparing(Violation::cluster).compare(this, other); + } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java index 12f3f025996..c7ae8f4f4a3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java @@ -2,17 +2,19 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.schema.Schema; import com.yahoo.schema.derived.SchemaInfo; import com.yahoo.schema.document.ComplexAttributeFieldUtils; import com.yahoo.schema.document.GeoPos; import com.yahoo.schema.document.ImmutableSDField; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.SearchCluster; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.logging.Level; import java.util.stream.Collectors; /** @@ -22,24 +24,24 @@ import java.util.stream.Collectors; * * @author geirst */ -public class ComplexFieldsWithStructFieldAttributesValidator implements Validator { +public class ComplexFieldsWithStructFieldAttributesValidator extends Validator { @Override - public void validate(Context context) { - List<SearchCluster> searchClusters = context.model().getSearchClusters(); + public void validate(VespaModel model, DeployState deployState) { + List<SearchCluster> searchClusters = model.getSearchClusters(); for (SearchCluster cluster : searchClusters) { if (cluster.isStreaming()) continue; for (SchemaInfo spec : cluster.schemas().values()) { - validateComplexFields(context, cluster.getClusterName(), spec.fullSchema(), context.deployState().getDeployLogger()); + validateComplexFields(cluster.getClusterName(), spec.fullSchema(), deployState.getDeployLogger()); } } } - private static void validateComplexFields(Context context, String clusterName, Schema schema, DeployLogger logger) { + private static void validateComplexFields(String clusterName, Schema schema, DeployLogger logger) { String unsupportedFields = validateComplexFields(schema); if (!unsupportedFields.isEmpty()) { - context.illegal(getErrorMessage(clusterName, schema, unsupportedFields)); + throw new IllegalArgumentException(getErrorMessage(clusterName, schema, unsupportedFields)); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java index 3dbba081400..b969387724c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java @@ -1,9 +1,11 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.schema.Schema; import com.yahoo.schema.document.ImmutableSDField; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import java.util.ArrayList; import java.util.Collection; @@ -19,21 +21,21 @@ import java.util.stream.Collectors; * * @author geirst */ -public class ComplexFieldsWithStructFieldIndexesValidator implements Validator { +public class ComplexFieldsWithStructFieldIndexesValidator extends Validator { @Override - public void validate(Context context) { - for (var cluster : context.model().getSearchClusters()) { + public void validate(VespaModel model, DeployState deployState) { + for (var cluster : model.getSearchClusters()) { if (cluster.isStreaming()) { continue; } for (var spec : cluster.schemas().values()) { - validateComplexFields(context, cluster.getClusterName(), spec.fullSchema()); + validateComplexFields(cluster.getClusterName(), spec.fullSchema(), deployState.getDeployLogger()); } } } - private static void validateComplexFields(Context context, String clusterName, Schema schema) { + private static void validateComplexFields(String clusterName, Schema schema, DeployLogger logger) { String unsupportedFields = schema.allFields() .filter(field -> hasStructFieldsWithIndex(field)) .map(ComplexFieldsWithStructFieldIndexesValidator::toString) @@ -41,8 +43,7 @@ public class ComplexFieldsWithStructFieldIndexesValidator implements Validator { if (!unsupportedFields.isEmpty()) { // TODO (Vespa 9 or before): Change back to an exception when no applications are using it wrong. - context.deployState().getDeployLogger().logApplicationPackage( - Level.WARNING, + logger.logApplicationPackage(Level.WARNING, String.format("For cluster '%s', schema '%s': The following complex fields have struct fields with 'indexing: index' which is not supported and has no effect: %s. " + "Remove setting or change to 'indexing: attribute' if needed for matching.", clusterName, schema.getName(), unsupportedFields)); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java index b3802bf211b..e4a07622ea3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ConstantValidator.java @@ -4,12 +4,13 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.application.provider.FilesApplicationPackage; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.path.Path; import com.yahoo.schema.DistributableResource; import com.yahoo.schema.RankProfile; import com.yahoo.schema.Schema; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ConstantTensorJsonValidator.InvalidConstantTensorException; -import com.yahoo.vespa.model.application.validation.Validation.Context; import java.io.FileNotFoundException; @@ -18,22 +19,22 @@ import java.io.FileNotFoundException; * * @author Vegard Sjonfjell */ -public class ConstantValidator implements Validator { +public class ConstantValidator extends Validator { @Override - public void validate(Context context) { + public void validate(VespaModel model, DeployState deployState) { var exceptionMessageCollector = new ExceptionMessageCollector("Invalid constant tensor file(s):"); - for (Schema schema : context.deployState().getSchemas()) { + for (Schema schema : deployState.getSchemas()) { for (var constant : schema.declaredConstants().values()) - validate(constant, context.deployState().getApplicationPackage(), exceptionMessageCollector); - for (var profile : context.deployState().rankProfileRegistry().rankProfilesOf(schema)) { + validate(constant, deployState.getApplicationPackage(), exceptionMessageCollector); + for (var profile : deployState.rankProfileRegistry().rankProfilesOf(schema)) { for (var constant : profile.declaredConstants().values()) - validate(constant, context.deployState().getApplicationPackage(), exceptionMessageCollector); + validate(constant, deployState.getApplicationPackage(), exceptionMessageCollector); } } if (exceptionMessageCollector.exceptionsOccurred) - context.illegal(exceptionMessageCollector.combinedMessage); + throw new IllegalArgumentException(exceptionMessageCollector.combinedMessage); } private void validate(RankProfile.Constant constant, diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidator.java index 71b5b47b732..49ff9b4cfde 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidator.java @@ -1,19 +1,20 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; /** * Validates that a Vespa Cloud application has at least one container cluster. * * @author jonmv */ -public class ContainerInCloudValidator implements Validator { +public class ContainerInCloudValidator extends Validator { @Override - public void validate(Context context) { - if (context.deployState().isHosted() && context.model().getContainerClusters().isEmpty()) - context.illegal("Vespa Cloud applications must have at least one container cluster"); + public void validate(VespaModel model, DeployState deployState) { + if (deployState.isHosted() && model.getContainerClusters().isEmpty()) + throw new IllegalArgumentException("Vespa Cloud applications must have at least one container cluster"); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java index 1675bacb387..7e0df4cf1fa 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidator.java @@ -3,8 +3,9 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.DeploymentInstanceSpec; import com.yahoo.config.application.api.DeploymentSpec; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.InstanceName; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.ContainerModel; import java.io.Reader; @@ -17,29 +18,29 @@ import java.util.Optional; * @author hmusum * @author bratseth */ -public class DeploymentSpecValidator implements Validator { +public class DeploymentSpecValidator extends Validator { @Override - public void validate(Context context) { - Optional<Reader> deployment = context.deployState().getApplicationPackage().getDeployment(); + public void validate(VespaModel model, DeployState deployState) { + Optional<Reader> deployment = deployState.getApplicationPackage().getDeployment(); if ( deployment.isEmpty()) return; Reader deploymentReader = deployment.get(); DeploymentSpec deploymentSpec = DeploymentSpec.fromXml(deploymentReader); - List<ContainerModel> containers = context.model().getRoot().configModelRepo().getModels(ContainerModel.class); + List<ContainerModel> containers = model.getRoot().configModelRepo().getModels(ContainerModel.class); for (DeploymentInstanceSpec instance : deploymentSpec.instances()) { instance.endpoints().forEach(endpoint -> { - requireClusterId(context, containers, instance.name(), - "Endpoint '" + endpoint.endpointId() + "'", endpoint.containerId()); + requireClusterId(containers, instance.name(), "Endpoint '" + endpoint.endpointId() + "'", + endpoint.containerId()); }); } } - private static void requireClusterId(Context context, List<ContainerModel> containers, InstanceName instanceName, - String endpoint, String id) { + private static void requireClusterId(List<ContainerModel> containers, InstanceName instanceName, String context, + String id) { if (containers.stream().noneMatch(container -> container.getCluster().getName().equals(id))) - context.illegal(endpoint + " in instance " + instanceName + ": '" + id + - "' specified in deployment.xml does not match any container cluster ID"); + throw new IllegalArgumentException(context + " in instance " + instanceName + ": '" + id + + "' specified in deployment.xml does not match any container cluster ID"); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java index f2887d9f9b9..635f7c67dd6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java @@ -1,17 +1,18 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.CertificateNotReadyException; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; -public class EndpointCertificateSecretsValidator implements Validator { +public class EndpointCertificateSecretsValidator extends Validator { /** This check is delayed until validation to allow node provisioning to complete while we are waiting for cert */ @Override - public void validate(Context context) { - if (context.deployState().endpointCertificateSecrets().isPresent() && context.deployState().endpointCertificateSecrets().get().isMissing()) { + public void validate(VespaModel model, DeployState deployState) { + if (deployState.endpointCertificateSecrets().isPresent() && deployState.endpointCertificateSecrets().get().isMissing()) { throw new CertificateNotReadyException("TLS enabled, but could not yet retrieve certificate version %s for application %s" - .formatted(context.deployState().endpointCertificateSecrets().get().version(), context.deployState().getProperties().applicationId().serializedForm())); + .formatted(deployState.endpointCertificateSecrets().get().version(), deployState.getProperties().applicationId().serializedForm())); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidator.java index d4f26530b75..30209d0bdee 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidator.java @@ -2,8 +2,9 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.model.ConfigModelContext; -import com.yahoo.config.provision.TenantName; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.vespa.model.VespaModel; import java.util.logging.Logger; @@ -12,19 +13,18 @@ import java.util.logging.Logger; * * @author mortent */ -public class InfrastructureDeploymentValidator implements Validator { +public class InfrastructureDeploymentValidator extends Validator { private static final Logger log = Logger.getLogger(InfrastructureDeploymentValidator.class.getName()); @Override - public void validate(Context context) { + public void validate(VespaModel model, DeployState deployState) { // Allow the internally defined tenant owning all infrastructure applications - if (TenantName.from("hosted-vespa").equals(context.model().applicationPackage().getApplicationId().tenant())) return; - ConfigModelContext.ApplicationType applicationType = context.model().getAdmin().getApplicationType(); + if (ApplicationId.global().tenant().equals(model.applicationPackage().getApplicationId().tenant())) return; + ConfigModelContext.ApplicationType applicationType = model.getAdmin().getApplicationType(); if (applicationType != ConfigModelContext.ApplicationType.DEFAULT) { - log.warning("Tenant %s is not allowed to use application type %s".formatted(context.model().applicationPackage().getApplicationId().toFullString(), applicationType)); - context.illegal("Tenant is not allowed to override application type"); + log.warning("Tenant %s is not allowed to use application type %s".formatted(model.applicationPackage().getApplicationId().toFullString(), applicationType)); + throw new IllegalArgumentException("Tenant is not allowed to override application type"); } } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java index 482c4477cdc..e9038ff2b0f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidator.java @@ -1,8 +1,10 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.text.Text; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import java.util.logging.Level; @@ -11,37 +13,37 @@ import java.util.logging.Level; * * @author bjorncs */ -public class JvmHeapSizeValidator implements Validator { +public class JvmHeapSizeValidator extends Validator { public static final int percentLimit = 15; public static final double gbLimit = 0.6; @Override - public void validate(Context context) { - if (!context.deployState().featureFlags().dynamicHeapSize()) return; - if (!context.deployState().isHostedTenantApplication(context.model().getAdmin().getApplicationType())) return; + public void validate(VespaModel model, DeployState ds) { + if (!ds.featureFlags().dynamicHeapSize()) return; + if (!ds.isHostedTenantApplication(model.getAdmin().getApplicationType())) return; - context.model().getContainerClusters().forEach((clusterId, appCluster) -> { + model.getContainerClusters().forEach((clusterId, appCluster) -> { var mp = appCluster.getMemoryPercentage().orElse(null); if (mp == null) return; if (mp.availableMemoryGb().isEmpty()) { - context.deployState().getDeployLogger().log(Level.FINE, "Host resources unknown or percentage overridden with 'allocated-memory'"); + ds.getDeployLogger().log(Level.FINE, "Host resources unknown or percentage overridden with 'allocated-memory'"); return; } long jvmModelCost = appCluster.onnxModelCostCalculator().aggregatedModelCostInBytes(); if (jvmModelCost > 0) { double availableMemoryGb = mp.availableMemoryGb().getAsDouble(); double modelCostGb = jvmModelCost / (1024D * 1024 * 1024); - context.deployState().getDeployLogger().log(Level.FINE, () -> Text.format("JVM: %d%% (limit: %d%%), %.2fGB (limit: %.2fGB), ONNX: %.2fGB", + ds.getDeployLogger().log(Level.FINE, () -> Text.format("JVM: %d%% (limit: %d%%), %.2fGB (limit: %.2fGB), ONNX: %.2fGB", mp.percentage(), percentLimit, availableMemoryGb, gbLimit, modelCostGb)); if (mp.percentage() < percentLimit) { - context.illegal(Text.format("Allocated percentage of memory of JVM in cluster '%s' is too low (%d%% < %d%%). " + + throw new IllegalArgumentException(Text.format("Allocated percentage of memory of JVM in cluster '%s' is too low (%d%% < %d%%). " + "Estimated cost of ONNX models is %.2fGB. Either use a node flavor with more memory or use less expensive models. " + "You may override this validation by specifying 'allocated-memory' (https://docs.vespa.ai/en/performance/container-tuning.html#jvm-heap-size).", clusterId, mp.percentage(), percentLimit, modelCostGb)); } if (availableMemoryGb < gbLimit) { - context.illegal( + throw new IllegalArgumentException( Text.format("Allocated memory to JVM in cluster '%s' is too low (%.2fGB < %.2fGB). " + "Estimated cost of ONNX models is %.2fGB. Either use a node flavor with more memory or use less expensive models. " + "You may override this validation by specifying 'allocated-memory' (https://docs.vespa.ai/en/performance/container-tuning.html#jvm-heap-size).", diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/NoPrefixForIndexes.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/NoPrefixForIndexes.java index 15d293e4abc..15d3e63c7fa 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/NoPrefixForIndexes.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/NoPrefixForIndexes.java @@ -1,15 +1,16 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.schema.Index; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.schema.Schema; -import com.yahoo.schema.derived.DerivedConfiguration; import com.yahoo.schema.document.ImmutableSDField; import com.yahoo.schema.document.MatchAlgorithm; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.schema.Index; +import com.yahoo.schema.derived.DerivedConfiguration; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.vespa.model.search.SearchCluster; import com.yahoo.vespa.model.search.DocumentDatabase; import com.yahoo.vespa.model.search.IndexedSearchCluster; -import com.yahoo.vespa.model.search.SearchCluster; import java.util.Map; @@ -18,11 +19,11 @@ import java.util.Map; * * @author vegardh */ -public class NoPrefixForIndexes implements Validator { +public class NoPrefixForIndexes extends Validator { @Override - public void validate(Context context) { - for (SearchCluster cluster : context.model().getSearchClusters()) { + public void validate(VespaModel model, DeployState deployState) { + for (SearchCluster cluster : model.getSearchClusters()) { if (cluster instanceof IndexedSearchCluster) { IndexedSearchCluster sc = (IndexedSearchCluster) cluster; for (DocumentDatabase docDb : sc.getDocumentDbs()) { @@ -32,11 +33,11 @@ public class NoPrefixForIndexes implements Validator { if (field.doesIndexing()) { //if (!field.getIndexTo().isEmpty() && !field.getIndexTo().contains(field.getName())) continue; if (field.getMatching().getAlgorithm().equals(MatchAlgorithm.PREFIX)) { - failField(context, schema, field); + failField(schema, field); } for (Map.Entry<String, Index> e : field.getIndices().entrySet()) { if (e.getValue().isPrefix()) { - failField(context, schema, field); + failField(schema, field); } } } @@ -46,8 +47,8 @@ public class NoPrefixForIndexes implements Validator { } } - private void failField(Context context, Schema schema, ImmutableSDField field) { - context.illegal("For " + schema + ", field '" + field.getName() + - "': match/index:prefix is not supported for indexes."); + private void failField(Schema schema, ImmutableSDField field) { + throw new IllegalArgumentException("For " + schema + ", field '" + field.getName() + + "': match/index:prefix is not supported for indexes."); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidator.java index e0a43f0988a..68e0172931a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidator.java @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import org.w3c.dom.Document; import java.util.Arrays; @@ -14,17 +15,17 @@ import java.util.logging.Level; public class PublicApiBundleValidator extends AbstractBundleValidator { @Override - protected void validateManifest(JarContext context, JarFile jar, Manifest mf) { + protected void validateManifest(DeployState state, JarFile jar, Manifest mf) { String nonPublicApiAttribute = mf.getMainAttributes().getValue("X-JDisc-Non-PublicApi-Import-Package"); if (nonPublicApiAttribute == null) return; var nonPublicApisUsed = Arrays.asList(nonPublicApiAttribute.split(",")); if (! nonPublicApisUsed.isEmpty()) { - log(context.deployState(), Level.WARNING, "Jar file '%s' uses non-public Vespa APIs: %s", filename(jar), nonPublicApisUsed); + log(state, Level.WARNING, "Jar file '%s' uses non-public Vespa APIs: %s", filename(jar), nonPublicApisUsed); } } @Override - protected void validatePomXml(JarContext context, JarFile jar, Document pom) { } + protected void validatePomXml(DeployState state, JarFile jar, Document pom) { } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java index 4d9386b5f19..86cedd3ebbf 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/QuotaValidator.java @@ -1,6 +1,7 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.Capacity; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; @@ -9,7 +10,6 @@ import com.yahoo.config.provision.QuotaExceededException; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.Context; import java.math.BigDecimal; import java.util.LinkedHashSet; @@ -24,16 +24,16 @@ import java.util.stream.Collectors; * * @author ogronnesby */ -public class QuotaValidator implements Validator { +public class QuotaValidator extends Validator { private static final Logger log = Logger.getLogger(QuotaValidator.class.getName()); private static final Capacity zeroCapacity = Capacity.from(new ClusterResources(0, 0, NodeResources.zero())); @Override - public void validate(Context context) { - var quota = context.deployState().getProperties().quota(); - quota.maxClusterSize().ifPresent(maxClusterSize -> validateMaxClusterSize(maxClusterSize, context.model())); - quota.budgetAsDecimal().ifPresent(budget -> validateBudget(budget, context.model(), context.deployState().getProperties().zone())); + public void validate(VespaModel model, DeployState deployState) { + var quota = deployState.getProperties().quota(); + quota.maxClusterSize().ifPresent(maxClusterSize -> validateMaxClusterSize(maxClusterSize, model)); + quota.budgetAsDecimal().ifPresent(budget -> validateBudget(budget, model, deployState.getProperties().zone())); } private void validateBudget(BigDecimal budget, VespaModel model, Zone zone) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankSetupValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankSetupValidator.java index ebfce1353d2..b3fb25be2e5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankSetupValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RankSetupValidator.java @@ -5,6 +5,7 @@ import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.collections.Pair; import com.yahoo.config.ConfigInstance; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AnyConfigProducer; import com.yahoo.io.IOUtils; import com.yahoo.log.InvalidLogFormatException; @@ -20,12 +21,11 @@ import com.yahoo.vespa.config.search.core.OnnxModelsConfig; import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import com.yahoo.vespa.defaults.Defaults; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.DocumentDatabase; import com.yahoo.vespa.model.search.IndexedSearchCluster; import com.yahoo.vespa.model.search.SearchCluster; import com.yahoo.yolean.Exceptions; - import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -44,7 +44,7 @@ import java.util.logging.Logger; * * @author vegardh */ -public class RankSetupValidator implements Validator { +public class RankSetupValidator extends Validator { private static final Logger log = Logger.getLogger(RankSetupValidator.class.getName()); private static final String binaryName = "vespa-verify-ranksetup-bin "; @@ -56,14 +56,14 @@ public class RankSetupValidator implements Validator { } @Override - public void validate(Context context) { + public void validate(VespaModel model, DeployState deployState) { File cfgDir = null; try { cfgDir = Files.createTempDirectory("verify-ranksetup." + - context.deployState().getProperties().applicationId().toFullString() + + deployState.getProperties().applicationId().toFullString() + ".") .toFile(); - for (SearchCluster cluster : context.model().getSearchClusters()) { + for (SearchCluster cluster : model.getSearchClusters()) { // Skipping ranking expression checking for streaming clusters, not implemented yet if (cluster.isStreaming()) continue; @@ -74,24 +74,24 @@ public class RankSetupValidator implements Validator { String schemaDir = clusterDir + schemaName + "/"; writeConfigs(schemaDir, docDb); writeExtraVerifyRankSetupConfig(schemaDir, docDb); - if (!validate(context, "dir:" + schemaDir, sc, schemaName, cfgDir)) { + if (!validate("dir:" + schemaDir, sc, schemaName, deployState.getDeployLogger(), cfgDir)) { return; } } } } catch (IOException e) { - context.illegal("unable to read rank setup", e); + throw new RuntimeException(e); } finally { if (cfgDir != null) deleteTempDir(cfgDir); } } - private boolean validate(Context context, String configId, SearchCluster searchCluster, String schema, File tempDir) { + private boolean validate(String configId, SearchCluster searchCluster, String schema, DeployLogger deployLogger, File tempDir) { Instant start = Instant.now(); try { log.log(Level.FINE, () -> String.format("Validating schema '%s' for cluster %s with config id %s", schema, searchCluster, configId)); - boolean ret = execValidate(context, configId, searchCluster, schema); + boolean ret = execValidate(configId, searchCluster, schema, deployLogger); if (!ret) { // Give up, don't log same error msg repeatedly deleteTempDir(tempDir); @@ -100,8 +100,7 @@ public class RankSetupValidator implements Validator { return ret; } catch (IllegalArgumentException e) { deleteTempDir(tempDir); - context.illegal("failed validating rank setup", e); - return false; + throw e; } } @@ -171,17 +170,17 @@ public class RankSetupValidator implements Validator { IOUtils.writeFile(dir + configName, StringUtilities.implodeMultiline(ConfigInstance.serialize(config)), false); } - private boolean execValidate(Context context, String configId, SearchCluster sc, String sdName) { + private boolean execValidate(String configId, SearchCluster sc, String sdName, DeployLogger deployLogger) { String command = String.format("%s %s", binaryName, configId); try { Pair<Integer, String> ret = new ProcessExecuter(true).exec(command); Integer exitCode = ret.getFirst(); String output = ret.getSecond(); if (exitCode != 0) { - validateFail(context, output, exitCode, sc, sdName); + validateFail(output, exitCode, sc, sdName, deployLogger); } } catch (IOException e) { - validateWarn(e, context.deployState().getDeployLogger()); + validateWarn(e, deployLogger); return false; } return true; @@ -194,7 +193,7 @@ public class RankSetupValidator implements Validator { deployLogger.logApplicationPackage(Level.WARNING, msg); } - private void validateFail(Context context, String output, int exitCode, SearchCluster sc, String sdName) { + private void validateFail(String output, int exitCode, SearchCluster sc, String sdName, DeployLogger deployLogger) { StringBuilder message = new StringBuilder("Error in rank setup in schema '").append(sdName) .append("' for content cluster '").append(sc.getClusterName()).append("'.").append(" Details:\n"); if (output.isEmpty()) { @@ -225,9 +224,9 @@ public class RankSetupValidator implements Validator { } if (ignoreValidationErrors) { - context.deployState().getDeployLogger().log(Level.WARNING, message.append("(Continuing since ignoreValidationErrors flag is set.)").toString()); + deployLogger.log(Level.WARNING, message.append("(Continuing since ignoreValidationErrors flag is set.)").toString()); } else { - context.illegal(message.toString()); + throw new IllegalArgumentException(message.toString()); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingSelectorValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingSelectorValidator.java index bd933f1c656..d2a26d87899 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingSelectorValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingSelectorValidator.java @@ -1,20 +1,21 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.document.select.DocumentSelector; -import com.yahoo.vespa.model.application.validation.Validation.Context; -import com.yahoo.vespa.model.search.IndexedSearchCluster; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.SearchCluster; +import com.yahoo.vespa.model.search.IndexedSearchCluster; /** * Validates routing selector for search and content clusters */ -public class RoutingSelectorValidator implements Validator { +public class RoutingSelectorValidator extends Validator { @Override - public void validate(Context context) { - for (SearchCluster cluster : context.model().getSearchClusters()) { + public void validate(VespaModel model, DeployState deployState) { + for (SearchCluster cluster : model.getSearchClusters()) { if (cluster instanceof IndexedSearchCluster) { IndexedSearchCluster sc = (IndexedSearchCluster) cluster; String routingSelector = sc.getRoutingSelector(); @@ -22,7 +23,8 @@ public class RoutingSelectorValidator implements Validator { try { new DocumentSelector(routingSelector); } catch (com.yahoo.document.select.parser.ParseException e) { - context.illegal("Failed to parse routing selector for search cluster '" + sc.getClusterName() + "'", e); + throw new IllegalArgumentException("Failed to parse routing selector for search cluster '" + + sc.getClusterName() + "'", e); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingValidator.java index 87b5e8c8fc4..25cc2596fb2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/RoutingValidator.java @@ -1,25 +1,27 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import java.util.List; /** * Validates routing + * */ -public class RoutingValidator implements Validator { +public class RoutingValidator extends Validator { @Override - public void validate(Context context) { - List<String> errors = context.model().getRouting().getErrors(); + public void validate(VespaModel model, DeployState deployState) { + List<String> errors = model.getRouting().getErrors(); if (!errors.isEmpty()) { StringBuilder msg = new StringBuilder(); msg.append("The routing specification contains ").append(errors.size()).append(" error(s):\n"); for (int i = 0, len = errors.size(); i < len; ++i) { msg.append(i + 1).append(". ").append(errors.get(i)).append("\n"); } - context.illegal(msg.toString()); + throw new IllegalArgumentException(msg.toString()); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SchemasDirValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SchemasDirValidator.java index b9278dbc08d..6827cba4030 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SchemasDirValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SchemasDirValidator.java @@ -3,7 +3,8 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.ApplicationPackage; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import java.util.logging.Level; @@ -12,14 +13,17 @@ import java.util.logging.Level; * * @author hmusum */ -public class SchemasDirValidator implements Validator { +public class SchemasDirValidator extends Validator { + + public SchemasDirValidator() { + } @Override - public void validate(Context context) { - ApplicationPackage app = context.deployState().getApplicationPackage(); + public void validate(VespaModel model, DeployState deployState) { + ApplicationPackage app = deployState.getApplicationPackage(); ApplicationFile sdDir = app.getFile(ApplicationPackage.SEARCH_DEFINITIONS_DIR); if (sdDir.exists() && sdDir.isDirectory()) - context.deployState().getDeployLogger().logApplicationPackage( + deployState.getDeployLogger().logApplicationPackage( Level.WARNING, "Directory " + ApplicationPackage.SEARCH_DEFINITIONS_DIR.getRelative() + "/ should not be used for schemas, use " + ApplicationPackage.SCHEMAS_DIR.getRelative() + "/ instead"); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SearchDataTypeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SearchDataTypeValidator.java index 6e21adc4fe4..51f58ea5f88 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SearchDataTypeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SearchDataTypeValidator.java @@ -1,20 +1,21 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.document.ArrayDataType; import com.yahoo.document.CollectionDataType; import com.yahoo.document.DataType; import com.yahoo.document.Field; import com.yahoo.document.MapDataType; +import com.yahoo.documentmodel.NewDocumentReferenceDataType; import com.yahoo.document.StructDataType; import com.yahoo.document.TensorDataType; import com.yahoo.document.WeightedSetDataType; -import com.yahoo.documentmodel.NewDocumentReferenceDataType; import com.yahoo.schema.Schema; import com.yahoo.schema.derived.SchemaInfo; import com.yahoo.schema.document.SDDocumentType; import com.yahoo.schema.document.SDField; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.SearchCluster; import java.util.List; @@ -25,11 +26,11 @@ import java.util.List; * * @author Simon Thoresen Hult */ -public class SearchDataTypeValidator implements Validator { +public class SearchDataTypeValidator extends Validator { @Override - public void validate(Context context) { - List<SearchCluster> clusters = context.model().getSearchClusters(); + public void validate(VespaModel model, DeployState deployState) { + List<SearchCluster> clusters = model.getSearchClusters(); for (SearchCluster cluster : clusters) { if (cluster.isStreaming()) { continue; @@ -39,22 +40,22 @@ public class SearchDataTypeValidator implements Validator { if (docType == null) { continue; } - validateDocument(context, cluster, spec.fullSchema(), docType); + validateDocument(cluster, spec.fullSchema(), docType); } } } - private void validateDocument(Context context, SearchCluster cluster, Schema schema, SDDocumentType doc) { + private void validateDocument(SearchCluster cluster, Schema schema, SDDocumentType doc) { for (SDDocumentType child : doc.getTypes()) { - validateDocument(context, cluster, schema, child); + validateDocument(cluster, schema, child); } for (Field field : doc.fieldSet()) { DataType fieldType = field.getDataType(); - disallowIndexingOfMaps(context, cluster, schema, field); + disallowIndexingOfMaps(cluster, schema, field); if ( ! isSupportedInSearchClusters(fieldType)) { - context.illegal("Field type '" + fieldType.getName() + "' is illegal for search " + - "clusters (field '" + field.getName() + "' in schema '" + - schema.getName() + "' for cluster '" + cluster.getClusterName() + "')."); + throw new IllegalArgumentException("Field type '" + fieldType.getName() + "' is illegal for search " + + "clusters (field '" + field.getName() + "' in schema '" + + schema.getName() + "' for cluster '" + cluster.getClusterName() + "')."); } } } @@ -84,13 +85,12 @@ public class SearchDataTypeValidator implements Validator { } } - private void disallowIndexingOfMaps(Context context, SearchCluster cluster, Schema schema, Field field) { + private void disallowIndexingOfMaps(SearchCluster cluster, Schema schema, Field field) { DataType fieldType = field.getDataType(); if ((fieldType instanceof MapDataType) && (((SDField) field).doesIndexing())) { - context.illegal("Field type '" + fieldType.getName() + "' cannot be indexed for search " + - "clusters (field '" + field.getName() + "' in definition '" + - schema.getName() + "' for cluster '" + cluster.getClusterName() + "')."); + throw new IllegalArgumentException("Field type '" + fieldType.getName() + "' cannot be indexed for search " + + "clusters (field '" + field.getName() + "' in definition '" + + schema.getName() + "' for cluster '" + cluster.getClusterName() + "')."); } } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SecretStoreValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SecretStoreValidator.java index afa29533b93..9c87415395b 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SecretStoreValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/SecretStoreValidator.java @@ -2,7 +2,8 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.model.ConfigModelContext.ApplicationType; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.IdentityProvider; @@ -13,18 +14,18 @@ import com.yahoo.vespa.model.container.component.Component; * * @author gjoranv */ -public class SecretStoreValidator implements Validator { +public class SecretStoreValidator extends Validator { @Override - public void validate(Context context) { - if (! context.deployState().isHosted()) return; - if (context.model().getAdmin().getApplicationType() != ApplicationType.DEFAULT) return; + public void validate(VespaModel model, DeployState deployState) { + if (! deployState.isHosted()) return; + if (model.getAdmin().getApplicationType() != ApplicationType.DEFAULT) return; - for (ContainerCluster<?> cluster : context.model().getContainerClusters().values()) { + for (ContainerCluster cluster : model.getContainerClusters().values()) { if (cluster.getSecretStore().isPresent() && ! hasIdentityProvider(cluster)) - context.illegal(String.format( - "Container cluster '%s' uses a secret store, so an Athenz domain and an Athenz service" + - " must be declared in deployment.xml.", cluster.getName())); + throw new IllegalArgumentException(String.format( + "Container cluster '%s' uses a secret store, so an Athenz domain and an Athenz service" + + " must be declared in deployment.xml.", cluster.getName())); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java index c0ad55fc8f4..42dc4df0d43 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/StreamingValidator.java @@ -2,6 +2,7 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.DeployLogger; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.document.DataType; import com.yahoo.document.NumericDataType; import com.yahoo.document.TensorDataType; @@ -9,7 +10,7 @@ import com.yahoo.documentmodel.NewDocumentReferenceDataType; import com.yahoo.schema.document.Attribute; import com.yahoo.schema.document.ImmutableSDField; import com.yahoo.schema.document.MatchType; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.search.SearchCluster; import com.yahoo.vespa.model.search.StreamingSearchCluster; @@ -19,18 +20,18 @@ import java.util.logging.Level; /** * Validates streaming mode */ -public class StreamingValidator implements Validator { +public class StreamingValidator extends Validator { @Override - public void validate(Context context) { - List<SearchCluster> searchClusters = context.model().getSearchClusters(); + public void validate(VespaModel model, DeployState deployState) { + List<SearchCluster> searchClusters = model.getSearchClusters(); for (SearchCluster cluster : searchClusters) { if ( ! cluster.isStreaming()) continue; var streamingCluster = (StreamingSearchCluster)cluster; - warnStreamingAttributes(streamingCluster, context.deployState().getDeployLogger()); - warnStreamingGramMatching(streamingCluster, context.deployState().getDeployLogger()); - failStreamingDocumentReferences(context, streamingCluster); + warnStreamingAttributes(streamingCluster, deployState.getDeployLogger()); + warnStreamingGramMatching(streamingCluster, deployState.getDeployLogger()); + failStreamingDocumentReferences(streamingCluster); } } @@ -80,14 +81,14 @@ public class StreamingValidator implements Validator { "': 'attribute' has same match semantics as 'index'."); } - private static void failStreamingDocumentReferences(Context context, StreamingSearchCluster sc) { + private static void failStreamingDocumentReferences(StreamingSearchCluster sc) { for (Attribute attribute : sc.derived().getAttributeFields().attributes()) { DataType dataType = attribute.getDataType(); if (dataType instanceof NewDocumentReferenceDataType) { String errorMessage = String.format("For streaming search cluster '%s': Attribute '%s' has type '%s'. " + "Document references and imported fields are not allowed in streaming search.", sc.getClusterName(), attribute.getName(), dataType.getName()); - context.illegal(errorMessage); + throw new IllegalArgumentException(errorMessage); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java index 3f519088a56..ed53aa581b1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UriBindingsValidator.java @@ -1,7 +1,8 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.container.component.BindingPattern; import com.yahoo.vespa.model.container.component.Handler; @@ -17,70 +18,70 @@ import java.util.logging.Level; * * @author bjorncs */ -class UriBindingsValidator implements Validator { +class UriBindingsValidator extends Validator { @Override - public void validate(Context context) { - for (ApplicationContainerCluster cluster : context.model().getContainerClusters().values()) { + public void validate(VespaModel model, DeployState deployState) { + for (ApplicationContainerCluster cluster : model.getContainerClusters().values()) { for (Handler handler : cluster.getHandlers()) { for (BindingPattern binding : handler.getServerBindings()) { - validateUserBinding(binding, context); + validateUserBinding(binding, model, deployState); } } Http http = cluster.getHttp(); if (http != null) { for (FilterBinding binding : cluster.getHttp().getBindings()) { - validateUserBinding(binding.binding(), context); + validateUserBinding(binding.binding(), model, deployState); } } } } - private static void validateUserBinding(BindingPattern binding, Context context) { - validateScheme(binding, context); - if (isHostedApplication(context)) { - validateHostedApplicationUserBinding(binding, context); + private static void validateUserBinding(BindingPattern binding, VespaModel model, DeployState deployState) { + validateScheme(binding, deployState); + if (isHostedApplication(model, deployState)) { + validateHostedApplicationUserBinding(binding, deployState); } } - private static void validateScheme(BindingPattern binding, Context context) { + private static void validateScheme(BindingPattern binding, DeployState deployState) { if (binding.scheme().equals("https")) { String message = createErrorMessage( binding, "'https' bindings are deprecated, use 'http' instead to bind to both http and https traffic."); - context.deployState().getDeployLogger().logApplicationPackage(Level.WARNING, message); + deployState.getDeployLogger().logApplicationPackage(Level.WARNING, message); } } - private static void validateHostedApplicationUserBinding(BindingPattern binding, Context context) { + private static void validateHostedApplicationUserBinding(BindingPattern binding, DeployState deployState) { // only perform these validation for used-generated bindings // bindings produced by the hosted config model amender will violate some of the rules below if (binding instanceof SystemBindingPattern) return; // Allow binding to port if we are restricting data plane bindings if (!binding.matchesAnyPort()) { - logOrThrow(createErrorMessage(binding, "binding with port is not allowed"), context); + logOrThrow(createErrorMessage(binding, "binding with port is not allowed"), deployState); } if (!binding.host().equals(BindingPattern.WILDCARD_PATTERN)) { - logOrThrow(createErrorMessage(binding, "only binding with wildcard ('*') for hostname is allowed"), context); + logOrThrow(createErrorMessage(binding, "only binding with wildcard ('*') for hostname is allowed"), deployState); } if (!binding.scheme().equals("http") && !binding.scheme().equals("https")) { - logOrThrow(createErrorMessage(binding, "only 'http' is allowed as scheme"), context); + logOrThrow(createErrorMessage(binding, "only 'http' is allowed as scheme"), deployState); } } /* * Logs to deploy logger in non-public systems, throw otherwise */ - private static void logOrThrow(String message, Context context) { - if (context.deployState().zone().system().isPublic()) { - context.illegal(message); + private static void logOrThrow(String message, DeployState deployState) { + if (deployState.zone().system().isPublic()) { + throw new IllegalArgumentException(message); } else { - context.deployState().getDeployLogger().log(Level.WARNING, message); + deployState.getDeployLogger().log(Level.WARNING, message); } } - private static boolean isHostedApplication(Context context) { - return context.deployState().isHostedTenantApplication(context.model().getAdmin().getApplicationType()); + private static boolean isHostedApplication(VespaModel model, DeployState deployState) { + return deployState.isHostedTenantApplication(model.getAdmin().getApplicationType()); } private static String createErrorMessage(BindingPattern binding, String message) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java index 5619e99308d..5db88dbd0db 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java @@ -1,8 +1,8 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.Context; import com.yahoo.vespa.model.container.ApplicationContainerCluster; /** @@ -10,15 +10,15 @@ import com.yahoo.vespa.model.container.ApplicationContainerCluster; * * @author hmusum */ -public class UrlConfigValidator implements Validator { +public class UrlConfigValidator extends Validator { @Override - public void validate(Context context) { - if (! context.deployState().isHostedTenantApplication(context.model().getAdmin().getApplicationType())) return; + public void validate(VespaModel model, DeployState state) { + if (! state.isHostedTenantApplication(model.getAdmin().getApplicationType())) return; - context.model().getContainerClusters().forEach((__, cluster) -> { - var isExclusive = hasExclusiveNodes(context.model(), cluster); - validateS3UlsInConfig(context, cluster, isExclusive); + model.getContainerClusters().forEach((__, cluster) -> { + var isExclusive = hasExclusiveNodes(model, cluster); + validateS3UlsInConfig(state, cluster, isExclusive); }); } @@ -30,15 +30,15 @@ public class UrlConfigValidator implements Validator { .anyMatch(membership -> membership.cluster().isExclusive()); } - private static void validateS3UlsInConfig(Context context, ApplicationContainerCluster cluster, boolean isExclusive) { + private static void validateS3UlsInConfig(DeployState state, ApplicationContainerCluster cluster, boolean isExclusive) { if (hasS3UrlInConfig(cluster)) { // TODO: Would be even better if we could add which config/field the url is set for in the error message String message = "Found s3:// urls in config for container cluster " + cluster.getName(); - if ( ! context.deployState().zone().system().isPublic()) - context.illegal(message + ". This is only supported in public systems"); + if ( ! state.zone().system().isPublic()) + throw new IllegalArgumentException(message + ". This is only supported in public systems"); else if ( ! isExclusive) - context.illegal(message + ". Nodes in the cluster need to be 'exclusive'," + - " see https://cloud.vespa.ai/en/reference/services#nodes"); + throw new IllegalArgumentException(message + ". Nodes in the cluster need to be 'exclusive'," + + " see https://cloud.vespa.ai/en/reference/services#nodes"); } } 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 edaaf7b206d..0f7a415c33a 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 @@ -10,6 +10,7 @@ import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.change.CertificateRemovalChangeValidator; +import com.yahoo.vespa.model.application.validation.change.ChangeValidator; import com.yahoo.vespa.model.application.validation.change.ConfigValueChangeValidator; import com.yahoo.vespa.model.application.validation.change.ContainerRestartValidator; import com.yahoo.vespa.model.application.validation.change.ContentClusterRemovalValidator; @@ -31,7 +32,6 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Set; import java.util.logging.Level; import java.util.stream.Collectors; @@ -65,16 +65,14 @@ public class Validation { validateModel(validationParameters, execution); - for (Validator validator : additionalValidators) { - validator.validate(execution); - } + additionalValidators.forEach(execution::run); if (deployState.getProperties().isFirstTimeDeployment()) { validateFirstTimeDeployment(execution); } - else if (deployState.getPreviousModel().isPresent() && (deployState.getPreviousModel().get() instanceof VespaModel)) { - validateChanges(execution); - // TODO: Why is this done here? It won't be done on more than one config server? + else if (deployState.getPreviousModel().isPresent() && (deployState.getPreviousModel().get() instanceof VespaModel vespaModel)) { + validateChanges(vespaModel, execution); + // TODO: Why is this done here? It won't be done on more than one config server? deferConfigChangesForClustersToBeRestarted(execution.actions, model); } @@ -83,58 +81,57 @@ public class Validation { } private static void validateRouting(Execution execution) { - new RoutingValidator().validate(execution); - new RoutingSelectorValidator().validate(execution); + execution.run(new RoutingValidator()); + execution.run(new RoutingSelectorValidator()); } private static void validateModel(ValidationParameters validationParameters, Execution execution) { - new SchemasDirValidator().validate(execution); - new BundleValidator().validate(execution); - new PublicApiBundleValidator().validate(execution); - new SearchDataTypeValidator().validate(execution); - new ComplexFieldsWithStructFieldAttributesValidator().validate(execution); - new ComplexFieldsWithStructFieldIndexesValidator().validate(execution); - new StreamingValidator().validate(execution); - new RankSetupValidator(validationParameters.ignoreValidationErrors()).validate(execution); - new NoPrefixForIndexes().validate(execution); - new ContainerInCloudValidator().validate(execution); - new DeploymentSpecValidator().validate(execution); - new ValidationOverridesValidator().validate(execution); - new ConstantValidator().validate(execution); - new SecretStoreValidator().validate(execution); - new AccessControlFilterValidator().validate(execution); - new QuotaValidator().validate(execution); - new UriBindingsValidator().validate(execution); - new CloudDataPlaneFilterValidator().validate(execution); - new AccessControlFilterExcludeValidator().validate(execution); - new CloudUserFilterValidator().validate(execution); - new CloudHttpConnectorValidator().validate(execution); - new UrlConfigValidator().validate(execution); - new JvmHeapSizeValidator().validate(execution); - new InfrastructureDeploymentValidator().validate(execution); - new EndpointCertificateSecretsValidator().validate(execution); + execution.run(new SchemasDirValidator()); + execution.run(new BundleValidator()); + execution.run(new PublicApiBundleValidator()); + execution.run(new SearchDataTypeValidator()); + execution.run(new ComplexFieldsWithStructFieldAttributesValidator()); + execution.run(new ComplexFieldsWithStructFieldIndexesValidator()); + execution.run(new StreamingValidator()); + execution.run(new RankSetupValidator(validationParameters.ignoreValidationErrors())); + execution.run(new NoPrefixForIndexes()); + execution.run(new ContainerInCloudValidator()); + execution.run(new DeploymentSpecValidator()); + execution.run(new ValidationOverridesValidator()); + execution.run(new ConstantValidator()); + execution.run(new SecretStoreValidator()); + execution.run(new EndpointCertificateSecretsValidator()); + execution.run(new AccessControlFilterValidator()); + execution.run(new QuotaValidator()); + execution.run(new UriBindingsValidator()); + execution.run(new CloudDataPlaneFilterValidator()); + execution.run(new AccessControlFilterExcludeValidator()); + execution.run(new CloudUserFilterValidator()); + execution.run(new CloudHttpConnectorValidator()); + execution.run(new UrlConfigValidator()); + execution.run(new JvmHeapSizeValidator()); } private static void validateFirstTimeDeployment(Execution execution) { - new RedundancyValidator().validate((Context) execution); + execution.run(new RedundancyValidator()); } - private static void validateChanges(Execution execution) { - new IndexingModeChangeValidator().validate(execution); - new GlobalDocumentChangeValidator().validate(execution); - new IndexedSearchClusterChangeValidator().validate(execution); - new StreamingSearchClusterChangeValidator().validate(execution); - new ConfigValueChangeValidator().validate(execution); - new StartupCommandChangeValidator().validate(execution); - new ContentTypeRemovalValidator().validate(execution); - new ContentClusterRemovalValidator().validate(execution); - new ResourcesReductionValidator().validate(execution); - new ContainerRestartValidator().validate(execution); - new NodeResourceChangeValidator().validate(execution); - new RedundancyIncreaseValidator().validate(execution); - new CertificateRemovalChangeValidator().validate(execution); - new RedundancyValidator().validate(execution); - new RestartOnDeployForOnnxModelChangesValidator().validate(execution); + private static void validateChanges(VespaModel currentModel, Execution execution) { + execution.run(new IndexingModeChangeValidator(), currentModel); + execution.run(new GlobalDocumentChangeValidator(), currentModel); + execution.run(new IndexedSearchClusterChangeValidator(), currentModel); + execution.run(new StreamingSearchClusterChangeValidator(), currentModel); + execution.run(new ConfigValueChangeValidator(), currentModel); + execution.run(new StartupCommandChangeValidator(), currentModel); + execution.run(new ContentTypeRemovalValidator(), currentModel); + execution.run(new ContentClusterRemovalValidator(), currentModel); + execution.run(new ResourcesReductionValidator(), currentModel); + execution.run(new ContainerRestartValidator(), currentModel); + execution.run(new NodeResourceChangeValidator(), currentModel); + execution.run(new RedundancyIncreaseValidator(), currentModel); + execution.run(new CertificateRemovalChangeValidator(), currentModel); + execution.run(new RedundancyValidator(), currentModel); + execution.run(new RestartOnDeployForOnnxModelChangesValidator(), currentModel); } private static void deferConfigChangesForClustersToBeRestarted(List<ConfigChangeAction> actions, VespaModel model) { @@ -153,101 +150,59 @@ public class Validation { } } - public interface Context { - /** Auxiliary deploy state of the application. */ - DeployState deployState(); - /** The model to validate. */ - VespaModel model(); - /** Report a failed validation which cannot be overridden; this results in an {@link IllegalArgumentException}. */ - default void illegal(String message) { illegal(message, null); } - /** Report a failed validation which cannot be overridden; this results in an {@link IllegalArgumentException}. */ - void illegal(String message, Throwable cause); - /** Report a failed validation which can be overridden; this results in a {@link ValidationException}. */ - void invalid(ValidationId id, String message); - } - - public interface ChangeContext extends Context { - /** The previous model, if any. */ - VespaModel previousModel(); - /** - * Report an action the user must take to change to the new configuration. - * If the action has a {@link ValidationId}, {@link #invalid} is also called for this id, and the action's message. - */ - void require(ConfigChangeAction action); - } - static class Execution implements ChangeContext { + private static class Execution { - private final List<String> errors = new ArrayList<>(); private final Map<ValidationId, List<String>> failures = new LinkedHashMap<>(); private final VespaModel model; private final DeployState deployState; private final List<ConfigChangeAction> actions = new ArrayList<>(); - Execution(VespaModel model, DeployState deployState) { + private Execution(VespaModel model, DeployState deployState) { this.model = model; this.deployState = deployState; } - void throwIfFailed() { - Optional<ValidationException> invalidException = deployState.validationOverrides().invalidException(failures, deployState.now()); - if (invalidException.isPresent() && deployState.isHosted() && deployState.zone().environment().isManuallyDeployed()) { - deployState.getDeployLogger().logApplicationPackage(Level.WARNING, - "Auto-overriding validation which would be disallowed in production: " + - Exceptions.toMessageString(invalidException.get())); - invalidException = Optional.empty(); + private void run(Validator validator) { + try { + validator.validate(model, deployState); } - - if ( ! errors.isEmpty()) { - String illegalMessage = errors.size() == 1 ? errors.get(0) - : "multiple errors:\n\t" + String.join("\n\t", errors); - if (invalidException.isPresent()) - illegalMessage += "\n" + invalidException.get().getMessage(); - - throw new IllegalArgumentException(illegalMessage); + catch (ValidationException e) { + e.messagesById().forEach((id, messages) -> failures.computeIfAbsent(id, __ -> new ArrayList<>()).addAll(messages)); } - - invalidException.ifPresent(e -> { throw e; }); - } - - List<ConfigChangeAction> actions() { - return actions; } - List<String> errors() { - return errors; - } - - @Override - public DeployState deployState() { - return deployState; - } - - @Override - public VespaModel model() { - return model; - } - - @Override - public VespaModel previousModel() { - return (VespaModel) deployState.getPreviousModel().get(); - } - - @Override - public void require(ConfigChangeAction action) { - actions.add(action); - action.validationId().ifPresent(id -> invalid(id, action.getMessage())); - } - - @Override - public void illegal(String message, Throwable cause) { - if (cause != null) message += ": " + Exceptions.toMessageString(cause); - errors.add(message); + private void run(ChangeValidator validator, VespaModel previousModel) { + try { + // Some change validators throw, while some return a list of changes that may again be disallowed. + for (ConfigChangeAction action : validator.validate(previousModel, model, deployState)) { + actions.add(action); + if (action.validationId().isPresent()) run(new Validator() { // Changes without a validation ID are always allowed. + @Override public void validate(VespaModel model, DeployState deployState) { + deployState.validationOverrides().invalid(action.validationId().get(), action.getMessage(), deployState.now()); + } + }); + } + } + catch (ValidationException e) { + e.messagesById().forEach((id, messages) -> failures.computeIfAbsent(id, __ -> new ArrayList<>()).addAll(messages)); + } } - @Override - public void invalid(ValidationId id, String message) { - failures.computeIfAbsent(id, __ -> new ArrayList<>()).add(message); + private void throwIfFailed() { + try { + if (failures.size() == 1 && failures.values().iterator().next().size() == 1) // Retain single-form exception message when possible. + deployState.validationOverrides().invalid(failures.keySet().iterator().next(), failures.values().iterator().next().get(0), deployState.now()); + else + deployState.validationOverrides().invalid(failures, deployState.now()); + } + catch (ValidationException e) { + if (deployState.isHosted() && deployState.zone().environment().isManuallyDeployed()) + deployState.getDeployLogger().logApplicationPackage(Level.WARNING, + "Auto-overriding validation which would be disallowed in production: " + + Exceptions.toMessageString(e)); + else throw e; + } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidator.java index c56208e5e35..38d94e55b8e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ValidationOverridesValidator.java @@ -2,7 +2,8 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.ValidationOverrides; -import com.yahoo.vespa.model.application.validation.Validation.Context; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import java.io.Reader; import java.util.Optional; @@ -13,15 +14,15 @@ import java.util.Optional; * * @author hmusum */ -public class ValidationOverridesValidator implements Validator { +public class ValidationOverridesValidator extends Validator { @Override - public void validate(Context context) { - Optional<Reader> overrides = context.deployState().getApplicationPackage().getValidationOverrides(); + public void validate(VespaModel model, DeployState deployState) { + Optional<Reader> overrides = deployState.getApplicationPackage().getValidationOverrides(); if (overrides.isEmpty()) return; ValidationOverrides validationOverrides = ValidationOverrides.fromXml(overrides.get()); - validationOverrides.validate(context.deployState().now(), context::illegal); + validationOverrides.validate(deployState.now()); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validator.java index fb1bf0b0ed8..c678938b5d9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validator.java @@ -1,17 +1,22 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; + /** * Abstract superclass of all application package validators. * * @author hmusum */ -public interface Validator { +public abstract class Validator { /** - * Validates the input Vespa model; illegal configuration should be reported through the context, - * while other problems (system error, insufficient quota, etc.) should be thrown. + * Validates the input vespamodel + * + * @param model a VespaModel object + * @param deployState the {@link DeployState} built from building the model */ - void validate(Validation.Context context); + public abstract void validate(VespaModel model, DeployState deployState); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java index 97e422c1a6a..c9e1a3bdea7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java @@ -2,13 +2,14 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.http.Client; import java.security.cert.X509Certificate; import java.util.Collection; import java.util.List; -import java.util.function.BiConsumer; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -23,20 +24,22 @@ public class CertificateRemovalChangeValidator implements ChangeValidator { private static final Logger logger = Logger.getLogger(CertificateRemovalChangeValidator.class.getName()); @Override - public void validate(ChangeContext context) { + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { // Skip for tester applications - if (context.previousModel().applicationPackage().getApplicationId().instance().isTester()) return; - context.previousModel().getContainerClusters() + if (current.applicationPackage().getApplicationId().instance().isTester()) return List.of(); + current.getContainerClusters() .forEach((clusterId, currentCluster) -> { - if(context.model().getContainerClusters().containsKey(clusterId)) + if(next.getContainerClusters().containsKey(clusterId)) validateClients(clusterId, currentCluster.getClients(), - context.model().getContainerClusters().get(clusterId).getClients(), - context::invalid); + next.getContainerClusters().get(clusterId).getClients(), + deployState); }); + + return List.of(); } - void validateClients(String clusterId, List<Client> current, List<Client> next, BiConsumer<ValidationId, String> reporter) { + void validateClients(String clusterId, List<Client> current, List<Client> next, DeployState deployState) { List<X509Certificate> currentCertificates = current.stream() .filter(client -> !client.internal()) .map(Client::certificates) @@ -55,11 +58,12 @@ public class CertificateRemovalChangeValidator implements ChangeValidator { List<X509Certificate> missingCerts = currentCertificates.stream().filter(cert -> !nextCertificates.contains(cert)).toList(); if (!missingCerts.isEmpty()) { - reporter.accept(ValidationId.certificateRemoval, - "Data plane certificate(s) from cluster '" + clusterId + "' is removed " + - "(removed certificates: " + missingCerts.stream().map(x509Certificate -> x509Certificate.getSubjectX500Principal().getName()).toList() + ") " + - "This can cause client connection issues."); + deployState.validationOverrides().invalid(ValidationId.certificateRemoval, + "Data plane certificate(s) from cluster '" + clusterId + "' is removed " + + "(removed certificates: " + missingCerts.stream().map(x509Certificate -> x509Certificate.getSubjectX500Principal().getName()).toList() + ") " + + "This can cause client connection issues.", + deployState.now()); } - } + } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java index 03479db88d5..107128fdd89 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ChangeValidator.java @@ -2,9 +2,11 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; -import com.yahoo.vespa.model.application.validation.Validator; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.config.application.api.ValidationOverrides; +import java.time.Instant; import java.util.List; /** @@ -15,9 +17,15 @@ import java.util.List; public interface ChangeValidator { /** - * Validates changes from the previous to the next model. Necessary actions by the user - * should be reported through the context; see {@link Validator} for more details. + * Validates the current active vespa model with the next model. + * Both current and next should be non-null. + * + * @param current the current active model + * @param next the next model we would like to activate + * @return a list of actions specifying what needs to be done in order to activate the new model. + * Return an empty list if nothing needs to be done + * @throws IllegalArgumentException if the change fails validation */ - void validate(ChangeContext context); + List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidator.java index bfd100f40c9..73d6b9509cb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidator.java @@ -5,17 +5,17 @@ import com.yahoo.config.ChangesRequiringRestart; import com.yahoo.config.ConfigInstance; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.Service; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.RestartConfigs; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; import com.yahoo.vespa.model.utils.internal.ReflectionUtil; import java.util.Arrays; import java.util.List; import java.util.Optional; -import java.util.function.Consumer; import java.util.logging.Level; import java.util.stream.Stream; @@ -32,13 +32,13 @@ public class ConfigValueChangeValidator implements ChangeValidator { /** Inspects the configuration in the new and old Vespa model to determine which services that require restart */ @Override - public void validate(ChangeContext context) { - findConfigChangesFromModels(context.previousModel(), context.model(), context.deployState().getDeployLogger()).forEach(context::require); + public List<ConfigChangeAction> validate(VespaModel currentModel, VespaModel nextModel, DeployState deployState) { + return findConfigChangesFromModels(currentModel, nextModel, deployState.getDeployLogger()).toList(); } - Stream<ConfigChangeAction> findConfigChangesFromModels(AbstractConfigProducerRoot currentModel, - AbstractConfigProducerRoot nextModel, - DeployLogger logger) { + public Stream<ConfigChangeAction> findConfigChangesFromModels(AbstractConfigProducerRoot currentModel, + AbstractConfigProducerRoot nextModel, + DeployLogger logger) { return nextModel.getDescendantServices().stream() .map(service -> findConfigChangeActionForService(service, currentModel, nextModel, logger)) .filter(Optional::isPresent) @@ -136,8 +136,8 @@ public class ConfigValueChangeValidator implements ChangeValidator { return Optional.of(ReflectionUtil.getChangesRequiringRestart(currentConfig.get(), nextConfig.get())); } - private static boolean hasConfigFieldsFlaggedWithRestart(Class<? extends ConfigInstance> configClass, - Class<? extends Service> serviceClass) { + private static boolean hasConfigFieldsFlaggedWithRestart( + Class<? extends ConfigInstance> configClass, Class<? extends Service> serviceClass) { if (!ReflectionUtil.hasRestartMethods(configClass)) { throw new IllegalStateException(String.format( "%s is listed as restart config for %s but does not contain any restart inspection methods.", diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java index 58b849d90e6..2a4e8a2a2a6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidator.java @@ -2,16 +2,19 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostSpec; import com.yahoo.container.QrConfig; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; import com.yahoo.vespa.model.container.ApplicationContainer; import com.yahoo.vespa.model.container.Container; import com.yahoo.vespa.model.container.ContainerCluster; +import java.util.ArrayList; +import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import static java.util.stream.Collectors.toUnmodifiableSet; @@ -23,16 +26,18 @@ import static java.util.stream.Collectors.toUnmodifiableSet; public class ContainerRestartValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - boolean nodesUnchanged = context.previousModel().allocatedHosts().equals(context.model().allocatedHosts()); - boolean contentUnchanged = contentHostsOf(context.previousModel()).equals(contentHostsOf(context.model())); - for (ContainerCluster<ApplicationContainer> cluster : context.model().getContainerClusters().values()) { - cluster.getContainers().stream() - .filter(container -> isExistingContainer(container, context.previousModel())) - .filter(container -> shouldContainerRestartOnDeploy(container, context.model())) - .map(container -> createConfigChangeAction(cluster.id(), container, context.model(), nodesUnchanged, contentUnchanged)) - .forEach(context::require); + public List<ConfigChangeAction> validate(VespaModel currentModel, VespaModel nextModel, DeployState deployState) { + boolean nodesUnchanged = currentModel.allocatedHosts().equals(nextModel.allocatedHosts()); + boolean contentUnchanged = contentHostsOf(currentModel).equals(contentHostsOf(nextModel)); + List<ConfigChangeAction> actions = new ArrayList<>(); + for (ContainerCluster<ApplicationContainer> cluster : nextModel.getContainerClusters().values()) { + actions.addAll(cluster.getContainers().stream() + .filter(container -> isExistingContainer(container, currentModel)) + .filter(container -> shouldContainerRestartOnDeploy(container, nextModel)) + .map(container -> createConfigChangeAction(cluster.id(), container, nextModel, nodesUnchanged, contentUnchanged)) + .toList()); } + return actions; } private Set<HostSpec> contentHostsOf(VespaModel model) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java index 3a0aa0afd76..fb48ec68c12 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentClusterRemovalValidator.java @@ -1,12 +1,18 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change; +import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.config.application.api.ValidationId; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; import com.yahoo.vespa.model.container.ApplicationContainer; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.content.cluster.ContentCluster; +import java.util.ArrayList; +import java.util.List; + /** * Checks that this does not remove a content cluster (or changes its id) * as that means losing all data of that cluster. @@ -16,22 +22,24 @@ import com.yahoo.vespa.model.content.cluster.ContentCluster; public class ContentClusterRemovalValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (String currentClusterId : context.previousModel().getContentClusters().keySet()) { - ContentCluster nextCluster = context.model().getContentClusters().get(currentClusterId); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + List<ConfigChangeAction> actions = new ArrayList<>(); + for (String currentClusterId : current.getContentClusters().keySet()) { + ContentCluster nextCluster = next.getContentClusters().get(currentClusterId); if (nextCluster == null) { - context.invalid(ValidationId.contentClusterRemoval, - "Content cluster '" + currentClusterId + "' is removed. " + - "This will cause loss of all data in this cluster"); + deployState.validationOverrides().invalid(ValidationId.contentClusterRemoval, + "Content cluster '" + currentClusterId + "' is removed. " + + "This will cause loss of all data in this cluster", + deployState.now()); // If we allow the removal, we must restart all containers to ensure mbus is OK. - for (ApplicationContainerCluster cluster : context.model().getContainerClusters().values()) { - context.require(new VespaRestartAction(cluster.id(), - "Content cluster '" + currentClusterId + "' has been removed", - cluster.getContainers().stream().map(ApplicationContainer::getServiceInfo).toList())); + for (ApplicationContainerCluster cluster : next.getContainerClusters().values()) { + actions.add(new VespaRestartAction(cluster.id(), + "Content cluster '" + currentClusterId + "' has been removed", + cluster.getContainers().stream().map(ApplicationContainer::getServiceInfo).toList())); } } } + return actions; } - } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java index 34e5a4b42a5..fec08f90b1e 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ContentTypeRemovalValidator.java @@ -2,10 +2,14 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; +import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.documentmodel.NewDocumentType; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.content.cluster.ContentCluster; +import java.util.List; + /** * Checks that this does not remove a data type in a cluster, as that causes deletion * of all data of that type. @@ -15,20 +19,22 @@ import com.yahoo.vespa.model.content.cluster.ContentCluster; public class ContentTypeRemovalValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (ContentCluster currentCluster : context.previousModel().getContentClusters().values()) { - ContentCluster nextCluster = context.model().getContentClusters().get(currentCluster.getSubId()); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + for (ContentCluster currentCluster : current.getContentClusters().values()) { + ContentCluster nextCluster = next.getContentClusters().get(currentCluster.getSubId()); if (nextCluster == null) continue; // validated elsewhere for (NewDocumentType type : currentCluster.getDocumentDefinitions().values()) { if ( ! nextCluster.getDocumentDefinitions().containsKey(type.getName())) { - context.invalid(ValidationId.contentTypeRemoval, - "Schema '" + type.getName() + "' is removed " + - "in content cluster '" + currentCluster.getName() + "'. " + - "This will cause loss of all data in this schema"); + deployState.validationOverrides().invalid(ValidationId.contentTypeRemoval, + "Schema '" + type.getName() + "' is removed " + + "in content cluster '" + currentCluster.getName() + "'. " + + "This will cause loss of all data in this schema", + deployState.now()); } } } + return List.of(); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java index df8bf0e9b01..0590bb2d1e6 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidator.java @@ -2,30 +2,35 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; -import com.yahoo.config.provision.ClusterSpec; +import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.documentmodel.NewDocumentType; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.content.cluster.ContentCluster; +import java.util.List; import java.util.Map; /** - * Class that adds a validation failure if global attribute changes for a document + * Class that fails via exception if global attribute changes for a document * type in a content cluster unless corresponding override is present. */ public class GlobalDocumentChangeValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (Map.Entry<String, ContentCluster> currentEntry : context.previousModel().getContentClusters().entrySet()) { - ContentCluster nextCluster = context.model().getContentClusters().get(currentEntry.getKey()); + public List<ConfigChangeAction> validate(VespaModel currentModel, VespaModel nextModel, DeployState deployState) { + if (!deployState.validationOverrides().allows(ValidationId.globalDocumentChange.value(), deployState.now())) { + for (Map.Entry<String, ContentCluster> currentEntry : currentModel.getContentClusters().entrySet()) { + ContentCluster nextCluster = nextModel.getContentClusters().get(currentEntry.getKey()); if (nextCluster == null) continue; - validateContentCluster(context, currentEntry.getValue(), nextCluster); + validateContentCluster(currentEntry.getValue(), nextCluster); } + } + return List.of(); } - private void validateContentCluster(ChangeContext context, ContentCluster currentCluster, ContentCluster nextCluster) { + private void validateContentCluster(ContentCluster currentCluster, ContentCluster nextCluster) { String clusterName = currentCluster.getName(); currentCluster.getDocumentDefinitions().forEach((documentTypeName, currentDocumentType) -> { NewDocumentType nextDocumentType = nextCluster.getDocumentDefinitions().get(documentTypeName); @@ -33,12 +38,10 @@ public class GlobalDocumentChangeValidator implements ChangeValidator { boolean currentIsGlobal = currentCluster.isGloballyDistributed(currentDocumentType); boolean nextIsGlobal = nextCluster.isGloballyDistributed(nextDocumentType); if (currentIsGlobal != nextIsGlobal) { - String reason = "Document type %s in cluster %s changed global from %s to %s. ".formatted(documentTypeName, clusterName, currentIsGlobal, nextIsGlobal) + - "To handle this change, first stop services on all content nodes. Then, deploy with validation override. Finally, start services on all content nodes"; - if ( ! context.deployState().validationOverrides().allows(ValidationId.globalDocumentChange, context.deployState().now())) - context.invalid(ValidationId.globalDocumentChange, reason); - else if (context.deployState().isHosted()) - context.require(new VespaRestartAction(ClusterSpec.Id.from(clusterName), reason)); + throw new IllegalStateException(String.format("Document type %s in cluster %s changed global from %s to %s. " + + "Add validation override '%s' to force this change through. " + + "First, stop services on all content nodes. Then, deploy with validation override. Finally, start services on all content nodes.", + documentTypeName, clusterName, currentIsGlobal, nextIsGlobal, ValidationId.globalDocumentChange.value())); } } }); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java index f4477bdb141..cc28be928ec 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexedSearchClusterChangeValidator.java @@ -5,14 +5,15 @@ import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.documentmodel.NewDocumentType; -import com.yahoo.vespa.model.AbstractService; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; +import com.yahoo.config.application.api.ValidationOverrides; import com.yahoo.vespa.model.application.validation.change.search.DocumentDatabaseChangeValidator; import com.yahoo.vespa.model.content.ContentSearchCluster; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.search.DocumentDatabase; import com.yahoo.vespa.model.search.IndexedSearchCluster; +import java.time.Instant; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -27,13 +28,15 @@ import java.util.stream.Collectors; public class IndexedSearchClusterChangeValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (Map.Entry<String, ContentCluster> currentEntry : context.previousModel().getContentClusters().entrySet()) { - ContentCluster nextCluster = context.model().getContentClusters().get(currentEntry.getKey()); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + List<ConfigChangeAction> result = new ArrayList<>(); + for (Map.Entry<String, ContentCluster> currentEntry : current.getContentClusters().entrySet()) { + ContentCluster nextCluster = next.getContentClusters().get(currentEntry.getKey()); if (nextCluster != null && nextCluster.getSearch().hasIndexedCluster()) { - validateContentCluster(currentEntry.getValue(), nextCluster, context.deployState()).forEach(context::require); + result.addAll(validateContentCluster(currentEntry.getValue(), nextCluster, deployState)); } } + return result; } private static List<ConfigChangeAction> validateContentCluster(ContentCluster currentCluster, @@ -85,16 +88,18 @@ public class IndexedSearchClusterChangeValidator implements ChangeValidator { } private static List<ServiceInfo> getSearchNodeServices(IndexedSearchCluster cluster) { - return cluster.getSearchNodes().stream().map(AbstractService::getServiceInfo).toList(); + return cluster.getSearchNodes().stream(). + map(node -> node.getServiceInfo()). + toList(); } private static List<ConfigChangeAction> modifyActions(List<VespaConfigChangeAction> result, List<ServiceInfo> services, String docTypeName) { - return result.stream() - .map(action -> action.modifyAction("Document type '" + docTypeName + "': " + action.getMessage(), - services, docTypeName)) - .collect(Collectors.toList()); + return result.stream(). + map(action -> action.modifyAction("Document type '" + docTypeName + "': " + action.getMessage(), + services, docTypeName)). + collect(Collectors.toList()); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java index 91ada2b602f..9621619f888 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidator.java @@ -4,8 +4,9 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.documentmodel.NewDocumentType; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.content.ContentSearchCluster; import com.yahoo.vespa.model.content.cluster.ContentCluster; import com.yahoo.vespa.model.search.SearchNode; @@ -15,8 +16,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; - -import static java.util.stream.Collectors.toCollection; +import java.util.stream.Collectors; /** * Returns any change to the indexing mode of a cluster. @@ -27,12 +27,14 @@ import static java.util.stream.Collectors.toCollection; public class IndexingModeChangeValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (Map.Entry<String, ContentCluster> currentEntry : context.previousModel().getContentClusters().entrySet()) { - ContentCluster nextCluster = context.model().getContentClusters().get(currentEntry.getKey()); + public List<ConfigChangeAction> validate(VespaModel currentModel, VespaModel nextModel, DeployState deployState) { + List<ConfigChangeAction> actions = new ArrayList<>(); + for (Map.Entry<String, ContentCluster> currentEntry : currentModel.getContentClusters().entrySet()) { + ContentCluster nextCluster = nextModel.getContentClusters().get(currentEntry.getKey()); if (nextCluster == null) continue; - validateContentCluster(currentEntry.getValue(), nextCluster).forEach(context::require); + actions.addAll(validateContentCluster(currentEntry.getValue(), nextCluster)); } + return actions; } private static List<ConfigChangeAction> validateContentCluster(ContentCluster currentCluster, ContentCluster nextCluster) { @@ -86,7 +88,7 @@ public class IndexingModeChangeValidator implements ChangeValidator { private static Set<String> toDocumentTypeNames(List<NewDocumentType> types) { return types.stream() .map(type -> type.getFullName().getName()) - .collect(toCollection(LinkedHashSet::new)); + .collect(Collectors.toCollection(() -> new LinkedHashSet<>())); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java index 0d4776ad00a..0bb30436272 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidator.java @@ -2,14 +2,15 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import com.yahoo.vespa.model.content.cluster.ContentCluster; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -25,14 +26,16 @@ import java.util.stream.Collectors; public class NodeResourceChangeValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (ClusterSpec.Id clusterId : context.previousModel().allClusters()) { - Optional<NodeResources> currentResources = resourcesOf(clusterId, context.previousModel()); - Optional<NodeResources> nextResources = resourcesOf(clusterId, context.model()); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + var restartActions = new ArrayList<ConfigChangeAction>(); + for (ClusterSpec.Id clusterId : current.allClusters()) { + Optional<NodeResources> currentResources = resourcesOf(clusterId, current); + Optional<NodeResources> nextResources = resourcesOf(clusterId, next); if (currentResources.isEmpty() || nextResources.isEmpty()) continue; // new or removed cluster if ( changeRequiresRestart(currentResources.get(), nextResources.get())) - createRestartActionsFor(clusterId, context.previousModel()).forEach(context::require); + restartActions.addAll(createRestartActionsFor(clusterId, current)); } + return restartActions; } private boolean changeRequiresRestart(NodeResources currentResources, NodeResources nextResources) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidator.java index 3fc3eafcc98..54e64d82921 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidator.java @@ -2,8 +2,11 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.content.cluster.ContentCluster; +import java.util.List; /** * Checks that redundancy is not increased (without a validation override), @@ -14,19 +17,21 @@ import com.yahoo.vespa.model.content.cluster.ContentCluster; public class RedundancyIncreaseValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (ContentCluster currentCluster : context.previousModel().getContentClusters().values()) { - ContentCluster nextCluster = context.model().getContentClusters().get(currentCluster.getSubId()); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + for (ContentCluster currentCluster : current.getContentClusters().values()) { + ContentCluster nextCluster = next.getContentClusters().get(currentCluster.getSubId()); if (nextCluster == null) continue; if (redundancyOf(nextCluster) > redundancyOf(currentCluster)) { - context.invalid(ValidationId.redundancyIncrease, - "Increasing redundancy from " + redundancyOf(currentCluster) + " to " + - redundancyOf(nextCluster) + " in '" + currentCluster + ". " + - "This is a safe operation but verify that you have room for a " + - redundancyOf(nextCluster) + "/" + redundancyOf(currentCluster) + "x increase " + - "in content size"); + deployState.validationOverrides().invalid(ValidationId.redundancyIncrease, + "Increasing redundancy from " + redundancyOf(currentCluster) + " to " + + redundancyOf(nextCluster) + " in '" + currentCluster + ". " + + "This is a safe operation but verify that you have room for a " + + redundancyOf(nextCluster) + "/" + redundancyOf(currentCluster) + "x increase " + + "in content size", + deployState.now()); } } + return List.of(); } private int redundancyOf(ContentCluster cluster) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java index 5d7a8779005..a09a730b00c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidator.java @@ -2,11 +2,14 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.ValidationId; +import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.NodeResources; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; + +import java.util.List; /** * Checks that no cluster sizes are reduced too much in one go. @@ -16,26 +19,31 @@ import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; public class ResourcesReductionValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - for (var clusterId : context.previousModel().allClusters()) { - if (context.model().allClusters().contains(clusterId)) - validate(clusterId, context); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + for (var clusterId : current.allClusters()) { + if (next.allClusters().contains(clusterId)) + validate(clusterId, current, next, deployState); } + return List.of(); } - private void validate(ClusterSpec.Id clusterId, ChangeContext context) { - ClusterResources current = clusterResources(clusterId, context.previousModel()); - ClusterResources next = clusterResources(clusterId, context.model()); + private void validate(ClusterSpec.Id clusterId, + VespaModel currentModel, + VespaModel nextModel, + DeployState deployState) { + ClusterResources current = clusterResources(clusterId, currentModel); + ClusterResources next = clusterResources(clusterId, nextModel); if (current == null || next == null) return; // No request recording - test if (current.nodeResources().isUnspecified() || next.nodeResources().isUnspecified()) { // Self-hosted - unspecified resources; compare node count int currentNodes = current.nodes(); int nextNodes = next.nodes(); if (nextNodes < 0.5 * currentNodes && nextNodes != currentNodes - 1) { - context.invalid(ValidationId.resourcesReduction, - "Size reduction in '" + clusterId.value() + "' is too large: " + - "To guard against mistakes, the new max nodes must be at least 50% of the current nodes. " + - "Current nodes: " + currentNodes + ", new nodes: " + nextNodes); + deployState.validationOverrides().invalid(ValidationId.resourcesReduction, + "Size reduction in '" + clusterId.value() + "' is too large: " + + "To guard against mistakes, the new max nodes must be at least 50% of the current nodes. " + + "Current nodes: " + currentNodes + ", new nodes: " + nextNodes, + deployState.now()); } } else { @@ -44,12 +52,13 @@ public class ResourcesReductionValidator implements ChangeValidator { if (nextResources.vcpu() < 0.5 * currentResources.vcpu() || nextResources.memoryGb() < 0.5 * currentResources.memoryGb() || nextResources.diskGb() < 0.5 * currentResources.diskGb()) - context.invalid(ValidationId.resourcesReduction, - "Resource reduction in '" + clusterId.value() + "' is too large: " + - "To guard against mistakes, the new max resources must be at least 50% of the current " + - "max resources in all dimensions. " + - "Current: " + currentResources.withBandwidthGbps(0) + // (don't output bandwidth here) - ", new: " + nextResources.withBandwidthGbps(0)); + deployState.validationOverrides().invalid(ValidationId.resourcesReduction, + "Resource reduction in '" + clusterId.value() + "' is too large: " + + "To guard against mistakes, the new max resources must be at least 50% of the current " + + "max resources in all dimensions. " + + "Current: " + currentResources.withBandwidthGbps(0) + // (don't output bandwidth here) + ", new: " + nextResources.withBandwidthGbps(0), + deployState.now()); } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java index 373bfe24984..e118a2940d7 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidator.java @@ -4,8 +4,9 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.OnnxModelCost; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.vespa.model.Host; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.container.ApplicationContainerCluster; import java.util.ArrayList; @@ -15,10 +16,10 @@ import java.util.Optional; import java.util.Set; import java.util.logging.Logger; -import static com.yahoo.config.model.api.OnnxModelCost.ModelInfo; import static com.yahoo.vespa.model.application.validation.JvmHeapSizeValidator.gbLimit; import static com.yahoo.vespa.model.application.validation.JvmHeapSizeValidator.percentLimit; import static java.util.logging.Level.FINE; +import static com.yahoo.config.model.api.OnnxModelCost.ModelInfo; import static java.util.logging.Level.INFO; /** @@ -33,26 +34,28 @@ public class RestartOnDeployForOnnxModelChangesValidator implements ChangeValida private static final Logger log = Logger.getLogger(RestartOnDeployForOnnxModelChangesValidator.class.getName()); @Override - public void validate(ChangeContext context) { - if ( ! context.deployState().featureFlags().restartOnDeployWhenOnnxModelChanges()) return; + public List<ConfigChangeAction> validate(VespaModel currentModel, VespaModel nextModel, DeployState deployState) { + if ( ! deployState.featureFlags().restartOnDeployWhenOnnxModelChanges()) return List.of(); + List<ConfigChangeAction> actions = new ArrayList<>(); // Compare onnx models used by each cluster and set restart on deploy for cluster if estimated cost, // model hash or model options have changed - for (var cluster : context.model().getContainerClusters().values()) { - var clusterInCurrentModel = context.previousModel().getContainerClusters().get(cluster.getName()); + for (var cluster : nextModel.getContainerClusters().values()) { + var clusterInCurrentModel = currentModel.getContainerClusters().get(cluster.getName()); if (clusterInCurrentModel == null) continue; var currentModels = clusterInCurrentModel.onnxModelCostCalculator().models(); var nextModels = cluster.onnxModelCostCalculator().models(); - if (enoughMemoryToAvoidRestart(clusterInCurrentModel, cluster, context.deployState().getDeployLogger())) + if (enoughMemoryToAvoidRestart(clusterInCurrentModel, cluster, deployState.getDeployLogger())) continue; log.log(FINE, "Validating %s, current Onnx models:%s, next Onnx models:%s" .formatted(cluster, currentModels, nextModels)); - validateModelChanges(cluster, currentModels, nextModels).forEach(context::require); - validateSetOfModels(cluster, currentModels, nextModels).forEach(context::require); + actions.addAll(validateModelChanges(cluster, currentModels, nextModels)); + actions.addAll(validateSetOfModels(cluster, currentModels, nextModels)); } + return actions; } private List<ConfigChangeAction> validateModelChanges(ApplicationContainerCluster cluster, diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StartupCommandChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StartupCommandChangeValidator.java index 0eee74fbe32..aac9ef28cdb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StartupCommandChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StartupCommandChangeValidator.java @@ -2,11 +2,13 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.Service; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; @@ -20,8 +22,8 @@ import java.util.stream.Stream; public class StartupCommandChangeValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - findServicesWithChangedStartupCommand(context.previousModel(), context.model()).forEach(context::require); + public List<ConfigChangeAction> validate(VespaModel currentModel, VespaModel nextModel, DeployState deployState) { + return findServicesWithChangedStartupCommand(currentModel, nextModel).toList(); } public Stream<ConfigChangeAction> findServicesWithChangedStartupCommand(AbstractConfigProducerRoot currentModel, diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java index 3b89467299d..10848947ee1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/StreamingSearchClusterChangeValidator.java @@ -3,12 +3,13 @@ package com.yahoo.vespa.model.application.validation.change; import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.api.ServiceInfo; +import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.documentmodel.NewDocumentType; import com.yahoo.schema.derived.AttributeFields; import com.yahoo.schema.document.Attribute; import com.yahoo.vespa.model.AbstractService; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; +import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.change.search.ChangeMessageBuilder; import com.yahoo.vespa.model.application.validation.change.search.DocumentTypeChangeValidator; import com.yahoo.vespa.model.content.cluster.ContentCluster; @@ -27,18 +28,20 @@ import java.util.stream.Collectors; public class StreamingSearchClusterChangeValidator implements ChangeValidator { @Override - public void validate(ChangeContext context) { - context.previousModel().getContentClusters().forEach((clusterName, currentCluster) -> { - ContentCluster nextCluster = context.model().getContentClusters().get(clusterName); + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + List<ConfigChangeAction> result = new ArrayList<>(); + current.getContentClusters().forEach((clusterName, currentCluster) -> { + ContentCluster nextCluster = next.getContentClusters().get(clusterName); if (nextCluster != null) { List<StreamingSearchCluster> nextStreamingClusters = nextCluster.getSearch().getStreamingClusters(); currentCluster.getSearch().getStreamingClusters().forEach(currentStreamingCluster -> { Optional<StreamingSearchCluster> nextStreamingCluster = findStreamingCluster(currentStreamingCluster.getClusterName(), nextStreamingClusters); - nextStreamingCluster.ifPresent(streamingSearchCluster -> validateStreamingCluster(currentCluster, currentStreamingCluster, - nextCluster, streamingSearchCluster).forEach(context::require)); + nextStreamingCluster.ifPresent(streamingSearchCluster -> result.addAll(validateStreamingCluster(currentCluster, currentStreamingCluster, + nextCluster, streamingSearchCluster))); }); } }); + return result; } private static Optional<StreamingSearchCluster> findStreamingCluster(String clusterName, List<StreamingSearchCluster> clusters) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/first/RedundancyValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/first/RedundancyValidator.java index cf552d05e28..d088c9e67ff 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/first/RedundancyValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/first/RedundancyValidator.java @@ -2,14 +2,14 @@ package com.yahoo.vespa.model.application.validation.first; import com.yahoo.config.application.api.ValidationId; +import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.ChangeContext; -import com.yahoo.vespa.model.application.validation.Validation.Context; import com.yahoo.vespa.model.application.validation.Validator; import com.yahoo.vespa.model.application.validation.change.ChangeValidator; import com.yahoo.vespa.model.content.cluster.ContentCluster; +import java.util.List; import java.util.stream.Stream; /** @@ -17,23 +17,24 @@ import java.util.stream.Stream; * * @author bratseth */ -public class RedundancyValidator implements Validator, ChangeValidator { +public class RedundancyValidator extends Validator implements ChangeValidator { /** Validate on first deployment. */ @Override - public void validate(Context context) { - if ( ! shouldValidate(context.deployState())) return; - clustersWithRedundancyOne(context.model()).forEach(cluster -> invalidRedundancy(cluster, context)); + public void validate(VespaModel model, DeployState deployState) { + if ( ! shouldValidate(deployState)) return; + clustersWithRedundancyOne(model).forEach(cluster -> invalidRedundancy(cluster, deployState)); } /** Validate on change. */ @Override - public void validate(ChangeContext context) { - if ( ! shouldValidate(context.deployState())) return; + public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, DeployState deployState) { + if ( ! shouldValidate(deployState)) return List.of(); - clustersWithRedundancyOne(context.model()) - .filter(cluster -> ! hasRedundancyOne(context.previousModel().getContentClusters().get(cluster.id().value()))) - .forEach(cluster -> invalidRedundancy(cluster, context)); + clustersWithRedundancyOne(next) + .filter(cluster -> ! hasRedundancyOne(current.getContentClusters().get(cluster.id().value()))) + .forEach(cluster -> invalidRedundancy(cluster, deployState)); + return List.of(); } private boolean shouldValidate(DeployState deployState) { @@ -48,11 +49,12 @@ public class RedundancyValidator implements Validator, ChangeValidator { return cluster != null && cluster.getRedundancy().finalRedundancy() == 1 && cluster.getRedundancy().groups() == 1; } - private void invalidRedundancy(ContentCluster cluster, Context context) { - context.invalid(ValidationId.redundancyOne, - cluster + " has redundancy 1, which will cause it to lose data " + - "if a node fails. This requires an override on first deployment " + - "in a production zone"); + private void invalidRedundancy(ContentCluster cluster, DeployState deployState) { + deployState.validationOverrides().invalid(ValidationId.redundancyOne, + cluster + " has redundancy 1, which will cause it to lose data " + + "if a node fails. This requires an override on first deployment " + + "in a production zone", + deployState.now()); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidatorTest.java index 4e88753b732..25053c536da 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterExcludeValidatorTest.java @@ -25,7 +25,6 @@ import java.io.IOException; import java.util.List; import java.util.Set; -import static com.yahoo.vespa.model.application.validation.ValidationTester.expect; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -57,8 +56,9 @@ public class AccessControlFilterExcludeValidatorTest { MapConfigModelRegistry.createFromList(new ModelBuilderAddingAccessControlFilter()), deployState); - expect(new AccessControlFilterExcludeValidator(), model, deployState, - "Application cluster container-cluster-with-access-control excludes paths from access control, this is not allowed and should be removed."); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> new AccessControlFilterExcludeValidator().validate(model, deployState)); + String expectedMessage = "Application cluster container-cluster-with-access-control excludes paths from access control, this is not allowed and should be removed."; + assertEquals(expectedMessage, exception.getMessage()); } @Test @@ -69,8 +69,9 @@ public class AccessControlFilterExcludeValidatorTest { MapConfigModelRegistry.createFromList(new ModelBuilderAddingAccessControlFilter()), deployState); - ValidationTester.validate(new AccessControlFilterExcludeValidator(), model, deployState); - assertTrue(logOutput.toString().contains("Application cluster container-cluster-with-access-control excludes paths from access control, this is not allowed and should be removed.")); + new AccessControlFilterExcludeValidator().validate(model, deployState); + String expectedMessage = "Application cluster container-cluster-with-access-control excludes paths from access control, this is not allowed and should be removed."; + assertTrue(logOutput.toString().contains(expectedMessage)); } @Test @@ -79,7 +80,7 @@ public class AccessControlFilterExcludeValidatorTest { VespaModel model = new VespaModel( MapConfigModelRegistry.createFromList(new ModelBuilderAddingAccessControlFilter()), deployState); - ValidationTester.validate(new AccessControlFilterExcludeValidator(), model, deployState); + new AccessControlFilterExcludeValidator().validate(model, deployState); } @Test @@ -89,7 +90,7 @@ public class AccessControlFilterExcludeValidatorTest { MapConfigModelRegistry.createFromList(new ModelBuilderAddingAccessControlFilter()), deployState); - ValidationTester.validate(new AccessControlFilterExcludeValidator(), model, deployState); + new AccessControlFilterExcludeValidator().validate(model, deployState); } private static DeployState createDeployState(Zone zone, StringBuffer buffer, boolean allowExcludes) { @@ -111,6 +112,6 @@ public class AccessControlFilterExcludeValidatorTest { Cloud.Builder cloudBuilder = Cloud.builder().name(cloudName); if (cloudName == CloudName.AWS) cloudBuilder.account(CloudAccount.from("123456789012")); return new Zone(cloudBuilder.build(), systemName, Environment.prod, RegionName.defaultName()); - } + } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidatorTest.java index 1bc59e118d4..aa42fbbf827 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/AccessControlFilterValidatorTest.java @@ -39,7 +39,7 @@ public class AccessControlFilterValidatorTest { VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); try { - ValidationTester.validate(new AccessControlFilterValidator(), model, deployState); + new AccessControlFilterValidator().validate(model, deployState); fail(); } catch (IllegalArgumentException e) { assertEquals("The 'access-control' feature is not available in open-source Vespa.", e.getMessage()); @@ -53,7 +53,7 @@ public class AccessControlFilterValidatorTest { MapConfigModelRegistry.createFromList(new ModelBuilderAddingAccessControlFilter()), deployState); - ValidationTester.validate(new AccessControlFilterValidator(), model, deployState); + new AccessControlFilterValidator().validate(model, deployState); } private static DeployState createDeployState() { @@ -61,5 +61,4 @@ public class AccessControlFilterValidatorTest { .applicationPackage(new MockApplicationPackage.Builder().withServices(SERVICES_XML).build()) .build(); } - } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java index 1aca0c2fe47..f41cc266db3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/BundleValidatorTest.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.vespa.model.application.validation.AbstractBundleValidator.JarContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -30,7 +29,7 @@ public class BundleValidatorTest { // Valid jar file JarFile ok = createTemporaryJarFile(tempDir, "ok"); BundleValidator bundleValidator = new BundleValidator(); - bundleValidator.validateJarFile(contextOf(DeployState.createTestState()), ok); + bundleValidator.validateJarFile(DeployState.createTestState(), ok); // No manifest validateWithException("nomanifest", "Non-existing or invalid manifest in nomanifest.jar"); @@ -40,7 +39,7 @@ public class BundleValidatorTest { try { JarFile jarFile = createTemporaryJarFile(tempDir, jarName); BundleValidator bundleValidator = new BundleValidator(); - bundleValidator.validateJarFile(contextOf(DeployState.createTestState()), jarFile); + bundleValidator.validateJarFile(DeployState.createTestState(), jarFile); assert (false); } catch (IllegalArgumentException e) { assertEquals(exceptionMessage, e.getMessage()); @@ -53,7 +52,7 @@ public class BundleValidatorTest { DeployState state = createDeployState(buffer); JarFile jarFile = createTemporaryJarFile(tempDir, "snapshot_bundle"); - new BundleValidator().validateJarFile(contextOf(state), jarFile); + new BundleValidator().validateJarFile(state, jarFile); assertTrue(buffer.toString().contains("Deploying snapshot bundle")); } @@ -63,7 +62,7 @@ public class BundleValidatorTest { DeployState state = createDeployState(buffer); BundleValidator validator = new BundleValidator(); JarFile jarFile = createTemporaryJarFile(tempDir, "import-warnings"); - validator.validateJarFile(contextOf(state), jarFile); + validator.validateJarFile(state, jarFile); String output = buffer.toString(); assertTrue(output .contains("JAR file 'import-warnings.jar' imports the packages [org.json] from 'org.json:json'. \n" + @@ -124,12 +123,5 @@ public class BundleValidatorTest { List.of("org.json", "version", "[0.0.0,1)", "org.eclipse.jetty.client.api", "version", "[9.4.46,10)")); } - private static JarContext contextOf(DeployState state) { - return new JarContext() { - @Override public void illegal(String error) { throw new IllegalArgumentException(error); } - @Override public void illegal(String error, Throwable cause) { throw new IllegalArgumentException(error, cause); } - @Override public DeployState deployState() { return state; } - }; - } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java index 80ef81ee6d7..8acbf00a5a3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java @@ -71,7 +71,7 @@ public class CloudDataPlaneFilterValidatorTest { certFile2, List.of(createCertificate("bar")))); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new CloudDataPlaneFilterValidator(), model, deployState); + new CloudDataPlaneFilterValidator().validate(model, deployState); } @Test @@ -100,8 +100,11 @@ public class CloudDataPlaneFilterValidatorTest { certFile2, List.of(certificate))); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.expect(new CloudDataPlaneFilterValidator(), model, deployState, - "Duplicate certificate(s) detected in files: [%s, %s]. Certificate subject of duplicates: [%s]".formatted(certFile1, certFile2, certificate.getSubjectX500Principal().getName())); + IllegalArgumentException illegalArgumentException = Assertions.assertThrows(IllegalArgumentException.class, () -> + new CloudDataPlaneFilterValidator().validate(model, deployState)); + assertEquals( + "Duplicate certificate(s) detected in files: [%s, %s]. Certificate subject of duplicates: [%s]".formatted(certFile1, certFile2, certificate.getSubjectX500Principal().getName()), + illegalArgumentException.getMessage()); } @Test @@ -124,8 +127,11 @@ public class CloudDataPlaneFilterValidatorTest { Map.of(certFile1, List.of(certificate, certificate))); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.expect(new CloudDataPlaneFilterValidator(), model, deployState, - "Duplicate certificate(s) detected in files: [%s]. Certificate subject of duplicates: [%s]".formatted(certFile1, certificate.getSubjectX500Principal().getName())); + IllegalArgumentException illegalArgumentException = Assertions.assertThrows(IllegalArgumentException.class, () -> + new CloudDataPlaneFilterValidator().validate(model, deployState)); + assertEquals( + "Duplicate certificate(s) detected in files: [%s]. Certificate subject of duplicates: [%s]".formatted(certFile1, certificate.getSubjectX500Principal().getName()), + illegalArgumentException.getMessage()); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java index 3be1cbd44e3..58aa0e8625e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudHttpConnectorValidatorTest.java @@ -104,7 +104,7 @@ class CloudHttpConnectorValidatorTest { .endpoints(Set.of(new ContainerEndpoint("container", ApplicationClusterEndpoint.Scope.zone, List.of("c.example.com")))) .build(); var model = new VespaModel(new NullConfigModelRegistry(), state); - ValidationTester.validate(new CloudHttpConnectorValidator(), model, state); + new CloudHttpConnectorValidator().validate(model, state); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidatorTest.java index ac5a08b1394..2aa678fd34b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudUserFilterValidatorTest.java @@ -67,7 +67,7 @@ class CloudUserFilterValidatorTest { .properties(new TestProperties().setHostedVespa(isHosted).setAllowUserFilters(false)) .build(); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new CloudUserFilterValidator(), model, deployState); + new CloudUserFilterValidator().validate(model, deployState); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidatorTest.java index b6484049eaf..43c51bea04a 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ContainerInCloudValidatorTest.java @@ -61,7 +61,7 @@ public class ContainerInCloudValidatorTest { } DeployState deployState = builder.build(); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new ContainerInCloudValidator(), model, deployState); + new ContainerInCloudValidator().validate(model, deployState); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java index c9b014d9301..4e388df3ef8 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/DeploymentSpecValidatorTest.java @@ -63,7 +63,7 @@ public class DeploymentSpecValidatorTest { try { var deployState = builder.build(); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new DeploymentSpecValidator(), model, deployState); + new DeploymentSpecValidator().validate(model, 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/application/validation/EndpointCertificateSecretsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java index 3b6a559ce31..821ad1be8fa 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java @@ -48,7 +48,7 @@ public class EndpointCertificateSecretsValidatorTest { DeployState deployState = deployState(servicesXml(), deploymentXml(), Optional.of(EndpointCertificateSecrets.missing(1))); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new EndpointCertificateSecretsValidator(), model, deployState); + new EndpointCertificateSecretsValidator().validate(model, deployState); }); assertTrue(exception.getMessage().contains("TLS enabled, but could not yet retrieve certificate version 1 for application default:default:default")); } @@ -58,7 +58,7 @@ public class EndpointCertificateSecretsValidatorTest { DeployState deployState = deployState(servicesXml(), deploymentXml(), Optional.of(new EndpointCertificateSecrets("cert", "key"))); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new EndpointCertificateSecretsValidator(), model, deployState); + new EndpointCertificateSecretsValidator().validate(model, deployState); } @Test @@ -66,7 +66,7 @@ public class EndpointCertificateSecretsValidatorTest { DeployState deployState = deployState(servicesXml(), deploymentXml(), Optional.empty()); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new EndpointCertificateSecretsValidator(), model, deployState); + new EndpointCertificateSecretsValidator().validate(model, deployState); } private static DeployState deployState(String servicesXml, String deploymentXml, Optional<EndpointCertificateSecrets> endpointCertificateSecretsSecrets) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidatorTest.java index 190f68e6956..bcec73432b3 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/InfrastructureDeploymentValidatorTest.java @@ -43,7 +43,6 @@ public class InfrastructureDeploymentValidatorTest { var model = new VespaModel(new NullConfigModelRegistry(), deployState); var validator = new InfrastructureDeploymentValidator(); - ValidationTester.validate(validator, model, deployState); + validator.validate(model, deployState); } - } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java index a53ef233746..e4ba003df64 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/JvmHeapSizeValidatorTest.java @@ -39,25 +39,27 @@ class JvmHeapSizeValidatorTest { @Test void fails_on_too_low_jvm_percentage() throws IOException, SAXException { - var deployState = createDeployState(9, 7L * 1024 * 1024 * 1024); + var deployState = createDeployState(8, 7L * 1024 * 1024 * 1024); var model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.expect(new JvmHeapSizeValidator(), model, deployState, - "Allocated percentage of memory of JVM in cluster 'container' is too low (12% < 15%). Estimated cost of ONNX models is 7.00GB"); + var e = assertThrows(IllegalArgumentException.class, () -> new JvmHeapSizeValidator().validate(model, deployState)); + String expectedMessage = "Allocated percentage of memory of JVM in cluster 'container' is too low (3% < 15%). Estimated cost of ONNX models is 7.00GB"; + assertTrue(e.getMessage().contains(expectedMessage), e.getMessage()); } @Test void fails_on_too_low_heap_size() throws IOException, SAXException { var deployState = createDeployState(2.2, 1024L * 1024 * 1024); var model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.expect(new JvmHeapSizeValidator(), model, deployState, - "Allocated memory to JVM in cluster 'container' is too low (0.50GB < 0.60GB). Estimated cost of ONNX models is 1.00GB."); + var e = assertThrows(IllegalArgumentException.class, () -> new JvmHeapSizeValidator().validate(model, deployState)); + String expectedMessage = "Allocated memory to JVM in cluster 'container' is too low (0.50GB < 0.60GB). Estimated cost of ONNX models is 1.00GB."; + assertTrue(e.getMessage().contains(expectedMessage), e.getMessage()); } @Test void accepts_adequate_heap_size() throws IOException, SAXException { var deployState = createDeployState(8, 1024L * 1024 * 1024); var model = new VespaModel(new NullConfigModelRegistry(), deployState); - assertDoesNotThrow(() -> ValidationTester.validate(new JvmHeapSizeValidator(), model, deployState)); + assertDoesNotThrow(() -> new JvmHeapSizeValidator().validate(model, deployState)); } @Test @@ -79,7 +81,7 @@ class JvmHeapSizeValidatorTest { </services>"""; var deployState = createDeployState(servicesXml, 2, 1024L * 1024 * 1024); var model = new VespaModel(new NullConfigModelRegistry(), deployState); - assertDoesNotThrow(() -> ValidationTester.validate(new JvmHeapSizeValidator(), model, deployState)); + assertDoesNotThrow(() -> new JvmHeapSizeValidator().validate(model, deployState)); } private static DeployState createDeployState(String servicesXml, double nodeGb, long modelCostBytes) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidatorTest.java index 19be886d3e5..c68599f4595 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/PublicApiBundleValidatorTest.java @@ -1,8 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation; -import com.yahoo.config.model.deploy.DeployState; -import com.yahoo.vespa.model.application.validation.AbstractBundleValidator.JarContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -15,7 +13,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasSize; -import static org.junit.jupiter.api.Assertions.fail; /** * @author gjoranv @@ -32,11 +29,7 @@ public class PublicApiBundleValidatorTest { var jarFile = BundleValidatorTest.createTemporaryJarFile(tempDir, "non-public-api"); var validator = new PublicApiBundleValidator(); - validator.validateJarFile(new JarContext() { - @Override public void illegal(String error) { fail(); } - @Override public void illegal(String error, Throwable cause) { fail(); } - @Override public DeployState deployState() { return deployState; } - }, jarFile); + validator.validateJarFile(deployState, jarFile); String output = outputBuf.toString(); assertThat(output, containsString("uses non-public Vespa APIs: [")); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/SecretStoreValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/SecretStoreValidatorTest.java index 9d53c5af61c..ae23b3b722d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/SecretStoreValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/SecretStoreValidatorTest.java @@ -48,7 +48,7 @@ public class SecretStoreValidatorTest { DeployState deployState = deployState(servicesXml(), deploymentXml(true)); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new SecretStoreValidator(), model, deployState); + new SecretStoreValidator().validate(model, deployState); } @Test @@ -58,7 +58,7 @@ public class SecretStoreValidatorTest { DeployState deployState = deployState(servicesXml(), deploymentXml(false)); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new SecretStoreValidator(), model, deployState); + new SecretStoreValidator().validate(model, deployState); }); assertTrue(exception.getMessage().contains("Container cluster 'default' uses a secret store, so an Athenz domain and" + @@ -74,7 +74,7 @@ public class SecretStoreValidatorTest { DeployState deployState = deployState(servicesXml, deploymentXml(false)); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new SecretStoreValidator(), model, deployState); + new SecretStoreValidator().validate(model, deployState); } private static DeployState deployState(String servicesXml, String deploymentXml) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java index 92c2b5276cd..9a2f9fadac6 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UriBindingsValidatorTest.java @@ -107,7 +107,7 @@ public class UriBindingsValidatorTest { .endpoints(Set.of(new ContainerEndpoint("default", ApplicationClusterEndpoint.Scope.zone, List.of("default.example.com")))) .build(); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new UriBindingsValidator(), model, deployState); + new UriBindingsValidator().validate(model, deployState); } private static String createServicesXmlWithHandler(String handlerBinding) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java index d4a324901e2..837de946e36 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java @@ -81,7 +81,7 @@ public class UrlConfigValidatorTest { .build(); DeployState deployState = createDeployState(app, systemName); VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); - ValidationTester.validate(new UrlConfigValidator(), model, deployState); + new UrlConfigValidator().validate(model, deployState); } private static DeployState createDeployState(ApplicationPackage app, SystemName systemName) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java index 29279635918..8dc07d8857d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ValidationTester.java @@ -16,15 +16,12 @@ import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.Validation.Execution; -import com.yahoo.vespa.model.application.validation.change.ChangeValidator; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import java.time.Instant; import java.time.LocalDate; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Set; @@ -33,8 +30,6 @@ import java.util.stream.Stream; import static com.yahoo.config.model.test.MockApplicationPackage.BOOK_SCHEMA; import static com.yahoo.config.model.test.MockApplicationPackage.MUSIC_SCHEMA; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author bratseth @@ -119,27 +114,4 @@ public class ValidationTester { return s.replaceAll("\\d", "-"); } - public static void expect(Validator validator, VespaModel model, DeployState deployState, String... expectedMessages) { - Execution execution = new Execution(model, deployState); - validator.validate(execution); - assertTrue( execution.errors().stream().allMatch(error -> Arrays.stream(expectedMessages).anyMatch(error::contains)) - && Arrays.stream(expectedMessages).allMatch(expected -> execution.errors().stream().anyMatch(error -> error.contains(expected))), - "Expected errors: " + Arrays.toString(expectedMessages) + "\nActual errors: " + execution.errors()); - } - - /** Runs validation, and throws on illegalities. */ - public static void validate(Validator validator, VespaModel model, DeployState deployState) { - Execution execution = new Execution(model, deployState); - validator.validate(execution); - execution.throwIfFailed(); - } - - /** Runs validation and returns the resulting config chance actions, without checking whether they're currently allowed; or throws on illegalities. */ - public static List<ConfigChangeAction> validateChanges(ChangeValidator validator, VespaModel model, DeployState deployState) { - Execution execution = new Execution(model, deployState); - validator.validate(execution); - if ( ! execution.errors().isEmpty()) execution.throwIfFailed(); - return execution.actions(); - } - } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java index 6b5db3b081f..bc36b800bfb 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java @@ -41,22 +41,22 @@ public class CertificateRemovalChangeValidatorTest { CertificateRemovalChangeValidator validator = new CertificateRemovalChangeValidator(); // Adding certs -> ok - validator.validateClients("clusterId", List.of(c1, c2), List.of(c1, c2, c3), (id, msg) -> ValidationOverrides.empty.invalid(id, msg, now)); + validator.validateClients("clusterId", List.of(c1, c2), List.of(c1, c2, c3), new DeployState.Builder().now(now).build()); // Removing certs -> fails assertThrows(ValidationOverrides.ValidationException.class, () ->validator.validateClients("clusterId", List.of(c1, c2, c3), List.of(c1, c3), - (id, msg) -> ValidationOverrides.empty.invalid(id, msg, now))); + new DeployState.Builder().now(now).build())); // Removing certs with validationoverrides -> ok validator.validateClients("clusterId", List.of(c1, c2, c3), List.of(c1, c3), - (id, msg) -> ValidationOverrides.fromXml(validationOverrides).invalid(id, msg, now)); + new DeployState.Builder().now(now).validationOverrides(ValidationOverrides.fromXml(validationOverrides)).build()); // Adding and removing internal certs are ok: validator.validateClients("clusterId", List.of(c1, c2), List.of(c1, c2, internal), - (id, msg) -> ValidationOverrides.empty.invalid(id, msg, now)); + new DeployState.Builder().build()); validator.validateClients("clusterId", List.of(c1, c2, internal), List.of(c1, c2), - (id, msg) -> ValidationOverrides.empty.invalid(id, msg, now)); + new DeployState.Builder().now(now).build()); } static X509Certificate certificate(String cn) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java index c9703fc34af..081e10ecea6 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigChangeTestUtils.java @@ -7,11 +7,9 @@ import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.provision.ClusterSpec; import java.util.ArrayList; -import java.util.Comparator; import java.util.List; import java.util.stream.Collectors; -import static java.util.Comparator.comparing; import static org.junit.jupiter.api.Assertions.assertEquals; public class ConfigChangeTestUtils { @@ -62,8 +60,8 @@ public class ConfigChangeTestUtils { public static void assertEqualActions(List<ConfigChangeAction> exp, List<ConfigChangeAction> act) { var mutableExp = new ArrayList<>(exp); var mutableAct = new ArrayList<>(act); - mutableExp.sort(comparing(ConfigChangeAction::getMessage)); - mutableAct.sort(comparing(ConfigChangeAction::getMessage)); + mutableExp.sort((lhs, rhs) -> lhs.getMessage().compareTo(rhs.getMessage())); + mutableAct.sort((lhs, rhs) -> lhs.getMessage().compareTo(rhs.getMessage())); assertEquals(mutableExp, mutableAct); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java index f68a1da7dfb..a41b538d3ca 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ConfigValueChangeValidatorTest.java @@ -1,15 +1,16 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change; +import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.test.AnotherrestartConfig; import com.yahoo.config.ConfigInstance; +import com.yahoo.test.RestartConfig; +import com.yahoo.test.SimpletypesConfig; import com.yahoo.config.model.api.ConfigChangeAction; -import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.model.producer.AnyConfigProducer; import com.yahoo.config.model.producer.TreeConfigProducer; +import com.yahoo.config.model.producer.AbstractConfigProducerRoot; import com.yahoo.config.model.test.MockRoot; -import com.yahoo.test.AnotherrestartConfig; -import com.yahoo.test.RestartConfig; -import com.yahoo.test.SimpletypesConfig; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.Host; import com.yahoo.vespa.model.HostResource; @@ -24,9 +25,7 @@ import org.junit.jupiter.api.Test; import java.util.List; import java.util.stream.Collectors; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; /** * Testing the validator on both a stub model and a real-life Vespa model. @@ -153,6 +152,11 @@ public class ConfigValueChangeValidatorTest { assertEmptyLog(); } + private List<ConfigChangeAction> getConfigChanges(VespaModel currentModel, VespaModel nextModel) { + ConfigValueChangeValidator validator = new ConfigValueChangeValidator(); + return validator.validate(currentModel, nextModel, new DeployState.Builder().deployLogger(logger).build()); + } + private List<ConfigChangeAction> getConfigChanges(AbstractConfigProducerRoot currentModel, AbstractConfigProducerRoot nextModel) { ConfigValueChangeValidator validator = new ConfigValueChangeValidator(); @@ -241,7 +245,8 @@ public class ConfigValueChangeValidatorTest { setHostResource(new HostResource(new Host(null, "localhost"))); } - @Override public int getPortCount() { + @Override + public int getPortCount() { return 0; } @@ -257,7 +262,8 @@ public class ConfigValueChangeValidatorTest { this.value = value; } - @Override public void getConfig(RestartConfig.Builder builder) { + @Override + public void getConfig(RestartConfig.Builder builder) { builder.value(value); } @@ -277,7 +283,8 @@ public class ConfigValueChangeValidatorTest { this.anotherValue = anotherValue; } - @Override public void getConfig(AnotherrestartConfig.Builder builder) { + @Override + public void getConfig(AnotherrestartConfig.Builder builder) { builder.anothervalue(anotherValue); } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidatorTest.java index a7485b177d3..ac59ca58cb5 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ContainerRestartValidatorTest.java @@ -5,7 +5,6 @@ import com.yahoo.config.model.api.ConfigChangeAction; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.vespa.defaults.Defaults; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import org.junit.jupiter.api.Test; @@ -68,7 +67,7 @@ public class ContainerRestartValidatorTest { } private static List<ConfigChangeAction> validateModel(VespaModel current, VespaModel next) { - return ValidationTester.validateChanges(new ContainerRestartValidator(), next, new DeployState.Builder().previousModel(current).build()); + return new ContainerRestartValidator().validate(current, next, new DeployState.Builder().build()); } private static VespaModel createModel(boolean restartOnDeploy, boolean alwaysRestart) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java index f4123a72e1f..cdc80754194 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/GlobalDocumentChangeValidatorTest.java @@ -1,7 +1,6 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.application.validation.change; -import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.application.validation.ValidationTester; @@ -15,7 +14,7 @@ import static org.junit.jupiter.api.Assertions.*; public class GlobalDocumentChangeValidatorTest { @Test - void testChangeGlobalAttribute() { + void testChangGlobalAttribute() { testChangeGlobalAttribute(true, false, false, null); testChangeGlobalAttribute(true, true, true, null); testChangeGlobalAttribute(false, false, true, null); @@ -28,16 +27,14 @@ public class GlobalDocumentChangeValidatorTest { ValidationTester tester = new ValidationTester(); VespaModel oldModel = tester.deploy(null, getServices(oldGlobal), Environment.prod, validationOverrides, "default.indexing").getFirst(); try { - var actions = tester.deploy(oldModel, getServices(newGlobal), Environment.prod, validationOverrides, "default.indexing").getSecond(); + tester.deploy(oldModel, getServices(newGlobal), Environment.prod, validationOverrides, "default.indexing").getSecond(); assertTrue(allowed); - assertEquals(validationOverrides == null ? 0 : 1, actions.size()); - if (validationOverrides != null) assertEquals(ClusterSpec.Id.from("default"), actions.get(0).clusterId()); - } catch (IllegalArgumentException e) { + } catch (IllegalStateException e) { assertFalse(allowed); - assertEquals("global-document-change: Document type music in cluster default changed global from " + oldGlobal + " to " + newGlobal + ". " + - "To handle this change, first stop services on all content nodes. Then, deploy with validation override. Finally, start services on all content nodes. " + - "To allow this add <allow until='yyyy-mm-dd'>global-document-change</allow> to validation-overrides.xml, see https://docs.vespa.ai/en/reference/validation-overrides.html", - e.getMessage()); + assertEquals("Document type music in cluster default changed global from " + oldGlobal + " to " + newGlobal + ". " + + "Add validation override 'global-document-change' to force this change through. " + + "First, stop services on all content nodes. Then, deploy with validation override. Finally, start services on all content nodes.", + e.getMessage()); } } @@ -55,10 +52,8 @@ public class GlobalDocumentChangeValidatorTest { } private static final String globalDocumentValidationOverrides = - """ - <validation-overrides> - <allow until='2000-01-14' comment='test override'>global-document-change</allow> - </validation-overrides> - """; + "<validation-overrides>\n" + + " <allow until='2000-01-14' comment='test override'>global-document-change</allow>\n" + + "</validation-overrides>\n"; } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSchemaClusterChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSchemaClusterChangeValidatorTest.java index 3555fc21471..fb4e9f1a00b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSchemaClusterChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexedSchemaClusterChangeValidatorTest.java @@ -7,7 +7,6 @@ import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.content.utils.ApplicationPackageBuilder; import com.yahoo.vespa.model.content.utils.ContentClusterBuilder; import com.yahoo.vespa.model.content.utils.SchemaBuilder; @@ -71,7 +70,8 @@ public class IndexedSchemaClusterChangeValidatorTest { } private List<ConfigChangeAction> validate() { - return normalizeServicesInActions(ValidationTester.validateChanges(validator, nextModel, new DeployState.Builder().previousModel(currentModel).build())); + return normalizeServicesInActions(validator.validate(currentModel, nextModel, + new DeployState.Builder().build())); } public void assertValidation() { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java index b2439651cf9..afa36ac271e 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/NodeResourceChangeValidatorTest.java @@ -13,7 +13,6 @@ import com.yahoo.config.provision.ClusterSpec; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.ProvisionLogger; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import org.junit.jupiter.api.Test; @@ -59,7 +58,7 @@ public class NodeResourceChangeValidatorTest { } private List<ConfigChangeAction> validate(VespaModel current, VespaModel next) { - return ValidationTester.validateChanges(new NodeResourceChangeValidator(), next, new DeployState.Builder().previousModel(current).build()); + return new NodeResourceChangeValidator().validate(current, next, new DeployState.Builder().build()); } private static VespaModel model(int mem1, int mem2, int mem3, int mem4) { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java index 67e8a4d512e..f2013f0f26b 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/RestartOnDeployForOnnxModelChangesValidatorTest.java @@ -8,7 +8,6 @@ import com.yahoo.config.model.api.OnnxModelOptions; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.deploy.TestProperties; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.test.utils.VespaModelCreatorWithMockPkg; import org.junit.jupiter.api.Test; @@ -72,7 +71,7 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { } private static List<ConfigChangeAction> validateModel(VespaModel current, VespaModel next) { - return ValidationTester.validateChanges(new RestartOnDeployForOnnxModelChangesValidator(), next, deployStateBuilder().previousModel(current).build()); + return new RestartOnDeployForOnnxModelChangesValidator().validate(current, next, deployStateBuilder().build()); } private static OnnxModelCost onnxModelCost() { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSchemaClusterChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSchemaClusterChangeValidatorTest.java index ee64ceb6969..8db9d39534d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSchemaClusterChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/StreamingSchemaClusterChangeValidatorTest.java @@ -7,7 +7,6 @@ import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.model.VespaModel; -import com.yahoo.vespa.model.application.validation.ValidationTester; import com.yahoo.vespa.model.content.utils.ApplicationPackageBuilder; import com.yahoo.vespa.model.content.utils.ContentClusterBuilder; import com.yahoo.vespa.model.content.utils.DocType; @@ -70,8 +69,8 @@ public class StreamingSchemaClusterChangeValidatorTest { } public List<ConfigChangeAction> validate() { - return normalizeServicesInActions(ValidationTester.validateChanges(validator, nextModel, - new DeployState.Builder().previousModel(currentModel).build())); + return normalizeServicesInActions(validator.validate(currentModel, nextModel, + new DeployState.Builder().build())); } public void assertValidation() { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index 5c049c61c1a..90087a25c59 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -293,8 +293,7 @@ public class SessionPreparer { (path, attr) -> attr.isRegularFile() && path.getFileName().toString().matches(".*\\.[Jj][Aa][Rr]"))) { paths.forEach(jarPath -> { try { - new BundleValidator().getPomXmlContent((msg, cause) -> { throw new IllegalArgumentException(msg, cause); }, - logger, new JarFile(jarPath.toFile())).ifPresent(pom -> { + new BundleValidator().getPomXmlContent(logger, new JarFile(jarPath.toFile())).ifPresent(pom -> { try { new ValidationProcessor().process(pom); } |