Require override for endpoint cluster change

3 files changed, 31 insertions, 6 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 a7d2d7ef385..c2a5389000b 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
@@ -108,7 +108,7 @@ public class ApplicationPackageValidator {
         var newEndpoints = allEndpointsOf(applicationPackage.deploymentSpec().requireInstance(instanceName));
 
         if (newEndpoints.containsAll(endpoints)) return; // Adding new endpoints is fine
-        if (containsAllRegions(newEndpoints, endpoints)) return; // Adding regions to endpoints is fine
+        if (containsAllDestinationsOf(endpoints, newEndpoints)) return; // Adding destinations is fine
 
         var removedEndpoints = new ArrayList<>(endpoints);
         removedEndpoints.removeAll(newEndpoints);
@@ -122,19 +122,23 @@ public class ApplicationPackageValidator {
                                            ". " + ValidationOverrides.toAllowMessage(validationId));
     }
 
-    /** Returns whether endpoint regions in newEndpoints contains all regions of corresponding endpoint in endpoints */
-    private static boolean containsAllRegions(List<Endpoint> newEndpoints, List<Endpoint> endpoints) {
+    /** Returns whether newEndpoints contains all destinations in endpoints */
+    private static boolean containsAllDestinationsOf(List<Endpoint> endpoints, List<Endpoint> newEndpoints) {
         var containsAllRegions = true;
+        var hasSameCluster = true;
         for (var endpoint : endpoints) {
             var endpointContainsAllRegions = false;
+            var endpointHasSameCluster = false;
             for (var newEndpoint : newEndpoints) {
                 if (endpoint.endpointId().equals(newEndpoint.endpointId())) {
                     endpointContainsAllRegions = newEndpoint.regions().containsAll(endpoint.regions());
+                    endpointHasSameCluster = newEndpoint.containerId().equals(endpoint.containerId());
                 }
             }
             containsAllRegions &= endpointContainsAllRegions;
+            hasSameCluster &= endpointHasSameCluster;
         }
-        return containsAllRegions;
+        return containsAllRegions && hasSameCluster;
     }
 
     /** Returns all configued endpoints of given deployment instance spec */
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 875e067156d..ad8c7d7c328 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
@@ -906,13 +906,34 @@ public class ControllerTest {
         assertEquals(ClusterSpec.Id.from("foo"), tester.applications().requireInstance(context.instanceId())
                                                        .rotations().get(0).clusterId());
 
-        // Redeploy with endpoint cluster changed
+        // Redeploy with endpoint cluster changed needs override
         applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)
                 .endpoint("default", "bar")
                 .region(west.region().value())
                 .region(east.region().value())
                 .build();
+        try {
+            context.submit(applicationPackage).deploy();
+            fail("Expected exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("global-endpoint-change: application 'tenant.application' has endpoints [endpoint " +
+                         "'default' (cluster foo) -> us-east-3, us-west-1], but does not include all of these in " +
+                         "deployment.xml. Deploying given deployment.xml will remove " +
+                         "[endpoint 'default' (cluster foo) -> us-east-3, us-west-1] and add " +
+                         "[endpoint 'default' (cluster bar) -> us-east-3, us-west-1]. To allow this add " +
+                         "<allow until='yyyy-mm-dd'>global-endpoint-change</allow> to validation-overrides.xml, see " +
+                         "https://docs.vespa.ai/documentation/reference/validation-overrides.html", e.getMessage());
+        }
+
+        // Redeploy with override succeeds
+        applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.prod)
+                .endpoint("default", "bar")
+                .region(west.region().value())
+                .region(east.region().value())
+                .allow(ValidationId.globalEndpointChange)
+                .build();
         context.submit(applicationPackage).deploy();
         assertEquals(ClusterSpec.Id.from("bar"), tester.applications().requireInstance(context.instanceId())
                                                        .rotations().get(0).clusterId());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java
index d7a28708049..77567a58ed2 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/RotationStatusUpdaterTest.java
@@ -65,7 +65,7 @@ public class RotationStatusUpdaterTest {
                 .region(zone1.region().value())
                 .region(zone2.region().value())
                 .region(zone3.region().value())
-                .endpoint("default", "default", "us-east-3", "us-west-1")
+                .endpoint("default", "foo", "us-east-3", "us-west-1")
                 .endpoint("eu", "default", "eu-west-1")
                 .build();
         context.submit(applicationPackage)