diff options
author | Valerij Fredriksen <freva@users.noreply.github.com> | 2023-09-22 15:58:17 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-22 15:58:17 +0200 |
commit | 32e94dd5580fa4c49104c1834b7a8f2a29ccdada (patch) | |
tree | d8bc289e15602ea575ed9c21b0a2f20ea162a6df | |
parent | 783d1958dbbebb68d3230bb25f6737964e0dc6b7 (diff) | |
parent | 05f2351f27be27416252b0df00acfeb63e51596b (diff) |
Merge pull request #28618 from vespa-engine/hmusum/validate-s3-urls-in-config-and-exclusive-cluster-nodes
Validate use of s3 urls in config
4 files changed, 164 insertions, 1 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 new file mode 100644 index 00000000000..e6cd1a9e192 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java @@ -0,0 +1,52 @@ +// 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 53a553ee624..90616c99979 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,6 +87,7 @@ 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 8bed5e64bf5..8d8b4d72f4d 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,7 +133,10 @@ public class UserConfiguredFiles implements Serializable { Path path; if (isModelType) { var modelReference = ModelReference.valueOf(builder.getValue()); - if (modelReference.path().isEmpty()) return; + if (modelReference.path().isEmpty()) { + modelReference.url().ifPresent(url -> fileRegistry.addUri(url.value())); + 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 new file mode 100644 index 00000000000..cef4d8c27dd --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java @@ -0,0 +1,107 @@ +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(); + } + +} |