Reject endpoints with clashing non-compactable IDs

2 files changed, 76 insertions, 0 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java
index 3e72936575d..bb2d8b3c553 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationPackageValidator.java
@@ -18,8 +18,10 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentSteps;
 
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
@@ -46,6 +48,7 @@ public class ApplicationPackageValidator {
         validateSteps(applicationPackage.deploymentSpec());
         validateEndpointRegions(applicationPackage.deploymentSpec());
         validateEndpointChange(application, applicationPackage, instant);
+        validateCompactedEndpoint(applicationPackage);
         validateSecurityClientsPem(applicationPackage);
     }
 
@@ -98,6 +101,25 @@ public class ApplicationPackageValidator {
                                                                                                    instant));
     }
 
+    /** Verify that compactable endpoint parts (instance aname nd endpoint ID) do not clash */
+    private void validateCompactedEndpoint(ApplicationPackage applicationPackage) {
+        Map<List<String>, InstanceEndpoint> instanceEndpoints = new HashMap<>();
+        for (var instanceSpec : applicationPackage.deploymentSpec().instances()) {
+            for (var endpoint : instanceSpec.endpoints()) {
+                List<String> nonCompactableIds = nonCompactableIds(instanceSpec.name(), endpoint);
+                InstanceEndpoint instanceEndpoint = new InstanceEndpoint(instanceSpec.name(), endpoint.endpointId());
+                InstanceEndpoint existingEndpoint = instanceEndpoints.get(nonCompactableIds);
+                if (existingEndpoint != null) {
+                    throw new IllegalArgumentException("Endpoint with ID '" + endpoint.endpointId() + "' in instance '"
+                                                       + instanceSpec.name().value() +
+                                                       "' clashes with endpoint '" + existingEndpoint.endpointId +
+                                                       "' in instance '" + existingEndpoint.instance + "'");
+                }
+                instanceEndpoints.put(nonCompactableIds, instanceEndpoint);
+            }
+        }
+    }
+
     /** Verify changes to endpoint configuration by comparing given application package to the existing one, if any */
     private void validateEndpointChange(Application application, InstanceName instanceName, ApplicationPackage applicationPackage, Instant instant) {
         var validationId = ValidationId.globalEndpointChange;
@@ -161,4 +183,28 @@ public class ApplicationPackageValidator {
         });
     }
 
+    /** Returns a list of the non-compactable IDs of given instance and endpoint */
+    private static List<String> nonCompactableIds(InstanceName instance, Endpoint endpoint) {
+        List<String> ids = new ArrayList<>(2);
+        if (!instance.isDefault()) {
+            ids.add(instance.value());
+        }
+        if (!"default".equals(endpoint.endpointId())) {
+            ids.add(endpoint.endpointId());
+        }
+        return ids;
+    }
+
+    private static class InstanceEndpoint {
+
+        private final InstanceName instance;
+        private final String endpointId;
+
+        public InstanceEndpoint(InstanceName instance, String endpointId) {
+            this.instance = instance;
+            this.endpointId = endpointId;
+        }
+
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index a3b22ccd649..569c22d8bf6 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -977,4 +977,34 @@ public class ControllerTest {
         app1.submit().deploy();
     }
 
+    @Test
+    public void testClashingEndpointIdAndInstanceName() {
+        String deploymentXml = "<deployment version='1.0' athenz-domain='domain' athenz-service='service'>\n" +
+                               "  <instance id=\"default\">\n" +
+                               "    <prod>\n" +
+                               "      <region active=\"true\">us-west-1</region>\n" +
+                               "    </prod>\n" +
+                               "    <endpoints>\n" +
+                               "      <endpoint id=\"dev\" container-id=\"qrs\"/>\n" +
+                               "    </endpoints>\n" +
+                               "  </instance>\n" +
+                               "  <instance id=\"dev\">\n" +
+                               "    <prod>\n" +
+                               "      <region active=\"true\">us-west-1</region>\n" +
+                               "    </prod>\n" +
+                               "    <endpoints>\n" +
+                               "      <endpoint id=\"default\" container-id=\"qrs\"/>\n" +
+                               "    </endpoints>\n" +
+                               "  </instance>\n" +
+                               "</deployment>\n";
+        ApplicationPackage applicationPackage = ApplicationPackageBuilder.fromDeploymentXml(deploymentXml);
+        try {
+            tester.newDeploymentContext().submit(applicationPackage);
+            fail("Expected exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Endpoint with ID 'default' in instance 'dev' clashes with endpoint 'dev' in instance 'default'",
+                         e.getMessage());
+        }
+    }
+
 }