Merge pull request #10251 from vespa-engine/mpolden/always-provision-certificate

Always provision certificate on deploy

3 files changed, 56 insertions, 15 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 bbe766b86de..4ad35f20c39 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
@@ -298,7 +298,7 @@ public class ApplicationController {
             ApplicationPackage applicationPackage;
             Set<String> legacyRotations = new LinkedHashSet<>();
             Set<ContainerEndpoint> endpoints = new LinkedHashSet<>();
-            ApplicationCertificate applicationCertificate;
+            Optional<ApplicationCertificate> applicationCertificate;
 
             try (Lock lock = lock(applicationId)) {
                 LockedApplication application = new LockedApplication(require(applicationId), lock);
@@ -365,10 +365,8 @@ public class ApplicationController {
                     app.rotations().stream().map(RotationId::asString).forEach(legacyRotations::add);
                 }
 
-
                 // Get application certificate (provisions a new certificate if missing)
-                application = withApplicationCertificate(application);
-                applicationCertificate = application.get().applicationCertificate().orElse(null);
+                applicationCertificate = getApplicationCertificate(application.get());
 
                 // Update application with information from application package
                 if (   ! preferOldestVersion
@@ -380,11 +378,13 @@ public class ApplicationController {
 
             // Carry out deployment without holding the application lock.
             options = withVersion(platformVersion, options);
-            ActivateResult result = deploy(applicationId, applicationPackage, zone, options, legacyRotations, endpoints, applicationCertificate);
+            ActivateResult result = deploy(applicationId, applicationPackage, zone, options, legacyRotations, endpoints,
+                                           applicationCertificate.orElse(null));
 
             lockOrThrow(applicationId, application ->
                     store(application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant(),
-                                                        warningsFrom(result))));
+                                                        warningsFrom(result))
+                                     .withApplicationCertificate(applicationCertificate)));
             return result;
         }
     }
@@ -534,16 +534,18 @@ public class ApplicationController {
         });
     }
 
-    private LockedApplication withApplicationCertificate(LockedApplication application) {
-        ApplicationId applicationId = application.get().id();
-
+    private Optional<ApplicationCertificate> getApplicationCertificate(Application application) {
+        // Re-use certificate if already provisioned
+        if (application.applicationCertificate().isPresent()) {
+            return application.applicationCertificate();
+        }
         // TODO(tokle): Verify that the application is deploying to a zone where certificate provisioning is enabled
-        boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
-        if (provisionCertificate) {
-            application = application.withApplicationCertificate(
-                    Optional.of(applicationCertificateProvider.requestCaSignedCertificate(applicationId)));
+        boolean provisionCertificate = provisionApplicationCertificate.with(FetchVector.Dimension.APPLICATION_ID,
+                                                                            application.id().serializedForm()).value();
+        if (!provisionCertificate) {
+            return Optional.empty();
         }
-        return application;
+        return Optional.of(applicationCertificateProvider.requestCaSignedCertificate(application.id()));
     }
 
     private ActivateResult unexpectedDeployment(ApplicationId application, ZoneId zone) {
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 f6135cd246c..f226fe9e4e3 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
@@ -19,6 +19,7 @@ import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
+import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.SourceRevision;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
@@ -703,6 +704,36 @@ public class ControllerTest {
                                      .metrics().warnings().get(DeploymentMetrics.Warning.all).intValue());
     }
 
+    @Test
+    public void testDeployProvisionsCertificate() {
+        ((InMemoryFlagSource) tester.controller().flagSource()).withBooleanFlag(Flags.PROVISION_APPLICATION_CERTIFICATE.id(), true);
+        Function<Application, Optional<ApplicationCertificate>> certificate = (application) -> tester.application(application.id()).applicationCertificate();
+
+        // Create app1
+        var app1 = tester.createApplication("app1", "tenant1", 1, 2L);
+        var applicationPackage = new ApplicationPackageBuilder().environment(Environment.prod)
+                                                                               .region("us-west-1")
+                                                                               .build();
+        // Deploy app1 in production
+        tester.deployCompletely(app1, applicationPackage);
+        var cert = certificate.apply(app1);
+        assertTrue("Provisions certificate in " + Environment.prod, cert.isPresent());
+
+        // Next deployment reuses certificate
+        tester.deployCompletely(app1, applicationPackage, BuildJob.defaultBuildNumber + 1);
+        assertEquals(cert, certificate.apply(app1));
+
+        // Create app2
+        var app2 = tester.createApplication("app2", "tenant2", 3, 4L);
+        ZoneId zone = ZoneId.from("dev", "us-east-1");
+
+        // Deploy app2 in dev
+        tester.controller().applications().deploy(app2.id(), zone, Optional.of(applicationPackage), DeployOptions.none());
+        assertTrue("Application deployed and activated",
+                   tester.controllerTester().configServer().application(app2.id()).get().activated());
+        assertTrue("Provisions certificate in " + Environment.dev, certificate.apply(app2).isPresent());
+    }
+
     private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) {
         Version next = Version.fromString("6.2");
         tester.upgradeSystem(next);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java
index 3246a260217..f3bee70db4c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ApplicationCertificateMock.java
@@ -5,10 +5,18 @@ import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider;
 
+import java.util.UUID;
+
+/**
+ * @author tokle
+ */
 public class ApplicationCertificateMock implements ApplicationCertificateProvider {
 
     @Override
     public ApplicationCertificate requestCaSignedCertificate(ApplicationId applicationId) {
-        return new ApplicationCertificate(String.format("vespa.tls.%s.%s", applicationId.tenant(),applicationId.application()));
+        return new ApplicationCertificate(String.format("vespa.tls.%s.%s@%s", applicationId.tenant(),
+                                                        applicationId.application(),
+                                                        UUID.randomUUID().toString()));
     }
+
 }