diff options
6 files changed, 189 insertions, 4 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..d9dd3729bd3 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java @@ -0,0 +1,50 @@ +// 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; + +/** + * 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 (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 ( ! 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 hasS3UrlInConfig(ApplicationContainerCluster cluster) { + return cluster.userConfiguredUrls().all().stream() + .anyMatch(url -> url.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 b9ecf7c2d22..30aafe67be7 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); new JvmHeapSizeValidator().validate(model, deployState); additionalValidators.forEach(v -> v.validate(model, deployState)); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index da6e3387d6a..0945dfaf54a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -40,9 +40,11 @@ import com.yahoo.vespa.model.container.component.SystemBindingPattern; import com.yahoo.vespa.model.container.configserver.ConfigserverCluster; import com.yahoo.vespa.model.filedistribution.UserConfiguredFiles; +import java.net.URL; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; @@ -104,6 +106,8 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat private List<ApplicationClusterEndpoint> endpoints = List.of(); + private UserConfiguredUrls userConfiguredUrls = new UserConfiguredUrls(); + public ApplicationContainerCluster(TreeConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState) { super(parent, configSubId, clusterId, deployState, true, 10); this.tlsClientAuthority = deployState.tlsClientAuthority(); @@ -134,6 +138,8 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat logger = deployState.getDeployLogger(); } + public UserConfiguredUrls userConfiguredUrls() { return userConfiguredUrls; } + @Override protected void doPrepare(DeployState deployState) { super.doPrepare(deployState); @@ -154,7 +160,9 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat if (containers.isEmpty()) return; // Files referenced from user configs to all components. - UserConfiguredFiles files = new UserConfiguredFiles(deployState.getFileRegistry(), deployState.getDeployLogger()); + UserConfiguredFiles files = new UserConfiguredFiles(deployState.getFileRegistry(), + deployState.getDeployLogger(), + userConfiguredUrls); for (Component<?, ?> component : getAllComponents()) { files.register(component); } @@ -382,4 +390,14 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat } } + public static class UserConfiguredUrls { + + private final Set<String> urls = new HashSet<>(); + + public void add(String url) { urls.add(url); } + + public Set<String> all() { return urls; } + + } + } 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..03541ecadf3 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 @@ -11,6 +11,7 @@ import com.yahoo.path.Path; import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigDefinitionKey; import com.yahoo.vespa.config.ConfigPayloadBuilder; + import com.yahoo.yolean.Exceptions; import java.io.File; @@ -21,6 +22,8 @@ import java.util.Map; import java.util.Optional; import java.util.logging.Level; +import static com.yahoo.vespa.model.container.ApplicationContainerCluster.UserConfiguredUrls; + /** * Utility methods for registering file distribution of files/paths/urls/models defined by the user. * @@ -30,10 +33,12 @@ public class UserConfiguredFiles implements Serializable { private final FileRegistry fileRegistry; private final DeployLogger logger; + private final UserConfiguredUrls userConfiguredUrls; - public UserConfiguredFiles(FileRegistry fileRegistry, DeployLogger logger) { + public UserConfiguredFiles(FileRegistry fileRegistry, DeployLogger logger, UserConfiguredUrls userConfiguredUrls) { this.fileRegistry = fileRegistry; this.logger = logger; + this.userConfiguredUrls = userConfiguredUrls; } /** @@ -133,7 +138,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 -> userConfiguredUrls.add(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(); + } + +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java index bb5ba840c2c..e2a25642575 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java @@ -13,6 +13,7 @@ import com.yahoo.vespa.config.ConfigDefinition; import com.yahoo.vespa.config.ConfigDefinitionKey; import com.yahoo.vespa.config.ConfigPayloadBuilder; import com.yahoo.vespa.model.SimpleConfigProducer; +import com.yahoo.vespa.model.container.ApplicationContainerCluster; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -68,7 +69,7 @@ public class UserConfiguredFilesTest { } private UserConfiguredFiles userConfiguredFiles() { - return new UserConfiguredFiles(fileRegistry, new BaseDeployLogger()); + return new UserConfiguredFiles(fileRegistry, new BaseDeployLogger(), new ApplicationContainerCluster.UserConfiguredUrls()); } @BeforeEach |