From 9ddc6b3541e59a20c8c58807518703d4187840b8 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Sat, 23 Sep 2023 10:08:52 +0200 Subject: Revert "Validate use of s3 urls in config" --- .../application/validation/UrlConfigValidator.java | 52 ---------- .../model/application/validation/Validation.java | 1 - .../filedistribution/UserConfiguredFiles.java | 5 +- .../validation/UrlConfigValidatorTest.java | 107 --------------------- 4 files changed, 1 insertion(+), 164 deletions(-) delete mode 100644 config-model/src/main/java/com/yahoo/vespa/model/application/validation/UrlConfigValidator.java delete mode 100644 config-model/src/test/java/com/yahoo/vespa/model/application/validation/UrlConfigValidatorTest.java 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 """ - - - - - - - - - - - - """.formatted(Boolean.toString(isExclusive)); - } - - private static void runValidatorOnApp(boolean isExclusive, SystemName systemName) throws IOException, SAXException { - String container = containerXml(isExclusive); - String servicesXml = """ - - %s - - """.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 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 getConfigDefinitions() { - return defs; - } - - @Override - public com.yahoo.vespa.config.buildergen.ConfigDefinition get(ConfigDefinitionKey key) { - return defs.get(key); - } - }); - return builder.build(); - } - -} -- cgit v1.2.3