Merge pull request #20118 from vespa-engine/mpolden/app-endpoint-requires-direct-routing

Disallow application endpoints with shared routing method

6 files changed, 47 insertions, 30 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
index 3794b69c023..2f5b92ca4c1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/RoutingController.java
@@ -145,16 +145,17 @@ public class RoutingController {
                                                                      .collect(Collectors.toMap(t -> new DeploymentId(application.id().instance(t.instance()),
                                                                                                                      ZoneId.from(Environment.prod, t.region())),
                                                                                                t -> t.weight()));
-            List<RoutingMethod> availableRoutingMethods = routingMethodsOfAll(deployments.keySet(), deploymentSpec);
-            for (var routingMethod : availableRoutingMethods) {
-                endpoints.add(Endpoint.of(application.id())
-                                      .targetApplication(EndpointId.of(declaredEndpoint.endpointId()),
-                                                         ClusterSpec.Id.from(declaredEndpoint.containerId()),
-                                                         deployments)
-                                      .routingMethod(routingMethod)
-                                      .on(Port.fromRoutingMethod(routingMethod))
-                                      .in(controller.system()));
-            }
+            // An application endpoint can only target a single zone, so we just pick the zone of any deployment target
+            ZoneId zone = deployments.keySet().iterator().next().zoneId();
+            // Application endpoints are only supported when using direct routing methods
+            RoutingMethod routingMethod = usesSharedRouting(zone) ? RoutingMethod.sharedLayer4 : RoutingMethod.exclusive;
+            endpoints.add(Endpoint.of(application.id())
+                                  .targetApplication(EndpointId.of(declaredEndpoint.endpointId()),
+                                                     ClusterSpec.Id.from(declaredEndpoint.containerId()),
+                                                     deployments)
+                                  .routingMethod(routingMethod)
+                                  .on(Port.fromRoutingMethod(routingMethod))
+                                  .in(controller.system()));
         }
         return EndpointList.copyOf(endpoints);
     }
@@ -354,6 +355,10 @@ public class RoutingController {
                                                                        Priority.normal));
     }
 
+    private boolean usesSharedRouting(ZoneId zone) {
+        return controller.zoneRegistry().routingMethods(zone).stream().anyMatch(RoutingMethod::isShared);
+    }
+
     /** Returns the routing methods that are available across all given deployments */
     private List<RoutingMethod> routingMethodsOfAll(Collection<DeploymentId> deployments, DeploymentSpec deploymentSpec) {
         var deploymentsByMethod = new HashMap<RoutingMethod, Set<DeploymentId>>();
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java
index c736863a57e..aee7c1052be 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Endpoint.java
@@ -60,7 +60,7 @@ public class Endpoint {
         this.instance = requireInstance(instanceName, scope);
         this.url = url;
         this.targets = List.copyOf(requireTargets(targets, application, instanceName, scope, certificateName));
-        this.scope = scope;
+        this.scope = requireScope(scope, routingMethod);
         this.legacy = legacy;
         this.routingMethod = routingMethod;
         this.tls = port.tls;
@@ -329,6 +329,11 @@ public class Endpoint {
         return instanceName;
     }
 
+    private static Scope requireScope(Scope scope, RoutingMethod routingMethod) {
+        if (scope == Scope.application && !routingMethod.isDirect()) throw new IllegalArgumentException("Routing method " + routingMethod + " does not support " + scope + "-scoped endpoints");
+        return scope;
+    }
+
     private static List<Target> requireTargets(List<Target> targets, TenantAndApplicationId application, Optional<InstanceName> instanceName, Scope scope, boolean certificateName) {
         if (!certificateName && targets.isEmpty()) throw new IllegalArgumentException("At least one target must be given for " + scope + " endpoints");
         if (scope == Scope.zone && targets.size() != 1) throw new IllegalArgumentException("Exactly one target must be given for " + scope + " endpoints");
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
index 2a39ed08014..634d76c8449 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicies.java
@@ -215,7 +215,7 @@ public class RoutingPolicies {
 
         Application application = controller.applications().requireApplication(routingTable.keySet().iterator().next().application());
         Map<DeploymentId, Map<EndpointId, Integer>> targetWeights = targetWeights(application);
-        Map<String, Set<AliasTarget>> targetsByEndpoint = new LinkedHashMap<>();
+        Map<Endpoint, Set<AliasTarget>> targetsByEndpoint = new LinkedHashMap<>();
         for (Map.Entry<RoutingId, List<RoutingPolicy>> routeEntry : routingTable.entrySet()) {
             RoutingId routingId = routeEntry.getKey();
             EndpointList endpoints = controller.routing().declaredEndpointsOf(application)
@@ -236,13 +236,20 @@ public class RoutingPolicies {
                     }
                     WeightedAliasTarget weightedAliasTarget = new WeightedAliasTarget(policy.canonicalName(), policy.dnsZone().get(),
                                                                                       target.deployment().zoneId(), weight);
-                    targetsByEndpoint.computeIfAbsent(endpoint.dnsName(), (k) -> new LinkedHashSet<>())
+                    targetsByEndpoint.computeIfAbsent(endpoint, (k) -> new LinkedHashSet<>())
                                      .add(weightedAliasTarget);
                 }
             }
         }
         targetsByEndpoint.forEach((applicationEndpoint, targets) -> {
-            controller.nameServiceForwarder().createAlias(RecordName.from(applicationEndpoint), targets, Priority.normal);
+            ZoneId targetZone = applicationEndpoint.targets().stream()
+                                             .map(Endpoint.Target::deployment)
+                                             .map(DeploymentId::zoneId)
+                                             .findFirst()
+                                             .get();
+            nameServiceForwarderIn(targetZone).createAlias(RecordName.from(applicationEndpoint.dnsName()),
+                                                           targets,
+                                                           Priority.normal);
         });
     }
 
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 9472801ef2c..be180f27af6 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
@@ -625,13 +625,13 @@ public class ControllerTest {
                 .instances("beta,main")
                 .region("us-west-1")
                 .region("us-east-3")
-                .applicationEndpoint("a", "qrs", "us-west-1",
+                .applicationEndpoint("a", "default", "us-west-1",
                                      Map.of(InstanceName.from("beta"), 2,
                                             InstanceName.from("main"), 8))
-                .applicationEndpoint("b", "qrs", "us-west-1",
+                .applicationEndpoint("b", "default", "us-west-1",
                                      Map.of(InstanceName.from("beta"), 1,
                                             InstanceName.from("main"), 1))
-                .applicationEndpoint("c", "qrs", "us-east-3",
+                .applicationEndpoint("c", "default", "us-east-3",
                                      Map.of(InstanceName.from("beta"), 4,
                                             InstanceName.from("main"), 6))
                 .build();
@@ -640,11 +640,11 @@ public class ControllerTest {
         // Endpoint names are passed to each deployment
         DeploymentId usWest = context.deploymentIdIn(ZoneId.from("prod", "us-west-1"));
         DeploymentId usEast = context.deploymentIdIn(ZoneId.from("prod", "us-east-3"));
-        Map<DeploymentId, List<String>> deploymentEndpoints = Map.of(usWest, List.of("a--app1--tenant1.us-west-1-r.vespa.oath.cloud", "b--app1--tenant1.us-west-1-r.vespa.oath.cloud"),
-                                                                     usEast, List.of("c--app1--tenant1.us-east-3-r.vespa.oath.cloud"));
+        Map<DeploymentId, List<String>> deploymentEndpoints = Map.of(usWest, List.of("a.app1.tenant1.us-west-1-r.vespa.oath.cloud", "b.app1.tenant1.us-west-1-r.vespa.oath.cloud"),
+                                                                     usEast, List.of("c.app1.tenant1.us-east-3-r.vespa.oath.cloud"));
         deploymentEndpoints.forEach((zone, endpointNames) -> {
             assertEquals("Endpoint names are passed to config server in " + zone,
-                         Set.of(new ContainerEndpoint("qrs", "application",
+                         Set.of(new ContainerEndpoint("default", "application",
                                                       endpointNames)),
                          tester.configServer().containerEndpoints().get(zone));
         });
@@ -653,21 +653,21 @@ public class ControllerTest {
         // DNS records are created for each endpoint
         Set<Record> records = tester.controllerTester().nameService().records();
         assertEquals(Set.of(new Record(Record.Type.CNAME,
-                                       RecordName.from("a--app1--tenant1.us-west-1-r.vespa.oath.cloud"),
+                                       RecordName.from("a.app1.tenant1.us-west-1-r.vespa.oath.cloud"),
                                        RecordData.from("vip.prod.us-west-1.")),
                             new Record(Record.Type.CNAME,
-                                       RecordName.from("b--app1--tenant1.us-west-1-r.vespa.oath.cloud"),
+                                       RecordName.from("b.app1.tenant1.us-west-1-r.vespa.oath.cloud"),
                                        RecordData.from("vip.prod.us-west-1.")),
                             new Record(Record.Type.CNAME,
-                                       RecordName.from("c--app1--tenant1.us-east-3-r.vespa.oath.cloud"),
+                                       RecordName.from("c.app1.tenant1.us-east-3-r.vespa.oath.cloud"),
                                        RecordData.from("vip.prod.us-east-3."))),
                      records);
         List<String> endpointDnsNames = tester.controller().routing().declaredEndpointsOf(context.application())
                                               .scope(Endpoint.Scope.application)
                                               .mapToList(Endpoint::dnsName);
-        assertEquals(List.of("a--app1--tenant1.us-west-1-r.vespa.oath.cloud",
-                             "b--app1--tenant1.us-west-1-r.vespa.oath.cloud",
-                             "c--app1--tenant1.us-east-3-r.vespa.oath.cloud"),
+        assertEquals(List.of("a.app1.tenant1.us-west-1-r.vespa.oath.cloud",
+                             "b.app1.tenant1.us-west-1-r.vespa.oath.cloud",
+                             "c.app1.tenant1.us-east-3-r.vespa.oath.cloud"),
                      endpointDnsNames);
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json
index fb6088f54b8..ab2a3bf945c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/deployment.json
@@ -24,9 +24,9 @@
     {
       "cluster": "foo",
       "tls": true,
-      "url": "https://a0--application1--tenant1.us-central-1-r.vespa.oath.cloud:4443/",
+      "url": "https://a0.application1.tenant1.us-central-1-r.vespa.oath.cloud/",
       "scope": "application",
-      "routingMethod": "shared",
+      "routingMethod": "sharedLayer4",
       "legacy": false
     }
   ],
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json
index 409e97b063c..62ad3a2db7e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/prod-us-central-1.json
@@ -27,9 +27,9 @@
     {
       "cluster": "foo",
       "tls": true,
-      "url": "https://a0--application1--tenant1.us-central-1-r.vespa.oath.cloud:4443/",
+      "url": "https://a0.application1.tenant1.us-central-1-r.vespa.oath.cloud/",
       "scope": "application",
-      "routingMethod": "shared",
+      "routingMethod": "sharedLayer4",
       "legacy": false
     }
   ],