From 221e259351b0f88547a0b96d693d580ba827a397 Mon Sep 17 00:00:00 2001 From: jonmv Date: Wed, 18 Jan 2023 09:16:19 +0100 Subject: Verify container cluster names in hsoted vespa --- .../com/yahoo/config/model/ConfigModelRepo.java | 2 +- .../vespa/model/builder/VespaModelBuilder.java | 2 +- .../model/builder/xml/dom/VespaDomBuilder.java | 29 +++++++++-- .../container/xml/ContainerModelBuilderTest.java | 59 +++++++++++++++++++++- 4 files changed, 86 insertions(+), 6 deletions(-) (limited to 'config-model') diff --git a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java index 95d97bc9e87..756646beddb 100644 --- a/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java +++ b/config-model/src/main/java/com/yahoo/config/model/ConfigModelRepo.java @@ -80,7 +80,7 @@ public class ConfigModelRepo implements ConfigModelRepoAdder, Serializable, Iter ConfigModelRegistry configModelRegistry) throws IOException { Element userServicesElement = getServicesFromApp(deployState.getApplicationPackage()); readConfigModels(root, userServicesElement, deployState, vespaModel, configModelRegistry); - builder.postProc(deployState.getDeployLogger(), root, this); + builder.postProc(deployState, root, this); } private Element getServicesFromApp(ApplicationPackage applicationPackage) throws IOException { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java index 2cf32f1e8ff..421e3a2902c 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/VespaModelBuilder.java @@ -22,6 +22,6 @@ public abstract class VespaModelBuilder { * @param producerRoot the root producer. * @param configModelRepo a {@link com.yahoo.config.model.ConfigModelRepo instance} */ - public abstract void postProc(DeployLogger deployLogger, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); + public abstract void postProc(DeployState deployState, AbstractConfigProducer producerRoot, ConfigModelRepo configModelRepo); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java index cb3c43074fc..a5ff77dc1eb 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/VespaDomBuilder.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.model.builder.xml.dom; +import ai.vespa.validation.Validation; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.ApplicationConfigProducerRoot; import com.yahoo.config.model.ConfigModelRepo; @@ -22,8 +23,15 @@ import com.yahoo.vespa.model.container.docproc.ContainerDocproc; import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.search.SearchCluster; import org.w3c.dom.Element; + +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Pattern; /** * Builds Vespa model components using the w3c dom api @@ -43,10 +51,12 @@ public class VespaDomBuilder extends VespaModelBuilder { public static final String VESPAMALLOC = "vespamalloc"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG = "vespamalloc-debug"; // Intended for vespa engineers public static final String VESPAMALLOC_DEBUG_STACKTRACE = "vespamalloc-debug-stacktrace"; // Intended for vespa engineers - private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; public static final String CPU_SOCKET_AFFINITY_ATTRIB_NAME = "cpu-socket-affinity"; public static final String Allocated_MEMORY_ATTRIB_NAME = "allocated-memory"; + private static final String CPU_SOCKET_ATTRIB_NAME = "cpu-socket"; + private static final Pattern clusterPattern = Pattern.compile("([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])"); + public static final Logger log = Logger.getLogger(VespaDomBuilder.class.getPackage().toString()); @@ -232,13 +242,14 @@ public class VespaDomBuilder extends VespaModelBuilder { * @param root root config producer * @param configModelRepo a {@link ConfigModelRepo} */ - public void postProc(DeployLogger deployLogger, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { + public void postProc(DeployState deployState, AbstractConfigProducer root, ConfigModelRepo configModelRepo) { setContentSearchClusterIndexes(configModelRepo); createDocprocMBusServersAndClients(configModelRepo); + if (deployState.isHosted()) validateContainerClusterIds(configModelRepo); } private void createDocprocMBusServersAndClients(ConfigModelRepo pc) { - for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { + for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { addServerAndClientsForChains(cluster.getDocproc()); } } @@ -248,6 +259,18 @@ public class VespaDomBuilder extends VespaModelBuilder { docproc.getChains().addServersAndClientsForChains(); } + private void validateContainerClusterIds(ConfigModelRepo pc) { + Map normalizedClusterIds = new LinkedHashMap<>(); + for (ContainerCluster cluster: ContainerModel.containerClusters(pc)) { + String name = cluster.getName(); + Validation.requireMatch(name, "container cluster name", clusterPattern); + String clashing = normalizedClusterIds.put(name.replaceAll("_", "-"), name); + if (clashing != null) throw new IllegalArgumentException("container clusters '" + clashing + "' and '" + name + + "' have clashing endpoint names, when '_' is replaced " + + "with '-' to form valid domain names"); + } + } + /** * For some reason, search clusters need to be enumerated. * @param configModelRepo a {@link ConfigModelRepo} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java index a5b396508ef..f0c39ecc920 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java @@ -23,8 +23,8 @@ import com.yahoo.config.provision.Flavor; import com.yahoo.config.provision.RegionName; import com.yahoo.config.provision.Zone; import com.yahoo.config.provision.ZoneEndpoint; -import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provision.ZoneEndpoint.AccessType; +import com.yahoo.config.provision.ZoneEndpoint.AllowedUrn; import com.yahoo.config.provisioning.FlavorsConfig; import com.yahoo.container.ComponentsConfig; import com.yahoo.container.QrConfig; @@ -180,6 +180,63 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase { createModel(root, cluster1Elem, cluster2Elem); } + @Test + void container_cluster_with_invalid_name_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + + + + + + """; + + assertEquals("container cluster name must match '([a-z0-9]|[a-z0-9][a-z0-9_-]{0,61}[a-z0-9])', but got: 'C-1'", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + + @Test + void two_clusters_with_clashing_cluster_names_throws_exception_when_hosted() throws IOException, SAXException { + String servicesXml = """ + + + + + + + + + """; + + assertEquals("container clusters 'c-1' and 'c_1' have clashing endpoint names, when '_' is replaced with '-' to form valid domain names", + assertThrows(IllegalArgumentException.class, + () -> + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(true)) + .build())) + .getMessage()); + + new VespaModel(new NullConfigModelRegistry(), new DeployState.Builder() + .modelHostProvisioner(new InMemoryProvisioner(4, false)) + .applicationPackage(new MockApplicationPackage.Builder().withServices(servicesXml).build()) + .properties(new TestProperties().setHostedVespa(false)) + .build()); + } + @Test void two_clusters_without_explicit_port_throws_exception() { Element cluster1Elem = DomBuilderTest.parse( -- cgit v1.2.3