Merge pull request #4721 from vespa-engine/jvenstad/secure-endpoints

Pass new secure global endpoint in deployment

7 files changed, 116 insertions, 55 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 7b53720954b..c79b467568a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -38,7 +38,6 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingEndpoint
 import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerator;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationRevision;
-import com.yahoo.vespa.hosted.controller.application.ApplicationRotation;
 import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
@@ -63,6 +62,7 @@ import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -324,7 +324,7 @@ public class ApplicationController {
             }
 
             if ( ! canDeployDirectlyTo(zone, options)) { // validate automated deployment
-                if (!application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying()))
+                if ( ! application.deploymentJobs().isDeployableTo(zone.environment(), application.deploying()))
                     throw new IllegalArgumentException("Rejecting deployment of " + application + " to " + zone +
                                                        " as " + application.deploying().get() + " is not tested");
                 Deployment existingDeployment = application.deployments().get(zone);
@@ -334,33 +334,20 @@ public class ApplicationController {
                                                        " the current version " + existingDeployment.version());
             }
 
-            Optional<Rotation> rotation;
-            try (RotationLock rotationLock = rotationRepository.lock()) {
-                rotation = getRotation(application, zone, rotationLock);
-                if (rotation.isPresent()) {
-                    application = application.with(rotation.get().id());
-                    store(application); // store assigned rotation even if deployment fails
-                    registerRotationInDns(rotation.get(), application.rotation().get().dnsName());
-                }
-            }
-
-            // TODO: Improve config server client interface and simplify
-            Set<String> cnames = application.rotation()
-                                            .map(ApplicationRotation::dnsName)
-                                            .map(Collections::singleton)
-                                            .orElseGet(Collections::emptySet);
+            application = withRotation(application, zone);
 
-            Set<com.yahoo.vespa.hosted.controller.api.rotation.Rotation> rotations = rotation
-                    .map(r -> new com.yahoo.vespa.hosted.controller.api.rotation.Rotation(
-                            new com.yahoo.vespa.hosted.controller.api.identifiers.RotationId(
-                                    r.id().asString()), r.name()))
-                    .map(Collections::singleton)
-                    .orElseGet(Collections::emptySet);
+            Set<String> rotationNames = new HashSet<>();
+            Set<String> cnames = new HashSet<>();
+            application.rotation().ifPresent(applicationRotation -> {
+                rotationNames.add(applicationRotation.id().asString());
+                cnames.add(applicationRotation.dnsName());
+                cnames.add(applicationRotation.secureDnsName());
+            });
 
             // Carry out deployment
             options = withVersion(version, options);
             ConfigServerClient.PreparedApplication preparedApplication =
-                    configserverClient.prepare(new DeploymentId(applicationId, zone), options, cnames, rotations,
+                    configserverClient.prepare(new DeploymentId(applicationId, zone), options, cnames, rotationNames,
                                                applicationPackage.zippedContent());
             preparedApplication.activate();
             application = application.withNewDeployment(zone, revision, version, clock.instant());
@@ -371,6 +358,21 @@ public class ApplicationController {
         }
     }
 
+    /** Makes sure the application has a global rotation, if eligible. */
+    private LockedApplication withRotation(LockedApplication application, ZoneId zone) {
+        if (zone.environment() == Environment.prod && application.deploymentSpec().globalServiceId().isPresent()) {
+            try (RotationLock rotationLock = rotationRepository.lock()) {
+                Rotation rotation = rotationRepository.getRotation(application, rotationLock);
+                application = application.with(rotation.id());
+                store(application); // store assigned rotation even if deployment fails
+
+                registerRotationInDns(rotation, application.rotation().get().dnsName());
+                registerRotationInDns(rotation, application.rotation().get().secureDnsName());
+            }
+        }
+        return application;
+    }
+
     private ActivateResult unexpectedDeployment(ApplicationId applicationId, ZoneId zone, ApplicationPackage applicationPackage) {
         Log logEntry = new Log();
         logEntry.level = "WARNING";
@@ -461,7 +463,7 @@ public class ApplicationController {
             RecordData rotationName = RecordData.fqdn(rotation.name());
             if (record.isPresent()) {
                 // Ensure that the existing record points to the correct rotation
-                if (!record.get().data().equals(rotationName)) {
+                if ( ! record.get().data().equals(rotationName)) {
                     nameService.updateRecord(record.get().id(), rotationName);
                     log.info("Updated mapping for record ID " + record.get().id().asString() + ": '" + dnsName
                              + "' -> '" + rotation.name() + "'");
@@ -476,15 +478,6 @@ public class ApplicationController {
         }
     }
 
-    /** Get an available rotation, if deploying to a production zone and a service ID is specified */
-    private Optional<Rotation> getRotation(Application application, ZoneId zone, RotationLock lock) {
-        if (zone.environment() != Environment.prod ||
-            !application.deploymentSpec().globalServiceId().isPresent()) {
-            return Optional.empty();
-        }
-        return Optional.of(rotationRepository.getRotation(application, lock));
-    }
-
     /** Returns the endpoints of the deployment, or empty if obtaining them failed */
     public Optional<InstanceEndpoints> getDeploymentEndpoints(DeploymentId deploymentId) {
         try {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationRotation.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationRotation.java
index e4aed04a01c..f0bf8f54447 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationRotation.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationRotation.java
@@ -13,10 +13,13 @@ import java.net.URI;
  */
 public class ApplicationRotation {
 
+    // TODO: TLS: Remove all non-secure stuff when all traffic is on HTTPS.
     public static final String DNS_SUFFIX = "global.vespa.yahooapis.com";
     private static final int port = 4080;
+    private static final int securePort = 4443;
 
     private final URI url;
+    private final URI secureUrl;
     private final RotationId id;
 
     public ApplicationRotation(ApplicationId application, RotationId id) {
@@ -25,6 +28,11 @@ public class ApplicationRotation {
                                             sanitize(application.tenant().value()),
                                             DNS_SUFFIX,
                                             port));
+        this.secureUrl = URI.create(String.format("https://%s--%s.%s:%d",
+                                                  sanitize(application.application().value()),
+                                                  sanitize(application.tenant().value()),
+                                                  DNS_SUFFIX,
+                                                  securePort));
         this.id = id;
     }
 
@@ -38,11 +46,21 @@ public class ApplicationRotation {
         return url;
     }
 
+    /** HTTPS URL to this rotation */
+    public URI secureUrl() {
+        return secureUrl;
+    }
+
     /** DNS name for this rotation */
     public String dnsName() {
         return url.getHost();
     }
 
+    /** DNS name for this rotation */
+    public String secureDnsName() {
+        return secureUrl.getHost();
+    }
+
     /** Sanitize by translating '_' to '-' as the former is not allowed in a DNS name */
     private static String sanitize(String s) {
         return s.replace('_', '-');
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
index 0f957c58a47..180e8f90ae2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainer.java
@@ -13,7 +13,6 @@ import com.yahoo.vespa.hosted.controller.rotation.RotationRepository;
 
 import java.time.Duration;
 import java.util.Map;
-import java.util.Optional;
 import java.util.logging.Logger;
 
 /**
@@ -48,14 +47,14 @@ public class DnsMaintainer extends Maintainer {
     /** Remove DNS alias for unassigned rotation */
     private void removeDnsAlias(Rotation rotation) {
         // When looking up CNAME by data, the data must be a FQDN
-        Optional<Record> record = nameService.findRecord(Record.Type.CNAME, RecordData.fqdn(rotation.name()));
-        record.filter(DnsMaintainer::canUpdate)
-              .ifPresent(r -> {
-                  log.info(String.format("Removing DNS record %s (%s) because it points to the unassigned " +
-                                         "rotation %s (%s)", record.get().id().asString(),
-                                         record.get().name().asString(), rotation.id().asString(), rotation.name()));
-                  nameService.removeRecord(r.id());
-              });
+        nameService.findRecord(Record.Type.CNAME, RecordData.fqdn(rotation.name())).stream()
+                .filter(DnsMaintainer::canUpdate)
+                .forEach(record -> {
+                    log.info(String.format("Removing DNS record %s (%s) because it points to the unassigned " +
+                                           "rotation %s (%s)", record.id().asString(),
+                                           record.name().asString(), rotation.id().asString(), rotation.name()));
+                    nameService.removeRecord(record.id());
+                });
     }
 
     /** Returns whether we can update the given record */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java
index e966cff652b..bd5aef1ec3a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ConfigServerClientMock.java
@@ -16,7 +16,6 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerClient;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse;
-import com.yahoo.vespa.hosted.controller.api.rotation.Rotation;
 import com.yahoo.vespa.serviceview.bindings.ApplicationView;
 import com.yahoo.vespa.serviceview.bindings.ClusterView;
 import com.yahoo.vespa.serviceview.bindings.ServiceView;
@@ -76,7 +75,7 @@ public class ConfigServerClientMock extends AbstractComponent implements ConfigS
     
     @Override
     public PreparedApplication prepare(DeploymentId deployment, DeployOptions deployOptions, Set<String> rotationCnames,
-                                       Set<Rotation> rotations, byte[] content) {
+                                       Set<String> rotationNames, byte[] content) {
         lastPrepareVersion = deployOptions.vespaVersion.map(Version::new).orElse(null);
         if (prepareException != null) {
             RuntimeException prepareException = this.prepareException;
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 f8a22e53993..ceed52d2dad 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
@@ -620,12 +620,20 @@ public class ControllerTest {
                 .environment(Environment.prod)
                 .globalServiceId("foo")
                 .region("us-west-1")
-                .region("us-central-1") // Two deployments should result in DNS alias being registered once
+                .region("us-central-1") // Two deployments should result in each DNS alias being registered once
                 .build();
 
         tester.deployCompletely(application, applicationPackage);
-        assertEquals(1, tester.controllerTester().nameService().records().size());
+        assertEquals(2, tester.controllerTester().nameService().records().size());
+
         Optional<Record> record = tester.controllerTester().nameService().findRecord(
+                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
+                                                                                    );
+        assertTrue(record.isPresent());
+        assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString());
+        assertEquals("rotation-fqdn-01.", record.get().data().asString());
+
+       record = tester.controllerTester().nameService().findRecord(
                 Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
         );
         assertTrue(record.isPresent());
@@ -644,12 +652,20 @@ public class ControllerTest {
                     .environment(Environment.prod)
                     .globalServiceId("foo")
                     .region("us-west-1")
-                    .region("us-central-1") // Two deployments should result in DNS alias being registered once
+                    .region("us-central-1") // Two deployments should result in each DNS alias being registered once
                     .build();
 
             tester.deployCompletely(app1, applicationPackage);
-            assertEquals(1, tester.controllerTester().nameService().records().size());
+            assertEquals(2, tester.controllerTester().nameService().records().size());
+
             Optional<Record> record = tester.controllerTester().nameService().findRecord(
+                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
+                                                                                        );
+            assertTrue(record.isPresent());
+            assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString());
+            assertEquals("rotation-fqdn-01.", record.get().data().asString());
+
+            record = tester.controllerTester().nameService().findRecord(
                     Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
             );
             assertTrue(record.isPresent());
@@ -672,7 +688,12 @@ public class ControllerTest {
                                  .containsKey(new RotationId("rotation-id-01")));
             }
 
-            // Record remains
+            // Records remain
+            record = tester.controllerTester().nameService().findRecord(
+                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
+            );
+            assertTrue(record.isPresent());
+
             record = tester.controllerTester().nameService().findRecord(
                     Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
             );
@@ -689,13 +710,22 @@ public class ControllerTest {
                     .region("us-central-1")
                     .build();
             tester.deployCompletely(app2, applicationPackage);
-            assertEquals(2, tester.controllerTester().nameService().records().size());
+            assertEquals(4, tester.controllerTester().nameService().records().size());
+
             Optional<Record> record = tester.controllerTester().nameService().findRecord(
+                    Record.Type.CNAME, RecordName.from("app2--tenant2.global.vespa.yahooapis.com")
+                                                                                        );
+            assertTrue(record.isPresent());
+            assertEquals("app2--tenant2.global.vespa.yahooapis.com", record.get().name().asString());
+            assertEquals("rotation-fqdn-01.", record.get().data().asString());
+
+            record = tester.controllerTester().nameService().findRecord(
                     Record.Type.CNAME, RecordName.from("app2.tenant2.global.vespa.yahooapis.com")
             );
             assertTrue(record.isPresent());
             assertEquals("app2.tenant2.global.vespa.yahooapis.com", record.get().name().asString());
             assertEquals("rotation-fqdn-01.", record.get().data().asString());
+
         }
 
         // Application 1 is recreated, deployed and assigned a new rotation
@@ -711,13 +741,21 @@ public class ControllerTest {
             app1 = tester.applications().require(app1.id());
             assertEquals("rotation-id-02", app1.rotation().get().id().asString());
 
-            // Existing DNS record is updated to point to the newly assigned rotation
-            assertEquals(2, tester.controllerTester().nameService().records().size());
+            // Existing DNS records are updated to point to the newly assigned rotation
+            assertEquals(4, tester.controllerTester().nameService().records().size());
+
             Optional<Record> record = tester.controllerTester().nameService().findRecord(
+                    Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
+                                                                                        );
+            assertTrue(record.isPresent());
+            assertEquals("rotation-fqdn-02.", record.get().data().asString());
+
+            record = tester.controllerTester().nameService().findRecord(
                     Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
             );
             assertTrue(record.isPresent());
             assertEquals("rotation-fqdn-02.", record.get().data().asString());
+
         }
 
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
index 1890a3ca956..8aa92ec5fce 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DnsMaintainerTest.java
@@ -46,8 +46,16 @@ public class DnsMaintainerTest {
 
         // Deploy application
         tester.deployCompletely(application, applicationPackage);
-        assertEquals(1, tester.controllerTester().nameService().records().size());
+        assertEquals(2, tester.controllerTester().nameService().records().size());
+
         Optional<Record> record = tester.controllerTester().nameService().findRecord(
+                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")
+                                                                                    );
+        assertTrue(record.isPresent());
+        assertEquals("app1--tenant1.global.vespa.yahooapis.com", record.get().name().asString());
+        assertEquals("rotation-fqdn-01.", record.get().data().asString());
+
+        record = tester.controllerTester().nameService().findRecord(
                 Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")
         );
         assertTrue(record.isPresent());
@@ -57,6 +65,8 @@ public class DnsMaintainerTest {
         // DnsMaintainer does nothing
         dnsMaintainer.maintain();
         assertTrue("DNS record is not removed", tester.controllerTester().nameService().findRecord(
+                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")).isPresent());
+        assertTrue("DNS record is not removed", tester.controllerTester().nameService().findRecord(
                 Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")).isPresent());
 
         // Remove application
@@ -70,7 +80,10 @@ public class DnsMaintainerTest {
         tester.applications().deactivate(application, ZoneId.from(Environment.staging, RegionName.from("us-east-3")));
         tester.applications().deleteApplication(application.id(), Optional.of(new NToken("ntoken")));
 
-        // DnsMaintainer removes record
+        // DnsMaintainer removes records
+        dnsMaintainer.maintain();
+        assertFalse("DNS record removed", tester.controllerTester().nameService().findRecord(
+                Record.Type.CNAME, RecordName.from("app1--tenant1.global.vespa.yahooapis.com")).isPresent());
         dnsMaintainer.maintain();
         assertFalse("DNS record removed", tester.controllerTester().nameService().findRecord(
                 Record.Type.CNAME, RecordName.from("app1.tenant1.global.vespa.yahooapis.com")).isPresent());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/MockRoutingGenerator.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/MockRoutingGenerator.java
index 89e3219b17f..bc2da7287e1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/MockRoutingGenerator.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/routing/MockRoutingGenerator.java
@@ -16,6 +16,7 @@ public class MockRoutingGenerator implements RoutingGenerator {
     @Override
     public List<RoutingEndpoint> endpoints(DeploymentId deployment) {
         List<RoutingEndpoint> endpoints = new ArrayList<>();
+        // TODO: TLS: Update to HTTPS when ready.
         endpoints.add(new RoutingEndpoint("http://old-endpoint.vespa.yahooapis.com:4080", false));
         endpoints.add(new RoutingEndpoint("http://qrs-endpoint.vespa.yahooapis.com:4080", "host1", false));
         endpoints.add(new RoutingEndpoint("http://feeding-endpoint.vespa.yahooapis.com:4080", "host2", false));