Merge pull request #19832 from vespa-engine/mpolden/remove-legacy-endpoint

Remove support for legacy endpoints in public

6 files changed, 43 insertions, 114 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 46c4d9d22b2..605f9f63724 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
@@ -72,7 +72,6 @@ public class RoutingController {
     private final RoutingPolicies routingPolicies;
     private final RotationRepository rotationRepository;
     private final BooleanFlag hideSharedRoutingEndpoint;
-    private final BooleanFlag legacyEndpointInCertificate;
 
     public RoutingController(Controller controller, RotationsConfig rotationsConfig) {
         this.controller = Objects.requireNonNull(controller, "controller must be non-null");
@@ -81,7 +80,6 @@ public class RoutingController {
                                                          controller.applications(),
                                                          controller.curator());
         this.hideSharedRoutingEndpoint = Flags.HIDE_SHARED_ROUTING_ENDPOINT.bindTo(controller.flagSource());
-        this.legacyEndpointInCertificate = Flags.LEGACY_ENDPOINT_IN_CERTIFICATE.bindTo(controller.flagSource());
     }
 
     public RoutingPolicies policies() {
@@ -178,13 +176,9 @@ public class RoutingController {
 
         // Build all endpoints
         for (var builder : builders) {
-            builder = builder.routingMethod(RoutingMethod.exclusive)
-                             .on(Port.tls());
-            Endpoint endpoint = builder.in(controller.system());
-            if (includeLegacyEndpoint(deployment.applicationId(), controller.system())) {
-                Endpoint legacyEndpoint = builder.legacy().in(controller.system());
-                endpointDnsNames.add(legacyEndpoint.dnsName());
-            }
+            Endpoint endpoint = builder.routingMethod(RoutingMethod.exclusive)
+                                       .on(Port.tls())
+                                       .in(controller.system());
             endpointDnsNames.add(endpoint.dnsName());
         }
         return Collections.unmodifiableList(endpointDnsNames);
@@ -356,14 +350,6 @@ public class RoutingController {
                                   .on(Port.fromRoutingMethod(method))
                                   .routingMethod(method)
                                   .in(controller.system()));
-            if (controller.system().isPublic()) {
-                endpoints.add(Endpoint.of(routingId.application())
-                                      .target(routingId.endpointId(), cluster, zones)
-                                      .on(Port.fromRoutingMethod(method))
-                                      .routingMethod(method)
-                                      .legacy()
-                                      .in(controller.system()));
-            }
             // Add legacy endpoints
             if (legacyNamesAvailable && method == RoutingMethod.shared) {
                 endpoints.add(Endpoint.of(routingId.application())
@@ -394,13 +380,7 @@ public class RoutingController {
     private String commonNameHashOf(ApplicationId application, SystemName system) {
         HashCode sha1 = Hashing.sha1().hashString(application.serializedForm(), StandardCharsets.UTF_8);
         String base32 = BaseEncoding.base32().omitPadding().lowerCase().encode(sha1.asBytes());
-        return 'v' + base32 + Endpoint.internalDnsSuffix(system, includeLegacyEndpoint(application, system));
-    }
-
-    private boolean includeLegacyEndpoint(ApplicationId application, SystemName system) {
-        return system.isPublic() && legacyEndpointInCertificate.with(FetchVector.Dimension.APPLICATION_ID,
-                                                                     application.serializedForm())
-                                                               .value();
+        return 'v' + base32 + Endpoint.internalDnsSuffix(system);
     }
 
     /** Returns direct routing endpoints if any exist and feature flag is set for given application */
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 a98e88210d2..e2d71fecc6b 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
@@ -32,8 +32,6 @@ public class Endpoint {
     private static final String OATH_DNS_SUFFIX = ".vespa.oath.cloud";
     private static final String PUBLIC_DNS_SUFFIX = ".vespa-app.cloud";
     private static final String PUBLIC_CD_DNS_SUFFIX = ".cd.vespa-app.cloud";
-    private static final String PUBLIC_DNS_LEGACY_SUFFIX = ".public.vespa.oath.cloud";
-    private static final String PUBLIC_CD_LEGACY_DNS_SUFFIX = ".public-cd.vespa.oath.cloud";
 
     private final EndpointId id;
     private final ClusterSpec.Id cluster;
@@ -206,21 +204,21 @@ public class Endpoint {
     }
 
     private static String scopePart(Scope scope, List<ZoneId> zones, SystemName system, boolean legacy) {
-        String scopeSymbol = scopeSymbol(scope, system, legacy);
+        String scopeSymbol = scopeSymbol(scope, system);
         if (scope == Scope.global) return scopeSymbol;
 
         ZoneId zone = zones.get(0);
         String region = zone.region().value();
         boolean skipEnvironment = zone.environment().isProduction() && (system.isPublic() || !legacy);
         String environment = skipEnvironment ? "" : "." + zone.environment().value();
-        if (system.isPublic() && !legacy) {
+        if (system.isPublic()) {
             return region + environment + "." + scopeSymbol;
         }
         return region + (scopeSymbol.isEmpty() ? "" : "-" + scopeSymbol) + environment;
     }
 
-    private static String scopeSymbol(Scope scope, SystemName system, boolean legacy) {
-        if (system.isPublic() && !legacy) {
+    private static String scopeSymbol(Scope scope, SystemName system) {
+        if (system.isPublic()) {
             switch (scope) {
                 case zone: return "z";
                 case regionSplit: return "w";
@@ -234,7 +232,7 @@ public class Endpoint {
             case region: return "r";
             case global: return "global";
         }
-        throw new IllegalArgumentException("No scope symbol defined for " + scope + " in " + system + " (legacy: " + legacy + ")");
+        throw new IllegalArgumentException("No scope symbol defined for " + scope + " in " + system);
     }
 
     private static String instancePart(Optional<InstanceName> instance, String separator) {
@@ -250,27 +248,26 @@ public class Endpoint {
     }
 
     /** Returns the DNS suffix used for endpoints in given system */
-    public static String dnsSuffix(SystemName system, boolean legacy) {
+    private static String dnsSuffix(SystemName system, boolean legacy) {
         switch (system) {
             case cd:
             case main:
                 if (legacy) return YAHOO_DNS_SUFFIX;
                 return OATH_DNS_SUFFIX;
             case Public:
-                if (legacy) return PUBLIC_DNS_LEGACY_SUFFIX;
+                if (legacy) throw new IllegalArgumentException("No legacy DNS suffix declared for system " + system);
                 return PUBLIC_DNS_SUFFIX;
             case PublicCd:
-                if (legacy) return PUBLIC_CD_LEGACY_DNS_SUFFIX;
+                if (legacy) throw new IllegalArgumentException("No legacy DNS suffix declared for system " + system);
                 return PUBLIC_CD_DNS_SUFFIX;
             default: throw new IllegalArgumentException("No DNS suffix declared for system " + system);
         }
     }
 
     /** Returns the DNS suffix used for internal names (i.e. names not exposed to tenants) in given system */
-    public static String internalDnsSuffix(SystemName system, boolean legacy) {
-        // TODO(mpolden): Stop exposing legacy parameter after legacy endpoints in public are completely removed
-        String suffix = dnsSuffix(system, legacy);
-        if (system.isPublic() && !legacy) {
+    public static String internalDnsSuffix(SystemName system) {
+        String suffix = dnsSuffix(system, false);
+        if (system.isPublic()) {
             // Certificate provider requires special approval for three-level DNS names, e.g. foo.vespa-app.cloud.
             // To avoid this in public we always add an extra level.
             return ".internal" + suffix;
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java
index d6c28e3f119..c15681e7424 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/routing/RoutingPolicy.java
@@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.controller.application.EndpointId;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
@@ -80,11 +79,8 @@ public class RoutingPolicy {
         if (infraEndpoint.isPresent()) {
             return List.of(infraEndpoint.get());
         }
-        List<Endpoint> endpoints = new ArrayList<>(3);
+        List<Endpoint> endpoints = new ArrayList<>();
         endpoints.add(endpoint(routingMethod).target(id.cluster(), id.zone()).in(system));
-        if (system.isPublic()) {
-            endpoints.add(endpoint(routingMethod).target(id.cluster(), id.zone()).legacy().in(system));
-        }
         // Add legacy endpoints
         if (routingMethod == RoutingMethod.shared) {
             endpoints.add(endpoint(routingMethod).target(id.cluster(), id.zone())
@@ -101,12 +97,7 @@ public class RoutingPolicy {
 
     /** Returns all region endpoints of this */
     public List<Endpoint> regionEndpointsIn(SystemName system, RoutingMethod routingMethod) {
-        List<Endpoint> endpoints = new ArrayList<>(2);
-        endpoints.add(regionEndpointIn(system, routingMethod, false));
-        if (system.isPublic()) {
-            endpoints.add(regionEndpointIn(system, routingMethod, true));
-        }
-        return Collections.unmodifiableList(endpoints);
+        return List.of(regionEndpointIn(system, routingMethod, false));
     }
 
     /** Returns the region endpoint of this */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java
index 9c0e886ee61..f9008e87b6b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/EndpointTest.java
@@ -66,17 +66,13 @@ public class EndpointTest {
                 "https://r2.i2.a2.t2.global.vespa.oath.cloud/",
                 Endpoint.of(instance2).target(EndpointId.of("r2")).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.main),
 
-                // Main endpoint in public system (legacy)
-                "https://a1.t1.global.public.vespa.oath.cloud/",
-                Endpoint.of(instance1).target(endpointId).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.Public)
+                // Main endpoint in public system
+                "https://a1.t1.g.vespa-app.cloud/",
+                Endpoint.of(instance1).target(endpointId).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.Public)
         );
         tests.forEach((expected, endpoint) -> assertEquals(expected, endpoint.url().toString()));
 
         Map<String, Endpoint> tests2 = Map.of(
-                // Main endpoint in public CD system (legacy)
-                "https://publiccd.a1.t1.global.public-cd.vespa.oath.cloud/",
-                Endpoint.of(instance1).target(endpointId).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.PublicCd),
-
                 // Default endpoint in public system
                 "https://a1.t1.g.vespa-app.cloud/",
                 Endpoint.of(instance1).target(endpointId).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.Public),
@@ -133,16 +129,16 @@ public class EndpointTest {
                 "https://r2.i2.a2.t2.global.vespa.oath.cloud/",
                 Endpoint.of(instance2).target(EndpointId.of("r2")).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.main),
 
-                // Main endpoint in public system (legacy)
-                "https://a1.t1.global.public.vespa.oath.cloud/",
-                Endpoint.of(instance1).target(endpointId).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.Public)
+                // Main endpoint in public system
+                "https://a1.t1.g.vespa-app.cloud/",
+                Endpoint.of(instance1).target(endpointId).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.Public)
         );
         tests.forEach((expected, endpoint) -> assertEquals(expected, endpoint.url().toString()));
 
         Map<String, Endpoint> tests2 = Map.of(
-                // Custom endpoint and instance in public CD system (legacy)
-                "https://foo.publiccd.i2.a2.t2.global.public-cd.vespa.oath.cloud/",
-                Endpoint.of(instance2).target(EndpointId.of("foo")).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.PublicCd),
+                // Custom endpoint and instance in public CD system)
+                "https://foo.i2.a2.t2.g.cd.vespa-app.cloud/",
+                Endpoint.of(instance2).target(EndpointId.of("foo")).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.PublicCd),
 
                 // Custom endpoint and instance in public system
                 "https://foo.i2.a2.t2.g.vespa-app.cloud/",
@@ -186,13 +182,13 @@ public class EndpointTest {
                 "https://i2--a2--t2.us-north-1.vespa.oath.cloud:4443/",
                 Endpoint.of(instance2).target(cluster, prodZone).on(Port.tls(4443)).in(SystemName.main),
 
-                // Non-default cluster in public (legacy)
-                "https://c1.a1.t1.us-north-1.public.vespa.oath.cloud/",
-                Endpoint.of(instance1).target(ClusterSpec.Id.from("c1"), prodZone).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.Public),
+                // Non-default cluster in public
+                "https://c1.a1.t1.us-north-1.z.vespa-app.cloud/",
+                Endpoint.of(instance1).target(ClusterSpec.Id.from("c1"), prodZone).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.Public),
 
-                // Non-default cluster and instance in public (legacy)
-                "https://c2.i2.a2.t2.us-north-1.public.vespa.oath.cloud/",
-                Endpoint.of(instance2).target(ClusterSpec.Id.from("c2"), prodZone).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.Public),
+                // Non-default cluster and instance in public
+                "https://c2.i2.a2.t2.us-north-1.z.vespa-app.cloud/",
+                Endpoint.of(instance2).target(ClusterSpec.Id.from("c2"), prodZone).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.Public),
 
                 // Endpoint in main using shared layer 4
                 "https://a1.t1.us-north-1.vespa.oath.cloud/",
@@ -202,8 +198,8 @@ public class EndpointTest {
 
         Map<String, Endpoint> tests2 = Map.of(
                 // Non-default cluster and instance in public CD (legacy)
-                "https://c2.publiccd.i2.a2.t2.us-north-1.public-cd.vespa.oath.cloud/",
-                Endpoint.of(instance2).target(ClusterSpec.Id.from("c2"), prodZone).on(Port.tls()).routingMethod(RoutingMethod.exclusive).legacy().in(SystemName.PublicCd),
+                "https://c2.i2.a2.t2.us-north-1.z.cd.vespa-app.cloud/",
+                Endpoint.of(instance2).target(ClusterSpec.Id.from("c2"), prodZone).on(Port.tls()).routingMethod(RoutingMethod.exclusive).in(SystemName.PublicCd),
 
                 // Custom cluster name in public
                 "https://c1.a1.t1.us-north-1.z.vespa-app.cloud/",
@@ -228,57 +224,43 @@ public class EndpointTest {
 
         var tests = Map.of(
                 // Default rotation
-                "https://a1.t1.global.public.vespa.oath.cloud/",
+                "https://a1.t1.g.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .target(EndpointId.defaultId())
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
 
                 // Wildcard to match other rotations
-                "https://*.a1.t1.global.public.vespa.oath.cloud/",
+                "https://*.a1.t1.g.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .wildcard()
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
 
                 // Default cluster in zone
-                "https://a1.t1.us-north-1.public.vespa.oath.cloud/",
+                "https://a1.t1.us-north-1.z.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .target(defaultCluster, prodZone)
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
-                        .in(SystemName.Public),
-
-                // Wildcard to match other clusters in zone
-                "https://*.a1.t1.us-north-1.public.vespa.oath.cloud/",
-                Endpoint.of(instance1)
-                        .wildcard(prodZone)
-                        .routingMethod(RoutingMethod.exclusive)
-                        .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
 
                 // Default cluster in test zone
-                "https://a1.t1.us-north-2.test.public.vespa.oath.cloud/",
+                "https://a1.t1.us-north-2.test.z.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .target(defaultCluster, testZone)
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
 
                 // Wildcard to match other clusters in test zone
-                "https://*.a1.t1.us-north-2.test.public.vespa.oath.cloud/",
+                "https://*.a1.t1.us-north-2.test.z.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .wildcard(testZone)
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
 
                 // Wildcard to match other clusters in zone
@@ -327,26 +309,17 @@ public class EndpointTest {
         var cluster = ClusterSpec.Id.from("default");
         var prodZone = ZoneId.from("prod", "us-north-2");
         Map<String, Endpoint> tests = Map.of(
-                "https://a1.t1.us-north-1-w.public.vespa.oath.cloud/",
+                "https://a1.t1.us-north-1.w.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .targetRegionSplit(cluster, ZoneId.from("prod", "us-north-1a"))
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
-                "https://a1.t1.us-north-2-w.public.vespa.oath.cloud/",
+                "https://a1.t1.us-north-2.w.vespa-app.cloud/",
                 Endpoint.of(instance1)
                         .targetRegionSplit(cluster, prodZone)
                         .routingMethod(RoutingMethod.exclusive)
                         .on(Port.tls())
-                        .legacy()
-                        .in(SystemName.Public),
-                "https://a1.t1.us-north-2-w.test.public.vespa.oath.cloud/",
-                Endpoint.of(instance1)
-                        .targetRegionSplit(cluster, ZoneId.from("test", "us-north-2"))
-                        .routingMethod(RoutingMethod.exclusive)
-                        .on(Port.tls())
-                        .legacy()
                         .in(SystemName.Public),
                 "https://c1.a1.t1.us-north-2.w.vespa-app.cloud/",
                 Endpoint.of(instance1)
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
index 2d1b50db1b0..3c8768ab09b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/RoutingPoliciesTest.java
@@ -363,16 +363,11 @@ public class RoutingPoliciesTest {
                              ClusterSpec.Id.from("default"), 0,
                              Map.of(zone1, 1L, zone2, 1L), true);
         assertEquals("Registers expected DNS names",
-                     Set.of("app1.tenant1.aws-eu-west-1-w.public.vespa.oath.cloud",
-                            "app1.tenant1.aws-eu-west-1.w.vespa-app.cloud",
-                            "app1.tenant1.aws-eu-west-1a.public.vespa.oath.cloud",
+                     Set.of("app1.tenant1.aws-eu-west-1.w.vespa-app.cloud",
                             "app1.tenant1.aws-eu-west-1a.z.vespa-app.cloud",
-                            "app1.tenant1.aws-us-east-1-w.public.vespa.oath.cloud",
                             "app1.tenant1.aws-us-east-1.w.vespa-app.cloud",
-                            "app1.tenant1.aws-us-east-1c.public.vespa.oath.cloud",
                             "app1.tenant1.aws-us-east-1c.z.vespa-app.cloud",
-                            "app1.tenant1.g.vespa-app.cloud",
-                            "app1.tenant1.global.public.vespa.oath.cloud"),
+                            "app1.tenant1.g.vespa-app.cloud"),
                      tester.recordNames());
     }
 
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 67c4d3d4218..c1feadf2d43 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -346,13 +346,6 @@ public class Flags {
             "Takes effect on restart of Docker container",
             APPLICATION_ID);
 
-    public static final UnboundBooleanFlag LEGACY_ENDPOINT_IN_CERTIFICATE = defineFeatureFlag(
-            "legacy-endpoint-in-certificate", false,
-            List.of("mpolden"), "2021-10-26", "2021-12-01",
-            "Whether to include legacy endpoint names in issued certificates",
-            "Takes effect on deployment through controller",
-            APPLICATION_ID);
-
     /** WARNING: public for testing: All flags should be defined in {@link Flags}. */
     public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners,
                                                        String createdAt, String expiresAt, String description,