aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHarald Musum <musum@yahooinc.com>2023-09-23 17:38:32 +0200
committerGitHub <noreply@github.com>2023-09-23 17:38:32 +0200
commitd437ab47820f3918420d75ed11613b17cd86e0bc (patch)
tree0f151cf2cac02e7da0c0c1c128f80bc2d6f64976
parent0ee9e27d5976301dbcee0bd80783bc92785e21d8 (diff)
parent9ddc6b3541e59a20c8c58807518703d4187840b8 (diff)
Merge pull request #28627 from vespa-engine/revert-28618-hmusum/validate-s3-urls-in-config-and-exclusive-cluster-nodesv8.231.32
-rw-r--r--config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java52
-rw-r--r--config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java1
-rw-r--r--config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java5
-rw-r--r--config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java107
4 files changed, 1 insertions, 164 deletions
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
deleted file mode 100644
index e6cd1a9e192..00000000000
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright Yahoo. 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.container.ApplicationContainerCluster;
-
-import java.util.Optional;
-
-/**
- * Validates that config using s3:// urls is used in public system and with nodes that are exclusive.
- *
- * @author hmusum
- */
-public class UrlConfigValidator extends Validator {
-
- @Override
- public void validate(VespaModel model, DeployState state) {
- if (! state.isHostedTenantApplication(model.getAdmin().getApplicationType())) return;
-
- model.getContainerClusters().forEach((__, cluster) -> {
- var isExclusive = hasExclusiveNodes(model, cluster);
- validateS3UlsInConfig(state, cluster, isExclusive);
- });
- }
-
- private static boolean hasExclusiveNodes(VespaModel model, ApplicationContainerCluster cluster) {
- return model.hostSystem().getHosts()
- .stream()
- .flatMap(hostResource -> hostResource.spec().membership().stream())
- .filter(membership -> membership.cluster().id().equals(cluster.id()))
- .anyMatch(membership -> membership.cluster().isExclusive());
- }
-
- private static void validateS3UlsInConfig(DeployState state, ApplicationContainerCluster cluster, boolean isExclusive) {
- if (hasUrlInConfig(state)) {
- // 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 ( ! state.zone().system().isPublic())
- throw new IllegalArgumentException(message + ". This is only supported in public systems");
- else if ( ! isExclusive)
- throw new IllegalArgumentException(message + ". Nodes in the cluster need to be 'exclusive'," +
- " see https://cloud.vespa.ai/en/reference/services#nodes");
- }
- }
-
- private static boolean hasUrlInConfig(DeployState state) {
- return state.getFileRegistry().export().stream()
- .anyMatch(fileReference -> fileReference.relativePath.startsWith("s3://"));
- }
-
-}
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 90616c99979..53a553ee624 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
@@ -87,7 +87,6 @@ public class Validation {
new AccessControlFilterExcludeValidator().validate(model, deployState);
new CloudUserFilterValidator().validate(model, deployState);
new CloudHttpConnectorValidator().validate(model, deployState);
- new UrlConfigValidator().validate(model, deployState);
additionalValidators.forEach(v -> v.validate(model, deployState));
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
index 8d8b4d72f4d..8bed5e64bf5 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
@@ -133,10 +133,7 @@ public class UserConfiguredFiles implements Serializable {
Path path;
if (isModelType) {
var modelReference = ModelReference.valueOf(builder.getValue());
- if (modelReference.path().isEmpty()) {
- modelReference.url().ifPresent(url -> fileRegistry.addUri(url.value()));
- return;
- }
+ if (modelReference.path().isEmpty()) return;
path = Path.fromString(modelReference.path().get().value());
}
else {
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
deleted file mode 100644
index cef4d8c27dd..00000000000
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java
+++ /dev/null
@@ -1,107 +0,0 @@
-package com.yahoo.vespa.model.application.validation;
-
-import com.yahoo.config.application.api.ApplicationPackage;
-import com.yahoo.config.model.NullConfigModelRegistry;
-import com.yahoo.config.model.api.ConfigDefinitionRepo;
-import com.yahoo.config.model.application.provider.MockFileRegistry;
-import com.yahoo.config.model.deploy.DeployState;
-import com.yahoo.config.model.deploy.TestProperties;
-import com.yahoo.config.model.test.MockApplicationPackage;
-import com.yahoo.config.provision.RegionName;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.config.provision.Zone;
-import com.yahoo.embedding.BertBaseEmbedderConfig;
-import com.yahoo.vespa.config.ConfigDefinitionKey;
-import com.yahoo.vespa.config.buildergen.ConfigDefinition;
-import com.yahoo.vespa.model.VespaModel;
-import org.junit.jupiter.api.Test;
-import org.xml.sax.SAXException;
-
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Map;
-
-import static com.yahoo.config.provision.Environment.prod;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-
-public class UrlConfigValidatorTest {
-
- @Test
- void failsWhenContainerNodesNotExclusive() throws IOException, SAXException {
- runValidatorOnApp(true, SystemName.Public); // Exclusive nodes in public => success
-
- assertEquals("Found s3:// urls in config for container cluster default. This is only supported in public systems",
- assertThrows(IllegalArgumentException.class,
- () -> runValidatorOnApp(false, SystemName.main))
- .getMessage());
-
- assertEquals("Found s3:// urls in config for container cluster default. This is only supported in public systems",
- assertThrows(IllegalArgumentException.class,
- () -> runValidatorOnApp(true, SystemName.main))
- .getMessage());
-
- assertEquals("Found s3:// urls in config for container cluster default. Nodes in the cluster need to be 'exclusive',"
- + " see https://cloud.vespa.ai/en/reference/services#nodes",
- assertThrows(IllegalArgumentException.class,
- () -> runValidatorOnApp(false, SystemName.Public))
- .getMessage());
- }
-
- private static String containerXml(boolean isExclusive) {
- return """
- <container version='1.0' id='default'>
- <component id='transformer' class='ai.vespa.embedding.BertBaseEmbedder' bundle='model-integration'>
- <config name='embedding.bert-base-embedder'>
- <transformerModel url='s3://models/minilm-l6-v2/sentence_all_MiniLM_L6_v2.onnx' path='foo'/>
- <tokenizerVocab url='s3://models/bert-base-uncased.txt'/>
- </config>
- </component>
- <search/>
- <document-api/>
- <nodes count='2' exclusive='%s' />
- </container>
- """.formatted(Boolean.toString(isExclusive));
- }
-
- private static void runValidatorOnApp(boolean isExclusive, SystemName systemName) throws IOException, SAXException {
- String container = containerXml(isExclusive);
- String servicesXml = """
- <services version='1.0'>
- %s
- </services>
- """.formatted(container);
- ApplicationPackage app = new MockApplicationPackage.Builder()
- .withServices(servicesXml)
- .build();
- DeployState deployState = createDeployState(app, systemName);
- VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
- new UrlConfigValidator().validate(model, deployState);
- }
-
- private static DeployState createDeployState(ApplicationPackage app, SystemName systemName) {
- boolean isHosted = true;
- var builder = new DeployState.Builder()
- .applicationPackage(app)
- .zone(new Zone(systemName, prod, RegionName.from("us-east-3")))
- .properties(new TestProperties().setHostedVespa(isHosted))
- .fileRegistry(new MockFileRegistry());
-
- Map<ConfigDefinitionKey, ConfigDefinition> defs = new HashMap<>();
- defs.put(new ConfigDefinitionKey(BertBaseEmbedderConfig.CONFIG_DEF_NAME, BertBaseEmbedderConfig.CONFIG_DEF_NAMESPACE),
- new ConfigDefinition(BertBaseEmbedderConfig.CONFIG_DEF_NAME, BertBaseEmbedderConfig.CONFIG_DEF_SCHEMA));
- builder.configDefinitionRepo(new ConfigDefinitionRepo() {
- @Override
- public Map<ConfigDefinitionKey, com.yahoo.vespa.config.buildergen.ConfigDefinition> getConfigDefinitions() {
- return defs;
- }
-
- @Override
- public com.yahoo.vespa.config.buildergen.ConfigDefinition get(ConfigDefinitionKey key) {
- return defs.get(key);
- }
- });
- return builder.build();
- }
-
-}