Refactor alias name generation

4 files changed, 29 insertions, 20 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java
index 0d1aff82bf2..33f937f6184 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicy.java
@@ -1,7 +1,6 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.application;
 
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSortedSet;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
@@ -9,11 +8,11 @@ import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.RotationName;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
@@ -24,8 +23,6 @@ import java.util.stream.Collectors;
  */
 public class RoutingPolicy {
 
-    private static final String ignoredEndpointPart = "default";
-
     private final ApplicationId owner;
     private final ZoneId zone;
     private final HostName alias;
@@ -33,6 +30,7 @@ public class RoutingPolicy {
     private final Optional<String> dnsZone;
     private final Set<RotationName> rotations;
 
+    /** DO NOT USE. Public for serialization purposes */
     public RoutingPolicy(ApplicationId owner, ZoneId zone, HostName alias, HostName canonicalName,
                          Optional<String> dnsZone, Set<RotationName> rotations) {
         this.owner = Objects.requireNonNull(owner, "owner must be non-null");
@@ -43,6 +41,11 @@ public class RoutingPolicy {
         this.rotations = ImmutableSortedSet.copyOf(Objects.requireNonNull(rotations, "rotations must be non-null"));
     }
 
+    public RoutingPolicy(ApplicationId owner, ZoneId zone, ClusterSpec.Id cluster, HostName canonicalName,
+                         Optional<String> dnsZone, Set<RotationName> rotations) {
+        this(owner, zone, HostName.from(aliasOf(cluster, owner, zone)), canonicalName, dnsZone, rotations);
+    }
+
     /** The application owning this */
     public ApplicationId owner() {
         return owner;
@@ -95,20 +98,21 @@ public class RoutingPolicy {
                              zone.value());
     }
 
-    public static String createAlias(ClusterSpec.Id clusterId, ApplicationId applicationId, ZoneId zoneId) {
-        List<String> parts = Arrays.asList(ignorePartIfDefault(clusterId.value()),
-                                           ignorePartIfDefault(applicationId.instance().value()),
-                                           applicationId.application().value(),
-                                           applicationId.tenant().value() +
-                                           "." + zoneId.value() + "." + "vespa.oath.cloud"
+    /** Returns the alias to use for the given application cluster in zone */
+    private static String aliasOf(ClusterSpec.Id cluster, ApplicationId application, ZoneId zone) {
+        List<String> parts = List.of(ignorePartIfDefault(cluster.value()),
+                                     ignorePartIfDefault(application.instance().value()),
+                                     application.application().value(),
+                                     application.tenant().value() +
+                                     "." + zone.value() + "." + "vespa.oath.cloud"
         );
         return parts.stream()
-                    .filter(s -> !Strings.isNullOrEmpty((s)))
+                    .filter(Predicate.not(String::isBlank))
                     .collect(Collectors.joining("--"));
     }
 
     private static String ignorePartIfDefault(String s) {
-        return ignoredEndpointPart.equalsIgnoreCase(s) ? "" : s;
+        return "default".equalsIgnoreCase(s) ? "" : s;
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java
index 80b03f50ebd..a79eef0244b 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/RoutingPolicyMaintainer.java
@@ -136,8 +136,10 @@ public class RoutingPolicyMaintainer extends Maintainer {
 
     /** Register DNS alias for given load balancer */
     private RoutingPolicy registerCname(ApplicationId application, ZoneId zone, LoadBalancer loadBalancer) {
-        HostName alias = HostName.from(RoutingPolicy.createAlias(loadBalancer.cluster(), application, zone));
-        RecordName name = RecordName.from(alias.value());
+        RoutingPolicy routingPolicy = new RoutingPolicy(application, zone, loadBalancer.cluster(),
+                                                        loadBalancer.hostname(), loadBalancer.dnsZone(),
+                                                        loadBalancer.rotations());
+        RecordName name = RecordName.from(routingPolicy.alias().value());
         RecordData data = RecordData.fqdn(loadBalancer.hostname().value());
         List<Record> existingRecords = nameService.findRecords(Record.Type.CNAME, name);
         if (existingRecords.size() > 1) {
@@ -151,8 +153,7 @@ public class RoutingPolicyMaintainer extends Maintainer {
         } else {
             nameService.createCname(name, data);
         }
-        return new RoutingPolicy(application, zone, alias, loadBalancer.hostname(), loadBalancer.dnsZone(),
-                                 loadBalancer.rotations());
+        return routingPolicy;
     }
 
     /** Remove all DNS records that point to non-existing load balancers */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicyTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicyTest.java
index 2e2ab3d9f67..8d0766829f1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicyTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/RoutingPolicyTest.java
@@ -3,11 +3,14 @@ package com.yahoo.vespa.hosted.controller.application;
 
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.HostName;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import org.junit.Test;
 
-import static com.yahoo.vespa.hosted.controller.application.RoutingPolicy.createAlias;
-import static org.junit.Assert.*;
+import java.util.Optional;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
 
 /**
  * @author mpolden
@@ -27,7 +30,8 @@ public class RoutingPolicyTest {
     }
 
     private void testAlias(String expected, String clusterName, ApplicationId applicationId, ZoneId zoneId) {
-        assertEquals(expected, createAlias(ClusterSpec.Id.from(clusterName), applicationId, zoneId));
+        assertEquals(expected, new RoutingPolicy(applicationId, zoneId, ClusterSpec.Id.from(clusterName),
+                                                 HostName.from("lb-0"), Optional.empty(), Set.of()).alias().value());
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index e2bf6a20684..e0657400a4b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -1292,7 +1292,7 @@ public class ApplicationApiTest extends ControllerContainerTest {
     public void applicationWithPerClusterGlobalRotation() {
         Application app = controllerTester.createApplication();
         RoutingPolicy policy = new RoutingPolicy(app.id(), ZoneId.from(Environment.prod, RegionName.from("us-west-1")),
-                                                 HostName.from("lb-0"), HostName.from("lb-0-canonical-name"),
+                                                 ClusterSpec.Id.from("default"), HostName.from("lb-0-canonical-name"),
                                                  Optional.of("dns-zone-1"), Set.of(RotationName.from("c0")));
         tester.controller().curator().writeRoutingPolicies(app.id(), Set.of(policy));