diff options
Diffstat (limited to 'config-model')
10 files changed, 163 insertions, 116 deletions
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java index 33dfee58d1a..f19341098f4 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java @@ -172,7 +172,7 @@ public class DeployState implements ConfigDefinitionStore { /** Get the global rank profile registry for this application. */ public final RankProfileRegistry rankProfileRegistry() { return rankProfileRegistry; } - /** Returns the validation overrides of this. This is never null */ + /** Returns the validation overrides of this. This is never null. */ public ValidationOverrides validationOverrides() { return validationOverrides; } @Override diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java index 2e1c661e09a..6f0254145d6 100644 --- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java +++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java @@ -141,7 +141,6 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea @Override public String summaryDecodePolicy() { return summaryDecodePolicy; } @Override public Optional<CloudAccount> cloudAccount() { return cloudAccount; } @Override public boolean allowUserFilters() { return allowUserFilters; } - @Override public boolean enableGlobalPhase() { return true; } // Enable global-phase by default for unit tests only @Override public List<DataplaneToken> dataplaneTokens() { return dataplaneTokens; } @Override public int contentLayerMetadataFeatureLevel() { return contentLayerMetadataFeatureLevel; } @Override public boolean dynamicHeapSize() { return dynamicHeapSize; } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java index 903f1c06024..65572d07cc2 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/VespaModelFactory.java @@ -217,13 +217,6 @@ public class VespaModelFactory implements ModelFactory { private List<ConfigChangeAction> validateModel(VespaModel model, DeployState deployState, ValidationParameters validationParameters) { try { return new Validation(additionalValidators).validate(model, validationParameters, deployState); - } catch (ValidationOverrides.ValidationException e) { - if (deployState.isHosted() && zone.environment().isManuallyDeployed()) - deployState.getDeployLogger().logApplicationPackage(Level.WARNING, - "Auto-overriding validation which would be disallowed in production: " + - Exceptions.toMessageString(e)); - else - rethrowUnlessIgnoreErrors(e, validationParameters.ignoreValidationErrors()); } catch (IllegalArgumentException | TransientException | QuotaExceededException e) { rethrowUnlessIgnoreErrors(e, validationParameters.ignoreValidationErrors()); } catch (Exception e) { 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 56277345515..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 @@ -3,8 +3,8 @@ package com.yahoo.vespa.model.application.validation; import com.yahoo.config.application.api.ValidationId; import com.yahoo.config.application.api.ValidationOverrides; +import com.yahoo.config.application.api.ValidationOverrides.ValidationException; import com.yahoo.config.model.api.ConfigChangeAction; -import com.yahoo.config.model.api.Model; import com.yahoo.config.model.api.ValidationParameters; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; @@ -25,21 +25,17 @@ import com.yahoo.vespa.model.application.validation.change.RestartOnDeployForOnn import com.yahoo.vespa.model.application.validation.change.StartupCommandChangeValidator; import com.yahoo.vespa.model.application.validation.change.StreamingSearchClusterChangeValidator; import com.yahoo.vespa.model.application.validation.first.RedundancyValidator; +import com.yahoo.yolean.Exceptions; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; +import java.util.ArrayList; +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; -import static java.util.stream.Collectors.groupingBy; -import static java.util.stream.Collectors.mapping; -import static java.util.stream.Collectors.toCollection; - /** * Executor of validators. This defines the right order of validator execution. * @@ -51,7 +47,7 @@ public class Validation { public Validation() { this(List.of()); } - /** Create instance taking additional validators (e.g for cloud applications) */ + /** Create instance taking additional validators (e.g., for cloud applications) */ public Validation(List<Validator> additionalValidators) { this.additionalValidators = additionalValidators; } /** @@ -62,85 +58,80 @@ public class Validation { * @throws ValidationOverrides.ValidationException if the change fails validation */ public List<ConfigChangeAction> validate(VespaModel model, ValidationParameters validationParameters, DeployState deployState) { + Execution execution = new Execution(model, deployState); if (validationParameters.checkRouting()) { - new RoutingValidator().validate(model, deployState); - new RoutingSelectorValidator().validate(model, deployState); + validateRouting(execution); } - new SchemasDirValidator().validate(model, deployState); - new BundleValidator().validate(model, deployState); - new PublicApiBundleValidator().validate(model, deployState); - new SearchDataTypeValidator().validate(model, deployState); - new ComplexFieldsWithStructFieldAttributesValidator().validate(model, deployState); - new ComplexFieldsWithStructFieldIndexesValidator().validate(model, deployState); - new StreamingValidator().validate(model, deployState); - new RankSetupValidator(validationParameters.ignoreValidationErrors()).validate(model, deployState); - new NoPrefixForIndexes().validate(model, deployState); - new ContainerInCloudValidator().validate(model, deployState); - new DeploymentSpecValidator().validate(model, deployState); - new ValidationOverridesValidator().validate(model, deployState); - new ConstantValidator().validate(model, deployState); - new SecretStoreValidator().validate(model, deployState); - new EndpointCertificateSecretsValidator().validate(model, deployState); - new AccessControlFilterValidator().validate(model, deployState); - new QuotaValidator().validate(model, deployState); - new UriBindingsValidator().validate(model, deployState); - new CloudDataPlaneFilterValidator().validate(model, deployState); - new AccessControlFilterExcludeValidator().validate(model, deployState); - new CloudUserFilterValidator().validate(model, deployState); - new CloudHttpConnectorValidator().validate(model, deployState); - new UrlConfigValidator().validate(model, deployState); - new JvmHeapSizeValidator().validate(model, deployState); - - additionalValidators.forEach(v -> v.validate(model, deployState)); - - List<ConfigChangeAction> result = Collections.emptyList(); + + validateModel(validationParameters, execution); + + additionalValidators.forEach(execution::run); + if (deployState.getProperties().isFirstTimeDeployment()) { - validateFirstTimeDeployment(model, deployState); - } else { - Optional<Model> currentActiveModel = deployState.getPreviousModel(); - if (currentActiveModel.isPresent() && (currentActiveModel.get() instanceof VespaModel)) { - result = validateChanges((VespaModel) currentActiveModel.get(), model, deployState); - deferConfigChangesForClustersToBeRestarted(result, model); - } + validateFirstTimeDeployment(execution); + } + 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); } - return result; + + execution.throwIfFailed(); + return execution.actions; } - private static List<ConfigChangeAction> validateChanges(VespaModel currentModel, VespaModel nextModel, - DeployState deployState) { - ChangeValidator[] validators = new ChangeValidator[] { - new IndexingModeChangeValidator(), - new GlobalDocumentChangeValidator(), - new IndexedSearchClusterChangeValidator(), - new StreamingSearchClusterChangeValidator(), - new ConfigValueChangeValidator(), - new StartupCommandChangeValidator(), - new ContentTypeRemovalValidator(), - new ContentClusterRemovalValidator(), - new ResourcesReductionValidator(), - new ResourcesReductionValidator(), - new ContainerRestartValidator(), - new NodeResourceChangeValidator(), - new RedundancyIncreaseValidator(), - new CertificateRemovalChangeValidator(), - new RedundancyValidator(), - new RestartOnDeployForOnnxModelChangesValidator(), - }; - List<ConfigChangeAction> actions = Arrays.stream(validators) - .flatMap(v -> v.validate(currentModel, nextModel, deployState).stream()) - .toList(); - - Map<ValidationId, Collection<String>> disallowableActions = actions.stream() - .filter(action -> action.validationId().isPresent()) - .collect(groupingBy(action -> action.validationId().orElseThrow(), - mapping(ConfigChangeAction::getMessage, - toCollection(LinkedHashSet::new)))); - deployState.validationOverrides().invalid(disallowableActions, deployState.now()); - return actions; + private static void validateRouting(Execution execution) { + execution.run(new RoutingValidator()); + execution.run(new RoutingSelectorValidator()); } - private static void validateFirstTimeDeployment(VespaModel model, DeployState deployState) { - new RedundancyValidator().validate(model, deployState); + private static void validateModel(ValidationParameters validationParameters, Execution 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) { + execution.run(new RedundancyValidator()); + } + + 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) { @@ -159,4 +150,61 @@ public class Validation { } } + + private static class Execution { + + private final Map<ValidationId, List<String>> failures = new LinkedHashMap<>(); + private final VespaModel model; + private final DeployState deployState; + private final List<ConfigChangeAction> actions = new ArrayList<>(); + + private Execution(VespaModel model, DeployState deployState) { + this.model = model; + this.deployState = deployState; + } + + private void run(Validator validator) { + try { + validator.validate(model, deployState); + } + catch (ValidationException e) { + e.messagesById().forEach((id, messages) -> failures.computeIfAbsent(id, __ -> new ArrayList<>()).addAll(messages)); + } + } + + 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)); + } + } + + 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/change/search/IndexingScriptChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java index fb07f65b5f4..e2dd3aca0b9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/IndexingScriptChangeValidator.java @@ -40,7 +40,7 @@ public class IndexingScriptChangeValidator { String fieldName = nextField.getName(); ImmutableSDField currentField = currentSchema.getConcreteField(fieldName); if (currentField != null) { - validateScripts(currentField, nextField).ifPresent(r -> result.add(r)); + validateScripts(currentField, nextField).ifPresent(result::add); } else if (nextField.isExtraField()) { result.add(VespaReindexAction.of(id, 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 0b32194e257..31e4c661151 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 @@ -126,16 +126,9 @@ class JvmHeapSizeValidatorTest { @Override public boolean restartOnDeploy() { return false;} @Override public void store() {} @Override public long aggregatedModelCostInBytes() { return totalCost.get(); } - @Override public void registerModel(ApplicationFile path) {} @Override public void registerModel(ApplicationFile path, OnnxModelOptions onnxModelOptions) {} @Override - public void registerModel(URI uri) { - assertEquals("https://my/url/model.onnx", uri.toString()); - totalCost.addAndGet(modelCost); - } - - @Override public void registerModel(URI uri, OnnxModelOptions onnxModelOptions) { assertEquals("https://my/url/model.onnx", uri.toString()); totalCost.addAndGet(modelCost); 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 6b7df8871aa..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 @@ -23,9 +23,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows; public class CertificateRemovalChangeValidatorTest { private static final String validationOverrides = - "<validation-overrides>\n" + - " <allow until='2000-01-14' comment='test override'>certificate-removal</allow>\n" + - "</validation-overrides>\n"; + """ + <validation-overrides> + <allow until='2000-01-14' comment='test override'>certificate-removal</allow> + </validation-overrides> + """; @Test void validate() { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java index 3fd3180b37e..9e0eab9aba7 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/IndexingModeChangeValidatorTest.java @@ -22,19 +22,28 @@ import static org.junit.jupiter.api.Assertions.fail; public class IndexingModeChangeValidatorTest { @Test + void testChangingIndexModeFromIndexedToStreamingWhenDisallowedButInDev() { + ValidationTester tester = new ValidationTester(); + + VespaModel oldModel = + tester.deploy(null, getServices("index"), Environment.dev, "<validation-overrides />").getFirst(); + List<ConfigChangeAction> actions = tester.deploy(oldModel, getServices("streaming"), Environment.dev, "<calidation-overrides />").getSecond(); + assertReindexingChange("Document type 'music' in cluster 'default-content' changed indexing mode from 'indexed' to 'streaming'", actions); + } + + @Test void testChangingIndexModeFromIndexedToStreamingWhenDisallowed() { ValidationTester tester = new ValidationTester(); VespaModel oldModel = tester.deploy(null, getServices("index"), Environment.prod, "<validation-overrides />").getFirst(); try { - List<ConfigChangeAction> changeActions = - tester.deploy(oldModel, getServices("streaming"), Environment.prod, "<calidation-overrides />").getSecond(); + tester.deploy(oldModel, getServices("streaming"), Environment.prod, "<calidation-overrides />").getSecond(); fail("Should throw on disallowed config change action"); } catch (ValidationException e) { - assertEquals("indexing-mode-change:\n" + - "\tDocument type 'music' in cluster 'default-content' changed indexing mode from 'indexed' to 'streaming'\n" + + assertEquals("indexing-mode-change: " + + "Document type 'music' in cluster 'default-content' changed indexing mode from 'indexed' to 'streaming'. " + "To allow this add <allow until='yyyy-mm-dd'>indexing-mode-change</allow> to validation-overrides.xml, see https://docs.vespa.ai/en/reference/validation-overrides.html", e.getMessage()); } @@ -94,8 +103,10 @@ public class IndexingModeChangeValidatorTest { } private static final String validationOverrides = - "<validation-overrides>\n" + - " <allow until='2000-01-14' comment='test override'>indexing-mode-change</allow>\n" + - "</validation-overrides>\n"; + """ + <validation-overrides> + <allow until='2000-01-14' comment='test override'>indexing-mode-change</allow> + </validation-overrides> + """; } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java index f9c0d00ac2c..b7e612127c1 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/ResourcesReductionValidatorTest.java @@ -29,6 +29,14 @@ public class ResourcesReductionValidatorTest { private final ValidationTester tester = new ValidationTester(provisioner); @Test + void ok_when_reduction_by_over_50_percent_in_hosted_dev() { + var fromResources = new NodeResources(8, 64, 800, 1); + var toResources = new NodeResources(8, 16, 800, 1); + VespaModel previous = tester.deploy(null, contentServices(6, fromResources), Environment.dev, null, CONTAINER_CLUSTER).getFirst(); + tester.deploy(previous, contentServices(6, toResources), Environment.dev, null, CONTAINER_CLUSTER); + } + + @Test void fail_when_reduction_by_over_50_percent() { var fromResources = new NodeResources(8, 64, 800, 1); var toResources = new NodeResources(8, 16, 800, 1); 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 adcf58785fa..13389689de5 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 @@ -15,7 +15,6 @@ import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -89,17 +88,11 @@ public class RestartOnDeployForOnnxModelChangesValidatorTest { public long aggregatedModelCostInBytes() { return estimatedCost; } @Override - public void registerModel(ApplicationFile path) {} - - @Override public void registerModel(ApplicationFile path, OnnxModelOptions onnxModelOptions) {} @Override - public void registerModel(URI uri) {} - - @Override public void registerModel(URI uri, OnnxModelOptions onnxModelOptions) { - models.put(uri.toString(), new OnnxModelCost.ModelInfo(uri.toString(), estimatedCost, hash, Optional.ofNullable(onnxModelOptions))); + models.put(uri.toString(), new OnnxModelCost.ModelInfo(uri.toString(), estimatedCost, hash, onnxModelOptions)); } @Override |