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

Disallow application endpoints with shared routing method

3 files changed, 31 insertions, 14 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);
         });
     }