Improve validation of endpoint IDs

Same validation rules that are in place for `<rotations/>' in `services.xml`,
which we are removing.

3 files changed, 72 insertions, 3 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
index 73328027540..7d31e07dac5 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/Endpoint.java
@@ -5,6 +5,7 @@ import com.yahoo.config.provision.RegionName;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 /**
@@ -13,19 +14,37 @@ import java.util.stream.Collectors;
  * should point to.
  *
  * If the endpointId is not set, it will default to the same as the containerId.
+ *
+ * @author ogronnesby
  */
 public class Endpoint {
+
+    /*
+     * Endpoint IDs must be:
+     * - lowercase
+     * - alphanumeric
+     * - begin with a character
+     * - contain zero consecutive dashes
+     * - have a length between 1 and 12
+     */
+    private static final Pattern endpointPattern = Pattern.compile("^[a-z](?:-?[a-z0-9]+)*$");
+    private static final int endpointMaxLength = 12;
+
     private final Optional<String> endpointId;
     private final String containerId;
     private final Set<RegionName> regions;
 
     public Endpoint(Optional<String> endpointId, String containerId, Set<String> regions) {
-        this.endpointId = endpointId;
-        this.containerId = containerId;
+        this.endpointId = Objects.requireNonNull(endpointId, "endpointId must be non-null");
+        this.containerId = Objects.requireNonNull(containerId, "containerId must be non-null");
         this.regions = Set.copyOf(
                 Objects.requireNonNull(
                         regions.stream().map(RegionName::from).collect(Collectors.toList()),
                         "Missing 'regions' parameter"));
+
+        if (endpointId().length() > endpointMaxLength || !endpointPattern.matcher(endpointId()).matches()) {
+            throw new IllegalArgumentException("Invalid endpoint ID: '" + endpointId() + "'");
+        }
     }
 
     public String endpointId() {
@@ -54,4 +73,5 @@ public class Endpoint {
     public int hashCode() {
         return Objects.hash(endpointId, containerId, regions);
     }
+
 }
diff --git a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
index 58795f6ea9e..650f68591b6 100644
--- a/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
+++ b/config-model-api/src/main/java/com/yahoo/config/application/api/xml/DeploymentSpecXmlReader.java
@@ -183,7 +183,11 @@ public class DeploymentSpecXmlReader {
                 regions.add(region);
             }
 
-            endpoints.add(new Endpoint(rotationId, containerId.get(), regions));
+            var endpoint = new Endpoint(rotationId, containerId.get(), regions);
+            if (endpoints.contains(endpoint)) {
+                throw new IllegalArgumentException("Duplicate 'endpoint' in 'endpoints' tag");
+            }
+            endpoints.add(endpoint);
         }
 
         return endpoints;
diff --git a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
index 3049d511bf1..c161b04087f 100644
--- a/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
+++ b/config-model-api/src/test/java/com/yahoo/config/application/api/DeploymentSpecTest.java
@@ -493,4 +493,49 @@ public class DeploymentSpecTest {
 
         assertEquals(Set.of(RegionName.from("us-east")), spec.endpoints().get(0).regions());
     }
+    @Test
+    public void invalidEndpoints() {
+        assertInvalid("<endpoint id='FOO' container-id='qrs'/>"); // Uppercase
+        assertInvalid("<endpoint id='123' container-id='qrs'/>"); // Starting with non-character
+        assertInvalid("<endpoint id='foo!' container-id='qrs'/>"); // Non-alphanumeric
+        assertInvalid("<endpoint id='foo.bar' container-id='qrs'/>");
+        assertInvalid("<endpoint id='foo--bar' container-id='qrs'/>"); // Multiple consecutive dashes
+        assertInvalid("<endpoint id='foo-' container-id='qrs'/>"); // Trailing dash
+        assertInvalid("<endpoint id='foooooooooooo' container-id='qrs'/>"); // Too long
+        assertInvalid("<endpoint id='foo' container-id='qrs'/><endpoint id='foo' container-id='qrs'/>"); // Duplicate
+    }
+
+    @Test
+    public void validEndpoints() {
+        assertEquals(List.of("qrs"), endpointIds("<endpoint container-id='qrs'/>"));
+        assertEquals(List.of("qrs"), endpointIds("<endpoint id='' container-id='qrs'/>"));
+        assertEquals(List.of("f"), endpointIds("<endpoint id='f' container-id='qrs'/>"));
+        assertEquals(List.of("foo"), endpointIds("<endpoint id='foo' container-id='qrs'/>"));
+        assertEquals(List.of("foo-bar"), endpointIds("<endpoint id='foo-bar' container-id='qrs'/>"));
+        assertEquals(List.of("foo", "bar"), endpointIds("<endpoint id='foo' container-id='qrs'/><endpoint id='bar' container-id='qrs'/>"));
+        assertEquals(List.of("fooooooooooo"), endpointIds("<endpoint id='fooooooooooo' container-id='qrs'/>"));
+    }
+
+    private static void assertInvalid(String endpointTag) {
+        try {
+            endpointIds(endpointTag);
+            fail("Expected exception for input '" + endpointTag + "'");
+        } catch (IllegalArgumentException ignored) {}
+    }
+
+    private static List<String> endpointIds(String endpointTag) {
+        var xml = "<deployment>" +
+                  "  <prod>" +
+                  "    <region active=\"true\">us-east</region>" +
+                  "  </prod>" +
+                  "  <endpoints>" +
+                  endpointTag +
+                  "  </endpoints>" +
+                  "</deployment>";
+
+        return DeploymentSpec.fromXml(xml).endpoints().stream()
+                             .map(Endpoint::endpointId)
+                             .collect(Collectors.toList());
+    }
+
 }