Verify container cluster names in hsoted vespa

4 files changed, 86 insertions, 6 deletions

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<String, String> 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;
@@ -181,6 +181,63 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
     }
 
     @Test
+    void container_cluster_with_invalid_name_throws_exception_when_hosted() throws IOException, SAXException {
+        String servicesXml = """
+                             <services version='1.0'>
+                               <container id='C-1' version='1.0'>
+                                 <nodes count='1' />
+                               </container>
+                             </services>
+                             """;
+
+        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 = """
+                             <services version='1.0'>
+                               <container id='c-1' version='1.0'>
+                                 <nodes count='1' />
+                               </container>
+                               <container id='c_1' version='1.0'>
+                                 <nodes count='1' />
+                               </container>
+                             </services>
+                             """;
+
+        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(
                 "<container id='cluster1' version='1.0'>",