Conflict resolved

8 files changed, 114 insertions, 30 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 b025a522580..28fa311b841 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
@@ -21,6 +21,10 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname;
 import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException;
 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.NoInstanceException;
@@ -41,10 +45,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobReport;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.application.SourceRevision;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsClient;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsException;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.maintenance.DeploymentExpirer;
 import com.yahoo.vespa.hosted.controller.persistence.ControllerDb;
@@ -270,10 +270,11 @@ public class ApplicationController {
     public ActivateResult deployApplication(ApplicationId applicationId, ZoneId zone,
                                             ApplicationPackage applicationPackage, DeployOptions options) {
         try (Lock lock = lock(applicationId)) {
-            // TODO: Shouldn't this go through the above method? Seems you can cheat the checks here ... ?
-            LockedApplication application = get(applicationId).map(application1 -> new LockedApplication(application1, lock)).orElse(new LockedApplication(
-                    new Application(applicationId), lock)
-                                                                                                                                    );
+            // Not ideal, but since we create on missing and return a result computed inside the lock,
+            // the lock-with-action methods cannot be used
+            LockedApplication application = get(applicationId)
+                    .map(app -> new LockedApplication(app, lock))
+                    .orElseGet(() -> new LockedApplication(new Application(applicationId), lock));
 
             // Determine what we are doing
             Version version;
@@ -504,31 +505,43 @@ public class ApplicationController {
     }
 
     /**
-     * Deletes the application with this id
+     * Deletes the the given application. All known instances of the applications will be deleted,
+     * including PR instances.
      * 
      * @throws IllegalArgumentException if the application has deployments or the caller is not authorized
-     * @throws NotExistsException if the application does not exist
+     * @throws NotExistsException if no instances of the application exist
      */
-    public void deleteApplication(ApplicationId id, Optional<NToken> token) {
-        if ( ! controller.applications().get(id).isPresent())
-            throw new NotExistsException("Could not delete application '" + id + "': Application not found");
+    public void deleteApplication(ApplicationId applicationId, Optional<NToken> token) {
+        // Find all instances of the application
+        List<ApplicationId> instances = controller.applications().asList(applicationId.tenant())
+                                                  .stream()
+                                                  .map(Application::id)
+                                                  .filter(id -> id.application().equals(applicationId.application()) &&
+                                                                id.tenant().equals(applicationId.tenant()))
+                                                  .collect(Collectors.toList());
+        if (instances.isEmpty()) {
+            throw new NotExistsException("Could not delete application '" + applicationId + "': Application not found");
+        }
 
-        lockOrThrow(id, application -> {
+        // TODO: Make this one transaction when database is moved to ZooKeeper
+        instances.forEach(id -> lockOrThrow(id, application -> {
             if ( ! application.deployments().isEmpty())
                 throw new IllegalArgumentException("Could not delete '" + application + "': It has active deployments");
-            
+
             Tenant tenant = controller.tenants().tenant(new TenantId(id.tenant().value())).get();
             if (tenant.isAthensTenant() && ! token.isPresent())
                 throw new IllegalArgumentException("Could not delete '" + application + "': No NToken provided");
 
-            // NB: Next 2 lines should have been one transaction
-            if (tenant.isAthensTenant())
+            // Only delete in Athenz once
+            if (id.instance().isDefault() && tenant.isAthensTenant()) {
                 zmsClientFactory.createZmsClientWithAuthorizedServiceToken(token.get())
-                        .deleteApplication(tenant.getAthensDomain().get(), new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value()));
+                                .deleteApplication(tenant.getAthensDomain().get(),
+                                                   new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value()));
+            }
             db.deleteApplication(id);
 
             log.info("Deleted " + application);
-        });
+        }));
     }
 
     /** 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzTrustStoreConfigurator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzTrustStoreConfigurator.java
new file mode 100644
index 00000000000..939a5667a36
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzTrustStoreConfigurator.java
@@ -0,0 +1,45 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.controller.athenz.filter;
+
+import com.google.inject.Inject;
+import com.yahoo.jdisc.http.ssl.SslTrustStoreConfigurator;
+import com.yahoo.jdisc.http.ssl.SslTrustStoreContext;
+import com.yahoo.vespa.hosted.controller.athenz.config.AthenzConfig;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+
+/**
+ * Load trust store with Athenz CA certificates
+ *
+ * @author bjorncs
+ */
+public class AthenzTrustStoreConfigurator implements SslTrustStoreConfigurator {
+
+    private final KeyStore trustStore;
+
+    @Inject
+    public AthenzTrustStoreConfigurator(AthenzConfig config) {
+        this.trustStore = createTrustStore(new File(config.athenzCaTrustStore()));
+    }
+
+    private static KeyStore createTrustStore(File trustStoreFile) {
+        try (FileInputStream in = new FileInputStream(trustStoreFile)) {
+            KeyStore trustStore = KeyStore.getInstance("JKS");
+            trustStore.load(in, "changeit".toCharArray());
+            return trustStore;
+        } catch (IOException | CertificateException | NoSuchAlgorithmException | KeyStoreException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    @Override
+    public void configure(SslTrustStoreContext context) {
+        context.updateTrustStore(trustStore);
+    }
+}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java
index ab653f48388..3a7a72ac8ae 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/impl/AthenzSslContextProviderImpl.java
@@ -58,14 +58,15 @@ public class AthenzSslContextProviderImpl implements AthenzSslContextProvider {
         try {
             AthenzIdentityCertificate identityCertificate = ztsClient.getIdentityCertificate();
             KeyStore keyStore = KeyStore.getInstance("JKS");
+            keyStore.load(null);
             keyStore.setKeyEntry("athenz-controller-key",
                                  identityCertificate.getPrivateKey(),
                                  new char[0],
                                  new Certificate[]{identityCertificate.getCertificate()});
-            KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("X509");
+            KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
             keyManagerFactory.init(keyStore, new char[0]);
             return keyManagerFactory.getKeyManagers();
-        } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException e) {
+        } catch (KeyStoreException | NoSuchAlgorithmException | UnrecoverableKeyException | CertificateException | IOException e) {
             throw new RuntimeException(e);
         }
     }
@@ -76,7 +77,7 @@ public class AthenzSslContextProviderImpl implements AthenzSslContextProvider {
             try (FileInputStream in = new FileInputStream(config.athenzCaTrustStore())) {
                 trustStore.load(in, "changeit".toCharArray());
             }
-            TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("X509");
+            TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
             trustManagerFactory.init(trustStore);
             return trustManagerFactory.getTrustManagers();
         } catch (CertificateException | IOException | KeyStoreException | NoSuchAlgorithmException e) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 5e0514de949..a7d072d1dae 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -379,10 +379,10 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
         // Rotation
         Cursor globalRotationsArray = object.setArray("globalRotations");
-        Map<String, RotationStatus> rotationHealthStatus = application.rotation()
-                           .map(rotation -> controller.getHealthStatus(rotation.dnsName()))
-                           .orElse(Collections.emptyMap());
-        application.rotation().ifPresent(rotation -> globalRotationsArray.addString(rotation.url().toString()));
+        application.rotation().ifPresent(rotation -> {
+            globalRotationsArray.addString(rotation.url().toString());
+            object.setString("rotationId", rotation.id().asString());
+        });
 
         // Deployments sorted according to deployment spec
         List<Deployment> deployments = controller.applications().deploymentTrigger()
@@ -395,8 +395,12 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
             deploymentObject.setString("environment", deployment.zone().environment().value());
             deploymentObject.setString("region", deployment.zone().region().value());
             deploymentObject.setString("instance", application.id().instance().value()); // pointless
-            if (application.rotation().isPresent())
+            if (application.rotation().isPresent()) {
+                Map<String, RotationStatus> rotationHealthStatus = application.rotation()
+                                                                              .map(rotation -> controller.getHealthStatus(rotation.dnsName()))
+                                                                              .orElse(Collections.emptyMap());
                 setRotationStatus(deployment, rotationHealthStatus, deploymentObject);
+            }
 
             if (recurseOverDeployments(request)) // List full deployment information when recursive.
                 toSlime(deploymentObject, new DeploymentId(application.id(), deployment.zone()), deployment, request);
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 4ed1edad57f..17801bde546 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
@@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService.BuildJob;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
@@ -31,7 +32,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
 import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.BuildSystem;
@@ -298,10 +298,14 @@ public class ControllerTest {
 
         // staging deployment
         long app1ProjectId = 22;
-        ApplicationId app1 = tester.createAndDeploy("tenant1",  "domain1", "application1", Environment.staging, app1ProjectId).id();
+        ApplicationId app1 = tester.createAndDeploy("tenant1",  "domain1",
+                                                    "application1", Environment.staging,
+                                                    app1ProjectId).id();
         
         // pull-request deployment - uses different instance id
-        ApplicationId app1pr = tester.createAndDeploy("tenant1",  "domain1", "application1", "default-pr1", Environment.staging, app1ProjectId, null).id();
+        ApplicationId app1pr = tester.createAndDeploy("tenant1",  "domain1",
+                                                      "application1", "default-pr1",
+                                                      Environment.staging, app1ProjectId, null).id();
 
         assertTrue(applications.get(app1).isPresent());
         assertEquals(app1, applications.get(app1).get().id());
@@ -316,6 +320,20 @@ public class ControllerTest {
         assertEquals(app1, applications.get(app1).get().id());
         assertTrue(applications.get(app1pr).isPresent());
         assertEquals(app1pr, applications.get(app1pr).get().id());
+
+        // Deleting application also removes PR instance
+        ApplicationId app2 = tester.createAndDeploy("tenant1",  "domain1",
+                                                    "application2", Environment.staging,
+                                                    33).id();
+        tester.controller().applications().deleteApplication(app1, Optional.of(new NToken("ntoken")));
+        assertEquals("All instances deleted", 0,
+                     tester.controller().applications().asList(app1.tenant()).stream()
+                                                    .filter(app -> app.id().application().equals(app1.application()))
+                                                    .count());
+        assertEquals("Other application survives", 1,
+                     tester.controller().applications().asList(app1.tenant()).stream()
+                           .filter(app -> app.id().application().equals(app2.application()))
+                           .count());
     }
     
     @Test
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
index 961e005bfbd..e46c755e8bf 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application-without-change-multiple-deployments.json
@@ -207,6 +207,7 @@
   "globalRotations": [
     "http://application1.tenant1.global.vespa.yahooapis.com:4080/"
   ],
+  "rotationId": "rotation-id-1",
   "instances": [
     {
       "environment": "prod",
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
index 3924cf51ca9..b5ed2d407df 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application.json
@@ -148,6 +148,7 @@
   "globalRotations": [
     "http://application1.tenant1.global.vespa.yahooapis.com:4080/"
   ],
+  "rotationId": "rotation-id-1",
   "instances": [
     {
       "environment": "dev",
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
index 5030fc7d0a6..caca0ad8970 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/application1-recursive.json
@@ -148,6 +148,7 @@
   "globalRotations": [
     "http://application1.tenant1.global.vespa.yahooapis.com:4080/"
   ],
+  "rotationId": "rotation-id-1",
   "instances": [
     @include(dev-us-west-1.json),
     @include(prod-corp-us-east-1.json)